fix: don't send SameSite=None on Chromium/Chrome < 80.x (#25)

eggjs · Feb 19, 2020 · c6e1e74 · c6e1e74
1 parent cedd470
commit c6e1e74
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 3 deletions.
diff --git a/lib/cookies.js b/lib/cookies.js
@@ -2,7 +2,7 @@
 
 const assert = require('assert');
 const utility = require('utility');
-const isSameSiteNoneCompatible = require('should-send-same-site-none').isSameSiteNoneCompatible;
+const _isSameSiteNoneCompatible = require('should-send-same-site-none').isSameSiteNoneCompatible;
 const Keygrip = require('./keygrip');
 const Cookie = require('./cookie');
 
@@ -114,7 +114,7 @@ class Cookies {
     // fixed SameSite=None: Known Incompatible Clients
     if (opts.sameSite && typeof opts.sameSite === 'string' && opts.sameSite.toLowerCase() === 'none') {
       const userAgent = this.ctx.get('user-agent');
-      if (userAgent && !isSameSiteNoneCompatible(userAgent)) {
+      if (userAgent && !this.isSameSiteNoneCompatible(userAgent)) {
         // Incompatible clients, don't send SameSite=None property
         opts.sameSite = false;
       }
@@ -137,6 +137,21 @@ class Cookies {
     this.ctx.set('set-cookie', headers);
     return this;
   }
+
+  isSameSiteNoneCompatible(userAgent) {
+    // Chrome >= 80.0.0.0
+    const result = parseChromiumAndMajorVersion(userAgent);
+    if (result.chromium) return result.majorVersion >= 80;
+    return _isSameSiteNoneCompatible(userAgent);
+  }
+}
+
+// https://github.com/linsight/should-send-same-site-none/blob/master/index.js#L86
+function parseChromiumAndMajorVersion(userAgent) {
+  const m = /Chrom[^ \/]+\/(\d+)[\.\d]* /.exec(userAgent);
+  if (!m) return { chromium: false, version: null };
+  // Extract digits from first capturing group.
+  return { chromium: true, majorVersion: parseInt(m[1]) };
 }
 
 const partternCache = new Map();

diff --git a/test/lib/cookies.test.js b/test/lib/cookies.test.js
@@ -264,7 +264,7 @@ describe('test/lib/cookies.test.js', () => {
     }
   });
 
-  it('should send SameSite=None property on compatible clients', () => {
+  it('should send not SameSite=None property on Chrome < 80', () => {
     const cookies = Cookies({
       secure: true,
       headers: {
@@ -276,6 +276,41 @@ describe('test/lib/cookies.test.js', () => {
     };
     cookies.set('foo', 'hello', opts);
 
+    assert(opts.signed === 1);
+    assert(opts.secure === undefined);
+    assert(cookies.ctx.response.headers['set-cookie'].join(';').match(/foo=hello/));
+    for (const str of cookies.ctx.response.headers['set-cookie']) {
+      assert(str.includes('; path=/; httponly'));
+    }
+  });
+
+  it('should send not SameSite=None property on Chrome >= 80', () => {
+    let cookies = Cookies({
+      secure: true,
+      headers: {
+        'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3945.29 Safari/537.36',
+      },
+    }, null, { sameSite: 'None' });
+    const opts = {
+      signed: 1,
+    };
+    cookies.set('foo', 'hello', opts);
+
+    assert(opts.signed === 1);
+    assert(opts.secure === undefined);
+    assert(cookies.ctx.response.headers['set-cookie'].join(';').match(/foo=hello/));
+    for (const str of cookies.ctx.response.headers['set-cookie']) {
+      assert(str.includes('; path=/; samesite=none; httponly'));
+    }
+
+    cookies = Cookies({
+      secure: true,
+      headers: {
+        'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/81.0.3945.29 Safari/537.36',
+      },
+    }, null, { sameSite: 'None' });
+    cookies.set('foo', 'hello', opts);
+
     assert(opts.signed === 1);
     assert(opts.secure === undefined);
     assert(cookies.ctx.response.headers['set-cookie'].join(';').match(/foo=hello/));