Thanks for your patience @evgeniycheban and apologies that I could not get this into 7.0. We had quite a workload for this round of majors.

I have scheduled this for 7.1.0-M1 and will review this soon. In the meantime, when you have a moment can you please rebase. Thank you.

Hi @jgrandja, I have rebased my branch and also updated license headers to meet new pattern, let me know your feedback once you review it, thanks.

@evgeniycheban I took a look at the PR and noticed that the implementation is different compared to the Servlet implementation.

RefreshTokenReactiveOAuth2AuthorizationSuccessHandler combines the logic in OidcAuthorizedClientRefreshedEventListener and OidcUserRefreshedEventListener and does not make use of Spring Framework's ApplicationEventPublisher and ApplicationListener.

The preference is to keep the Servlet and Reactive implementations as close as possible. Is there a reason you did not make use of ApplicationEventPublisher and ApplicationListener ?

Just a heads up that I'm on PTO starting tomorrow and returning Jan 5 so I'll reply when I return and we can work through this.

Hi @jgrandja, thank you for reviewing it. The reason I did not use the ApplicationEventPublisher is because in the current implementation there is no support for reactive application events, I can recall that there was a draft PR raised by @marcusdacoregio but it didn't get merged.

@evgeniycheban Got it. I'll do a detailed review next week and we can work towards merging this.

Hi @jgrandja, thank you for the review, after we moved refreshTokenSuccessHandler to DefaultReactiveOAuth2AuthorizedClientManager I noticed that we no longer can access OAuth2AccessTokenResponse which is needed to get id_token from additionalParameters, would it be an option to add a String idToken field to OAuth2AuthorizedClient or even Map<String, Object> additionalParameters to store an additional context for the client?

@evgeniycheban Instead of adding String idToken to OAuth2AuthorizedClient please add private Map<String, Object> attributes and in there you can map OidcParameterNames.ID_TOKEN.

Please ensure to update OAuth2AuthorizedClient in OAuth2LoginAuthenticationFilter and the reactive counterpart.

Hi @jgrandja, I've pushed the latest changes, please have a final look :)
I've also replaced deprecated org.springframework.lang.Nullable annotation with jspecify in some files.

@evgeniycheban The addition of OAuth2AuthorizedClient.attributes resulted in too many touch points in the code that doesn't feel right. In addition to the last updates, we still need to update oauth2-client-schema.sql, oauth2-client-schema-postgres.sql, JdbcOAuth2AuthorizedClientService and R2dbcReactiveOAuth2AuthorizedClientService, which is making me rethink things.

Looking at my previous comment:

ReactiveOAuth2AuthorizationSuccessHandler is not designed to be used with a ReactiveOAuth2AuthorizedClientProvider and instead is meant to be used with a ReactiveOAuth2AuthorizedClientManager.

Can you re-work this and set RefreshTokenReactiveOAuth2AuthorizationSuccessHandler in DefaultReactiveOAuth2AuthorizedClientManager ?

This is where things took a turn and after reviewing the API's, I now feel ReactiveOAuth2AuthorizationSuccessHandler can also be used by a ReactiveOAuth2AuthorizedClientProvider even though it was initially designed to be used with a ReactiveOAuth2AuthorizedClientManager.

Having said that, your initial motivation to associate RefreshOidcUserReactiveOAuth2AuthorizationSuccessHandler with RefreshTokenReactiveOAuth2AuthorizedClientProvider was correct and I should have stuck with that as it's a much cleaner implementation and keeps the changes very isolated. Apologies for this but can you please revert OAuth2AuthorizedClient.attributes and associated updates and add RefreshTokenReactiveOAuth2AuthorizedClientProvider.setAuthorizationSuccessHandler(ReactiveOAuth2AuthorizationSuccessHandler) making RefreshOidcUserReactiveOAuth2AuthorizationSuccessHandler the default.

@evgeniycheban The addition of OAuth2AuthorizedClient.attributes resulted in too many touch points in the code that doesn't feel right. In addition to the last updates, we still need to update oauth2-client-schema.sql, oauth2-client-schema-postgres.sql, JdbcOAuth2AuthorizedClientService and R2dbcReactiveOAuth2AuthorizedClientService, which is making me rethink things.

Looking at my previous comment:

ReactiveOAuth2AuthorizationSuccessHandler is not designed to be used with a ReactiveOAuth2AuthorizedClientProvider and instead is meant to be used with a ReactiveOAuth2AuthorizedClientManager.
Can you re-work this and set RefreshTokenReactiveOAuth2AuthorizationSuccessHandler in DefaultReactiveOAuth2AuthorizedClientManager ?

This is where things took a turn and after reviewing the API's, I now feel ReactiveOAuth2AuthorizationSuccessHandler can also be used by a ReactiveOAuth2AuthorizedClientProvider even though it was initially designed to be used with a ReactiveOAuth2AuthorizedClientManager.

Having said that, your initial motivation to associate RefreshOidcUserReactiveOAuth2AuthorizationSuccessHandler with RefreshTokenReactiveOAuth2AuthorizedClientProvider was correct and I should have stuck with that as it's a much cleaner implementation and keeps the changes very isolated. Apologies for this but can you please revert OAuth2AuthorizedClient.attributes and associated updates and add RefreshTokenReactiveOAuth2AuthorizedClientProvider.setAuthorizationSuccessHandler(ReactiveOAuth2AuthorizationSuccessHandler) making RefreshOidcUserReactiveOAuth2AuthorizationSuccessHandler the default.

Hi @jgrandja, no worries, I will do it and notify you once I finish. Initially, I wasn't sure about the design of this feature and chose the minimal changes path to see how it would work in the current architecture.

Hi @jgrandja, I've updated the PR, please have a look when you have time, thanks.

Thank you for all the updates and patience @evgeniycheban. Very solid work! 👍

This is now merged.