SEC-689: Further tests, logging improvements.

core/src/main/java/org/springframework/security/ui/SessionFixationProtectionFilter.java

@@ -99,13 +99,16 @@ public class SessionFixationProtectionFilter extends SpringSecurityFilter {
     /**
      * Response wrapper to handle the situation where we need to migrate the session after a redirect or sendError.
      * Similar in function to Martin Algesten's OnRedirectUpdateSessionResponseWrapper used in 
-     * HttpSessionContextIntegrationFilter.  
+     * HttpSessionContextIntegrationFilter.
+     * <p>
+     * Only used to wrap the response if the conditions are right at the start of the request to potentially 
+     * require starting a new session, i.e. that the user isn't authenticated and a session existed to begin with.  
      */
-    private class SessionFixationProtectionResponseWrapper extends HttpServletResponseWrapper {
+    class SessionFixationProtectionResponseWrapper extends HttpServletResponseWrapper {
         private HttpServletRequest request;
         private boolean newSessionStarted;
 
-        public SessionFixationProtectionResponseWrapper(HttpServletResponse response, HttpServletRequest request) {
+        SessionFixationProtectionResponseWrapper(HttpServletResponse response, HttpServletRequest request) {
             super(response);
             this.request = request;
         }
@@ -148,9 +151,8 @@ public class SessionFixationProtectionFilter extends SpringSecurityFilter {
             newSessionStarted = true;
         }
 
-        private boolean isNewSessionStarted() {
+        boolean isNewSessionStarted() {
             return newSessionStarted;
         }
-    }    
-
+    }
 }

core/src/main/java/org/springframework/security/util/SessionUtils.java

@@ -36,7 +36,7 @@ public final class SessionUtils {
         String originalSessionId = session.getId();
 
         if (logger.isDebugEnabled()) {
-            logger.debug("Invalidating session " + (migrateAttributes ? "and" : "without") +  " migrating attributes.");
+            logger.debug("Invalidating session with Id '" + originalSessionId +"' " + (migrateAttributes ? "and" : "without") +  " migrating attributes.");
         }        
 
         HashMap attributesToMigrate = null;
@@ -55,6 +55,10 @@ public final class SessionUtils {
         session.invalidate();
         session = request.getSession(true); // we now have a new session
 
+        if (logger.isDebugEnabled()) {
+            logger.debug("Started new session: " + session.getId());
+        }
+        
         if (attributesToMigrate != null) {
             Iterator iter = attributesToMigrate.entrySet().iterator();
 

core/src/test/java/org/springframework/security/ui/SessionFixationProtectionFilterTests.java

@@ -45,7 +45,6 @@ public class SessionFixationProtectionFilterTests {
         SessionFixationProtectionFilter filter = new SessionFixationProtectionFilter();
         HttpServletRequest request = new MockHttpServletRequest();
         String sessionId = request.getSession().getId();
-//        SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", "pass", null));
         
         filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain());
         
@@ -68,7 +67,7 @@ public class SessionFixationProtectionFilterTests {
         SessionFixationProtectionFilter filter = new SessionFixationProtectionFilter();
         HttpServletRequest request = new MockHttpServletRequest();
         String sessionId = request.getSession().getId();
-        SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", "pass", null));
+        authenticateUser();
         
         filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain());
         
@@ -83,7 +82,7 @@ public class SessionFixationProtectionFilterTests {
         
         filter.doFilter(request, new MockHttpServletResponse(), new UserAuthenticatingFilterChain());
         
-        assertFalse("Session Id should have changed", sessionId.equals(request.getSession().getId()));         
+        assertFalse(sessionId.equals(request.getSession().getId()));         
     }
     
     @Test
@@ -99,12 +98,47 @@ public class SessionFixationProtectionFilterTests {
                 SessionFixationProtectionFilter.SessionFixationProtectionResponseWrapper);
         assertTrue("New session should have been created by session wrapper",
                 ((SessionFixationProtectionFilter.SessionFixationProtectionResponseWrapper)chain.getResponse()).isNewSessionStarted());
-        assertFalse("Session Id should have changed", sessionId.equals(request.getSession().getId()));
+        assertFalse(sessionId.equals(request.getSession().getId()));
+    }
+    
+    @Test
+    public void wrapperSendErrorCreatesNewSession() throws Exception {
+        authenticateUser();
+        SessionFixationProtectionFilter filter = new SessionFixationProtectionFilter();
+        HttpServletRequest request = new MockHttpServletRequest();
+        String sessionId = request.getSession().getId();
+        SessionFixationProtectionFilter.SessionFixationProtectionResponseWrapper wrapper = 
+            filter.new SessionFixationProtectionResponseWrapper(new MockHttpServletResponse(), request);
+        wrapper.sendError(HttpServletResponse.SC_FORBIDDEN);
+        assertFalse(sessionId.equals(request.getSession().getId()));
+        
+        // Message version
+        request = new MockHttpServletRequest();
+        sessionId = request.getSession().getId();
+        wrapper = filter.new SessionFixationProtectionResponseWrapper(new MockHttpServletResponse(), request);
+        wrapper.sendError(HttpServletResponse.SC_FORBIDDEN, "Hi. I'm your friendly forbidden message.");
+        assertFalse(sessionId.equals(request.getSession().getId()));        
+    }
+
+    @Test
+    public void wrapperRedirectCreatesNewSession() throws Exception {
+        authenticateUser();
+        SessionFixationProtectionFilter filter = new SessionFixationProtectionFilter();
+        HttpServletRequest request = new MockHttpServletRequest();
+        String sessionId = request.getSession().getId();
+        SessionFixationProtectionFilter.SessionFixationProtectionResponseWrapper wrapper = 
+            filter.new SessionFixationProtectionResponseWrapper(new MockHttpServletResponse(), request);
+        wrapper.sendRedirect("/somelocation");
+        assertFalse(sessionId.equals(request.getSession().getId()));
+    }
+    
+    private void authenticateUser() {
+        SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", "pass", null));
     }
     
     private class UserAuthenticatingFilterChain implements FilterChain {
-        public void doFilter(ServletRequest request, ServletResponse response) throws IOException {           
-            SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", "pass", null));
+        public void doFilter(ServletRequest request, ServletResponse response) throws IOException { 
+            authenticateUser();
         }
     }