Prefer `sub` claim as OIDC principal name

This commit removes preference for `name` claim as principal name in `DefaultOidcUser` so that the default is now `sub` claim. In addition to that, `DefaultOidcUser` now also provides constructors to explicitly define the claim to be preferred as principal name.

Fixes gh-4515

oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/user/DefaultOAuth2User.java

@@ -51,7 +51,7 @@ public class DefaultOAuth2User implements OAuth2User {
 			throw new IllegalArgumentException("Invalid nameAttributeKey: " + nameAttributeKey);
 		}
 		this.authorities = Collections.unmodifiableSet(this.sortAuthorities(authorities));
-		this.setAttributes(attributes);
+		this.attributes = Collections.unmodifiableMap(new LinkedHashMap<>(attributes));
 		this.nameAttributeKey = nameAttributeKey;
 	}
 
@@ -70,11 +70,6 @@ public class DefaultOAuth2User implements OAuth2User {
 		return this.attributes;
 	}
 
-	protected final void setAttributes(Map<String, Object> attributes) {
-		Assert.notEmpty(attributes, "attributes cannot be empty");
-		this.attributes = Collections.unmodifiableMap(new LinkedHashMap<>(attributes));
-	}
-
 	private Set<GrantedAuthority> sortAuthorities(Set<GrantedAuthority> authorities) {
 		SortedSet<GrantedAuthority> sortedAuthorities =
 			new TreeSet<>((g1, g2) -> g1.getAuthority().compareTo(g2.getAuthority()));

oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/oidc/core/user/DefaultOidcUser.java

@@ -13,21 +13,19 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.security.oauth2.oidc.core.user;
 
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.oauth2.core.user.DefaultOAuth2User;
 import org.springframework.security.oauth2.oidc.core.IdToken;
 import org.springframework.security.oauth2.oidc.core.IdTokenClaim;
-import org.springframework.security.oauth2.oidc.core.StandardClaim;
 import org.springframework.security.oauth2.oidc.core.UserInfo;
-
-import java.util.Map;
-import java.util.Set;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
-
-import static org.springframework.security.oauth2.oidc.core.StandardClaim.NAME;
+import org.springframework.util.Assert;
 
 /**
  * The default implementation of an {@link OidcUser}.
@@ -35,10 +33,10 @@ import static org.springframework.security.oauth2.oidc.core.StandardClaim.NAME;
  * <p>
  * The claim used for accessing the &quot;name&quot; of the
  * user <code>Principal</code> via {@link #getClaims()}
- * is {@link StandardClaim#NAME} or if not available
- * will default to {@link IdTokenClaim#SUB}.
+ * is {@link IdTokenClaim#SUB}.
  *
  * @author Joe Grandja
+ * @author Vedran Pavic
  * @since 5.0
  * @see OidcUser
  * @see DefaultOAuth2User
@@ -50,20 +48,22 @@ public class DefaultOidcUser extends DefaultOAuth2User implements OidcUser {
 	private final UserInfo userInfo;
 
 	public DefaultOidcUser(Set<GrantedAuthority> authorities, IdToken idToken) {
-		this(authorities, idToken, null);
+		this(authorities, idToken, IdTokenClaim.SUB);
+	}
+
+	public DefaultOidcUser(Set<GrantedAuthority> authorities, IdToken idToken, String nameAttributeKey) {
+		this(authorities, idToken, null, nameAttributeKey);
 	}
 
 	public DefaultOidcUser(Set<GrantedAuthority> authorities, IdToken idToken, UserInfo userInfo) {
-		super(authorities, idToken.getClaims(), IdTokenClaim.SUB);
+		this(authorities, idToken, userInfo, IdTokenClaim.SUB);
+	}
+
+	public DefaultOidcUser(Set<GrantedAuthority> authorities, IdToken idToken, UserInfo userInfo,
+			String nameAttributeKey) {
+		super(authorities, resolveAttributes(idToken, userInfo), nameAttributeKey);
 		this.idToken = idToken;
 		this.userInfo = userInfo;
-		if (userInfo != null) {
-			this.setAttributes(
-				Stream.of(this.getAttributes(), userInfo.getClaims())
-					.flatMap(m -> m.entrySet().stream())
-					.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (k1, k2) -> k1))
-			);
-		}
 	}
 
 	@Override
@@ -71,9 +71,21 @@ public class DefaultOidcUser extends DefaultOAuth2User implements OidcUser {
 		return this.getAttributes();
 	}
 
-	@Override
-	public String getName() {
-		String name = this.getClaimAsString(NAME);
-		return (name != null ? name : super.getName());
+	public IdToken getIdToken() {
+		return this.idToken;
+	}
+
+	public UserInfo getUserInfo() {
+		return this.userInfo;
+	}
+
+	private static Map<String, Object> resolveAttributes(IdToken idToken, UserInfo userInfo) {
+		Assert.notNull(idToken, "idToken cannot be null");
+		Map<String, Object> attributes = new HashMap<>();
+		attributes.putAll(idToken.getClaims());
+		if (userInfo != null) {
+			attributes.putAll(userInfo.getClaims());
+		}
+		return attributes;
 	}
 }

oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/user/DefaultOAuth2UserTests.java

@@ -0,0 +1,110 @@
+/*
+ * Copyright 2012-2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.security.oauth2.core.user;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link DefaultOAuth2User}.
+ *
+ * @author Vedran Pavic
+ */
+public class DefaultOAuth2UserTests {
+
+	private static final SimpleGrantedAuthority TEST_AUTHORITY = new SimpleGrantedAuthority("ROLE_USER");
+
+	private static final Set<GrantedAuthority> TEST_AUTHORITIES = Collections.singleton(TEST_AUTHORITY);
+
+	private static final String TEST_ATTRIBUTE_NAME_KEY = "username";
+
+	private static final String TEST_USERNAME = "test";
+
+	private static final Map<String, Object> TEST_ATTRIBUTES = Collections.singletonMap(
+			TEST_ATTRIBUTE_NAME_KEY, TEST_USERNAME);
+
+	@Rule
+	public ExpectedException thrown = ExpectedException.none();
+
+	@Test
+	public void constructorWhenParametersAreValidThenIsCreated() {
+		DefaultOAuth2User user = new DefaultOAuth2User(TEST_AUTHORITIES, TEST_ATTRIBUTES, TEST_ATTRIBUTE_NAME_KEY);
+
+		assertThat(user.getName()).isEqualTo(TEST_USERNAME);
+		assertThat(user.getAuthorities()).hasSize(1);
+		assertThat(user.getAuthorities().iterator().next()).isEqualTo(TEST_AUTHORITY);
+		assertThat(user.getAttributes()).containsOnlyKeys(TEST_ATTRIBUTE_NAME_KEY);
+	}
+
+	@Test
+	public void constructorWhenAuthoritiesAreNullThenThrowsException() {
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("authorities cannot be empty");
+
+		new DefaultOAuth2User(null, TEST_ATTRIBUTES, TEST_ATTRIBUTE_NAME_KEY);
+	}
+
+	@Test
+	public void constructorWhenAuthoritiesAreEmptyThenThrowsException() {
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("authorities cannot be empty");
+
+		new DefaultOAuth2User(Collections.emptySet(), TEST_ATTRIBUTES, TEST_ATTRIBUTE_NAME_KEY);
+	}
+
+	@Test
+	public void constructorWhenAttributesAreNullThenThrowsException() {
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("attributes cannot be empty");
+
+		new DefaultOAuth2User(TEST_AUTHORITIES, null, TEST_ATTRIBUTE_NAME_KEY);
+	}
+
+	@Test
+	public void constructorWhenAttributesAreEmptyThenThrowsException() {
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("attributes cannot be empty");
+
+		new DefaultOAuth2User(TEST_AUTHORITIES, Collections.emptyMap(), TEST_ATTRIBUTE_NAME_KEY);
+	}
+
+	@Test
+	public void constructorWhenNameAttributeKeyIsNullThenThrowsException() {
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("nameAttributeKey cannot be empty");
+
+		new DefaultOAuth2User(TEST_AUTHORITIES, TEST_ATTRIBUTES, null);
+	}
+
+	@Test
+	public void constructorWhenNameAttributeKeyIsInvalidThenThrowsException() {
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("Invalid nameAttributeKey: invalid");
+
+		new DefaultOAuth2User(TEST_AUTHORITIES, TEST_ATTRIBUTES, "invalid");
+	}
+
+}

oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/oidc/core/user/DefaultOidcUserTests.java

@@ -0,0 +1,122 @@
+/*
+ * Copyright 2012-2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.security.oauth2.oidc.core.user;
+
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
+import org.springframework.security.oauth2.oidc.core.IdToken;
+import org.springframework.security.oauth2.oidc.core.IdTokenClaim;
+import org.springframework.security.oauth2.oidc.core.StandardClaim;
+import org.springframework.security.oauth2.oidc.core.UserInfo;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link DefaultOidcUser}.
+ *
+ * @author Vedran Pavic
+ */
+public class DefaultOidcUserTests {
+
+	private static final SimpleGrantedAuthority TEST_AUTHORITY = new SimpleGrantedAuthority("ROLE_USER");
+
+	private static final Set<GrantedAuthority> TEST_AUTHORITIES = Collections.singleton(TEST_AUTHORITY);
+
+	private static final String TEST_SUBJECT = "test";
+
+	private static final String TEST_EMAIL = "test@example.com";
+
+	private static final Map<String, Object> TEST_ID_TOKEN_CLAIMS = new HashMap<>();
+
+	static {
+		TEST_ID_TOKEN_CLAIMS.put(IdTokenClaim.ISS, "https://example.com");
+		TEST_ID_TOKEN_CLAIMS.put(IdTokenClaim.SUB, TEST_SUBJECT);
+	}
+
+	private static final IdToken TEST_ID_TOKEN = new IdToken("value", Instant.EPOCH, Instant.MAX, TEST_ID_TOKEN_CLAIMS);
+
+	private static final UserInfo TEST_USER_INFO = new UserInfo(Collections.singletonMap(StandardClaim.EMAIL, TEST_EMAIL));
+
+	@Rule
+	public ExpectedException thrown = ExpectedException.none();
+
+	@Test
+	public void constructorWhenAuthoritiesAndIdTokenThenIsCreated() {
+		DefaultOidcUser user = new DefaultOidcUser(TEST_AUTHORITIES, TEST_ID_TOKEN);
+
+		assertThat(user.getName()).isEqualTo(TEST_SUBJECT);
+		assertThat(user.getAuthorities()).hasSize(1);
+		assertThat(user.getAuthorities().iterator().next()).isEqualTo(TEST_AUTHORITY);
+		assertThat(user.getAttributes()).containsOnlyKeys(IdTokenClaim.ISS, IdTokenClaim.SUB);
+	}
+
+	@Test
+	public void constructorWhenAuthoritiesAndIdTokenAndNameAttributeKeyThenIsCreated() {
+		DefaultOidcUser user = new DefaultOidcUser(TEST_AUTHORITIES, TEST_ID_TOKEN, IdTokenClaim.SUB);
+
+		assertThat(user.getName()).isEqualTo(TEST_SUBJECT);
+		assertThat(user.getAuthorities()).hasSize(1);
+		assertThat(user.getAuthorities().iterator().next()).isEqualTo(TEST_AUTHORITY);
+		assertThat(user.getAttributes()).containsOnlyKeys(IdTokenClaim.ISS, IdTokenClaim.SUB);
+	}
+
+	@Test
+	public void constructorWhenAuthoritiesAndIdTokenAndUserInfoThenIsCreated() {
+		DefaultOidcUser user = new DefaultOidcUser(TEST_AUTHORITIES, TEST_ID_TOKEN, TEST_USER_INFO);
+
+		assertThat(user.getName()).isEqualTo(TEST_SUBJECT);
+		assertThat(user.getAuthorities()).hasSize(1);
+		assertThat(user.getAuthorities().iterator().next()).isEqualTo(TEST_AUTHORITY);
+		assertThat(user.getAttributes()).containsOnlyKeys(IdTokenClaim.ISS, IdTokenClaim.SUB, StandardClaim.EMAIL);
+	}
+
+	@Test
+	public void constructorWhenAuthoritiesAndIdTokenAndUserInfoAndNameAttributeKeyThenIsCreated() {
+		DefaultOidcUser user = new DefaultOidcUser(TEST_AUTHORITIES, TEST_ID_TOKEN, TEST_USER_INFO, StandardClaim.EMAIL);
+
+		assertThat(user.getName()).isEqualTo(TEST_EMAIL);
+		assertThat(user.getAuthorities()).hasSize(1);
+		assertThat(user.getAuthorities().iterator().next()).isEqualTo(TEST_AUTHORITY);
+		assertThat(user.getAttributes()).containsOnlyKeys(IdTokenClaim.ISS, IdTokenClaim.SUB, StandardClaim.EMAIL);
+	}
+
+	@Test
+	public void constructorWhenIdTokenIsNullThenThrowsException() {
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("idToken cannot be null");
+
+		new DefaultOidcUser(TEST_AUTHORITIES, null);
+	}
+
+	@Test
+	public void constructorWhenNameAttributeKeyClaimIsNotPresentThenThrowsException() {
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("Invalid nameAttributeKey: " + StandardClaim.NAME);
+
+		new DefaultOidcUser(TEST_AUTHORITIES, TEST_ID_TOKEN, TEST_USER_INFO, StandardClaim.NAME);
+	}
+
+}