prebid / Prebid.js

Setup and manage header bidding advertising partners without writing code or confusing line items. Prebid.js is open source and free.
https://docs.prebid.org
Apache License 2.0
1.28k stars 2.05k forks source link

Id Modules: not respecting publisher configuration #10710

Open patmmccann opened 10 months ago

patmmccann commented 10 months ago

Type of issue

Bug

Description

https://docs.prebid.org/dev-docs/modules/userId.html

image shows publishers can choose where to set storage, but several modules ignore eg Verizon

https://github.com/prebid/Prebid.js/blob/015f12b3a2ab62d993e2578d6725b6acf8456910/modules/connectIdSystem.js#L45

Lotame appears to not have this setting available and just sets both https://github.com/prebid/Prebid.js/blob/015f12b3a2ab62d993e2578d6725b6acf8456910/modules/lotamePanoramaIdSystem.js#L45

LiveRamp appears to not use the parameter for html5 or cookie but instead has an additional proprietary parameter that seems related

https://github.com/prebid/Prebid.js/blob/015f12b3a2ab62d993e2578d6725b6acf8456910/modules/identityLinkIdSystem.js#L122

image

Feature to add: allow a new publisher configuration option which prevents submodules from writing to storage, so that only userid is responsible for writing and persisting the identifier coming back from the submodule.

dgirardi commented 10 months ago

These are ID modules that use storage directly (grepping for getStorageManager). It doesn't necessarily mean that they don't respect publisher config, but in theory ID modules do not need to deal with storage - it's done for them by the base ID module.

        admixerIdSystem.js  
        adqueryIdSystem.js  
        adriverIdSystem.js  
        amxIdSystem.js  
        connectIdSystem.js  
        criteoIdSystem.js  
        czechAdIdSystem.js  
        dacIdSystem.js  
        deepintentDpesIdSystem.js  
        euidIdSystem.js  
        freepassIdSystem.js  
        ftrackIdSystem.js  
        gravitoIdSystem.js  
        growthCodeIdSystem.js  
        hadronIdSystem.js  
        id5IdSystem.js  
        identityLinkIdSystem.js  
        idxIdSystem.js  
        imuIdSystem.js  
        intentIqIdSystem.js  
        liveIntentIdSystem.js  
        lotamePanoramaIdSystem.js  
        merkleIdSystem.js  
        mwOpenLinkIdSystem.js  
        naveggIdSystem.js  
        novatiqIdSystem.js  
        pairIdSystem.js  
        parrableIdSystem.js  
        publinkIdSystem.js  
        quantcastIdSystem.js  
        teadsIdSystem.js  
        uid2IdSystem.js  
        zeotapIdPlusIdSystem.js  
patmmccann commented 10 months ago

We can see id modules, as noted in the example above, violating the publisher parameter choices for storage.type listed at https://docs.prebid.org/dev-docs/modules/userId.html#basic-configuration

For example, panoramasets the cookie when the publisher chooses html5. This means it is operating outside of how it is configured and documented to behave and is a bug we need to fix.

patmmccann commented 10 months ago

I propose we boot all id modules doing this in Prebid 9. @jdwieland8282 let's talk about it in committee?

danielsao commented 10 months ago

@patmmccann LiveRamp user id module respects the publisher settings in storage.type. The line of code you are referring to is about logging the identity envelope source type (true or false) in a cookie for analytics purposes and not about overwriting the storage location where the actual identity envelope is saved.

leonardlabat commented 10 months ago

Hello @patmmccann

However, Criteo team can you also have your bidder module do the same when it sets cto_bundle?

Do you have an example showing how the storage configuration can be retrieved from a bidder adapter?

patmmccann commented 4 months ago

Might be too ambitious for prebid 9? what do you think @lcorrigall @dgirardi

patmmccann commented 4 months ago

Do you have an example showing how the storage configuration can be retrieved from a bidder adapter?

You could take it as a bidder param potentially?

jdwieland8282 commented 4 months ago

I don't think we should boot the modules who are out of compliance. If we include it in pb 9 what does that practically mean?

muuki88 commented 4 months ago

If being removed in pb9 , publishers that use those id modules would not upgrade, until the adapter has been resubmitted.

I'm actually in favor of removing ( booting? ) the modules that do not comply. We had issues with cookie headers being too large and when the storage settings are ignored, this may cause frustration.

jdwieland8282 commented 4 months ago

Thats a fair point @muuki88

@dgirardi I see your list above, how can I figure out which of those do not respect the pubs cookies|html5 choice?

patmmccann commented 4 months ago

Committee discussion result: we should subset the above list to those that are writing outside of the configured location, not reading outside of configuration.

patmmccann commented 4 months ago

Only thing to do for 9 will be to add doc warnings for adapters using set methods from storage manager.

"This adapter writes to device storage and may not follow the configuration set by the publisher. A future version of Prebid may disallow this behavior."

We also need to compile the actual list. As @danielsao points out, their write is of a different value, not the id returned.

jdwieland8282 commented 4 months ago

Let me know how I can help compiling the list.

patmmccann commented 3 months ago

Checking the above list for writes:

https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/adqueryIdSystem.js#L114 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/adriverIdSystem.js#L73 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/connectIdSystem.js#L58 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/dacIdSystem.js#L94 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/hadronIdSystem.js#L104 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/id5IdSystem.js#L433 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/imuIdSystem.js#L71 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/lotamePanoramaIdSystem.js#L52 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/teadsIdSystem.js#L79 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/merkleIdSystem.js#L41 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/mwOpenLinkIdSystem.js#L106 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/naveggIdSystem.js#L33 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/quantcastIdSystem.js#L52 Intentiq is unique, they always write to local storage, but check for cookie config. If the pub prefers only cookies, they should honor that: see both https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/intentIqIdSystem.js#L74 and https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/intentIqIdSystem.js#L128

As @danielsao points out, Liveramp is not respecting publisher choices on other things they are writing to storage, not the id https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/identityLinkIdSystem.js#L141

These appear to be bugs, as 'both' is now an option https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/uid2IdSystem_shared.js#L685 https://github.com/prebid/Prebid.js/blob/dbf981739da6d402b0c09e0aa2289cdbb6e86be1/modules/ftrackIdSystem.js#L178

jdwieland8282 commented 3 months ago

How does this text sound for our docs repo?

Please be aware this UserId sub adapter may not respect `storage.type` parameter setting. Please reach out directly to the sub adapter maintainer to ensure your storage location selection is respected.
jdwieland8282 commented 3 months ago

@dgirardi can you find a module that respects the param.storage.type so we id maintainers can see how this should be supported?

patmmccann commented 3 months ago

@dgirardi can you find a module that respects the param.storage.type so we id maintainers can see how this should be supported?

Yandex recently simplified their submission to conform

See https://github.com/prebid/Prebid.js/pull/11196#issuecomment-2158909789

abazylewicz-id5 commented 2 months ago

Is it ok if in our user module, when we do a server call to get an ID, in the response we set cookie with given ID? Or we would need to require cookie storage/not set the cookie when our module is configured to use only html5 storage type?

If we have an option to use external module through explicit module configuration parameter, does the external module also should respect prebid storage configuration or it's enough if we add to the description that the external module uses some additional storage regardless of what's set in prebid storage type?

(Asking here instead of separate issue as it's related and may help others)

patmmccann commented 2 months ago

Your response set cookie header is no problem. The problem is if your js sets a cookie in the first party context when the publisher directed you to use local storage for the first party. For the external module, why would you want to ignore your customers instructions? it's not very friendly to them. We can't police the external js activity, but I do not recommend you set first party cookies when pubs ask you not to. They may have good reasons, respect your customers.

abazylewicz-id5 commented 2 months ago

Thanks @patmmccann , for some reason I missed the fact that Prebid indeed can set first party cookies. We do not store any first party cookies in external module, and submitted the fix to the storing on prebid side in this MR: #11965

jdwieland8282 commented 1 month ago

I heard offline from Mediawalla and Lotame that they have fixed their modules, can anyone confirm?

jdwieland8282 commented 1 month ago

Mediawallah's fix https://github.com/prebid/Prebid.js/pull/12149

Tonsil commented 10 hours ago

I heard offline from Mediawalla and Lotame that they have fixed their modules, can anyone confirm?

As a Lotame representative, I can tell you that work has been prioritized and planned for Q4 but not yet completed. Is there a deadline for when this change needs to go live? I tried to find a schedule posted for "10.0 API Change" timing, but was unsuccessful.