From dbc64853db32c3e9f2211b44285492f71aff1e83 Mon Sep 17 00:00:00 2001 From: Nick Jacob Date: Wed, 12 Apr 2023 08:43:24 -0400 Subject: [PATCH] AMX ID System: allow cookie storage (#9761) * Update AMXIdSystem logic, allow non-html5 storage, refactor sharedId domainOverride function into library * Fix failing test, bad invocation of getStorageManager --- libraries/domainOverrideToRootDomain/index.js | 39 ++++++++++ modules/amxIdSystem.js | 43 ++++++----- modules/amxIdSystem.md | 14 ++-- modules/sharedIdSystem.js | 31 +------- .../domainOverrideToRootDomain/index_spec.js | 77 +++++++++++++++++++ test/spec/modules/amxIdSystem_spec.js | 39 ++++------ test/spec/modules/sharedIdSystem_spec.js | 46 ----------- 7 files changed, 163 insertions(+), 126 deletions(-) create mode 100644 libraries/domainOverrideToRootDomain/index.js create mode 100644 test/spec/libraries/domainOverrideToRootDomain/index_spec.js diff --git a/libraries/domainOverrideToRootDomain/index.js b/libraries/domainOverrideToRootDomain/index.js new file mode 100644 index 00000000000..95a334755d1 --- /dev/null +++ b/libraries/domainOverrideToRootDomain/index.js @@ -0,0 +1,39 @@ +/** + * Create a domainOverride callback for an ID module, closing over + * an instance of StorageManager. + * + * The domainOverride function, given document.domain, will return + * the topmost domain we are able to set a cookie on. For example, + * given subdomain.example.com, it would return example.com. + * + * @param {StorageManager} storage e.g. from getStorageManager() + * @param {string} moduleName the name of the module using this function + * @returns {function(): string} + */ +export function domainOverrideToRootDomain(storage, moduleName) { + return function() { + const domainElements = document.domain.split('.'); + const cookieName = `_gd${Date.now()}_${moduleName}`; + + for (let i = 0, topDomain, testCookie; i < domainElements.length; i++) { + const nextDomain = domainElements.slice(i).join('.'); + + // write test cookie + storage.setCookie(cookieName, '1', undefined, undefined, nextDomain); + + // read test cookie to verify domain was valid + testCookie = storage.getCookie(cookieName); + + // delete test cookie + storage.setCookie(cookieName, '', 'Thu, 01 Jan 1970 00:00:01 GMT', undefined, nextDomain); + + if (testCookie === '1') { + // cookie was written successfully using test domain so the topDomain is updated + topDomain = nextDomain; + } else { + // cookie failed to write using test domain so exit by returning the topDomain + return topDomain; + } + } + } +} diff --git a/modules/amxIdSystem.js b/modules/amxIdSystem.js index 9dbab496f2c..5deffa00dfe 100644 --- a/modules/amxIdSystem.js +++ b/modules/amxIdSystem.js @@ -10,28 +10,25 @@ import {ajaxBuilder} from '../src/ajax.js'; import {submodule} from '../src/hook.js'; import {getRefererInfo} from '../src/refererDetection.js'; import {deepAccess, logError} from '../src/utils.js'; +import {getStorageManager} from '../src/storageManager.js'; +import {MODULE_TYPE_UID} from '../src/activities/modules.js'; +import {domainOverrideToRootDomain} from '../libraries/domainOverrideToRootDomain/index.js'; const NAME = 'amxId'; const GVL_ID = 737; const ID_KEY = NAME; -const version = '1.0'; +const version = '2.0'; const SYNC_URL = 'https://id.a-mx.com/sync/'; const AJAX_TIMEOUT = 300; +const AJAX_OPTIONS = {method: 'GET', withCredentials: true, contentType: 'text/plain'}; -function validateConfig(config) { - if (config == null || config.storage == null) { - logError(`${NAME}: config.storage is required.`); - return false; - } - - if (config.storage.type !== 'html5') { - logError( - `${NAME} only supports storage.type "html5". ${config.storage.type} was provided` - ); - return false; - } +export const storage = getStorageManager({moduleName: NAME, moduleType: MODULE_TYPE_UID}); +const AMUID_KEY = '__amuidpb'; +const getBidAdapterID = () => storage.localStorageIsEnabled() ? storage.getDataFromLocalStorage(AMUID_KEY) : null; +function validateConfig(config) { if ( + config.storage != null && typeof config.storage.expires === 'number' && config.storage.expires > 30 ) { @@ -44,7 +41,7 @@ function validateConfig(config) { return true; } -function handleSyncResponse(client, response, callback) { +function handleSyncResponse(client, response, params, callback) { if (response.id != null && response.id.length > 0) { callback(response.id); return; @@ -72,7 +69,7 @@ function handleSyncResponse(client, response, callback) { logError(`${NAME} invalid value`, complete); callback(null); }, - }); + }, params, AJAX_OPTIONS); } export const amxIdSubmodule = { @@ -97,6 +94,8 @@ export const amxIdSubmodule = { ? { [ID_KEY]: value } : undefined, + domainOverride: domainOverrideToRootDomain(storage, NAME), + getId(config, consentData, _extant) { if (!validateConfig(config)) { return undefined; @@ -109,12 +108,18 @@ export const amxIdSubmodule = { const params = { tagId: deepAccess(config, 'params.tagId', ''), - // TODO: are these referer values correct? + ref: ref.ref, u: ref.location, + tl: ref.topmostLocation, + nf: ref.numIframes, + rt: ref.reachedTop, + v: '$prebid.version$', + av: version, vg: '$$PREBID_GLOBAL$$', us_privacy: usp, + am: getBidAdapterID(), gdpr: consent.gdprApplies ? 1 : 0, gdpr_consent: consent.consentString, }; @@ -131,7 +136,7 @@ export const amxIdSubmodule = { if (responseText != null && responseText.length > 0) { try { const parsed = JSON.parse(responseText); - handleSyncResponse(client, parsed, done); + handleSyncResponse(client, parsed, params, done); return; } catch (e) { logError(`${NAME} invalid response`, responseText); @@ -142,9 +147,7 @@ export const amxIdSubmodule = { }, }, params, - { - method: 'GET' - } + AJAX_OPTIONS ); return { callback }; diff --git a/modules/amxIdSystem.md b/modules/amxIdSystem.md index f67fefe261e..5d2b0c48478 100644 --- a/modules/amxIdSystem.md +++ b/modules/amxIdSystem.md @@ -29,15 +29,15 @@ pbjs.setConfig({ | Param under `userSync.userIds[]` | Scope | Type | Description | Example | | -------------------------------- | -------- | ------ | --------------------------- | ----------------------------------------- | | name | Required | string | ID for the amxId module | `"amxId"` | -| storage | Required | Object | Settings for amxId storage | See [storage settings](#storage-settings) | +| storage | Optional | Object | Settings for amxId storage | See [storage settings](#storage-settings) | | params | Optional | Object | Parameters for amxId module | See [params](#params) | ### Storage Settings -The following settings are available for the `storage` property in the `userSync.userIds[]` object: +The following settings are suggested for the `storage` property in the `userSync.userIds[]` object: -| Param under `storage` | Scope | Type | Description | Example | -| --------------------- | -------- | ------------ | -------------------------------------------------------------------------------- | --------- | -| name | Required | String | Where the ID will be stored | `"amxId"` | -| type | Required | String | This must be `"html5"` | `"html5"` | -| expires | Required | Number <= 30 | number of days until the stored ID expires. **Must be less than or equal to 30** | `14` | +| Param under `storage` | Type | Description | Example | +| --------------------- | ------------ | -------------------------------------------------------------------------------- | --------- | +| name | String | Where the ID will be stored | `"amxId"` | +| type | String | For best performance, this should be `"html5"` | `"html5"` | +| expires | Number <= 30 | number of days until the stored ID expires. **Must be less than or equal to 30** | `14` | diff --git a/modules/sharedIdSystem.js b/modules/sharedIdSystem.js index 69c19929975..ef56e10870b 100644 --- a/modules/sharedIdSystem.js +++ b/modules/sharedIdSystem.js @@ -5,12 +5,13 @@ * @requires module:modules/userId */ -import { parseUrl, buildUrl, triggerPixel, logInfo, hasDeviceAccess, generateUUID } from '../src/utils.js'; +import {parseUrl, buildUrl, triggerPixel, logInfo, hasDeviceAccess, generateUUID} from '../src/utils.js'; import {submodule} from '../src/hook.js'; -import { coppaDataHandler } from '../src/adapterManager.js'; +import {coppaDataHandler} from '../src/adapterManager.js'; import {getStorageManager} from '../src/storageManager.js'; import {VENDORLESS_GVLID} from '../src/consentHandler.js'; import {MODULE_TYPE_UID} from '../src/activities/modules.js'; +import {domainOverrideToRootDomain} from '../libraries/domainOverrideToRootDomain/index.js'; export const storage = getStorageManager({moduleType: MODULE_TYPE_UID, moduleName: 'pubCommonId'}); const COOKIE = 'cookie'; @@ -172,31 +173,7 @@ export const sharedIdSystemSubmodule = { } }, - domainOverride: function () { - const domainElements = document.domain.split('.'); - const cookieName = `_gd${Date.now()}`; - for (let i = 0, topDomain, testCookie; i < domainElements.length; i++) { - const nextDomain = domainElements.slice(i).join('.'); - - // write test cookie - storage.setCookie(cookieName, '1', undefined, undefined, nextDomain); - - // read test cookie to verify domain was valid - testCookie = storage.getCookie(cookieName); - - // delete test cookie - storage.setCookie(cookieName, '', 'Thu, 01 Jan 1970 00:00:01 GMT', undefined, nextDomain); - - if (testCookie === '1') { - // cookie was written successfully using test domain so the topDomain is updated - topDomain = nextDomain; - } else { - // cookie failed to write using test domain so exit by returning the topDomain - return topDomain; - } - } - } - + domainOverride: domainOverrideToRootDomain(storage, 'sharedId'), }; submodule('userId', sharedIdSystemSubmodule); diff --git a/test/spec/libraries/domainOverrideToRootDomain/index_spec.js b/test/spec/libraries/domainOverrideToRootDomain/index_spec.js new file mode 100644 index 00000000000..b490d80fd40 --- /dev/null +++ b/test/spec/libraries/domainOverrideToRootDomain/index_spec.js @@ -0,0 +1,77 @@ +import {domainOverrideToRootDomain} from 'libraries/domainOverrideToRootDomain/index.js'; +import {getStorageManager} from 'src/storageManager.js'; +import {MODULE_TYPE_UID} from '../../../../src/activities/modules'; + +const storage = getStorageManager({ moduleName: 'test', moduleType: MODULE_TYPE_UID }); +const domainOverride = domainOverrideToRootDomain(storage, 'test'); + +describe('domainOverride', () => { + let sandbox, domain, cookies, rejectCookiesFor; + let setCookieStub; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(document, 'domain').get(() => domain); + cookies = {}; + sandbox.stub(storage, 'getCookie').callsFake((key) => cookies[key]); + rejectCookiesFor = null; + setCookieStub = sandbox.stub(storage, 'setCookie').callsFake((key, value, expires, sameSite, domain) => { + if (domain !== rejectCookiesFor) { + if (expires != null) { + expires = new Date(expires); + } + if (expires == null || expires > Date.now()) { + cookies[key] = value; + } else { + delete cookies[key]; + } + } + }); + }); + + afterEach(() => sandbox.restore()) + + it('test cookies include the module name', () => { + domain = 'greatpublisher.com' + rejectCookiesFor = 'greatpublisher.com' + + // stub Date.now() to return a constant value + sandbox.stub(Date, 'now').returns(1234567890) + + const randomName = `adapterV${(Math.random() * 1e8).toString(16)}` + const localDomainOverride = domainOverrideToRootDomain(storage, randomName) + + const time = Date.now(); + localDomainOverride(); + + sandbox.assert.callCount(setCookieStub, 2) + sandbox.assert.calledWith(setCookieStub, `_gd${time}_${randomName}`, '1', undefined, undefined, 'greatpublisher.com') + }); + + it('will return the root domain when given a subdomain', () => { + const test_domains = [ + 'deeply.nested.subdomain.for.greatpublisher.com', + 'greatpublisher.com', + 'subdomain.greatpublisher.com', + 'a-subdomain.greatpublisher.com', + ]; + + test_domains.forEach((testDomain) => { + domain = testDomain + rejectCookiesFor = 'com' + expect(domainOverride()).to.equal('greatpublisher.com'); + }); + }); + + it(`If we can't set cookies on the root domain, we'll return the subdomain`, () => { + domain = 'subdomain.greatpublisher.com' + rejectCookiesFor = 'greatpublisher.com' + expect(domainOverride()).to.equal('subdomain.greatpublisher.com'); + }); + + it('Will return undefined if we can\'t set cookies on the root domain or the subdomain', () => { + domain = 'subdomain.greatpublisher.com' + rejectCookiesFor = 'subdomain.greatpublisher.com' + expect(domainOverride()).to.equal(undefined); + }); +}); diff --git a/test/spec/modules/amxIdSystem_spec.js b/test/spec/modules/amxIdSystem_spec.js index dea79e87baa..c1ae2c791d5 100644 --- a/test/spec/modules/amxIdSystem_spec.js +++ b/test/spec/modules/amxIdSystem_spec.js @@ -1,4 +1,4 @@ -import { amxIdSubmodule } from 'modules/amxIdSystem.js'; +import { amxIdSubmodule, storage } from 'modules/amxIdSystem.js'; import { server } from 'test/mocks/xhr.js'; import * as utils from 'src/utils.js'; @@ -48,38 +48,17 @@ describe('validateConfig', () => { logErrorSpy.restore(); }); - it('should return undefined if config.storage is not present', () => { + it('should allow configuration with no storage', () => { expect( amxIdSubmodule.getId( { ...config, - storage: null, + storage: undefined }, null, null ) - ).to.equal(undefined); - - expect(logErrorSpy.calledOnce).to.be.true; - expect(logErrorSpy.lastCall.lastArg).to.contain('storage is required'); - }); - - it('should return undefined if config.storage.type !== "html5"', () => { - expect( - amxIdSubmodule.getId( - { - ...config, - storage: { - type: 'cookie', - }, - }, - null, - null - ) - ).to.equal(undefined); - - expect(logErrorSpy.calledOnce).to.be.true; - expect(logErrorSpy.lastCall.lastArg).to.contain('cookie'); + ).to.not.equal(undefined); }); it('should return undefined if expires > 30', () => { @@ -111,10 +90,18 @@ describe('getId', () => { }); it('should call the sync endpoint and accept a valid response', () => { + storage.setDataInLocalStorage('__amuidpb', TEST_ID); + const { callback } = amxIdSubmodule.getId(config, null, null); callback(spy); const [request] = server.requests; + expect(request.withCredentials).to.be.true + expect(request.requestHeaders['Content-Type']).to.match(/text\/plain/) + + const { search } = utils.parseUrl(request.url); + expect(search.av).to.equal(amxIdSubmodule.version); + expect(search.am).to.equal(TEST_ID); expect(request.method).to.equal('GET'); request.respond( @@ -187,7 +174,7 @@ describe('getId', () => { ); const [, secondRequest] = server.requests; - expect(secondRequest.url).to.be.equal(intermediateValue); + expect(secondRequest.url).to.match(new RegExp(`^${intermediateValue}\?`)); secondRequest.respond( 200, {}, diff --git a/test/spec/modules/sharedIdSystem_spec.js b/test/spec/modules/sharedIdSystem_spec.js index 8ef34a1599e..fcfbe5f7c3f 100644 --- a/test/spec/modules/sharedIdSystem_spec.js +++ b/test/spec/modules/sharedIdSystem_spec.js @@ -91,50 +91,4 @@ describe('SharedId System', function () { expect(result).to.be.undefined; }); }); - - describe('SharedID System domainOverride', () => { - let sandbox, domain, cookies, rejectCookiesFor; - - beforeEach(() => { - sandbox = sinon.createSandbox(); - sandbox.stub(document, 'domain').get(() => domain); - cookies = {}; - sandbox.stub(storage, 'getCookie').callsFake((key) => cookies[key]); - rejectCookiesFor = null; - sandbox.stub(storage, 'setCookie').callsFake((key, value, expires, sameSite, domain) => { - if (domain !== rejectCookiesFor) { - if (expires != null) { - expires = new Date(expires); - } - if (expires == null || expires > Date.now()) { - cookies[key] = value; - } else { - delete cookies[key]; - } - } - }); - }); - - afterEach(() => { - sandbox.restore(); - }); - - it('should return TLD if cookies can be set there', () => { - domain = 'sub.domain.com'; - rejectCookiesFor = 'com'; - expect(sharedIdSystemSubmodule.domainOverride()).to.equal('domain.com'); - }); - - it('should return undefined when cookies cannot be set', () => { - domain = 'sub.domain.com'; - rejectCookiesFor = 'sub.domain.com'; - expect(sharedIdSystemSubmodule.domainOverride()).to.be.undefined; - }); - - it('should return half-way domain if parent domain rejects cookies', () => { - domain = 'inner.outer.domain.com'; - rejectCookiesFor = 'domain.com'; - expect(sharedIdSystemSubmodule.domainOverride()).to.equal('outer.domain.com'); - }); - }); });