Refreshed Keycloak tokens are not saved in the session when using Spring-Session-Redis

[haraldpusch]

Describe the bug
When using Spring-Session-Redis and Keycloak, the token data is correctly saved to the session after logging in, but subsequently refreshed tokens are not stored.
When the SSO Token time out has been reached, the next refresh will cause a new token to be generated using the refresh token. However the new token is not saved in the Session - so the next request will again cause a new refresh and so on. Even though we continuously issue requests, the Keycloak auth will be terminated when the SSO Idle Timeout is reached - because the refresh token has not been updated either and therefore is now invalid too.
This currently causes us severe issues in our production environment as users always lose their authentication when the SSO Idle Timeout is reached - even though they are actively using the software.

To Reproduce
When using Spring-Session-Redis and Keycloak, configure Keycloak tokens with a short lifespan like

• SSO Session Idle: 5 Minutes
• Access Token Lifespan: 1 Minute
  Then Login and issue some requests. The first Minute everything is just normal. After one minute the Access token has reached its ttl and needs to be refreshed. That happens in the background for every following request. After 5 Minutes the refresh token reached its ttl and the refresh fails.

Expected behavior
The refreshed Keycloak token data should automatically be saved using the HttpSessionSecurityContextRepository.

Dependency Versions

• spring-security 5.4.2
• keycloak-spring-security-adapter 12.0.1
• spring-session-data-redis 2.4.1

Further Information
I spend a lot of time with this issue because I assumed the error might be with our configuration. But I think I found the root cause of this issue in the HttpSessionSecurityContextRepository.

The RefreshableKeycloakSecurityContext that manages the refresh logic updates the token information in place, mutating its own field values. See: https://github.com/keycloak/keycloak/blob/80bf0b6bad8bb8546db742f67736fdca2c743217/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/RefreshableKeycloakSecurityContext.java#L173 and the following lines.

The RefreshableKeycloakSecurityContext instance is saved in the principal Field of SecurityContextHolder.getContext().getAuthentication()-

Now, when the response is processed by the SecurityContextPersistenceFilter it calls SecurityContextRepository#saveContext. See

spring-security/web/src/main/java/org/springframework/security/web/context/SecurityContextPersistenceFilter.java

Line 116 in 95da121

this.repo.saveContext(contextAfterChainExecution, holder.getRequest(), holder.getResponse());

The repository here is an instance of HttpSessionSecurityContextRepository which internally defers some logic into its SaveToSessionRequestWrapper. This wrapper performs a check if the context has been changed to determine if it should be saved or not. See

spring-security/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java

Line 357 in 95da121

if (contextChanged(context) || httpSession.getAttribute(springSecurityContextKey) == null) {

And here is the actual cause:

spring-security/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java

Lines 368 to 369 in 95da121

	return this.isSaveContextInvoked || context != this.contextBeforeExecution
	|| context.getAuthentication() != this.authBeforeExecution;

The contextChanged Method now checks context != this.contextBeforeExecution || context.getAuthentication() != this.authBeforeExecution but this.authBeforeExecution is the same object as context.getAuthentication but it has been mutated. This causes this check to evaluate to false even though the context acutally changed. Therefore, the changed context is not saved (so in our case updated in redis) and the next request uses the old tokens again.
This issue does not appear in the tomcat session, because an explicit save action is not required.

I have overridden the contextChanged method to always return true - this completly fixed the issue for me.

---

Im sorry if this report seems a bit convoluted/unorganized - debugging this was pretty exhausting. Feel free to edit / reformat this.

[rwinch]

Thanks for the report. Unfortunately we cannot always save. For now, I'd encourage Keycloak to create a new SecurityContext as operating on the same SecurityContext can cause race conditions. While the ThreadLocal provides isolation to a single thread, in memory session environments refer to the same SecurityContext on every Thread that is accessing it. See https://docs.spring.io/spring-security/site/docs/5.4.x/reference/html5/#servlet-authentication-securitycontextholder

We start by creating an empty SecurityContext.
It is important to create a new SecurityContext instance instead of using SecurityContextHolder.getContext().setAuthentication(authentication) to avoid race conditions across multiple threads.

In the long term this will be fixed by #9634

[rwinch]

Closing as invalid since users should not change the SecurityContext directly but instead should create a new SecurityContext.

[lukeway]

Posting my hacky workaround in case it helps others. I tried to find a way to override some small piece in Keycloak to do it "the right way", but I couldn't figure it out. It seemed to involve overriding entire classes. So what I ended up doing was putting a custom filter right before KeycloakSecurityContextRequestFilter:

.addFilterBefore(new OncePerRequestFilter() {
    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
        String accessTokenStringBefore = getAccessTokenString();
        filterChain.doFilter(request, response);
        String accessTokenStringAfter = getAccessTokenString();
        if(!Objects.equals(accessTokenStringBefore, accessTokenStringAfter)) {
            //RequestContextHolder.currentRequestAttributes().setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, SecurityContextHolder.getContext(), RequestAttributes.SCOPE_SESSION);
            SecurityContext context = SecurityContextHolder.createEmptyContext();
            context.setAuthentication(SecurityContextHolder.getContext().getAuthentication());
            SecurityContextHolder.setContext(context);
        }
    }

    private String getAccessTokenString() {
        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();

        if (authentication != null) {
            Object principal = authentication.getPrincipal();

            if (principal instanceof KeycloakPrincipal) {
                KeycloakSecurityContext context = KeycloakPrincipal.class.cast(principal).getKeycloakSecurityContext();
                if(context != null) {
                    return context.getTokenString();
                }
            }
        }

        return null;
    }
}, KeycloakSecurityContextRequestFilter.class)

This basically compares the access token before and after. If it changed, then it creates a new context and uses the existing authentication instance (that Keycloak modified in-place).

I left in a commented line with RequestContextHolder, which is what I was doing prior to reading this issue. I'm not sure that is any better/worse that what I'm doing now.

Definitely not ideal, but it seems to work for now.