Browse Source

Align setRetrieveUserInfo() between OidcUserService and OidcReactiveOAuth2UserService

Closes gh-18057
Joe Grandja 6 days ago
parent
commit
af1de950ae

+ 4 - 5
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcReactiveOAuth2UserService.java

@@ -156,15 +156,14 @@ public class OidcReactiveOAuth2UserService implements ReactiveOAuth2UserService<
 	 * 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} and scopes in the access token
-	 * are defined in the {@link ClientRegistration}</li>
+	 * {@link AuthorizationGrantType#AUTHORIZATION_CODE}</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) {

+ 8 - 23
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserRequestUtils.java

@@ -28,7 +28,6 @@ import org.springframework.security.oauth2.core.oidc.OidcUserInfo;
 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.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 
 /**
@@ -40,38 +39,24 @@ import org.springframework.util.StringUtils;
 final class OidcUserRequestUtils {
 
 	/**
-	 * Determines if an {@link OidcUserRequest} should attempt to retrieve the user info
-	 * endpoint. Will return true if all of the following are true:
+	 * Determines if an {@link OidcUserRequest} should attempt to retrieve the user info.
+	 * Will return true 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} and scopes in the access token
-	 * are defined in the {@link ClientRegistration}</li>
+	 * {@link AuthorizationGrantType#AUTHORIZATION_CODE}</li>
 	 * </ul>
 	 * @param userRequest
 	 * @return
 	 */
 	static boolean shouldRetrieveUserInfo(OidcUserRequest userRequest) {
 		// Auto-disabled if UserInfo Endpoint URI is not provided
-		ClientRegistration clientRegistration = userRequest.getClientRegistration();
-		if (!StringUtils.hasLength(clientRegistration.getProviderDetails().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(clientRegistration.getAuthorizationGrantType())) {
-			// Return true if there is at least one match between the authorized scope(s)
-			// and UserInfo scope(s)
-			return CollectionUtils.containsAny(userRequest.getAccessToken().getScopes(),
-					userRequest.getClientRegistration().getScopes());
+		ClientRegistration.ProviderDetails providerDetails = userRequest.getClientRegistration().getProviderDetails();
+		if (StringUtils.hasLength(providerDetails.getUserInfoEndpoint().getUri())
+				&& AuthorizationGrantType.AUTHORIZATION_CODE
+					.equals(userRequest.getClientRegistration().getAuthorizationGrantType())) {
+			return true;
 		}
 		return false;
 	}

+ 1 - 14
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserService.java

@@ -27,7 +27,6 @@ import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.oauth2.client.registration.ClientRegistration;
-import org.springframework.security.oauth2.client.registration.ClientRegistration.ProviderDetails;
 import org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService;
 import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest;
 import org.springframework.security.oauth2.client.userinfo.OAuth2UserService;
@@ -45,7 +44,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.StringUtils;
 
 /**
  * An implementation of an {@link OAuth2UserService} that supports OpenID Connect 1.0
@@ -72,7 +70,7 @@ public class OidcUserService implements OAuth2UserService<OidcUserRequest, OidcU
 	private Function<ClientRegistration, Converter<Map<String, Object>, Map<String, Object>>> claimTypeConverterFactory = (
 			clientRegistration) -> DEFAULT_CLAIM_TYPE_CONVERTER;
 
-	private Predicate<OidcUserRequest> retrieveUserInfo = this::shouldRetrieveUserInfo;
+	private Predicate<OidcUserRequest> retrieveUserInfo = OidcUserRequestUtils::shouldRetrieveUserInfo;
 
 	private Converter<OidcUserSource, OidcUser> oidcUserConverter = OidcUserRequestUtils::getUser;
 
@@ -139,17 +137,6 @@ public class OidcUserService implements OAuth2UserService<OidcUserRequest, OidcU
 		return DEFAULT_CLAIM_TYPE_CONVERTER.convert(oauth2User.getAttributes());
 	}
 
-	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())
-				&& AuthorizationGrantType.AUTHORIZATION_CODE
-					.equals(userRequest.getClientRegistration().getAuthorizationGrantType())) {
-			return true;
-		}
-		return false;
-	}
-
 	/**
 	 * Sets the {@link OAuth2UserService} used when requesting the user info resource.
 	 * @param oauth2UserService the {@link OAuth2UserService} used when requesting the

+ 3 - 16
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcReactiveOAuth2UserServiceTests.java

@@ -199,22 +199,6 @@ public class OidcReactiveOAuth2UserServiceTests {
 		verify(customClaimTypeConverterFactory).apply(same(userRequest.getClientRegistration()));
 	}
 
-	@Test
-	public void loadUserWhenTokenScopesIsEmptyThenUserInfoNotRetrieved() {
-		// @formatter:off
-		OAuth2AccessToken accessToken = new OAuth2AccessToken(
-				this.accessToken.getTokenType(),
-				this.accessToken.getTokenValue(),
-				this.accessToken.getIssuedAt(),
-				this.accessToken.getExpiresAt(),
-				Collections.emptySet());
-		// @formatter:on
-		OidcUserRequest userRequest = new OidcUserRequest(this.registration.build(), accessToken, this.idToken);
-		OidcUser oidcUser = this.userService.loadUser(userRequest).block();
-		assertThat(oidcUser).isNotNull();
-		assertThat(oidcUser.getUserInfo()).isNull();
-	}
-
 	@Test
 	public void loadUserWhenCustomRetrieveUserInfoSetThenUsed() {
 		Map<String, Object> attributes = new HashMap<>();
@@ -281,6 +265,7 @@ public class OidcReactiveOAuth2UserServiceTests {
 				IdTokenClaimNames.SUB);
 		given(customOidcUserMapper.apply(any(OidcUserRequest.class), isNull())).willReturn(Mono.just(actualUser));
 		this.userService.setOidcUserMapper(customOidcUserMapper);
+		this.userService.setRetrieveUserInfo((oidcUserRequest) -> false);
 		OidcUserRequest userRequest = userRequest();
 		OidcUser oidcUser = this.userService.loadUser(userRequest).block();
 		assertThat(oidcUser).isNotNull();
@@ -291,6 +276,7 @@ public class OidcReactiveOAuth2UserServiceTests {
 	@Test
 	public void loadUserWhenTokenContainsScopesThenIndividualScopeAuthorities() {
 		OidcReactiveOAuth2UserService userService = new OidcReactiveOAuth2UserService();
+		userService.setRetrieveUserInfo((oidcUserRequest) -> false);
 		OidcUserRequest request = new OidcUserRequest(TestClientRegistrations.clientRegistration().build(),
 				TestOAuth2AccessTokens.scopes("message:read", "message:write"), TestOidcIdTokens.idToken().build());
 		OidcUser user = userService.loadUser(request).block();
@@ -304,6 +290,7 @@ public class OidcReactiveOAuth2UserServiceTests {
 	@Test
 	public void loadUserWhenTokenDoesNotContainScopesThenNoScopeAuthorities() {
 		OidcReactiveOAuth2UserService userService = new OidcReactiveOAuth2UserService();
+		userService.setRetrieveUserInfo((oidcUserRequest) -> false);
 		OidcUserRequest request = new OidcUserRequest(TestClientRegistrations.clientRegistration().build(),
 				TestOAuth2AccessTokens.noScopes(), TestOidcIdTokens.idToken().build());
 		OidcUser user = userService.loadUser(request).block();

+ 1 - 7
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserRequestUtilsTests.java

@@ -45,7 +45,7 @@ public class OidcUserRequestUtilsTests {
 			Instant.now().plus(Duration.ofDays(1)), Collections.singleton("read:user"));
 
 	@Test
-	public void shouldRetrieveUserInfoWhenEndpointDefinedAndScopesOverlapThenTrue() {
+	public void shouldRetrieveUserInfoWhenUserInfoUriThenTrue() {
 		assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest())).isTrue();
 	}
 
@@ -55,12 +55,6 @@ public class OidcUserRequestUtilsTests {
 		assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest())).isFalse();
 	}
 
-	@Test
-	public void shouldRetrieveUserInfoWhenDifferentScopesThenFalse() {
-		this.registration.scope("notintoken");
-		assertThat(OidcUserRequestUtils.shouldRetrieveUserInfo(userRequest())).isFalse();
-	}
-
 	@Test
 	public void shouldRetrieveUserInfoWhenNotAuthorizationCodeThenFalse() {
 		this.registration.authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS);