Skip to content

OAuth2 client doesn't update token when refresh of token had failed #10016

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
geobreze opened this issue Jun 29, 2021 · 4 comments
Closed

OAuth2 client doesn't update token when refresh of token had failed #10016

geobreze opened this issue Jun 29, 2021 · 4 comments
Assignees
Labels
for: stackoverflow A question that's better suited to stackoverflow.com

Comments

@geobreze
Copy link

Describe the bug

We're using Spring security OAuth2 client with grant type password. It uses both access_token and refresh_token. When access_token expires, token tries to refresh and we're getting error from backend.

In this case, token is not refreshed and not removed from token repository and next call to API, tries to refresh token with same (wrong) refresh token, thus causing infinite loop for token refresh.

To Reproduce

Please, see attached sample.

Expected behavior

It's expected to get new access_token and refresh_token pair using grant type other than refresh_token when refresh had failed.

Sample

https://github.com/geobreze/oauth2-client-refresh-token-demo

@geobreze geobreze added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jun 29, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Jun 29, 2021

The refresh token is removed when the error in the response is of an expected type. For example, if I change your unit test to return:

{ "error" : "invalid_grant", "error_description" : "test desc" }

then the test passes.

Or, if I change your configuration to do:

clientManager.setAuthorizationFailureHandler(
    new RemoveAuthorizedClientOAuth2AuthorizationFailureHandler(
        (clientRegistrationId, principal, attributes) -> clientService
            .removeAuthorizedClient(clientRegistrationId, principal.getName()), 
        Collections.singleton("test")
    )
);

then the test also passes.

Or, if you need something more flexible than that, you can wire your own OAuth2AuthorizationFailureHandler. If I change your configuration like so:

clientManager.setAuthorizationFailureHandler((exception, principal, attributes) -> {
    String registrationId = ((ClientAuthorizationException) exception).getClientRegistrationId();
    clientService.removeAuthorizedClient(registrationId, principal.getName());
});

then the test also passes.

I'm going to close this as answered, but please comment if you feel like there's more to discuss.

@jzheaux jzheaux closed this as completed Jun 29, 2021
@jzheaux jzheaux self-assigned this Jun 29, 2021
@jzheaux jzheaux added for: stackoverflow A question that's better suited to stackoverflow.com and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jun 29, 2021
@geobreze
Copy link
Author

geobreze commented Jun 30, 2021

Thank you for your quick answer!

We're getting invalid_grant error from backend, so it should work for us!

But this will require upgrading from Spring Boot 2.2.8.RELEASE (Spring Security 5.2.5.RELEASE) to newer version. And latest Spring Security versions doesn't seem to be compatible with Spring Boot 2.2.8. So, I'm wondering, is there any chance, that we can use this behavior without upgrading Spring Boot now?

Also, I've created spring-boot-2.2.8 branch in demo repository.

Thank you!

@jzheaux
Copy link
Contributor

jzheaux commented Jun 30, 2021

The functionality is available in Security 5.3/Boot 2.3 - are you able to at least update to that? Note that Spring Boot 2.2 is scheduled to reach End-of-Life in July 2021.

Otherwise, I imagine that you could create a delegate implementation of OAuth2AuthorizedClientManager:

OAuth2AuthorizedClientManager wrapperManager = (request) -> {
    try {
        return clientManager.authorize(request);
    } catch (ClientAuthorizationException ex) {
        String registrationId = ex.getClientRegistrationId();
        String name = request.getPrincipal().getName();
        clientService.removeAuthorizedClient(registrationId, name);
    }
};

@geobreze
Copy link
Author

geobreze commented Jul 1, 2021

Thanks for the help!

We will schedule an update of our applications and use this approach until it's done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: stackoverflow A question that's better suited to stackoverflow.com
Projects
None yet
Development

No branches or pull requests

2 participants