Skip to content

SameSite attribute support for ResponseCookieCollection#299

Merged
Tratcher merged 1 commit into
aspnet:devfrom
Gradi:cookies-samesite
Sep 6, 2019
Merged

SameSite attribute support for ResponseCookieCollection#299
Tratcher merged 1 commit into
aspnet:devfrom
Gradi:cookies-samesite

Conversation

@Gradi

@Gradi Gradi commented Jul 15, 2019

Copy link
Copy Markdown
Contributor

I have added a way to set SameSite attribute value for Set-Cookie headers.
You can do so by setting CookieOptions.SameSite property to some non null value.
If CookieOptions.SameSite property is null then SameSite attribute will not be added at all.

I must point out that if you use SameSiteMode.None it will be set as SameSite=None.

You can read about that behavior here and here

@dnfclas

dnfclas commented Jul 15, 2019

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@Gradi

Gradi commented Jul 15, 2019

Copy link
Copy Markdown
Contributor Author

Also it may solve #201 issue

@Tratcher

Copy link
Copy Markdown
Member

SameSite=None is still an early draft and breaks Safari. We can't merge that specific behavior until that gets worked out.
dotnet/aspnetcore#12125 (comment)

@Tratcher

Copy link
Copy Markdown
Member

There are many other places that need to be updated, such as exposing this option for several auth components that emit cookies. SameSite breaks OAuth and OIDC so they need to disable it. Null is OK for them only until the implied default becomes Lax.
https://github.com/aspnet/AspNetKatana/search?q=CookieOptions&unscoped_q=CookieOptions


var cookie = new HttpCookie(escapedKey, Uri.EscapeDataString(value));
if (domainHasValue)
{
cookie.Domain = options.Domain;
}



var cookieOptions = new CookieOptions
{
HttpOnly = true,
Secure = Request.IsSecure
};

new CookieOptions
{
HttpOnly = true,
Secure = Request.IsSecure,
Expires = DateTime.UtcNow + Options.ProtocolValidator.NonceLifetime
});

@Gradi

Gradi commented Jul 15, 2019

Copy link
Copy Markdown
Contributor Author

Hm. System.Web already contains SameSiteMode enum, but it is not available in Microsoft.Owin project.

SameSite breaks OAuth and OIDC so they need to disable it. Null is OK for them only until the implied default becomes Lax.

I'm sorry. I don't quite get what is wrong. If you set to null(or just do nothing) then SameSite would not be included at all as it does right now. Default behavior with default options is not broken.

@Tratcher

Copy link
Copy Markdown
Member

System.Web.SameSiteMode was added in 4.7.2. Microsoft.Owin targets 4.5.0. We'd probably have to cross compile Microsoft.Owin.Host.SystemWeb to support that.
https://docs.microsoft.com/en-us/dotnet/api/system.web.samesitemode?view=netframework-4.7.2#applies-to

The new SameSite behavior defined in https://tools.ietf.org/html/draft-west-cookie-incrementalism-00 is that Lax becomes the default and you have to specify None to opt out. That default would break OAuth and OIDC unless they opt out, and they can't opt out without breaking Safari.

@Tratcher Tratcher added this to the 4.0.2 milestone Jul 27, 2019
@Tratcher Tratcher self-assigned this Sep 6, 2019
@Tratcher

Tratcher commented Sep 6, 2019

Copy link
Copy Markdown
Member

I'm going to merge this as is and keep iterating. Thanks for getting it started for us.

@Tratcher
Tratcher merged commit 39221f8 into aspnet:dev Sep 6, 2019
@Tratcher Tratcher mentioned this pull request Sep 9, 2019
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.

3 participants