Browse Source

Allow configurable accessible scopes for UserInfo resource

Fixes gh-6886
Joe Grandja 6 years ago
parent
commit
3f2108921e

+ 20 - 4
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserService.java

@@ -62,8 +62,8 @@ public class OidcUserService implements OAuth2UserService<OidcUserRequest, OidcU
 	private static final String INVALID_USER_INFO_RESPONSE_ERROR_CODE = "invalid_user_info_response";
 	private static final Converter<Map<String, Object>, Map<String, Object>> DEFAULT_CLAIM_TYPE_CONVERTER =
 			new ClaimTypeConverter(createDefaultClaimTypeConverters());
-	private final Set<String> userInfoScopes = new HashSet<>(
-		Arrays.asList(OidcScopes.PROFILE, OidcScopes.EMAIL, OidcScopes.ADDRESS, OidcScopes.PHONE));
+	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 =
 			clientRegistration -> DEFAULT_CLAIM_TYPE_CONVERTER;
@@ -160,8 +160,9 @@ public class OidcUserService implements OAuth2UserService<OidcUserRequest, OidcU
 		if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(
 			userRequest.getClientRegistration().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(), this.userInfoScopes);
+			// Return true if there is at least one match between the authorized scope(s) and accessible scope(s)
+			return this.accessibleScopes.isEmpty() ||
+					CollectionUtils.containsAny(userRequest.getAccessToken().getScopes(), this.accessibleScopes);
 		}
 
 		return false;
@@ -190,4 +191,19 @@ public class OidcUserService implements OAuth2UserService<OidcUserRequest, OidcU
 		Assert.notNull(claimTypeConverterFactory, "claimTypeConverterFactory cannot be null");
 		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.
+	 *
+	 * @since 5.2
+	 * @param accessibleScopes the scope(s) that allow access to the user info resource
+	 */
+	public final void setAccessibleScopes(Set<String> accessibleScopes) {
+		Assert.notNull(accessibleScopes, "accessibleScopes cannot be null");
+		this.accessibleScopes = accessibleScopes;
+	}
 }

+ 90 - 10
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserServiceTests.java

@@ -41,11 +41,9 @@ import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority;
 
 import java.time.Instant;
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
-import java.util.LinkedHashSet;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 
@@ -116,6 +114,17 @@ public class OidcUserServiceTests {
 				.isInstanceOf(IllegalArgumentException.class);
 	}
 
+	@Test
+	public void setAccessibleScopesWhenNullThenThrowIllegalArgumentException() {
+		assertThatThrownBy(() -> this.userService.setAccessibleScopes(null))
+				.isInstanceOf(IllegalArgumentException.class);
+	}
+
+	@Test
+	public void setAccessibleScopesWhenEmptyThenSet() {
+		this.userService.setAccessibleScopes(Collections.emptySet());
+	}
+
 	@Test
 	public void loadUserWhenUserRequestIsNullThenThrowIllegalArgumentException() {
 		this.exception.expect(IllegalArgumentException.class);
@@ -130,20 +139,91 @@ public class OidcUserServiceTests {
 	}
 
 	@Test
-	public void loadUserWhenAuthorizedScopesDoesNotContainUserInfoScopesThenUserInfoEndpointNotRequested() {
+	public void loadUserWhenNonStandardScopesAuthorizedThenUserInfoEndpointNotRequested() {
 		ClientRegistration clientRegistration = this.clientRegistrationBuilder
 				.userInfoUri("https://provider.com/user").build();
-
-		Set<String> authorizedScopes = new LinkedHashSet<>(Arrays.asList("scope1", "scope2"));
-		OAuth2AccessToken accessToken = new OAuth2AccessToken(
-				OAuth2AccessToken.TokenType.BEARER, "access-token",
-				Instant.MIN, Instant.MAX, authorizedScopes);
+		this.accessToken = scopes("scope1", "scope2");
 
 		OidcUser user = this.userService.loadUser(
-			new OidcUserRequest(clientRegistration, accessToken, this.idToken));
+			new OidcUserRequest(clientRegistration, this.accessToken, this.idToken));
 		assertThat(user.getUserInfo()).isNull();
 	}
 
+	// gh-6886
+	@Test
+	public void loadUserWhenNonStandardScopesAuthorizedAndAccessibleScopesMatchThenUserInfoEndpointRequested() {
+		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";
+		this.server.enqueue(jsonResponse(userInfoResponse));
+
+		String userInfoUri = this.server.url("/user").toString();
+
+		ClientRegistration clientRegistration = this.clientRegistrationBuilder
+				.userInfoUri(userInfoUri).build();
+
+		this.accessToken = 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() {
+		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";
+		this.server.enqueue(jsonResponse(userInfoResponse));
+
+		String userInfoUri = this.server.url("/user").toString();
+
+		ClientRegistration clientRegistration = this.clientRegistrationBuilder
+				.userInfoUri(userInfoUri).build();
+
+		this.accessToken = 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() {
+		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";
+		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 loadUserWhenUserInfoSuccessResponseThenReturnUser() {
 		String userInfoResponse = "{\n" +