-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
33Across User ID sub-module: Introduce first-party ID support #10714
Changes from all commits
34824bb
9ccfa5d
eb0e584
4a74bfd
c815cbd
f16a5fb
21ec0c6
6ffeb3d
54fd3a6
548a480
6e1df19
2293d9d
9c2d7d1
dce6c5a
534871e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,44 +5,53 @@ | |
* @requires module:modules/userId | ||
*/ | ||
|
||
import { logMessage, logError } from '../src/utils.js'; | ||
import { logMessage, logError, logWarn } 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, STORAGE_TYPE_COOKIES, STORAGE_TYPE_LOCALSTORAGE } from '../src/storageManager.js'; | ||
import { MODULE_TYPE_UID } from '../src/activities/modules.js'; | ||
|
||
const MODULE_NAME = '33acrossId'; | ||
const API_URL = 'https://lexicon.33across.com/v1/envelope'; | ||
const AJAX_TIMEOUT = 10000; | ||
const CALLER_NAME = 'pbjs'; | ||
const GVLID = 58; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you're going to abort in GDPR countries, why list a gvlid? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see #10495 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @patmmccann If we remove the GVLID, it looks like it will affect the scenario where all of the following hold:
In theory the publisher's CMP shouldn't indicate that GDPR applies outside of the EU, but in practice I'm not sure whether CMPs reliably report this. Personally I don't see any harm in keeping the GVLID, but if you feel strongly that we should remove it then let us know and we'll do so. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need some way of preventing network traffic in regimes you don't support. Feel free to keep the gvlid and detect gdpr applies and suppress the call. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have made the corresponding changes and now the ID module suppresses the calls where GDPR applies. Changes have been incorporated to this PR and once approved they should fix #10495 |
||
|
||
function getEnvelope(response) { | ||
const STORAGE_FPID_KEY = '33acrossIdFp'; | ||
|
||
export const storage = getStorageManager({ moduleType: MODULE_TYPE_UID, moduleName: MODULE_NAME }); | ||
|
||
function calculateResponseObj(response) { | ||
if (!response.succeeded) { | ||
if (response.error == 'Cookied User') { | ||
logMessage(`${MODULE_NAME}: Unsuccessful response`.concat(' ', response.error)); | ||
} else { | ||
logError(`${MODULE_NAME}: Unsuccessful response`.concat(' ', response.error)); | ||
} | ||
return; | ||
return {}; | ||
} | ||
|
||
if (!response.data.envelope) { | ||
logMessage(`${MODULE_NAME}: No envelope was received`); | ||
|
||
return; | ||
return {}; | ||
} | ||
|
||
return response.data.envelope; | ||
return { | ||
envelope: response.data.envelope, | ||
fp: response.data.fp | ||
}; | ||
} | ||
|
||
function calculateQueryStringParams(pid, gdprConsentData) { | ||
function calculateQueryStringParams(pid, gdprConsentData, storageConfig) { | ||
const uspString = uspDataHandler.getConsentData(); | ||
const gdprApplies = Boolean(gdprConsentData?.gdprApplies); | ||
const coppaValue = coppaDataHandler.getCoppa(); | ||
const gppConsent = gppDataHandler.getConsentData(); | ||
|
||
const params = { | ||
pid, | ||
gdpr: Number(gdprApplies), | ||
gdpr: 0, | ||
src: CALLER_NAME, | ||
ver: '$prebid.version$', | ||
coppa: Number(coppaValue) | ||
|
@@ -63,9 +72,49 @@ function calculateQueryStringParams(pid, gdprConsentData) { | |
params.gdpr_consent = gdprConsentData.consentString; | ||
} | ||
|
||
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()) { | ||
const expirationInMs = 60 * 60 * 24 * 1000 * storageConfig.expires; | ||
const expirationTime = new Date(Date.now() + expirationInMs); | ||
|
||
storage.setCookie(key, value, expirationTime.toUTCString(), 'Lax'); | ||
} 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) { | ||
return storage.getDataFromLocalStorage(key); | ||
} | ||
} | ||
|
||
function handleFpId(fpId, storageConfig = {}) { | ||
fpId | ||
? storeValue(STORAGE_FPID_KEY, fpId, storageConfig) | ||
: deleteFromStorage(STORAGE_FPID_KEY); | ||
} | ||
|
||
/** @type {Submodule} */ | ||
export const thirthyThreeAcrossIdSubmodule = { | ||
/** | ||
|
@@ -74,7 +123,7 @@ export const thirthyThreeAcrossIdSubmodule = { | |
*/ | ||
name: MODULE_NAME, | ||
|
||
gvlid: 58, | ||
gvlid: GVLID, | ||
|
||
/** | ||
* decode the stored id value for passing to bid requests | ||
|
@@ -96,34 +145,49 @@ export const thirthyThreeAcrossIdSubmodule = { | |
* @param {SubmoduleConfig} [config] | ||
* @returns {IdResponse|undefined} | ||
*/ | ||
getId({ params = { } }, gdprConsentData) { | ||
getId({ params = { }, storage: storageConfig }, gdprConsentData) { | ||
if (typeof params.pid !== 'string') { | ||
logError(`${MODULE_NAME}: Submodule requires a partner ID to be defined`); | ||
|
||
return; | ||
} | ||
|
||
const { pid, apiUrl = API_URL } = params; | ||
if (gdprConsentData?.gdprApplies === true) { | ||
logWarn(`${MODULE_NAME}: Submodule cannot be used where GDPR applies`); | ||
|
||
return; | ||
} | ||
|
||
const { pid, storeFpid, apiUrl = API_URL } = params; | ||
|
||
return { | ||
callback(cb) { | ||
ajaxBuilder(AJAX_TIMEOUT)(apiUrl, { | ||
success(response) { | ||
let envelope; | ||
let responseObj = { }; | ||
|
||
try { | ||
envelope = getEnvelope(JSON.parse(response)) | ||
responseObj = calculateResponseObj(JSON.parse(response)); | ||
} catch (err) { | ||
logError(`${MODULE_NAME}: ID reading error:`, err); | ||
} | ||
cb(envelope); | ||
|
||
if (!responseObj.envelope) { | ||
deleteFromStorage(MODULE_NAME); | ||
} | ||
|
||
if (storeFpid) { | ||
handleFpId(responseObj.fp, storageConfig); | ||
} | ||
|
||
cb(responseObj.envelope); | ||
}, | ||
error(err) { | ||
logError(`${MODULE_NAME}: ID error response`, err); | ||
|
||
cb(); | ||
} | ||
}, calculateQueryStringParams(pid, gdprConsentData), { method: 'GET', withCredentials: true }); | ||
}, calculateQueryStringParams(pid, gdprConsentData, storageConfig), { method: 'GET', withCredentials: true }); | ||
} | ||
}; | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
publishers who evaluated your module before and arent comfortable with these functions might consider this breaking in a minor version. Please put all this new functionality behind a configuration flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@patmmccann When you say configuration flag, do you mean a module configuration flag that's passed to pbjs.setConfig() or a compile-time feature flag that lives in features.json?
Can we default its value to true if unspecified as long as we provide clear messaging and documentation for this fiag to existing publishers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No; you should flip the default in a major version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Configuration flag has been added, for the moment the default value is false and will be changed to true in a major version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent, that branch will open in April