spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.83k stars 5.9k forks source link

OAuth2 should be easier to set up without a servlet context #11890

Closed xenoterracide closed 2 years ago

xenoterracide commented 2 years ago

Context

Right now I'm spending a lot of time futzing around trying to figure out how to use spring-cloud-openfeign with the modern spring security oauth 2 because they haven't implemented it yet. It occurs to me while working with this, that there is no reason a client_credentials workflow needs to have a servlet container. you could easily be running a non http daemon or a command line app or a number of other things, that arguably shouldn't even require a spring security context. In this case I don't feel like -web should be "needed" to get a quick client up and tested inside of my feign RequestInterceptor, and for testing it. I get that there's probably a very manual way to do this (that I have yet to figure out from the documentation) but it feels like it should be as simple as configure client in spring boot, get client, grab token, refresh as necessary by just asking for the token.

org.springframework.boot:spring-boot-starter-oauth2-client:2.7.3
org.springframework.security:spring-security-oauth2-client:5.7.3
org.springframework.security:spring-security-oauth2-core:5.7.3
org.springframework.security:spring-security-oauth2-jose:5.7.3
xenoterracide commented 2 years ago

This appears to be what I wanted to do.

package com.capitalone.e1.configuration.feign

import feign.RequestInterceptor
import feign.RequestTemplate
import org.springframework.http.HttpHeaders
import org.springframework.security.oauth2.client.OAuth2AuthorizeRequest
import org.springframework.security.oauth2.client.OAuth2AuthorizedClientManager
import org.springframework.stereotype.Component

@Component
class DevExchangeOAuth2FeignRequestInterceptor(
  private val manager: OAuth2AuthorizedClientManager
) : RequestInterceptor {
  override fun apply(template: RequestTemplate?) {
    template!!
    val authorizeRequest = OAuth2AuthorizeRequest.withClientRegistrationId("devexchange")
      .principal("devexchange")
      .build()

    val client = manager.authorize(authorizeRequest)!!

    template.header(HttpHeaders.AUTHORIZATION, "${client.accessToken.tokenType.value} ${client.accessToken.tokenValue}")
  }
}

I still have no idea why principal is required though, or why I also had to define this (seems like it could be created by the autoconfiguration). I also have to figure out how to make this more efficient for production but still being simple (without a web server) for tests.

  @Bean
  open fun authorizedClientManager(
    clientRegistrationRepository: ClientRegistrationRepository,
    authorizedClientRepository: OAuth2AuthorizedClientRepository
  ): OAuth2AuthorizedClientManager {
    val authorizedClientProvider = OAuth2AuthorizedClientProviderBuilder.builder()
      .clientCredentials()
      .build()
    val authorizedClientManager = DefaultOAuth2AuthorizedClientManager(
      clientRegistrationRepository, authorizedClientRepository
    )
    authorizedClientManager.setAuthorizedClientProvider(authorizedClientProvider)
    return authorizedClientManager
  }
sjohnr commented 2 years ago

@xenoterracide, thanks for getting in touch, but it feels like this is a question that would be better suited to Stack Overflow. We prefer to use GitHub issues only for bugs and enhancements. Feel free to update this issue with a link to the re-posted question (so that other people can find it) or add a minimal sample that reproduces this issue if you feel this is a genuine bug.

xenoterracide commented 2 years ago

You know it took me like a day to figure this out right. I'm not entirely certain why it's not straightforward from the documentation. Or why that it seems to be under servelet when there shouldn't be any servlet required for this kind of oath.

I think you can do better at least on the documentation. And perhaps auto loading this one piece of code if nobody is overriding it. Hey, that's just me.

I'm also pretty sure that those two snippets of code count as a minimal example of the issue. They are indeed very small. One of them being an exact copy and paste from your documentation (which again implies that it could be already written for us)

jgrandja commented 2 years ago

@xenoterracide Please see the section OAuth2AuthorizedClientManager / OAuth2AuthorizedClientProvider in the reference doc:

The DefaultOAuth2AuthorizedClientManager is designed to be used within the context of a HttpServletRequest. When operating outside of a HttpServletRequest context, use AuthorizedClientServiceOAuth2AuthorizedClientManager instead.

A service application is a common use case for when to use an AuthorizedClientServiceOAuth2AuthorizedClientManager. Service applications often run in the background, without any user interaction, and typically run under a system-level account instead of a user account. An OAuth 2.0 Client configured with the client_credentials grant type can be considered a type of service application

Hence, do not use DefaultOAuth2AuthorizedClientManager and instead use AuthorizedClientServiceOAuth2AuthorizedClientManager. Sample code snippet has been provided in the reference documentation:

@Bean
public OAuth2AuthorizedClientManager authorizedClientManager(
        ClientRegistrationRepository clientRegistrationRepository,
        OAuth2AuthorizedClientService authorizedClientService) {

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

    AuthorizedClientServiceOAuth2AuthorizedClientManager authorizedClientManager =
            new AuthorizedClientServiceOAuth2AuthorizedClientManager(
                    clientRegistrationRepository, authorizedClientService);
    authorizedClientManager.setAuthorizedClientProvider(authorizedClientProvider);

    return authorizedClientManager;
}
jzheaux commented 2 years ago

I wonder if the documentation could benefit from a header above the paragraph that @jgrandja highlighted, for example:


Authorizing clients outside of a servlet context

The DefaultOAuth2AuthorizedClientManager is designed to be used within the context of a HttpServletRequest. When operating outside of a HttpServletRequest context...


I think the docs in general should move more towards being use-case driven, but a header like the above may help folks find what they need a little easier in the meantime.

xenoterracide commented 2 years ago

You know what would also help? https://docs.spring.io/spring-security/reference/servlet/index.html if client credentials (which is machine accessible, for all kinds of not servlet code) was not buried under servlet. There should be another section for it.