Skip to content

Conversation

@ttddyy
Copy link
Contributor

@ttddyy ttddyy commented Nov 23, 2025

We encountered an issue where authentication was being mixed across threads.
During our analysis, we discovered that SecurityContextHolderThreadLocalAccessor propagates the same SecurityContext to other threads when using Micrometer Context Propagation.

Below is a simplified example that demonstrates the problem:

Authentication authA = ...
Authentication authB = ...

// Set authA in the main thread
SecurityContext securityContext = SecurityContextHolder.getContext();
securityContext.setAuthentication(authA);

Runnable runnable = () -> {
    // This retrieves the *same* SecurityContext instance as the main thread
    SecurityContext context = SecurityContextHolder.getContext();
    context.setAuthentication(authB);
};

// Executor integrated with Micrometer Context Propagation
ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
executor.setTaskDecorator(new ContextPropagatingTaskDecorator());
executor.afterPropertiesSet();

executor.execute(runnable);

// Wait until runnable finishes
...

// Authentication has now changed unexpectedly
SecurityContext current = SecurityContextHolder.getContext();
current.getAuthentication();  // returns authB instead of authA

When Micrometer context propagation is in use, the current implementation effectively shares the SecurityContext instance 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 Authentication directly is rarely a good practice. However, if a new SecurityContext had been used for the different thread, it would have been isolated and harmless. The underlying issue happens only because the same SecurityContext instance is shared across threads.

Proposed Change

This PR updates SecurityContextHolderThreadLocalAccessor to create a new SecurityContext whenever a thread switch happens, and propagte only the Authentication.

I have targetted the PR to 6.5.x branch as I consider it is a bug and should be fixed in there and up.

`SecurityContextHolderThreadLocalAccessor` currently propagates the
same `SecurityContext` to other threads when Micrometer
Context Propagation is used. This leads to unintended sharing of
mutable state and can cause authentication to leak between threads.

This change updates the accessor to create a new
`SecurityContext` for the target thread while reusing only the
`Authentication` value. Each thread now receives its own
`SecurityContext` instance, preventing cross-thread interference and
aligning with recommended `SecurityContext` usage.

Signed-off-by: Tadaya Tsuyukubo <[email protected]>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 23, 2025
@jzheaux jzheaux self-assigned this Nov 24, 2025
@jzheaux jzheaux added in: core An issue in spring-security-core type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 24, 2025
@jzheaux jzheaux added this to the 6.5.8 milestone Nov 24, 2025
@jzheaux jzheaux changed the title SecurityContextHolderThreadLocalAccessor shares SecurityContext instance across threads SecurityContextHolderThreadLocalAccessor should not share SecurityContext instance across threads Nov 24, 2025
public void setValue(SecurityContext securityContext) {
Assert.notNull(securityContext, "securityContext cannot be null");
SecurityContextHolder.setContext(securityContext);
SecurityContext newContext = SecurityContextHolder.createEmptyContext();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question : Can we apply singleton concept here?

Assert.notNull(securityContext, "securityContext cannot be null");
SecurityContextHolder.setContext(securityContext);
SecurityContext newContext = SecurityContextHolder.createEmptyContext();
newContext.setAuthentication(securityContext.getAuthentication());
Copy link
Contributor

@ronodhirSoumik ronodhirSoumik Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newContext.setAuthentication(Objects.requireNonNull(securityContext.getAuthentication()))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: core An issue in spring-security-core type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants