diff --git a/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionEvent.java b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionEvent.java new file mode 100644 index 00000000000..d1e6b4c3b8d --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionEvent.java @@ -0,0 +1,73 @@ +/* + * Copyright 2002-2012 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.session; + +import org.springframework.security.authentication.event.AbstractAuthenticationEvent; +import org.springframework.security.core.Authentication; +import org.springframework.util.Assert; + +/** + * Indicates a session ID was changed for the purposes of session fixation protection. + * + * @since 3.2 + * @see SessionFixationProtectionStrategy + * @author Nick Williams + */ +public class SessionFixationProtectionEvent extends AbstractAuthenticationEvent { + //~ Instance fields ================================================================================================ + + private final String oldSessionId; + + private final String newSessionId; + + //~ Constructors =================================================================================================== + + /** + * Constructs a new session fixation protection event. + * + * @param authentication The authentication object + * @param oldSessionId The old session ID before it was changed + * @param newSessionId The new session ID after it was changed + */ + public SessionFixationProtectionEvent(Authentication authentication, String oldSessionId, String newSessionId) { + super(authentication); + Assert.hasLength(oldSessionId); + Assert.hasLength(newSessionId); + this.oldSessionId = oldSessionId; + this.newSessionId = newSessionId; + } + + //~ Methods ======================================================================================================== + + /** + * Getter for the session ID before it was changed. + * + * @return the old session ID. + */ + public String getOldSessionId() { + return this.oldSessionId; + } + + /** + * Getter for the session ID after it was changed. + * + * @return the new session ID. + */ + public String getNewSessionId() { + return this.newSessionId; + } +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionMigrationEvent.java b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionMigrationEvent.java new file mode 100644 index 00000000000..ef048b5c97f --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionMigrationEvent.java @@ -0,0 +1,59 @@ +/* + * Copyright 2002-2012 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.authentication.session; + +import org.springframework.security.core.Authentication; + +/** + * Indicates a session was migrated for the purposes of session fixation protection. + * + * @since 3.2 + * @see SessionFixationProtectionStrategy + * @author Nick Williams + */ +public class SessionFixationProtectionMigrationEvent extends SessionFixationProtectionEvent { + //~ Instance fields ================================================================================================ + + private final boolean sessionAttributesMigrated; + + //~ Constructors =================================================================================================== + + /** + * Constructs a new session migration event. + * + * @param authentication The authentication object + * @param oldSessionId The old session ID before the session was migrated + * @param newSessionId The new session ID after the session was migrated + * @param sessionAttributesMigrated Whether or not all session attributes were migrated + */ + public SessionFixationProtectionMigrationEvent(Authentication authentication, String oldSessionId, + String newSessionId, boolean sessionAttributesMigrated) { + super(authentication, oldSessionId, newSessionId); + this.sessionAttributesMigrated = sessionAttributesMigrated; + } + + /** + * Getter that indicates whether all session attributes were migrated. If all session attributes were not migrated + * (due to the session fixation protection strategy being "new session"), the Spring Security-related session + * attributes were still migrated, regardless. + * + * @return {@code true} if all session attributes were migrated, {@code false} otherwise. + */ + public boolean sessionAttributesWereMigrated() { + return this.sessionAttributesMigrated; + } +} diff --git a/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java index 95326452127..68718335a07 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java +++ b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java @@ -2,6 +2,8 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.security.core.Authentication; import org.springframework.util.Assert; @@ -36,9 +38,12 @@ * @author Luke Taylor * @since 3.0 */ -public class SessionFixationProtectionStrategy implements SessionAuthenticationStrategy { +public class SessionFixationProtectionStrategy implements SessionAuthenticationStrategy, + ApplicationEventPublisherAware { protected final Log logger = LogFactory.getLog(this.getClass()); + private ApplicationEventPublisher applicationEventPublisher; + /** * Indicates that the session attributes of an existing session * should be migrated to the new session. Defaults to true. @@ -112,12 +117,22 @@ public void onAuthentication(Authentication authentication, HttpServletRequest r /** * Called when the session has been changed and the old attributes have been migrated to the new session. * Only called if a session existed to start with. Allows subclasses to plug in additional behaviour. + *

+ * The default implementation of this method publishes a {@link SessionFixationProtectionMigrationEvent} to notify + * the application that the session ID has changed and about which attributes were migrated. If you override this + * method and still wish these events to be published, you should call {@code super.onSessionChange()} within your + * overriding method. * * @param originalSessionId the original session identifier * @param newSession the newly created session * @param auth the token for the newly authenticated principal */ protected void onSessionChange(String originalSessionId, HttpSession newSession, Authentication auth) { + if(applicationEventPublisher != null) { + applicationEventPublisher.publishEvent(new SessionFixationProtectionMigrationEvent( + auth, originalSessionId, newSession.getId(), migrateSessionAttributes + )); + } } /** @@ -193,6 +208,10 @@ public void setMigrateSessionAttributes(boolean migrateSessionAttributes) { this.migrateSessionAttributes = migrateSessionAttributes; } + public void setApplicationEventPublisher(ApplicationEventPublisher applicationEventPublisher) { + this.applicationEventPublisher = applicationEventPublisher; + } + /** * @deprecated Override the {@code extractAttributes} method instead */ diff --git a/web/src/test/java/org/springframework/security/web/authentication/session/ConcurrentSessionControlStrategyTests.java b/web/src/test/java/org/springframework/security/web/authentication/session/ConcurrentSessionControlStrategyTests.java index c997162fdbf..a4ea70904f7 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/session/ConcurrentSessionControlStrategyTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/session/ConcurrentSessionControlStrategyTests.java @@ -1,17 +1,22 @@ package org.springframework.security.web.authentication.session; +import static org.junit.Assert.*; import static org.mockito.AdditionalMatchers.not; import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.context.ApplicationEvent; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.core.Authentication; @@ -55,9 +60,24 @@ public void onAuthenticationNewSession() { public void onAuthenticationChangeSession() { String originalSessionId = request.getSession().getId(); + // See SEC-2002: Make sure SessionFixationProtectionEvent or subclass is published + ApplicationEventPublisher eventPublisher = mock(ApplicationEventPublisher.class); + strategy.setApplicationEventPublisher(eventPublisher); + strategy.onAuthentication(authentication, request, response); verify(sessionRegistry,times(0)).removeSessionInformation(anyString()); verify(sessionRegistry).registerNewSession(not(eq(originalSessionId)), anyObject()); + + ArgumentCaptor eventArgumentCaptor = ArgumentCaptor.forClass(ApplicationEvent.class); + verify(eventPublisher).publishEvent(eventArgumentCaptor.capture()); + + // See SEC-2002: Make sure SessionFixationProtectionEvent or subclass is published + assertNotNull(eventArgumentCaptor.getValue()); + assertTrue(eventArgumentCaptor.getValue() instanceof SessionFixationProtectionEvent); + SessionFixationProtectionEvent event = (SessionFixationProtectionEvent)eventArgumentCaptor.getValue(); + assertEquals(originalSessionId, event.getOldSessionId()); + assertEquals(request.getSession().getId(), event.getNewSessionId()); + assertSame(authentication, event.getAuthentication()); } } diff --git a/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java b/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java index 63ccc731767..28e52e4b6cc 100644 --- a/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java +++ b/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java @@ -1,17 +1,22 @@ package org.springframework.security.web.session; -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.mockito.ArgumentCaptor; +import org.springframework.context.ApplicationEvent; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.core.Authentication; +import org.springframework.security.web.authentication.session.SessionFixationProtectionMigrationEvent; import org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy; -import org.springframework.security.web.savedrequest.HttpSessionRequestCache; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; + +import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; /** * @@ -33,11 +38,35 @@ public void newSessionShouldNotBeCreatedIfNoSessionExistsAndAlwaysCreateIsFalse( public void newSessionIsCreatedIfSessionAlreadyExists() throws Exception { SessionFixationProtectionStrategy strategy = new SessionFixationProtectionStrategy(); HttpServletRequest request = new MockHttpServletRequest(); - String sessionId = request.getSession().getId(); + HttpSession session = request.getSession(); + session.setAttribute("blah", "blah"); + session.setAttribute("SPRING_SECURITY_SAVED_REQUEST_KEY", "DefaultSavedRequest"); + String oldSessionId = session.getId(); - strategy.onAuthentication(mock(Authentication.class), request, new MockHttpServletResponse()); + // See SEC-2002: Make sure SessionFixationProtectionMigrationEvent is published + ApplicationEventPublisher eventPublisher = mock(ApplicationEventPublisher.class); + strategy.setApplicationEventPublisher(eventPublisher); + + Authentication mockAuthentication = mock(Authentication.class); + + strategy.onAuthentication(mockAuthentication, request, new MockHttpServletResponse()); + + ArgumentCaptor eventArgumentCaptor = ArgumentCaptor.forClass(ApplicationEvent.class); + verify(eventPublisher).publishEvent(eventArgumentCaptor.capture()); + + assertFalse(oldSessionId.equals(request.getSession().getId())); + assertNotNull(request.getSession().getAttribute("blah")); + assertNotNull(request.getSession().getAttribute("SPRING_SECURITY_SAVED_REQUEST_KEY")); - assertFalse(sessionId.equals(request.getSession().getId())); + // See SEC-2002: Make sure SessionFixationProtectionMigrationEvent is published + assertNotNull(eventArgumentCaptor.getValue()); + assertTrue(eventArgumentCaptor.getValue() instanceof SessionFixationProtectionMigrationEvent); + SessionFixationProtectionMigrationEvent event = + (SessionFixationProtectionMigrationEvent)eventArgumentCaptor.getValue(); + assertEquals(oldSessionId, event.getOldSessionId()); + assertEquals(request.getSession().getId(), event.getNewSessionId()); + assertSame(mockAuthentication, event.getAuthentication()); + assertTrue(event.sessionAttributesWereMigrated()); } // See SEC-1077 @@ -49,11 +78,31 @@ public void onlySavedRequestAttributeIsMigratedIfMigrateAttributesIsFalse() thro HttpSession session = request.getSession(); session.setAttribute("blah", "blah"); session.setAttribute("SPRING_SECURITY_SAVED_REQUEST_KEY", "DefaultSavedRequest"); + String oldSessionId = session.getId(); - strategy.onAuthentication(mock(Authentication.class), request, new MockHttpServletResponse()); + // See SEC-2002: Make sure SessionFixationProtectionMigrationEvent is published + ApplicationEventPublisher eventPublisher = mock(ApplicationEventPublisher.class); + strategy.setApplicationEventPublisher(eventPublisher); + + Authentication mockAuthentication = mock(Authentication.class); + + strategy.onAuthentication(mockAuthentication, request, new MockHttpServletResponse()); + + ArgumentCaptor eventArgumentCaptor = ArgumentCaptor.forClass(ApplicationEvent.class); + verify(eventPublisher).publishEvent(eventArgumentCaptor.capture()); assertNull(request.getSession().getAttribute("blah")); assertNotNull(request.getSession().getAttribute("SPRING_SECURITY_SAVED_REQUEST_KEY")); + + // See SEC-2002: Make sure SessionFixationProtectionMigrationEvent is published + assertNotNull(eventArgumentCaptor.getValue()); + assertTrue(eventArgumentCaptor.getValue() instanceof SessionFixationProtectionMigrationEvent); + SessionFixationProtectionMigrationEvent event = + (SessionFixationProtectionMigrationEvent)eventArgumentCaptor.getValue(); + assertEquals(oldSessionId, event.getOldSessionId()); + assertEquals(request.getSession().getId(), event.getNewSessionId()); + assertSame(mockAuthentication, event.getAuthentication()); + assertFalse(event.sessionAttributesWereMigrated()); } @Test @@ -65,4 +114,83 @@ public void sessionIsCreatedIfAlwaysCreateTrue() throws Exception { assertNotNull(request.getSession(false)); } + // See SEC-2002 + @Test + public void onSessionChangePublishesMigrationEventIfMigrateAttributesIsTrue() throws Exception { + SessionFixationProtectionStrategyWithPublicOnSessionChange strategy = + new SessionFixationProtectionStrategyWithPublicOnSessionChange(); + HttpServletRequest request = new MockHttpServletRequest(); + HttpSession session = request.getSession(); + + ApplicationEventPublisher eventPublisher = mock(ApplicationEventPublisher.class); + strategy.setApplicationEventPublisher(eventPublisher); + + Authentication mockAuthentication = mock(Authentication.class); + + strategy.onSessionChange("oldId01", session, mockAuthentication); + + ArgumentCaptor eventArgumentCaptor = ArgumentCaptor.forClass(ApplicationEvent.class); + verify(eventPublisher).publishEvent(eventArgumentCaptor.capture()); + + assertNotNull(eventArgumentCaptor.getValue()); + assertTrue(eventArgumentCaptor.getValue() instanceof SessionFixationProtectionMigrationEvent); + SessionFixationProtectionMigrationEvent event = + (SessionFixationProtectionMigrationEvent)eventArgumentCaptor.getValue(); + assertEquals("oldId01", event.getOldSessionId()); + assertEquals(request.getSession().getId(), event.getNewSessionId()); + assertSame(mockAuthentication, event.getAuthentication()); + assertTrue(event.sessionAttributesWereMigrated()); + } + + // See SEC-2002 + @Test + public void onSessionChangePublishesIdChangeEventIfMigrateAttributesIsFalse() throws Exception { + SessionFixationProtectionStrategyWithPublicOnSessionChange strategy = + new SessionFixationProtectionStrategyWithPublicOnSessionChange(); + strategy.setMigrateSessionAttributes(false); + HttpServletRequest request = new MockHttpServletRequest(); + HttpSession session = request.getSession(); + + ApplicationEventPublisher eventPublisher = mock(ApplicationEventPublisher.class); + strategy.setApplicationEventPublisher(eventPublisher); + + Authentication mockAuthentication = mock(Authentication.class); + + strategy.onSessionChange("oldId02", session, mockAuthentication); + + ArgumentCaptor eventArgumentCaptor = ArgumentCaptor.forClass(ApplicationEvent.class); + verify(eventPublisher).publishEvent(eventArgumentCaptor.capture()); + + assertNotNull(eventArgumentCaptor.getValue()); + assertTrue(eventArgumentCaptor.getValue() instanceof SessionFixationProtectionMigrationEvent); + SessionFixationProtectionMigrationEvent event = + (SessionFixationProtectionMigrationEvent)eventArgumentCaptor.getValue(); + assertEquals("oldId02", event.getOldSessionId()); + assertEquals(request.getSession().getId(), event.getNewSessionId()); + assertSame(mockAuthentication, event.getAuthentication()); + assertFalse(event.sessionAttributesWereMigrated()); + } + + // See SEC-2002 + @Test + public void onSessionChangeDoesNotThrowNullPointerExceptionIfEventPublisherNotSet() throws Exception { + SessionFixationProtectionStrategyWithPublicOnSessionChange strategy = + new SessionFixationProtectionStrategyWithPublicOnSessionChange(); + strategy.setMigrateSessionAttributes(false); + HttpServletRequest request = new MockHttpServletRequest(); + HttpSession session = request.getSession(); + + Authentication mockAuthentication = mock(Authentication.class); + + strategy.onSessionChange("oldId03", session, mockAuthentication); + } + + // See SEC-2002 + private static class SessionFixationProtectionStrategyWithPublicOnSessionChange + extends SessionFixationProtectionStrategy { + @Override + public void onSessionChange(String originalSessionId, HttpSession newSession, Authentication auth) { + super.onSessionChange(originalSessionId, newSession , auth); + } + } }