Skip to content

Add Spring Session support to OIDC Back-Channel Logout #14904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pzgadzaj opened this issue Apr 13, 2024 · 5 comments
Closed

Add Spring Session support to OIDC Back-Channel Logout #14904

pzgadzaj opened this issue Apr 13, 2024 · 5 comments
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement

Comments

@pzgadzaj
Copy link

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 pzgadzaj added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Apr 13, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Apr 26, 2024

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 jzheaux added type: enhancement A general enhancement in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Apr 26, 2024
@jzheaux jzheaux changed the title Back channel logout fails to invalidate the session when Redis-base session store is being used Add Spring Session support to OIDC Back-Channel Logout Apr 26, 2024
@aelillie
Copy link

aelillie commented Jun 4, 2024

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.

@xiechangning20
Copy link

@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;
    }

@pzgadzaj
Copy link
Author

@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 #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

@dalbani
Copy link

dalbani commented Jul 12, 2024

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.

jzheaux added a commit to jzheaux/spring-security that referenced this issue Aug 8, 2024
This is specifically helpful for Spring Session support

Closes spring-projectsgh-14904
jzheaux added a commit to jzheaux/spring-security that referenced this issue Aug 8, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Aug 8, 2024
This is specifically helpful for Spring Session support

Closes spring-projectsgh-14904
jzheaux added a commit to jzheaux/spring-security that referenced this issue Aug 8, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
This is specifically helpful for Spring Session support

Closes spring-projectsgh-14904
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 4, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 16, 2024
This component already uses by default a URI that doesn't require
a CSRF token and aalready allows for configuring a cookie name.

So, by making it public and configurable in the DSL, both
of these tickets quite naturally close.

Closes spring-projectsgh-13841
Closes spring-projectsgh-14904
jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 16, 2024
This component already uses by default a URI that doesn't require
a CSRF token and aalready allows for configuring a cookie name.

So, by making it public and configurable in the DSL, both
of these tickets quite naturally close.

Closes spring-projectsgh-13841
Closes spring-projectsgh-14904
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants