From d3edc1b18189f790222862d66375c3b761f8ca18 Mon Sep 17 00:00:00 2001 From: "justin.fiedler" Date: Thu, 16 Feb 2023 22:00:11 +0000 Subject: [PATCH 1/4] fix: wait to create cookie storage based on user options in init() --- src/amplitude-client.js | 2 +- src/cookiestorage.js | 4 ++-- test/amplitude-client.js | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index d4f5de02..1d704d74 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -47,7 +47,6 @@ var AmplitudeClient = function AmplitudeClient(instanceName) { plan: { ...DEFAULT_OPTIONS.plan }, trackingOptions: { ...DEFAULT_OPTIONS.trackingOptions }, }; - this.cookieStorage = new cookieStorage().getStorage(); this._q = []; // queue for proxied functions before script load this._sending = false; this._updateScheduled = false; @@ -127,6 +126,7 @@ AmplitudeClient.prototype.init = function init(apiKey, opt_userId, opt_config, o this._cookieName = Constants.COOKIE_PREFIX + '_' + this._storageSuffixV5; + this.cookieStorage = new cookieStorage().getStorage(this.options.disableCookies); this.cookieStorage.options({ expirationDays: this.options.cookieExpiration, domain: this.options.domain, diff --git a/src/cookiestorage.js b/src/cookiestorage.js index 367c4fea..5a9e9140 100644 --- a/src/cookiestorage.js +++ b/src/cookiestorage.js @@ -12,12 +12,12 @@ var cookieStorage = function () { this.storage = null; }; -cookieStorage.prototype.getStorage = function () { +cookieStorage.prototype.getStorage = function (disableCookies) { if (this.storage !== null) { return this.storage; } - if (baseCookie.areCookiesEnabled()) { + if (!disableCookies && baseCookie.areCookiesEnabled()) { this.storage = Cookie; } else { // if cookies disabled, fallback to localstorage diff --git a/test/amplitude-client.js b/test/amplitude-client.js index 0174b94f..58c4cb74 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -888,11 +888,11 @@ describe('AmplitudeClient', function () { var onErrorSpy = sinon.spy(); var amplitude = new AmplitudeClient(); - sinon.stub(amplitude.cookieStorage, 'options').throws(); + sinon.stub(amplitude, '_refreshDynamicConfig').throws(); amplitude.init(apiKey, null, { onError: onErrorSpy }); assert.isTrue(onErrorSpy.calledOnce); - amplitude.cookieStorage.options.restore(); + amplitude['_refreshDynamicConfig'].restore(); }); it('should set observer plan options', function () { From a96f993bb09012c21d71b48207d907c19c3ae140 Mon Sep 17 00:00:00 2001 From: "justin.fiedler" Date: Thu, 16 Feb 2023 22:30:05 +0000 Subject: [PATCH 2/4] chore: add test for disableCookies cookie creation --- test/amplitude-client.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/amplitude-client.js b/test/amplitude-client.js index 58c4cb74..ff3ad890 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -2728,6 +2728,35 @@ describe('AmplitudeClient', function () { }); }); + it('should not create any cookies if disabledCookies = true', function () { + const deleteAllCookies = () => + document.cookie.split(';').forEach(function (c) { + document.cookie = c.replace(/^ +/, '').replace(/=.*/, '=;expires=' + new Date().toUTCString() + ';path=/'); + }); + const getAllCookies = () => + document.cookie + .split(';') + .map((c) => c.trimStart()) + .filter((c) => !utils.isEmptyString(c)); + + deleteAllCookies(); + clock.tick(20); + + var cookieArray = getAllCookies(); + assert.equal(cookieArray.length, 0); + + var deviceId = 'test_device_id'; + var amplitude2 = new AmplitudeClient(); + + amplitude2.init(apiKey, null, { + deviceId: deviceId, + disableCookies: true, + }); + + cookieArray = getAllCookies(); + assert.equal(cookieArray.length, 0); + }); + it('should validate event properties', function () { var e = new Error('oops'); clock.tick(1); From 562e908eb2d66443b31e1af994665ccb2ca1deb9 Mon Sep 17 00:00:00 2001 From: "justin.fiedler" Date: Thu, 16 Feb 2023 22:34:48 +0000 Subject: [PATCH 3/4] chore: add test for disableCookies=false cookie creation --- test/amplitude-client.js | 57 +++++++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/test/amplitude-client.js b/test/amplitude-client.js index ff3ad890..5d712287 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -13,6 +13,17 @@ import { mockCookie, restoreCookie, getCookie } from './mock-cookie'; import { AmplitudeServerZone } from '../src/server-zone.js'; import Request from '../src/xhr'; +const deleteAllCookies = () => + document.cookie.split(';').forEach(function (c) { + document.cookie = c.replace(/^ +/, '').replace(/=.*/, '=;expires=' + new Date().toUTCString() + ';path=/'); + }); + +const getAllCookies = () => + document.cookie + .split(';') + .map((c) => c.trimStart()) + .filter((c) => !utils.isEmptyString(c)); + // maintain for testing backwards compatability describe('AmplitudeClient', function () { var apiKey = '000000'; @@ -2729,16 +2740,44 @@ describe('AmplitudeClient', function () { }); it('should not create any cookies if disabledCookies = true', function () { - const deleteAllCookies = () => - document.cookie.split(';').forEach(function (c) { - document.cookie = c.replace(/^ +/, '').replace(/=.*/, '=;expires=' + new Date().toUTCString() + ';path=/'); - }); - const getAllCookies = () => - document.cookie - .split(';') - .map((c) => c.trimStart()) - .filter((c) => !utils.isEmptyString(c)); + deleteAllCookies(); + clock.tick(20); + var cookieArray = getAllCookies(); + assert.equal(cookieArray.length, 0); + + var deviceId = 'test_device_id'; + var amplitude2 = new AmplitudeClient(); + + amplitude2.init(apiKey, null, { + deviceId: deviceId, + disableCookies: true, + }); + + cookieArray = getAllCookies(); + assert.equal(cookieArray.length, 0); + }); + + it('should create cookies if disabledCookies = false', function () { + deleteAllCookies(); + clock.tick(20); + + var cookieArray = getAllCookies(); + assert.equal(cookieArray.length, 0); + + var deviceId = 'test_device_id'; + var amplitude2 = new AmplitudeClient(); + + amplitude2.init(apiKey, null, { + deviceId: deviceId, + disableCookies: false, + }); + + cookieArray = getAllCookies(); + assert.equal(cookieArray.length, 1); + }); + + it('should not create any cookies if disabledCookies = true', function () { deleteAllCookies(); clock.tick(20); From 47adb72362ef085eaf768a4b40d0b68194de7210 Mon Sep 17 00:00:00 2001 From: "justin.fiedler" Date: Thu, 16 Feb 2023 22:37:09 +0000 Subject: [PATCH 4/4] chore: remove duplicated test --- test/amplitude-client.js | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/test/amplitude-client.js b/test/amplitude-client.js index 5d712287..e647e59c 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -2777,25 +2777,6 @@ describe('AmplitudeClient', function () { assert.equal(cookieArray.length, 1); }); - it('should not create any cookies if disabledCookies = true', function () { - deleteAllCookies(); - clock.tick(20); - - var cookieArray = getAllCookies(); - assert.equal(cookieArray.length, 0); - - var deviceId = 'test_device_id'; - var amplitude2 = new AmplitudeClient(); - - amplitude2.init(apiKey, null, { - deviceId: deviceId, - disableCookies: true, - }); - - cookieArray = getAllCookies(); - assert.equal(cookieArray.length, 0); - }); - it('should validate event properties', function () { var e = new Error('oops'); clock.tick(1);