fix: skip top domain when cookie is disabled (#580)

amplitude · Mar 6, 2023 · a9ff324 · a9ff324
1 parent 04ed78d
commit a9ff324
Show file tree

Hide file tree

Showing 10 changed files with 82 additions and 42 deletions.
diff --git a/src/amplitude-client.js b/src/amplitude-client.js
@@ -15,7 +15,6 @@ import utils from './utils';
 import UUID from './uuid';
 import base64Id from './base64Id';
 import DEFAULT_OPTIONS from './options';
-import getHost from './get-host';
 import baseCookie from './base-cookie';
 import { AmplitudeServerZone, getEventLogApi } from './server-zone';
 import ConfigManager from './config-manager';
@@ -309,7 +308,7 @@ AmplitudeClient.prototype._runNewSessionStartCallbacks = function () {
 };
 
 AmplitudeClient.prototype.deleteLowerLevelDomainCookies = function () {
-  const host = getHost();
+  const host = utils.getHost();
 
   const cookieHost =
     this.options.domain && this.options.domain[0] === '.' ? this.options.domain.slice(1) : this.options.domain;

diff --git a/src/base-cookie.js b/src/base-cookie.js
@@ -1,5 +1,4 @@
 import Constants from './constants';
-import base64Id from './base64Id';
 import utils from './utils';
 
 const get = (name) => {
@@ -94,7 +93,7 @@ const sortByEventTime = (cookies) => {
 
 // test that cookies are enabled - navigator.cookiesEnabled yields false positives in IE, need to test directly
 const areCookiesEnabled = (opts = {}) => {
-  const cookieName = Constants.COOKIE_TEST_PREFIX + base64Id();
+  const cookieName = Constants.COOKIE_TEST_PREFIX;
   if (typeof document === 'undefined') {
     return false;
   }

diff --git a/src/cookie.js b/src/cookie.js
@@ -4,7 +4,6 @@
 
 import Base64 from './base64';
 import utils from './utils';
-import getLocation from './get-location';
 import baseCookie from './base-cookie';
 import topDomain from './top-domain';
 
@@ -31,7 +30,7 @@ var options = function (opts) {
   _options.secure = opts.secure;
   _options.sameSite = opts.sameSite;
 
-  var domain = !utils.isEmptyString(opts.domain) ? opts.domain : '.' + topDomain(getLocation().href);
+  var domain = !utils.isEmptyString(opts.domain) ? opts.domain : '.' + topDomain(utils.getLocation().href);
   var token = Math.random();
   _options.domain = domain;
   set('amplitude_test', token);

diff --git a/src/get-host.js b/src/get-host.js
diff --git a/src/get-location.js b/src/get-location.js
diff --git a/src/metadata-storage.js b/src/metadata-storage.js
@@ -6,7 +6,6 @@
 import Base64 from './base64';
 import baseCookie from './base-cookie';
 import Constants from './constants';
-import getLocation from './get-location';
 import ampLocalStorage from './localstorage';
 import topDomain from './top-domain';
 import utils from './utils';
@@ -35,8 +34,8 @@ class MetadataStorage {
     this.expirationDays = expirationDays;
 
     this.cookieDomain = '';
-    const loc = getLocation() ? getLocation().href : undefined;
-    const writableTopDomain = topDomain(loc);
+    const loc = utils.getLocation() ? utils.getLocation().href : undefined;
+    const writableTopDomain = !disableCookies ? topDomain(loc) : '';
     this.cookieDomain = domain || (writableTopDomain ? '.' + writableTopDomain : null);
 
     if (storageOptionExists[storage]) {

diff --git a/src/top-domain.js b/src/top-domain.js
@@ -1,11 +1,10 @@
 import baseCookie from './base-cookie';
 import base64Id from './base64Id';
-import getHost from './get-host';
 import utils from './utils';
 
 // Utility that finds top level domain to write to
 const topDomain = (url) => {
-  const host = getHost(url);
+  const host = utils.getHost(url);
   const parts = host.split('.');
   const levels = [];
   const cname = '_tldtest_' + base64Id();

diff --git a/src/utils.js b/src/utils.js
@@ -287,6 +287,26 @@ const validateSessionId = (sessionId) => {
   return false;
 };
 
+const getLocation = () => {
+  return GlobalScope.location;
+};
+
+const getHost = (url) => {
+  const defaultHostname = GlobalScope.location ? GlobalScope.location.hostname : '';
+  if (url) {
+    if (typeof document !== 'undefined') {
+      const a = document.createElement('a');
+      a.href = url;
+      return a.hostname || defaultHostname;
+    }
+    if (typeof URL === 'function') {
+      const u = new URL(url);
+      return u.hostname || defaultHostname;
+    }
+  }
+  return defaultHostname;
+};
+
 export default {
   setLogLevel,
   getLogLevel,
@@ -303,4 +323,6 @@ export default {
   validateDeviceId,
   validateTransport,
   validateSessionId,
+  getLocation,
+  getHost,
 };
diff --git a/test/amplitude-client.js b/test/amplitude-client.js
@@ -2739,23 +2739,54 @@ describe('AmplitudeClient', function () {
       });
     });
 
-    it('should not create any cookies if disabledCookies = true', function () {
+    it('should not use any cookies with simple host 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();
+      var amplitude = new AmplitudeClient();
 
-      amplitude2.init(apiKey, null, {
+      amplitude.init(apiKey, null, {
+        deviceId: deviceId,
+        disableCookies: true,
+      });
+
+      cookieArray = getAllCookies();
+      assert.equal(cookieArray.length, 0);
+    });
+
+    it('should not use any cookies with multiple domains in host if disabledCookies = true', function () {
+      const stubbedGetLocation = sinon.stub(utils, 'getLocation').returns({ href: 'https://abc.def.xyz/test' });
+      const spiedSet = sinon.spy(baseCookie, 'set');
+      const spiedGet = sinon.spy(baseCookie, 'get');
+      deleteAllCookies();
+      clock.tick(20);
+
+      var cookieArray = getAllCookies();
+      assert.equal(cookieArray.length, 0);
+
+      var deviceId = 'test_device_id';
+      var amplitude = new AmplitudeClient();
+
+      amplitude.init(apiKey, null, {
         deviceId: deviceId,
         disableCookies: true,
       });
 
       cookieArray = getAllCookies();
       assert.equal(cookieArray.length, 0);
+
+      // spied cookie operations should not be fired
+      assert.equal(spiedSet.called, false);
+      assert.equal(spiedGet.called, false);
+
+      // restore stub, spy
+      stubbedGetLocation.restore();
+      spiedSet.restore();
+      spiedGet.restore();
     });
 
     it('should create cookies if disabledCookies = false', function () {
@@ -2766,9 +2797,9 @@ describe('AmplitudeClient', function () {
       assert.equal(cookieArray.length, 0);
 
       var deviceId = 'test_device_id';
-      var amplitude2 = new AmplitudeClient();
+      var amplitude = new AmplitudeClient();
 
-      amplitude2.init(apiKey, null, {
+      amplitude.init(apiKey, null, {
         deviceId: deviceId,
         disableCookies: false,
       });

diff --git a/test/utils.js b/test/utils.js
@@ -1,6 +1,7 @@
 import sinon from 'sinon';
 import utils from '../src/utils.js';
 import constants from '../src/constants.js';
+import GlobalScope from '../src/global-scope';
 
 describe('utils', function () {
   describe('isEmptyString', function () {
@@ -280,4 +281,21 @@ describe('utils', function () {
       assert.isFalse(utils.validateSessionId(false));
     });
   });
+
+  describe('getHost', function () {
+    it('should return hostname for url', function () {
+      const url = 'https://amplitude.is.good.com/test';
+      assert.equal(utils.getHost(url), 'amplitude.is.good.com');
+    });
+
+    it('should return current hostname if no url is provided', function () {
+      assert.equal(utils.getHost(), GlobalScope.location.hostname);
+    });
+  });
+
+  describe('getLocation', function () {
+    it('should return global location', function () {
+      assert.equal(utils.getLocation(), GlobalScope.location);
+    });
+  });
 });