filters = getFilters("/someurl");
-
- assertFalse(filters.get(1) instanceof SessionFixationProtectionFilter);
+ assertTrue(filters.get(8) instanceof ExceptionTranslationFilter);
+ assertFalse(filters.get(9) instanceof SessionFixationProtectionFilter);
}
/**
diff --git a/web/src/main/java/org/springframework/security/web/session/DefaultAuthenticatedSessionStrategy.java b/web/src/main/java/org/springframework/security/web/session/DefaultAuthenticatedSessionStrategy.java
index ec33205b03..573cb812f2 100644
--- a/web/src/main/java/org/springframework/security/web/session/DefaultAuthenticatedSessionStrategy.java
+++ b/web/src/main/java/org/springframework/security/web/session/DefaultAuthenticatedSessionStrategy.java
@@ -14,16 +14,19 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.security.authentication.concurrent.SessionRegistry;
import org.springframework.security.core.Authentication;
-import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.web.savedrequest.SavedRequest;
/**
* The default implementation of {@link AuthenticatedSessionStrategy}.
*
- * Creates a new session for the newly authenticated user if they already have a session, and copies their
+ * Creates a new session for the newly authenticated user if they already have a session (as a defence against
+ * session-fixation protection attacks), and copies their
* session attributes across to the new session (can be disabled by setting migrateSessionAttributes to
* false).
*
+ * This approach will only be effective if your servlet container always assigns a new session Id when a session is
+ * invalidated and a new session created by calling {@link HttpServletRequest#getSession()}.
+ *
* If concurrent session control is in use, then a SessionRegistry must be injected.
*
* @author Luke Taylor
@@ -91,6 +94,11 @@ public class DefaultAuthenticatedSessionStrategy implements AuthenticatedSession
logger.debug("Started new session: " + session.getId());
}
+ if (originalSessionId.equals(session.getId())) {
+ logger.warn("Your servlet container did not change the session ID when a new session was created. You will" +
+ " not be adequately protected against session-fixation attacks");
+ }
+
// Copy attributes to new session
if (attributesToMigrate != null) {
for (Map.Entry entry : attributesToMigrate.entrySet()) {
@@ -101,8 +109,7 @@ public class DefaultAuthenticatedSessionStrategy implements AuthenticatedSession
// Update the session registry
if (sessionRegistry != null) {
sessionRegistry.removeSessionInformation(originalSessionId);
- sessionRegistry.registerNewSession(session.getId(),
- SecurityContextHolder.getContext().getAuthentication().getPrincipal());
+ sessionRegistry.registerNewSession(session.getId(), authentication.getPrincipal());
}
}
@@ -152,4 +159,8 @@ public class DefaultAuthenticatedSessionStrategy implements AuthenticatedSession
public void setRetainedAttributes(List retainedAttributes) {
this.retainedAttributes = retainedAttributes;
}
+
+ public void setAlwaysCreateSession(boolean alwaysCreateSession) {
+ this.alwaysCreateSession = alwaysCreateSession;
+ }
}
diff --git a/web/src/test/java/org/springframework/security/web/session/DefaultAuthenticatedSessionStrategyTests.java b/web/src/test/java/org/springframework/security/web/session/DefaultAuthenticatedSessionStrategyTests.java
index 912e4862a5..bda335d4c2 100644
--- a/web/src/test/java/org/springframework/security/web/session/DefaultAuthenticatedSessionStrategyTests.java
+++ b/web/src/test/java/org/springframework/security/web/session/DefaultAuthenticatedSessionStrategyTests.java
@@ -4,11 +4,14 @@ import static org.junit.Assert.*;
import static org.mockito.Mockito.mock;
import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpSession;
import org.junit.Test;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.authentication.concurrent.SessionRegistry;
import org.springframework.security.core.Authentication;
+import org.springframework.security.web.savedrequest.SavedRequest;
/**
*
@@ -30,6 +33,7 @@ public class DefaultAuthenticatedSessionStrategyTests {
@Test
public void newSessionIsCreatedIfSessionAlreadyExists() throws Exception {
DefaultAuthenticatedSessionStrategy strategy = new DefaultAuthenticatedSessionStrategy();
+ strategy.setSessionRegistry(mock(SessionRegistry.class));
HttpServletRequest request = new MockHttpServletRequest();
String sessionId = request.getSession().getId();
@@ -38,4 +42,27 @@ public class DefaultAuthenticatedSessionStrategyTests {
assertFalse(sessionId.equals(request.getSession().getId()));
}
+ // See SEC-1077
+ @Test
+ public void onlySavedRequestAttributeIsMigratedIfMigrateAttributesIsFalse() throws Exception {
+ DefaultAuthenticatedSessionStrategy strategy = new DefaultAuthenticatedSessionStrategy();
+ strategy.setMigrateSessionAttributes(false);
+ HttpServletRequest request = new MockHttpServletRequest();
+ HttpSession session = request.getSession();
+ session.setAttribute("blah", "blah");
+ session.setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, "SavedRequest");
+
+ strategy.onAuthenticationSuccess(mock(Authentication.class), request, new MockHttpServletResponse());
+
+ assertNull(request.getSession().getAttribute("blah"));
+ assertNotNull(request.getSession().getAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY));
+ }
+
+ @Test
+ public void sessionIsCreatedIfAlwaysCreateTrue() throws Exception {
+ DefaultAuthenticatedSessionStrategy strategy = new DefaultAuthenticatedSessionStrategy();
+ strategy.setAlwaysCreateSession(true);
+
+ }
+
}