feat: set default sameSite cookie values, short: lax, long: none

panva · May 10, 2019 · cfb1a70 · panva · May 11, 2019 · cfb1a70
1 parent a056bfd
commit cfb1a70
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 11 deletions.
diff --git a/docs/README.md b/docs/README.md
@@ -1678,7 +1678,8 @@ _**default value**_:
 {
   httpOnly: true,
   maxAge: 1209600000,
-  overwrite: true
+  overwrite: true,
+  sameSite: 'none'
 }
 ```
 
@@ -1708,7 +1709,8 @@ _**default value**_:
 {
   httpOnly: true,
   maxAge: 600000,
-  overwrite: true
+  overwrite: true,
+  sameSite: 'lax'
 }
 ```
 

diff --git a/lib/helpers/configuration.js b/lib/helpers/configuration.js
@@ -71,6 +71,10 @@ module.exports = class Configuration {
     this.ensureMaps();
     this.ensureSets();
 
+    // TODO: revert this back to its original state once
+    // https://github.com/pillarjs/cookies/issues/109 lands
+    this.removeCookiesSameSiteNone();
+
     // cookies
     // featuresValidations
     // routesValidations
@@ -417,6 +421,15 @@ module.exports = class Configuration {
     }
   }
 
+  removeCookiesSameSiteNone() {
+    if (/^none$/g.test(this.cookies.short.sameSite)) {
+      this.cookies.short.sameSite = undefined;
+    }
+    if (/^none$/g.test(this.cookies.long.sameSite)) {
+      this.cookies.long.sameSite = undefined;
+    }
+  }
+
   logDraftNotice() {
     const ENABLED_DRAFTS = new Set();
     let throwDraft = false;

diff --git a/lib/helpers/defaults.js b/lib/helpers/defaults.js
@@ -190,6 +190,7 @@ const DEFAULTS = {
       httpOnly: true, // cookies are not readable by client-side javascript
       maxAge: (14 * 24 * 60 * 60) * 1000, // 14 days in ms
       overwrite: true,
+      sameSite: 'none',
     },
 
     /*
@@ -202,6 +203,7 @@ const DEFAULTS = {
       httpOnly: true, // cookies are not readable by client-side javascript
       maxAge: (10 * 60) * 1000, // 10 minutes in ms
       overwrite: true,
+      sameSite: 'lax',
     },
 
     /*

diff --git a/lib/shared/session.js b/lib/shared/session.js
@@ -20,19 +20,25 @@ module.exports = async function sessionHandler(ctx, next) {
     await next();
   } finally {
     const sessionCookieName = ctx.oidc.provider.cookieName('session');
+    const stateCookieName = ctx.oidc.provider.cookieName('state');
+    const longRegexp = new RegExp(`^(${sessionCookieName}|${stateCookieName}\\.[^=]+)(?:\\.sig)?=`);
 
     // refresh the session duration
     if ((!ctx.oidc.session.new || ctx.oidc.session.touched) && !ctx.oidc.session.destroyed) {
       ctx.cookies.set(sessionCookieName, ctx.oidc.session.id, instance(ctx.oidc.provider).configuration('cookies.long'));
       await ctx.oidc.session.save();
     }
 
-    if (ctx.oidc.session.transient) {
-      const stateCookieName = ctx.oidc.provider.cookieName('state');
-
+    // TODO: revert this back to its original state once
+    // https://github.com/pillarjs/cookies/issues/109 lands
+    if (ctx.response.get('set-cookie')) {
       ctx.response.get('set-cookie').forEach((cookie, index, ary) => {
-        const isLong = cookie.startsWith(sessionCookieName) || cookie.startsWith(stateCookieName);
-        if (isLong && !cookie.includes('expires=Thu, 01 Jan 1970')) {
+        if (!cookie.includes('samesite=')) {
+          ary[index] = cookie = `${cookie}; samesite=none`; // eslint-disable-line no-param-reassign, no-multi-assign
+        }
+
+        const isLong = cookie.match(longRegexp);
+        if (isLong && ctx.oidc.session.transient && cookie.includes('expires=') && !cookie.includes('expires=Thu, 01 Jan 1970')) {
           ary[index] = cookie.replace(/(; ?expires=([\w\d:, ]+))/, ''); // eslint-disable-line no-param-reassign
         }
       });

diff --git a/test/session_management/end_session.test.js b/test/session_management/end_session.test.js
@@ -49,7 +49,7 @@ describe('[session_management]', () => {
         .type('form')
         .expect(302)
         .expect((response) => {
-          expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; httponly');
+          expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; httponly; samesite=none');
           expect(adapter.destroy.called).to.be.true;
           expect(adapter.upsert.called).not.to.be.true;
           expect(adapter.destroy.withArgs(sessionId).calledOnce).to.be.true;
@@ -71,7 +71,7 @@ describe('[session_management]', () => {
         .expect(302)
         .expect((response) => {
           session = this.getSession();
-          expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; httponly');
+          expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; httponly; samesite=none');
           expect(session.authorizations.client).to.be.undefined;
           expect(session.state).to.be.undefined;
           expect(this.getSessionId()).not.to.eql(oldId);
@@ -92,7 +92,7 @@ describe('[session_management]', () => {
         })
         .expect(302)
         .expect((response) => {
-          expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=.oidc.dev; httponly');
+          expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=.oidc.dev; httponly; samesite=none');
         });
     });
 
@@ -112,7 +112,7 @@ describe('[session_management]', () => {
         .expect(302)
         .expect('location', '/?state=foobar')
         .expect((response) => {
-          expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=.oidc.dev; httponly');
+          expect(response.headers['set-cookie']).to.contain('_state.client=; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=.oidc.dev; httponly; samesite=none');
         });
     });