Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

33Across UIM: Add cookie storage support for first-party ID #78

Merged
merged 2 commits into from
Nov 20, 2023
Merged

33Across UIM: Add cookie storage support for first-party ID #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c815cbd
33Across User ID sub-module: Add cookie storage support for first-par…
carlosfelix Nov 17, 2023
f16a5fb
33Across User ID sub-module: Also remove first-party ID from cookie s…
carlosfelix Nov 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 41 additions & 10 deletions modules/33acrossIdSystem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { logMessage, logError } from '../src/utils.js';
import { ajaxBuilder } from '../src/ajax.js';
import { submodule } from '../src/hook.js';
import { uspDataHandler, coppaDataHandler, gppDataHandler } from '../src/adapterManager.js';
import { getStorageManager } from '../src/storageManager.js';
import { getStorageManager, STORAGE_TYPE_COOKIES, STORAGE_TYPE_LOCALSTORAGE } from '../src/storageManager.js';
import { MODULE_TYPE_UID } from '../src/activities/modules.js';

const MODULE_NAME = '33acrossId';
Expand Down Expand Up @@ -44,7 +44,7 @@ function calculateResponseObj(response) {
};
}

function calculateQueryStringParams(pid, gdprConsentData) {
function calculateQueryStringParams(pid, gdprConsentData, storageConfig) {
const uspString = uspDataHandler.getConsentData();
const gdprApplies = Boolean(gdprConsentData?.gdprApplies);
const coppaValue = coppaDataHandler.getCoppa();
Expand Down Expand Up @@ -73,14 +73,49 @@ function calculateQueryStringParams(pid, gdprConsentData) {
params.gdpr_consent = gdprConsentData.consentString;
}

const fp = storage.getDataFromLocalStorage(STORAGE_FPID_KEY);
const fp = getStoredValue(STORAGE_FPID_KEY, storageConfig);
if (fp) {
params.fp = fp;
}

return params;
}

function deleteFromStorage(key) {
if (storage.cookiesAreEnabled()) {
const expiredDate = new Date(0).toUTCString();

storage.setCookie(key, '', expiredDate, 'Lax');
}

storage.removeDataFromLocalStorage(key);
}

function storeValue(key, value, storageConfig = {}) {
if (storageConfig.type === STORAGE_TYPE_COOKIES && storage.cookiesAreEnabled()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is storage is default of {} or type is not set? Also what is the expected structure of storageConfig the pub should pass? Have we documented this in the md?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we won't be storing any values if the storage is {} or the type is not set.. that's the behavior that can be found in the baseID module modules/userId/index.js as well.

The recommended structure is part of the documentation --> https://docs.prebid.org/dev-docs/modules/userid-submodules/33across.html

pbjs.setConfig({
  userSync: {
    userIds: [{
      name: "33acrossId",
      params: {
        pid: "0010b00002GYU4eBAH" // Example ID
      },
      storage: {
        name: "33acrossId",
        type: "html5",
        expires: 90,
        refreshInSeconds: 8 * 3600
      }
    }]
  }
});

const expirationInMs = 60 * 60 * 24 * 1000 * storageConfig.expires;
const expirationTime = new Date(Date.now() + expirationInMs);

storage.setCookie(key, value, expirationTime.toUTCString(), 'Lax');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we are not specifying an domain per

Prebid.js/src/storageManager.js

Line 56 in db9d236

* If not specified, defaults to the host portion of the current document location.
it will default to hostname which I am assuming will also include subdomain if the location url includes that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's correct, the subdomain is included. I've verified this behavior and that's how most of the IdSystems set the cookies, without a hostname, since that part is handled by the storage manager util.

} else if (storageConfig.type === STORAGE_TYPE_LOCALSTORAGE) {
storage.setDataInLocalStorage(key, value);
}
}

function getStoredValue(key, storageConfig = {}) {
if (storageConfig.type === STORAGE_TYPE_COOKIES && storage.cookiesAreEnabled()) {
return storage.getCookie(key);
} else if (storageConfig.type === STORAGE_TYPE_LOCALSTORAGE) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about checking whether LocalStorage is enabled via localStorageIsEnabled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the case of local storage, each function checks internally if local storage storage is enabled

e.g., here is how setDataInLocalStorage is defined:

 const setDataInLocalStorage = function (key, value, done) {
    let cb = function (result) {
      if (result && result.valid && hasLocalStorage()) { // <-- hasLocalStorage is called
        window.localStorage.setItem(key, value);
      }
    }
    return schedule(cb, STORAGE_TYPE_LOCALSTORAGE, done);
  }

return storage.getDataFromLocalStorage(key);
}
}

function handleFpId(fpId, storageConfig = {}) {
fpId
? storeValue(STORAGE_FPID_KEY, fpId, storageConfig)
: deleteFromStorage(STORAGE_FPID_KEY);
}

/** @type {Submodule} */
export const thirthyThreeAcrossIdSubmodule = {
/**
Expand Down Expand Up @@ -111,7 +146,7 @@ export const thirthyThreeAcrossIdSubmodule = {
* @param {SubmoduleConfig} [config]
* @returns {IdResponse|undefined}
*/
getId({ params = { } }, gdprConsentData) {
getId({ params = { }, storage }, gdprConsentData) {
if (typeof params.pid !== 'string') {
logError(`${MODULE_NAME}: Submodule requires a partner ID to be defined`);

Expand All @@ -132,11 +167,7 @@ export const thirthyThreeAcrossIdSubmodule = {
logError(`${MODULE_NAME}: ID reading error:`, err);
}

if (responseObj.fp) {
storage.setDataInLocalStorage(STORAGE_FPID_KEY, responseObj.fp);
} else {
storage.removeDataFromLocalStorage(STORAGE_FPID_KEY);
}
handleFpId(responseObj.fp, storage);

cb(responseObj.envelope);
},
Expand All @@ -145,7 +176,7 @@ export const thirthyThreeAcrossIdSubmodule = {

cb();
}
}, calculateQueryStringParams(pid, gdprConsentData), { method: 'GET', withCredentials: true });
}, calculateQueryStringParams(pid, gdprConsentData, storage), { method: 'GET', withCredentials: true });
}
};
},
Expand Down
127 changes: 120 additions & 7 deletions test/spec/modules/33acrossIdSystem_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,35 +51,119 @@ describe('33acrossIdSystem', () => {
});

context('if the response includes a first-party ID', () => {
it('should store the first-party ID provided in the response', () => {
context('and the storage type is "cookie"', () => {
it('should store the provided first-party ID in a cookie', () => {
const completeCallback = () => {};

const { callback } = thirthyThreeAcrossIdSubmodule.getId({
params: {
pid: '12345'
},
storage: {
type: 'cookie',
expires: 90
}
});

callback(completeCallback);

const [request] = server.requests;

const setCookie = sinon.stub(storage, 'setCookie');
const cookiesAreEnabled = sinon.stub(storage, 'cookiesAreEnabled').returns(true);

request.respond(200, {
'Content-Type': 'application/json'
}, JSON.stringify({
succeeded: true,
data: {
envelope: 'foo',
fp: 'bar'
},
expires: 1645667805067
}));

expect(setCookie.calledOnceWithExactly('33acrossIdFp', 'bar', sinon.match.string, 'Lax')).to.be.true;

setCookie.restore();
cookiesAreEnabled.restore();
});
});

context('and the storage type is "html5"', () => {
it('should store the provided first-party ID in local storage', () => {
const completeCallback = () => {};

const { callback } = thirthyThreeAcrossIdSubmodule.getId({
params: {
pid: '12345'
},
storage: {
type: 'html5'
}
});

callback(completeCallback);

const [request] = server.requests;

const setDataInLocalStorage = sinon.stub(storage, 'setDataInLocalStorage');

request.respond(200, {
'Content-Type': 'application/json'
}, JSON.stringify({
succeeded: true,
data: {
envelope: 'foo',
fp: 'bar'
},
expires: 1645667805067
}));

expect(setDataInLocalStorage.calledOnceWithExactly('33acrossIdFp', 'bar')).to.be.true;

setDataInLocalStorage.restore();
});
});
});

context('if the response doesn\'t include a first-party ID', () => {
it('should try to remove any first-party ID that could be stored', () => {
const completeCallback = () => {};

const { callback } = thirthyThreeAcrossIdSubmodule.getId({
params: {
pid: '12345'
},
storage: {
type: 'html5'
}
});

callback(completeCallback);

const [request] = server.requests;

const setDataInLocalStorage = sinon.stub(storage, 'setDataInLocalStorage');
const removeDataFromLocalStorage = sinon.stub(storage, 'removeDataFromLocalStorage');
const setCookie = sinon.stub(storage, 'setCookie');
const cookiesAreEnabled = sinon.stub(storage, 'cookiesAreEnabled').returns(true);

request.respond(200, {
'Content-Type': 'application/json'
}, JSON.stringify({
succeeded: true,
data: {
envelope: 'foo',
fp: 'bar'
envelope: 'foo' // no 'fp' field
},
expires: 1645667805067
}));

expect(setDataInLocalStorage.calledOnceWithExactly('33acrossIdFp', 'bar')).to.be.true;
expect(removeDataFromLocalStorage.calledOnceWithExactly('33acrossIdFp')).to.be.true;
expect(setCookie.calledOnceWithExactly('33acrossIdFp', '', sinon.match.string, 'Lax')).to.be.true;

setDataInLocalStorage.restore();
removeDataFromLocalStorage.restore();
setCookie.restore();
cookiesAreEnabled.restore();
});
});

Expand Down Expand Up @@ -317,12 +401,15 @@ describe('33acrossIdSystem', () => {
});
});

context('when a first-party ID is present in storage', () => {
context('when a first-party ID is present in local storage', () => {
it('should call endpoint with the first-party ID included', () => {
const completeCallback = () => {};
const { callback } = thirthyThreeAcrossIdSubmodule.getId({
params: {
pid: '12345'
},
storage: {
type: 'html5'
}
});

Expand All @@ -340,6 +427,32 @@ describe('33acrossIdSystem', () => {
});
});

context('when a first-party ID is present in cookie storage', () => {
it('should call endpoint with the first-party ID included', () => {
const completeCallback = () => {};
const { callback } = thirthyThreeAcrossIdSubmodule.getId({
params: {
pid: '12345'
},
storage: {
type: 'cookie'
}
});

sinon.stub(storage, 'getCookie')
.withArgs('33acrossIdFp')
.returns('33acrossIdFpValue');

callback(completeCallback);

const [request] = server.requests;

expect(request.url).to.contain('fp=33acrossIdFpValue');

storage.getCookie.restore();
});
});

context('when a first-party ID is not present in storage', () => {
it('should not call endpoint with the first-party ID included', () => {
const completeCallback = () => {};
Expand Down