From ffe2834f4cd900d99c4a490af62613d087c9aceb Mon Sep 17 00:00:00 2001
From: Rob Winch <rwinch@vmware.com>
Date: Tue, 7 Aug 2012 15:37:17 -0500
Subject: [PATCH] SEC-2027: Invoke SecurityContextHolder.clearContext() only on
 outer invocation of FilterChainProxy

When SEC-1950 was introduced it caused problems when a <filter-mapping> was mapped
to multiple dispatchers (i.e. REQUEST and FORWARD) since when the second dispatcher
completed execution it cleared the SecurityContext and the original FilterChain
would then save the cleared out SecurityContext.

We now use a pattern similar to the OncePerRequestFilter to only invoke
SecurityContextHolder.clearContext() on the first invocation of the Filter. We do not simply extend
OncePerRequestFilter because we want to invoke the delegate filters for every request.
---
 .../security/web/FilterChainProxy.java        | 16 ++++++++---
 .../security/web/FilterChainProxyTests.java   | 27 +++++++++++++++++++
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java
index e4df6edbe9..34b7497ae7 100644
--- a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java
+++ b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java
@@ -125,6 +125,8 @@ public class FilterChainProxy extends GenericFilterBean {
 
     //~ Instance fields ================================================================================================
 
+    private final static String FILTER_APPLIED = FilterChainProxy.class.getName().concat(".APPLIED");
+
     private List<SecurityFilterChain> filterChains;
 
     private FilterChainValidator filterChainValidator = new NullFilterChainValidator();
@@ -151,11 +153,17 @@ public class FilterChainProxy extends GenericFilterBean {
 
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
             throws IOException, ServletException {
-        try {
+        boolean clearContext = request.getAttribute(FILTER_APPLIED) == null;
+        if(clearContext) {
+            try {
+                request.setAttribute(FILTER_APPLIED, Boolean.TRUE);
+                doFilterInternal(request, response, chain);
+            } finally {
+                SecurityContextHolder.clearContext();
+                request.removeAttribute(FILTER_APPLIED);
+            }
+        } else {
             doFilterInternal(request, response, chain);
-        } finally {
-            // SEC-1950
-            SecurityContextHolder.clearContext();
         }
     }
 
diff --git a/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java b/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java
index 78f9b5f500..71f1d91ab5 100644
--- a/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java
+++ b/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java
@@ -196,4 +196,31 @@ public class FilterChainProxyTests {
 
         assertNull(SecurityContextHolder.getContext().getAuthentication());
     }
+
+    // SEC-2027
+    @Test
+    public void doFilterClearsSecurityContextHolderOnceOnForwards() throws Exception {
+        final FilterChain innerChain = mock(FilterChain.class);
+        when(matcher.matches(any(HttpServletRequest.class))).thenReturn(true);
+        doAnswer(new Answer<Object>() {
+            public Object answer(InvocationOnMock inv) throws Throwable {
+                TestingAuthenticationToken expected = new TestingAuthenticationToken("username", "password");
+                SecurityContextHolder.getContext().setAuthentication(expected);
+                doAnswer(new Answer<Object>() {
+                    public Object answer(InvocationOnMock inv) throws Throwable {
+                        innerChain.doFilter(request, response);
+                        return null;
+                    }
+                }).when(filter).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class), any(FilterChain.class));;
+                fcp.doFilter(request, response, innerChain);
+                assertSame(expected, SecurityContextHolder.getContext().getAuthentication());
+                return null;
+            }
+        }).when(filter).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class), any(FilterChain.class));
+
+        fcp.doFilter(request, response, chain);
+
+        verify(innerChain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
+        assertNull(SecurityContextHolder.getContext().getAuthentication());
+    }
 }