diff --git a/lib/http/preprocessors.js b/lib/http/preprocessors.js index bf195736a..6df3ddda7 100644 --- a/lib/http/preprocessors.js +++ b/lib/http/preprocessors.js @@ -112,9 +112,12 @@ const authHandler = ({ Sessions, Users, Auth }, context) => { if (token == null) return; + 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(decodeURIComponent(token)); + 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. diff --git a/test/unit/http/preprocessors.js b/test/unit/http/preprocessors.js index 78eb6a6d3..357ff36ea 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 reject cookie which 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)