SEC-1587: HttpSessionSecurityContextRepository should clear session context when current context is anonmous or empty #1826

Closed
spring-issuemaster opened this Issue Oct 7, 2010 · 9 comments

1 participant

@spring-issuemaster

Janning Vygen (Migrated from SEC-1587) said:

I have written a LogoutFilter which does not redirect to a logout page. It just logs out the user and goes on with the filter chain.
(it's needed because we have different areas and the user should be logged out when switching to another "area")

Before it comes to saving the security context with HttpSessionSecurityContextRepository the anonymousFilter sets an AnonymousAuthentication. So when it comes to saving the context, it is not saved.

This is because of this code in HttpSessionSecurityContextRepository

protected void saveContext(SecurityContext context) {
// See SEC-776
if (authenticationTrustResolver.isAnonymous(context.getAuthentication())) {
if (logger.isDebugEnabled()) {
logger.debug("SecurityContext contents are anonymous - context will not be stored in HttpSession. ");
}
return;
}

BTW my LogoutFilter is configured with invalidateHttpSession=false otherwise this bug wouldn't occur i guess.

So imho, there should be another check to see what is actually saved in the session.

regards
Janning

@spring-issuemaster

Luke Taylor said:

I don't really see what the bug is. It is intended that anonymous authentication tokens aren't store in the session.

@spring-issuemaster

Janning Vygen said:

hmm, it's quite clear to me ;-) i'll try it again, if you still don't see the bug, i will try to create a test project.

  • we have a userAuthentication saved in the session.
  • user clicks "logout"
  • first HttpSessionSecurityContextRepository loads the userAuthentication from the session into the SecurityContext
  • logoutfilter clears the securityContext in memory, but not in the session because of invalidateHttpSession=false
  • logoutfilter does not "return", but calls filterChain.doFilter()
  • anonymousfilter sets anonymousAuthentication into SecurityContext
  • HttpSessionSecurityContextRepository does not save the current context to the session because its anonymous
  • userAuthentication is still saved in the session

on the next page HttpSessionSecurityContextRepository loads the userAuthentication from the session. user is still logged in.

I think this bug only occurs if your logoutfilter does not call "return" but filter.doChain(). Maybe this is a bug in my LogoutFilter implementation.
But i think HttpSessionSecurityContextRepository should do an additional check to prevent situation like the one described above.

@spring-issuemaster

Luke Taylor said:

Ok, I see what you mean now. The problem is that the original context is retained in the session when it should be removed. I'll take a look at how that can be accomodated. As a workaround you can probably just directly remove the context attribute from the session in your LogoutFilter (rather than invalidating the session as most users would require for a logout).

@spring-issuemaster

Luke Taylor said:

Hmm. Looking at this again, I'm still not quite sure why it would happen (unless you are using clustering). The SecurityContext in the ThreadLocal should be the same instance as the one in the session (since that's where it's loaded from), so when it is overwritten, the one in the session should be also. It probably still makes sense to add a fix with an explicit removeAttribute() call when clustering is being used, but it would be useful if you could check again what is actually happening in your case.

@spring-issuemaster

Luke Taylor said:

I've added appropriate checks in the context saving logic to explicitly call session.removeAttribute() when the security context is empty or anonymous.

@spring-issuemaster

Janning Vygen said:

From your previous post, i was just about starting to look at this again. But i think you already fixed it, right? If not, please send me a mail.

@spring-issuemaster

Luke Taylor said:

Yeah, I've added the fix, but I'm still not sure why it would be needed in a single-VM situation.

@spring-issuemaster

Janning Vygen said:

I have attached a maven test project. i hope it helps. if not please ask.

the main reason for this to happen is:

  • LogoutFilter does not return after logout but continues with doChain
  • LogoutFilter does not invalidate session

please take a loolk at the source code. project is very small.
tets fails with 3.0.3, it succeeds with 3.0.5

@spring-issuemaster

Janning Vygen said:

Maven test project

@spring-issuemaster spring-issuemaster added this to the 3.1.0.M2 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment