spring-projects / spring-security

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

Oauth2 API/Documentation inconsistency, API Breakage/Regression? #11891

Closed xenoterracide closed 1 year ago

xenoterracide commented 1 year ago

Describe the bug https://docs.spring.io/spring-security/reference/servlet/oauth2/client/authorization-grants.html says you can do ClientRegistration.withRegistrationId("...").principal(...);, no such method exists

following that, this doesn't compile due to the wrong type, but the documentation says it should

    val registration = ClientRegistration.withRegistrationId("devexchange").build();
    clientService.authorize(registration)

I'm using 5.7.3 and the docs have 5.7.3 selected. I'm not even sure how this happened? because this was new in 5.x right? so the published API's shouldn't have changed...

jzheaux commented 1 year ago

Thanks for posting this, @xenoterracide. I'm not clear on where you are looking in the documentation, though. The link you gave talks about

OAuth2AuthorizeRequest.withClientRegistrationId("okta").principal(authentication)

which does exist. Can you update the link to where you are seeing the incorrect documentation or point out what I've overlooked?

xenoterracide commented 1 year ago

it doesn't for me... no such method, even when I jump into sources I haven't been able to find the principal method

❯ fd gradle.lockfile -x grep security {} | awk -F'=' '{ print $1 }' | sort -u
org.springframework.security:spring-security-config:5.7.3
org.springframework.security:spring-security-core:5.7.3
org.springframework.security:spring-security-crypto:5.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
org.springframework.security:spring-security-rsa:1.0.11.RELEASE
org.springframework.security:spring-security-test:5.7.3
org.springframework.security:spring-security-web:5.7.3

doesn't compile due to missing method

package e1.configuration.feign

import com.github.tomakehurst.wiremock.client.WireMock.get
import com.github.tomakehurst.wiremock.client.WireMock.ok
import com.github.tomakehurst.wiremock.client.WireMock.post
import com.github.tomakehurst.wiremock.client.WireMock.stubFor
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.cloud.contract.wiremock.AutoConfigureWireMock
import org.springframework.cloud.openfeign.FeignClient
import org.springframework.security.oauth2.client.registration.ClientRegistration
import org.springframework.security.oauth2.core.OAuth2AccessToken
import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse
import org.springframework.web.bind.annotation.GetMapping
import wiremock.com.fasterxml.jackson.databind.node.JsonNodeFactory
import java.time.Duration
import java.util.UUID

@SpringBootTest
@AutoConfigureWireMock(port = 0)
internal class DevExchangeOauth2FeignRequestInterceptorTest {

  @Test
  fun oauth( @Autowired fooRepo: FooRepo) {
    ClientRegistration.withRegistrationId("...").principal(null)
  }
}

now I'm mad at myself for not doing a better job of documentating the location. I can swear I saw it 3 times on the same page...

xenoterracide commented 1 year ago

yep, in my hours and hours of hunting down how to do client credentials without a servlet container I confused these 2

    OAuth2AuthorizeRequest.withClientRegistrationId("").principal(null)
    ClientRegistration.withRegistrationId("...").principal(null)
jzheaux commented 1 year ago

Glad you were able to sort it out.