[2.1] CookieChunkingManager needs to flow the Secure attribute for Delete operations #17953
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tratcher
added
Servicing-consider
No, it's reached end of life so we don't intend to backport. |
jamshedd
added
Servicing-approved
|
Approved for Feb. |
|
Moved to Mar as per Tactics. Will need to be explicitly approved for Feb, if required. |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
analogrelay
approved these changes
Jan 15, 2020
This was referenced Jan 17, 2020
mindlink
pushed a commit
to mindlink/aspnetcore
that referenced
this pull request
Nov 25, 2020
…e Secure attribute for Delete operations dotnet#17953. This is an internal port of the 2.1 fix for extended partners support only.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#17833
This comes from an external report of customers using IdentityServer4. When testing SameSite behavior in Chrome with Core 2.1 they were able to login successfully but had trouble logging out. The new SameSite browser policy requires all cookies that set
SameSite=Noneto also set thesecureattribute.Securewas included for sign-in, but it was not included for the sign-out delete cookie because previously there was no need.This is possible to work around by copying the ChunkingCookieManager class (300 lines) into your application and applying the changes (5 lines).
This was previously fixed in 3.0 independently from the SameSite changes and was not included in the SameSite downlevel patches.
Impact: Moderate. Hitting this issue requires setting a non-default but commonly used option (SameSite=None on the authentication cookie). Applications did this in the past to avoid problems with iOS 12.
Risk: Low. These changes are small and were already included in 3.0.
Set as Draft until the next 2.1.x servicing window opens.
Should this be ported to 2.2.x? 2.2 has reached end-of-life.