From d111029078382135759cb45507fdc6b1bda456df 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 170f260c19..516f485f41 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 @@ -32,6 +32,7 @@ import org.springframework.util.Assert; * {@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 */ @@ -53,7 +54,9 @@ public final class SecurityContextHolderThreadLocalAccessor implements ThreadLoc @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 e04d0277ff..0ab2e776d2 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 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException * Tests for {@link SecurityContextHolderThreadLocalAccessor}. * * @author Steve Riesenberg + * @author Tadaya Tsuyukubo */ public class SecurityContextHolderThreadLocalAccessorTests { @@ -65,9 +73,11 @@ public class SecurityContextHolderThreadLocalAccessorTests { @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 class SecurityContextHolderThreadLocalAccessorTests { 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); + } + }