fix: cache 400 and 404, and refactor token auth (#4388)

* fix(cache): also cache 400 and 404 responses

* refactor(token_auth)

actions/DisplayAction.php

@@ -23,7 +23,7 @@ class DisplayAction implements ActionInterface
         $noproxy = $request->get('_noproxy');
 
         if (!$bridgeName) {
-            return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Missing bridge parameter']), 400);
+            return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Missing bridge name parameter']), 400);
         }
         $bridgeClassName = $this->bridgeFactory->createBridgeClassName($bridgeName);
         if (!$bridgeClassName) {

actions/FrontpageAction.php

@@ -12,7 +12,7 @@ final class FrontpageAction implements ActionInterface
 
     public function __invoke(Request $request): Response
     {
-        $token = $request->attribute('token');
+        $token = $request->getAttribute('token');
 
         $messages = [];
         $activeBridges = 0;

lib/http.php

@@ -220,7 +220,7 @@ final class Request
         return $clone;
     }
 
-    public function attribute(string $key, $default = null)
+    public function getAttribute(string $key, $default = null)
     {
         return $this->attributes[$key] ?? $default;
     }

middlewares/CacheMiddleware.php

@@ -13,7 +13,7 @@ class CacheMiddleware implements Middleware
 
     public function __invoke(Request $request, $next): Response
     {
-        $action = $request->attribute('action');
+        $action = $request->getAttribute('action');
 
         if ($action !== 'DisplayAction') {
             // We only cache DisplayAction (for now)
@@ -43,9 +43,14 @@ class CacheMiddleware implements Middleware
         /** @var Response $response */
         $response = $next($request);
 
-        if (in_array($response->getCode(), [403, 429, 500, 503])) {
+        if ($response->getCode() === 200) {
+            // Do nothing because DisplayAction has already cached this on $cacheKey
+        } elseif (in_array($response->getCode(), [400, 403, 404, 429, 500, 503])) {
             // Cache these responses for about ~10 mins on average
             $this->cache->set($cacheKey, $response, 60 * 5 + rand(1, 60 * 10));
+        } else {
+            // Should never happen
+            $this->cache->set($cacheKey, $response, 60 * 5);
         }
 
         // For 1% of requests, prune cache

middlewares/TokenAuthenticationMiddleware.php

@@ -10,20 +10,24 @@ class TokenAuthenticationMiddleware implements Middleware
             return $next($request);
         }
 
-        // Always add token to request attribute
+        $token = $request->get('token');
-        $request = $request->withAttribute('token', $request->get('token'));
 
-        if (! $request->attribute('token')) {
+        if (! $token) {
             return new Response(render(__DIR__ . '/../templates/token.html.php', [
                 'message'   => 'Missing token',
+                'token'     => '',
             ]), 401);
         }
-        if (! hash_equals(Configuration::getConfig('authentication', 'token'), $request->attribute('token'))) {
+
+        if (! hash_equals(Configuration::getConfig('authentication', 'token'), $token)) {
             return new Response(render(__DIR__ . '/../templates/token.html.php', [
                 'message'   => 'Invalid token',
+                'token'     => $token,
             ]), 401);
         }
 
+        $request = $request->withAttribute('token', $token);
+
         return $next($request);
     }
 }

templates/token.html.php

@@ -13,8 +13,8 @@
     <?= e($message) ?>
 </p>
 
-<form action="" method="get">
+<form action="" method="get" autocomplete="off">
     <label for="token">Token:</label>
-    <input type="password" name="token" id="token" placeholder="token">
+    <input type="text" name="token" id="token" placeholder="token" value="<?= e($token) ?>">
     <input type="submit" value="OK">
 </form>