diff --git a/core/src/main/java/org/springframework/security/ui/SessionFixationProtectionFilter.java b/core/src/main/java/org/springframework/security/ui/SessionFixationProtectionFilter.java index 12a6c3b0db..34285df2e7 100644 --- a/core/src/main/java/org/springframework/security/ui/SessionFixationProtectionFilter.java +++ b/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. + *
+ * 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; } - } - + } } diff --git a/core/src/main/java/org/springframework/security/util/SessionUtils.java b/core/src/main/java/org/springframework/security/util/SessionUtils.java index 7c42f0914a..c85be7d258 100644 --- a/core/src/main/java/org/springframework/security/util/SessionUtils.java +++ b/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(); diff --git a/core/src/test/java/org/springframework/security/ui/SessionFixationProtectionFilterTests.java b/core/src/test/java/org/springframework/security/ui/SessionFixationProtectionFilterTests.java index 37d1eb1377..cf34f39b23 100644 --- a/core/src/test/java/org/springframework/security/ui/SessionFixationProtectionFilterTests.java +++ b/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(); } }