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

Chrome 85.0.4183 fix for SameSite=None cookie rejection policy #5719

Merged
merged 1 commit into from
Sep 10, 2020

Conversation

bansawbanchee
Copy link
Contributor

@bansawbanchee bansawbanchee commented Sep 9, 2020

Tests for this Analytics Adapter fail locally without this change when a user is using Chrome 85 or newer. Chrome 85 added rejection of insecure SameSite=None cookies. secure probably could have been added to fix this issue but SameSite=Lax works as well for the test case.

https://developers.google.com/web/updates/2020/07/chrome-85-deps-rems
https://en.wikipedia.org/wiki/Google_Chrome_version_history

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Does this change affect user-facing APIs or examples documented on http://prebid.org?
  • Other

… added rejection of insecure SameSite=None cookies
@smenzer
Copy link
Collaborator

smenzer commented Sep 9, 2020

Shouldn't you be using storageManager rather than directly writing cookies via document.cookie?

@bansawbanchee
Copy link
Contributor Author

bansawbanchee commented Sep 9, 2020

This is not our adapter.

The tests for this adapter fail locally when I am trying to run gulp serve to test our userID submodule because my installed version of Chrome (85) on Mac is newer then the version Prebid BrowserStack tests against (Chrome 80).

Chrome 85 and newer includes the rejection for SameSite=None without secure flag so anybody who attempts to run tests locally on Chrome 85 or newer will receive these failed tests for this adapter.

I am only including this change to ensure the tests pass.

Here is the direct link to the update Chrome added that will make these tests fail if you have a newer version of Chrome installed locally. https://developers.google.com/web/updates/2020/07/chrome-85-deps-rems#reject_insecure_samesitenone_cookies

Maybe we should also update https://github.com/prebid/Prebid.js/blob/master/browsers.json to include newer versions of Chrome? Or change it from 80 to 85?

Here is the pull request that added that document.cookie in the spec: https://github.com/prebid/Prebid.js/pull/4292/files#diff-6ac962c2306f557acbe6661536d02c30R10

@patmmccann
Copy link
Collaborator

LGTM

Copy link
Collaborator

@smenzer smenzer left a comment

Choose a reason for hiding this comment

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

this adapter probably needs to update to use storageManager, but as this is a quick fix to get tests working again, I'll merge it.

@smenzer smenzer merged commit 2acca6f into prebid:master Sep 10, 2020
BrightMountainMediaInc pushed a commit to BrightMountainMediaInc/Prebid.js that referenced this pull request Sep 14, 2020
… added rejection of insecure SameSite=None cookies (prebid#5719)
BrightMountainMediaInc added a commit to BrightMountainMediaInc/Prebid.js that referenced this pull request Sep 14, 2020
…83 which added rejection of insecure SameSite=None cookies (prebid#5719)"

This reverts commit 100a364.
mxdvl pushed a commit to guardian/Prebid.js that referenced this pull request Apr 28, 2021
… added rejection of insecure SameSite=None cookies (prebid#5719)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants