-
Notifications
You must be signed in to change notification settings - Fork 10.3k
[2.1] Re-implement SameSite for 2019 #13746
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
Conversation
@aspnet/build what's up with Windows in 2.1?
|
Hmm, I thought vside.myget.org was not authenticated? In any case, whatever credentials we're using for these downlevel pipelines are either insufficient (i.e. we don't have a token to access everything we need) or expired. Could you please compare the variable groups mentioned in the pipelines for this branch with those used in (say) 'release/3.0' and let us know if there's a glaring difference? |
@dougbu yes Also, Linux is working for some reason. |
Wondering if this was a temporary situation… |
/azp run all |
No pipelines are associated with this pull request. |
@dougbu nope |
@Tratcher I'll follow up w/ the owner of that feed after lunch. Looks like auth is now required and we can't handle that; there are almost no secrets available in our public builds and adding one here wouldn't be good. |
Should we pick a more clear name instead of a 2016 date? Maybe UseSameSiteLaxAsDefault ? |
There's a lot more behavior in that 2016 rfc. Most of it affects the parser though. |
Sure but specifically if the only thing we are really doing is switching between none/lax with this switch, maybe its better to be more specific with the name |
SuppressSameSiteNone? |
Found the right email. We should update any https://vside.myget.org/F/vssdk/api/v3/index.json reference to instead use https://pkgs.dev.azure.com/azure-public/vside/_packaging/vssdk/nuget/v3/index.json Could you do that in this PR and your similar 2.2 update? We don't use the MyGet feeds in more recent branches. Also, https://vside.myget.org/F/devcore/api/v3/index.json, https://vside.myget.org/F/vsmac/api/v3/index.json and https://vside.myget.org/F/vs-impl/api/v3/index.json have all been deprecated. The replacement for them is https://pkgs.dev.azure.com/azure-public/vside/_packaging/vs-impl/nuget/v3/index.json And, no, I haven't checked how much we use the old feeds in 2.x branches… |
sure SuppressSameSiteNone sounds good |
f8c6ff1
to
af0d13a
Compare
Will this be ported to 2.2? EDIT: Ah, found #13858. |
@ulrichb yes, see #14996 (comment) |
@Tratcher thx! |
@Tratcher Somehow off-topic, but related: Will there be a VS 2017 SDK with .NET Core Runtime 2.2.8 (which will contain this fix)? |
Yes, there's going to be a SDK that'll work with VS 2017 15.9 that contains this fix. |
Okay, cool. That's perfect! |
2.1 patch for #12125. Similar patches are in flight for System.Web and Microsoft.Owin.
Issue: Chrome is redefining how the SameSite cookie property works in a non-backwards compatible way. Cross site components like OpenIdConnect authentication must opt-out by sending a new "None" value or they won't work anymore.
Customer impact: Customers using OpenIdConnect to log into their web sites will break without this patch starting in January (Chrome 80).
Risk: Medium. The SameSite changes are known to be incompatible with older OSs and browsers, especially iOS 12 and OSX Mojave (latest - 1). These represent a small but influential portion of the web client user base. Updating to the latest OS version addresses the incompatibility.
Mitigations:
A) This patch includes an AppContext switch to revert everything to the old behavior. "Microsoft.AspNetCore.SuppressSameSiteNone"
B)
(SameSiteMode)(-1)
can be used to exclude the SameSite attribute for cookies on a case by case basis.C) We're providing browser sniffing samples and guidance to account for the incompatibilities in some old clients.
@blowdart