diff --git a/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java b/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java index e653e6417e..001da04b8a 100644 --- a/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java +++ b/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java @@ -127,8 +127,7 @@ import javax.servlet.http.HttpSession; * is true. * * @author Ben Alex - * @version $Id: AbstractProcessingFilter.java 1909 2007-06-19 04:08:19Z - * vishalpuri $ + * @version $Id$ */ public abstract class AbstractProcessingFilter extends SpringSecurityFilter implements InitializingBean, ApplicationEventPublisherAware, MessageSourceAware { @@ -364,45 +363,46 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl private void startNewSessionIfRequired(HttpServletRequest request) { HttpSession session = request.getSession(false); - if (session != null) { + if (session == null) { + return; + } - if (!migrateInvalidatedSessionAttributes) { + if (!migrateInvalidatedSessionAttributes) { - if (logger.isDebugEnabled()) { - logger.debug("Invalidating session without migrating attributes."); - } + if (logger.isDebugEnabled()) { + logger.debug("Invalidating session without migrating attributes."); + } - session.invalidate(); - session = null; + session.invalidate(); + session = null; - // this is probably not necessary, but seems cleaner since - // there already was a session going. - request.getSession(true); + // this is probably not necessary, but seems cleaner since + // there already was a session going. + request.getSession(true); - } else { + } else { - if (logger.isDebugEnabled()) { - logger.debug("Invalidating session and migrating attributes."); - } + if (logger.isDebugEnabled()) { + logger.debug("Invalidating session and migrating attributes."); + } - HashMap migratedAttributes = new HashMap(); + HashMap migratedAttributes = new HashMap(); - Enumeration enumer = session.getAttributeNames(); + Enumeration enumer = session.getAttributeNames(); - while (enumer.hasMoreElements()) { - String key = (String) enumer.nextElement(); - migratedAttributes.put(key, session.getAttribute(key)); - } + while (enumer.hasMoreElements()) { + String key = (String) enumer.nextElement(); + migratedAttributes.put(key, session.getAttribute(key)); + } - session.invalidate(); - session = request.getSession(true); // we now have a new session + session.invalidate(); + session = request.getSession(true); // we now have a new session - Iterator iter = migratedAttributes.entrySet().iterator(); + Iterator iter = migratedAttributes.entrySet().iterator(); - while (iter.hasNext()) { - Map.Entry entry = (Map.Entry) iter.next(); - session.setAttribute((String) entry.getKey(), entry.getValue()); - } + while (iter.hasNext()) { + Map.Entry entry = (Map.Entry) iter.next(); + session.setAttribute((String) entry.getKey(), entry.getValue()); } } } @@ -558,5 +558,4 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl public void setUseRelativeContext(boolean useRelativeContext) { this.useRelativeContext = useRelativeContext; } - } diff --git a/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java b/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java index c0ec89342c..64039a9c0a 100644 --- a/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java +++ b/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java @@ -31,6 +31,7 @@ import org.springframework.security.util.PortResolverImpl; import org.springframework.mock.web.MockFilterConfig; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.mock.web.MockHttpSession; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -40,6 +41,7 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; import java.io.IOException; import java.util.Properties; @@ -254,6 +256,7 @@ public class AbstractProcessingFilterTests extends TestCase { public void testNormalOperationWithDefaultFilterProcessesUrl() throws Exception { // Setup our HTTP request MockHttpServletRequest request = createMockRequest(); + HttpSession sessionPreAuth = request.getSession(); // Setup our filter configuration MockFilterConfig config = new MockFilterConfig(null, null); @@ -275,6 +278,8 @@ public class AbstractProcessingFilterTests extends TestCase { assertEquals("/mycontext/logged_in.jsp", response.getRedirectedUrl()); assertNotNull(SecurityContextHolder.getContext().getAuthentication()); assertEquals("test", SecurityContextHolder.getContext().getAuthentication().getPrincipal().toString()); + // Should still have the same session + assertEquals(sessionPreAuth, request.getSession()); } public void testStartupDetectsInvalidAuthenticationFailureUrl() throws Exception { @@ -373,7 +378,7 @@ public class AbstractProcessingFilterTests extends TestCase { } public void testSuccessfulAuthenticationButWithAlwaysUseDefaultTargetUrlCausesRedirectToDefaultTargetUrl() - throws Exception { + throws Exception { // Setup our HTTP request MockHttpServletRequest request = createMockRequest(); request.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl()); @@ -433,7 +438,6 @@ public class AbstractProcessingFilterTests extends TestCase { // Setup our test object, to grant access MockAbstractProcessingFilter filter = new MockAbstractProcessingFilter(true); - filter.setFilterProcessesUrl("/j_mock_post"); filter.setDefaultTargetUrl("http://monkeymachine.co.uk/"); filter.setAlwaysUseDefaultTargetUrl(true); @@ -442,6 +446,48 @@ public class AbstractProcessingFilterTests extends TestCase { assertNotNull(SecurityContextHolder.getContext().getAuthentication()); } + public void testNewSessionIsCreatedIfInvalidateSessionOnSuccessfulAuthenticationIsSet() throws Exception { + MockHttpServletRequest request = createMockRequest(); + HttpSession oldSession = request.getSession(); + oldSession.setAttribute("test","test"); + MockFilterConfig config = new MockFilterConfig(null, null); + + MockFilterChain chain = new MockFilterChain(true); + MockHttpServletResponse response = new MockHttpServletResponse(); + + // Setup our test object, to grant access + MockAbstractProcessingFilter filter = new MockAbstractProcessingFilter(true); + filter.setInvalidateSessionOnSuccessfulAuthentication(true); + filter.setDefaultTargetUrl("http://monkeymachine.co.uk/"); + + executeFilterInContainerSimulator(config, filter, request, response, chain); + + HttpSession newSession = request.getSession(); + assertFalse(newSession.getId().equals(oldSession.getId())); + assertEquals("test", newSession.getAttribute("test")); + } + + public void testAttributesAreNotMigratedToNewlyCreatedSessionIfMigrateAttributesIsFalse() throws Exception { + MockHttpServletRequest request = createMockRequest(); + HttpSession oldSession = request.getSession(); + MockFilterConfig config = new MockFilterConfig(null, null); + + MockFilterChain chain = new MockFilterChain(true); + MockHttpServletResponse response = new MockHttpServletResponse(); + + // Setup our test object, to grant access + MockAbstractProcessingFilter filter = new MockAbstractProcessingFilter(true); + filter.setInvalidateSessionOnSuccessfulAuthentication(true); + filter.setMigrateInvalidatedSessionAttributes(false); + filter.setDefaultTargetUrl("http://monkeymachine.co.uk/"); + + executeFilterInContainerSimulator(config, filter, request, response, chain); + + HttpSession newSession = request.getSession(); + assertFalse(newSession.getId().equals(oldSession.getId())); + assertNull(newSession.getAttribute("test")); + } + //~ Inner Classes ================================================================================================== private class MockAbstractProcessingFilter extends AbstractProcessingFilter {