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.