-
Notifications
You must be signed in to change notification settings - Fork 2.2k
GDPR consentManagement module #2213
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
Changes from 18 commits
bcf27d8
cda55fe
77de2e5
ebe1ee8
3cda142
6337a2e
78f0e9c
2412b4f
b8c1da6
173d945
6f57a89
83a4ed9
2a587eb
2018378
0081610
a46ca14
6739fd0
63d0d21
effc19c
e6d8068
326e712
112a61b
91b6d83
a425228
f273018
2e465fd
540b4b5
4de3df0
7f78734
d6a4807
8d23307
2ccfedf
a96b129
b7811f8
9a1f09b
73d02f0
5ae5eff
78fcc64
c39b12d
213f2fa
405a6f2
fc41a5a
296e9ca
750797a
effa5b5
a3ca63f
4bc0f9b
4a6e273
2f32574
f042d7c
56f6df4
1a35235
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 |
---|---|---|
@@ -0,0 +1,142 @@ | ||
import * as utils from 'src/utils'; | ||
import { config } from 'src/config'; | ||
import { setTimeout } from 'core-js/library/web/timers'; | ||
|
||
// add new CMPs here | ||
const availCMPs = ['iab']; | ||
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 should probably call this 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. updated references of |
||
|
||
export let userCMP; | ||
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. I don't think these vars need to be exported? 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. I'm exporting these vars so I can read them in the |
||
export let consentId = ''; | ||
export let consentTimeout; | ||
export let lookUpFailureChoice; | ||
|
||
export function requestBidsHook(config, fn) { | ||
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. would like to see JSdocs notation on exported functions. 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. Added some more descriptive information to the exported functions. |
||
let adUnits = config.adUnits || $$PREBID_GLOBAL$$.adUnits; | ||
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. should we be using 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. I removed the 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. update I need to have this value in place to actually grab the adUnits list object. The config attribute that's included in the @mkendall07 Please let me know if there are any strong concerns about relying on |
||
let context = this; | ||
let args = arguments; | ||
let nextFn = fn; | ||
let cmpActive = true; | ||
let _timer; | ||
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. use underscore consistently please. All or none 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. Removed the underscore for this var to make it consistent with the others. |
||
|
||
// in case we already have consent (eg during bid refresh) | ||
if (consentId) { | ||
adUnits = applyConsent(consentId); | ||
return fn.apply(context, args); | ||
} | ||
|
||
// ensure user requested supported cmp, otherwise abort and return to hooked function | ||
if (!availCMPs.includes(userCMP)) { | ||
utils.logWarn(`CMP framework ${userCMP} is not a supported framework. Aborting consentManagement module and resuming auction.`); | ||
return fn.apply(context, args); | ||
} | ||
|
||
// start of CMP specific logic | ||
if (userCMP === 'iab') { | ||
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. can you move this logic to a separate function and define at the top of the file. That will make it easier / cleaner for other CMPs to integrate. 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. I have reorganized the functions, please take a look on the next commit and let me know what you think. |
||
if (!window.__cmp) { | ||
utils.logWarn('IAB CMP framework is not detected. Aborting consentManagement module and resuming auction.'); | ||
return fn.apply(context, args); | ||
} | ||
|
||
lookupIabId(); | ||
} | ||
|
||
function lookupIabId () { | ||
// lookup times and user interaction with CMP prompts can greatly vary, so enforcing a timeout on the CMP process | ||
_timer = setTimeout(cmpTimedOut, consentTimeout); | ||
|
||
// first lookup - to determine if new or existing user | ||
// if new user, then wait for user to make a choice and then run postLookup method | ||
// if existing user, then skip to postLookup method | ||
window.__cmp('getConsentData', 'vendorConsents', function(consentString) { | ||
if (cmpActive) { | ||
if (consentString === null || !consentString) { | ||
window.__cmp('addEventListener', 'onSubmit', function() { | ||
if (cmpActive) { | ||
// redo lookup to find new string based on user's choices | ||
window.__cmp('getConsentData', 'vendorConsents', postLookup); | ||
} | ||
}); | ||
} else { | ||
postLookup(consentString); | ||
} | ||
} | ||
}); | ||
} | ||
|
||
// after we have grabbed ideal ID from CMP, apply the data to adUnits object and finish up the module | ||
function postLookup(consentString) { | ||
if (cmpActive) { | ||
if (typeof consentString != 'string' || consentString === '') { | ||
exitFailedCMP(`CMP returned unexpected value during lookup process; returned value was (${consentString}).`); | ||
} else { | ||
clearTimeout(_timer); | ||
consentId = consentString; | ||
adUnits = applyConsent(consentString); | ||
fn.apply(context, args); | ||
} | ||
} | ||
} | ||
|
||
// assuming we have valid consent ID, apply to adUnits object | ||
function applyConsent(consentString) { | ||
adUnits.forEach(adUnit => { | ||
adUnit['gdprConsent'] = { | ||
consentString: consentString, | ||
consentRequired: true | ||
}; | ||
}); | ||
return adUnits; | ||
} | ||
|
||
function cmpTimedOut () { | ||
if (cmpActive) { | ||
exitFailedCMP('CMP workflow exceeded timeout threshold.'); | ||
} | ||
} | ||
|
||
function exitFailedCMP(message) { | ||
cmpActive = false; | ||
if (lookUpFailureChoice === 'proceed') { | ||
utils.logWarn(message + ' Resuming auction without consent data as per consentManagement config.'); | ||
adUnits = applyConsent(undefined); | ||
nextFn.apply(context, args); | ||
} else { | ||
utils.logError(message + ' Canceling auction as per consentManagement config.'); | ||
} | ||
} | ||
} | ||
|
||
export function resetConsentId() { | ||
consentId = ''; | ||
} | ||
|
||
export function setConfig(config) { | ||
if (typeof config.cmp === 'string') { | ||
userCMP = config.cmp; | ||
} else { | ||
userCMP = 'iab'; | ||
utils.logInfo(`consentManagement config did not specify cmp. Using system default setting (${userCMP}).`); | ||
} | ||
|
||
if (typeof config.waitForConsentTimeout === 'number') { | ||
consentTimeout = config.waitForConsentTimeout; | ||
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. I think this should just be called 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. Good point, I changed the name to |
||
} else { | ||
consentTimeout = 5000; | ||
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. defaults like these should be made constant values and defined at the top of the file 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. I have created default variables and utilized them in the setConfig part of the module code in place of the actual values. |
||
utils.logInfo(`consentManagement config did not specify waitForConsentTimeout. Using system default setting (${consentTimeout}).`); | ||
} | ||
|
||
if (typeof config.lookUpFailureResolution === 'string') { | ||
if (config.lookUpFailureResolution === 'proceed' || config.lookUpFailureResolution === 'cancel') { | ||
lookUpFailureChoice = config.lookUpFailureResolution; | ||
} else { | ||
lookUpFailureChoice = 'proceed'; | ||
utils.logWarn(`Invalid choice was set for consentManagement lookUpFailureResolution property. Using system default (${lookUpFailureChoice}).`); | ||
} | ||
} else { | ||
lookUpFailureChoice = 'proceed'; | ||
utils.logInfo(`consentManagement config did not specify lookUpFailureResolution. Using system default setting (${lookUpFailureChoice}).`); | ||
} | ||
|
||
$$PREBID_GLOBAL$$.requestBids.addHook(requestBidsHook, 50); | ||
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. I always forget what the 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 comment above for the change in the |
||
} | ||
config.getConfig('consentManagement', config => setConfig(config.consentManagement)); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -124,7 +124,7 @@ pbjs.requestBids.addHook((config, next = config) => { | |
} else { | ||
logWarn(`${MODULE_NAME} module: concurrency has been disabled and "$$PREBID_GLOBAL$$.requestBids" call was queued`); | ||
} | ||
}, 100); | ||
}, 5); | ||
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. what's the impact of this change? 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. The number (also relates to the other comment below) acts as a priority level for the hooked functions (when there are multiple hooked functions on the same hook). The Having the The 5 specifically is lower than the default priority that's set for any hooked function (which is 10), but still higher than the base function |
||
|
||
Object.keys(auctionPropMap).forEach(prop => { | ||
if (prop === 'allBidsAvailable') { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -211,6 +211,11 @@ exports.makeBidRequests = function(adUnits, auctionStart, auctionId, cbTimeout, | |
bidRequests.push(bidderRequest); | ||
} | ||
}); | ||
if (adUnits[0].gdprConsent) { | ||
bidRequests.forEach(bidRequest => { | ||
bidRequest['gdprConsent'] = adUnits[0].gdprConsent; | ||
}); | ||
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. I wonder if there's a better way to do this.... |
||
} | ||
return bidRequests; | ||
}; | ||
|
||
|
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.
What's the purpose of this? All the browsers we support seem to support
setTimeout
in the way you are using it w/o this polyfill.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.
I removed it, it got added automatically.