Update default settings for SameSite #1232
Conversation
|
@JunTaoLuo, |
| @@ -22,7 +22,7 @@ public CookieAuthenticationOptions() | |||
| ReturnUrlParameter = CookieAuthenticationDefaults.ReturnUrlParameter; | |||
| ExpireTimeSpan = TimeSpan.FromDays(14); | |||
| SlidingExpiration = true; | |||
| CookieSameSite = SameSiteMode.Strict; | |||
| CookieSameSite = SameSiteMode.Lax; | |||
There was a problem hiding this comment.
Can you put a comment in indicating why we're using lax? Even just a link to the issue would be sufficient. Just something so that somebody doesn't come along 6 months later and decide to change it :)
| @@ -203,7 +203,7 @@ protected virtual void GenerateCorrelationId(AuthenticationProperties properties | |||
| var cookieOptions = new CookieOptions | |||
| { | |||
| HttpOnly = true, | |||
| SameSite = SameSiteMode.Lax, | |||
| SameSite = SameSiteMode.Strict, | |||
There was a problem hiding this comment.
Hum, does that really work with the OIDC client when using response_mode=form_post? (in this case, the IdP returns an auto-submit HTML form pointing to the callback URL).
There was a problem hiding this comment.
BTW, I'm curious, what's the point of using a same-site policy for the correlation/anti-XSRF cookies?
There was a problem hiding this comment.
You're right, Strict isn't going to work.
For consistency we're considering setting SameSite for all cookies that can work with it.
There was a problem hiding this comment.
Consistency is great and should always win, but be careful: RemoteAuthenticationHandler is a very generic base class that can be used to implement custom protocols (I personally wrote an OpenID 2.0 middleware leveraging it). That would be annoying to break scenarios that do fancy things with the callback dance... this same-site thingy tends to be really zealous 😄
There was a problem hiding this comment.
You're right, Strict isn't going to work.
@Tratcher I'm not sure Lax will work with response_mode=form_post since POST is not considered as a "safe" method by the specification.
@JunTaoLuo did Lax work for you?
| @@ -161,7 +161,7 @@ public TwitterHandler(IOptionsSnapshot<TwitterOptions> options, ILoggerFactory l | |||
| var cookieOptions = new CookieOptions | |||
| { | |||
| HttpOnly = true, | |||
| SameSite = SameSiteMode.Lax, | |||
| SameSite = SameSiteMode.Strict, | |||
There was a problem hiding this comment.
Same remark as above. Twitter - that implements OAuth1 - uses GET/302 responses to return the user back to the client app. If Strict doesn't work with the final application cookie due to the cross-origin 302 dance - as explained by @anurse - it will likely not work here.
|
Tested OIDCSample and OIDC.AzureAdSample. This is ready to merge |
JunTaoLuo
force-pushed
the
johluo/samesite-update
branch
from
4d0a986 to
cf99250
Compare
- Need Lax policy for social authentication - Need None policy for OIDC
JunTaoLuo
force-pushed
the
johluo/samesite-update
branch
from
cf99250 to
c523839
Compare
Addresses #1231 Need to test this with the auth samples before merging.