From e1fdabd8f269b0c7a21b9cd20d564fdae3253edc Mon Sep 17 00:00:00 2001 From: Tadaya Tsuyukubo Date: Sat, 22 Nov 2025 20:58:56 -0800 Subject: [PATCH] Prevent sharing SecurityContext across threads `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 --- ...urityContextHolderThreadLocalAccessor.java | 5 +- ...ContextHolderThreadLocalAccessorTests.java | 51 ++++++++++++++++++- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/springframework/security/core/context/SecurityContextHolderThreadLocalAccessor.java b/core/src/main/java/org/springframework/security/core/context/SecurityContextHolderThreadLocalAccessor.java index 178e5084c7f..239dc423f0f 100644 --- a/core/src/main/java/org/springframework/security/core/context/SecurityContextHolderThreadLocalAccessor.java +++ b/core/src/main/java/org/springframework/security/core/context/SecurityContextHolderThreadLocalAccessor.java @@ -31,6 +31,7 @@ * {@link java.util.ServiceLoader} mechanism when context-propagation is on the classpath. * * @author Steve Riesenberg + * @author Tadaya Tsuyukubo * @since 6.5 * @see io.micrometer.context.ContextRegistry */ @@ -52,7 +53,9 @@ public SecurityContext getValue() { @Override public void setValue(SecurityContext securityContext) { Assert.notNull(securityContext, "securityContext cannot be null"); - SecurityContextHolder.setContext(securityContext); + SecurityContext newContext = SecurityContextHolder.createEmptyContext(); + newContext.setAuthentication(securityContext.getAuthentication()); + SecurityContextHolder.setContext(newContext); } @Override diff --git a/core/src/test/java/org/springframework/security/core/context/SecurityContextHolderThreadLocalAccessorTests.java b/core/src/test/java/org/springframework/security/core/context/SecurityContextHolderThreadLocalAccessorTests.java index e04d0277ff5..0ab2e776d2b 100644 --- a/core/src/test/java/org/springframework/security/core/context/SecurityContextHolderThreadLocalAccessorTests.java +++ b/core/src/test/java/org/springframework/security/core/context/SecurityContextHolderThreadLocalAccessorTests.java @@ -16,11 +16,18 @@ package org.springframework.security.core.context; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.core.task.support.ContextPropagatingTaskDecorator; +import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; import org.springframework.security.authentication.TestingAuthenticationToken; +import org.springframework.security.core.Authentication; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -29,6 +36,7 @@ * Tests for {@link SecurityContextHolderThreadLocalAccessor}. * * @author Steve Riesenberg + * @author Tadaya Tsuyukubo */ public class SecurityContextHolderThreadLocalAccessorTests { @@ -65,9 +73,11 @@ public void getValueWhenSecurityContextHolderSetThenReturnsSecurityContext() { @Test public void setValueWhenSecurityContextThenSetsSecurityContextHolder() { SecurityContext securityContext = SecurityContextHolder.createEmptyContext(); - securityContext.setAuthentication(new TestingAuthenticationToken("user", "password")); + Authentication authentication = new TestingAuthenticationToken("user", "password"); + securityContext.setAuthentication(authentication); this.threadLocalAccessor.setValue(securityContext); - assertThat(SecurityContextHolder.getContext()).isSameAs(securityContext); + assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext); + assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs(authentication); } @Test @@ -90,4 +100,41 @@ public void setValueWhenSecurityContextSetThenClearsSecurityContextHolder() { assertThat(SecurityContextHolder.getContext()).isEqualTo(emptyContext); } + @Test + public void newSecurityContextInDifferentThread() throws Exception { + Authentication authA = new TestingAuthenticationToken("foo", "password"); + Authentication authB = new TestingAuthenticationToken("bar", "password"); + + SecurityContext securityContext = SecurityContextHolder.createEmptyContext(); + securityContext.setAuthentication(authA); + SecurityContextHolder.setContext(securityContext); + + CountDownLatch latch = new CountDownLatch(1); + AtomicReference contextHolder = new AtomicReference<>(); + AtomicReference authHolder = new AtomicReference<>(); + Runnable runnable = () -> { + SecurityContext context = SecurityContextHolder.getContext(); + contextHolder.set(context); + authHolder.set(context.getAuthentication()); + context.setAuthentication(authB); + latch.countDown(); + }; + + ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); + executor.setTaskDecorator(new ContextPropagatingTaskDecorator()); + executor.afterPropertiesSet(); + + executor.execute(runnable); + + boolean finished = latch.await(10, TimeUnit.SECONDS); + assertThat(finished).isTrue(); + + assertThat(contextHolder.get()).isNotSameAs(securityContext); + assertThat(authHolder.get()).isSameAs(authA); + + SecurityContext current = SecurityContextHolder.getContext(); + assertThat(current).isSameAs(securityContext); + assertThat(current.getAuthentication()).isSameAs(authA); + } + }