refactor: prepare for introduction of token based authentication (#3921)

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) {

actions/SetBridgeCacheAction.php

@@ -11,9 +11,30 @@ class SetBridgeCacheAction implements ActionInterface
 
     public function execute(array $request)
     {
-        $authenticationMiddleware = new ApiAuthenticationMiddleware();
+        // Authentication
-        $authenticationMiddleware($request);
+        $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!');

index.php

@@ -1,18 +1,22 @@
 <?php
 
+if (version_compare(\PHP_VERSION, '7.4.0') === -1) {
+    http_response_code(500);
+    print 'RSS-Bridge requires minimum PHP version 7.4';
+    exit;
+}
+
 require_once __DIR__ . '/lib/bootstrap.php';
 
-// Consider: ini_set('error_reporting', E_ALL & ~E_DEPRECATED);
-date_default_timezone_set(Configuration::getConfig('system', 'timezone'));
-
 set_exception_handler(function (\Throwable $e) {
+    $response = new Response(render(__DIR__ . '/templates/exception.html.php', ['e' => $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("<pre>%s</pre>\n", e($message));
         }
     }
 });
 
-$rssBridge = new RssBridge();
+$errors = Configuration::checkInstallation();
+if ($errors) {
+    http_response_code(500);
+    print '<pre>' . implode("\n", $errors) . '</pre>';
+    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]);
+}

lib/ApiAuthenticationMiddleware.php

@@ -1,40 +0,0 @@
-<?php
-
-final class ApiAuthenticationMiddleware
-{
-    public function __invoke($request): void
-    {
-        $accessTokenInConfig = Configuration::getConfig('authentication', 'access_token');
-        if (!$accessTokenInConfig) {
-            $this->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);
-    }
-}

lib/AuthenticationMiddleware.php

@@ -1,39 +0,0 @@
-<?php
-
-final class AuthenticationMiddleware
-{
-    public function __construct()
-    {
-        if (Configuration::getConfig('authentication', 'password') === '') {
-            throw new \Exception('The authentication password cannot be the empty string');
-        }
-    }
-
-    public function __invoke(): void
-    {
-        $user = $_SERVER['PHP_AUTH_USER'] ?? null;
-        $password = $_SERVER['PHP_AUTH_PW'] ?? null;
-
-        if ($user === null || $password === null) {
-            print $this->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!',
-        ]);
-    }
-}

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<BridgeAbstract> $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
-            <form method="GET" action="?">
-                <input type="hidden" name="action" value="display" />
-                <input type="hidden" name="bridge" value="{$bridgeClassName}" />
-EOD;
-
-        if (!empty($parameterName)) {
-            $form .= sprintf('<input type="hidden" name="context" value="%s" />', $parameterName);
-        }
-
-        if (!$isHttps) {
-            $form .= '<div class="secure-warning">Warning: This bridge is not fetching its content through a secure connection</div>';
-        }
-
-        return $form;
-    }
-
     /**
      * Get the form body for a bridge
      *
@@ -152,19 +127,10 @@ EOD;
                     $inputEntry['defaultValue'] = '';
                 }
 
-                $idArg = 'arg-'
+                $idArg = 'arg-' . urlencode($bridgeClassName) . '-' . urlencode($parameterName) . '-' . urlencode($id);
-                    . urlencode($bridgeClassName)
-                    . '-'
-                    . urlencode($parameterName)
-                    . '-'
-                    . urlencode($id);
 
-                $form .= '<label for="'
+                $inputName = filter_var($inputEntry['name'], FILTER_SANITIZE_FULL_SPECIAL_CHARS);
-                    . $idArg
+                $form .= '<label for="' . $idArg . '">' . $inputName . '</label>' . PHP_EOL;
-                    . '">'
-                    . filter_var($inputEntry['name'], FILTER_SANITIZE_FULL_SPECIAL_CHARS)
-                    . '</label>'
-                    . 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
+     * @param class-string<BridgeAbstract> $bridgeClassName The bridge name
-     * @return string The input field attributes
+     * @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
+            <form method="GET" action="?">
+                <input type="hidden" name="action" value="display" />
+                <input type="hidden" name="bridge" value="{$bridgeClassName}" />
+EOD;
 
-        if (isset($entry['required']) && $entry['required'] === true) {
+        if (!empty($parameterName)) {
-            $retVal .= ' required';
+            $form .= sprintf('<input type="hidden" name="context" value="%s" />', $parameterName);
         }
 
-        if (isset($entry['pattern'])) {
+        if (!$isHttps) {
-            $retVal .= ' pattern="' . $entry['pattern'] . '"';
+            $form .= '<div class="secure-warning">Warning: This bridge is not fetching its content through a secure connection</div>';
         }
 
-        return $retVal;
+        return $form;
     }
 
-    /**
+    public static function getTextInput(array $entry, string $id, string $name): string
-     * 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)
     {
-        return '<input '
+        $defaultValue = filter_var($entry['defaultValue'], FILTER_SANITIZE_FULL_SPECIAL_CHARS);
-        . self::getInputAttributes($entry)
+        $exampleValue = filter_var($entry['exampleValue'], FILTER_SANITIZE_FULL_SPECIAL_CHARS);
-        . ' id="'
+        $attributes = self::getInputAttributes($entry);
-        . $id
+
-        . '" type="text" value="'
+        return sprintf('<input %s id="%s" type="text" value="%s" placeholder="%s" name="%s" />' . "\n", $attributes, $id, $defaultValue, $exampleValue, $name);
-        . filter_var($entry['defaultValue'], FILTER_SANITIZE_FULL_SPECIAL_CHARS)
-        . '" placeholder="'
-        . filter_var($entry['exampleValue'], FILTER_SANITIZE_FULL_SPECIAL_CHARS)
-        . '" name="'
-        . $name
-        . '" />'
-        . PHP_EOL;
     }
 
-    /**
+    public static function getNumberInput(array $entry, string $id, string $name): string
-     * 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)
     {
-        return '<input '
+        $defaultValue = filter_var($entry['defaultValue'], FILTER_SANITIZE_NUMBER_INT);
-        . self::getInputAttributes($entry)
+        $exampleValue = filter_var($entry['exampleValue'], FILTER_SANITIZE_NUMBER_INT);
-        . ' id="'
+        $attributes = self::getInputAttributes($entry);
-        . $id
+
-        . '" type="number" value="'
+        return sprintf('<input %s id="%s" type="number" value="%s" placeholder="%s" name="%s" />' . "\n", $attributes, $id, $defaultValue, $exampleValue, $name);
-        . filter_var($entry['defaultValue'], FILTER_SANITIZE_NUMBER_INT)
-        . '" placeholder="'
-        . filter_var($entry['exampleValue'], FILTER_SANITIZE_NUMBER_INT)
-        . '" name="'
-        . $name
-        . '" />'
-        . PHP_EOL;
     }
 
-    /**
+    public static function getListInput(array $entry, string $id, string $name): string
-     * 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)
     {
-        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 = '<select '
+        $attributes = self::getInputAttributes($entry);
-        . self::getInputAttributes($entry)
+        $list = sprintf('<select %s id="%s" name="%s" >', $attributes, $id, $name);
-        . ' id="'
-        . $id
-        . '" name="'
-        . $name
-        . '" >';
 
         foreach ($entry['values'] as $name => $value) {
             if (is_array($value)) {
@@ -305,17 +234,9 @@ EOD;
                         $entry['defaultValue'] === $subname
                         || $entry['defaultValue'] === $subvalue
                     ) {
-                        $list .= '<option value="'
+                        $list .= '<option value="' . $subvalue . '" selected>' . $subname . '</option>';
-                            . $subvalue
-                            . '" selected>'
-                            . $subname
-                            . '</option>';
                     } else {
-                        $list .= '<option value="'
+                        $list .= '<option value="' . $subvalue . '">' . $subname . '</option>';
-                            . $subvalue
-                            . '">'
-                            . $subname
-                            . '</option>';
                     }
                 }
                 $list .= '</optgroup>';
@@ -324,17 +245,9 @@ EOD;
                     $entry['defaultValue'] === $name
                     || $entry['defaultValue'] === $value
                 ) {
-                    $list .= '<option value="'
+                    $list .= '<option value="' . $value . '" selected>' . $name . '</option>';
-                        . $value
-                        . '" selected>'
-                        . $name
-                        . '</option>';
                 } else {
-                    $list .= '<option value="'
+                    $list .= '<option value="' . $value . '">' . $name . '</option>';
-                        . $value
-                        . '">'
-                        . $name
-                        . '</option>';
                 }
             }
         }
@@ -344,30 +257,35 @@ EOD;
         return $list;
     }
 
-    /**
+
-     * Get checkbox input
+    public static function getCheckboxInput(array $entry, string $id, string $name): string
-     *
-     * @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)
     {
-        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 '<input '
+        $checked = $entry['defaultValue'] === 'checked' ? 'checked' : '';
-        . self::getInputAttributes($entry)
+        $attributes = self::getInputAttributes($entry);
-        . ' id="'
+
-        . $id
+        return sprintf('<input %s id="%s" type="checkbox" name="%s" %s />' . "\n", $attributes, $id, $name, $checked);
-        . '" type="checkbox" name="'
+    }
-        . $name
+
-        . '" '
+    public static function getInputAttributes(array $entry): string
-        . ($entry['defaultValue'] === 'checked' ? 'checked' : '')
+    {
-        . ' />'
+        $result = '';
-        . PHP_EOL;
+
+        $required = $entry['required'] ?? null;
+        if ($required) {
+            $result .= ' required';
+        }
+
+        $pattern = $entry['pattern'] ?? null;
+        if ($pattern) {
+            $result .= ' pattern="' . $pattern . '"';
+        }
+
+        return $result;
     }
 }

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 {
+        if (Configuration::getConfig('system', 'enable_maintenance_mode')) {
-            foreach ($request as $key => $value) {
+            return new Response(render(__DIR__ . '/../templates/error.html.php', [
-                if (!is_string($value)) {
+                'title'     => '503 Service Unavailable',
-                    throw new \Exception("Query parameter \"$key\" is not a string.");
+                'message'   => 'RSS-Bridge is down for maintenance.',
-                }
+            ]), 503);
-            }
-
-            $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('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

lib/bootstrap.php

@@ -1,9 +1,5 @@
 <?php
 
-if (version_compare(\PHP_VERSION, '7.4.0') === -1) {
-    exit('RSS-Bridge requires minimum PHP version 7.4.0!');
-}
-
 // Path to the formats library
 const PATH_LIB_FORMATS = __DIR__ . '/../formats/';
 
@@ -51,14 +47,3 @@ spl_autoload_register(function ($className) {
         }
     }
 });
-
-$errors = Configuration::checkInstallation();
-if ($errors) {
-    exit('<pre>' . implode("\n", $errors) . '</pre>');
-}
-
-$customConfig = [];
-if (file_exists(__DIR__ . '/../config.ini.php')) {
-    $customConfig = parse_ini_file(__DIR__ . '/../config.ini.php', true, INI_SCANNER_TYPED);
-}
-Configuration::loadConfiguration($customConfig, getenv());

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
 {

tests/BridgeCardTest.php

@@ -0,0 +1,57 @@
+<?php
+
+declare(strict_types=1);
+
+namespace RssBridge\Tests;
+
+use BridgeCard;
+use PHPUnit\Framework\TestCase;
+
+class BridgeCardTest extends TestCase
+{
+    public function test()
+    {
+        $sut = new BridgeCard();
+        $this->assertSame('', BridgeCard::getInputAttributes([]));
+        $this->assertSame(' required pattern="\d+"', BridgeCard::getInputAttributes(['required' => true, 'pattern' => '\d+']));
+
+        $entry = [
+            'defaultValue' => 'checked',
+        ];
+        $this->assertSame('<input  id="id" type="checkbox" name="name" checked />' . "\n", BridgeCard::getCheckboxInput($entry, 'id', 'name'));
+
+        $entry = [
+            'defaultValue' => 42,
+            'exampleValue' => 43,
+        ];
+        $this->assertSame('<input  id="id" type="number" value="42" placeholder="43" name="name" />' . "\n", BridgeCard::getNumberInput($entry, 'id', 'name'));
+
+        $entry = [
+            'defaultValue' => 'yo1',
+            'exampleValue' => 'yo2',
+        ];
+        $this->assertSame('<input  id="id" type="text" value="yo1" placeholder="yo2" name="name" />' . "\n", BridgeCard::getTextInput($entry, 'id', 'name'));
+
+        $entry = [
+            'values' => [],
+        ];
+        $this->assertSame('<select  id="id" name="name" ></select>', BridgeCard::getListInput($entry, 'id', 'name'));
+
+        $entry = [
+            'defaultValue' => 2,
+            'values' => [
+                'foo' => 'bar',
+            ],
+        ];
+        $this->assertSame('<select  id="id" name="name" ><option value="bar">foo</option></select>', BridgeCard::getListInput($entry, 'id', 'name'));
+
+        // optgroup
+        $entry = [
+            'defaultValue' => 2,
+            'values' => ['kek' => [
+                'f' => 'b',
+            ]],
+        ];
+        $this->assertSame('<select  id="id" name="name" ><optgroup label="kek"><option value="b">f</option></optgroup></select>', BridgeCard::getListInput($entry, 'id', 'name'));
+    }
+}