Remove OidcUserService.setAccessibleScopes()

Closes gh-18056

Author: jgrandja

config/src/test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java

@@ -60,7 +60,9 @@
 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<OAuth2UserRequest, OAuth2User> oauth2UserService;
 
+	@Autowired(required = false)
+	private OAuth2UserService<OAuth2UserRequest, OAuth2User> oidcUserService;
+
 	@Autowired(required = false)
 	private JwtDecoderFactory<ClientRegistration> jwtDecoderFactory;
 
@@ -286,6 +291,8 @@ public void requestWhenOidcAuthenticationResponseValidThenJwtDecoderFactoryCalle
 		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<String, String> params = new LinkedMultiValueMap<>();
 		params.add("code", "code123");
 		params.add("state", authorizationRequest.getState());
@@ -339,6 +346,8 @@ public void requestWhenCustomGrantedAuthoritiesMapperThenCalled() throws Excepti
 		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

config/src/test/resources/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests-MultiClientRegistration-WithCustomGrantedAuthorities.xml

@@ -28,6 +28,7 @@
 		<intercept-url pattern="/**" access="authenticated"/>
 		<oauth2-login access-token-response-client-ref="accessTokenResponseClient"
 						user-service-ref="oauth2UserService"
+					  	oidc-user-service-ref="oidcUserService"
 						user-authorities-mapper-ref="userAuthoritiesMapper"
 						jwt-decoder-factory-ref="jwtDecoderFactory"
 						authorization-request-repository-ref="authorizationRequestRepository"
@@ -39,6 +40,9 @@
 </b:bean>
 	<b:bean id="oauth2UserService" class="org.mockito.Mockito" factory-method="mock">
     <b:constructor-arg value="org.springframework.security.oauth2.client.userinfo.OAuth2UserService" type="java.lang.Class"/>
+</b:bean>
+	<b:bean id="oidcUserService" class="org.mockito.Mockito" factory-method="mock">
+	<b:constructor-arg value="org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService" type="java.lang.Class"/>
 </b:bean>
 	<b:bean id="userAuthoritiesMapper" class="org.mockito.Mockito" factory-method="mock">
     <b:constructor-arg value="org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper" type="java.lang.Class"/>

config/src/test/resources/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests-SingleClientRegistration-WithJwtDecoderFactoryAndDefaultSuccessHandler.xml

@@ -27,13 +27,17 @@
 	<http auto-config="true">
 		<intercept-url pattern="/**" access="authenticated"/>
 		<oauth2-login access-token-response-client-ref="accessTokenResponseClient"
+					  	oidc-user-service-ref="oidcUserService"
 						jwt-decoder-factory-ref="jwtDecoderFactory"
 						authorization-request-repository-ref="authorizationRequestRepository"/>
 		<request-cache ref="requestCache" />
 	</http>
 
 	<b:bean id="accessTokenResponseClient" class="org.mockito.Mockito" factory-method="mock">
     <b:constructor-arg value="org.springframework.security.oauth2.client.endpoint.OAuth2AccessTokenResponseClient" type="java.lang.Class"/>
+</b:bean>
+	<b:bean id="oidcUserService" class="org.mockito.Mockito" factory-method="mock">
+	<b:constructor-arg value="org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService" type="java.lang.Class"/>
 </b:bean>
 	<b:bean id="jwtDecoderFactory" class="org.mockito.Mockito" factory-method="mock">
     <b:constructor-arg value="org.springframework.security.oauth2.jwt.JwtDecoderFactory" type="java.lang.Class"/>

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,15 +38,13 @@
 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;
 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<OidcUserRequest, OidcU
 	private static final Converter<Map<String, Object>, Map<String, Object>> DEFAULT_CLAIM_TYPE_CONVERTER = new ClaimTypeConverter(
 			createDefaultClaimTypeConverters());
 
-	private Set<String> accessibleScopes = new HashSet<>(
-			Arrays.asList(OidcScopes.PROFILE, OidcScopes.EMAIL, OidcScopes.ADDRESS, OidcScopes.PHONE));
-
 	private OAuth2UserService<OAuth2UserRequest, OAuth2User> oauth2UserService = new DefaultOAuth2UserService();
 
 	private Function<ClientRegistration, Converter<Map<String, Object>, Map<String, Object>>> claimTypeConverterFactory = (
@@ -150,30 +142,10 @@ private Map<String, Object> getClaims(OidcUserRequest userRequest, OAuth2User oa
 	private boolean shouldRetrieveUserInfo(OidcUserRequest userRequest) {
 		// Auto-disabled if UserInfo Endpoint URI is not provided
 		ProviderDetails providerDetails = userRequest.getClientRegistration().getProviderDetails();
-		if (!StringUtils.hasLength(providerDetails.getUserInfoEndpoint().getUri())) {
-			return false;
-		}
-		// The Claims requested by the profile, email, address, and phone scope values
-		// are returned from the UserInfo Endpoint (as described in Section 5.3.2),
-		// when a response_type value is used that results in an Access Token being
-		// issued.
-		// However, when no Access Token is issued, which is the case for the
-		// response_type=id_token,
-		// the resulting Claims are returned in the ID Token.
-		// The Authorization Code Grant Flow, which is response_type=code, results in an
-		// Access Token being issued.
-		if (AuthorizationGrantType.AUTHORIZATION_CODE
-			.equals(userRequest.getClientRegistration().getAuthorizationGrantType())) {
-			// Return true if there is at least one match between the authorized scope(s)
-			// and accessible scope(s)
-			//
-			// Also return true if authorized scope(s) is empty, because the provider has
-			// not indicated which scopes are accessible via the access token
-			// @formatter:off
-			return this.accessibleScopes.isEmpty()
-					|| CollectionUtils.isEmpty(userRequest.getAccessToken().getScopes())
-					|| CollectionUtils.containsAny(userRequest.getAccessToken().getScopes(), this.accessibleScopes);
-			// @formatter:on
+		if (StringUtils.hasLength(providerDetails.getUserInfoEndpoint().getUri())
+				&& AuthorizationGrantType.AUTHORIZATION_CODE
+					.equals(userRequest.getClientRegistration().getAuthorizationGrantType())) {
+			return true;
 		}
 		return false;
 	}
@@ -204,40 +176,18 @@ public final void setClaimTypeConverterFactory(
 		this.claimTypeConverterFactory = claimTypeConverterFactory;
 	}
 
-	/**
-	 * Sets the scope(s) that allow access to the user info resource. The default is
-	 * {@link OidcScopes#PROFILE profile}, {@link OidcScopes#EMAIL email},
-	 * {@link OidcScopes#ADDRESS address} and {@link OidcScopes#PHONE phone}. The scope(s)
-	 * are checked against the "granted" scope(s) associated to the
-	 * {@link OidcUserRequest#getAccessToken() access token} to determine if the user info
-	 * resource is accessible or not. If there is at least one match, the user info
-	 * resource will be requested, otherwise it will not.
-	 * @param accessibleScopes the scope(s) that allow access to the user info resource
-	 * @since 5.2
-	 * @deprecated Use {@link #setRetrieveUserInfo(Predicate)} instead
-	 */
-	@Deprecated(since = "6.3", forRemoval = true)
-	public final void setAccessibleScopes(Set<String> 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).
 	 * <p>
-	 * 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:
 	 * <ul>
 	 * <li>The user info endpoint is defined on the ClientRegistration</li>
 	 * <li>The Client Registration uses the
 	 * {@link AuthorizationGrantType#AUTHORIZATION_CODE}</li>
-	 * <li>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</li>
 	 * </ul>
-	 * @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<OidcUserRequest> retrieveUserInfo) {

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 void setClaimTypeConverterFactoryWhenNullThenThrowIllegalArgumentExceptio
 		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 void loadUserWhenUserInfoUriIsNullThenUserInfoEndpointNotRequested() {
 		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 void loadUserWhenCustomClaimTypeConverterFactorySetThenApplied() {
 	@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);