From 4f741bc91421f7f2676de21227b33de4440d423c Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Wed, 3 Oct 2012 09:20:31 -0500 Subject: [PATCH] SEC-2057: ConcurrentSessionFilter is now after SecurityContextPersistenceFilter Previously, ConcurrentSessionFilter was placed after SecurityContextPersistenceFilter which meant that the SecurityContextHolder was empty when ConcurrentSessionFilter was invoked. This caused the Authentication to be null when performing a logout. It also caused complications with LogoutHandler implementations that would be accessing the SecurityContextHolder and potentially clear it out expecting that SecurityContextPersistenceFilter would then clear the SecurityContextRepository. The ConcurrentSessionFilter is now positioned after the SecurityContextPersistenceFilter to ensure that the SecurityContextHolder is populated and cleared out appropriately. --- .../security/config/http/SecurityFilters.java | 2 +- .../http/SessionManagementConfigTests.groovy | 24 +++++++++++++++---- docs/manual/src/docbook/namespace-config.xml | 10 ++++---- .../src/docbook/security-filter-chain.xml | 12 +++++----- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/http/SecurityFilters.java b/config/src/main/java/org/springframework/security/config/http/SecurityFilters.java index 0e7568bb9d..f65c20f0fe 100644 --- a/config/src/main/java/org/springframework/security/config/http/SecurityFilters.java +++ b/config/src/main/java/org/springframework/security/config/http/SecurityFilters.java @@ -10,8 +10,8 @@ package org.springframework.security.config.http; enum SecurityFilters { FIRST (Integer.MIN_VALUE), CHANNEL_FILTER, - CONCURRENT_SESSION_FILTER, SECURITY_CONTEXT_FILTER, + CONCURRENT_SESSION_FILTER, LOGOUT_FILTER, X509_FILTER, PRE_AUTH_FILTER, 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 16db3d69f6..09c35ef96d 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 @@ -106,7 +106,7 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { } createAppContext(); List filters = getFilters("/someurl"); - def concurrentSessionFilter = filters.get(0) + def concurrentSessionFilter = filters.get(1) then: concurrentSessionFilter instanceof ConcurrentSessionFilter @@ -134,7 +134,7 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { createAppContext(); List filters = getFilters("/someurl") - ConcurrentSessionFilter concurrentSessionFilter = filters.get(0) + ConcurrentSessionFilter concurrentSessionFilter = filters.get(1) def logoutHandlers = concurrentSessionFilter.handlers then: 'ConcurrentSessionFilter contains the customized LogoutHandlers' @@ -159,7 +159,7 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { createAppContext() List filters = getFilters("/someurl") - ConcurrentSessionFilter concurrentSessionFilter = filters.get(0) + ConcurrentSessionFilter concurrentSessionFilter = filters.get(1) def logoutHandlers = concurrentSessionFilter.handlers then: 'SecurityContextLogoutHandler and RememberMeServices are in ConcurrentSessionFilter logoutHandlers' @@ -181,7 +181,7 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { createAppContext() List filters = getFilters("/someurl") - ConcurrentSessionFilter concurrentSessionFilter = filters.get(0) + ConcurrentSessionFilter concurrentSessionFilter = filters.get(1) def logoutHandlers = concurrentSessionFilter.handlers then: 'Only SecurityContextLogoutHandler is found in ConcurrentSessionFilter logoutHandlers' @@ -191,6 +191,20 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { securityCtxlogoutHandler.invalidateHttpSession == true } + def 'SEC-2057: ConcurrentSessionFilter is after SecurityContextPersistenceFilter'() { + httpAutoConfig { + 'session-management'() { + 'concurrency-control'() + } + } + createAppContext() + List filters = getFilters("/someurl") + + expect: + filters.get(0) instanceof SecurityContextPersistenceFilter + filters.get(1) instanceof ConcurrentSessionFilter + } + def 'concurrency-control handles default expired-url as null'() { httpAutoConfig { 'session-management'() { @@ -201,7 +215,7 @@ class SessionManagementConfigTests extends AbstractHttpConfigTests { List filters = getFilters("/someurl"); expect: - filters.get(0).expiredUrl == null + filters.get(1).expiredUrl == null } def externalSessionStrategyIsSupported() { diff --git a/docs/manual/src/docbook/namespace-config.xml b/docs/manual/src/docbook/namespace-config.xml index 8ed77c0b62..0759c12b8d 100644 --- a/docs/manual/src/docbook/namespace-config.xml +++ b/docs/manual/src/docbook/namespace-config.xml @@ -683,16 +683,16 @@ List<OpenIDAttribute> attributes = token.getAttributes();The ChannelProcessingFilter http/intercept-url@requires-channel - - CONCURRENT_SESSION_FILTER - ConcurrentSessionFilter - session-management/concurrency-control - SECURITY_CONTEXT_FILTER SecurityContextPersistenceFilter http + + CONCURRENT_SESSION_FILTER + ConcurrentSessionFilter + session-management/concurrency-control + LOGOUT_FILTER LogoutFilter diff --git a/docs/manual/src/docbook/security-filter-chain.xml b/docs/manual/src/docbook/security-filter-chain.xml index ceed5883f7..5424047248 100644 --- a/docs/manual/src/docbook/security-filter-chain.xml +++ b/docs/manual/src/docbook/security-filter-chain.xml @@ -137,12 +137,6 @@ ChannelProcessingFilter, because it might need to redirect to a different protocol - - ConcurrentSessionFilter, because it doesn't use any - SecurityContextHolder functionality but needs to update - the SessionRegistry to reflect ongoing requests - from the principal - SecurityContextPersistenceFilter, so a SecurityContext can be set up in the @@ -151,6 +145,12 @@ copied to the HttpSession when the web request ends (ready for use with the next web request) + + ConcurrentSessionFilter, because it uses the + SecurityContextHolder functionality and needs to update + the SessionRegistry to reflect ongoing requests + from the principal + Authentication processing mechanisms - UsernamePasswordAuthenticationFilter,