Skip to content
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

User id module: force calls to getId if there was previously no consent data stored #6760

Conversation

smenzer
Copy link
Collaborator

@smenzer smenzer commented May 14, 2021

Type of change

  • Bugfix

Description of change

The user id module currently treats "empty" stored hashed consent data as matching the current consent data available in the auction, thus not forcing a new call to getId() on its own. The intention is that if consent changes from one request to the next, that a getId() call will be made to the user id modules can update their ID (or remove it) based on the latest consent data.

After implementing #6551, however, the hashed consent data was only stored AFTER gdpr checks and consent for local storage given. This means that if the user at first did not give consent (perhaps the CMP timed out and the auction proceeded with empty consent data), then nothing would be stored in the hashed consent data but the ID module could have been called and set a "no-consent" ID. A refresh would not be required on subsequent calls because the ID had already been set and the stored consent data was empty, thus not forcing a refresh.

This PR changes the behavior and treats empty stored consent data as a reason to force a getId() call to the ID module.

Note that even if there is no consent data (i.e. a user from outside of the EU), the stored consent data still stores a value due to the way the hashed value is created. This means that on the first call where cookies are allowed to be written and the user id module executes, a value will be stored based on the current consent data. If consent is not given, then the user id module essentially doesn't execute the submodules, so this doesn't force getId() calls on every page view.

@idettman I don't know if this would help solve #6738 at all, but would love you to take a look either way.

@smenzer smenzer added the bugfix label May 14, 2021
@smenzer smenzer requested a review from idettman May 18, 2021 08:22
@ChrisHuie ChrisHuie changed the title user id module - force calls to getId if there was previously no consent data stored User id module: force calls to getId if there was previously no consent data stored May 19, 2021
@smenzer
Copy link
Collaborator Author

smenzer commented May 20, 2021

@idettman do you think you can get to this before next week's release? it's continuing to cause issues for our publishers

@smenzer smenzer requested a review from Fawke May 24, 2021 07:44
Copy link
Contributor

@Fawke Fawke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smenzer Just wanted to understand something, correct me if am wrong. If the user does not give GDPR consent (on purpose, not because of timeout), then we won't write anything to this cookie _pbjs_userid_consent_data and that means storedConsentDataMatchesConsentData will always return false.

In that case won't we be calling getId() function on each page refresh even though the user hasn't given any consent?

@smenzer
Copy link
Collaborator Author

smenzer commented May 26, 2021

In that case won't we be calling getId() function on each page refresh even though the user hasn't given any consent?

it's possible i'm mis-reading the code a bit...so please correct me if you see it differently.

the module would exit early if there's no consent for purpose 1 and gdpr enforcement is enabled, which means that we shouldn't even run the submodules without consent.

if gdpr enforcement is not enabled (but consentmanagement module is, meaning it won't prevent modules from running but will provide a consent string), then i think it would get passed the line above to exit and would then store the consent hash in the cookie.

so...i don't think we would be in a situation where the getId() method would be called on every single page view. but again, i could be reading the code incorrectly...

@Fawke
Copy link
Contributor

Fawke commented May 27, 2021

@smenzer Yes, you are right...it won't execute the submodules if no consent. Silly of me to overlook that.

@smenzer smenzer added the needs 2nd review Core module updates require two approvals from the core team label May 27, 2021
@smenzer smenzer requested a review from patmmccann May 27, 2021 15:05
@patmmccann patmmccann merged commit bcd1ebe into prebid:master May 27, 2021
@smenzer smenzer deleted the fix-refresh-of-user-id-modules-from-consent-check branch May 27, 2021 18:54
stsepelin pushed a commit to cointraffic/Prebid.js that referenced this pull request May 28, 2021
prebidtappx pushed a commit to prebidtappx/Prebid.js that referenced this pull request Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix minor needs 2nd review Core module updates require two approvals from the core team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants