Describe the bug Apparently, neither the ID token nor the userinfo are updated during the refresh token flow in Spring clients with oauth2Login. This has at least two consequences: - the principal (OidcUser or OAuth2User) might contain outdated data - in case of an RP-Initiated Logout, the OidcClientInitiated(Server)LogoutSuccessHandler might build a redirection URI to the authorization server end_session_endpoint with an expired or outdated ID token, in which case the second part of the logout fails and the user session on the OpenID Provider might not be ended.

Additional context about ID tokens ID tokens are JWTs and, as with all JWTs, they expire.

ID tokens can be refreshed.

During RP-Initiated Logout, OpenID Providers SHOULD (not MUST) accept ID tokens even when the exp time has passed. This means that some OPs might not accept expired tokens and that clients would better do their best to send valid tokens.

Also, aside from expiration considerations, most OPs refreshing ID tokens accept only the last token they issued for a client. So when a new ID token is returned as part of a refresh token flow, the client should use this last issued ID token to build the RP-Initiated Logout location URI.

Last, I know no constraint in the specs about the different tokens relative lifespans. As a consequence, it seems possible that an ID token expires before the access one. And as far as I understood the source, the RefreshToken(Reactive)OAuth2AuthorizedClientProvider refreshes tokens only if the access token expired which leaves room for expired ID token in the security context.

To Reproduce Using an authorization server refreshing ID token as part of the refresh token flow (Keyckloak is one, and, according to the Stackoverflow question linked below, Spring Authorization Server seems to be another): - login to an oauth2Client configured with oauth2Login and RP-Initated Logout - wait until the access token is expired - have refresh token flow executed. Using the (Reactive)OAuth2AuthorizedClientManager to retrieve the access token - like Spring Cloud Gateway TokenRelay= filter does - is enough. - initiate RP-Initiated Logout (send a POST request to the /logout endpoint)

Expected behavior - tokens should be refreshed if either the access or the ID token has expired (or will before the clockSkew) - if the security context contains an ID token (the principal is an OidcUser) and if the refresh token response contains an ID token, then the ID token in the security context should be updated - redirection to the authorization server end_session_endpoint should be built with the last issued ID token

The OidcClientInitiated(Server)LogoutSuccessHandler currently uses the (Reactive)ClientRegistrationRepository which doesn't trigger the refresh token flow in case of expired tokens. Shouldn't it use the (Reactive)OAuth2AuthorizedClientManager instead?

Some StackOverflow questions related to this issue: - https://stackoverflow.com/questions/78901208/spring-authorization-server-refreshes-the-client-tokenoidc-id-token-when-the-a - https://stackoverflow.com/questions/78685992/issue-in-spring-authorization-server-and-spring-cloud-gateway-refresh-token-flow - https://stackoverflow.com/questions/77175866/spring-authorization-server-oidc-logout-not-working-after-refreshing-token-in-sp - https://stackoverflow.com/questions/77665448/oidc-rp-initiated-logout-doesnt-work-once-the-first-access-token-got-refreshed

Comment From: ch4mpy

@sjohnr I think I progressed in my understanding of the root cause for this issue and re-wrote the description accordingly.

Comment From: mlitcher

Just wanted to confirm, Spring Authorization Server does refresh the id_token during the refresh token flow and only seems to accept the last one during an RP-initiated logout by default. So, using Spring Cloud Gateway w/ oauth2 client + Spring Authorization Server, you will get an error during RP-initiated logout after the first token refresh due to this issue.

Comment From: yhao3

I encountered the same issue when using the Spring OAuth2 client with the Spring Authorization Server.

During the refresh token process, if the scopes include the openid scope, the Spring Authorization Server updates the value of id_token in org.springframework.security.oauth2.server.authorization.OAuth2Authorization. However, on the client side, the SecurityContext does not update the id_token after the refresh token process. This happens because org.springframework.security.oauth2.client.RefreshTokenOAuth2AuthorizedClientProvider does not retrieve and update the new id_token from the additionalParameters in OAuth2AccessTokenResponse.

As a result, when the client side later initiates an RP-Initiated Logout, the value of the id_token_hint parameter in the request still uses the old id_token. This causes the Spring Authorization Server to throw an invalid_token error.

Comment From: ch4mpy

This issue does not break solely the RP-Initiated Logout. It might also break the access control in all applications configured with oauth2Login: what is converted to authorities isn't re-evaluated and the Authentication object is not updated.

So, this issue seems rather critical. @sjohnr is it planned to solve it in a specific sprint?

Comment From: sjohnr

Thanks for being patient, and sorry I was unable to look at this closely sooner. I'm doing some thinking about this issue this week. I think the challenges with this issue are as follows:

  1. Information about the id_token is not returned when refreshing an access token. It will be captured in additionalParameters in the OAuth2AccessTokenResponse but is not able to be returned in the resulting OAuth2AuthorizedClient. See #16253.
  2. The RefreshTokenOAuth2AuthorizedClientProvider is likely the best component to "know" that the token is being refreshed (for obvious reasons), and therefore would somehow need to be responsible for updating the SecurityContext. However, it is not related to authentication at all, and therefore should not directly update this.
  3. Also, the RefreshTokenOAuth2AuthorizedClientProvider has no direct knowledge of OIDC, and as such shouldn't care about an id_token.
  4. Logic for authentication and updating the SecurityContext typically lives in authentication filters and is related directly to authentication requests. This issue is not related to an authentication request, so logic for this update needs to live somewhere else.

With all of that as context, I think that assuming we can somehow solve (1) above, a solution for this issue could be to publish an event from the RefreshTokenOAuth2AuthorizedClientProvider and place logic for updating the SecurityContext in an event listener. Concretely:

  1. Get an ApplicationEventPublisher when it is published as a bean (as we do in other parts of Spring Security) and configure it with RefreshTokenOAuth2AuthorizedClientProvider.
  2. Update RefreshTokenOAuth2AuthorizedClientProvider to publish an ApplicationEvent with all of the relevant information when an ApplicationEventPublisher is available.
  3. Create an ApplicationListener that receives this event, determines that it is refreshing an id_token, and uses a SecurityContextRepository to update the current SecurityContext.

Does anyone have additional thoughts on this?

Comment From: sjohnr

One additional thought I have is that we could publish the event with information directly from the OAuth2AccessTokenResponse which would contain the id_token parameter. This could actually make the event listener look much more like the OAuth2LoginAuthenticationFilter. Doing so would also solve (1) without requiring any changes to the OAuth2AuthorizedClient domain model.

Comment From: ch4mpy

A re-authentication triggered by the RefreshTokenOAuth2AuthorizedClientProvider seems a good thing. But I think it should happen every time the refresh token flow is used, even when there is no ID token: in the case of "regular" OAuth2, it could be worth checking that the userinfo has not changed.

A new access-token can mean new authorities. Whatever the source for these authorities is (userinfo endpoint / ID token in the client refreshing tokens, or JWT claims / introspection endpoint on the resource server this client will send the new token to), it needs to be accurate to enforce security (MVC @Controller for Thymeleaf, JSP, ...) or improve UX (prevent the user from taking an action for which he'd get an error from the resource server)

Comment From: sjohnr

Thanks @ch4mpy.

But I think it should happen every time the refresh token flow is used, even when there is no ID token: in the case of "regular" OAuth2, it could be worth checking that the userinfo has not changed.

By "regular" OAuth2 do you mean the original OAuth2 Login with Twitter or Facebook social login use case?

Comment From: ch4mpy

I mean OAuth2 without OpenID, when there is never an ID token and the OAuth2 client with oauth2Login builds a DefaultOAuth2User instance from the userinfo endpoint.

Reading the OpenID spec again, the "every time the refresh token flow is used" might not be strictly required. Something like "if an ID token is part of the response or the openid scope not used in the request" is probably enough. But in the case of a refresh token request with the openid scope and a response without an ID token, maybe the ID token already in the security context can be used for the re-authentication (which could then happen for every refresh token flow)?

Comment From: sjohnr

Ok. I think we can keep it in mind, and see if it's possible to handle for OAuth2 Login without OIDC. However, I think focusing on updating the SecurityContext for the OIDC use-case of RP-Initiated Logout is still the primary focus.

Comment From: filiphr

Thanks for the additional details here @sjohnr as I was the one that proposes #16253 I have also been following this issue. Let's assume we tackle that issue separately. For the other points, perhaps an option would be to tackle this using the OAuth2AuthorizationSuccessHandler. The DefaultOAuth2AuthorizedClientManager is responsible for doing the authorization, and handles the refresh token. So assuming that with #16253 we would have the additional parameters and thus the id_token in OAuth2AuthorizedClient, then an OIDC implementation could do what needs to be done to refresh the OIDC user.

Currently the OAuth2AuthorizationSuccessHandler is used to persist the token information in a session or somewhere else. An alternative could also be to throw an event in the DefaultOAuth2AuthorizedClientManager that an authorization was successful and then have the existing storing of the token listening on that, but also the new OIDC relevant stuff.

Comment From: sjohnr

Thanks @filiphr, appreciate your perspective on this.

For the other points, perhaps an option would be to tackle this using the OAuth2AuthorizationSuccessHandler.

It is certainly an option to solve #16253 first with changes to the domain model, and then use a success handler configured with the OAuth2AuthorizedClientManager to publish an event. However, this would still require opting into by configuring an ApplicationEventPublisher with the success handler. I consider re-configuring the success handler an advanced customization, and as such it's not ideal to require users to do that in order to achieve this use case.

The DefaultOAuth2AuthorizedClientManager is responsible for doing the authorization, and handles the refresh token.

Also, I don't consider adding a setApplicationEventPublisher() to DefaultOAuth2AuthorizedClientManager and hiding event publishing inside an internal OAuth2AuthorizationSuccessHandler an ideal solution. Because the RefreshTokenOAuth2AuthorizedClientProvider is responsible for obtaining refresh tokens, it seems an ideal place to publish an event related to refreshing tokens. It is also reusable across implementations of OAuth2AuthorizedClientManager.

This would be extremely easy to opt into and in fact could potentially be handled by the framework in many cases without requiring manual configuration by the user, since we could detect the ApplicationEventPublisher as a bean and use it automatically.

Comment From: sjohnr

Hey folks, sorry for the delay, it has taken some time to make progress on this issue (which I suspected would be the case). I have updated the PR #16589 based on work by @yhao3 with changes to fully capture this idea.

There are two pairs of events and listeners:

The first listener (OidcAuthorizedClientRefreshedEventListener) captures the "refresh" of an OAuth2AuthorizedClient and the associated OAuth2AccessTokenResponse by listening for the event OAuth2AuthorizedClientRefreshedEvent. This listener processes the response and determines if an OidcUser needs to be refreshed by looking for an id_token. If found, it publishes another event, OidcUserRefreshedEvent.

The second listener (OidcUserRefreshedEventListener) simply listens for OidcUserRefreshedEvent and updates the SecurityContext.

The reason there are two events is that it is nice to keep separate the logic of creating an updated Authentication (for this specific scenario) and actually updating the SecurityContext. I feel that updating of a SecurityContext via an event is something that could likely change and possibly be made more general and/or reusable, so I've kept it separate and hidden for now.

@yhao3 @ch4mpy @filiphr @mlitcher If you have a chance, please review #16589 with my updates and provide feedback. Please feel free to also test this out on the branch, or if you prefer you can just provide feedback and wait until it gets merged and test in a milestone release.

Comment From: ch4mpy

Publishing a UserRefreshedEvent as part of the refresh token flow and, in applications with oauth2Login, listening to such events to update the OAuth2AuthenticationToken in the security context could solve this issue.

However, from what I understand from the PR, it will work only when the openid scope is included: only the case where the principal is an OidcUser is considered.

That said, I only use as authorization servers some OpenID Providers fully complying with OIDC, and I always request the openid scope. So, for my use cases, the PR should be enough. Thank you.

Comment From: sjohnr

Hi @ch4mpy, thanks for the feedback.

from what I understand from the PR, it will work only when the openid scope is included: only the case where the principal is an OidcUser is considered.

This is true currently. While this issue is general and includes both OAuth2 and OpenID Connect forms of login, the solution is currently aimed at just OIDC, which has a clear use case and associated problem (the id_token not being refreshed). I also wanted to keep the initial iteration small. We could of course add support for refreshing an OAuth2User later.

The issue I see there is that we only have one piece of information telling us that an "OAuth2 Login is being refreshed", and that's the clientRegistrationId. Since OAuth2 Login (which is not bound by a formal spec) does not utilize an id_token, it would be somewhat of a guess that the user needs to be refreshed. And worse, that guess comes from the client side. However, when an authorization server returns an id_token in the response for the OIDC case, it is very clear that the user should be refreshed.

For that reason, I feel the first stab at this should focus on OIDC. It may be possible and worthwhile to follow up with an enhancement to focus on the non-OIDC case but I'm not fully convinced yet of the need. The ground work would be laid for this to be added in the framework later or just as a custom event listener. That's also the reason that I've kept the 2nd event listener hidden, so we could expand it to possibly respond to refreshing other user types, such as OAuth2User. We might leave this issue open after the PR is merged since it's not fully solved as you originally outlined it. Or we could open a new issue for the OAuth2 scenario. I'm open to other ideas as well.

Comment From: ch4mpy

Since OAuth2 Login (which is not bound by a formal spec) does not utilize an id_token, it would be somewhat of a guess that the user needs to be refreshed.

As I understand oauth2Login, it is intended to configure a Spring OAuth2 client with authorization code and refresh token flows.

In this context, the ID token is no more than one of the possible sources for knowing who the user is. The userinfo endpoint is another one. But to me, the Authentication interface is not only about who the user is (his name). It is also about what he can do (his authorities).

As granted authorities usually drive what a user can access, it should probably be derived from the access token (which is not the case with the default authorities mapper, but that's another story). So, the OAuth2AuthenticationToken should be updated each time an authorization or refresh token response is received: with a new access token, the accessible resources are possibly modified (because of potentially modified scopes, roles, or whatever resources access control is based on). And in the case where the response has an error status, the security context should probably be set with an AnonymousAuthenticationToken.

So, if the application with oauth2Login is just a UI that uses the access token to call a REST API, it might need a refreshed Authentication to adapt the UI to what the user can perform with this new access token. And if this application serves resources itself (access token not used to call another app), then the Authentication refresh becomes critical: the new access rules should be applied not only for user experience but also for resource access decisions.

We might leave this issue open after the PR is merged since it's not fully solved

@sjohnr yes, I think it should remain open. Even if it was my motivation in the 1st place, what I report is not only about the need to update the ID token, it is the need to update all of the OAuth2AuthenticationToken properties each time an access or ID token is issued.

Comment From: toob2

@ch4mpy do you know if this was fixed? Im having the same issue when using SCG RP initiated logout even after upgrading to 6.5.0

Comment From: ch4mpy

@toob2, I know 2 scenarios for which it is not fixed: - Reactive applications. For instance, the current fix works with spring-cloud-gateway-mvc, but not with spring-cloud-gateway. - Frontends sending parallel requests.

Sample scenario for parallel requests: 1. the OAuth2 client get tokens (access + refresh). I mean an app with oauth2Login, either a REST API itself or a Spring Cloud Gateway instance with the TokenRelay= filter in front of apps configured with oauth2ResourceServer 2. The user changes the page to display, requiring the app with oauth2Login to process several requests in parallel (load referential data, fetch entities, ...), after the access tokens expire, but before the refresh token does 4. As the access token expired (or is about to expire within the clockSkew), the OAuth2 client refreshes tokens

As stated by @sjohnr, Spring Security is not designed to handle parallel requests from the same frontend. In this case, each request may be running a refresh token flow of its own, resulting in potentially as many new ID tokens as parallel requests, with at least two consequences: - Latency might increase dramatically. - There is no guarantee that the last ID token emitted by the authorization server is the one saved in the client session => RP-Initiated Logout might fail because of an outdated ID token (when the OP accepts only the last ID token it emitted, which is frequently the case).

So, when having frontends sending parallel requests (which is a very frequent use-case for me), we should override the RefreshToken(Reactive)OAuth2AuthorizedClientProvider to ensure that only one refresh token flow runs at a time for a given session.

This failure is difficult to spot because it occurs only when there are concurrent requests from the same frontend when the tokens should be refreshed, and when the ID token saved in the session isn't the last emitted, which makes this bug flickery and hard to reproduce.

Comment From: toob2

@toob2, I know 2 scenarios for which it is not fixed:

  • Reactive applications. For instance, the current fix works with spring-cloud-gateway-mvc, but not with spring-cloud-gateway.
  • Frontends sending parallel requests.

Sample scenario for parallel requests:

  1. the OAuth2 client get tokens (access + refresh). I mean an app with oauth2Login, either a REST API itself or a Spring Cloud Gateway instance with the TokenRelay= filter in front of apps configured with oauth2ResourceServer
  2. The user changes the page to display, requiring the app with oauth2Login to process several requests in parallel (load referential data, fetch entities, ...), after the access tokens expire, but before the refresh token does
  3. As the access token expired (or is about to expire within the clockSkew), the OAuth2 client refreshes tokens

As stated by @sjohnr, Spring Security is not designed to handle parallel requests from the same frontend. In this case, each request may be running a refresh token flow of its own, resulting in potentially as many new ID tokens as parallel requests, with at least two consequences:

  • Latency might increase dramatically.
  • There is no guarantee that the last ID token emitted by the authorization server is the one saved in the client session => RP-Initiated Logout might fail because of an outdated ID token (when the OP accepts only the last ID token it emitted, which is frequently the case).

So, when having frontends sending parallel requests (which is a very frequent use-case for me), we should override the RefreshToken(Reactive)OAuth2AuthorizedClientProvider to ensure that only one refresh token flow runs at a time for a given session.

This failure is difficult to spot because it occurs only when there are concurrent requests from the same frontend when the tokens should be refreshed, and when the ID token saved in the session isn't the last emitted, which makes this bug flickery and hard to reproduce.

@ch4mpy thanks, I'm using the reactive version, will wait for the fix in #17188

Comment From: ch4mpy

Well, if using Java 21 or above (with virtual threads), you should probably switch to the servlet version.

Anyhow, unless the Spring Security team changes its mind about parallel requests within the same session, you might face the issue I describe above, even after gh-17188 is fixed.

Comment From: joaquinjsb

Well, if using Java 21 or above (with virtual threads), you should probably switch to the servlet version.

Anyhow, unless the Spring Security team changes its mind about parallel requests within the same session, you might face the issue I describe above, even after gh-17188 is fixed.

For the case you described for parallel requests, do you have any PoC in MVC? I have this exact issue

Comment From: ch4mpy

@joaquinjsb I don't have a public reproducer, which, as I wrote above, is not that easy to make. We'd need: - an OAuth2 client with oauth2Login - an authorization server accepting only the last emitted ID token for RP-Initiated Logout (this is not complicated as most do, but still requires a Docker compose file with several services, and user database initialization) - a UI sending parallel requests - an end-to-end script to: - login - wait until the access token expiration and perform a page reload or whatever triggers the parallel requests - perform a logout - enough repetitions to have a race condition and a failed logout (the token saved in the OAuth2 client session is not the last emitted by the authorization server)

This is quite some work and is mostly useless, considering that the Spring Security team stated that it is an expected behavior (when using Spring Security, we should serialize requests).

So, for now, the point is more about gathering enough community feedback that parallel requests should be a supported use case, rather than putting reproducers together or even submitting PRs.

Comment From: joaquinjsb

got it, I guess I'll try to implement a way to synchronize the RefreshToken(Reactive)OAuth2AuthorizedClientProvider, any hints on it?

Thanks