From d32aa3c6d6d1e263359391a302868991dc04291e Mon Sep 17 00:00:00 2001
From: Joe Grandja <jgrandja@pivotal.io>
Date: Mon, 25 Jun 2018 16:44:04 -0400
Subject: [PATCH] Validate sub claim in UserInfo Response

Fixes gh-5447
---
 .../client/oidc/userinfo/OidcUserService.java | 17 +++++++---
 .../oidc/userinfo/OidcUserServiceTests.java   | 31 +++++++++++++++++++
 2 files changed, 43 insertions(+), 5 deletions(-)

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 aaae86454a..ae3c6505a0 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
@@ -15,10 +15,6 @@
  */
 package org.springframework.security.oauth2.client.oidc.userinfo;
 
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Set;
-
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService;
 import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest;
@@ -36,6 +32,10 @@ import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
 /**
  * An implementation of an {@link OAuth2UserService} that supports OpenID Connect 1.0 Provider's.
  *
@@ -62,7 +62,14 @@ public class OidcUserService implements OAuth2UserService<OidcUserRequest, OidcU
 			userInfo = new OidcUserInfo(oauth2User.getAttributes());
 
 			// http://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse
-			// Due to the possibility of token substitution attacks (see Section 16.11),
+
+			// 1) The sub (subject) Claim MUST always be returned in the UserInfo Response
+			if (userInfo.getSubject() == null) {
+				OAuth2Error oauth2Error = new OAuth2Error(INVALID_USER_INFO_RESPONSE_ERROR_CODE);
+				throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
+			}
+
+			// 2) Due to the possibility of token substitution attacks (see Section 16.11),
 			// the UserInfo Response is not guaranteed to be about the End-User
 			// identified by the sub (subject) element of the ID Token.
 			// The sub Claim in the UserInfo Response MUST be verified to exactly match
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 7ac59eb954..6164022292 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
@@ -168,6 +168,37 @@ public class OidcUserServiceTests {
 		assertThat(userAuthority.getUserInfo()).isEqualTo(user.getUserInfo());
 	}
 
+	// gh-5447
+	@Test
+	public void loadUserWhenUserInfoSuccessResponseAndUserInfoSubjectIsNullThenThrowOAuth2AuthenticationException() throws Exception {
+		this.exception.expect(OAuth2AuthenticationException.class);
+		this.exception.expectMessage(containsString("invalid_user_info_response"));
+
+		MockWebServer server = new MockWebServer();
+
+		String userInfoResponse = "{\n" +
+				"	\"email\": \"full_name@provider.com\",\n" +
+				"	\"name\": \"full name\"\n" +
+				"}\n";
+		server.enqueue(new MockResponse()
+				.setHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE)
+				.setBody(userInfoResponse));
+
+		server.start();
+
+		String userInfoUri = server.url("/user").toString();
+
+		when(this.userInfoEndpoint.getUri()).thenReturn(userInfoUri);
+		when(this.userInfoEndpoint.getUserNameAttributeName()).thenReturn(StandardClaimNames.EMAIL);
+		when(this.accessToken.getTokenValue()).thenReturn("access-token");
+
+		try {
+			this.userService.loadUser(new OidcUserRequest(this.clientRegistration, this.accessToken, this.idToken));
+		} finally {
+			server.shutdown();
+		}
+	}
+
 	@Test
 	public void loadUserWhenUserInfoSuccessResponseAndUserInfoSubjectNotSameAsIdTokenSubjectThenThrowOAuth2AuthenticationException() throws Exception {
 		this.exception.expect(OAuth2AuthenticationException.class);