SEC-1316: Remove 'removeAfterRequest' property from AnonymousAuthenticationFilter

web/src/main/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilter.java

@@ -23,7 +23,6 @@ import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
 
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.security.authentication.AnonymousAuthenticationToken;
@@ -36,7 +35,7 @@ import org.springframework.web.filter.GenericFilterBean;
 
 
 /**
- * Detects if there is no <code>Authentication</code> object in the <code>SecurityContextHolder</code>,  and
+ * Detects if there is no {@code Authentication} object in the {@code SecurityContextHolder}, and
  * populates it with one if needed.
  *
  * @author Ben Alex
@@ -49,7 +48,6 @@ public class AnonymousAuthenticationFilter extends GenericFilterBean  implements
     private AuthenticationDetailsSource authenticationDetailsSource = new WebAuthenticationDetailsSource();
     private String key;
     private UserAttribute userAttribute;
-    private boolean removeAfterRequest = true;
 
     //~ Methods ========================================================================================================
 
@@ -59,6 +57,28 @@ public class AnonymousAuthenticationFilter extends GenericFilterBean  implements
         Assert.hasLength(key);
     }
 
+    public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
+            throws IOException, ServletException {
+
+        if (applyAnonymousForThisRequest((HttpServletRequest) req)) {
+            if (SecurityContextHolder.getContext().getAuthentication() == null) {
+                SecurityContextHolder.getContext().setAuthentication(createAuthentication((HttpServletRequest) req));
+
+                if (logger.isDebugEnabled()) {
+                    logger.debug("Populated SecurityContextHolder with anonymous token: '"
+                        + SecurityContextHolder.getContext().getAuthentication() + "'");
+                }
+            } else {
+                if (logger.isDebugEnabled()) {
+                    logger.debug("SecurityContextHolder not populated with anonymous token, as it already contained: '"
+                        + SecurityContextHolder.getContext().getAuthentication() + "'");
+                }
+            }
+        }
+
+        chain.doFilter(req, res);
+    }
+
     /**
      * Enables subclasses to determine whether or not an anonymous authentication token should be setup for
      * this request. This is useful if anonymous authentication should be allowed only for specific IP subnet ranges
@@ -77,45 +97,11 @@ public class AnonymousAuthenticationFilter extends GenericFilterBean  implements
     protected Authentication createAuthentication(HttpServletRequest request) {
         AnonymousAuthenticationToken auth = new AnonymousAuthenticationToken(key, userAttribute.getPassword(),
                 userAttribute.getAuthorities());
-        auth.setDetails(authenticationDetailsSource.buildDetails((HttpServletRequest) request));
+        auth.setDetails(authenticationDetailsSource.buildDetails(request));
 
         return auth;
     }
 
-    public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
-            throws IOException, ServletException {
-        HttpServletRequest request = (HttpServletRequest) req;
-        HttpServletResponse response = (HttpServletResponse) res;
-
-        boolean addedToken = false;
-
-        if (applyAnonymousForThisRequest(request)) {
-            if (SecurityContextHolder.getContext().getAuthentication() == null) {
-                SecurityContextHolder.getContext().setAuthentication(createAuthentication(request));
-                addedToken = true;
-
-                if (logger.isDebugEnabled()) {
-                    logger.debug("Populated SecurityContextHolder with anonymous token: '"
-                        + SecurityContextHolder.getContext().getAuthentication() + "'");
-                }
-            } else {
-                if (logger.isDebugEnabled()) {
-                    logger.debug("SecurityContextHolder not populated with anonymous token, as it already contained: '"
-                        + SecurityContextHolder.getContext().getAuthentication() + "'");
-                }
-            }
-        }
-
-        try {
-            chain.doFilter(request, response);
-        } finally {
-            if (addedToken && removeAfterRequest
-                && createAuthentication(request).equals(SecurityContextHolder.getContext().getAuthentication())) {
-                SecurityContextHolder.getContext().setAuthentication(null);
-            }
-        }
-    }
-
     public String getKey() {
         return key;
     }
@@ -124,10 +110,6 @@ public class AnonymousAuthenticationFilter extends GenericFilterBean  implements
         return userAttribute;
     }
 
-    public boolean isRemoveAfterRequest() {
-        return removeAfterRequest;
-    }
-
     public void setAuthenticationDetailsSource(AuthenticationDetailsSource authenticationDetailsSource) {
         Assert.notNull(authenticationDetailsSource, "AuthenticationDetailsSource required");
         this.authenticationDetailsSource = authenticationDetailsSource;
@@ -137,21 +119,6 @@ public class AnonymousAuthenticationFilter extends GenericFilterBean  implements
         this.key = key;
     }
 
-    /**
-     * Controls whether the filter will remove the Anonymous token after the request is complete. Generally
-     * this is desired to avoid the expense of a session being created by {@link
-     * org.springframework.security.web.context.HttpSessionContextIntegrationFilter HttpSessionContextIntegrationFilter}
-     * simply to store the Anonymous authentication token.
-     * <p>
-     * Defaults to <code>true</code>, being the most optimal and appropriate
-     * option &ndash; <code>AnonymousAuthenticationFilter</code> will clear the token at the end of each request,
-     * thus avoiding session creation overhead in a typical configuration.
-     *
-     */
-    public void setRemoveAfterRequest(boolean removeAfterRequest) {
-        this.removeAfterRequest = removeAfterRequest;
-    }
-
     public void setUserAttribute(UserAttribute userAttributeDefinition) {
         this.userAttribute = userAttributeDefinition;
     }

web/src/test/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilterTests.java

@@ -15,6 +15,7 @@
 
 package org.springframework.security.web.authentication;
 
+import static org.junit.Assert.*;
 import static org.mockito.Mockito.mock;
 
 import java.io.IOException;
@@ -26,8 +27,9 @@ import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 
-import junit.framework.TestCase;
-
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.security.authentication.TestingAuthenticationToken;
@@ -45,27 +47,22 @@ import org.springframework.security.core.userdetails.memory.UserAttribute;
  * @author Ben Alex
  * @version $Id$
  */
-public class AnonymousAuthenticationFilterTests extends TestCase {
+public class AnonymousAuthenticationFilterTests {
 
     //~ Methods ========================================================================================================
 
     private void executeFilterInContainerSimulator(FilterConfig filterConfig, Filter filter, ServletRequest request,
         ServletResponse response, FilterChain filterChain) throws ServletException, IOException {
-//        filter.init(filterConfig);
         filter.doFilter(request, response, filterChain);
-//        filter.destroy();
-    }
-
-    protected void setUp() throws Exception {
-        super.setUp();
-        SecurityContextHolder.clearContext();
     }
 
-    protected void tearDown() throws Exception {
-        super.tearDown();
+    @Before
+    @After
+    public void clearContext() throws Exception {
         SecurityContextHolder.clearContext();
     }
 
+    @Test(expected=IllegalArgumentException.class)
     public void testDetectsMissingKey() throws Exception {
         UserAttribute user = new UserAttribute();
         user.setPassword("anonymousUsername");
@@ -73,27 +70,17 @@ public class AnonymousAuthenticationFilterTests extends TestCase {
 
         AnonymousAuthenticationFilter filter = new AnonymousAuthenticationFilter();
         filter.setUserAttribute(user);
-
-        try {
-            filter.afterPropertiesSet();
-            fail("Should have thrown IllegalArgumentException");
-        } catch (IllegalArgumentException expected) {
-            assertTrue(true);
-        }
+        filter.afterPropertiesSet();
     }
 
+    @Test(expected=IllegalArgumentException.class)
     public void testDetectsUserAttribute() throws Exception {
         AnonymousAuthenticationFilter filter = new AnonymousAuthenticationFilter();
         filter.setKey("qwerty");
-
-        try {
-            filter.afterPropertiesSet();
-            fail("Should have thrown IllegalArgumentException");
-        } catch (IllegalArgumentException expected) {
-            assertTrue(true);
-        }
+        filter.afterPropertiesSet();
     }
 
+    @Test
     public void testGettersSetters() throws Exception {
         UserAttribute user = new UserAttribute();
         user.setPassword("anonymousUsername");
@@ -102,15 +89,13 @@ public class AnonymousAuthenticationFilterTests extends TestCase {
         AnonymousAuthenticationFilter filter = new AnonymousAuthenticationFilter();
         filter.setKey("qwerty");
         filter.setUserAttribute(user);
-        assertTrue(filter.isRemoveAfterRequest());
         filter.afterPropertiesSet();
 
         assertEquals("qwerty", filter.getKey());
         assertEquals(user, filter.getUserAttribute());
-        filter.setRemoveAfterRequest(false);
-        assertFalse(filter.isRemoveAfterRequest());
     }
 
+    @Test
     public void testOperationWhenAuthenticationExistsInContextHolder()
         throws Exception {
         // Put an Authentication object into the SecurityContextHolder
@@ -138,6 +123,7 @@ public class AnonymousAuthenticationFilterTests extends TestCase {
         assertEquals(originalAuth, SecurityContextHolder.getContext().getAuthentication());
     }
 
+    @Test
     public void testOperationWhenNoAuthenticationInSecurityContextHolder() throws Exception {
         UserAttribute user = new UserAttribute();
         user.setPassword("anonymousUsername");
@@ -146,7 +132,6 @@ public class AnonymousAuthenticationFilterTests extends TestCase {
         AnonymousAuthenticationFilter filter = new AnonymousAuthenticationFilter();
         filter.setKey("qwerty");
         filter.setUserAttribute(user);
-        filter.setRemoveAfterRequest(false); // set to non-default value
         filter.afterPropertiesSet();
 
         MockHttpServletRequest request = new MockHttpServletRequest();
@@ -158,12 +143,6 @@ public class AnonymousAuthenticationFilterTests extends TestCase {
         assertEquals("anonymousUsername", auth.getPrincipal());
         assertTrue(AuthorityUtils.authorityListToSet(auth.getAuthorities()).contains("ROLE_ANONYMOUS"));
         SecurityContextHolder.getContext().setAuthentication(null); // so anonymous fires again
-
-        // Now test operation if we have removeAfterRequest = true
-        filter.setRemoveAfterRequest(true); // set to default value
-        executeFilterInContainerSimulator(mock(FilterConfig.class), filter, request, new MockHttpServletResponse(),
-            new MockFilterChain(true));
-        assertNull(SecurityContextHolder.getContext().getAuthentication());
     }
 
     //~ Inner Classes ==================================================================================================
@@ -175,11 +154,8 @@ public class AnonymousAuthenticationFilterTests extends TestCase {
             this.expectToProceed = expectToProceed;
         }
 
-        public void doFilter(ServletRequest request, ServletResponse response)
-            throws IOException, ServletException {
-            if (expectToProceed) {
-                assertTrue(true);
-            } else {
+        public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException {
+            if (!expectToProceed) {
                 fail("Did not expect filter chain to proceed");
             }
         }