From 23de257508e98772225ec925b939785f08f4d748 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Wed, 22 Jul 2015 16:48:04 -0500 Subject: [PATCH] SEC-3031: DelegatingSecurityContext(Runnable|Callable) only modify SecurityContext on new Thread Modifying the SecurityContext on the same Thread can cause issues. For example, with a RejectedExecutionHandler the SecurityContext may be cleared out on the original Thread. This change modifies both the DelegatingSecurityContextRunnable and DelegatingSecurityContextCallable to, by default, only modify the SecurityContext if they are invoked on a new Thread. The behavior can be changed by setting the property enableOnOrigionalThread to true. --- .../DelegatingSecurityContextCallable.java | 38 ++++- .../DelegatingSecurityContextRunnable.java | 38 ++++- ...elegatingSecurityContextCallableTests.java | 52 ++++-- ...elegatingSecurityContextRunnableTests.java | 65 ++++++-- ...yContextHolderAwareRequestFilterTests.java | 153 ++++++++++++------ 5 files changed, 269 insertions(+), 77 deletions(-) diff --git a/core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextCallable.java b/core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextCallable.java index 3b4d428665..834732f99c 100644 --- a/core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextCallable.java +++ b/core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextCallable.java @@ -19,8 +19,17 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.util.Assert; /** - * Wraps a delegate {@link Callable} with logic for setting up a {@link SecurityContext} before invoking the delegate - * {@link Callable} and then removing the {@link SecurityContext} after the delegate has completed. + *

+ * Wraps a delegate {@link Callable} with logic for setting up a + * {@link SecurityContext} before invoking the delegate {@link Callable} and + * then removing the {@link SecurityContext} after the delegate has completed. + *

+ *

+ * By default the {@link SecurityContext} is only setup if {@link #call()} is + * invoked on a separate {@link Thread} than the + * {@link DelegatingSecurityContextCallable} was created on. This can be + * overridden by setting {@link #setEnableOnOriginalThread(boolean)} to true. + *

* * @author Rob Winch * @since 3.2 @@ -31,6 +40,10 @@ public final class DelegatingSecurityContextCallable implements Callable { private final SecurityContext securityContext; + private final Thread originalThread; + + private boolean enableOnOriginalThread; + /** * Creates a new {@link DelegatingSecurityContextCallable} with a specific {@link SecurityContext}. * @param delegate the delegate {@link DelegatingSecurityContextCallable} to run with the specified @@ -43,6 +56,7 @@ public final class DelegatingSecurityContextCallable implements Callable { Assert.notNull(securityContext, "securityContext cannot be null"); this.delegate = delegate; this.securityContext = securityContext; + this.originalThread = Thread.currentThread(); } /** @@ -54,7 +68,27 @@ public final class DelegatingSecurityContextCallable implements Callable { this(delegate, SecurityContextHolder.getContext()); } + /** + * Determines if the SecurityContext should be transfered if {@link #call()} + * is invoked on the same {@link Thread} the + * {@link DelegatingSecurityContextCallable} was created on. + * + * @param enableOnOriginalThread + * if false (default), will only transfer the + * {@link SecurityContext} if {@link #call()} is invoked on a + * different {@link Thread} than the + * {@link DelegatingSecurityContextCallable} was created on. + * + * @since 4.0.2 + */ + public void setEnableOnOriginalThread(boolean enableOnOriginalThread) { + this.enableOnOriginalThread = enableOnOriginalThread; + } + public V call() throws Exception { + if(!enableOnOriginalThread && originalThread == Thread.currentThread()) { + return delegate.call(); + } try { SecurityContextHolder.setContext(securityContext); return delegate.call(); diff --git a/core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextRunnable.java b/core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextRunnable.java index 4bae805dfb..28d018c9d1 100644 --- a/core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextRunnable.java +++ b/core/src/main/java/org/springframework/security/concurrent/DelegatingSecurityContextRunnable.java @@ -17,8 +17,17 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.util.Assert; /** - * Wraps a delegate {@link Runnable} with logic for setting up a {@link SecurityContext} before invoking the delegate - * {@link Runnable} and then removing the {@link SecurityContext} after the delegate has completed. + *

+ * Wraps a delegate {@link Runnable} with logic for setting up a {@link SecurityContext} + * before invoking the delegate {@link Runnable} and then removing the + * {@link SecurityContext} after the delegate has completed. + *

+ *

+ * By default the {@link SecurityContext} is only setup if {@link #run()} is + * invoked on a separate {@link Thread} than the + * {@link DelegatingSecurityContextRunnable} was created on. This can be + * overridden by setting {@link #setEnableOnOriginalThread(boolean)} to true. + *

* * @author Rob Winch * @since 3.2 @@ -29,6 +38,10 @@ public final class DelegatingSecurityContextRunnable implements Runnable { private final SecurityContext securityContext; + private final Thread originalThread; + + private boolean enableOnOriginalThread; + /** * Creates a new {@link DelegatingSecurityContextRunnable} with a specific {@link SecurityContext}. * @param delegate the delegate {@link Runnable} to run with the specified {@link SecurityContext}. Cannot be null. @@ -40,6 +53,7 @@ public final class DelegatingSecurityContextRunnable implements Runnable { Assert.notNull(securityContext, "securityContext cannot be null"); this.delegate = delegate; this.securityContext = securityContext; + this.originalThread = Thread.currentThread(); } /** @@ -51,7 +65,27 @@ public final class DelegatingSecurityContextRunnable implements Runnable { this(delegate, SecurityContextHolder.getContext()); } + /** + * Determines if the SecurityContext should be transfered if {@link #call()} + * is invoked on the same {@link Thread} the + * {@link DelegatingSecurityContextCallable} was created on. + * + * @param enableOnOriginalThread + * if false (default), will only transfer the + * {@link SecurityContext} if {@link #call()} is invoked on a + * different {@link Thread} than the + * {@link DelegatingSecurityContextCallable} was created on. + * @since 4.0.2 + */ + public void setEnableOnOriginalThread(boolean enableOnOriginalThread) { + this.enableOnOriginalThread = enableOnOriginalThread; + } + public void run() { + if(!enableOnOriginalThread && originalThread == Thread.currentThread()) { + delegate.run(); + return; + } try { SecurityContextHolder.setContext(securityContext); delegate.run(); diff --git a/core/src/test/java/org/springframework/security/concurrent/DelegatingSecurityContextCallableTests.java b/core/src/test/java/org/springframework/security/concurrent/DelegatingSecurityContextCallableTests.java index 17bb536a0b..48527482bc 100644 --- a/core/src/test/java/org/springframework/security/concurrent/DelegatingSecurityContextCallableTests.java +++ b/core/src/test/java/org/springframework/security/concurrent/DelegatingSecurityContextCallableTests.java @@ -17,6 +17,9 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import org.junit.After; import org.junit.Before; @@ -45,6 +48,8 @@ public class DelegatingSecurityContextCallableTests { private Callable callable; + private ExecutorService executor; + @Before @SuppressWarnings("serial") public void setUp() throws Exception { @@ -55,6 +60,7 @@ public class DelegatingSecurityContextCallableTests { return super.answer(invocation); } }); + executor = Executors.newFixedThreadPool(1); } @After @@ -88,15 +94,34 @@ public class DelegatingSecurityContextCallableTests { @Test public void call() throws Exception { - callable = new DelegatingSecurityContextCallable(delegate, securityContext); - assertWrapped(callable.call()); + callable = new DelegatingSecurityContextCallable(delegate, + securityContext); + assertWrapped(callable); } @Test public void callDefaultSecurityContext() throws Exception { SecurityContextHolder.setContext(securityContext); callable = new DelegatingSecurityContextCallable(delegate); - SecurityContextHolder.clearContext(); // ensure callable is what sets up the SecurityContextHolder + SecurityContextHolder.clearContext(); // ensure callable is what sets up the + // SecurityContextHolder + assertWrapped(callable); + } + + // SEC-3031 + @Test + public void callOnSameThread() throws Exception { + callable = new DelegatingSecurityContextCallable(delegate, + securityContext); + securityContext = SecurityContextHolder.createEmptyContext(); + assertWrapped(callable.call()); + } + + @Test + public void callOnSameThreadExplicitlyEnabled() throws Exception { + DelegatingSecurityContextCallable callable = new DelegatingSecurityContextCallable(delegate, + securityContext); + callable.setEnableOnOriginalThread(true); assertWrapped(callable.call()); } @@ -116,14 +141,15 @@ public class DelegatingSecurityContextCallableTests { public void createNullSecurityContext() throws Exception { SecurityContextHolder.setContext(securityContext); callable = DelegatingSecurityContextCallable.create(delegate, null); - SecurityContextHolder.clearContext(); // ensure callable is what sets up the SecurityContextHolder - assertWrapped(callable.call()); + SecurityContextHolder.clearContext(); // ensure callable is what sets up the + // SecurityContextHolder + assertWrapped(callable); } @Test public void create() throws Exception { callable = DelegatingSecurityContextCallable.create(delegate, securityContext); - assertWrapped(callable.call()); + assertWrapped(callable); } // --- toString @@ -131,13 +157,19 @@ public class DelegatingSecurityContextCallableTests { // SEC-2682 @Test public void toStringDelegates() { - callable = new DelegatingSecurityContextCallable(delegate, securityContext); + callable = new DelegatingSecurityContextCallable(delegate, + securityContext); assertThat(callable.toString()).isEqualTo(delegate.toString()); } - private void assertWrapped(Object actualResult) throws Exception { - assertThat(actualResult).isEqualTo(callableResult); + private void assertWrapped(Callable callable) throws Exception { + Future submit = executor.submit(callable); + assertWrapped(submit.get()); + } + + private void assertWrapped(Object callableResult) throws Exception { verify(delegate).call(); - assertThat(SecurityContextHolder.getContext()).isEqualTo(SecurityContextHolder.createEmptyContext()); + assertThat(SecurityContextHolder.getContext()).isEqualTo( + SecurityContextHolder.createEmptyContext()); } } \ No newline at end of file diff --git a/core/src/test/java/org/springframework/security/concurrent/DelegatingSecurityContextRunnableTests.java b/core/src/test/java/org/springframework/security/concurrent/DelegatingSecurityContextRunnableTests.java index c5aaf3c4c9..cbbae4e642 100644 --- a/core/src/test/java/org/springframework/security/concurrent/DelegatingSecurityContextRunnableTests.java +++ b/core/src/test/java/org/springframework/security/concurrent/DelegatingSecurityContextRunnableTests.java @@ -16,6 +16,10 @@ import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.verify; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -24,6 +28,8 @@ import org.mockito.Mock; import org.mockito.invocation.InvocationOnMock; import org.mockito.runners.MockitoJUnitRunner; import org.mockito.stubbing.Answer; +import org.springframework.core.task.SyncTaskExecutor; +import org.springframework.core.task.support.ExecutorServiceAdapter; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; @@ -43,6 +49,8 @@ public class DelegatingSecurityContextRunnableTests { private Runnable runnable; + private ExecutorService executor; + @Before public void setUp() throws Exception { doAnswer(new Answer() { @@ -50,8 +58,9 @@ public class DelegatingSecurityContextRunnableTests { assertThat(SecurityContextHolder.getContext()).isEqualTo(securityContext); return null; } - }) - .when(delegate).run(); + }).when(delegate).run(); + + executor = Executors.newFixedThreadPool(1); } @After @@ -86,17 +95,35 @@ public class DelegatingSecurityContextRunnableTests { @Test public void call() throws Exception { runnable = new DelegatingSecurityContextRunnable(delegate, securityContext); - runnable.run(); - assertWrapped(); + assertWrapped(runnable); } @Test public void callDefaultSecurityContext() throws Exception { SecurityContextHolder.setContext(securityContext); runnable = new DelegatingSecurityContextRunnable(delegate); - SecurityContextHolder.clearContext(); // ensure runnable is what sets up the SecurityContextHolder - runnable.run(); - assertWrapped(); + SecurityContextHolder.clearContext(); // ensure runnable is what sets up the + // SecurityContextHolder + assertWrapped(runnable); + } + + // SEC-3031 + @Test + public void callOnSameThread() throws Exception { + executor = synchronousExecutor(); + runnable = new DelegatingSecurityContextRunnable(delegate, + securityContext); + securityContext = SecurityContextHolder.createEmptyContext(); + assertWrapped(runnable); + } + + @Test + public void callOnSameThreadExplicitlyEnabled() throws Exception { + executor = synchronousExecutor(); + DelegatingSecurityContextRunnable runnable = new DelegatingSecurityContextRunnable(delegate, + securityContext); + runnable.setEnableOnOriginalThread(true); + assertWrapped(runnable); } // --- create --- @@ -112,19 +139,18 @@ public class DelegatingSecurityContextRunnableTests { } @Test - public void createNullSecurityContext() { + public void createNullSecurityContext() throws Exception { SecurityContextHolder.setContext(securityContext); runnable = DelegatingSecurityContextRunnable.create(delegate, null); - SecurityContextHolder.clearContext(); // ensure runnable is what sets up the SecurityContextHolder - runnable.run(); - assertWrapped(); + SecurityContextHolder.clearContext(); // ensure runnable is what sets up the + // SecurityContextHolder + assertWrapped(runnable); } @Test - public void create() { + public void create() throws Exception { runnable = DelegatingSecurityContextRunnable.create(delegate, securityContext); - runnable.run(); - assertWrapped(); + assertWrapped(runnable); } // --- toString @@ -136,8 +162,15 @@ public class DelegatingSecurityContextRunnableTests { assertThat(runnable.toString()).isEqualTo(delegate.toString()); } - private void assertWrapped() { + private void assertWrapped(Runnable runnable) throws Exception { + Future submit = executor.submit(runnable); + submit.get(); verify(delegate).run(); - assertThat(SecurityContextHolder.getContext()).isEqualTo(SecurityContextHolder.createEmptyContext()); + assertThat(SecurityContextHolder.getContext()).isEqualTo( + SecurityContextHolder.createEmptyContext()); + } + + private static ExecutorService synchronousExecutor() { + return new ExecutorServiceAdapter(new SyncTaskExecutor()); } } \ No newline at end of file diff --git a/web/src/test/java/org/springframework/security/web/servletapi/SecurityContextHolderAwareRequestFilterTests.java b/web/src/test/java/org/springframework/security/web/servletapi/SecurityContextHolderAwareRequestFilterTests.java index cf0a51bd5f..054ce5437f 100644 --- a/web/src/test/java/org/springframework/security/web/servletapi/SecurityContextHolderAwareRequestFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/servletapi/SecurityContextHolderAwareRequestFilterTests.java @@ -63,7 +63,6 @@ import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.web.authentication.logout.LogoutHandler; import org.springframework.util.ClassUtils; - /** * Tests {@link SecurityContextHolderAwareRequestFilter}. * @@ -107,18 +106,23 @@ public class SecurityContextHolderAwareRequestFilterTests { SecurityContextHolder.clearContext(); } - //~ Methods ======================================================================================================== + // ~ Methods + // ======================================================================================================== @Test public void expectedRequestWrapperClassIsUsed() throws Exception { filter.setRolePrefix("ROLE_"); - filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), filterChain); + filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), + filterChain); // Now re-execute the filter, ensuring our replacement wrapper is still used - filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), filterChain); + filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), + filterChain); - verify(filterChain, times(2)).doFilter(any(SecurityContextHolderAwareRequestWrapper.class), any(HttpServletResponse.class)); + verify(filterChain, times(2)).doFilter( + any(SecurityContextHolderAwareRequestWrapper.class), + any(HttpServletResponse.class)); filter.destroy(); } @@ -126,17 +130,20 @@ public class SecurityContextHolderAwareRequestFilterTests { @Test public void authenticateFalse() throws Exception { assertThat(wrappedRequest().authenticate(response)).isFalse(); - verify(authenticationEntryPoint).commence(eq(requestCaptor.getValue()), eq(response), any(AuthenticationException.class)); + verify(authenticationEntryPoint).commence(eq(requestCaptor.getValue()), + eq(response), any(AuthenticationException.class)); verifyZeroInteractions(authenticationManager, logoutHandler); verify(request, times(0)).authenticate(any(HttpServletResponse.class)); } @Test public void authenticateTrue() throws Exception { - SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("test","password","ROLE_USER")); + SecurityContextHolder.getContext().setAuthentication( + new TestingAuthenticationToken("test", "password", "ROLE_USER")); assertThat(wrappedRequest().authenticate(response)).isTrue(); - verifyZeroInteractions(authenticationEntryPoint, authenticationManager, logoutHandler); + verifyZeroInteractions(authenticationEntryPoint, authenticationManager, + logoutHandler); verify(request, times(0)).authenticate(any(HttpServletResponse.class)); } @@ -147,7 +154,8 @@ public class SecurityContextHolderAwareRequestFilterTests { assertThat(wrappedRequest().authenticate(response)).isFalse(); verify(request).authenticate(response); - verifyZeroInteractions(authenticationEntryPoint, authenticationManager, logoutHandler); + verifyZeroInteractions(authenticationEntryPoint, authenticationManager, + logoutHandler); } @Test @@ -158,53 +166,73 @@ public class SecurityContextHolderAwareRequestFilterTests { assertThat(wrappedRequest().authenticate(response)).isTrue(); verify(request).authenticate(response); - verifyZeroInteractions(authenticationEntryPoint, authenticationManager, logoutHandler); + verifyZeroInteractions(authenticationEntryPoint, authenticationManager, + logoutHandler); } @Test public void login() throws Exception { - TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", "password","ROLE_USER"); - when(authenticationManager.authenticate(any(UsernamePasswordAuthenticationToken.class))).thenReturn(expectedAuth); + TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", + "password", "ROLE_USER"); + when( + authenticationManager + .authenticate(any(UsernamePasswordAuthenticationToken.class))) + .thenReturn(expectedAuth); - wrappedRequest().login(expectedAuth.getName(),String.valueOf(expectedAuth.getCredentials())); + wrappedRequest().login(expectedAuth.getName(), + String.valueOf(expectedAuth.getCredentials())); - assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs(expectedAuth); + assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs( + expectedAuth); verifyZeroInteractions(authenticationEntryPoint, logoutHandler); - verify(request, times(0)).login(anyString(),anyString()); + verify(request, times(0)).login(anyString(), anyString()); } // SEC-2296 @Test public void loginWithExstingUser() throws Exception { - TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", "password","ROLE_USER"); - when(authenticationManager.authenticate(any(UsernamePasswordAuthenticationToken.class))).thenReturn(new TestingAuthenticationToken("newuser","not be found","ROLE_USER")); + TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", + "password", "ROLE_USER"); + when( + authenticationManager + .authenticate(any(UsernamePasswordAuthenticationToken.class))) + .thenReturn( + new TestingAuthenticationToken("newuser", "not be found", + "ROLE_USER")); SecurityContextHolder.getContext().setAuthentication(expectedAuth); try { - wrappedRequest().login(expectedAuth.getName(),String.valueOf(expectedAuth.getCredentials())); + wrappedRequest().login(expectedAuth.getName(), + String.valueOf(expectedAuth.getCredentials())); fail("Expected Exception"); - } catch(ServletException success) { - assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs(expectedAuth); + } + catch (ServletException success) { + assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs( + expectedAuth); verifyZeroInteractions(authenticationEntryPoint, logoutHandler); - verify(request, times(0)).login(anyString(),anyString()); + verify(request, times(0)).login(anyString(), anyString()); } } @Test public void loginFail() throws Exception { AuthenticationException authException = new BadCredentialsException("Invalid"); - when(authenticationManager.authenticate(any(UsernamePasswordAuthenticationToken.class))).thenThrow(authException); + when( + authenticationManager + .authenticate(any(UsernamePasswordAuthenticationToken.class))) + .thenThrow(authException); try { - wrappedRequest().login("invalid","credentials"); + wrappedRequest().login("invalid", "credentials"); Assert.fail("Expected Exception"); - } catch(ServletException success) { + } + catch (ServletException success) { assertThat(success.getCause()).isEqualTo(authException); } assertThat(SecurityContextHolder.getContext().getAuthentication()).isNull(); verifyZeroInteractions(authenticationEntryPoint, logoutHandler); - verify(request, times(0)).login(anyString(),anyString()); + verify(request, times(0)).login(anyString(), anyString()); } @Test @@ -218,7 +246,8 @@ public class SecurityContextHolderAwareRequestFilterTests { wrappedRequest().login(username, password); verify(request).login(username, password); - verifyZeroInteractions(authenticationEntryPoint, authenticationManager, logoutHandler); + verifyZeroInteractions(authenticationEntryPoint, authenticationManager, + logoutHandler); } @Test @@ -234,16 +263,19 @@ public class SecurityContextHolderAwareRequestFilterTests { try { wrappedRequest().login(username, password); Assert.fail("Expected Exception"); - } catch(ServletException success) { + } + catch (ServletException success) { assertThat(success).isEqualTo(authException); } - verifyZeroInteractions(authenticationEntryPoint, authenticationManager, logoutHandler); + verifyZeroInteractions(authenticationEntryPoint, authenticationManager, + logoutHandler); } @Test public void logout() throws Exception { - TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", "password","ROLE_USER"); + TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", + "password", "ROLE_USER"); SecurityContextHolder.getContext().setAuthentication(expectedAuth); HttpServletRequest wrappedRequest = wrappedRequest(); @@ -262,78 +294,105 @@ public class SecurityContextHolderAwareRequestFilterTests { wrappedRequest().logout(); verify(request).logout(); - verifyZeroInteractions(authenticationEntryPoint, authenticationManager, logoutHandler); + verifyZeroInteractions(authenticationEntryPoint, authenticationManager, + logoutHandler); } @Test public void getAsyncContextStart() throws Exception { ArgumentCaptor runnableCaptor = ArgumentCaptor.forClass(Runnable.class); SecurityContext context = SecurityContextHolder.createEmptyContext(); - TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", "password","ROLE_USER"); + TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", + "password", "ROLE_USER"); context.setAuthentication(expectedAuth); SecurityContextHolder.setContext(context); AsyncContext asyncContext = mock(AsyncContext.class); when(request.getAsyncContext()).thenReturn(asyncContext); Runnable runnable = new Runnable() { - public void run() {} + public void run() { + } }; wrappedRequest().getAsyncContext().start(runnable); verifyZeroInteractions(authenticationManager, logoutHandler); verify(asyncContext).start(runnableCaptor.capture()); - DelegatingSecurityContextRunnable wrappedRunnable = (DelegatingSecurityContextRunnable) runnableCaptor.getValue(); - assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, SecurityContext.class)).isEqualTo(context); - assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, Runnable.class)).isEqualTo(runnable); + DelegatingSecurityContextRunnable wrappedRunnable = (DelegatingSecurityContextRunnable) runnableCaptor + .getValue(); + assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, SecurityContext.class)) + .isEqualTo(context); + assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, "delegate")) + .isEqualTo(runnable); } @Test public void startAsyncStart() throws Exception { ArgumentCaptor runnableCaptor = ArgumentCaptor.forClass(Runnable.class); SecurityContext context = SecurityContextHolder.createEmptyContext(); - TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", "password","ROLE_USER"); + TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", + "password", "ROLE_USER"); context.setAuthentication(expectedAuth); SecurityContextHolder.setContext(context); AsyncContext asyncContext = mock(AsyncContext.class); when(request.startAsync()).thenReturn(asyncContext); Runnable runnable = new Runnable() { - public void run() {} + public void run() { + } }; wrappedRequest().startAsync().start(runnable); verifyZeroInteractions(authenticationManager, logoutHandler); verify(asyncContext).start(runnableCaptor.capture()); - DelegatingSecurityContextRunnable wrappedRunnable = (DelegatingSecurityContextRunnable) runnableCaptor.getValue(); - assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, SecurityContext.class)).isEqualTo(context); - assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, Runnable.class)).isEqualTo(runnable); + DelegatingSecurityContextRunnable wrappedRunnable = (DelegatingSecurityContextRunnable) runnableCaptor + .getValue(); + assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, SecurityContext.class)) + .isEqualTo(context); + assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, "delegate")) + .isEqualTo(runnable); } @Test public void startAsyncWithRequestResponseStart() throws Exception { ArgumentCaptor runnableCaptor = ArgumentCaptor.forClass(Runnable.class); SecurityContext context = SecurityContextHolder.createEmptyContext(); - TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", "password","ROLE_USER"); + TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", + "password", "ROLE_USER"); context.setAuthentication(expectedAuth); SecurityContextHolder.setContext(context); AsyncContext asyncContext = mock(AsyncContext.class); - when(request.startAsync(request,response)).thenReturn(asyncContext); + when(request.startAsync(request, response)).thenReturn(asyncContext); Runnable runnable = new Runnable() { - public void run() {} + public void run() { + } }; wrappedRequest().startAsync(request, response).start(runnable); verifyZeroInteractions(authenticationManager, logoutHandler); verify(asyncContext).start(runnableCaptor.capture()); - DelegatingSecurityContextRunnable wrappedRunnable = (DelegatingSecurityContextRunnable) runnableCaptor.getValue(); - assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, SecurityContext.class)).isEqualTo(context); - assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, Runnable.class)).isEqualTo(runnable); + DelegatingSecurityContextRunnable wrappedRunnable = (DelegatingSecurityContextRunnable) runnableCaptor + .getValue(); + assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, SecurityContext.class)) + .isEqualTo(context); + assertThat(WhiteboxImpl.getInternalState(wrappedRunnable, "delegate")) + .isEqualTo(runnable); + } + + // SEC-3047 + @Test + public void updateRequestFactory() throws Exception { + SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", + "password", "PREFIX_USER")); + filter.setRolePrefix("PREFIX_"); + + assertThat(wrappedRequest().isUserInRole("PREFIX_USER")).isTrue();; } private HttpServletRequest wrappedRequest() throws Exception { filter.doFilter(request, response, filterChain); - verify(filterChain).doFilter(requestCaptor.capture(), any(HttpServletResponse.class)); + verify(filterChain).doFilter(requestCaptor.capture(), + any(HttpServletResponse.class)); return requestCaptor.getValue(); }