Permalink
Browse files

SEC-2057: ConcurrentSessionFilter is now after SecurityContextPersist…

…enceFilter

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.
  • Loading branch information...
1 parent 2c234b9 commit 4f741bc91421f7f2676de21227b33de4440d423c @rwinch rwinch committed Oct 3, 2012
@@ -10,8 +10,8 @@
enum SecurityFilters {
FIRST (Integer.MIN_VALUE),
CHANNEL_FILTER,
- CONCURRENT_SESSION_FILTER,
SECURITY_CONTEXT_FILTER,
+ CONCURRENT_SESSION_FILTER,
LOGOUT_FILTER,
X509_FILTER,
PRE_AUTH_FILTER,
@@ -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() {
@@ -683,16 +683,16 @@ List&lt;OpenIDAttribute> attributes = token.getAttributes();</programlisting>The
<entry><literal>ChannelProcessingFilter</literal></entry>
<entry><literal>http/intercept-url@requires-channel</literal></entry>
</row>
- <row>
- <entry> CONCURRENT_SESSION_FILTER</entry>
- <entry><literal>ConcurrentSessionFilter</literal> </entry>
- <entry><literal>session-management/concurrency-control</literal></entry>
- </row>
<row>
<entry> SECURITY_CONTEXT_FILTER</entry>
<entry><classname>SecurityContextPersistenceFilter</classname></entry>
<entry><literal>http</literal></entry>
</row>
+ <row>
+ <entry> CONCURRENT_SESSION_FILTER</entry>
+ <entry><literal>ConcurrentSessionFilter</literal> </entry>
+ <entry><literal>session-management/concurrency-control</literal></entry>
+ </row>
<row>
<entry> LOGOUT_FILTER </entry>
<entry><literal>LogoutFilter</literal></entry>
@@ -137,12 +137,6 @@
<para><classname>ChannelProcessingFilter</classname>, because it might need to
redirect to a different protocol</para>
</listitem>
- <listitem>
- <para><classname>ConcurrentSessionFilter</classname>, because it doesn't use any
- <classname>SecurityContextHolder</classname> functionality but needs to update
- the <interfacename>SessionRegistry</interfacename> to reflect ongoing requests
- from the principal</para>
- </listitem>
<listitem>
<para><classname>SecurityContextPersistenceFilter</classname>, so a
<interfacename>SecurityContext</interfacename> can be set up in the
@@ -151,6 +145,12 @@
copied to the <literal>HttpSession</literal> when the web request ends (ready
for use with the next web request)</para>
</listitem>
+ <listitem>
+ <para><classname>ConcurrentSessionFilter</classname>, because it uses the
+ <classname>SecurityContextHolder</classname> functionality and needs to update
+ the <interfacename>SessionRegistry</interfacename> to reflect ongoing requests
+ from the principal</para>
+ </listitem>
<listitem>
<para>Authentication processing mechanisms -
<classname>UsernamePasswordAuthenticationFilter</classname>,

0 comments on commit 4f741bc

Please sign in to comment.