12

We are using Spring Security in our web application. Most of the pages are secured, i.e. a user must be logged in to access these pages. It works fine usually. However, we encounter an unwanted behavior during logout.

Suppose that a user is logged in and sends a request to the sever to load some (secured) page. Before this request is completed, the same user sends a logout request (i.e. request with servlet_path "/j_spring_security_logout"). The logout request is usually very fast and it can be completed earlier than the former request. Of course, the logout request clears the security context. Hence, the former request loses the security context in the middle of its life and this usually cause an exception.

In fact, the user need not start the first request "manually". This scenario can happen on a page with automatic refresh, i.e. the user presses the logout link just a fraction of second after a refreshing has been sent automatically.

From one point of view, this can be considered to be a meaningful behavior. On the other hand, I would prefer to prevent such loss of security context in the middle of the life of a request.

Is there a way to configure Spring Security to avoid this? (something like "postpone clearing of security context when there are other concurrent requests from the same session" or "read the security context just once during a single request and cache it for further use")

Thanks.

SkyWalker
  • 28,384
  • 14
  • 74
  • 132
Cimlman
  • 3,404
  • 5
  • 25
  • 35
  • I'm pretty sure I'm seeing the same behaviour in my app as well. Unfortunately, everything ive tried so far hasn't fixed it (`@AuthenticationPrincipal` method param, `Principal` method param, `SecurityContextHolder.getContext().getAuthentication()` are `null` once a concurrent logout has completed) – jlb Oct 21 '16 at 00:11
  • Does clicking on logout button redirects to your logout page in normal scenario? – gschambial Oct 21 '16 at 00:25

4 Answers4

15

So this is all (unsurprisingly) by design. These spring security docs give a good explanation as to what's happening - quoting:

In an application which receives concurrent requests in a single session, the same SecurityContext instance will be shared between threads. Even though a ThreadLocal is being used, it is the same instance that is retrieved from the HttpSession for each thread. This has implications if you wish to temporarily change the context under which a thread is running. If you just use SecurityContextHolder.getContext(), and call setAuthentication(anAuthentication) on the returned context object, then the Authentication object will change in all concurrent threads which share the same SecurityContext instance. You can customize the behaviour of SecurityContextPersistenceFilter to create a completely new SecurityContext for each request, preventing changes in one thread from affecting another. Alternatively you can create a new instance just at the point where you temporarily change the context. The method SecurityContextHolder.createEmptyContext() always returns a new context instance.

A snippet of the above quote says:

... you can customise the behaviour of SpringContextPersistenceFilter ...

Unfortunately, the docs do not provide any information as to how to go about doing this, or how it would even be approached. This SO question asks the very thing (essentially, is a distilled version of this question), but it has not received much attention.

There's also this SO answer that provides a bit more depth into the inner workings of HttpSessionSecurityContextRepository, which is likely to be the piece that would need to be re-written/updated in order to tackle this issue.

I'll update this answer if I come by a good way of addressing this (such as creating a new instance of the context) in implementation.

Update

The root of the problem I'm experiencing was related to reading a user id attribute out of the HttpSession (after it had been cleared by a concurrent "logout" request). Instead of implementing my own SpringContextRepository I decided to create a simple filter that saves the current Authentication to the request, and then work from there.

Here's the basic filter:

public class SaveAuthToRequestFilter extends OncePerRequestFilter {

    public static final String REQUEST_ATTR = SaveAuthToRequestFilter.class.getCanonicalName();

    @Override
    protected void doFilterInternal(final HttpServletRequest request, final HttpServletResponse response, final FilterChain filterChain)
            throws ServletException, IOException {

        final SecurityContext context = SecurityContextHolder.getContext();
        if (context != null) {
            request.setAttribute(REQUEST_ATTR, context.getAuthentication());
        }

        filterChain.doFilter(request, response);
    }

}

Which has to be added after the SecurityContextPersistenceFilter by adding the following to your WebSecurityConfigurerAdapter's configure(final HttpSecurity http) method.

http.addFilterAfter(new SaveAuthToRequestFilter(), SecurityContextPersistenceFilter.class)

After you have that in place, you can read the 'current' (per thread/request) Authentication from the HttpServletRequest (@Autowired or injected into your controller methods) and work from there. I think this solution still leaves something to be desired, but it's the most lightweight option I could think of. Thanks to @chimmi and @sura2k for the inspiration.

Community
  • 1
  • 1
jlb
  • 19,090
  • 8
  • 34
  • 65
3

Disclaimer: behaviour in question was implemented purposefully. And if anyone decides to disable it they should read SEC-2025 that describes the problem you get in return.


If I understand this correctly the problem is that logout clears SecurityContext's authentication data thus making

SecurityContextHolder.getContext().getAuthentication()

return null. But logout does not clear context itself, if your first request managed to grab it before logout happened it stays in ThreadLocal for first request.

So all we need is not to clear authentication, and it turns out (because Spring is awesome) that SecurityContextLogoutHandler that is responsible of this, has a property:

private boolean clearAuthentication = true;

that does exactly what we need, Javadoc:

If true, removes the Authentication from the SecurityContext to prevent issues with concurrent requests.

chimmi
  • 2,054
  • 16
  • 30
  • 1
    I think the issue in `SEC-2025` should not be taken lightly. See the fix commit which succinctly exemplifies the problem: https://github.com/spring-projects/spring-security/commit/d3339a1e32ae356fc108a1bc3ee890706a3a119a – jlb Oct 27 '16 at 23:14
3

Your requirement was reported as a bug (not a feature request) in JIRA SEC-2025. And They have fixed it in Spring 3.2, so which you expect here to happen/solve implicitly prevents by its design.

Default behavior is to save the SecurityContext in the HttpSession and that is the only implementation afaik spring security provides at the moment.

Even the SecurityContext is ThreadLocal it shares the same which is in the HttpSession. So when SecurityContext gets cleaned, it will remove from the HttpSession, hence will be unavailable from the entire user session.

What you need to do is, store the SecurityContext additionally in HttpServletRequest (or something bound to the HTTP request) other than the HttpSession and read it back from HttpSession and if not found, read it from HttpServletRequest. Make sure to save a deep-copy of SecurityContext in the HttpServletRequest. When you logs out, clean the SecurityContext only from HttpSession which is currently happening. In this case whatever the running threads (bound to HTTP requests) will have access to the SecurityContext via HttpServletRequest (if it not found in HttpSession - which is happening to you right now) even if the user has logged out. Next new HTTP requests will need an authentication because new requests has no SecurityContext in the HttpSession( or HttpServletRequest).

Keep a new copy of SecurityContext in each HttpServletRequest may be an overhead only to address a corner case.

To do that you need to read and understand following spring implementations.

  1. HttpSessionSecurityContextRepository class

    SecurityContextPersistenceFilter uses SecurityContextRepository to load and save SecurityContext. HttpSessionSecurityContextRepository is an implemntation of SecurityContextRepository.

  2. SaveToSessionResponseWrapper class

And you might need to replace above 2 classes by providing your own implementations or overriding necessary implementations. (And there may some others as well)

Refer:

method implementations.

I think Spring Security docs clearly mentions not to deal with HttpSession. That is why there is a SecurityContext.

IMO just stick to the Spring Security implementations when there are no recommendations from Spring Security engineers to do something on your own. What I'm sugessting here may not correct, and make sure there will no security holes and it should not break the other use cases when you do some non-recommended changes only to cover a corner case. I will never do this if this happened to me, because it is the design that Spring Security experts have decided to go with by considering many many many security facts which I have no idea at all.

sura2k
  • 7,365
  • 13
  • 61
  • 80
  • `SEC-2025` doesn't _directly_ map to this problem since that ticket refers to accidentally logging a user *back in*; whereas here we're facing the problem where a thread finds itself "logged out" even though it started as "logged in". That being said, your answer seems like a good way to tackle this. – jlb Oct 27 '16 at 23:21
  • 1
    @jlb You should understand that with this you will basically get a read-only copy that only you know exists, which means none of Spring Secuity code will use it. And if this is what you want then the whole question is moot, you can make a simple filter that will store context in request/thread local and get the same result. – chimmi Oct 28 '16 at 05:06
  • @chimmi you're absolutely right - i realised this shortly after accepting the bounty. I've got an idea that I'll be updating my answer with shortly - i believe it to be a cleaner approach, though it's catered to my problem. – jlb Oct 28 '16 at 07:19
  • @chimmi i've updated my answer with a simple solution. it's specifically catered to me, but I think it would apply to most cases. – jlb Oct 28 '16 at 08:37
0

We think we have fixed this problem by providing a custom HttpSessionSecurityContextRepository object.

Spring security config file:

  <sec:http auto-config="false"
            entry-point-ref="authenticationProcessingFilterEntryPoint"
            security-context-repository-ref="securityContextRepository">

  <bean id="securityContextRepository"
    class="tn.view.security.CustomHttpSessionSecurityContextRepository"/>

Implementation of CustomHttpSessionSecurityContextRepository:

public class CustomHttpSessionSecurityContextRepository extends HttpSessionSecurityContextRepository
{
  @Override
  public SecurityContext loadContext(final HttpRequestResponseHolder requestResponseHolder)
  {
    final SecurityContext context = super.loadContext(requestResponseHolder);

    // Return a copy of the security context instead of the original security context object
    final SecurityContext contextCopy = new SecurityContextImpl();
    contextCopy.setAuthentication(context.getAuthentication());

    return contextCopy;
  }
}
paulf
  • 1