spring-security icon indicating copy to clipboard operation
spring-security copied to clipboard

Add Spring Session support to OIDC Back-Channel Logout

Open pzgadzaj opened this issue 1 year ago • 4 comments

Describe the bug When using Spring boot in version 3.2.1, together with Redis-base session store, session invalidation fails because of lack of Base64 cookie encoding

When back channel logout implementation tries to invalidate the session, It makes a POST with Session cookie created based on session stored in OidcSessionRegistry. Value of the session cookie is not being base64-encoded: https://github.com/spring-projects/spring-security/blob/main/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OidcBackChannelLogoutHandler.java#L108

When the this POST is being handled, Session cookie is by default base64-decoded: https://github.com/spring-projects/spring-session/blob/main/spring-session-core/src/main/java/org/springframework/session/web/http/DefaultCookieSerializer.java#L101

which cause that the session invalidation fails

To Reproduce

  1. Prepare an application which uses Spring session stored in Redis + OIDC back channel configured
  2. Log in to the application using OIDC integration
  3. Trigger OIDC back channel logout

Expected behavior

  1. Session established in step 2 is invalidated

Sample

A link to a GitHub repository with a minimal, reproducible sample.

Reports that include a sample will take priority over reports that do not. At times, we may require a sample, so it is good to try and include a sample up front.

pzgadzaj avatar Apr 13 '24 07:04 pzgadzaj

Hi, @pzgadzaj-equinix, thanks for reaching out. Spring Session support for OIDC Backchannel Logout is forthcoming as we also need to expose the ability to change the cookie name. Or it may be the case that Spring Session publishes a LogoutHandler of its own so that it can apply the CookieSerializer directly.

I'll leave this ticket to explore the best route for that.

jzheaux avatar Apr 26 '24 00:04 jzheaux

When supporting Spring Session with OIDC Backchannel Logout you also need to consider how the OidcSessionRegistry should be notified about removing the stored session information, as it is expected to be initiated by the SessionDestroyedEvent (via HttpSessionEventPublisher) and handled in OAuth2LoginConfigurer.OidcClientSessionEventListener.onApplicationEvent.

aelillie avatar Jun 04 '24 15:06 aelillie

@aelillie @pzgadzaj-equinix

Thanks for identifying the cause. We had exactly the same problems, and managed to resolve it by customizing the CookieSerializer according to https://docs.spring.io/spring-session/reference/guides/java-custom-cookie.html

   @Bean
    public CookieSerializer cookieSerializer() {
        DefaultCookieSerializer serializer = new DefaultCookieSerializer();
        serializer.setCookieName("JSESSIONID");
        serializer.setUseBase64Encoding(false);
        return serializer;
    }

xiechangning20 avatar Jun 04 '24 18:06 xiechangning20

@xiechangning20 I guess that disabling bas64 encoding is an option but then this also requires using Session ids which don't need to be base64-encoded

In my case, I base64 encoding is not needed (we use default session id generator for redis, which i assume is UUIDv4 based), but i don't want to change how Session id is being constructed as this would mean that i have to logout users during the release which introduces such change

@aelillie, I'm aware of OidcSessionRegistry. To make back channel work in clustered environment, we had to provide another implementation of OidcSessionRegistry which stores entries in JDBC store - see https://github.com/spring-projects/spring-security/issues/14511

Anyway, i would appreciate if back channel logout is able to cooperate with Spring sessions without the need of disabling base64 encoding on the session cookie

pzgadzaj avatar Jun 11 '24 11:06 pzgadzaj

Not really relevant to the question of OIDC Back-channel Logout, but I've personally disabled Base64 encoding as it lets (malicious) users encode an incorrect session ID value, e.g. one containing a NULL byte, which ends up causing an error when sessions are stored in a PostgreSQL database:

org.springframework.dao.DataIntegrityViolationException: PreparedStatementCallback;
    SQL [SELECT S.PRIMARY_ID, S.SESSION_ID, S.CREATION_TIME, S.LAST_ACCESS_TIME, S.MAX_INACTIVE_INTERVAL, SA.ATTRIBUTE_NAME, SA.ATTRIBUTE_BYTES
        FROM SPRING_SESSION S
        LEFT JOIN SPRING_SESSION_ATTRIBUTES SA ON S.PRIMARY_ID = SA.SESSION_PRIMARY_ID
        WHERE S.SESSION_ID = ?
    ]; ERROR: invalid byte sequence for encoding "UTF8": 0x00

I just didn't see the benefit of Base64 encoding, so I bit the bullet and deployed the change, which indeed logged all users out.
Even though in the case of OIDC-backed login, the users were obviously not logged out from the IdP, so the change didn't cause any real disruption.

dalbani avatar Jul 12 '24 11:07 dalbani