From 3fc38c15a3afa7e0377e3b6cb4ffec1335a36f63 Mon Sep 17 00:00:00 2001
From: Dag <me@dvikan.no>
Date: Fri, 3 Jan 2025 06:19:24 +0100
Subject: [PATCH] fix: cache 400 and 404, and refactor token auth (#4388)

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

* refactor(token_auth)
---
 actions/DisplayAction.php                     |  2 +-
 actions/FrontpageAction.php                   |  2 +-
 lib/http.php                                  |  2 +-
 middlewares/CacheMiddleware.php               |  9 +++++++--
 middlewares/TokenAuthenticationMiddleware.php | 16 ++++++++++------
 templates/token.html.php                      |  4 ++--
 6 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php
index 845bfd84..10af8ad7 100644
--- a/actions/DisplayAction.php
+++ b/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) {
diff --git a/actions/FrontpageAction.php b/actions/FrontpageAction.php
index 824441b2..79ffb4f5 100644
--- a/actions/FrontpageAction.php
+++ b/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;
diff --git a/lib/http.php b/lib/http.php
index d1043b33..15d6ebec 100644
--- a/lib/http.php
+++ b/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;
     }
diff --git a/middlewares/CacheMiddleware.php b/middlewares/CacheMiddleware.php
index bffde4af..b8a34754 100644
--- a/middlewares/CacheMiddleware.php
+++ b/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
diff --git a/middlewares/TokenAuthenticationMiddleware.php b/middlewares/TokenAuthenticationMiddleware.php
index f8234629..31544ab7 100644
--- a/middlewares/TokenAuthenticationMiddleware.php
+++ b/middlewares/TokenAuthenticationMiddleware.php
@@ -10,20 +10,24 @@ class TokenAuthenticationMiddleware implements Middleware
             return $next($request);
         }
 
-        // Always add token to request attribute
-        $request = $request->withAttribute('token', $request->get('token'));
+        $token = $request->get('token');
 
-        if (! $request->attribute('token')) {
+        if (! $token) {
             return new Response(render(__DIR__ . '/../templates/token.html.php', [
-                'message' => 'Missing token',
+                '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',
+                'message'   => 'Invalid token',
+                'token'     => $token,
             ]), 401);
         }
 
+        $request = $request->withAttribute('token', $token);
+
         return $next($request);
     }
 }
diff --git a/templates/token.html.php b/templates/token.html.php
index 1a036dbb..43c60972 100644
--- a/templates/token.html.php
+++ b/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>