Browse Source

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.
Rob Winch 12 years ago
parent
commit
3437ef714a

+ 5 - 5
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
  * 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} when {@link #preProcess(NativeWebRequest, Callable)} is invoked. It also clear out the
  * {@link SecurityContextHolder} by invoking {@link SecurityContextHolder#clearContext()} in the
  * {@link SecurityContextHolder} by invoking {@link SecurityContextHolder#clearContext()} in the
- * {@link #afterCompletion(NativeWebRequest, Callable)} method.
+ * {@link #postProcess(NativeWebRequest, Callable, Object)} method.
  * </p>
  * </p>
  *
  *
  * @author Rob Winch
  * @author Rob Winch
@@ -64,13 +64,13 @@ public final class SecurityContextCallableProcessingInterceptor extends Callable
     }
     }
 
 
     @Override
     @Override
-    public <T> void afterCompletion(NativeWebRequest request, Callable<T> task) throws Exception {
-        SecurityContextHolder.clearContext();
+    public <T> void preProcess(NativeWebRequest request, Callable<T> task) throws Exception {
+        SecurityContextHolder.setContext(securityContext);
     }
     }
 
 
     @Override
     @Override
-    public <T> void preProcess(NativeWebRequest request, Callable<T> task) throws Exception {
-        SecurityContextHolder.setContext(securityContext);
+    public <T> void postProcess(NativeWebRequest request, Callable<T> task, Object concurrentResult) throws Exception {
+        SecurityContextHolder.clearContext();
     }
     }
 
 
     private void setSecurityContext(SecurityContext securityContext) {
     private void setSecurityContext(SecurityContext securityContext) {

+ 2 - 2
web/src/test/java/org/springframework/security/web/context/request/async/SecurityContextCallableProcessingInterceptorTests.java

@@ -59,7 +59,7 @@ public class SecurityContextCallableProcessingInterceptorTests {
         interceptor.preProcess(webRequest, callable);
         interceptor.preProcess(webRequest, callable);
         assertThat(SecurityContextHolder.getContext()).isSameAs(securityContext);
         assertThat(SecurityContextHolder.getContext()).isSameAs(securityContext);
 
 
-        interceptor.afterCompletion(webRequest, callable);
+        interceptor.postProcess(webRequest, callable, null);
         assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext);
         assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext);
     }
     }
 
 
@@ -71,7 +71,7 @@ public class SecurityContextCallableProcessingInterceptorTests {
         interceptor.preProcess(webRequest, callable);
         interceptor.preProcess(webRequest, callable);
         assertThat(SecurityContextHolder.getContext()).isSameAs(securityContext);
         assertThat(SecurityContextHolder.getContext()).isSameAs(securityContext);
 
 
-        interceptor.afterCompletion(webRequest, callable);
+        interceptor.postProcess(webRequest, callable, null);
         assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext);
         assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext);
     }
     }
 }
 }

+ 16 - 0
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.mock.web.MockFilterChain;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
 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.AsyncWebRequest;
+import org.springframework.web.context.request.async.CallableProcessingInterceptorAdapter;
 import org.springframework.web.context.request.async.WebAsyncManager;
 import org.springframework.web.context.request.async.WebAsyncManager;
 import org.springframework.web.context.request.async.WebAsyncUtils;
 import org.springframework.web.context.request.async.WebAsyncUtils;
 
 
@@ -84,6 +86,13 @@ public class WebAsyncManagerIntegrationFilterTests {
     @Test
     @Test
     public void doFilterInternalRegistersSecurityContextCallableProcessor() throws Exception {
     public void doFilterInternalRegistersSecurityContextCallableProcessor() throws Exception {
         SecurityContextHolder.setContext(securityContext);
         SecurityContextHolder.setContext(securityContext);
+        asyncManager.registerCallableInterceptors(new CallableProcessingInterceptorAdapter() {
+            @Override
+            public <T> void postProcess(NativeWebRequest request, Callable<T> task, Object concurrentResult)
+                    throws Exception {
+                assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext);
+            }
+        });
         filter.doFilterInternal(request, response, filterChain);
         filter.doFilterInternal(request, response, filterChain);
 
 
         VerifyingCallable verifyingCallable = new VerifyingCallable();
         VerifyingCallable verifyingCallable = new VerifyingCallable();
@@ -95,6 +104,13 @@ public class WebAsyncManagerIntegrationFilterTests {
     @Test
     @Test
     public void doFilterInternalRegistersSecurityContextCallableProcessorContextUpdated() throws Exception {
     public void doFilterInternalRegistersSecurityContextCallableProcessorContextUpdated() throws Exception {
         SecurityContextHolder.setContext(SecurityContextHolder.createEmptyContext());
         SecurityContextHolder.setContext(SecurityContextHolder.createEmptyContext());
+        asyncManager.registerCallableInterceptors(new CallableProcessingInterceptorAdapter() {
+            @Override
+            public <T> void postProcess(NativeWebRequest request, Callable<T> task, Object concurrentResult)
+                    throws Exception {
+                assertThat(SecurityContextHolder.getContext()).isNotSameAs(securityContext);
+            }
+        });
         filter.doFilterInternal(request, response, filterChain);
         filter.doFilterInternal(request, response, filterChain);
         SecurityContextHolder.setContext(securityContext);
         SecurityContextHolder.setContext(securityContext);