From 02658a3b01468295ce06ec887d3c55ceac500862 Mon Sep 17 00:00:00 2001 From: Krishna Rajendran Date: Fri, 3 Apr 2020 15:15:23 -0700 Subject: [PATCH] Add a same site cookie option --- src/amplitude-client.js | 4 +++- src/base-cookie.js | 4 +++- src/cookie.js | 1 + src/options.js | 1 + test/amplitude-client.js | 21 +++++++++++++++++++++ test/base-cookie.js | 24 ++++++++++++++++++++++++ test/mock-cookie.js | 36 ++++++++++++++++++++++++++++++++++++ test/tests.js | 1 + 8 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 test/base-cookie.js create mode 100644 test/mock-cookie.js diff --git a/src/amplitude-client.js b/src/amplitude-client.js index 255d4efc..1e48bcc0 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -98,6 +98,7 @@ AmplitudeClient.prototype.init = function init(apiKey, opt_userId, opt_config, o expirationDays: this.options.cookieExpiration, domain: this.options.domain, secure: this.options.secureCookie, + sameSite: this.options.sameSiteCookie }); this.options.domain = this.cookieStorage.options().domain; @@ -786,7 +787,8 @@ AmplitudeClient.prototype.setDomain = function setDomain(domain) { this.cookieStorage.options({ expirationDays: this.options.cookieExpiration, secure: this.options.secureCookie, - domain: domain + domain: domain, + sameSite: this.options.sameSiteCookie }); this.options.domain = this.cookieStorage.options().domain; _loadCookieData(this); diff --git a/src/base-cookie.js b/src/base-cookie.js index e0fd038e..30e9abf6 100644 --- a/src/base-cookie.js +++ b/src/base-cookie.js @@ -39,7 +39,9 @@ const set = (name, value, opts) => { if (opts.secure) { str += '; Secure'; } - str += '; SameSite=Lax'; + if (opts.sameSite) { + str += '; SameSite=' + opts.sameSite; + } document.cookie = str; }; diff --git a/src/cookie.js b/src/cookie.js index bae0c6ae..c630b16f 100644 --- a/src/cookie.js +++ b/src/cookie.js @@ -70,6 +70,7 @@ var options = function(opts) { _options.expirationDays = opts.expirationDays; _options.secure = opts.secure; + _options.sameSite = opts.sameSite; var domain = (!utils.isEmptyString(opts.domain)) ? opts.domain : '.' + topDomain(getLocation().href); var token = Math.random(); diff --git a/src/options.js b/src/options.js index ebd8262a..769478dd 100644 --- a/src/options.js +++ b/src/options.js @@ -16,6 +16,7 @@ export default { batchEvents: false, cookieExpiration: 365 * 10, cookieName: 'amplitude_id', + sameSiteCookie: 'None', deviceIdFromUrlParam: false, domain: '', eventUploadPeriodMillis: 30 * 1000, // 30s diff --git a/test/amplitude-client.js b/test/amplitude-client.js index ef25f6f3..d26b1314 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -9,6 +9,7 @@ import queryString from 'query-string'; import Identify from '../src/identify.js'; import Revenue from '../src/revenue.js'; import constants from '../src/constants.js'; +import { mockCookie, restoreCookie, getCookie } from './mock-cookie'; // maintain for testing backwards compatability describe('AmplitudeClient', function() { @@ -34,6 +35,7 @@ describe('AmplitudeClient', function() { function reset() { localStorage.clear(); sessionStorage.clear(); + restoreCookie(); cookie.remove(amplitude.options.cookieName); cookie.remove(amplitude.options.cookieName + keySuffix); cookie.remove(amplitude.options.cookieName + '_new_app'); @@ -74,6 +76,24 @@ describe('AmplitudeClient', function() { assert.ok(onInitCalled); }); + it('should set the Secure flag on cookie with the secureCookie option', () => { + mockCookie(); + amplitude.init(apiKey, null, { secureCookie: true }); + assert.include(getCookie('amplitude_id_' + apiKey).options, 'Secure'); + }); + + it('should set the SameSite cookie option to None by default', () => { + mockCookie(); + amplitude.init(apiKey); + assert.include(getCookie('amplitude_id_' + apiKey).options, 'SameSite=None'); + }); + + it('should set the sameSite option on a cookie with the sameSiteCookie Option', () => { + mockCookie(); + amplitude.init(apiKey, null, {sameSiteCookie: 'Strict'}); + assert.include(getCookie('amplitude_id_' + apiKey).options, 'SameSite=Strict'); + }); + it('should immediately invoke onInit callbacks if already initialized', function() { let onInitCalled = false; amplitude.init(apiKey); @@ -3352,6 +3372,7 @@ describe('setVersionName', function() { assert.lengthOf(server.requests, 1, 'should have sent a request to Amplitude'); assert.equal(events[0].event_type, '$identify'); }); + describe('prior to opting into analytics', function () { beforeEach(function () { reset(); diff --git a/test/base-cookie.js b/test/base-cookie.js new file mode 100644 index 00000000..e49c433f --- /dev/null +++ b/test/base-cookie.js @@ -0,0 +1,24 @@ +import cookie from '../src/base-cookie'; +import { mockCookie, restoreCookie, getCookie } from './mock-cookie'; + +describe('cookie', function() { + beforeEach(() => { + mockCookie(); + }) + + afterEach(() => { + restoreCookie(); + }); + + describe('set', () => { + it('should set the secure flag with the secure option', () => { + cookie.set('key', 'val', {secure: true}); + assert.include(getCookie('key').options, 'Secure'); + }) + + it('should set the same site value with the sameSite option', () => { + cookie.set('key', 'val', {sameSite: "Lax"}); + assert.include(getCookie('key').options, 'SameSite=Lax'); + }) + }) +}); diff --git a/test/mock-cookie.js b/test/mock-cookie.js new file mode 100644 index 00000000..7c2b5a6b --- /dev/null +++ b/test/mock-cookie.js @@ -0,0 +1,36 @@ +let rawCookieData = {}; + +let isMocked = false; + +export const mockCookie = () => { + isMocked = true; + + document.__defineGetter__('cookie', function () { + return Object.keys(rawCookieData).map(key => `${key}=${rawCookieData[key].val}`).join(";"); + }); + + document.__defineSetter__('cookie', function (str) { + const indexEquals = str.indexOf("="); + const key = str.substr(0, indexEquals); + const remainingStr = str.substring(str + 1); + const splitSemi = remainingStr.split(';').map((str)=> str.trim()); + + rawCookieData[key] = { + val: splitSemi[0], + options: splitSemi.slice(1) + }; + return str; + }); +}; + +export const restoreCookie = () => { + if (isMocked) { + delete document['cookie']; + rawCookieData = {}; + isMocked = false; + } +}; + +export const getCookie = (key) => { + return rawCookieData[key]; +}; diff --git a/test/tests.js b/test/tests.js index 466e0211..5c2d0493 100644 --- a/test/tests.js +++ b/test/tests.js @@ -11,3 +11,4 @@ import './amplitude.js'; import './amplitude-client.js'; import './utils.js'; import './revenue.js'; +import './base-cookie.js';