From 3437ef714abfca0a559204c0a7be2a65cbc7339e Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 6 Dec 2012 12:59:21 -0600 Subject: [PATCH] SEC-1998: SecurityContextCallableProcessingInterceptor uses postProcess Previously SecurityContextCallableProcessingInterceptor used afterCompletion to clear the SecurityContextHolder. This does not work since afterCompletion is invoked on the Servlet Container thread. Now SecurityContextCallableProcessingInterceptor clears the SecurityContextHolder on postProcess which is invoked on the same thread that the Callable is processed on. --- ...rityContextCallableProcessingInterceptor.java | 10 +++++----- ...ontextCallableProcessingInterceptorTests.java | 4 ++-- .../WebAsyncManagerIntegrationFilterTests.java | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/context/request/async/SecurityContextCallableProcessingInterceptor.java b/web/src/main/java/org/springframework/security/web/context/request/async/SecurityContextCallableProcessingInterceptor.java index d9e04372c3..2b27422025 100644 --- a/web/src/main/java/org/springframework/security/web/context/request/async/SecurityContextCallableProcessingInterceptor.java +++ b/web/src/main/java/org/springframework/security/web/context/request/async/SecurityContextCallableProcessingInterceptor.java @@ -29,7 +29,7 @@ import org.springframework.web.context.request.async.CallableProcessingIntercept * A {@link CallableProcessingInterceptor} that establishes the injected {@link SecurityContext} on the * {@link SecurityContextHolder} when {@link #preProcess(NativeWebRequest, Callable)} is invoked. It also clear out the * {@link SecurityContextHolder} by invoking {@link SecurityContextHolder#clearContext()} in the - * {@link #afterCompletion(NativeWebRequest, Callable)} method. + * {@link #postProcess(NativeWebRequest, Callable, Object)} method. *

* * @author Rob Winch @@ -64,13 +64,13 @@ public final class SecurityContextCallableProcessingInterceptor extends Callable } @Override - public void afterCompletion(NativeWebRequest request, Callable task) throws Exception { - SecurityContextHolder.clearContext(); + public void preProcess(NativeWebRequest request, Callable task) throws Exception { + SecurityContextHolder.setContext(securityContext); } @Override - public void preProcess(NativeWebRequest request, Callable task) throws Exception { - SecurityContextHolder.setContext(securityContext); + public void postProcess(NativeWebRequest request, Callable task, Object concurrentResult) throws Exception { + SecurityContextHolder.clearContext(); } private void setSecurityContext(SecurityContext securityContext) { diff --git a/web/src/test/java/org/springframework/security/web/context/request/async/SecurityContextCallableProcessingInterceptorTests.java b/web/src/test/java/org/springframework/security/web/context/request/async/SecurityContextCallableProcessingInterceptorTests.java index 0f8c107380..541f8b2eff 100644 --- a/web/src/test/java/org/springframework/security/web/context/request/async/SecurityContextCallableProcessingInterceptorTests.java +++ b/web/src/test/java/org/springframework/security/web/context/request/async/SecurityContextCallableProcessingInterceptorTests.java @@ -59,7 +59,7 @@ public class SecurityContextCallableProcessingInterceptorTests { interceptor.preProcess(webRequest, callable); assertThat(SecurityContextHolder.getContext()).isSameAs(securityContext); - interceptor.afterCompletion(webRequest, callable); + interceptor.postProcess(webRequest, callable, null); assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext); } @@ -71,7 +71,7 @@ public class SecurityContextCallableProcessingInterceptorTests { interceptor.preProcess(webRequest, callable); assertThat(SecurityContextHolder.getContext()).isSameAs(securityContext); - interceptor.afterCompletion(webRequest, callable); + interceptor.postProcess(webRequest, callable, null); assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext); } } diff --git a/web/src/test/java/org/springframework/security/web/context/request/async/WebAsyncManagerIntegrationFilterTests.java b/web/src/test/java/org/springframework/security/web/context/request/async/WebAsyncManagerIntegrationFilterTests.java index 33d6740b1b..ddf368fc85 100644 --- a/web/src/test/java/org/springframework/security/web/context/request/async/WebAsyncManagerIntegrationFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/context/request/async/WebAsyncManagerIntegrationFilterTests.java @@ -31,7 +31,9 @@ import org.springframework.core.task.SimpleAsyncTaskExecutor; import org.springframework.mock.web.MockFilterChain; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.async.AsyncWebRequest; +import org.springframework.web.context.request.async.CallableProcessingInterceptorAdapter; import org.springframework.web.context.request.async.WebAsyncManager; import org.springframework.web.context.request.async.WebAsyncUtils; @@ -84,6 +86,13 @@ public class WebAsyncManagerIntegrationFilterTests { @Test public void doFilterInternalRegistersSecurityContextCallableProcessor() throws Exception { SecurityContextHolder.setContext(securityContext); + asyncManager.registerCallableInterceptors(new CallableProcessingInterceptorAdapter() { + @Override + public void postProcess(NativeWebRequest request, Callable task, Object concurrentResult) + throws Exception { + assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext); + } + }); filter.doFilterInternal(request, response, filterChain); VerifyingCallable verifyingCallable = new VerifyingCallable(); @@ -95,6 +104,13 @@ public class WebAsyncManagerIntegrationFilterTests { @Test public void doFilterInternalRegistersSecurityContextCallableProcessorContextUpdated() throws Exception { SecurityContextHolder.setContext(SecurityContextHolder.createEmptyContext()); + asyncManager.registerCallableInterceptors(new CallableProcessingInterceptorAdapter() { + @Override + public void postProcess(NativeWebRequest request, Callable task, Object concurrentResult) + throws Exception { + assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext); + } + }); filter.doFilterInternal(request, response, filterChain); SecurityContextHolder.setContext(securityContext);