Respect SameSite in Auth Cookie (#188)

auth0 · Feb 8, 2021 · bcc141a · bcc141a
1 parent a25f393
commit bcc141a
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,10 +10,10 @@
 [Full Changelog](https://github.com/auth0/express-openid-connect/compare/v2.1.0...v2.2.0)
 
 **Added**
-- afterCallback Hook [#168](https://github.com/auth0/express-openid-connect/pull/168) ([davidpatrick](https://github.com/davidpatrick))
+- afterCallback Hook [#171](https://github.com/auth0/express-openid-connect/pull/171) ([davidpatrick](https://github.com/davidpatrick))
 
 **Changed**
-- Move transient cookies into single cookie [#171](https://github.com/auth0/express-openid-connect/pull/171) ([davidpatrick](https://github.com/davidpatrick))
+- Move transient cookies into single cookie [#168](https://github.com/auth0/express-openid-connect/pull/168) ([davidpatrick](https://github.com/davidpatrick))
 - Use native node hkdf when available (Node >=15) [#177](https://github.com/auth0/express-openid-connect/pull/177) ([panva](https://github.com/panva))
 
 ## [2.1.0](https://github.com/auth0/express-openid-connect/tree/v2.1.0) (2020-12-15)

diff --git a/lib/context.js b/lib/context.js
@@ -223,26 +223,32 @@ class ResponseContext {
       const authVerification = {
         nonce: transient.generateNonce(),
         state: encodeState(stateValue),
-        ...(options.authorizationParams.max_age ? {
-          "max_age": options.authorizationParams.max_age
-        } : undefined)
-      }
-      const authParams = { ...options.authorizationParams, ...authVerification };
+        ...(options.authorizationParams.max_age
+          ? {
+              max_age: options.authorizationParams.max_age,
+            }
+          : undefined),
+      };
+      const authParams = {
+        ...options.authorizationParams,
+        ...authVerification,
+      };
 
       if (usePKCE) {
         authVerification.code_verifier = transient.generateNonce();
 
-        authParams.code_challenge_method= 'S256';
+        authParams.code_challenge_method = 'S256';
         authParams.code_challenge = transient.calculateCodeChallenge(
           authVerification.code_verifier
         );
       }
 
       transient.store('auth_verification', req, res, {
-        sameSite: options.authorizationParams.response_mode === 'form_post'
-          ? 'None'
-          : 'Lax',
-        value: JSON.stringify(authVerification)
+        sameSite:
+          options.authorizationParams.response_mode === 'form_post'
+            ? 'None'
+            : config.session.cookie.sameSite,
+        value: JSON.stringify(authVerification),
       });
 
       const authorizationUrl = client.authorizationUrl(authParams);

diff --git a/test/login.tests.js b/test/login.tests.js
@@ -16,18 +16,24 @@ const filterRoute = (method, path) => {
     r.route && r.route.path === path && r.route.methods[method.toLowerCase()];
 };
 
-const fetchFromAuthCookie = (res, cookieName) => {
+const fetchAuthCookie = (res) => {
   const cookieHeaders = res.headers['set-cookie'];
-  const authCookie = cookieHeaders.filter(
+  return cookieHeaders.filter(
     (header) => header.split('=')[0] === 'auth_verification'
   )[0];
+};
+
+const fetchFromAuthCookie = (res, cookieName) => {
+  const authCookie = fetchAuthCookie(res);
 
   if (!authCookie) {
-    return false
+    return false;
   }
 
   const decodedAuthCookie = querystring.decode(authCookie);
-  const cookieValuePart = decodedAuthCookie.auth_verification.split('; ')[0].split('.')[0];
+  const cookieValuePart = decodedAuthCookie.auth_verification
+    .split('; ')[0]
+    .split('.')[0];
   const authCookieParsed = JSON.parse(cookieValuePart);
 
   return authCookieParsed[cookieName];
@@ -356,4 +362,43 @@ describe('auth', () => {
 
     assert.isDefined(fetchFromAuthCookie(res, 'code_verifier'));
   });
+
+  it('should respect sameSite when response_mode is not form_post', async () => {
+    server = await createServer(
+      auth({
+        ...defaultConfig,
+        clientSecret: '__test_client_secret__',
+        session: {
+          cookie: {
+            sameSite: 'Strict',
+          },
+        },
+        authorizationParams: {
+          response_mode: 'query',
+          response_type: 'code',
+        },
+      })
+    );
+    const res = await request.get('/login', { baseUrl, followRedirect: false });
+    assert.equal(res.statusCode, 302);
+
+    assert.include(fetchAuthCookie(res), 'SameSite=Strict');
+  });
+
+  it('should overwrite SameSite to None when response_mode is form_post', async () => {
+    server = await createServer(
+      auth({
+        ...defaultConfig,
+        session: {
+          cookie: {
+            sameSite: 'Strict',
+          },
+        },
+      })
+    );
+    const res = await request.get('/login', { baseUrl, followRedirect: false });
+    assert.equal(res.statusCode, 302);
+
+    assert.include(fetchAuthCookie(res), 'SameSite=None');
+  });
 });