From 62eba7fa4b54eaaf61306e01c0934e483e88db82 Mon Sep 17 00:00:00 2001
From: alxndrsn <alxndrsn>
Date: Mon, 4 Nov 2024 09:38:42 +0000
Subject: [PATCH 1/5] http: handle bad url-encoded Cookie session token

---
 lib/http/preprocessors.js       |  8 +++++++-
 test/unit/http/preprocessors.js | 24 ++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/lib/http/preprocessors.js b/lib/http/preprocessors.js
index c582f7c15..083ad82b7 100644
--- a/lib/http/preprocessors.js
+++ b/lib/http/preprocessors.js
@@ -114,7 +114,13 @@ const authHandler = ({ Sessions, Users, Auth }, context) => {
 
     // actually try to authenticate with it. no Problem on failure. short circuit
     // out if we have a GET or HEAD request.
-    const maybeSession = authBySessionToken(decodeURIComponent(token));
+    let decodedToken;
+    try {
+      decodedToken = decodeURIComponent(token);
+    } catch (err) {
+      reject(Problem.user.authenticationFailed());
+    }
+    const maybeSession = authBySessionToken(decodedToken);
     if ((context.method === 'GET') || (context.method === 'HEAD')) return maybeSession;
 
     // if non-GET run authentication as usual but we'll have to check CSRF afterwards.
diff --git a/test/unit/http/preprocessors.js b/test/unit/http/preprocessors.js
index 868c12d41..7541345c1 100644
--- a/test/unit/http/preprocessors.js
+++ b/test/unit/http/preprocessors.js
@@ -272,6 +272,30 @@ describe('preprocessors', () => {
           context.auth.session.should.eql(Option.of(new Session({ test: 'session' })));
         }));
 
+      it('should do nothing if cookie cannot be url-decoded', () => {
+        let caught = false;
+        Promise.resolve(authHandler(
+          { Auth, Sessions: mockSessions('alohomora') },
+          new Context(
+            createRequest({
+              method: 'GET',
+              headers: {
+                'X-Forwarded-Proto': 'https',
+                Cookie: 'session=aloho%25eamora'
+              },
+              cookies: { session: 'aloho%eamora' },
+              url: ''
+            }),
+            { auth: { isAuthenticated() { return false; } }, fieldKey: Option.none() }
+          )
+        )).catch((err) => {
+          err.problemCode.should.equal(401.2);
+          caught = true;
+        }).then(() => {
+          caught.should.equal(true);
+        });
+      });
+
       describe('CSRF protection', () => {
         const mockSessionsWithCsrf = (expectedToken, csrf) => ({
           getByBearerToken: (token) => Promise.resolve((token === expectedToken)

From a32c646ad3eaba5274ebdad3d92dc8eeb496f607 Mon Sep 17 00:00:00 2001
From: alxndrsn <alxndrsn>
Date: Sat, 9 Nov 2024 07:30:04 +0000
Subject: [PATCH 2/5] ramda

---
 lib/http/preprocessors.js | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/http/preprocessors.js b/lib/http/preprocessors.js
index 5169a6255..250013d5c 100644
--- a/lib/http/preprocessors.js
+++ b/lib/http/preprocessors.js
@@ -114,13 +114,10 @@ const authHandler = ({ Sessions, Users, Auth }, context) => {
 
     // actually try to authenticate with it. no Problem on failure. short circuit
     // out if we have a GET or HEAD request.
-    let decodedToken;
-    try {
-      decodedToken = decodeURIComponent(token);
-    } catch (err) {
-      reject(Problem.user.authenticationFailed());
-    }
-    const maybeSession = authBySessionToken(decodedToken);
+    const decodedToken = urlDecode(token);
+    if(decodedToken.isEmpty()) return reject(Problem.user.authenticationFailed());
+
+    const maybeSession = authBySessionToken(decodedToken.get());
     if ((context.method === 'GET') || (context.method === 'HEAD')) return maybeSession;
 
     // if non-GET run authentication as usual but we'll have to check CSRF afterwards.

From 9663c3ca6ac5f91e11e1da6db11e97bafa100d0d Mon Sep 17 00:00:00 2001
From: alxndrsn <alxndrsn>
Date: Sat, 9 Nov 2024 07:30:30 +0000
Subject: [PATCH 3/5] lint

---
 lib/http/preprocessors.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/http/preprocessors.js b/lib/http/preprocessors.js
index 250013d5c..b53906ef3 100644
--- a/lib/http/preprocessors.js
+++ b/lib/http/preprocessors.js
@@ -115,7 +115,7 @@ const authHandler = ({ Sessions, Users, Auth }, context) => {
     // actually try to authenticate with it. no Problem on failure. short circuit
     // out if we have a GET or HEAD request.
     const decodedToken = urlDecode(token);
-    if(decodedToken.isEmpty()) return reject(Problem.user.authenticationFailed());
+    if (decodedToken.isEmpty()) return reject(Problem.user.authenticationFailed());
 
     const maybeSession = authBySessionToken(decodedToken.get());
     if ((context.method === 'GET') || (context.method === 'HEAD')) return maybeSession;

From 97d5d940ac2db75c2b9886e48287dac97acc072d Mon Sep 17 00:00:00 2001
From: alxndrsn <alxndrsn>
Date: Sat, 9 Nov 2024 07:32:18 +0000
Subject: [PATCH 4/5] update test name

---
 test/unit/http/preprocessors.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/unit/http/preprocessors.js b/test/unit/http/preprocessors.js
index fc4f5a26c..357ff36ea 100644
--- a/test/unit/http/preprocessors.js
+++ b/test/unit/http/preprocessors.js
@@ -272,7 +272,7 @@ describe('preprocessors', () => {
           context.auth.session.should.eql(Option.of(new Session({ test: 'session' })));
         }));
 
-      it('should do nothing if cookie cannot be url-decoded', () => {
+      it('should reject cookie which cannot be url-decoded', () => {
         let caught = false;
         Promise.resolve(authHandler(
           { Auth, Sessions: mockSessions('alohomora') },

From 2fa94cb65050a4638a16929774d51866012e3359 Mon Sep 17 00:00:00 2001
From: alxndrsn <alxndrsn>
Date: Sun, 10 Nov 2024 11:57:32 +0000
Subject: [PATCH 5/5] move comments

---
 lib/http/preprocessors.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/http/preprocessors.js b/lib/http/preprocessors.js
index b53906ef3..6df3ddda7 100644
--- a/lib/http/preprocessors.js
+++ b/lib/http/preprocessors.js
@@ -112,11 +112,11 @@ const authHandler = ({ Sessions, Users, Auth }, context) => {
     if (token == null)
       return;
 
-    // actually try to authenticate with it. no Problem on failure. short circuit
-    // out if we have a GET or HEAD request.
     const decodedToken = urlDecode(token);
     if (decodedToken.isEmpty()) return reject(Problem.user.authenticationFailed());
 
+    // actually try to authenticate with it. no Problem on failure. short circuit
+    // out if we have a GET or HEAD request.
     const maybeSession = authBySessionToken(decodedToken.get());
     if ((context.method === 'GET') || (context.method === 'HEAD')) return maybeSession;