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

Provide Servlet equivalent of UnAuthenticatedServerOAuth2AuthorizedClientRepository #6683

Closed
DarrenForsythe opened this issue Mar 26, 2019 · 18 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Milestone

Comments

@DarrenForsythe
Copy link
Contributor

Summary

Currently when Using ServletOAuth2AuthorizedClientExchangeFilterFunction (this might also apply to the Server impl) this requires a OAuth2AuthorizedClientRepository`.

This work fine when there is a some sort of request in progress however when calling to OAuth2 secured resources when no web request is in progress each will fail.

A user can provide their own no-op impl such as,

var oauth2 = new ServletOAuth2AuthorizedClientExchangeFilterFunction(clientRegistrationRepo,
                     new OAuth2AuthorizedClientRepository() {
                         @Override
                         public <T extends OAuth2AuthorizedClient> T loadAuthorizedClient(String s,
                                 Authentication authentication, HttpServletRequest httpServletRequest) {
                             return null;
                         }

                         @Override
                         public void saveAuthorizedClient(OAuth2AuthorizedClient oAuth2AuthorizedClient,
                                 Authentication authentication, HttpServletRequest httpServletRequest,
                                 HttpServletResponse httpServletResponse) {

                         }

                         @Override
                         public void removeAuthorizedClient(String s, Authentication authentication,
                                 HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) {

                         }
                     });

However this doesn't lend itself to be easily undersood why, and any use use to call OAuth2 secured resources without an HttpServlet Request/Response will need it. Across mutiple apps this can get messy.

Actual Behavior

Provide no-op implementations of the OAuth2AuthorizedClientRepository to allow easier discovery/documentation for use cases that exist for calling secured OAuth2 resources outside of a web request.

Expected Behavior

OAuth2 Resource call does not require users to impl their own no-op impl. Spring Security provides class to allow easier documentation of use cases for no-op impls. .

Configuration

Example failure impl,

   @Bean
    public ApplicationRunner test(WebClient.Builder builder,
            ClientRegistrationRepository clientRegistrationRepo, OAuth2AuthorizedClientRepository authorizedClient) {
        return args -> {

            try {
                var oauth2 = new ServletOAuth2AuthorizedClientExchangeFilterFunction(clientRegistrationRepo,
                        authorizedClient);
                oauth2.setDefaultClientRegistrationId("test");
                var response = builder.apply(oauth2.oauth2Configuration()).build().get()
                        .uri("test").retrieve()
                        .bodyToMono(String.class).block();
                log.info("Response - {}", response);
            } catch (Exception e) {
                log.error("Failed to call test.", e);
            }

        };
    }

Working impl,

   @Bean
    public ApplicationRunner test(WebClient.Builder builder,
            ClientRegistrationRepository clientRegistrationRepo, OAuth2AuthorizedClientRepository authorizedClient) {
        return args -> {

            try {
                       var oauth2 = new ServletOAuth2AuthorizedClientExchangeFilterFunction(clientRegistrationRepo,
                        new OAuth2AuthorizedClientRepository() {
                            @Override
                            public <T extends OAuth2AuthorizedClient> T loadAuthorizedClient(String s,
                                    Authentication authentication, HttpServletRequest httpServletRequest) {
                                return null;
                            }

                            @Override
                            public void saveAuthorizedClient(OAuth2AuthorizedClient oAuth2AuthorizedClient,
                                    Authentication authentication, HttpServletRequest httpServletRequest,
                                    HttpServletResponse httpServletResponse) {

                            }

                            @Override
                            public void removeAuthorizedClient(String s, Authentication authentication,
                                    HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) {

                            }
                        });
                oauth2.setDefaultClientRegistrationId("test");
                var response = builder.apply(oauth2.oauth2Configuration()).build().get()
                        .uri("test").retrieve()
                        .bodyToMono(String.class).block();
                log.info("Response - {}", response);
            } catch (Exception e) {
                log.error("Failed to call test.", e);
            }

        };
    }

Version

Spring Boot 2.1.3, Spring Security 5.1.3

Sample

@jzheaux
Copy link
Contributor

jzheaux commented Mar 26, 2019

@DarrenForsythe thanks for the report.

Have you already tried UnAuthenticatedServerOAuth2AuthorizedClientRepository?

@DarrenForsythe
Copy link
Contributor Author

DarrenForsythe commented Mar 26, 2019

@jzheaux That appears only to be applicable for fully reactive stacks unless I'm mistaken. Currently Using WebClient within a spring-web/servlet stack, no Reactive Client repository beans etc. would be available?

Looking at that class, perhaps the ask of this issue is an implementation of that class for servlet implementing the OAuth2AuthorizedClientRepository?

@jzheaux
Copy link
Contributor

jzheaux commented Mar 26, 2019

Thanks, @DarrenForsythe, it's a good question. Usually, we recommend the reactive stack for non-webapps, so it's not clear to me yet whether a servlet version of that class makes sense. What can you tell me about your scenario so that I can understand it better?

Are you trying to make a call from a background thread, from a CLI, or something else?

@DarrenForsythe
Copy link
Contributor Author

Background thread that runs attestation processing for a pub/sub flow. Also a few API exposed within in the application for data access. I've seen quite a few uses cases, working in a large enterprise environment, like this (background threads in a servlet stack) where it simply isn't feasible to switch the apps to a reactive stack, but want to take advantage of the new features currently and coming to WebClient.

@DarrenForsythe
Copy link
Contributor Author

To add, I think this is a minor blocker to more enterprise companies picking up Spring 5 easily. I've seen enough Spring Boot applications that the usage of Oauth2Resttemplate can just be used anywhere without fail, developers seem to be expecting the same experience between Servlet/Reactive.

I wouldn't expect WebClient to be as easily transferable but lack of a Spring Security class to expose a no-op/anon client may lead companies to add their own (and potentially wrong) implementations.

I'd be happy to provide a PR for it if the Spring Security team deem it valuable to have and by proxy maintain

@jgrandja
Copy link
Contributor

jgrandja commented Apr 1, 2019

@DarrenForsythe I'm not in favour of adding a no-op OAuth2AuthorizedClientRepository as it simply does not provide any value to the framework. Also, I don't see how the Working Impl you provided even works?

You have oauth2.setDefaultClientRegistrationId("test"), which would attempt to lookup the "test" client registration in ClientRegistrationRepository and than the associated OAuth2AuthorizedClient in OAuth2AuthorizedClientRepository right after that. However, a no-op OAuth2AuthorizedClientRepository would return null and therefore no access token would be passed in the request and would fail with 401. Where is the OAuth2AuthorizedClient being supplied if it's not coming from OAuth2AuthorizedClientRepository?

Can you provide a complete and minimal sample that demonstrates a no-op OAuth2AuthorizedClientRepository working? Maybe I'm missing something.

I understand your use-case and have seen demand for this as of late. I do agree that there is a need to call oauth2 protected resources that are running outside of the context of a HttpServletRequest and ServerWebExchange. I feel we need to spend some time looking into this further and ensuring we are building the right solution to meet this use case.

@jgrandja jgrandja added status: waiting-for-feedback We need additional information before we can continue in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Apr 1, 2019
@DarrenForsythe
Copy link
Contributor Author

DarrenForsythe commented Apr 2, 2019

@jgrandja

https://github.com/DarrenForsythe/spring-security-6683-issue-verification

I was able to hack together what I've been seeing, based of mixing a couple of your samples to re-create a client credentials flow. Sorry its a lot messy,

  1. the SecurityConfig sets all requests to Authenticated.
  2. the jwt decoder if it picks up the access_token is equal to test-token it creates a rough jwt.
  3. the jwtAuthenticationConverter creates a basic TestingAuthenticationToken with one granted authority.

If you run the application you'll see the BackgroundClient on a scheduled fixed rate calling to the application api root, http://localhost:8080/ simply with a get and a preconfigured ServletOAuth2AuthorizedClientExchangeFilterFunction.

By default it uses the getNoOpOauth2 which creates the no-op implementation as discussed.

If you remove the application of the filter exchange the WebClient call will 401 as expected.

The only reason I tried a no-op was the loadAuthorizedClient was called first, and it was looking up the HttpServletRequest meaning if it was an initial call I couldn't see how it would return anything but null, but I agree something more along the lines of the UnAuthenticatedServerOAuth2AuthorizedClientRepository would be appropriate (given I assume that impl. has some caching avilable to it to not call the IDP provider with each call?).

@jgrandja
Copy link
Contributor

jgrandja commented Apr 8, 2019

@DarrenForsythe I put together a sample (branch unauthn-oauth2client-repo) that includes the equivalent of UnAuthenticatedServerOAuth2AuthorizedClientRepository (on the reactive side) for Servlet applications - UnauthenticatedPrincipalOAuth2AuthorizedClientRepository.

Take a look at how it's configured for WebClient and used in a @Scheduled method.

The README provides detailed steps to get up and running to test this out.

Can you integrate this into your application and provide feedback on the experience. I'll submit a PR that includes this feature targeted for 5.2. Your initial feedback would be helpful before I prepare the PR. Thanks.

@jgrandja jgrandja changed the title Add No-Op OAuth2AuthorizedClientRepository Provide Servlet equivalent of UnAuthenticatedServerOAuth2AuthorizedClientRepository Apr 8, 2019
@jgrandja jgrandja self-assigned this Apr 8, 2019
@jgrandja jgrandja added New Feature and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 8, 2019
@jgrandja jgrandja added this to the 5.2.0.RC1 milestone Apr 8, 2019
@DarrenForsythe
Copy link
Contributor Author

@jgrandja Tested it this morning, looks good to me.

@jgrandja
Copy link
Contributor

@DarrenForsythe Thank you for validating.

Did you notice UnauthenticatedPrincipalOAuth2AuthorizedClientRepository.setUnauthenticatedPrincipalSupplier() and would you find this useful? The goal is to provide the flexibility that will allow the user to specify an Authentication to use (via a Supplier) when looking up the OAuth2AuthorizedClient. I'd like to hear from you if there is a use case where you would find this helpful?

@DarrenForsythe
Copy link
Contributor Author

Struggling to think of use cases currently for it but I do always enjoy extensibility if its easy to provide/maintain, esp as it's a final class.

I was able to do another verification through another use case, MQ Message > Call back to client creds secured resource > Assertation Exception due to no servlet request. Dropped in your impl and worked.

@jgrandja
Copy link
Contributor

Thanks @DarrenForsythe. I'll get to this after I complete a couple of other pre-requisite tasks.

@DarrenForsythe
Copy link
Contributor Author

No, thank you! I appreciate this being added and quick turn around. Hopefully bump into you in the elevator at S1P again this year @jgrandja 😄

@rwinch
Copy link
Member

rwinch commented Jul 26, 2019

It seems like this is not tied to a Thread local or any Servlet APIs, so it should leverage the fully reactive support. This sample is a simple CLI app and can be used as a model on how to use Spring Security OAuth in a background thread. Does this help you?

@jgrandja
Copy link
Contributor

@rwinch Please see this comment

I've seen quite a few uses cases, working in a large enterprise environment, like this (background threads in a servlet stack) where it simply isn't feasible to switch the apps to a reactive stack

As you can see, the requirement is to provide the ability to operate outside of a request context (e.g. background thread) in a Servlet environment.

@DarrenForsythe This issue will be resolved when #7122 is merged, which builds on the recently merged #6811.

@jzheaux jzheaux modified the milestones: 5.2.0.RC1, 5.2.0 Sep 5, 2019
@jgrandja jgrandja modified the milestones: 5.2.0, 5.2.0.RC1 Sep 6, 2019
@jgrandja
Copy link
Contributor

jgrandja commented Sep 6, 2019

Fixed via #7122

@jgrandja jgrandja closed this as completed Sep 6, 2019
@abhishekvangumalla
Copy link

Not able to get the accesstoken with webclient, my issue is exactly similar to this, am using webclient in server and tomcat , this is not a complete reactive springboot application. i could not find UnauthenticatedPrincipalOAuth2AuthorizedClientRepository class in web package using oauth-client 5.45 and spring boot 2.4.4 version

@jzheaux
Copy link
Contributor

jzheaux commented May 26, 2021

Thanks for reaching out, @abhishekvangumalla. Although this ticket was closed, it was closed by enhancing OAuth2AuthorizedClientManager. UnauthenticatedPrincipalOAuth2AuthorizedClientRepository was not added due to it being unnecessary.

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) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants