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

Add support for requesting protected resources with RestClient #13588

Closed
Tracked by #15299
mjeffrey opened this issue Jul 27, 2023 · 45 comments · Fixed by #15437
Closed
Tracked by #15299

Add support for requesting protected resources with RestClient #13588

mjeffrey opened this issue Jul 27, 2023 · 45 comments · Fixed by #15437
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Milestone

Comments

@mjeffrey
Copy link

Expected Behavior
Allow the use RestClient (to be introduced in Spring 6.1) for blocking calls in a non reactive application In Oauth2 Client.
See https://spring.io/blog/2023/07/13/new-in-spring-6-1-restclient.

Current Behavior
Only WebClient is supported which means a lot of reactive dependencies are pulled in when using Oauth2 Client even in a blocking application.

Context
To avoid the dependency issue we are using RestTemplate (with interceptors) but would prefer to see a supported solution.

@mjeffrey mjeffrey added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Jul 27, 2023
@sjohnr
Copy link
Member

sjohnr commented Jul 27, 2023

@mjeffrey thanks for starting the conversation on this!

To clarify, are you specifically directing this issue toward providing support for RestClient in OAuth2 equivalent to the existing ServletOAuth2AuthorizedClientExchangeFilterFunction which supports WebClient?

@sjohnr sjohnr added 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 labels Jul 27, 2023
@mjeffrey
Copy link
Author

@sjohnr
I don't want to ask for too much related to implementation, you guys know this much better than me but I think what you said would do what is needed for us.

What I'd like to achieve is to be able to use the new RestClient with the Oauth2 client and to not need the reactive libraries.

In my company we are in the process of migrating a number of Spring Boot 2 (keycloak Oauth2 client) to Spring Boot 3 projects.
The Keycloak client no longer supports Spring Boot 3 and so we are moving to Spring Security Oauth2 client.
However the requirement to use WebClient is one of the things holding some teams back.

@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 28, 2023
@sjohnr sjohnr changed the title Support RestClient that will be released in Spring 6.1 Add support for requesting protected resources with RestClient Jul 28, 2023
@sjohnr sjohnr added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: feedback-provided Feedback has been provided labels Jul 28, 2023
@sjohnr
Copy link
Member

sjohnr commented Jul 28, 2023

Ok, I've changed the title of the issue to what I think aligns with the request as a starting point, and we can go from there. Since the only part of OAuth2 Client for a Servlet environment that directly uses reactive libraries is ServletOAuth2AuthorizedClientExchangeFilterFunction, this issue can be used to provide equivalent support.

@alexcrownus
Copy link

I was about to create a similar issue, glad someone already did. Many thanks @mjeffrey.

Since support for RestTemplate was dropped because it is now in maintenance mode, there should now be support for the new RestClient for Servlet applications without using WebClient in blocking mode.

@tudburley
Copy link

I am maintaining a library, which provides a blocking WebClient with OAuth2 client credentials support and also want to migrate to the new RestClient. At the moment the OAuth2 support in this library is built with ServerOAuth2AuthorizedClientExchangeFilterFunction.

Will there be also non-reactive equivalents for ServerOAuth2AuthorizedClientExchangeFilterFunction?

@sjohnr
Copy link
Member

sjohnr commented Aug 15, 2023

@tudburley we already have ServletOAuth2AuthorizedClientExchangeFilterFunction, is that what you're asking about? Otherwise, I'm not sure I understand the question. If you have a different enhancement suggestion than what is being discussed here, feel free to open a new issue with the details.

@tudburley
Copy link

tudburley commented Sep 14, 2023

@tudburley we already have ServletOAuth2AuthorizedClientExchangeFilterFunction, is that what you're asking about? Otherwise, I'm not sure I understand the question. If you have a different enhancement suggestion than what is being discussed here, feel free to open a new issue with the details.

Its hard to understand what is the difference of ServletOAuth2AuthorizedClientExchangeFilterFunction and ServerOAuth2AuthorizedClientExchangeFilterFunction. My guess is, that the servlet variant stores authorized clients in the servlet context or to the current servlet request attributes.

But I am maintaining a web client library, and I am confused why I should use a ServletOAuth2AuthorizedClientExchangeFilterFunction and coupling it to servlets. This client library should by used by any Spring Boot application, even non-servlet applications.

And even if I would switch to ServletOAuth2AuthorizedClientExchangeFilterFunction: This class also depends on reactive classes. So I wouldn't get rid off the reactive stuff.

@sjohnr
Copy link
Member

sjohnr commented Sep 15, 2023

Ah, thanks for clarifying @tudburley!

Its hard to understand what is the difference of ServletOAuth2AuthorizedClientExchangeFilterFunction and ServerOAuth2AuthorizedClientExchangeFilterFunction. My guess is, that the servlet variant stores authorized clients in the servlet context or to the current servlet request attributes.

Without some extra context, it is indeed confusing. The difference is in which components are utilized internally to process the request.

Spring Security (and Spring more widely) provides support for both reactive and non-reactive stacks and they have completely separate interfaces/implementations/etc. There's probably a better guide on the history and naming conventions, but typically "servlet" refers to the imperative stack and "reactive" obviously refers to the reactive stack. However, some components use the name "server" to also refer to the reactive stack.

It just so happens that both implementations can be plugged into a WebClient through the above-mentioned ExchangeFilterFunctions. You would use only the "Server" or "Servlet" variant for the stack your application is using. Hopefully that's enough context for now.

Will there be also non-reactive equivalents for ServerOAuth2AuthorizedClientExchangeFilterFunction?

I now understand your question. Such support would be possible only with this issue, through the use of a RestClient. So yes, but "only" not "also".

There is no planned support for using these underlying components with a RestTemplate, though the servlet components themselves currently use RestTemplate internally.

@rawadolb
Copy link

rawadolb commented Oct 19, 2023

Hi @sjohnr
i don't understand why this issue was closed.
What is the suggest solution to use RestClient to call protected resources with OAuth2 ?

for the moment i'm doing it in this way :

      @Bean("client-credentials")
      RestClient restClient(OAuth2AuthorizedClientManager authorizedClientManager,
                         ClientRegistrationRepository clientRegistrationRepository) {
      ClientRegistration clientRegistration = clientRegistrationRepository.findByRegistrationId("registration-id");
      ClientHttpRequestInterceptor oauthInterceptor = new OAuthClientCredentialsRestTemplateInterceptor(authorizedClientManager, clientRegistration);
       return RestClient.builder()
              .requestInterceptor(oauthInterceptor)
               .build();
   }

and in my inteceptor i call the authorization method to fetch a token and add it as a header

     @Override
    public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException {
        OAuth2AuthorizeRequest oAuth2AuthorizeRequest = OAuth2AuthorizeRequest
                .withClientRegistrationId(clientRegistration.getRegistrationId())
                .principal(principal)
                .build();
        OAuth2AuthorizedClient client = manager.authorize(oAuth2AuthorizeRequest);
        if (isNull(client)) {
            throw new IllegalStateException("client credentials flow on " + clientRegistration.getRegistrationId() + " failed, client is null");
        }
        System.out.println("Bearer " + client.getAccessToken().getTokenValue() +
                " - issued at : " + client.getAccessToken().getIssuedAt() +
                " - expired at : " + client.getAccessToken().getExpiresAt()
        );
        request.getHeaders().add(HttpHeaders.AUTHORIZATION, "Bearer " + client.getAccessToken().getTokenValue());
        return execution.execute(request, body);
    }

The question is why we need to do it manually ? it could be better if it is handled by spring?
it is not possible to add the bean ClientRegistration inject by spring oauth2-client directly to the RestClient object ?

Regards

@sjohnr
Copy link
Member

sjohnr commented Oct 19, 2023

@rawadolb

i don't understand why this issue was closed.

This issue is still open. Perhaps you saw that I closed gh-8882 which is a separate issue? We do still intend to add support for RestClient, but we don't have a timeline for when this support will be added yet.

@rawadolb
Copy link

@rawadolb

i don't understand why this issue was closed.

This issue is still open. Perhaps you saw that I closed gh-8882 which is a separate issue? We do still intend to add support for RestClient, but we don't have a timeline for when this support will be added yet.

my bad sorry :(

@bigunyak
Copy link

I'm also very much waiting when the new RestClient is supported, as this is what we want to switch to from the old RestTemplate.
Hope this issue can be prioritised. 🙏

@dev-praveen
Copy link

I am just curious and couldn't find equivalent bean configuration for newly introduced RestClient. Attaching the code snippet that can make call to oauth2 protected apis with the WebClient. Looking for similar with RestClient.
oauth2

@Nhattd97
Copy link

May I please have an update on this matter?

@ThomasKasene
Copy link
Contributor

ThomasKasene commented Jan 4, 2024

Making a custom interceptor is simple enough to do (see for example rawaldop's comment) but since Spring Security is a framework its components have to be able to function in a variety of different setups. For example, the reactive ServletOAuth2AuthorizedClientExchangeFilterFunction and ServerOAuth2AuthorizedClientExchangeFilterFunction allow the user to define some attributes courtesy of the ClientRequest's API, and then those attributes are later used by the *ExchangeFilterFunction and beyond:

webClient
     .get()
     .uri(uri)
     .attributes(clientRegistrationId("my-special-client-registration"))
     // ...
     .retrieve()
     .bodyToMono(String.class);

Neither RestClient nor ClientHttpRequest has an "attribute" concept as far as I've been able to find out. Does anyone have any notion as to how one might pass customizations to a ClientHttpRequestInterceptor on a per-request basis? Is it even possible to do it (cleanly) with the RestClient in its current state?

@mohmf
Copy link

mohmf commented Jan 8, 2024

Hi @sjohnr i don't understand why this issue was closed. What is the suggest solution to use RestClient to call protected resources with OAuth2 ?

for the moment i'm doing it in this way :

      @Bean("client-credentials")
      RestClient restClient(OAuth2AuthorizedClientManager authorizedClientManager,
                         ClientRegistrationRepository clientRegistrationRepository) {
      ClientRegistration clientRegistration = clientRegistrationRepository.findByRegistrationId("registration-id");
      ClientHttpRequestInterceptor oauthInterceptor = new OAuthClientCredentialsRestTemplateInterceptor(authorizedClientManager, clientRegistration);
       return RestClient.builder()
              .requestInterceptor(oauthInterceptor)
               .build();
   }

and in my inteceptor i call the authorization method to fetch a token and add it as a header

     @Override
    public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException {
        OAuth2AuthorizeRequest oAuth2AuthorizeRequest = OAuth2AuthorizeRequest
                .withClientRegistrationId(clientRegistration.getRegistrationId())
                .principal(principal)
                .build();
        OAuth2AuthorizedClient client = manager.authorize(oAuth2AuthorizeRequest);
        if (isNull(client)) {
            throw new IllegalStateException("client credentials flow on " + clientRegistration.getRegistrationId() + " failed, client is null");
        }
        System.out.println("Bearer " + client.getAccessToken().getTokenValue() +
                " - issued at : " + client.getAccessToken().getIssuedAt() +
                " - expired at : " + client.getAccessToken().getExpiresAt()
        );
        request.getHeaders().add(HttpHeaders.AUTHORIZATION, "Bearer " + client.getAccessToken().getTokenValue());
        return execution.execute(request, body);
    }

The question is why we need to do it manually ? it could be better if it is handled by spring? it is not possible to add the bean ClientRegistration inject by spring oauth2-client directly to the RestClient object ?

Regards

Could you provide the full implementation for this please?

@ThomasKasene
Copy link
Contributor

@mohmf I do believe that is the full implementation, except they've left out the interceptor's constructor and the definition of the principal. For a normal client_credentials flow this can perhaps be as simple as using the client ID:

public class OAuthClientCredentialsRestTemplateInterceptor implements ClientHttpRequestInterceptor {

    private final OAuth2AuthorizedClientManager authorizedClientManager;
    private final ClientRegistration clientRegistration;

    public OAuthClientCredentialsRestTemplateInterceptor(
            OAuth2AuthorizedClientManager authorizedClientManager, ClientRegistration clientRegistration) {
        this.authorizedClientManager = authorizedClientManager;
        this.clientRegistration = clientRegistration;
    }

    @Override
    public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException {
        var clientRegistrationId = this.clientRegistration.getRegistrationId();
        var principal = this.clientRegistration.getClientId();
        var authorizeRequest = OAuth2AuthorizeRequest
                .withClientRegistrationId(clientRegistrationId)
                .principal(principal)
                .build();
        var authorizedClient = manager.authorize(authorizeRequest);
        if (authorizedClient == null) {
            throw new IllegalStateException("client credentials flow on " + clientRegistrationId + " failed, client is null");
        }
        request.setBearerAuth(client.getAccessToken().getTokenValue());
        return execution.execute(request, body);
    }
}

@ThomasVitale
Copy link
Contributor

@sjohnr I really like the design of this final proposal, especially the fact that it requires one single interface and that it re-uses the familiar concept of Interceptor (and consistent with BasicAuthenticationInterceptor).

@mcordeiro73
Copy link

@sjohnr Your Interceptor is almost identical to what I did.

One other thing we created was an ErrorHandler that will remove the authorized client when an authorization error occurs. We interact with some 3rd party services that will revoke a token before it expires, so we have a need to ensure when that occurs we remove the authorize client so the next attempt will result in a new token being requested.

Hers is what we did for an ErrorHandler:

public class RemoveAuthorizedClientOAuth2ResponseErrorHandler extends DefaultResponseErrorHandler {

	private static final Predicate<HttpStatusCode> STATUS_PREDICATE = httpStatusCode -> httpStatusCode.value() == HttpStatus.UNAUTHORIZED.value();

	private final OAuth2AuthorizedClientService oauth2AuthorizedClientService;
	private final ClientRegistration clientRegistration;

	public RemoveAuthorizedClientOAuth2ResponseErrorHandler(@NotNull OAuth2AuthorizedClientService oauth2AuthorizedClientService, @NotNull ClientRegistration clientRegistration) {
		this.oauth2AuthorizedClientService = oauth2AuthorizedClientService;
		this.clientRegistration = clientRegistration;
	}

	@Override
	public void handleError(ClientHttpResponse clientHttpResponse) throws IOException {
		if (STATUS_PREDICATE.test(clientHttpResponse.getStatusCode())) {
			oauth2AuthorizedClientService.removeAuthorizedClient(clientRegistration.getRegistrationId(), clientRegistration.getClientId());
		}
		super.handleError(clientHttpResponse);
	}

	public static RestClient.ResponseSpec.ErrorHandler createErrorHandler(@NotNull OAuth2AuthorizedClientService oauth2AuthorizedClientService, @NotNull ClientRegistration clientRegistration) {
		ResponseErrorHandler responseErrorHandler = new RemoveAuthorizedClientOAuth2ResponseErrorHandler(oauth2AuthorizedClientService, clientRegistration);
		return (request, response) -> responseErrorHandler.handleError(response);
	}

	public static Predicate<HttpStatusCode> unauthorizedStatusPredicate() {
		return STATUS_PREDICATE;
	}

}

With my RestClient config then looking like:

@Bean
protected RestClient restClient(RestClient.Builder builder, ClientRegistrationRepository clientRegistrationRepository, OAuth2AuthorizedClientService oauth2AuthorizedClientService) {
	OAuth2AuthorizedClientManager authorizedClientManager = new AuthorizedClientServiceOAuth2AuthorizedClientManager(clientRegistrationRepository, oauth2AuthorizedClientService);
	ClientRegistration clientRegistration = clientRegistrationRepository.findByRegistrationId(CLIENT_REGISTRATION_ID);

	return builder.requestInterceptor(new Oauth2ClientHttpRequestInterceptor(authorizedClientManager, clientRegistration))
			.defaultStatusHandler(RemoveAuthorizedClientOAuth2ResponseErrorHandler.unauthorizedStatusPredicate(), RemoveAuthorizedClientOAuth2ResponseErrorHandler.createErrorHandler(authorizedClientService, clientRegistration))
			.build();
}

This is by no means a perfect solution as it essentially requires the use of the DefaultResponseErrorHandler to handle other errors. I may have been better served to provide my ErrorHandler a delegate ResponseErrorHandler instead.

Removing the authorized client is one thing that used to be automatic with Resttemplate that was lost when moving to WebClient (though there were ways to ensure it was in place) and then to RestClient. Another thing lost was the automatic retry of a request with a new token after an authorization failure (via OAuth2RestTemplate). My hope is that we can bring both these features back to the core Spring libraries as they were very useful in certain situations.

@sjohnr sjohnr moved this from Planning to In Progress in Spring Security Team May 14, 2024
@goafabric

This comment was marked as off-topic.

sjohnr added a commit to sjohnr/spring-security that referenced this issue May 21, 2024
@sjohnr
Copy link
Member

sjohnr commented May 21, 2024

Thanks for the feedback @mcordeiro73! The failure handling is what I wanted to work on next, which I have added in this version. I have further simplified things by dropping support for a separate OAuth2AuthorizedClient, in favor of a single interceptor that accepts only a clientRegistrationId. This makes the usage pattern very simple, and we can evaluate whether we need anything further via feedback from users. This commit contains the latest version.

The per-application configuration looks like this (with only a new setter added):

@Bean
public RestClient restClient(OAuth2AuthorizedClientManager authorizedClientManager,
		OAuth2AuthorizedClientRepository authorizedClientRepository) {

	OAuth2ClientHttpRequestInterceptor requestInterceptor =
		new OAuth2ClientHttpRequestInterceptor(authorizedClientManager, CLIENT_REGISTRATION_ID);

	// Configure the interceptor to remove invalid authorized clients
	requestInterceptor.setAuthorizedClientRepository(authorizedClientRepository);

	return RestClient.builder()
		.requestInterceptor(requestInterceptor)
		.build();
}

and a per-request arrangement might look like this:

@GetMapping("/cashcards")
public Cashcard[] cashcards() {
	OAuth2ClientHttpRequestInterceptor requestInterceptor =
		new OAuth2ClientHttpRequestInterceptor(this.authorizedClientManager, CLIENT_REGISTRATION_ID);

	// Configure the interceptor to remove invalid authorized clients
	requestInterceptor.setAuthorizedClientRepository(this.authorizedClientRepository);

	return this.restClient.get()
		.uri("http://localhost:8090/cashcards")
		.httpRequest(requestInterceptor.httpRequest())
		.retrieve()
		.onStatus(requestInterceptor.errorHandler())
		.body(Cashcard[].class);
}

In the per-request case, we are taking a generic (globally configured) RestClient and adding a Consumer<ClientHttpRequest> via requestInterceptor.httpRequest() and a ResponseErrorHandler via requestInterceptor.errorHandler() on a request-by-request basis.

This is necessary since we don't have the ability to intercept the request so we have to specify an error handler separately. The error handler is optional if removal of the authorized client isn't required.

@ithander

This comment was marked as off-topic.

@barbetb
Copy link

barbetb commented Jun 21, 2024

I like this proposed oauth interceptor. Will it be possible to have the interceptor only work for specific endpoints?

What is the eta for this issue?

@sjohnr sjohnr added this to the 6.4.x milestone Jun 21, 2024
@sjohnr
Copy link
Member

sjohnr commented Jun 21, 2024

@barbetb thanks for your interest in this issue!

Will it be possible to have the interceptor only work for specific endpoints?

Yes. See the per-request arrangement example in my above comment.

What is the eta for this issue?

I've added this issue to the 6.4.x bucket. I'm currently looking to make this available for testing and gathering feedback in one of the 6.4 milestones, ideally 6.4.0-M1 which will be released in July. If it makes it, 6.4.0 will be released in November.

sjohnr added a commit to sjohnr/spring-security that referenced this issue Jul 2, 2024
sjohnr added a commit to sjohnr/spring-security that referenced this issue Jul 17, 2024
sjohnr added a commit to sjohnr/spring-security that referenced this issue Aug 7, 2024
sjohnr added a commit to sjohnr/spring-security that referenced this issue Aug 16, 2024
@sjohnr sjohnr added the status: duplicate A duplicate of another issue label Aug 16, 2024
@sjohnr sjohnr modified the milestones: 6.4.x, 6.4.0-M2 Aug 16, 2024
@sjohnr sjohnr moved this from In Progress to Done in Spring Security Team Aug 16, 2024
@sjohnr sjohnr closed this as completed in e3c19ba Aug 16, 2024
@azizabah
Copy link

@sjohnr - Thanks for the work on this one. I think this solves one of the big use cases for OAuth and RestClient however I think it misses the other one. In situations where our service is called with a token and we want to pass that token on to subsequent calls we have historically built out a WebClient bean like this

 @Bean
  public WebClient profileServiceWebClient(WebClient.Builder webClientBuilder) {
    return webClientBuilder.filter(new ServletBearerExchangeFilterFunction()).baseUrl(profileServiceBaseUrl)
        .defaultHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE).build();
  }

and taken advantage of the ServletBearerExchangeFilterFunction to automatically get the user's token and pass that on for subsequent calls.

Given the title of this issue (after the rename), I would have expected it to also solve this use case (unless it has and I'm missing something).

@sjohnr
Copy link
Member

sjohnr commented Sep 17, 2024

@azizabah thanks for mentioning this. This issue is specifically for the OAuth2 Client use case, though I understand why you feel the title is generic enough it ought to apply to OAuth2 Resource Server. Would you please consider opening a new issue for your use case? I would be happy to take a look at what it would take.

@azizabah
Copy link

@sjohnr - #15820 added per your request.

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: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.