Skip to content
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

WebClient support should get new access token when expired and password #8831

Closed
FilipKittnar opened this issue Jul 14, 2020 · 9 comments
Closed
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: invalid An issue that we don't feel is valid

Comments

@FilipKittnar
Copy link

Describe the bug
I know this has been addressed and fixed before for client_credentials grant type: #5893 . Would it be possible to do the same for password grant type? I need to use the password grant type and it works but after 30 minutes the token expires and Spring Security does nothing about it and the API stops working and keeps returning 403 until I restart the whole application. Refresh token doesn't help because after that one expires, it just crashes on the expired refresh token and again, the API stops working until restart. I'm pretty sure that standard behavior would be to obtain new access token once it expires. When I switch to client_credentials grant type it works perfectly, but as I said, this grant type will be forbidden for me on production.

To Reproduce
application.yml:

spring:
  security:
    oauth2:
      resourceserver:
        jwt:
          issuer-uri: https://our.idp.keycloak.host/auth/realms/firstrealm
      client:
        registration:
          my-client-authorization:
            client-id: my_client
            client-secret: ${CLIENT_SECRET}
            authorization-grant-type: password
            scope: openid, profile
        provider:
          my-client-authorization:
            token-uri: https://our.idp.keycloak.host/auth/realms/secondrealm/protocol/openid-connect/token

MyClientConfig.java:

import lombok.RequiredArgsConstructor;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Profile;
import org.springframework.security.oauth2.client.OAuth2AuthorizationContext;
import org.springframework.security.oauth2.client.OAuth2AuthorizedClientManager;
import org.springframework.security.oauth2.client.OAuth2AuthorizedClientProvider;
import org.springframework.security.oauth2.client.OAuth2AuthorizedClientProviderBuilder;
import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
import org.springframework.security.oauth2.client.web.DefaultOAuth2AuthorizedClientManager;
import org.springframework.security.oauth2.client.web.OAuth2AuthorizedClientRepository;
import org.springframework.security.oauth2.client.web.reactive.function.client.ServletOAuth2AuthorizedClientExchangeFilterFunction;
import org.springframework.web.reactive.function.client.WebClient;

import java.util.Map;

@Configuration
@RequiredArgsConstructor
public class MyClientConfig {
    @Bean
    WebClient webClient(OAuth2AuthorizedClientManager authorizedClientManager) {
        ServletOAuth2AuthorizedClientExchangeFilterFunction oauth2Client = new ServletOAuth2AuthorizedClientExchangeFilterFunction(authorizedClientManager);
        oauth2Client.setDefaultClientRegistrationId("my-client-authorization");
        return WebClient.builder()
                .apply(oauth2Client.oauth2Configuration())
                .baseUrl("https://the.api.host.to.call")
                .build();
    }

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

        DefaultOAuth2AuthorizedClientManager result = new DefaultOAuth2AuthorizedClientManager(
                clientRegistrationRepository,
                authorizedClientRepository
        );

        result.setAuthorizedClientProvider(authorizedClientProvider);
        result.setContextAttributesMapper(oAuth2AuthorizeRequest -> Map.of(
                OAuth2AuthorizationContext.USERNAME_ATTRIBUTE_NAME, "user",
                OAuth2AuthorizationContext.PASSWORD_ATTRIBUTE_NAME, "password"
        ));

        return result;
    }
}

The API call itself:

private <T> T callApi(Function<UriBuilder, URI> uriFunction, Class<T> resultType) {
    return this.webClient
            .get()
            .uri(uriFunction)
            .retrieve()
            .bodyToMono(resultType)
            .block();
}

Expected behavior
Get a new access token just before the expiration of the old one.

@FilipKittnar FilipKittnar added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jul 14, 2020
@jgrandja jgrandja self-assigned this Jul 14, 2020
@jgrandja
Copy link
Contributor

@FilipKittnar What I suspect is happening here is that a refresh token is returned on the initial call and associated to the OAuth2AuthorizedClient. Because you didn't configure refreshToken() via the builder:

OAuth2AuthorizedClientProvider authorizedClientProvider = OAuth2AuthorizedClientProviderBuilder.builder()
                .password()
                .build();

It won't refresh the expired token because the refresh_token OAuth2AuthorizedClientProvider is not configured.

I would try adding refreshToken() via the builder and that should resolve it.

Regarding...

Refresh token doesn't help because after that one expires, it just crashes on the expired refresh token and again

When the refresh_token grant fails because it's expired then the OAuth2AuthorizedClient should be removed from the OAuth2AuthorizedClientRepository, which will force the client to go through the password grant from the start. This was introduced in #7840 via RemoveAuthorizedClientOAuth2AuthorizationFailureHandler associated to the DefaultOAuth2AuthorizedClientManager.

Give these 2 options a try and let me know if the issue is resolved.

@jgrandja jgrandja added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jul 14, 2020
@FilipKittnar
Copy link
Author

@jgrandja Hello, thanks a lot for your reply. Yes, I know, that the builder does not have a refreshToken configured. I was experimenting with that as well, it just wasn't in this snipped I posted here.
What's more interesting is your suggestion about OAuth2AuthorizationFailureHandler. That sounds very promising. However, I have no clue how to do this. Is this described somewhere? Is there any example? I tried to search, but did not find anything.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 15, 2020
@jgrandja
Copy link
Contributor

jgrandja commented Jul 15, 2020

@FilipKittnar OAuth2AuthorizationFailureHandler was introduced in 5.3.0 release so all you need to do is upgrade to 5.3+ and you will automatically inherit the new behaviour. OAuth2AuthorizationSuccessHandler and OAuth2AuthorizationFailureHandler is documented in the reference.

@jgrandja jgrandja added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 15, 2020
@FilipKittnar
Copy link
Author

Thanks for the tip. However, I'm still not totally sure from the documentation how to do it. Here's what I tried:

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.oauth2.client.OAuth2AuthorizationContext;
import org.springframework.security.oauth2.client.OAuth2AuthorizationFailureHandler;
import org.springframework.security.oauth2.client.OAuth2AuthorizedClientManager;
import org.springframework.security.oauth2.client.OAuth2AuthorizedClientProvider;
import org.springframework.security.oauth2.client.OAuth2AuthorizedClientProviderBuilder;
import org.springframework.security.oauth2.client.RemoveAuthorizedClientOAuth2AuthorizationFailureHandler;
import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
import org.springframework.security.oauth2.client.web.DefaultOAuth2AuthorizedClientManager;
import org.springframework.security.oauth2.client.web.OAuth2AuthorizedClientRepository;
import org.springframework.security.oauth2.client.web.reactive.function.client.ServletOAuth2AuthorizedClientExchangeFilterFunction;
import org.springframework.web.reactive.function.client.WebClient;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.util.Map;


@Configuration
@RequiredArgsConstructor
@Slf4j
public class MyClientConfig {
    @Bean
    WebClient webClient(OAuth2AuthorizedClientManager authorizedClientManager) {
        ServletOAuth2AuthorizedClientExchangeFilterFunction oauth2Client = new ServletOAuth2AuthorizedClientExchangeFilterFunction(authorizedClientManager);
        oauth2Client.setDefaultClientRegistrationId("my-client-authorization");
        return WebClient.builder()
                .apply(oauth2Client.oauth2Configuration())
                .baseUrl("https://the.api.host.to.call")
                .build();
    }

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

        DefaultOAuth2AuthorizedClientManager result = new DefaultOAuth2AuthorizedClientManager(
                clientRegistrationRepository,
                authorizedClientRepository
        );

        result.setAuthorizationFailureHandler(authorizationFailureHandler(authorizedClientRepository));
        result.setAuthorizedClientProvider(authorizedClientProvider);
        result.setContextAttributesMapper(oAuth2AuthorizeRequest -> Map.of(
                OAuth2AuthorizationContext.USERNAME_ATTRIBUTE_NAME, "user",
                OAuth2AuthorizationContext.PASSWORD_ATTRIBUTE_NAME, "password"
        ));

        return result;
    }

    @Bean
    public OAuth2AuthorizationFailureHandler authorizationFailureHandler(OAuth2AuthorizedClientRepository authorizedClientRepository) {
        return new RemoveAuthorizedClientOAuth2AuthorizationFailureHandler((clientRegistrationId, principal, attributes) -> {
            log.info("Removing authorized client");
            authorizedClientRepository.removeAuthorizedClient(clientRegistrationId, principal, (HttpServletRequest) attributes.get(HttpServletRequest.class.getName()), (HttpServletResponse) attributes.get(HttpServletResponse.class.getName()));
            log.info("Removed authorized client");
        });
    }
}

And here's, what heppened after I called the API after 30 minutes:

2020-07-15 15:16:21.748  INFO 4780 --- [oundedElastic-2] MyClientConfig      : Removing authorized client
2020-07-15 15:16:21.749  INFO 4780 --- [oundedElastic-2] MyClientConfig      : Removed authorized client

So it initialized the handler and did the removal. But then:

2020-07-15 15:16:21.791 ERROR 4780 --- [nio-8080-exec-4] o.a.c.c.C.[.[.[.[dispatcherServlet]      : Servlet.service() for servlet [dispatcherServlet] in context with path [/fup] threw exception [Request processing failed; nested exception is org.springframework.security.oauth2.client.ClientAuthorizationException: [invalid_grant] Refresh token expired] with root cause

org.springframework.security.oauth2.core.OAuth2AuthorizationException: [invalid_grant] Refresh token expired
	at org.springframework.security.oauth2.client.http.OAuth2ErrorResponseErrorHandler.handleError(OAuth2ErrorResponseErrorHandler.java:62) ~[spring-security-oauth2-client-5.3.2.RELEASE.jar:5.3.2.RELEASE]
	at org.springframework.web.client.ResponseErrorHandler.handleError(ResponseErrorHandler.java:63) ~[spring-web-5.2.6.RELEASE.jar:5.2.6.RELEASE]
	at org.springframework.web.client.RestTemplate.handleResponse(RestTemplate.java:782) ~[spring-web-5.2.6.RELEASE.jar:5.2.6.RELEASE]
	at org.springframework.web.client.RestTemplate.doExecute(RestTemplate.java:740) ~[spring-web-5.2.6.RELEASE.jar:5.2.6.RELEASE]
	at org.springframework.web.client.RestTemplate.exchange(RestTemplate.java:641) ~[spring-web-5.2.6.RELEASE.jar:5.2.6.RELEASE]
	at org.springframework.security.oauth2.client.endpoint.DefaultRefreshTokenTokenResponseClient.getTokenResponse(DefaultRefreshTokenTokenResponseClient.java:74) ~[spring-security-oauth2-client-5.3.2.RELEASE.jar:5.3.2.RELEASE]
	at org.springframework.security.oauth2.client.endpoint.DefaultRefreshTokenTokenResponseClient.getTokenResponse(DefaultRefreshTokenTokenResponseClient.java:51) ~[spring-security-oauth2-client-5.3.2.RELEASE.jar:5.3.2.RELEASE]
	at org.springframework.security.oauth2.client.RefreshTokenOAuth2AuthorizedClientProvider.authorize(RefreshTokenOAuth2AuthorizedClientProvider.java:93) ~[spring-security-oauth2-client-5.3.2.RELEASE.jar:5.3.2.RELEASE]
	at org.springframework.security.oauth2.client.DelegatingOAuth2AuthorizedClientProvider.authorize(DelegatingOAuth2AuthorizedClientProvider.java:67) ~[spring-security-oauth2-client-5.3.2.RELEASE.jar:5.3.2.RELEASE]
	at org.springframework.security.oauth2.client.web.DefaultOAuth2AuthorizedClientManager.authorize(DefaultOAuth2AuthorizedClientManager.java:165) ~[spring-security-oauth2-client-5.3.2.RELEASE.jar:5.3.2.RELEASE]
	at org.springframework.security.oauth2.client.web.reactive.function.client.ServletOAuth2AuthorizedClientExchangeFilterFunction.lambda$authorizeClient$24(ServletOAuth2AuthorizedClientExchangeFilterFunction.java:534) ~[spring-security-oauth2-client-5.3.2.RELEASE.jar:5.3.2.RELEASE]
	at reactor.core.publisher.MonoSupplier.call(MonoSupplier.java:85) ~[reactor-core-3.3.5.RELEASE.jar:3.3.5.RELEASE]
	at reactor.core.publisher.FluxSubscribeOnCallable$CallableSubscribeOnSubscription.run(FluxSubscribeOnCallable.java:225) ~[reactor-core-3.3.5.RELEASE.jar:3.3.5.RELEASE]
	at reactor.core.scheduler.SchedulerTask.call(SchedulerTask.java:68) ~[reactor-core-3.3.5.RELEASE.jar:3.3.5.RELEASE]
	at reactor.core.scheduler.SchedulerTask.call(SchedulerTask.java:28) ~[reactor-core-3.3.5.RELEASE.jar:3.3.5.RELEASE]
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) ~[na:na]
	at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) ~[na:na]
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130) ~[na:na]
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630) ~[na:na]
	at java.base/java.lang.Thread.run(Thread.java:832) ~[na:na]

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 15, 2020
@jgrandja
Copy link
Contributor

jgrandja commented Jul 15, 2020

@FilipKittnar DefaultOAuth2AuthorizedClientManager is initialized with RemoveAuthorizedClientOAuth2AuthorizationFailureHandler as the default so no need to configure on your end.

Based on your stacktrace, it looks like the behaviour is correct as the OAuth2AuthorizedClient is removed. The last call fails when the refresh token is expired but if you trigger the flow again it will authorize the client once again since the previous OAuth2AuthorizedClient was removed.

@jgrandja jgrandja added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 15, 2020
@FilipKittnar
Copy link
Author

Oh, I get it! So when I call the API again after the refresh token expires, it will actually re-authenticate and get a new access token. So the actual recommended solution is to wrap the call in try-catch and if we get "Refresh token expired", we call the API again? Seems a little bit clunky, but it gets the job done.
Thanks a ton for your help!

@jgrandja jgrandja added status: invalid An issue that we don't feel is valid and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 15, 2020
@Saljack
Copy link

Saljack commented Feb 25, 2021

Just addition for others. If you use

ServletOAuth2AuthorizedClientExchangeFilterFunction(OAuth2AuthorizedClientManager authorizedClientManager);

then there is a note in javadoc:

When this constructor is used, authentication (HTTP 401) and authorization (HTTP 403) failures returned from an OAuth 2.0 Resource Server will NOT be forwarded to an OAuth2AuthorizationFailureHandler. Therefore, future requests to the Resource Server will most likely use the same (likely invalid) token, resulting in the same errors returned from the Resource Server. It is recommended to configure a RemoveAuthorizedClientOAuth2AuthorizationFailureHandler via setAuthorizationFailureHandler(OAuth2AuthorizationFailureHandler) so that authentication and authorization failures returned from a Resource Server will result in removing the authorized client, so that a new token is retrieved for future requests.

And here is default implemenation of RemoveAuthorizedClientOAuth2AuthorizationFailureHandler it is same in both ServletOAuth2AuthorizedClientExchangeFilterFunction and DefaultOAuth2AuthorizedClientManager

OAuth2AuthorizationFailureHandler authorizationFailureHandler =
		new RemoveAuthorizedClientOAuth2AuthorizationFailureHandler(
				(clientRegistrationId, principal, attributes) ->
						authorizedClientRepository.removeAuthorizedClient(clientRegistrationId, principal,
								(HttpServletRequest) attributes.get(HttpServletRequest.class.getName()),
								(HttpServletResponse) attributes.get(HttpServletResponse.class.getName())));

So I recommend to use another constructor

ServletOAuth2AuthorizedClientExchangeFilterFunction(ClientRegistrationRepository clientRegistrationRepository, OAuth2AuthorizedClientRepository authorizedClientRepository)

which creates one RemoveAuthorizedClientOAuth2AuthorizationFailureHandler and use it in DefaultOAuth2AuthorizedClientManager and ServletOAuth2AuthorizedClientExchangeFilterFunction as a clientResponseHandler.
It would help if there would be a method OAuth2AuthorizationFailureHandler getAuthorizationFailureHandler() in DefaultOAuth2AuthorizedClientManager and then you would be able to simply do:

ServletOAuth2AuthorizedClientExchangeFilterFunction servletOAuth2AuthorizedClientExchangeFilterFunction = new ServletOAuth2AuthorizedClientExchangeFilterFunction(defaultOAuth2AuthorizedClientManager);
servletOAuth2AuthorizedClientExchangeFilterFunction.setAuthorizationFailureHandler(defaultOAuth2AuthorizedClientManager.getAuthorizationFailureHandler());

and create a new default class DefaultRemoveAuthorizedClientOAuth2AuthorizationFailureHandler with this implementation:

new RemoveAuthorizedClientOAuth2AuthorizationFailureHandler(
						(clientRegistrationId, principal, attributes) ->
								authorizedClientRepository.removeAuthorizedClient(clientRegistrationId, principal,
										(HttpServletRequest) attributes.get(HttpServletRequest.class.getName()),
										(HttpServletResponse) attributes.get(HttpServletResponse.class.getName())));

@yasammez
Copy link

yasammez commented Apr 28, 2021

The behaviour of .refreshToken() is not what I would have suspected. My idea: when making an API call, the WebClient checks if the access token has expired and if so, requests a new one using the refresh token. WebClient's idea: don't check the time, just always request a new access token. I have to make requests every second, which now hammers my Keycloak with unnecessary load :(

Edit: turns out this is actually not the case. I just tested with a token lifespan of one minute and this is the default clock skew in RefreshTokenReactiveOAuth2AuthorizedClientProvider. It would be nice if documentation mentioned this somewhere.

Also, the "wrap the call into a try/catch and try again when the refresh token has expired" solution is not pretty, as it has to go to every call site or maybe a hand-written filter if that is possible. However, if I omit it, there will be an exception in my logfiles every 14 hours (our refresh token lifespan).

Edit: this turned out to be easier than I though as well: WebClient.builder().filter((request, next) -> next.exchange(request).retry(1L)).build()

@Xiphoseer
Copy link

Sorry to dig up this old ticket, but I think there is an inconsistency in the assumption made by

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 RefreshTokenReactiveOAuth2AuthorizedClientProvider to
// handle the refresh
return Mono.empty();
}

This piece of code assumes that there is a DelegatingReactiveOAuth2AuthorizedClientProvider instance wrapping this PasswordReactiveOAuth2AuthorizedClientProvider, with a sibling RefreshTokenReactiveOAuth2AuthorizedClientProvider that will try to get a new access token with the refresh_token and either succeed (and get an AuthorizedClient) or fail (and trigger the RemoveAuthorizedClientReactiveOAuth2AuthorizationFailureHandler which will then make the ExchangeFilterFunctions reauthenticate.

But that assumption is invalid in the general case, because there may be no call to refreshToken() on the builder or the ClientProvider is used directly with the manager. Can this be fixed by making the PasswordReactiveOAuth2AuthorizedClientProvider start out assuming (e.g. via a boolean field) that there is no refresh_token mechanism and then having ReactiveOAuth2AuthorizedClientProviderBuilder::build override that flag if there is? And then add that as an additional requirement for returning Mono.empty() in the code linked above?

Or, if you compare it to ClientCredentialsReactiveOAuth2AuthorizedClientProvider, that one doesn't have this case at all, so maybe it can be removed? Which makes me wonder whether the order of the elements returned by this.builders.values() in ReactiveOAuth2AuthorizedClientProviderBuilder::build is load bearing i.e. doesn't ever use the refresh_token mechanism if RefreshTokenReactiveOAuth2AuthorizedClientProvider ends up after ClientCredentialsReactiveOAuth2AuthorizedClientProvider in the list of authorizedClientProviders.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

6 participants