Prebid Core Storage Manager: add gdpr check before device write tests to ensure purpose1 gets respected #8410
Conversation
|
I would need assistance for that. The test all access storage regardless of consent, but since tcf one cannot rely on granted access so all of the tests are some kind of incomplete as they do not respect the users choice. IMO, either the test should use a mockedup constent string or anything else, but I'm not able to take care of this. |
patmmccann
changed the title
|
Our model has been to put stuff like this in the gdpr module(s), eg the enforcement module, as many publishers don't load gdpr related code outside of gdpr countries. |
|
@renebaudisch do you have concerns with the enforcement module or the device access flag now? It's not clear what issue this pr is trying to solve. |
|
Please reopen and engage if you're looking for a path to merge |
|
The problem we need to solve is that the "userSync" module initially calls "cookiesAreEnabled" from the sessionManager what right now is writing and reading the device storage regardless of consent. We need to ensure that this check respects the given laws and if needed checks consent before doing this write test. I also cannot reopen the pr. |
|
Indicates this was solved in #6323 |
|
@patmmccann This PR is to not let pbjs write cookieTest without having consent amd addresses this privacy part of #6323. |
|
@Fawke could you comment? Do we need to reopen 6289? |
|
I prepared a testpage where you can see that before the cmp is shown the LS get's written with cookieTest: If it is too fast, set a breakpoint here: |
src/storageManager.js
Outdated
| /** | ||
| * @param {string} consentData | ||
| */ | ||
| const hasPurposeOneConsent = function(consentData) { |
There was a problem hiding this comment.
There is already a similar function in src/utils/gpdr.js under the name hasPurpose1Consent. Would you be able to use that function instead of redefining it here?
src/userSync.js
Outdated
| @@ -315,7 +315,9 @@ export function newUserSync(userSyncDependencies) { | |||
| return publicApi; | |||
| } | |||
|
|
|||
| const browserSupportsCookies = !isSafariBrowser() && storage.cookiesAreEnabled(); | |||
| const browserSupportsCookies = function() { | |||
There was a problem hiding this comment.
Does the type of this variable really need to change to a function, instead of its boolean resolution?
If so, can you please update the corresponding unit tests in test/spec/userSync_spec.js to update the signature references it has for this param (from a boolean to a function)?
There was a problem hiding this comment.
This way the check is done later. At the time it is done now, the cmp(-module?) isn't ready, or at least gdprDataHandler.getConsentData() doesn't deliver anything. When called at time needed, the module or the cmp is set and we get response.
I'll update the PR
renebaudisch
force-pushed
the
asnmt_purpose1_fix
branch
from
113dd19
to
41551c5
Compare
renebaudisch
force-pushed
the
asnmt_purpose1_fix
branch
from
41551c5
to
dbb3dac
Compare
Type of change
Description of change
implemented getting consent before trying to write to storage