From 06b299e62733ef06f98204266cd151949deaf149 Mon Sep 17 00:00:00 2001 From: Dag Date: Wed, 24 Jan 2024 23:06:23 +0100 Subject: [PATCH] refactor: prepare for introduction of token based authentication (#3921) --- actions/DisplayAction.php | 8 +- actions/SetBridgeCacheAction.php | 25 +++- index.php | 44 +++++- lib/ApiAuthenticationMiddleware.php | 40 ------ lib/AuthenticationMiddleware.php | 39 ----- lib/BridgeCard.php | 212 +++++++++------------------- lib/RssBridge.php | 89 +++++++----- lib/bootstrap.php | 15 -- lib/html.php | 2 +- tests/BridgeCardTest.php | 57 ++++++++ 10 files changed, 240 insertions(+), 291 deletions(-) delete mode 100644 lib/ApiAuthenticationMiddleware.php delete mode 100644 lib/AuthenticationMiddleware.php create mode 100644 tests/BridgeCardTest.php diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php index 080da52e..3ad8495f 100644 --- a/actions/DisplayAction.php +++ b/actions/DisplayAction.php @@ -13,13 +13,6 @@ class DisplayAction implements ActionInterface public function execute(array $request) { - if (Configuration::getConfig('system', 'enable_maintenance_mode')) { - return new Response(render(__DIR__ . '/../templates/error.html.php', [ - 'title' => '503 Service Unavailable', - 'message' => 'RSS-Bridge is down for maintenance.', - ]), 503); - } - $cacheKey = 'http_' . json_encode($request); /** @var Response $cachedResponse */ $cachedResponse = $this->cache->get($cacheKey); @@ -118,6 +111,7 @@ class DisplayAction implements ActionInterface } $feed = $bridge->getFeed(); } catch (\Exception $e) { + // Probably an exception inside a bridge if ($e instanceof HttpException) { // Reproduce (and log) these responses regardless of error output and report limit if ($e->getCode() === 429) { diff --git a/actions/SetBridgeCacheAction.php b/actions/SetBridgeCacheAction.php index 2e9d7147..e4d245df 100644 --- a/actions/SetBridgeCacheAction.php +++ b/actions/SetBridgeCacheAction.php @@ -11,9 +11,30 @@ class SetBridgeCacheAction implements ActionInterface public function execute(array $request) { - $authenticationMiddleware = new ApiAuthenticationMiddleware(); - $authenticationMiddleware($request); + // Authentication + $accessTokenInConfig = Configuration::getConfig('authentication', 'access_token'); + if (!$accessTokenInConfig) { + return new Response('Access token is not set in this instance', 403, ['content-type' => 'text/plain']); + } + if (isset($request['access_token'])) { + $accessTokenGiven = $request['access_token']; + } else { + $header = trim($_SERVER['HTTP_AUTHORIZATION'] ?? ''); + $position = strrpos($header, 'Bearer '); + if ($position !== false) { + $accessTokenGiven = substr($header, $position + 7); + } else { + $accessTokenGiven = ''; + } + } + if (!$accessTokenGiven) { + return new Response('No access token given', 403, ['content-type' => 'text/plain']); + } + if (! hash_equals($accessTokenInConfig, $accessTokenGiven)) { + return new Response('Incorrect access token', 403, ['content-type' => 'text/plain']); + } + // Begin actual work $key = $request['key'] ?? null; if (!$key) { returnClientError('You must specify key!'); diff --git a/index.php b/index.php index 126200da..029b673c 100644 --- a/index.php +++ b/index.php @@ -1,18 +1,22 @@ $e]), 500); + $response->send(); RssBridge::getLogger()->error('Uncaught Exception', ['e' => $e]); - http_response_code(500); - exit(render(__DIR__ . '/templates/exception.html.php', ['e' => $e])); }); set_error_handler(function ($code, $message, $file, $line) { if ((error_reporting() & $code) === 0) { + // Deprecation messages and other masked errors are typically ignored here return false; } // In the future, uncomment this: @@ -39,11 +43,37 @@ register_shutdown_function(function () { ); RssBridge::getLogger()->error($message); if (Debug::isEnabled()) { + // This output can interfere with json output etc + // This output is written at the bottom print sprintf("
%s
\n", e($message)); } } }); -$rssBridge = new RssBridge(); +$errors = Configuration::checkInstallation(); +if ($errors) { + http_response_code(500); + print '
' . implode("\n", $errors) . '
'; + exit; +} -$rssBridge->main($argv ?? []); +$customConfig = []; +if (file_exists(__DIR__ . '/config.ini.php')) { + $customConfig = parse_ini_file(__DIR__ . '/config.ini.php', true, INI_SCANNER_TYPED); +} +Configuration::loadConfiguration($customConfig, getenv()); + +// Consider: ini_set('error_reporting', E_ALL & ~E_DEPRECATED); + +date_default_timezone_set(Configuration::getConfig('system', 'timezone')); + +try { + $rssBridge = new RssBridge(); + $response = $rssBridge->main($argv ?? []); + $response->send(); +} catch (\Throwable $e) { + // Probably an exception inside an action + RssBridge::getLogger()->error('Exception in RssBridge::main()', ['e' => $e]); + http_response_code(500); + print render(__DIR__ . '/templates/exception.html.php', ['e' => $e]); +} diff --git a/lib/ApiAuthenticationMiddleware.php b/lib/ApiAuthenticationMiddleware.php deleted file mode 100644 index 62886314..00000000 --- a/lib/ApiAuthenticationMiddleware.php +++ /dev/null @@ -1,40 +0,0 @@ -exit('Access token is not set in this instance', 403); - } - - if (isset($request['access_token'])) { - $accessTokenGiven = $request['access_token']; - } else { - $header = trim($_SERVER['HTTP_AUTHORIZATION'] ?? ''); - $position = strrpos($header, 'Bearer '); - - if ($position !== false) { - $accessTokenGiven = substr($header, $position + 7); - } else { - $accessTokenGiven = ''; - } - } - - if (!$accessTokenGiven) { - $this->exit('No access token given', 403); - } - - if ($accessTokenGiven != $accessTokenInConfig) { - $this->exit('Incorrect access token', 403); - } - } - - private function exit($message, $code) - { - http_response_code($code); - header('content-type: text/plain'); - die($message); - } -} diff --git a/lib/AuthenticationMiddleware.php b/lib/AuthenticationMiddleware.php deleted file mode 100644 index a91420f8..00000000 --- a/lib/AuthenticationMiddleware.php +++ /dev/null @@ -1,39 +0,0 @@ -renderAuthenticationDialog(); - exit; - } - if ( - Configuration::getConfig('authentication', 'username') === $user - && Configuration::getConfig('authentication', 'password') === $password - ) { - return; - } - print $this->renderAuthenticationDialog(); - exit; - } - - private function renderAuthenticationDialog(): string - { - http_response_code(401); - header('WWW-Authenticate: Basic realm="RSS-Bridge"'); - return render(__DIR__ . '/../templates/error.html.php', [ - 'message' => 'Please authenticate in order to access this instance!', - ]); - } -} diff --git a/lib/BridgeCard.php b/lib/BridgeCard.php index a82f8e5a..8dc2472a 100644 --- a/lib/BridgeCard.php +++ b/lib/BridgeCard.php @@ -23,6 +23,7 @@ final class BridgeCard $icon = $bridge->getIcon(); $description = $bridge->getDescription(); $parameters = $bridge->getParameters(); + if (Configuration::getConfig('proxy', 'url') && Configuration::getConfig('proxy', 'by_bridge')) { $parameters['global']['_noproxy'] = [ 'name' => 'Disable proxy (' . (Configuration::getConfig('proxy', 'name') ?: Configuration::getConfig('proxy', 'url')) . ')', @@ -93,32 +94,6 @@ CARD; return $card; } - /** - * Get the form header for a bridge card - * - * @param class-string $bridgeClassName The bridge name - * @param bool $isHttps If disabled, adds a warning to the form - * @return string The form header - */ - private static function getFormHeader($bridgeClassName, $isHttps = false, $parameterName = '') - { - $form = << - - -EOD; - - if (!empty($parameterName)) { - $form .= sprintf('', $parameterName); - } - - if (!$isHttps) { - $form .= '
Warning: This bridge is not fetching its content through a secure connection
'; - } - - return $form; - } - /** * Get the form body for a bridge * @@ -152,19 +127,10 @@ EOD; $inputEntry['defaultValue'] = ''; } - $idArg = 'arg-' - . urlencode($bridgeClassName) - . '-' - . urlencode($parameterName) - . '-' - . urlencode($id); + $idArg = 'arg-' . urlencode($bridgeClassName) . '-' . urlencode($parameterName) . '-' . urlencode($id); - $form .= '' - . PHP_EOL; + $inputName = filter_var($inputEntry['name'], FILTER_SANITIZE_FULL_SPECIAL_CHARS); + $form .= '' . PHP_EOL; if (!isset($inputEntry['type']) || $inputEntry['type'] === 'text') { $form .= self::getTextInput($inputEntry, $idArg, $id); @@ -206,96 +172,59 @@ EOD; } /** - * Get input field attributes + * Get the form header for a bridge card * - * @param array $entry The current entry - * @return string The input field attributes + * @param class-string $bridgeClassName The bridge name + * @param bool $isHttps If disabled, adds a warning to the form + * @return string The form header */ - private static function getInputAttributes($entry) + private static function getFormHeader($bridgeClassName, $isHttps = false, $parameterName = '') { - $retVal = ''; + $form = << + + +EOD; - if (isset($entry['required']) && $entry['required'] === true) { - $retVal .= ' required'; + if (!empty($parameterName)) { + $form .= sprintf('', $parameterName); } - if (isset($entry['pattern'])) { - $retVal .= ' pattern="' . $entry['pattern'] . '"'; + if (!$isHttps) { + $form .= '
Warning: This bridge is not fetching its content through a secure connection
'; } - return $retVal; + return $form; } - /** - * Get text input - * - * @param array $entry The current entry - * @param string $id The field ID - * @param string $name The field name - * @return string The text input field - */ - private static function getTextInput($entry, $id, $name) + public static function getTextInput(array $entry, string $id, string $name): string { - return '' - . PHP_EOL; + $defaultValue = filter_var($entry['defaultValue'], FILTER_SANITIZE_FULL_SPECIAL_CHARS); + $exampleValue = filter_var($entry['exampleValue'], FILTER_SANITIZE_FULL_SPECIAL_CHARS); + $attributes = self::getInputAttributes($entry); + + return sprintf('' . "\n", $attributes, $id, $defaultValue, $exampleValue, $name); } - /** - * Get number input - * - * @param array $entry The current entry - * @param string $id The field ID - * @param string $name The field name - * @return string The number input field - */ - private static function getNumberInput($entry, $id, $name) + public static function getNumberInput(array $entry, string $id, string $name): string { - return '' - . PHP_EOL; + $defaultValue = filter_var($entry['defaultValue'], FILTER_SANITIZE_NUMBER_INT); + $exampleValue = filter_var($entry['exampleValue'], FILTER_SANITIZE_NUMBER_INT); + $attributes = self::getInputAttributes($entry); + + return sprintf('' . "\n", $attributes, $id, $defaultValue, $exampleValue, $name); } - /** - * Get list input - * - * @param array $entry The current entry - * @param string $id The field ID - * @param string $name The field name - * @return string The list input field - */ - private static function getListInput($entry, $id, $name) + public static function getListInput(array $entry, string $id, string $name): string { - if (isset($entry['required']) && $entry['required'] === true) { + $required = $entry['required'] ?? null; + if ($required) { Debug::log('The "required" attribute is not supported for lists.'); unset($entry['required']); } - $list = '', $attributes, $id, $name); foreach ($entry['values'] as $name => $value) { if (is_array($value)) { @@ -305,17 +234,9 @@ EOD; $entry['defaultValue'] === $subname || $entry['defaultValue'] === $subvalue ) { - $list .= ''; + $list .= ''; } else { - $list .= ''; + $list .= ''; } } $list .= ''; @@ -324,17 +245,9 @@ EOD; $entry['defaultValue'] === $name || $entry['defaultValue'] === $value ) { - $list .= ''; + $list .= ''; } else { - $list .= ''; + $list .= ''; } } } @@ -344,30 +257,35 @@ EOD; return $list; } - /** - * Get checkbox input - * - * @param array $entry The current entry - * @param string $id The field ID - * @param string $name The field name - * @return string The checkbox input field - */ - private static function getCheckboxInput($entry, $id, $name) + + public static function getCheckboxInput(array $entry, string $id, string $name): string { - if (isset($entry['required']) && $entry['required'] === true) { + $required = $entry['required'] ?? null; + if ($required) { Debug::log('The "required" attribute is not supported for checkboxes.'); unset($entry['required']); } - return '' - . PHP_EOL; + $checked = $entry['defaultValue'] === 'checked' ? 'checked' : ''; + $attributes = self::getInputAttributes($entry); + + return sprintf('' . "\n", $attributes, $id, $name, $checked); + } + + public static function getInputAttributes(array $entry): string + { + $result = ''; + + $required = $entry['required'] ?? null; + if ($required) { + $result .= ' required'; + } + + $pattern = $entry['pattern'] ?? null; + if ($pattern) { + $result .= ' pattern="' . $pattern . '"'; + } + + return $result; } } diff --git a/lib/RssBridge.php b/lib/RssBridge.php index d56c59b8..5938f824 100644 --- a/lib/RssBridge.php +++ b/lib/RssBridge.php @@ -23,48 +23,71 @@ final class RssBridge } } - public function main(array $argv = []): void + public function main(array $argv = []): Response { if ($argv) { parse_str(implode('&', array_slice($argv, 1)), $cliArgs); $request = $cliArgs; } else { - if (Configuration::getConfig('authentication', 'enable')) { - $authenticationMiddleware = new AuthenticationMiddleware(); - $authenticationMiddleware(); - } $request = array_merge($_GET, $_POST); } - try { - foreach ($request as $key => $value) { - if (!is_string($value)) { - throw new \Exception("Query parameter \"$key\" is not a string."); - } - } - - $actionName = $request['action'] ?? 'Frontpage'; - $actionName = strtolower($actionName) . 'Action'; - $actionName = implode(array_map('ucfirst', explode('-', $actionName))); - - $filePath = __DIR__ . '/../actions/' . $actionName . '.php'; - if (!file_exists($filePath)) { - throw new \Exception('Invalid action', 400); - } - $className = '\\' . $actionName; - $action = new $className(); - - $response = $action->execute($request); - if (is_string($response)) { - print $response; - } elseif ($response instanceof Response) { - $response->send(); - } - } catch (\Throwable $e) { - self::$logger->error('Exception in RssBridge::main()', ['e' => $e]); - http_response_code(500); - print render(__DIR__ . '/../templates/exception.html.php', ['e' => $e]); + if (Configuration::getConfig('system', 'enable_maintenance_mode')) { + return new Response(render(__DIR__ . '/../templates/error.html.php', [ + 'title' => '503 Service Unavailable', + 'message' => 'RSS-Bridge is down for maintenance.', + ]), 503); } + + if (Configuration::getConfig('authentication', 'enable')) { + if (Configuration::getConfig('authentication', 'password') === '') { + return new Response('The authentication password cannot be the empty string', 500); + } + $user = $_SERVER['PHP_AUTH_USER'] ?? null; + $password = $_SERVER['PHP_AUTH_PW'] ?? null; + if ($user === null || $password === null) { + $html = render(__DIR__ . '/../templates/error.html.php', [ + 'message' => 'Please authenticate in order to access this instance!', + ]); + return new Response($html, 401, ['WWW-Authenticate' => 'Basic realm="RSS-Bridge"']); + } + if ( + (Configuration::getConfig('authentication', 'username') !== $user) + || (! hash_equals(Configuration::getConfig('authentication', 'password'), $password)) + ) { + $html = render(__DIR__ . '/../templates/error.html.php', [ + 'message' => 'Please authenticate in order to access this instance!', + ]); + return new Response($html, 401, ['WWW-Authenticate' => 'Basic realm="RSS-Bridge"']); + } + // At this point the username and password was correct + } + + foreach ($request as $key => $value) { + if (!is_string($value)) { + return new Response(render(__DIR__ . '/../templates/error.html.php', [ + 'message' => "Query parameter \"$key\" is not a string.", + ]), 400); + } + } + + $actionName = $request['action'] ?? 'Frontpage'; + $actionName = strtolower($actionName) . 'Action'; + $actionName = implode(array_map('ucfirst', explode('-', $actionName))); + $filePath = __DIR__ . '/../actions/' . $actionName . '.php'; + if (!file_exists($filePath)) { + return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Invalid action']), 400); + } + + $className = '\\' . $actionName; + $action = new $className(); + + $response = $action->execute($request); + + if (is_string($response)) { + $response = new Response($response); + } + return $response; } public static function getCache(): CacheInterface diff --git a/lib/bootstrap.php b/lib/bootstrap.php index 9a69d756..01828f67 100644 --- a/lib/bootstrap.php +++ b/lib/bootstrap.php @@ -1,9 +1,5 @@ ' . implode("\n", $errors) . ''); -} - -$customConfig = []; -if (file_exists(__DIR__ . '/../config.ini.php')) { - $customConfig = parse_ini_file(__DIR__ . '/../config.ini.php', true, INI_SCANNER_TYPED); -} -Configuration::loadConfiguration($customConfig, getenv()); diff --git a/lib/html.php b/lib/html.php index d65d1b20..eeaf2b32 100644 --- a/lib/html.php +++ b/lib/html.php @@ -36,7 +36,7 @@ function render(string $template, array $context = []): string /** * Render php template with context * - * DO NOT PASS USER INPUT IN $template or $context + * DO NOT PASS USER INPUT IN $template OR $context (keys!) */ function render_template(string $template, array $context = []): string { diff --git a/tests/BridgeCardTest.php b/tests/BridgeCardTest.php new file mode 100644 index 00000000..7a8f084d --- /dev/null +++ b/tests/BridgeCardTest.php @@ -0,0 +1,57 @@ +assertSame('', BridgeCard::getInputAttributes([])); + $this->assertSame(' required pattern="\d+"', BridgeCard::getInputAttributes(['required' => true, 'pattern' => '\d+'])); + + $entry = [ + 'defaultValue' => 'checked', + ]; + $this->assertSame('' . "\n", BridgeCard::getCheckboxInput($entry, 'id', 'name')); + + $entry = [ + 'defaultValue' => 42, + 'exampleValue' => 43, + ]; + $this->assertSame('' . "\n", BridgeCard::getNumberInput($entry, 'id', 'name')); + + $entry = [ + 'defaultValue' => 'yo1', + 'exampleValue' => 'yo2', + ]; + $this->assertSame('' . "\n", BridgeCard::getTextInput($entry, 'id', 'name')); + + $entry = [ + 'values' => [], + ]; + $this->assertSame('', BridgeCard::getListInput($entry, 'id', 'name')); + + $entry = [ + 'defaultValue' => 2, + 'values' => [ + 'foo' => 'bar', + ], + ]; + $this->assertSame('', BridgeCard::getListInput($entry, 'id', 'name')); + + // optgroup + $entry = [ + 'defaultValue' => 2, + 'values' => ['kek' => [ + 'f' => 'b', + ]], + ]; + $this->assertSame('', BridgeCard::getListInput($entry, 'id', 'name')); + } +} \ No newline at end of file