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

Adds config for samesite support #68

Merged
merged 5 commits into from
May 23, 2020
Merged

Adds config for samesite support #68

merged 5 commits into from
May 23, 2020

Conversation

mding5692
Copy link
Contributor

@mding5692 mding5692 commented May 17, 2020

Opening a PR to address issue #67

This PR adds configuration options and proptypes that can handle setting samesite/first-party and crosssite/third-party cookies.

Samesite options goes from strict to lax to none

Cookies that match the domain of the current site, i.e. what's displayed in the browser's address bar, are referred to as first-party cookies.

These use strict or lax samesite options

Cookies from domains other than the current site are referred to as third-party cookies

These use option none, which sets cookie as samesite=none and secure=true (aka needs https)

Using below recommended implementation:

For cookies where they are only needed in a first-party context you should ideally mark them as SameSite=Lax or SameSite=Strict depending on your needs. You can also choose to do nothing and just allow the browser to enforce its default, but this comes with the risk of inconsistent >behavior across browsers and potential console warnings for each cookie.

Set-Cookie: first_party_var=value; SameSite=Lax

For cookies needed in a third-party context, you will need to ensure they are marked as SameSite=None; Secure. Note that you need both attributes together. If you just specify None without Secure the cookie will be rejected.

Set-Cookie: third_party_var=value; SameSite=None; Secure

BROWSER SUPPORT FOR SECURITY 'SameSite' cookie attribute

Google Chrome
Chrome version 4 to 50 doesn't suppports. Chrome version 51 to 60 partially supports it. Chrome 61 to 70 supports SECURITY 'SameSite' cookie attribute.

Mozilla Firefox
Firefox version 2 to 59 doesn't supports. Firefox version 60 to 63 supports SECURITY 'SameSite' cookie attribute.

Internet Explorer
IE browser version 6 and 10 doesn't supports. IE browser version 11 partially supports SECURITY 'SameSite' cookie attribute.

Safari
Safari browser version 3.1 to 11.1 doesn't supports. Safari browser version 12 supports SECURITY 'SameSite' cookie attribute.

Microsoft Edge
Microsoft Edge browser version 12 to 15 doesn't supports. Microsoft Edge browser version 16 to 18 supports SECURITY 'SameSite' cookie attribute property.

Opera
Opera version 10.1 to 38 doesn't supports. Opera version 39 to 53 supports SECURITY 'SameSite' cookie attribute.

Relevant links:
https://github.com/js-cookie/js-cookie#samesite
https://adzerk.com/blog/chrome-samesite/
https://web.dev/samesite-cookies-explained/
https://web.dev/samesite-cookie-recipes/
https://www.thinktecture.com/en/identity/samesite/samesite-in-a-nutshell/
https://www.thinktecture.com/en/identity/samesite/samesite-in-a-nutshell/#the-chrome-vs-safari-issue
https://www.chromium.org/updates/same-site/incompatible-clients
https://www.chromium.org/updates/same-site/test-debug
https://www.lambdatest.com/SameSite-cookie-attribute

@mding5692 mding5692 marked this pull request as draft May 17, 2020 16:14
@mding5692
Copy link
Contributor Author

mding5692 commented May 18, 2020

Probably will need these recommended helper functions for browsers which are incompatible with samesite=None

Some user agents are known to be incompatible with the SameSite=None attribute:
Versions of Chrome from Chrome 51 to Chrome 66 (inclusive on both ends). These Chrome versions will reject a cookie with SameSite=None. This also affects older versions of Chromium-derived browsers, as well as Android WebView.
Versions of UC Browser on Android prior to version 12.13.2. Older versions will reject a cookie with SameSite=None.
Versions of Safari and embedded browsers on MacOS 10.14 and all browsers on iOS 12. These versions will erroneously treat cookies marked with SameSite=None as if they were marked SameSite=Strict.

https://www.chromium.org/updates/same-site/incompatible-clients

Or implement like below:

As these changes to include None and update default behavior are still relatively new, there are inconsistencies amongst browsers as to how these changes are handled. You can refer to the updates page on chromium.org for the issues currently known, however it's not possible to say if this is exhaustive. While this is not ideal, there are workarounds you can employ during this transitionary phase. The general rule though is to treat incompatible clients as the special case. Do not create an exception for browsers implementing the newer rules.

The first option is to set both the new and old style cookies:

Set-cookie: 3pcookie=value; SameSite=None; Secure
Set-cookie: 3pcookie-legacy=value; Secure

Browsers implementing the newer behavior will set the cookie with the SameSite value, while other browsers may ignore or incorrectly set it. However, those same browsers will set the 3pcookie-legacy cookie. When processing included cookies, the site should first check for the presence of the new style cookie and if it's not found, then fallback to the legacy cookie.

@mding5692 mding5692 marked this pull request as ready for review May 19, 2020 20:07
@mding5692 mding5692 marked this pull request as draft May 19, 2020 20:07
@mding5692 mding5692 marked this pull request as ready for review May 21, 2020 00:38
@Mastermindzh
Copy link
Owner

Changes look good but the component is getting a tad complicated now.
Might have to look at some refactoring later :)

Anyway, I will merge tomorrow so I can release it as well.

Thanks for the support man, good PR!

@mding5692
Copy link
Contributor Author

mding5692 commented May 21, 2020

No worries :)

Copy link
Owner

@Mastermindzh Mastermindzh left a comment

Choose a reason for hiding this comment

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

2 remarks and you're missing README changes.

src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
@Mastermindzh
Copy link
Owner

Trying some stuff out and it seems like your default implementation of secure: true would fail for sites without HTTPS as well. We need to work around that as some sites might (still) not use SSL.

- extracted code which handles setting/getting cookies.
- added correct implementation of the legacy cookie fix according to the provided url
- added cookieSecurity attribute with default value based on runtime environment
- updated README
- ran builds
@Mastermindzh Mastermindzh merged commit e2fb5f3 into Mastermindzh:master May 23, 2020
@Mastermindzh
Copy link
Owner

Yo. I couldn't help myself so I pushed some code and fixed the issues myself.
Please review the changes and feel free to ask me any questions about it.

I marked the new package as a major version (5.0.0) because the default cookieSecurity value might break very strict implementations.

@mding5692
Copy link
Contributor Author

Ok much thanks!

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.

2 participants