SecurityContextHolderThreadLocalAccessor shares SecurityContext instance across threads #18210
+53
−3
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.
We encountered an issue where authentication was being mixed across threads.
During our analysis, we discovered that
SecurityContextHolderThreadLocalAccessorpropagates the sameSecurityContextto other threads when using Micrometer Context Propagation.Below is a simplified example that demonstrates the problem:
When Micrometer context propagation is in use, the current implementation effectively shares the
SecurityContextinstance between threads. This is generally not recommended and can cause subtle and hard-to-diagnose behavior, such as authentication leakage across threads.We understand that swapping the
Authenticationdirectly is rarely a good practice. However, if a newSecurityContexthad been used for the different thread, it would have been isolated and harmless. The underlying issue happens only because the sameSecurityContextinstance is shared across threads.Proposed Change
This PR updates
SecurityContextHolderThreadLocalAccessorto create a newSecurityContextwhenever a thread switch happens, and propagte only theAuthentication.I have targetted the PR to
6.5.xbranch as I consider it is a bug and should be fixed in there and up.