OAuth2: No new token is requested when only password grant is used

[geobreze]

Describe the bug

When RefreshTokenOAuth2AuthorizedClientProvider is not registered, but PasswordOAuth2AuthorizedClientProvider does, next issue may happen. In case when accessToken expired, but there is refreshToken in the context, no authorization is performed.

This happens because of next block which assumes that RefreshTokenOAuth2AuthorizedClientProvider presents, which is not always true.

spring-security/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/PasswordOAuth2AuthorizedClientProvider.java

Lines 95 to 101 in 7905409

	if (authorizedClient != null && hasTokenExpired(authorizedClient.getAccessToken())
	&& authorizedClient.getRefreshToken() != null) {
	// If client is already authorized and access token is expired and a refresh
	// token is available, than return and allow
	// RefreshTokenOAuth2AuthorizedClientProvider to handle the refresh
	return null;
	}

To Reproduce

• When building provider viaOAuth2AuthorizedClientProviderBuilder, use password, but not refreshToken providers
• Get an expired token or wait until token expires
• Now all requests will have no authorized client

Expected behavior

Password provider should get new token in case when there is no RefreshTokenOAuth2AuthorizedClientProvider.

[sjohnr]

Hi @geobreze, thanks for the report. Perhaps I'm misunderstanding and you can help clarify: Based on the code snippet from PasswordOAuth2AuthorizedClientProvider, why would you have a refresh token but not have the refresh token provider enabled? Or said differently, why would authorizedClient.getRefreshToken() != null be true in your case?

[geobreze]

Hi @sjohnr, In our case, oauth2 backend had some issues with token refresh, but it was sending us both accessToken and refreshToken. So we temporary removed RefreshTokenOAuth2AuthorizedClientProvider from our configuration and after that this issue appeared.

[sjohnr]

Thanks for the clarification there, that clears it up for me. Unfortunately, I don't see an easy way for the framework to solve your case while still providing the delegation-based strategy (loosely coupled) between different providers. Since yours is more of an edge case, I would suggest removing the refreshToken from the response. This can be accomplished with something like the following configuration:

    @Bean
    public OAuth2AuthorizedClientManager authorizedClientManager(
            ClientRegistrationRepository clientRegistrationRepository,
            OAuth2AuthorizedClientRepository authorizedClientRepository) {
        OAuth2AuthorizedClientProvider authorizedClientProvider =
            OAuth2AuthorizedClientProviderBuilder.builder()
                .password((builder) -> builder
                    .accessTokenResponseClient(createPasswordAccessTokenResponseClient())
                    .build())
                .build();

        DefaultOAuth2AuthorizedClientManager authorizedClientManager =
            new DefaultOAuth2AuthorizedClientManager(clientRegistrationRepository, authorizedClientRepository);
        authorizedClientManager.setAuthorizedClientProvider(authorizedClientProvider);

        return authorizedClientManager;
    }

    private OAuth2AccessTokenResponseClient<OAuth2PasswordGrantRequest> createPasswordAccessTokenResponseClient() {
        // Deprecated in 5.6. See note.
        MapOAuth2AccessTokenResponseConverter defaultAccessTokenResponseConverter =
            new MapOAuth2AccessTokenResponseConverter();

        Converter<Map<String, String>, OAuth2AccessTokenResponse> noRefreshTokenConverter =
            defaultAccessTokenResponseConverter
                .andThen((accessTokenResponse) ->
                    OAuth2AccessTokenResponse.withResponse(accessTokenResponse)
                        .refreshToken(null)
                        .build());

        OAuth2AccessTokenResponseHttpMessageConverter accessTokenResponseHttpMessageConverter =
            new OAuth2AccessTokenResponseHttpMessageConverter();
        accessTokenResponseHttpMessageConverter.setTokenResponseConverter(noRefreshTokenConverter);

        RestTemplate restTemplate = new RestTemplate(
            Arrays.asList(new FormHttpMessageConverter(), accessTokenResponseHttpMessageConverter));
        restTemplate.setErrorHandler(new OAuth2ErrorResponseErrorHandler());

        DefaultPasswordTokenResponseClient passwordTokenResponseClient = new DefaultPasswordTokenResponseClient();
        passwordTokenResponseClient.setRestOperations(restTemplate);

        return passwordTokenResponseClient;
    }

Note: MapOAuth2AccessTokenResponseConverter is deprecated in 5.6. due to gh-9685. The new class name is DefaultMapOAuth2AccessTokenResponseConverter.

Is this an acceptable workaround for your case?

[geobreze]

Thanks for the solution! We've applied another workaround for this case (error handling for token response, as advised in #10016). I just left this issue here to let you know about this (maybe, incredibly rare) edge case. Thanks for your input and your time investment, we will consider this workaround if something fails on production again 😃

[sjohnr]

Thanks. I will keep this in mind, as there is a theme around these APIs in the community right now. In this case though, I think the framework seems to be doing the sensible thing out of the box, and there are ways (some possibly even simpler than my suggestion) to customize for special cases.

I'm going to close this for now, but feel free to re-open or add additional comments if you think of anything we missed in the discussion.

[rakheen-dama]

You can create a wrapper class for the OAuth2AuthorizedClientManager and ensure the expired token is removed before it is used

@RequiredArgsConstructor
public class OAuth2ClientManagerWrapper implements OAuth2AuthorizedClientManager {

    private final OAuth2AuthorizedClientManager manager;
    private final OAuth2AuthorizedClientService clientService;

    @Override
    public OAuth2AuthorizedClient authorize(final OAuth2AuthorizeRequest authorizeRequest) {
        if (authorizeRequest.getAuthorizedClient() != null &&
                Objects.requireNonNull(authorizeRequest.getAuthorizedClient().getRefreshToken()).getExpiresAt()
                        .isBefore(Instant.now().plus(1, ChronoUnit.MINUTES))) {
            String registrationId = authorizeRequest.getAuthorizedClient().getClientRegistration().getRegistrationId();
            clientService.removeAuthorizedClient(registrationId, authorizeRequest.getPrincipal().getName());
        }
        return manager.authorize(authorizeRequest);
    }
}

@Bean
    public OAuth2AuthorizedClientManager authorizedClientManager(
            final ClientRegistrationRepository clientRegistrationRepository,
            final OAuth2AuthorizedClientService oAuth2AuthorizedClientService) {
        final String username = env.getProperty("spring.security.oauth2.client.registration.foo.username");
        final String password = env.getProperty("spring.security.oauth2.client.registration.foo.password");

        var provider = OAuth2AuthorizedClientProviderBuilder.builder()
                .password()
                .refreshToken(rt -> rt.clockSkew(Duration.ofMinutes(1)).build())
                .build();

        var authorizedClientManager = new AuthorizedClientServiceOAuth2AuthorizedClientManager(clientRegistrationRepository, oAuth2AuthorizedClientService);
        
        var authManager = new OAuth2ClientManagerWrapper(authorizedClientManager, oAuth2AuthorizedClientService);
        authorizedClientManager.setAuthorizedClientProvider(provider);
        authorizedClientManager.setContextAttributesMapper(
                c -> Map.of(
                        OAuth2AuthorizationContext.USERNAME_ATTRIBUTE_NAME, Objects.requireNonNull(username),
                        OAuth2AuthorizationContext.PASSWORD_ATTRIBUTE_NAME, Objects.requireNonNull(password))
        );
        return authManager;
    }