Skip to content

Use SameSite everywhere#308

Merged
Tratcher merged 5 commits into
devfrom
tratcher/samesite
Sep 20, 2019
Merged

Use SameSite everywhere#308
Tratcher merged 5 commits into
devfrom
tratcher/samesite

Conversation

@Tratcher

@Tratcher Tratcher commented Sep 9, 2019

Copy link
Copy Markdown
Member

@blowdart we took a community PR #299 that provided the initial SameSite implementation. This PR expands on that to more components, sets None for OIDC, lights up the System.Web implementation, etc..

The updated SystemWebCookieManager won't fix OIDC until System.Web gets patched.

I know there is some duplication here, this is old, decoupled code that we want to limit changes to.

Note that since this API never existed before we were able to make it nullable rather than adding Unspecified (-1) to the enum.

Also fixes #234

@Tratcher Tratcher self-assigned this Sep 9, 2019
@Tratcher
Tratcher requested a review from HaoK September 9, 2019 21:04
@Tratcher Tratcher added this to the 4.1.0 milestone Sep 15, 2019
@Tratcher
Tratcher marked this pull request as ready for review September 17, 2019 21:44
@Tratcher

Copy link
Copy Markdown
Member Author

@anurse let's get this one reviewed and merged so we can unblock partner testing via nightlies. I've run compat testing already. See the sample SameSiteCookieManager that we'll include in docs.

@Tratcher
Tratcher merged commit 0fc4611 into dev Sep 20, 2019
@Tratcher
Tratcher deleted the tratcher/samesite branch September 20, 2019 21:30
@josundt

josundt commented Feb 10, 2020

Copy link
Copy Markdown

@Tratcher and @anurse
Cookies that do not include any SameSite property has up till now been treated by browsers as if no SameSite policy was precent (= SameSite=None).

With the Chromium 80 update, the browser will start treating cookies without the SameSite property as SameSite=Lax. (Ref: https://www.chromium.org/updates/same-site).

This behavioral browser change breaks many applications, especially applications using OpenID Connect for authentication/authorization (since the OpenID Connect spec relies on IFRAMEs for certain features - Single-Sign-Out etc).

Such applications now need to explicitly include the SameSite=None property to work with Chromium 80.

But then another problem arises: Cookies with the SameSite=None property is misinterpreted in certain versions of Safari on Mac and iOS (SameSite=None is interpreted as SameSite=Strict !!!).
For this reason, user-agent sniffing is unfortunately required to make OpenID connect dependent applications work with Chromium 80 and certain Safari versions at the same time.

The code to detect user-agents that suffers from this SameSite=None defect can be found in the following article:

https://www.thinktecture.com/identity/samesite/prepare-your-identityserver/

I think Microsoft.Security.Owin.Cookies should include this user-agent detection in the CookieAuthenticationHandler class.

When CookieAuthenticationOptions.CookieSameSite = SameSiteMode.None, the cookie should omit the SameSite property when a browser with the SameSite=None defect is detected.

Until this is in place, I can't use the CookieAuhenticationMiddleware or CookieAuhenticationHandler classes, I needed to make my own implementations based on existing source code.

Let me know if a PRs are accepted as I have already implemented it for our applications.

I have now created an issue for this:
#333

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.

Please respect HttpOnly / Secure options in SignOut cookie

3 participants