Search code examples
springspring-bootspring-securitymigrationdeprecation-warning

With deprecation of the `SecurityContextRepository.loadContext(HttpRequestResponseHolder)` method, how do we add `Set-Cookie` to the response?

Context

We have for some time been using Spring-Security to protect our edge Spring-Boot Java applications.

Our system-wide authentication architecture (which is used by Spring-Boot apps, other Java apps and NodeJS apps) makes use of client-side session cookies (i.e. those that die when a browser window is closed) which store a session ID, and a cluster-internal session store keyed on those IDs.

Implementation in Spring-Boot 2.6.x and below

For our Spring-Boot applications, we have a long-standing custom implementation of SecurityContextRepository where the implementation of the SecurityContext loadContext(HttpRequestResponseHolder) method performs something along the lines of the following:

  1. If there is no session ID cookie, return an empty SecurityContext.
  2. If there is a session ID cookie, which corresponds to a valid session, return a valid security context and, in some circumstances, add a Set-Cookie directive on the response to update the cookie.
  3. If there is a session ID cookie, which does not correspond to a valid session, return an empty SecurityContext, and add a Set-Cookie directive on the response to clear the invalid cookie.

Proposed implementation in Spring-Boot 2.7.x and above

Making the upgrade from Spring-Boot 2.6.x to 2.7.x, the SecurityContext loadContext(HttpRequestResponseHolder) method has been deprecated. I've been looking at the alternative advised, namely Supplier<SecurityContext> loadContext(HttpServletRequest), and wondering how to keep our system behaviour constant while migrating to the new APIs. Referring to my numbering above, I have come to the conclusion so far that:

  1. No change required - the response was never used
  2. If we need to update the cookie on the response, then ensure that the relevant information is available in the SecurityContext that we return, and then customise the saveContext() implementation to ensure the cookie is updated if necessary
  3. Return an empty SecurityContext as before, and customise the saveContext() implementation to ensure the cookie is cleared in if the security context is empty.

The question(s)

I'd be grateful if one of the Spring team, or another enlightened StackOverflow user, would be able to confirm that the above migration strategy is appropriate, or correct me if I have misunderstood anything.

If I have got it right, perhaps it would be worth updating the Javadoc of the deprecated method to make it clearer for other users also?

Many thanks.

Solution

  • 18 months on from asking this question, I went on to tackle this, as when you upgrade to Spring-Boot 3 it's no longer optional - although the loadContext(HttpRequestResponseHolder) method is still there and deprecated, when it's invoked by Spring, the inner HttpServletResponse object is always null.

    My original answer (from March 2024, now deprecated)

    The answer to my question is fundamentally that my proposed implementation was a good one.

    Some more details of how we moved forward, in case it is useful to readers of this question:

    1. Our implementation of SecurityContext now includes a List<AuthorizationResultAction> actions field, in addition to the nullable Authentication field.
    2. The AuthorizationResultAction class is really just a way of defining a deferred operation to be performed on the HttpServletResponse object, once that's available.
    3. In our implementation of SecurityContextRepository.loadContext() / loadDeferredContext() etc, we populate the actions with whatever we want to be done, and ensure that we always return an instance of our SecurityContext class (including where we previously might have called SecurityContextHolder.createEmptyContext())
    4. In our implementation of SecurityContextRepository.saveContext(), where previously we did nothing, we now:
      • if the provided SecurityContext is an instance of our implementation, then check for any actions, and apply them
      • otherwise, if the SecurityContext has getAuthentication() == null - which might be, for example, from a rejected containsContext() operation - then do what we want to in this context (which in our case, is to clear the session ID cookie).

    My updated answer (from April 2024):

    It turns out my proposed implementation did not work as expected for the following reasons.

    1. The saveContext() method isn't automatically called. After discovering this, and code reading a little, we tried setting requireExplicitSave to false, like this: httpSecurity.securityContext(security -> security .securityContextRepository(ourRepositoryImpl).requireExplicitSave(false)). See https://docs.spring.io/spring-security/reference/servlet/authentication/persistence.html for details of what this does.

    2. After this change, it still didn't work - and it took diving into the debugger to figure out why. The TL;DR is that by the time Spring calls the saveContext() method, the HttpServletResponse has already been written/finalised, so although you can call response.addCookie() to your heart's content, it won't actually do anything!

    Eventually we have got things to work as desired, with the following setup:

    • Our auth middleware implemented as public class AuthMiddlewareFilter extends OncePerRequestFilter; this class is not defined as a bean, but instead is injected into the Spring Security filter chain via a configuration call to httpSecurity.addFilterBefore(new AuthMiddlewareFilter(), SecurityContextHolderFilter.class)

    • The AuthMiddlewareFilter does everything it needs to in terms of setting / clearing cookies, and then calls request.setAttribute("our-auth-result", authResult);

    • Our SecurityContextRepository implementation now just calls request.getAttribute("our-auth-result"); in the loadContext() / loadDeferredContext() methods, to set up Spring's security context.

    I hope these details are useful to others who may have been struggling with the same migration.