SEC-1092: SpringContext is null then ConcurrentSessionFilter worked #1343

Closed
spring-issuemaster opened this Issue Feb 1, 2009 · 5 comments

1 participant

@spring-issuemaster

Nikita Koksharov (Migrated from SEC-1092) said:

ConcurrentSessionFilter don’t fill SpringContext then invalidate the session. I think, it would be nice to place HttpSessionContextIntegrationFilter, to fill SpringContext, before ConcurrentSessionFilter.

@spring-issuemaster

Nikita Koksharov said:

Sorry for mistake SecurityContext, not SpringContext.

@spring-issuemaster

Luke Taylor said:

It’s not very clear from your title/description what the problem is.

Could you please explain in detail why you think there is a bug, where it is in the code and if possible supply a test case?

@spring-issuemaster

Nikita Koksharov said:

It’s a bug because of i should have access to SecurityContext when HttpSession destroyed:

public void onApplicationEvent(ApplicationEvent event) { if (!(event instanceof HttpSessionDestroyedEvent)) { return; } SecurityContext context = SecurityContextHolder.getContext(); …. }

but with current version of Spring security i wrote:

private void setupSecurityContext(HttpSession session) { if (SecurityContextHolder.getContext() != null && SecurityContextHolder.getContext().getAuthentication() != null) { return; } SecurityContext context = (SecurityContext) session.getAttribute( HttpSessionContextIntegrationFilter.SPRING_SECURITY_CONTEXT_KEY); if (context != null) { SecurityContextHolder.setContext(context); } } public void onApplicationEvent(ApplicationEvent event) { if (!(event instanceof HttpSessionDestroyedEvent)) { return; } HttpSessionDestroyedEvent httpEvent = (HttpSessionDestroyedEvent)event; setupSecurityContext(httpEvent.getSession()); SecurityContext context = SecurityContextHolder.getContext(); …. }
@spring-issuemaster

Nikita Koksharov said:

ConcurrentSessionFilter drop concurrent session —> HttpSessionDestroyedEvent appear and current SecurityContext is null.

@spring-issuemaster

Luke Taylor said:

So you are essentially saying that you want to access the security context via the thread local rather than having to read it from the session?

If so, that doesn’t seem like a bug to me. The information you need is already available through the session and there isn’t really any reason why an event listener should make assumptions about the context in which it’s called (such as thread local information) – its job is to handle the information passed to it via the event object. The current filter ordering has been established for years now and I don’t think the risk of side effects from changing it is justified here.

The subject of session management within Spring Security should probably be revisited in the future as there are now may areas where it crops up (e.g. session fixation protection) and there may be future changes to concurrent session handling which may affect the filters, but I don’t believe switching the order just to save a few lines of code is worthwhile.

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