Skip to content

Commit

Permalink
ID5 User ID module - don't send empty fields to server (prebid#6581)
Browse files Browse the repository at this point in the history 
* for optional fields, only send to servers if populated

* adding some logging to id5 submodule to make debugging easier

* update user id integration example for id5 to user html5 not cookie

* remove .only from tests

* remove checks for loginfo since now we are using it more broadly
  • Loading branch information
@smenzer @umakajan
smenzer authored and umakajan committed May 6, 2021
1 parent aef569c commit 5d603eb
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 34 deletions.
2 changes: 1 addition & 1 deletion integrationExamples/gpt/userId_example.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
partner: 173 //Set your real ID5 partner ID here for production, please ask for one at http://id5.io/prebid
},
storage: {
type: "cookie",
type: "html5",
name: "id5id",
expires: 90,
refreshInSeconds: 8*3600 // Refresh frequency of cookies, defaulting to 'expires'
Expand Down
70 changes: 48 additions & 22 deletions modules/id5IdSystem.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ export const ID5_STORAGE_NAME = 'id5id';
export const ID5_PRIVACY_STORAGE_NAME = `${ID5_STORAGE_NAME}_privacy`;
const LOCAL_STORAGE = 'html5';
const ABTEST_RESOLUTION = 10000;
const LOG_PREFIX = 'User ID - ID5 submodule: ';

// order the legacy cookie names in reverse priority order so the last
// cookie in the array is the most preferred to use
const LEGACY_COOKIE_NAMES = [ 'pbjs-id5id', 'id5id.1st' ];
const LEGACY_COOKIE_NAMES = [ 'pbjs-id5id', 'id5id.1st', 'id5id' ];

const storage = getStorageManager(GVLID, MODULE_NAME);

Expand Down Expand Up @@ -63,15 +64,15 @@ export const id5IdSubmodule = {
const controlGroup = isInControlGroup(universalUid, abConfig.controlGroupPct);
if (abConfig.enabled === true && typeof controlGroup === 'undefined') {
// A/B Testing is enabled, but configured improperly, so skip A/B testing
utils.logError('User ID - ID5 submodule: A/B Testing controlGroupPct must be a number >= 0 and <= 1! Skipping A/B Testing');
utils.logError(LOG_PREFIX + 'A/B Testing controlGroupPct must be a number >= 0 and <= 1! Skipping A/B Testing');
} else if (abConfig.enabled === true && controlGroup === true) {
// A/B Testing is enabled and user is in the Control Group, so do not share the ID5 ID
utils.logInfo('User ID - ID5 submodule: A/B Testing Enabled - user is in the Control Group, so the ID5 ID is NOT exposed');
utils.logInfo(LOG_PREFIX + 'A/B Testing Enabled - user is in the Control Group, so the ID5 ID is NOT exposed');
universalUid = '';
linkType = 0;
} else if (abConfig.enabled === true) {
// A/B Testing is enabled but user is not in the Control Group, so ID5 ID is shared
utils.logInfo('User ID - ID5 submodule: A/B Testing Enabled - user is NOT in the Control Group, so the ID5 ID is exposed');
utils.logInfo(LOG_PREFIX + 'A/B Testing Enabled - user is NOT in the Control Group, so the ID5 ID is exposed');
}

let responseObj = {
Expand All @@ -87,6 +88,8 @@ export const id5IdSubmodule = {
utils.deepSetValue(responseObj, 'id5id.ext.abTestingControlGroup', (typeof controlGroup === 'undefined' ? false : controlGroup));
}

utils.logInfo(LOG_PREFIX + 'Decoded ID', responseObj);

return responseObj;
},

Expand All @@ -105,24 +108,38 @@ export const id5IdSubmodule = {

const url = `https://id5-sync.com/g/v2/${config.params.partner}.json`;
const hasGdpr = (consentData && typeof consentData.gdprApplies === 'boolean' && consentData.gdprApplies) ? 1 : 0;
const usp = uspDataHandler.getConsentData();
const referer = getRefererInfo();
const signature = (cacheIdObj && cacheIdObj.signature) ? cacheIdObj.signature : getLegacyCookieSignature();
const data = {
'gdpr': hasGdpr,
'gdpr_consent': hasGdpr ? consentData.consentString : '',
'partner': config.params.partner,
'gdpr': hasGdpr,
'nbPage': incrementNb(config.params.partner),
'o': 'pbjs',
'pd': config.params.pd || '',
'provider': config.params.provider || '',
'rf': referer.referer,
's': signature,
'top': referer.reachedTop ? 1 : 0,
'u': referer.stack[0] || window.location.href,
'us_privacy': uspDataHandler.getConsentData() || '',
'v': '$prebid.version$'
};

// pass in optional data, but only if populated
if (hasGdpr && typeof consentData.consentString !== 'undefined' && !utils.isEmpty(consentData.consentString) && !utils.isEmptyStr(consentData.consentString)) {
data.gdpr_consent = consentData.consentString;
}
if (typeof usp !== 'undefined' && !utils.isEmpty(usp) && !utils.isEmptyStr(usp)) {
data.us_privacy = usp;
}
if (typeof signature !== 'undefined' && !utils.isEmptyStr(signature)) {
data.s = signature;
}
if (typeof config.params.pd !== 'undefined' && !utils.isEmptyStr(config.params.pd)) {
data.pd = config.params.pd;
}
if (typeof config.params.provider !== 'undefined' && !utils.isEmptyStr(config.params.provider)) {
data.provider = config.params.provider;
}

// pass in feature flags, if applicable
if (getAbTestingConfig(config).enabled === true) {
utils.deepSetValue(data, 'features.ab', 1);
}
Expand All @@ -134,7 +151,10 @@ export const id5IdSubmodule = {
if (response) {
try {
responseObj = JSON.parse(response);
utils.logInfo(LOG_PREFIX + 'response received from the server', responseObj);

resetNb(config.params.partner);

if (responseObj.privacy) {
storeInLocalStorage(ID5_PRIVACY_STORAGE_NAME, JSON.stringify(responseObj.privacy), NB_EXP_DAYS);
}
Expand All @@ -145,16 +165,17 @@ export const id5IdSubmodule = {
removeLegacyCookies(config.params.partner);
}
} catch (error) {
utils.logError(error);
utils.logError(LOG_PREFIX + error);
}
}
callback(responseObj);
},
error: error => {
utils.logError(`User ID - ID5 submodule getId fetch encountered an error`, error);
utils.logError(LOG_PREFIX + 'getId fetch encountered an error', error);
callback();
}
};
utils.logInfo(LOG_PREFIX + 'requesting an ID from the server', data);
ajax(url, callbacks, JSON.stringify(data), { method: 'POST', withCredentials: true });
};
return {callback: resp};
Expand All @@ -172,30 +193,34 @@ export const id5IdSubmodule = {
* @return {(IdResponse|function(callback:function))} A response object that contains id and/or callback.
*/
extendId(config, consentData, cacheIdObj) {
hasRequiredConfig(config);

const partnerId = (config && config.params && config.params.partner) || 0;
incrementNb(partnerId);

utils.logInfo(LOG_PREFIX + 'using cached ID', cacheIdObj);
return cacheIdObj;
}
};

function hasRequiredConfig(config) {
if (!config || !config.params || !config.params.partner || typeof config.params.partner !== 'number') {
utils.logError(`User ID - ID5 submodule requires partner to be defined as a number`);
utils.logError(LOG_PREFIX + 'partner required to be defined as a number');
return false;
}

if (!config.storage || !config.storage.type || !config.storage.name) {
utils.logError(`User ID - ID5 submodule requires storage to be set`);
utils.logError(LOG_PREFIX + 'storage required to be set');
return false;
}

// TODO: in a future release, return false if storage type or name are not set as required
// in a future release, we may return false if storage type or name are not set as required
if (config.storage.type !== LOCAL_STORAGE) {
utils.logWarn(`User ID - ID5 submodule recommends storage type to be '${LOCAL_STORAGE}'. In a future release this will become a strict requirement`);
utils.logWarn(LOG_PREFIX + `storage type recommended to be '${LOCAL_STORAGE}'. In a future release this may become a strict requirement`);
}
// TODO: in a future release, return false if storage type or name are not set as required
// in a future release, we may return false if storage type or name are not set as required
if (config.storage.name !== ID5_STORAGE_NAME) {
utils.logWarn(`User ID - ID5 submodule recommends storage name to be '${ID5_STORAGE_NAME}'. In a future release this will become a strict requirement`);
utils.logWarn(LOG_PREFIX + `storage name recommended to be '${ID5_STORAGE_NAME}'. In a future release this may become a strict requirement`);
}

return true;
Expand Down Expand Up @@ -240,11 +265,12 @@ function getLegacyCookieSignature() {
* @param {integer} partnerId
*/
function removeLegacyCookies(partnerId) {
utils.logInfo(LOG_PREFIX + 'removing legacy cookies');
LEGACY_COOKIE_NAMES.forEach(function(cookie) {
storage.setCookie(`${cookie}`, '', expDaysStr(-1));
storage.setCookie(`${cookie}_nb`, '', expDaysStr(-1));
storage.setCookie(`${cookie}_${partnerId}_nb`, '', expDaysStr(-1));
storage.setCookie(`${cookie}_last`, '', expDaysStr(-1));
storage.setCookie(`${cookie}`, ' ', expDaysStr(-1));
storage.setCookie(`${cookie}_nb`, ' ', expDaysStr(-1));
storage.setCookie(`${cookie}_${partnerId}_nb`, ' ', expDaysStr(-1));
storage.setCookie(`${cookie}_last`, ' ', expDaysStr(-1));
});
}

Expand Down
20 changes: 9 additions & 11 deletions test/spec/modules/id5IdSystem_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,26 +144,26 @@ describe('ID5 ID System', function() {
expect(request.withCredentials).to.be.true;
expect(requestBody.partner).to.eq(ID5_TEST_PARTNER_ID);
expect(requestBody.o).to.eq('pbjs');
expect(requestBody.pd).to.eq('');
expect(requestBody.s).to.eq('');
expect(requestBody.provider).to.eq('');
expect(requestBody.pd).to.be.undefined;
expect(requestBody.s).to.be.undefined;
expect(requestBody.provider).to.be.undefined
expect(requestBody.v).to.eq('$prebid.version$');
expect(requestBody.gdpr).to.exist;
expect(requestBody.gdpr_consent).to.exist
expect(requestBody.us_privacy).to.exist;
expect(requestBody.gdpr_consent).to.be.undefined;
expect(requestBody.us_privacy).to.be.undefined;

request.respond(200, responseHeader, JSON.stringify(ID5_JSON_RESPONSE));
expect(callbackSpy.calledOnce).to.be.true;
expect(callbackSpy.lastCall.lastArg).to.deep.equal(ID5_JSON_RESPONSE);
});

it('should call the ID5 server with empty signature field when no stored object', function () {
it('should call the ID5 server with no signature field when no stored object', function () {
let submoduleCallback = id5IdSubmodule.getId(getId5FetchConfig(), undefined, undefined).callback;
submoduleCallback(callbackSpy);

let request = server.requests[0];
let requestBody = JSON.parse(request.requestBody);
expect(requestBody.s).to.eq('');
expect(requestBody.s).to.be.undefined;

request.respond(200, responseHeader, JSON.stringify(ID5_JSON_RESPONSE));
});
Expand Down Expand Up @@ -195,7 +195,7 @@ describe('ID5 ID System', function() {
request.respond(200, responseHeader, JSON.stringify(ID5_JSON_RESPONSE));
});

it('should call the ID5 server with empty pd field when pd config is not set', function () {
it('should call the ID5 server with no pd field when pd config is not set', function () {
let id5Config = getId5FetchConfig();
id5Config.params.pd = undefined;

Expand All @@ -204,7 +204,7 @@ describe('ID5 ID System', function() {

let request = server.requests[0];
let requestBody = JSON.parse(request.requestBody);
expect(requestBody.pd).to.eq('');
expect(requestBody.pd).to.be.undefined;

request.respond(200, responseHeader, JSON.stringify(ID5_JSON_RESPONSE));
});
Expand Down Expand Up @@ -505,7 +505,6 @@ describe('ID5 ID System', function() {
let decoded = id5IdSubmodule.decode(ID5_STORED_OBJ, testConfig);
expect(decoded).to.deep.equal(expectedDecodedObjectWithIdAbOff);
sinon.assert.notCalled(logErrorSpy);
sinon.assert.notCalled(logInfoSpy);
});
});

Expand All @@ -523,7 +522,6 @@ describe('ID5 ID System', function() {
testConfig.params.abTesting = testAbTestingConfig;
id5IdSubmodule.decode(ID5_STORED_OBJ, testConfig);
sinon.assert.notCalled(logErrorSpy);
sinon.assert.calledOnce(logInfoSpy);
});
});
});
Expand Down

0 comments on commit 5d603eb

Please sign in to comment.