From 6650429283df5c424b6f523e25ca174433d3814d Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 15 Sep 2016 14:30:52 -0500 Subject: [PATCH] Polish SessionInformationExpiredStrategy * Fix passivity and add tests * Introduce SessionInformationExpiredEvent as a value object * Rename ExpiredSessionStrategy to SessionInformationExpiredStrategy to account for the need of SessionInformation * Switch to Constructor Injection * Move the changes to the xsd to 4.2 xsd instead of 4.1 Issue gh-3808 --- .../SessionManagementConfigurer.java | 27 ++- .../config/http/HttpConfigurationBuilder.java | 8 +- .../security/config/spring-security-4.1.rnc | 6 - .../security/config/spring-security-4.1.xsd | 14 -- .../security/config/spring-security-4.2.rnc | 6 + .../security/config/spring-security-4.2.xsd | 14 ++ .../NamespaceSessionManagementTests.groovy | 2 +- .../http/SessionManagementConfigTests.groovy | 4 +- .../web/session/ConcurrentSessionFilter.java | 93 ++++++-- .../SessionInformationExpiredEvent.java | 68 ++++++ ...=> SessionInformationExpiredStrategy.java} | 11 +- ...ectSessionInformationExpiredStrategy.java} | 16 +- .../ConcurrentSessionFilterTests.java | 220 +++++++++++++++++- .../SessionInformationExpiredEventTests.java | 48 ++++ 14 files changed, 458 insertions(+), 79 deletions(-) create mode 100644 web/src/main/java/org/springframework/security/web/session/SessionInformationExpiredEvent.java rename web/src/main/java/org/springframework/security/web/session/{ExpiredSessionStrategy.java => SessionInformationExpiredStrategy.java} (80%) rename web/src/main/java/org/springframework/security/web/session/{SimpleRedirectExpiredSessionStrategy.java => SimpleRedirectSessionInformationExpiredStrategy.java} (74%) create mode 100644 web/src/test/java/org/springframework/security/web/session/SessionInformationExpiredEventTests.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurer.java index 4a3e8e4eb5..ef0ecd7fae 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurer.java @@ -48,11 +48,11 @@ import org.springframework.security.web.context.SecurityContextRepository; import org.springframework.security.web.savedrequest.NullRequestCache; import org.springframework.security.web.savedrequest.RequestCache; import org.springframework.security.web.session.ConcurrentSessionFilter; -import org.springframework.security.web.session.ExpiredSessionStrategy; import org.springframework.security.web.session.InvalidSessionStrategy; +import org.springframework.security.web.session.SessionInformationExpiredStrategy; import org.springframework.security.web.session.SessionManagementFilter; -import org.springframework.security.web.session.SimpleRedirectExpiredSessionStrategy; import org.springframework.security.web.session.SimpleRedirectInvalidSessionStrategy; +import org.springframework.security.web.session.SimpleRedirectSessionInformationExpiredStrategy; import org.springframework.util.Assert; /** @@ -99,7 +99,7 @@ public final class SessionManagementConfigurer> private SessionAuthenticationStrategy sessionAuthenticationStrategy; private SessionAuthenticationStrategy providedSessionAuthenticationStrategy; private InvalidSessionStrategy invalidSessionStrategy; - private ExpiredSessionStrategy expiredSessionStrategy; + private SessionInformationExpiredStrategy expiredSessionStrategy; private List sessionAuthenticationStrategies = new ArrayList(); private SessionRegistry sessionRegistry; private Integer maximumSessions; @@ -355,7 +355,7 @@ public final class SessionManagementConfigurer> } public ConcurrencyControlConfigurer expiredSessionStrategy( - ExpiredSessionStrategy expiredSessionStrategy) { + SessionInformationExpiredStrategy expiredSessionStrategy) { SessionManagementConfigurer.this.expiredSessionStrategy = expiredSessionStrategy; return this; } @@ -470,16 +470,23 @@ public final class SessionManagementConfigurer> http.addFilter(sessionManagementFilter); if (isConcurrentSessionControlEnabled()) { - ConcurrentSessionFilter concurrentSessionFilter = new ConcurrentSessionFilter( - getSessionRegistry(http)); - concurrentSessionFilter - .setExpiredSessionStrategy(getExpiredSessionStrategy()); + ConcurrentSessionFilter concurrentSessionFilter = createConccurencyFilter(http); concurrentSessionFilter = postProcess(concurrentSessionFilter); http.addFilter(concurrentSessionFilter); } } + private ConcurrentSessionFilter createConccurencyFilter(H http) { + SessionInformationExpiredStrategy expireStrategy = getExpiredSessionStrategy(); + SessionRegistry sessionRegistry = getSessionRegistry(http); + if(expireStrategy == null) { + return new ConcurrentSessionFilter(sessionRegistry); + } + + return new ConcurrentSessionFilter(sessionRegistry, expireStrategy); + } + /** * Gets the {@link InvalidSessionStrategy} to use. If null and * {@link #invalidSessionUrl} is not null defaults to @@ -505,7 +512,7 @@ public final class SessionManagementConfigurer> return this.invalidSessionStrategy; } - ExpiredSessionStrategy getExpiredSessionStrategy() { + SessionInformationExpiredStrategy getExpiredSessionStrategy() { if (this.expiredSessionStrategy != null) { return this.expiredSessionStrategy; } @@ -515,7 +522,7 @@ public final class SessionManagementConfigurer> } if (this.expiredSessionStrategy == null) { - this.expiredSessionStrategy = new SimpleRedirectExpiredSessionStrategy( + this.expiredSessionStrategy = new SimpleRedirectSessionInformationExpiredStrategy( this.expiredUrl); } return this.expiredSessionStrategy; diff --git a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java index c2e42e205b..f8534a62d5 100644 --- a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java +++ b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java @@ -65,7 +65,7 @@ import org.springframework.security.web.savedrequest.RequestCacheAwareFilter; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.session.ConcurrentSessionFilter; import org.springframework.security.web.session.SessionManagementFilter; -import org.springframework.security.web.session.SimpleRedirectExpiredSessionStrategy; +import org.springframework.security.web.session.SimpleRedirectSessionInformationExpiredStrategy; import org.springframework.security.web.session.SimpleRedirectInvalidSessionStrategy; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.util.ClassUtils; @@ -511,11 +511,11 @@ class HttpConfigurationBuilder { if (StringUtils.hasText(expiryUrl)) { BeanDefinitionBuilder expiredSessionBldr = BeanDefinitionBuilder - .rootBeanDefinition(SimpleRedirectExpiredSessionStrategy.class); + .rootBeanDefinition(SimpleRedirectSessionInformationExpiredStrategy.class); expiredSessionBldr.addConstructorArgValue(expiryUrl); - filterBuilder.addPropertyValue("expiredSessionStrategy", expiredSessionBldr.getBeanDefinition()); + filterBuilder.addConstructorArgValue(expiredSessionBldr.getBeanDefinition()); } else if (StringUtils.hasText(expiredSessionStrategyRef)) { - filterBuilder.addPropertyReference("expiredSessionStrategy", expiredSessionStrategyRef); + filterBuilder.addConstructorArgReference(expiredSessionStrategyRef); } pc.popAndRegisterContainingComponent(); diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-4.1.rnc b/config/src/main/resources/org/springframework/security/config/spring-security-4.1.rnc index 899ac3ff95..5ce746a2eb 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-4.1.rnc +++ b/config/src/main/resources/org/springframework/security/config/spring-security-4.1.rnc @@ -535,9 +535,6 @@ session-management.attlist &= session-management.attlist &= ## The URL to which a user will be redirected if they submit an invalid session indentifier. Typically used to detect session timeouts. attribute invalid-session-url {xsd:token}? -session-management.attlist &= - ## Allows injection of the InvalidSessionStrategy instance used by the SessionManagementFilter - attribute invalid-session-strategy-ref {xsd:token}? session-management.attlist &= ## Allows injection of the SessionAuthenticationStrategy instance used by the SessionManagementFilter attribute session-authentication-strategy-ref {xsd:token}? @@ -556,9 +553,6 @@ concurrency-control.attlist &= concurrency-control.attlist &= ## The URL a user will be redirected to if they attempt to use a session which has been "expired" because they have logged in again. attribute expired-url {xsd:token}? -concurrency-control.attlist &= - ## Allows injection of the ExpiredSessionStrategy instance used by the ConcurrentSessionFilter - attribute expired-session-strategy-ref {xsd:token}? concurrency-control.attlist &= ## Specifies that an unauthorized error should be reported when a user attempts to login when they already have the maximum configured sessions open. The default behaviour is to expire the original session. If the session-authentication-error-url attribute is set on the session-management URL, the user will be redirected to this URL. attribute error-if-maximum-exceeded {xsd:boolean}? diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-4.1.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-4.1.xsd index 5dcee117f1..4175b9599a 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-4.1.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-4.1.xsd @@ -1743,13 +1743,6 @@ - - - Allows injection of the InvalidSessionStrategy instance used by the - SessionManagementFilter - - - Allows injection of the SessionAuthenticationStrategy instance used by the @@ -1784,13 +1777,6 @@ - - - Allows injection of the ExpiredSessionStrategy instance used by the - ConcurrentSessionFilter - - - Specifies that an unauthorized error should be reported when a user attempts to login when diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-4.2.rnc b/config/src/main/resources/org/springframework/security/config/spring-security-4.2.rnc index 8f0b806074..6a0058cab2 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-4.2.rnc +++ b/config/src/main/resources/org/springframework/security/config/spring-security-4.2.rnc @@ -535,6 +535,9 @@ session-management.attlist &= session-management.attlist &= ## The URL to which a user will be redirected if they submit an invalid session indentifier. Typically used to detect session timeouts. attribute invalid-session-url {xsd:token}? +session-management.attlist &= + ## Allows injection of the InvalidSessionStrategy instance used by the SessionManagementFilter + attribute invalid-session-strategy-ref {xsd:token}? session-management.attlist &= ## Allows injection of the SessionAuthenticationStrategy instance used by the SessionManagementFilter attribute session-authentication-strategy-ref {xsd:token}? @@ -553,6 +556,9 @@ concurrency-control.attlist &= concurrency-control.attlist &= ## The URL a user will be redirected to if they attempt to use a session which has been "expired" because they have logged in again. attribute expired-url {xsd:token}? +concurrency-control.attlist &= + ## Allows injection of the SessionInformationExpiredStrategy instance used by the ConcurrentSessionFilter + attribute expired-session-strategy-ref {xsd:token}? concurrency-control.attlist &= ## Specifies that an unauthorized error should be reported when a user attempts to login when they already have the maximum configured sessions open. The default behaviour is to expire the original session. If the session-authentication-error-url attribute is set on the session-management URL, the user will be redirected to this URL. attribute error-if-maximum-exceeded {xsd:boolean}? diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-4.2.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-4.2.xsd index 77ae31861b..47e4713b68 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-4.2.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-4.2.xsd @@ -1743,6 +1743,13 @@ + + + Allows injection of the InvalidSessionStrategy instance used by the + SessionManagementFilter + + + Allows injection of the SessionAuthenticationStrategy instance used by the @@ -1777,6 +1784,13 @@ + + + Allows injection of the SessionInformationExpiredStrategy instance used by the + ConcurrentSessionFilter + + + Specifies that an unauthorized error should be reported when a user attempts to login when diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceSessionManagementTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceSessionManagementTests.groovy index b95db13253..4e9d493a0f 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceSessionManagementTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/web/configurers/NamespaceSessionManagementTests.groovy @@ -67,7 +67,7 @@ class NamespaceSessionManagementTests extends BaseSpringSpec { concurrentStrategy.maximumSessions == 1 concurrentStrategy.exceptionIfMaximumExceeded concurrentStrategy.sessionRegistry == CustomSessionManagementConfig.SR - findFilter(ConcurrentSessionFilter).expiredSessionStrategy.destinationUrl == "/expired-session" + findFilter(ConcurrentSessionFilter).sessionInformationExpiredStrategy.destinationUrl == "/expired-session" } @EnableWebSecurity diff --git a/config/src/test/groovy/org/springframework/security/config/http/SessionManagementConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/SessionManagementConfigTests.groovy index 9b5b00ba14..56be20884a 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/SessionManagementConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/SessionManagementConfigTests.groovy @@ -163,7 +163,7 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { then: concurrentSessionFilter instanceof ConcurrentSessionFilter - concurrentSessionFilter.expiredSessionStrategy.destinationUrl == '/expired' + concurrentSessionFilter.sessionInformationExpiredStrategy.destinationUrl == '/expired' appContext.getBean("sr") != null getFilter(SessionManagementFilter.class) != null sessionRegistryIsValid(); @@ -270,7 +270,7 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { List filters = getFilters("/someurl"); expect: - filters.get(1).expiredSessionStrategy == null + filters.get(1).sessionInformationExpiredStrategy.class.name == 'org.springframework.security.web.session.ConcurrentSessionFilter$ResponseBodySessionInformationExpiredStrategy' } def externalSessionStrategyIsSupported() { diff --git a/web/src/main/java/org/springframework/security/web/session/ConcurrentSessionFilter.java b/web/src/main/java/org/springframework/security/web/session/ConcurrentSessionFilter.java index 298676372b..64d6c98e0f 100644 --- a/web/src/main/java/org/springframework/security/web/session/ConcurrentSessionFilter.java +++ b/web/src/main/java/org/springframework/security/web/session/ConcurrentSessionFilter.java @@ -17,6 +17,7 @@ package org.springframework.security.web.session; import java.io.IOException; + import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.ServletRequest; @@ -29,11 +30,11 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.session.SessionInformation; import org.springframework.security.core.session.SessionRegistry; -import org.springframework.security.web.DefaultRedirectStrategy; import org.springframework.security.web.RedirectStrategy; import org.springframework.security.web.authentication.logout.CompositeLogoutHandler; import org.springframework.security.web.authentication.logout.LogoutHandler; import org.springframework.security.web.authentication.logout.SecurityContextLogoutHandler; +import org.springframework.security.web.util.UrlUtils; import org.springframework.util.Assert; import org.springframework.web.filter.GenericFilterBean; @@ -49,7 +50,7 @@ import org.springframework.web.filter.GenericFilterBean; * as expired. If it has been marked as expired, the configured logout handlers will be * called (as happens with * {@link org.springframework.security.web.authentication.logout.LogoutFilter}), typically - * to invalidate the session. To handle the expired session a call to the {@link ExpiredSessionStrategy} is made. + * to invalidate the session. To handle the expired session a call to the {@link SessionInformationExpiredStrategy} is made. * The session invalidation will cause an * {@link org.springframework.security.web.session.HttpSessionDestroyedEvent} to be * published via the @@ -62,12 +63,15 @@ import org.springframework.web.filter.GenericFilterBean; * @author Marten Deinum */ public class ConcurrentSessionFilter extends GenericFilterBean { + // ~ Instance fields // ================================================================================================ private final SessionRegistry sessionRegistry; + private String expiredUrl; + private RedirectStrategy redirectStrategy; private LogoutHandler handlers = new CompositeLogoutHandler(new SecurityContextLogoutHandler()); - private ExpiredSessionStrategy expiredSessionStrategy; + private SessionInformationExpiredStrategy sessionInformationExpiredStrategy; // ~ Methods // ======================================================================================================== @@ -75,12 +79,42 @@ public class ConcurrentSessionFilter extends GenericFilterBean { public ConcurrentSessionFilter(SessionRegistry sessionRegistry) { Assert.notNull(sessionRegistry, "SessionRegistry required"); this.sessionRegistry = sessionRegistry; + this.sessionInformationExpiredStrategy = new ResponseBodySessionInformationExpiredStrategy(); } + /** + * Creates a new instance + * + * @param sessionRegistry the SessionRegistry to use + * @param expiredUrl the URL to redirect to + * @deprecated use {@link #ConcurrentSessionFilter(SessionRegistry, SessionInformationExpiredStrategy)} with {@link SimpleRedirectSessionInformationExpiredStrategy} instead. + */ + @Deprecated public ConcurrentSessionFilter(SessionRegistry sessionRegistry, String expiredUrl) { Assert.notNull(sessionRegistry, "SessionRegistry required"); + Assert.isTrue(expiredUrl == null || UrlUtils.isValidRedirectUrl(expiredUrl), + expiredUrl + " isn't a valid redirect URL"); + this.expiredUrl = expiredUrl; this.sessionRegistry = sessionRegistry; - this.expiredSessionStrategy = new SimpleRedirectExpiredSessionStrategy(expiredUrl); + this.sessionInformationExpiredStrategy = new SessionInformationExpiredStrategy() { + + @Override + public void onExpiredSessionDetected(SessionInformationExpiredEvent event) throws IOException, ServletException { + HttpServletRequest request = event.getRequest(); + HttpServletResponse response = event.getResponse(); + SessionInformation info = event.getSessionInformation(); + + redirectStrategy.sendRedirect(request, response, determineExpiredUrl(request, info)); + } + + }; + } + + public ConcurrentSessionFilter(SessionRegistry sessionRegistry, SessionInformationExpiredStrategy sessionInformationExpiredStrategy) { + Assert.notNull(sessionRegistry, "sessionRegistry required"); + Assert.notNull(sessionInformationExpiredStrategy, "sessionInformationExpiredStrategy cannot be null"); + this.sessionRegistry = sessionRegistry; + this.sessionInformationExpiredStrategy = sessionInformationExpiredStrategy; } @Override @@ -108,18 +142,7 @@ public class ConcurrentSessionFilter extends GenericFilterBean { } doLogout(request, response); - if (this.expiredSessionStrategy != null) { - this.expiredSessionStrategy.onExpiredSessionDetected(request, response); - - return; - } - else { - response.getWriter().print( - "This session has been expired (possibly due to multiple concurrent " - + "logins being attempted as the same user)."); - response.flushBuffer(); - } - + this.sessionInformationExpiredStrategy.onExpiredSessionDetected(new SessionInformationExpiredEvent(info, request, response)); return; } else { @@ -132,6 +155,17 @@ public class ConcurrentSessionFilter extends GenericFilterBean { chain.doFilter(request, response); } + /** + * Determine the URL for expiration + * @param request the HttpServletRequest + * @param info the {@link SessionInformation} + * @return the URL for expiration + * @deprecated Use {@link #ConcurrentSessionFilter(SessionRegistry, SessionInformationExpiredStrategy)} instead. + */ + protected String determineExpiredUrl(HttpServletRequest request, + SessionInformation info) { + return expiredUrl; + } private void doLogout(HttpServletRequest request, HttpServletResponse response) { Authentication auth = SecurityContextHolder.getContext().getAuthentication(); @@ -143,7 +177,30 @@ public class ConcurrentSessionFilter extends GenericFilterBean { this.handlers = new CompositeLogoutHandler(handlers); } - public void setExpiredSessionStrategy(ExpiredSessionStrategy expiredSessionStrategy) { - this.expiredSessionStrategy=expiredSessionStrategy; + /** + * Sets the {@link RedirectStrategy} used with {@link #ConcurrentSessionFilter(SessionRegistry, String)} + * @param redirectStrategy the {@link RedirectStrategy} to use + * @deprecated use {@link #ConcurrentSessionFilter(SessionRegistry, SessionInformationExpiredStrategy)} instead. + */ + public void setRedirectStrategy(RedirectStrategy redirectStrategy) { + this.redirectStrategy = redirectStrategy; + } + + /** + * A {@link SessionInformationExpiredStrategy} that writes an error message to the response body. + * @author Rob Winch + * @since 4.2 + */ + private static final class ResponseBodySessionInformationExpiredStrategy + implements SessionInformationExpiredStrategy { + @Override + public void onExpiredSessionDetected(SessionInformationExpiredEvent event) + throws IOException, ServletException { + HttpServletResponse response = event.getResponse(); + response.getWriter().print( + "This session has been expired (possibly due to multiple concurrent " + + "logins being attempted as the same user)."); + response.flushBuffer(); + } } } diff --git a/web/src/main/java/org/springframework/security/web/session/SessionInformationExpiredEvent.java b/web/src/main/java/org/springframework/security/web/session/SessionInformationExpiredEvent.java new file mode 100644 index 0000000000..033a213dd3 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/session/SessionInformationExpiredEvent.java @@ -0,0 +1,68 @@ +/* + * Copyright 2012-2016 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.session; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.springframework.context.ApplicationEvent; +import org.springframework.security.core.session.SessionInformation; +import org.springframework.util.Assert; + +/** + * An event for when a {@link SessionInformation} is expired. + * @author Rob Winch + * @since 4.2 + */ +public final class SessionInformationExpiredEvent extends ApplicationEvent { + + private HttpServletRequest request; + private HttpServletResponse response; + + /** + * Creates a new instance + * + * @param sessionInformation the SessionInformation that is expired + * @param request the HttpServletRequest + * @param response the HttpServletResponse + */ + public SessionInformationExpiredEvent(SessionInformation sessionInformation, HttpServletRequest request, HttpServletResponse response) { + super(sessionInformation); + Assert.notNull(request, "request cannot be null"); + Assert.notNull(response, "response cannot be null"); + this.request = request; + this.response = response; + } + + /** + * @return the request + */ + public HttpServletRequest getRequest() { + return request; + } + + /** + * @return the response + */ + public HttpServletResponse getResponse() { + return response; + } + + public SessionInformation getSessionInformation() { + return (SessionInformation) getSource(); + } +} diff --git a/web/src/main/java/org/springframework/security/web/session/ExpiredSessionStrategy.java b/web/src/main/java/org/springframework/security/web/session/SessionInformationExpiredStrategy.java similarity index 80% rename from web/src/main/java/org/springframework/security/web/session/ExpiredSessionStrategy.java rename to web/src/main/java/org/springframework/security/web/session/SessionInformationExpiredStrategy.java index 0c8908cfbc..af863949ab 100644 --- a/web/src/main/java/org/springframework/security/web/session/ExpiredSessionStrategy.java +++ b/web/src/main/java/org/springframework/security/web/session/SessionInformationExpiredStrategy.java @@ -16,20 +16,19 @@ package org.springframework.security.web.session; import java.io.IOException; + import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; /** * Determines the behaviour of the {@code ConcurrentSessionFilter} when an expired session * is detected in the {@code ConcurrentSessionFilter}. * * @author Marten Deinum - * @since 4.1.0 + * @author Rob Winch + * @since 4.2.0 */ -public interface ExpiredSessionStrategy { +public interface SessionInformationExpiredStrategy { - void onExpiredSessionDetected(HttpServletRequest request, HttpServletResponse response) + void onExpiredSessionDetected(SessionInformationExpiredEvent eventØ) throws IOException, ServletException; - } diff --git a/web/src/main/java/org/springframework/security/web/session/SimpleRedirectExpiredSessionStrategy.java b/web/src/main/java/org/springframework/security/web/session/SimpleRedirectSessionInformationExpiredStrategy.java similarity index 74% rename from web/src/main/java/org/springframework/security/web/session/SimpleRedirectExpiredSessionStrategy.java rename to web/src/main/java/org/springframework/security/web/session/SimpleRedirectSessionInformationExpiredStrategy.java index 31a68f59cc..e4b9a51f55 100644 --- a/web/src/main/java/org/springframework/security/web/session/SimpleRedirectExpiredSessionStrategy.java +++ b/web/src/main/java/org/springframework/security/web/session/SimpleRedirectSessionInformationExpiredStrategy.java @@ -16,11 +16,10 @@ package org.springframework.security.web.session; import java.io.IOException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.security.web.DefaultRedirectStrategy; import org.springframework.security.web.RedirectStrategy; import org.springframework.security.web.util.UrlUtils; @@ -31,28 +30,27 @@ import org.springframework.util.Assert; * {@code ConcurrentSessionFilter}. * * @author Marten Deinum - * @since 4.1.0 + * @since 4.2.0 */ -public final class SimpleRedirectExpiredSessionStrategy implements ExpiredSessionStrategy { +public final class SimpleRedirectSessionInformationExpiredStrategy implements SessionInformationExpiredStrategy { private final Log logger = LogFactory.getLog(getClass()); private final String destinationUrl; private final RedirectStrategy redirectStrategy; - public SimpleRedirectExpiredSessionStrategy(String invalidSessionUrl) { + public SimpleRedirectSessionInformationExpiredStrategy(String invalidSessionUrl) { this(invalidSessionUrl, new DefaultRedirectStrategy()); } - public SimpleRedirectExpiredSessionStrategy(String invalidSessionUrl, RedirectStrategy redirectStrategy) { + public SimpleRedirectSessionInformationExpiredStrategy(String invalidSessionUrl, RedirectStrategy redirectStrategy) { Assert.isTrue(UrlUtils.isValidRedirectUrl(invalidSessionUrl), "url must start with '/' or with 'http(s)'"); this.destinationUrl=invalidSessionUrl; this.redirectStrategy=redirectStrategy; } - public void onExpiredSessionDetected(HttpServletRequest request, - HttpServletResponse response) throws IOException { + public void onExpiredSessionDetected(SessionInformationExpiredEvent event) throws IOException { logger.debug("Redirecting to '" + destinationUrl + "'"); - redirectStrategy.sendRedirect(request, response, destinationUrl); + redirectStrategy.sendRedirect(event.getRequest(), event.getResponse(), destinationUrl); } } diff --git a/web/src/test/java/org/springframework/security/web/concurrent/ConcurrentSessionFilterTests.java b/web/src/test/java/org/springframework/security/web/concurrent/ConcurrentSessionFilterTests.java index 0dd2530397..43789b8713 100644 --- a/web/src/test/java/org/springframework/security/web/concurrent/ConcurrentSessionFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/concurrent/ConcurrentSessionFilterTests.java @@ -17,21 +17,37 @@ package org.springframework.security.web.concurrent; import java.util.Date; -import javax.servlet.FilterChain; +import javax.servlet.FilterChain; +import javax.servlet.http.HttpServletRequest; + +import org.junit.After; import org.junit.Test; + +import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockHttpSession; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.session.SessionInformation; import org.springframework.security.core.session.SessionRegistry; import org.springframework.security.core.session.SessionRegistryImpl; +import org.springframework.security.web.RedirectStrategy; import org.springframework.security.web.authentication.logout.LogoutHandler; import org.springframework.security.web.authentication.logout.SecurityContextLogoutHandler; import org.springframework.security.web.session.ConcurrentSessionFilter; -import org.springframework.security.web.session.SimpleRedirectExpiredSessionStrategy; +import org.springframework.security.web.session.SessionInformationExpiredStrategy; +import org.springframework.security.web.session.SimpleRedirectSessionInformationExpiredStrategy; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.*; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; /** * Tests {@link ConcurrentSessionFilter}. @@ -41,6 +57,33 @@ import static org.mockito.Mockito.*; */ public class ConcurrentSessionFilterTests { + @After + public void cleanup() { + SecurityContextHolder.clearContext(); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorSessionRegistryWhenSessionRegistryNullThenExceptionThrown() { + new ConcurrentSessionFilter(null); + } + + @SuppressWarnings("deprecation") + @Test(expected = IllegalArgumentException.class) + public void constructorSessionRegistryExpiresUrlWhenInvalidUrlThenExceptionThrown() { + new ConcurrentSessionFilter(new SessionRegistryImpl(), "oops"); + } + + @SuppressWarnings("deprecation") + @Test(expected = IllegalArgumentException.class) + public void constructorSessionRegistryExpiresUrlWhenSessionRegistryNullThenExceptionThrown() { + new ConcurrentSessionFilter(null, "/expired"); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorSessionRegistrySessionInformationExpiredStrategyWhenStrategyIsNullThenThrowsException() { + new ConcurrentSessionFilter(new SessionRegistryImpl(), (SessionInformationExpiredStrategy) null); + } + @Test public void detectsExpiredSessions() throws Exception { // Setup our HTTP request @@ -56,9 +99,8 @@ public class ConcurrentSessionFilterTests { // Setup our test fixture and registry to want this session to be expired - SimpleRedirectExpiredSessionStrategy expiredSessionStrategy = new SimpleRedirectExpiredSessionStrategy("/expired.jsp"); - ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry); - filter.setExpiredSessionStrategy(expiredSessionStrategy); + SimpleRedirectSessionInformationExpiredStrategy expiredSessionStrategy = new SimpleRedirectSessionInformationExpiredStrategy("/expired.jsp"); + ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry, expiredSessionStrategy); filter.setLogoutHandlers(new LogoutHandler[] { new SecurityContextLogoutHandler() }); filter.afterPropertiesSet(); @@ -110,9 +152,8 @@ public class ConcurrentSessionFilterTests { // Setup our test fixture SessionRegistry registry = new SessionRegistryImpl(); registry.registerNewSession(session.getId(), "principal"); - SimpleRedirectExpiredSessionStrategy expiredSessionStrategy = new SimpleRedirectExpiredSessionStrategy("/expired.jsp"); - ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry); - filter.setExpiredSessionStrategy(expiredSessionStrategy); + SimpleRedirectSessionInformationExpiredStrategy expiredSessionStrategy = new SimpleRedirectSessionInformationExpiredStrategy("/expired.jsp"); + ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry, expiredSessionStrategy); Date lastRequest = registry.getSessionInformation(session.getId()).getLastRequest(); @@ -123,4 +164,165 @@ public class ConcurrentSessionFilterTests { verify(fc).doFilter(request, response); assertThat(registry.getSessionInformation(session.getId()).getLastRequest().after(lastRequest)).isTrue(); } + + @Test + public void doFilterWhenNoSessionThenChainIsContinued() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletResponse response = new MockHttpServletResponse(); + + RedirectStrategy redirect = mock(RedirectStrategy.class); + SessionRegistry registry = mock(SessionRegistry.class); + SessionInformation information = new SessionInformation("user", "sessionId", new Date(System.currentTimeMillis() - 1000)); + information.expireNow(); + when(registry.getSessionInformation(anyString())).thenReturn(information); + + + String expiredUrl = "/expired"; + ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry, + expiredUrl); + filter.setRedirectStrategy(redirect); + + MockFilterChain chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + assertThat(chain.getRequest()).isNotNull(); + } + + @Test + public void doFilterWhenNoSessionInformationThenChainIsContinued() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setSession(new MockHttpSession()); + MockHttpServletResponse response = new MockHttpServletResponse(); + + RedirectStrategy redirect = mock(RedirectStrategy.class); + SessionRegistry registry = mock(SessionRegistry.class); + + + String expiredUrl = "/expired"; + ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry, + expiredUrl); + filter.setRedirectStrategy(redirect); + + MockFilterChain chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + assertThat(chain.getRequest()).isNotNull(); + } + + @Test + public void doFilterWhenCustomRedirectStrategyThenCustomRedirectStrategyUsed() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpSession session = new MockHttpSession(); + request.setSession(session); + MockHttpServletResponse response = new MockHttpServletResponse(); + + RedirectStrategy redirect = mock(RedirectStrategy.class); + SessionRegistry registry = mock(SessionRegistry.class); + SessionInformation information = new SessionInformation("user", "sessionId", new Date(System.currentTimeMillis() - 1000)); + information.expireNow(); + when(registry.getSessionInformation(anyString())).thenReturn(information); + + + String expiredUrl = "/expired"; + ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry, + expiredUrl); + filter.setRedirectStrategy(redirect); + + filter.doFilter(request, response, new MockFilterChain()); + + verify(redirect).sendRedirect(request, response, expiredUrl); + } + + @Test + public void doFilterWhenOverrideThenCustomRedirectStrategyUsed() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpSession session = new MockHttpSession(); + request.setSession(session); + MockHttpServletResponse response = new MockHttpServletResponse(); + + RedirectStrategy redirect = mock(RedirectStrategy.class); + SessionRegistry registry = mock(SessionRegistry.class); + SessionInformation information = new SessionInformation("user", "sessionId", new Date(System.currentTimeMillis() - 1000)); + information.expireNow(); + when(registry.getSessionInformation(anyString())).thenReturn(information); + + + final String expiredUrl = "/expired"; + ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry, + expiredUrl + "will-be-overrridden") { + /* (non-Javadoc) + * @see org.springframework.security.web.session.ConcurrentSessionFilter#determineExpiredUrl(javax.servlet.http.HttpServletRequest, org.springframework.security.core.session.SessionInformation) + */ + @Override + protected String determineExpiredUrl(HttpServletRequest request, + SessionInformation info) { + return expiredUrl; + } + + }; + filter.setRedirectStrategy(redirect); + + filter.doFilter(request, response, new MockFilterChain()); + + verify(redirect).sendRedirect(request, response, expiredUrl); + } + + @Test + public void doFilterWhenNoExpiredUrlThenResponseWritten() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpSession session = new MockHttpSession(); + request.setSession(session); + MockHttpServletResponse response = new MockHttpServletResponse(); + + SessionRegistry registry = mock(SessionRegistry.class); + SessionInformation information = new SessionInformation("user", "sessionId", new Date(System.currentTimeMillis() - 1000)); + information.expireNow(); + when(registry.getSessionInformation(anyString())).thenReturn(information); + + + ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry); + + filter.doFilter(request, response, new MockFilterChain()); + + assertThat(response.getContentAsString()).contains( + "This session has been expired (possibly due to multiple concurrent logins being attempted as the same user)."); + } + + @Test + public void doFilterWhenCustomLogoutHandlersThenHandlersUsed() throws Exception { + LogoutHandler handler = mock(LogoutHandler.class); + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpSession session = new MockHttpSession(); + request.setSession(session); + MockHttpServletResponse response = new MockHttpServletResponse(); + + SessionRegistry registry = mock(SessionRegistry.class); + SessionInformation information = new SessionInformation("user", "sessionId", new Date(System.currentTimeMillis() - 1000)); + information.expireNow(); + when(registry.getSessionInformation(anyString())).thenReturn(information); + + + ConcurrentSessionFilter filter = new ConcurrentSessionFilter(registry); + filter.setLogoutHandlers(new LogoutHandler[] { handler } ); + + filter.doFilter(request, response, new MockFilterChain()); + + verify(handler).logout(eq(request), eq(response), any(Authentication.class)); + } + + @Test(expected = IllegalArgumentException.class) + public void setLogoutHandlersWhenNullThenThrowsException() { + ConcurrentSessionFilter filter = new ConcurrentSessionFilter(new SessionRegistryImpl()); + + filter.setLogoutHandlers(null); + } + + @Test(expected = IllegalArgumentException.class) + public void setLogoutHandlersWhenEmptyThenThrowsException() { + ConcurrentSessionFilter filter = new ConcurrentSessionFilter(new SessionRegistryImpl()); + + filter.setLogoutHandlers(new LogoutHandler[0]); + } } diff --git a/web/src/test/java/org/springframework/security/web/session/SessionInformationExpiredEventTests.java b/web/src/test/java/org/springframework/security/web/session/SessionInformationExpiredEventTests.java new file mode 100644 index 0000000000..da81781042 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/session/SessionInformationExpiredEventTests.java @@ -0,0 +1,48 @@ +/* + * Copyright 2012-2016 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.session; + +import java.util.Date; + +import org.junit.Test; + +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.core.session.SessionInformation; + +/** + * @author Rob Winch + * @since 4.2 + */ +public class SessionInformationExpiredEventTests { + + @Test(expected = IllegalArgumentException.class) + public void constructorWhenSessionInformationNullThenThrowsException() { + new SessionInformationExpiredEvent(null, new MockHttpServletRequest(), new MockHttpServletResponse()); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorWhenRequestNullThenThrowsException() { + new SessionInformationExpiredEvent(new SessionInformation("fake", "sessionId", new Date()), null, new MockHttpServletResponse()); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorWhenResponseNullThenThrowsException() { + new SessionInformationExpiredEvent(new SessionInformation("fake", "sessionId", new Date()), new MockHttpServletRequest(), null); + } + +}