Skip to content

SameSite attribute support for ResponseCookieCollection #299

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

Merged
merged 1 commit into from
Sep 6, 2019
Merged

SameSite attribute support for ResponseCookieCollection #299

merged 1 commit into from
Sep 6, 2019

Conversation

Gradi
Copy link
Contributor

@Gradi Gradi commented Jul 15, 2019

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
Copy link

dnfclas commented Jul 15, 2019

CLA assistant check
All CLA requirements met.

@Gradi
Copy link
Contributor Author

Gradi commented Jul 15, 2019

Also it may solve #201 issue

@Tratcher
Copy link
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
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
Copy link
Contributor Author

Gradi commented Jul 15, 2019

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
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
Copy link
Member

Tratcher commented Sep 6, 2019

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