diff --git a/web/src/main/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilter.java b/web/src/main/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilter.java index a10791c7ce..c0049e5fd6 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilter.java +++ b/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 Authentication object in the SecurityContextHolder, 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. - *

- * Defaults to true, being the most optimal and appropriate - * option – AnonymousAuthenticationFilter 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; } diff --git a/web/src/test/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilterTests.java b/web/src/test/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilterTests.java index afe9b60a66..bdaffa0bf4 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilterTests.java +++ b/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"); } }