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 <tadaya@ttddyy.net>
This commit is contained in:
committed by
Josh Cummings
parent
5b923c78ea
commit
d111029078
+4
-1
@@ -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
|
||||
|
||||
+49
-2
@@ -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<SecurityContext> contextHolder = new AtomicReference<>();
|
||||
AtomicReference<Authentication> 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);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user