From 7f29585df48ab7b4ea50c491e4881210a40c134e Mon Sep 17 00:00:00 2001 From: Joe Grandja <10884212+jgrandja@users.noreply.github.com> Date: Wed, 15 Oct 2025 14:00:19 -0400 Subject: [PATCH] Remove OidcUserService.setAccessibleScopes() Closes gh-18056 --- .../OAuth2LoginBeanDefinitionParserTests.java | 9 ++ ...istration-WithCustomGrantedAuthorities.xml | 4 + ...DecoderFactoryAndDefaultSuccessHandler.xml | 4 + .../client/oidc/userinfo/OidcUserService.java | 64 ++----------- .../oidc/userinfo/OidcUserServiceTests.java | 89 +------------------ 5 files changed, 25 insertions(+), 145 deletions(-) diff --git a/config/src/test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java index e401f9ba7b..983ced4b29 100644 --- a/config/src/test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java @@ -60,7 +60,9 @@ import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequ import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.core.endpoint.TestOAuth2AccessTokenResponses; import org.springframework.security.oauth2.core.endpoint.TestOAuth2AuthorizationRequests; +import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser; import org.springframework.security.oauth2.core.oidc.user.OidcUser; +import org.springframework.security.oauth2.core.oidc.user.TestOidcUsers; import org.springframework.security.oauth2.core.user.OAuth2User; import org.springframework.security.oauth2.core.user.TestOAuth2Users; import org.springframework.security.oauth2.jwt.Jwt; @@ -133,6 +135,9 @@ public class OAuth2LoginBeanDefinitionParserTests { @Autowired(required = false) private OAuth2UserService oauth2UserService; + @Autowired(required = false) + private OAuth2UserService oidcUserService; + @Autowired(required = false) private JwtDecoderFactory jwtDecoderFactory; @@ -286,6 +291,8 @@ public class OAuth2LoginBeanDefinitionParserTests { given(this.accessTokenResponseClient.getTokenResponse(any())).willReturn(accessTokenResponse); Jwt jwt = TestJwts.user(); given(this.jwtDecoderFactory.createDecoder(any())).willReturn((token) -> jwt); + DefaultOidcUser oidcUser = TestOidcUsers.create(); + given(this.oidcUserService.loadUser(any())).willReturn(oidcUser); MultiValueMap params = new LinkedMultiValueMap<>(); params.add("code", "code123"); params.add("state", authorizationRequest.getState()); @@ -339,6 +346,8 @@ public class OAuth2LoginBeanDefinitionParserTests { given(this.accessTokenResponseClient.getTokenResponse(any())).willReturn(accessTokenResponse); Jwt jwt = TestJwts.user(); given(this.jwtDecoderFactory.createDecoder(any())).willReturn((token) -> jwt); + DefaultOidcUser oidcUser = TestOidcUsers.create(); + given(this.oidcUserService.loadUser(any())).willReturn(oidcUser); given(this.userAuthoritiesMapper.mapAuthorities(any())) .willReturn((Collection) AuthorityUtils.createAuthorityList("ROLE_OIDC_USER")); // @formatter:off diff --git a/config/src/test/resources/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests-MultiClientRegistration-WithCustomGrantedAuthorities.xml b/config/src/test/resources/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests-MultiClientRegistration-WithCustomGrantedAuthorities.xml index bac6d391a5..6df5a1425a 100644 --- a/config/src/test/resources/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests-MultiClientRegistration-WithCustomGrantedAuthorities.xml +++ b/config/src/test/resources/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests-MultiClientRegistration-WithCustomGrantedAuthorities.xml @@ -28,6 +28,7 @@ + + + diff --git a/config/src/test/resources/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests-SingleClientRegistration-WithJwtDecoderFactoryAndDefaultSuccessHandler.xml b/config/src/test/resources/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests-SingleClientRegistration-WithJwtDecoderFactoryAndDefaultSuccessHandler.xml index d7746b9480..31c0c2a8da 100644 --- a/config/src/test/resources/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests-SingleClientRegistration-WithJwtDecoderFactoryAndDefaultSuccessHandler.xml +++ b/config/src/test/resources/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests-SingleClientRegistration-WithJwtDecoderFactoryAndDefaultSuccessHandler.xml @@ -27,6 +27,7 @@ @@ -34,6 +35,9 @@ + + + diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserService.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserService.java index b838ef6f23..88edde1c7b 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserService.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserService.java @@ -17,11 +17,8 @@ package org.springframework.security.oauth2.client.oidc.userinfo; import java.time.Instant; -import java.util.Arrays; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; -import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; import java.util.function.Predicate; @@ -41,7 +38,6 @@ import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.converter.ClaimConversionService; import org.springframework.security.oauth2.core.converter.ClaimTypeConverter; import org.springframework.security.oauth2.core.oidc.OidcIdToken; -import org.springframework.security.oauth2.core.oidc.OidcScopes; import org.springframework.security.oauth2.core.oidc.OidcUserInfo; import org.springframework.security.oauth2.core.oidc.StandardClaimNames; import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser; @@ -49,7 +45,6 @@ import org.springframework.security.oauth2.core.oidc.user.OidcUser; import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority; import org.springframework.security.oauth2.core.user.OAuth2User; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; /** @@ -72,9 +67,6 @@ public class OidcUserService implements OAuth2UserService, Map> DEFAULT_CLAIM_TYPE_CONVERTER = new ClaimTypeConverter( createDefaultClaimTypeConverters()); - private Set accessibleScopes = new HashSet<>( - Arrays.asList(OidcScopes.PROFILE, OidcScopes.EMAIL, OidcScopes.ADDRESS, OidcScopes.PHONE)); - private OAuth2UserService oauth2UserService = new DefaultOAuth2UserService(); private Function, Map>> claimTypeConverterFactory = ( @@ -150,30 +142,10 @@ public class OidcUserService implements OAuth2UserService accessibleScopes) { - Assert.notNull(accessibleScopes, "accessibleScopes cannot be null"); - this.accessibleScopes = accessibleScopes; - } - /** * Sets the {@code Predicate} used to determine if the UserInfo Endpoint should be * called to retrieve information about the End-User (Resource Owner). *

- * By default, the UserInfo Endpoint is called if all of the following are true: + * By default, the UserInfo Endpoint is called if all the following are true: *

    *
  • The user info endpoint is defined on the ClientRegistration
    • *
  • The Client Registration uses the * {@link AuthorizationGrantType#AUTHORIZATION_CODE}
    • - *
  • The access token contains one or more scopes allowed to access the UserInfo - * Endpoint ({@link OidcScopes#PROFILE profile}, {@link OidcScopes#EMAIL email}, - * {@link OidcScopes#ADDRESS address} or {@link OidcScopes#PHONE phone}) or the access - * token scopes are empty
    • *
- * @param retrieveUserInfo the function used to determine if the UserInfo Endpoint - * should be called + * @param retrieveUserInfo the {@code Predicate} used to determine if the UserInfo + * Endpoint should be called * @since 6.3 */ public final void setRetrieveUserInfo(Predicate retrieveUserInfo) { diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserServiceTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserServiceTests.java index a6b0fff3c7..0716b4f21f 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserServiceTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserServiceTests.java @@ -17,7 +17,6 @@ package org.springframework.security.oauth2.client.oidc.userinfo; import java.time.Instant; -import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -130,16 +129,6 @@ public class OidcUserServiceTests { assertThatIllegalArgumentException().isThrownBy(() -> this.userService.setClaimTypeConverterFactory(null)); } - @Test - public void setAccessibleScopesWhenNullThenThrowIllegalArgumentException() { - assertThatIllegalArgumentException().isThrownBy(() -> this.userService.setAccessibleScopes(null)); - } - - @Test - public void setAccessibleScopesWhenEmptyThenSet() { - this.userService.setAccessibleScopes(Collections.emptySet()); - } - @Test public void setRetrieveUserInfoWhenNullThenThrowIllegalArgumentException() { // @formatter:off @@ -179,83 +168,6 @@ public class OidcUserServiceTests { assertThat(user.getUserInfo()).isNull(); } - @Test - public void loadUserWhenNonStandardScopesAuthorizedThenUserInfoEndpointNotRequested() { - ClientRegistration clientRegistration = this.clientRegistrationBuilder.userInfoUri("https://provider.com/user") - .build(); - this.accessToken = TestOAuth2AccessTokens.scopes("scope1", "scope2"); - OidcUser user = this.userService - .loadUser(new OidcUserRequest(clientRegistration, this.accessToken, this.idToken)); - assertThat(user.getUserInfo()).isNull(); - } - - // gh-6886 - @Test - public void loadUserWhenNonStandardScopesAuthorizedAndAccessibleScopesMatchThenUserInfoEndpointRequested() { - // @formatter:off - String userInfoResponse = "{\n" - + " \"sub\": \"subject1\",\n" - + " \"name\": \"first last\",\n" - + " \"given_name\": \"first\",\n" - + " \"family_name\": \"last\",\n" - + " \"preferred_username\": \"user1\",\n" - + " \"email\": \"user1@example.com\"\n" - + "}\n"; - // @formatter:on - this.server.enqueue(jsonResponse(userInfoResponse)); - String userInfoUri = this.server.url("/user").toString(); - ClientRegistration clientRegistration = this.clientRegistrationBuilder.userInfoUri(userInfoUri).build(); - this.accessToken = TestOAuth2AccessTokens.scopes("scope1", "scope2"); - this.userService.setAccessibleScopes(Collections.singleton("scope2")); - OidcUser user = this.userService - .loadUser(new OidcUserRequest(clientRegistration, this.accessToken, this.idToken)); - assertThat(user.getUserInfo()).isNotNull(); - } - - // gh-6886 - @Test - public void loadUserWhenNonStandardScopesAuthorizedAndAccessibleScopesEmptyThenUserInfoEndpointRequested() { - // @formatter:off - String userInfoResponse = "{\n" - + " \"sub\": \"subject1\",\n" - + " \"name\": \"first last\",\n" - + " \"given_name\": \"first\",\n" - + " \"family_name\": \"last\",\n" - + " \"preferred_username\": \"user1\",\n" - + " \"email\": \"user1@example.com\"\n" - + "}\n"; - // @formatter:on - this.server.enqueue(jsonResponse(userInfoResponse)); - String userInfoUri = this.server.url("/user").toString(); - ClientRegistration clientRegistration = this.clientRegistrationBuilder.userInfoUri(userInfoUri).build(); - this.accessToken = TestOAuth2AccessTokens.scopes("scope1", "scope2"); - this.userService.setAccessibleScopes(Collections.emptySet()); - OidcUser user = this.userService - .loadUser(new OidcUserRequest(clientRegistration, this.accessToken, this.idToken)); - assertThat(user.getUserInfo()).isNotNull(); - } - - // gh-6886 - @Test - public void loadUserWhenStandardScopesAuthorizedThenUserInfoEndpointRequested() { - // @formatter:off - String userInfoResponse = "{\n" - + " \"sub\": \"subject1\",\n" - + " \"name\": \"first last\",\n" - + " \"given_name\": \"first\",\n" - + " \"family_name\": \"last\",\n" - + " \"preferred_username\": \"user1\",\n" - + " \"email\": \"user1@example.com\"\n" - + "}\n"; - // @formatter:on - this.server.enqueue(jsonResponse(userInfoResponse)); - String userInfoUri = this.server.url("/user").toString(); - ClientRegistration clientRegistration = this.clientRegistrationBuilder.userInfoUri(userInfoUri).build(); - OidcUser user = this.userService - .loadUser(new OidcUserRequest(clientRegistration, this.accessToken, this.idToken)); - assertThat(user.getUserInfo()).isNotNull(); - } - @Test public void loadUserWhenCustomRetrieveUserInfoSetThenUsed() { // @formatter:off @@ -571,6 +483,7 @@ public class OidcUserServiceTests { @Test public void loadUserWhenTokenContainsScopesThenIndividualScopeAuthorities() { OidcUserService userService = new OidcUserService(); + userService.setRetrieveUserInfo((req) -> false); OidcUserRequest request = new OidcUserRequest(TestClientRegistrations.clientRegistration().build(), TestOAuth2AccessTokens.scopes("message:read", "message:write"), TestOidcIdTokens.idToken().build()); OidcUser user = userService.loadUser(request);