FPD Enrichment: support for Cookie Deprecation Label #10683

jlquaccia · 2023-11-02T04:17:35Z

Type of change

Feature

Description of change

Support for the Privacy Sandbox cookie deprecation label
Core will access the Privacy Sandbox JS API for testing mode and place the result in device.ext.dcep
A check for publisher purpose 1 consent before calling the Privacy Sandbox JS API will occur if the gdprEnforcement module is active

Other information

jlquaccia · 2023-11-02T04:26:29Z

@patmmccann @dgirardi, from #10516, it sounded like we wanted this change in enrichment.js. Let me know what you guys think.

dgirardi · 2023-11-02T17:34:14Z

src/fpd/enrichment.js

@@ -78,6 +95,34 @@ function removeUndef(obj) {
  return getDefinedParams(obj, Object.keys(obj))
 }

+export async function tryToGetCdepLabel(cb = getCookieDeprecationLabel) {
+  let cdep;
+  const consentData = gdprDataHandler.getConsentData();


This should do a check on the accessDevice activity instead (somewhat like this).

Normally that would be enough - the TCF logic (which is just one of the ways storage is gated) is attached to accessDevice in its own module.

However for this case we decided that the TCF logic should be changed to check for publisher consent (publisher.consents[1] in the parsed TCF data), apparently that's what we should have been doing from the beginning, but we need to wait for 9 to avoid breaking people. Paging also @patmmccann since I can't find a summary of what was decided, I think we may also want to add transmitUfpd (purpose 4) checks.

For this PR that means:

the TCF logic linked above should be updated to look for publisher.*. I would put in a special case for a new component name, e.g. if in here you check isActivityAllowed(ACTIVITY_ACCESS_DEVICE, activityParams(MODULE_TYPE_PREBID, 'cdep')), then here you can have a special case for when params[ACTIVITY_PARAM_COMPONENT_NAME] === 'cdep'. That would then get conflated into the existing special case for MODULE_TYPE_PREBID in prebid 9.

If we need purpose 4 checks it should be enough to add device.ext.cdep to this list

Update:

The new TCF logic should (in version 9) replace this so that if gvlid is VENDORLESS_GVLID, instead of looking at vendorData.purpose and switching off the check on vendorData.vendor, we check vendorData.publisher instead. So my suggestion is to introduce a second, temporary gvlid placeholder (maybe FIRST_PARTY_GVLID); use it to implement the new publisher logic, and update this to return it only for the cookie deprecation label for now - then in 9 we can get rid of the existing VENDORLESS_GVLID logic.

We do want transmitUfpd / purpose 4 checks, so device.ext.cdep should be added to the ORTB fields that are redacted out when that is denied

jlquaccia · 2023-11-02T18:53:31Z

Thanks @dgirardi, will look into the feedback items you mentioned above today 👍

jlquaccia · 2023-11-03T16:59:07Z

Hey @dgirardi, could you take another look? Left a comment below about one question I had about the last commit i made. Also, I can address tests once the general flow of logic looks good for the rest of this PR.

jlquaccia · 2023-11-03T17:02:24Z

modules/gdprEnforcement.js

    if (shouldEnforce(consentData, purposeNo, modName)) {
      const gvlid = getGvlid(params[ACTIVITY_PARAM_COMPONENT_TYPE], modName, gvlidFallback(params));
-      let allow = !!checkConsent(consentData, modName, gvlid);
+      let allow =


@dgirardi had one question about the logic here. know we want to go with publisher.consent in this scenario.. but was wondering if i put the FIRST_PARTY_GVLID ternary in the right place?

IMO it should be inside validateRules, so that you can keep it consistent with some of the other logic - for example if the publisher has disabled purpose enforcement it should allow regardless of consent data.

dgirardi · 2023-11-07T19:13:21Z

modules/gdprEnforcement.js

-      let allow = !!checkConsent(consentData, modName, gvlid);
+      let allow =
+        gvlid === FIRST_PARTY_GVLID
+          ? !!deepAccess(consentData, `vendorData.publisher.consents.1`)


vendorData.publisher.consents[${purposeNo}]; currently this path will always have purpose 1 but I don't want us to forget about it in the future

dgirardi · 2023-11-07T19:15:10Z

src/activities/redactor.js

@@ -17,7 +17,8 @@ export const ORTB_UFPD_PATHS = [
  'kwarray',
  'id',
  'buyeruid',
-  'customdata'
+  'customdata',
+  'device.ext.cdep'


this becomes user.device.ext.cdep because of the .map below, which is not correct. this PR adds the first field that's not under user so it needs to be outside the map.

dgirardi · 2023-11-07T19:38:33Z

src/fpd/enrichment.js

+export async function tryToGetCdepLabel(cb = getCookieDeprecationLabel) {
+  let cdep;
+  if (isActivityAllowed(ACTIVITY_ACCESS_DEVICE, activityParams(MODULE_TYPE_PREBID, 'cdep'))) {
+    return GreedyPromise.resolve(


I would do return GreedyPromise.resolve(isActivityAllowed(...) && cb()). Your version wraps the promise two more times (with await and the .then() below), which seems unnecessary, plus if getCookieDeprecationLabel rejects this promise never resolves.

dgirardi · 2023-11-07T19:43:31Z

modules/gdprEnforcement.js

    if (shouldEnforce(consentData, purposeNo, modName)) {
      const gvlid = getGvlid(params[ACTIVITY_PARAM_COMPONENT_TYPE], modName, gvlidFallback(params));
-      let allow = !!checkConsent(consentData, modName, gvlid);
+      let allow =


IMO it should be inside validateRules, so that you can keep it consistent with some of the other logic - for example if the publisher has disabled purpose enforcement it should allow regardless of consent data.

jlquaccia · 2023-11-09T02:04:55Z

modules/gdprEnforcement.js

+  let validation = (!rule.enforcePurpose || purpose) && (!vendorConsentRequred || vendor);
+
+  if (gvlId === FIRST_PARTY_GVLID) {
+    validation = (!rule.enforcePurpose || !!deepAccess(consentData, `vendorData.publisher.consents.${ruleOptions.id}`));


One question.. when FIRST_PARTY_GVLID is present I’m guessing we don’t need to worry about vendor for this validation? (looks to be the case with VENDORLESS_GVLID as well, so I was following down that road)

yep, in this case purposeConsent is required, vendorConsent is not, and the flag to check is publisher.consents

jlquaccia · 2023-11-09T02:11:30Z

src/fpd/enrichment.js

+// }
+
+export async function tryToGetCdepLabel(cb = getCookieDeprecationLabel) {
+  return GreedyPromise.resolve(isActivityAllowed(ACTIVITY_ACCESS_DEVICE, activityParams(MODULE_TYPE_PREBID, 'cdep')) && await cb());


I think the getCookieDeprecationLabel func would require that we wait for it to resolve right? otherwise the label (if there is one) most likely wouldn’t get retrieved in time before the bid reqs go out.. (i'm not able to fetch it in time from my end at least). Was wondering what your thoughts on this use of await here are? Or if you have any other suggestions?

One thing I was thinking.. maybe right here a setTimeout would be good? One that would only wait x amount of time for navigator.cookieDeprecationLabel.getValue() to resolve and if it doesn’t we can resolve manually (and not block the auction)? I added in some arbitrary setTimeout code as an example, but of course that can be changed based on what you think.

getCookieDeprecationLabel returns a promise; since this also returns a promise, await is not necessary. Unless I'm mistaken await P; [...] is just syntactic sugar for P.then(() => [...]) so in this case it does nothing.

You're correct! I think when I originally tried out your suggestion there was a bug on my end 😂 I've since tried it again and things are working as expected on my end 👍

jlquaccia · 2023-11-09T02:12:09Z

hey @dgirardi, could you please take another look?

jlquaccia · 2023-11-09T17:15:29Z

added a quick "on hold" label to this PR temporarily, just so pubmatic can test for #10695

dgirardi · 2023-11-09T18:26:00Z

modules/gdprEnforcement.js

+  let validation = (!rule.enforcePurpose || purpose) && (!vendorConsentRequred || vendor);
+
+  if (gvlId === FIRST_PARTY_GVLID) {
+    validation = (!rule.enforcePurpose || !!deepAccess(consentData, `vendorData.publisher.consents.${ruleOptions.id}`));


yep, in this case purposeConsent is required, vendorConsent is not, and the flag to check is publisher.consents

dgirardi · 2023-11-09T18:28:41Z

src/fpd/enrichment.js

+// }
+
+export async function tryToGetCdepLabel(cb = getCookieDeprecationLabel) {
+  return GreedyPromise.resolve(isActivityAllowed(ACTIVITY_ACCESS_DEVICE, activityParams(MODULE_TYPE_PREBID, 'cdep')) && await cb());


getCookieDeprecationLabel returns a promise; since this also returns a promise, await is not necessary. Unless I'm mistaken await P; [...] is just syntactic sugar for P.then(() => [...]) so in this case it does nothing.

dgirardi · 2023-11-09T18:31:24Z

src/fpd/enrichment.js

    });
+    setTimeout(() => {


we have exising examples of FPD that waits for promises without timeouts (like user agent hints). I think it's OK because none of them actually need to wait for anything - there's no network calls or user prompts as far as I can tell, they should always resolve (or reject) very quickly. If we do introduce a timeout it should be configurable, and this should reject - not resolve to false.

Ok, thanks for the input! Sounds like we are ok without the setTimeout in this case at this time then. I removed it for now.

jlquaccia · 2023-11-09T21:19:02Z

@dgirardi let me know if you think anything else is needed on this PR? Only labeled this PR as "on hold" temporarily to give our team a small window to QA #10695. We are aiming to have QA done early next week.

patmmccann · 2023-11-09T21:29:32Z

#10695 is already merged and released

patmmccann · 2023-11-09T21:33:14Z

is the failed test bc of

Prebid.js/test/spec/modules/ixBidAdapter_spec.js

Line 4564 in e3b99f8

expect(diagObj.url).to.equal('http://localhost:9876/context.html')

jlquaccia · 2023-11-10T19:37:45Z

@patmmccann removed the label I had added onto this ticket, good to go there!

Also, interesting with the failed test.. I couldn't reproduce it locally. I also remember seeing that failed test in the circleci build for this PR too the other day but now it looks as though everything is green and passing?

jlquaccia · 2023-11-10T19:40:43Z

@dgirardi, attached a screenshot below of the failed test error that came up the other day when the build failed. Seems like an intermittent thing.. All checks look to have passed now. Is this something we should look into more or are we ok?

jlquaccia · 2023-11-10T22:11:29Z

Looks as though the same error crept up here as well fyi: #10635

dgirardi · 2023-11-13T17:42:26Z

test/spec/fpd/enrichment_spec.js

@@ -310,6 +312,55 @@ describe('FPD enrichment', () => {
    });
  });

+  describe('privacy sandbox cookieDeprecationLabel', () => {
+    it('attempts to set ext.cdep on device obj when the gdprEnforcement module is not active', () => {


The test does not match its description - and also mixes the behavior of enrichment and gdprEnforcement. IMO a better set of tests is:

if isActivityAllowed is mocked to true, and the navigator API returns a promise to some label X, the enrichment puts X in device.ext.cdep (your tests only check that the navigator is invoked)

if isActivityAllowed is mocked to false, the navigator API is not called and no enrichment happens

if the navigator API returns a promise that rejects, the enrichment does not halt forever (I expect your current version to fail this one)

if validateRules is passed FIRST_PARTY_GVLID, it will use publisher.consents - this should be part of the gdprEnforcement suite

dgirardi · 2023-11-13T17:48:32Z

src/fpd/enrichment.js

+  return GreedyPromise.resolve(isActivityAllowed(ACTIVITY_ACCESS_DEVICE, activityParams(MODULE_TYPE_PREBID, 'cdep')) && cb());
+}
+
+function getCookieDeprecationLabel() {


I'll be more specific - this function, as far as I can tell, behaves the same as directly calling navigator.cookieDeprecationLabel.getValue() except that you lose rejections / errors, the promise you return never completes in that case.

jlquaccia · 2023-11-16T01:45:43Z

@dgirardi thanks! Last few comments you left make sense, addressed them and pushed another commit. Could you review again when you have time? (also, can see your changes from #10723 reflecting now too).

dgirardi · 2023-11-16T14:49:15Z

test/spec/fpd/enrichment_spec.js

+      if (navigator.cookieDeprecationLabel) delete navigator.cookieDeprecationLabel;
+    });
+
+    it('enrichment sets device.ext.cdep when navigator.getCookieDeprecationLabel exists and isActivityAllowed is mocked to true', () => {


these are not actually mocking isActivityAllowed, they are still relying on gdprEnforcement to actually set up the activity rule. If you try to run only this test suite (gulp serve-and-test --file test/spec/fpd/enrichment_spec.js) it fails, since the gdpr module is not loaded.

dgirardi · 2023-11-16T14:54:06Z

test/spec/fpd/enrichment_spec.js

+
+    it('if the navigator API returns a promise that rejects, the enrichment does not halt forever', () => {
+      navigator.cookieDeprecationLabel = {};
+      sandbox.stub(dep, 'getCookieDeprecationLabel').returns(Promise.reject(new Error('oops, something went wrong')));


More mismatch between intent and code - because you are not mocking the navigator API but instead the broken piece of logic, this test is not picking up the problem. It would pass no matter what you put in getCookieDeprecationLabel.

dgirardi

I pushed 83e3d3b to show what I meant with my last comments - consider it a suggestion, it it looks good to you then of course it also LGTM

With the caveat that my browser does not have 'navigator.cookieDeprecationLabel', so I did not test the actual integration - is there a flag to turn that on, or do we need to wait until it's too late?

dgirardi · 2023-11-16T16:06:28Z

Update: chrome 120 does have the API, but only on (I think) https sites. For me it returns an empty string which with this results in device.ext.cdep not being set. I think that makes sense.

pm-nitin-nimbalkar · 2023-11-16T16:19:39Z

Update: chrome 120 does have the API, but only on (I think) https sites. For me it returns an empty string which with this results in device.ext.cdep not being set. I think that makes sense.

@dgirardi Try using chrome flags chrome://flags/#tpc-phase-out-facilitated-testing. This might help.

jlquaccia · 2023-11-16T17:20:37Z

@dgirardi saw your last commit, see what you mean about isActivityAllowed. also the other changes made sense and lgtm too!

jlquaccia · 2023-11-22T06:04:43Z

@patmmccann @dgirardi are we ok to merge this one?

patmmccann · 2023-11-27T17:48:36Z

LGTM

jlquaccia · 2023-11-27T18:29:43Z

Awesome, thanks!

jlquaccia requested review from dgirardi, patmmccann and pm-harshad-mane November 2, 2023 04:17

ChrisHuie assigned dgirardi Nov 2, 2023

dgirardi reviewed Nov 2, 2023

View reviewed changes

jlquaccia commented Nov 3, 2023

View reviewed changes

jlquaccia mentioned this pull request Nov 3, 2023

PubMatic Bid Adapter: Support for cookie deprecation label #10695

Merged

1 task

dgirardi reviewed Nov 7, 2023

View reviewed changes

jlquaccia commented Nov 9, 2023

View reviewed changes

jlquaccia added on hold do not merge and removed do not merge labels Nov 9, 2023

dgirardi reviewed Nov 9, 2023

View reviewed changes

jlquaccia removed the on hold label Nov 10, 2023

dgirardi reviewed Nov 13, 2023

View reviewed changes

patmmccann mentioned this pull request Nov 13, 2023

unit test sometimes calls done multiple times #10722

Closed

cookie deprecation label

db263ac

jlquaccia added 10 commits November 15, 2023 11:55

reverted unneeded changes made during development

635ae07

addressed feedback

a2c1fee

removed unnecessary else statement

a394e6b

added comment for FIRST_PARTY_GVLID param

3106553

ensure promise resolves when label is falsey

ad6ed3c

addressed feedback

eb10d23

removed async/await from getCookieDeprecationLabel

1c1d261

updating tests

56efa46

more changes

3e83177

addressed feedback

f85c2b3

jlquaccia force-pushed the ISSUE-10516 branch from 0eeb42c to f85c2b3 Compare November 16, 2023 01:40

Small improvements

83e3d3b

dgirardi reviewed Nov 16, 2023

View reviewed changes

dgirardi approved these changes Nov 16, 2023

View reviewed changes

ChrisHuie added the feature label Nov 16, 2023

patmmccann merged commit ecf4272 into prebid:master Nov 27, 2023
4 checks passed

patmmccann mentioned this pull request Dec 11, 2023

Passing Chrome facilitated Testing modes to bidders #10516

Closed

FPD Enrichment: support for Cookie Deprecation Label #10683

FPD Enrichment: support for Cookie Deprecation Label #10683

Conversation

jlquaccia commented Nov 2, 2023

Type of change

Description of change

Other information

jlquaccia commented Nov 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlquaccia commented Nov 2, 2023 • edited Loading

jlquaccia commented Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlquaccia commented Nov 9, 2023

jlquaccia commented Nov 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlquaccia commented Nov 9, 2023

patmmccann commented Nov 9, 2023

patmmccann commented Nov 9, 2023

jlquaccia commented Nov 10, 2023

jlquaccia commented Nov 10, 2023

jlquaccia commented Nov 10, 2023

Choose a reason for hiding this comment

dgirardi Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

jlquaccia commented Nov 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgirardi left a comment • edited Loading

Choose a reason for hiding this comment

dgirardi commented Nov 16, 2023

pm-nitin-nimbalkar commented Nov 16, 2023 • edited Loading

jlquaccia commented Nov 16, 2023

jlquaccia commented Nov 22, 2023

patmmccann commented Nov 27, 2023

jlquaccia commented Nov 27, 2023

jlquaccia commented Nov 2, 2023 •

edited

Loading

jlquaccia commented Nov 3, 2023 •

edited

Loading

dgirardi Nov 13, 2023 •

edited

Loading

dgirardi left a comment •

edited

Loading

pm-nitin-nimbalkar commented Nov 16, 2023 •

edited

Loading