فهرست منبع

Polish gh-4442

Joe Grandja 6 سال پیش
والد
کامیت
d3b7a47ef8
13فایلهای تغییر یافته به همراه248 افزوده شده و 202 حذف شده
  1. 3 7
      config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2ClientConfigurerTests.java
  2. 8 7
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java
  3. 10 11
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeReactiveAuthenticationManager.java
  4. 19 17
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java
  5. 19 17
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java
  6. 31 79
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProviderTests.java
  7. 70 16
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeReactiveAuthenticationManagerTests.java
  8. 1 0
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactoryTests.java
  9. 1 0
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactoryTests.java
  10. 54 27
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java
  11. 4 9
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java
  12. 27 11
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java
  13. 1 1
      oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/oidc/endpoint/OidcParameterNames.java

+ 3 - 7
config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2ClientConfigurerTests.java

@@ -75,7 +75,6 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
  * Tests for {@link OAuth2ClientConfigurer}.
  * Tests for {@link OAuth2ClientConfigurer}.
  *
  *
  * @author Joe Grandja
  * @author Joe Grandja
- * @author Mark Heckler
  */
  */
 public class OAuth2ClientConfigurerTests {
 public class OAuth2ClientConfigurerTests {
 	private static ClientRegistrationRepository clientRegistrationRepository;
 	private static ClientRegistrationRepository clientRegistrationRepository;
@@ -139,8 +138,7 @@ public class OAuth2ClientConfigurerTests {
 		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
 		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
 				"response_type=code&client_id=client-1&" +
 				"response_type=code&client_id=client-1&" +
 				"scope=user&state=.{15,}&" +
 				"scope=user&state=.{15,}&" +
-				"redirect_uri=http://localhost/client-1&" +
-				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+				"redirect_uri=http://localhost/client-1");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -153,8 +151,7 @@ public class OAuth2ClientConfigurerTests {
 		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
 		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
 				"response_type=code&client_id=client-1&" +
 				"response_type=code&client_id=client-1&" +
 				"scope=user&state=.{15,}&" +
 				"scope=user&state=.{15,}&" +
-				"redirect_uri=http://localhost/client-1&" +
-				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+				"redirect_uri=http://localhost/client-1");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -206,8 +203,7 @@ public class OAuth2ClientConfigurerTests {
 		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
 		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
 				"response_type=code&client_id=client-1&" +
 				"response_type=code&client_id=client-1&" +
 				"scope=user&state=.{15,}&" +
 				"scope=user&state=.{15,}&" +
-				"redirect_uri=http://localhost/client-1&" +
-				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+				"redirect_uri=http://localhost/client-1");
 
 
 		verify(requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class));
 		verify(requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class));
 	}
 	}

+ 8 - 7
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProvider.java

@@ -158,21 +158,22 @@ public class OidcAuthorizationCodeAuthenticationProvider implements Authenticati
 				null);
 				null);
 			throw new OAuth2AuthenticationException(invalidIdTokenError, invalidIdTokenError.toString());
 			throw new OAuth2AuthenticationException(invalidIdTokenError, invalidIdTokenError.toString());
 		}
 		}
-			OidcIdToken idToken = createOidcToken(clientRegistration, accessTokenResponse);
+		OidcIdToken idToken = createOidcToken(clientRegistration, accessTokenResponse);
 
 
+		// Validate nonce
 		String requestNonce = authorizationRequest.getAttribute(OidcParameterNames.NONCE);
 		String requestNonce = authorizationRequest.getAttribute(OidcParameterNames.NONCE);
 		if (requestNonce != null) {
 		if (requestNonce != null) {
 			String nonceHash;
 			String nonceHash;
-
 			try {
 			try {
 				nonceHash = createHash(requestNonce);
 				nonceHash = createHash(requestNonce);
 			} catch (NoSuchAlgorithmException e) {
 			} catch (NoSuchAlgorithmException e) {
-				throw new OAuth2AuthenticationException(new OAuth2Error(INVALID_NONCE_ERROR_CODE));
+				OAuth2Error oauth2Error = new OAuth2Error(INVALID_NONCE_ERROR_CODE);
+				throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
 			}
 			}
-
-			String nonceHashClaim = idToken.getClaim(OidcParameterNames.NONCE);
+			String nonceHashClaim = idToken.getNonce();
 			if (nonceHashClaim == null || !nonceHashClaim.equals(nonceHash)) {
 			if (nonceHashClaim == null || !nonceHashClaim.equals(nonceHash)) {
-				throw new OAuth2AuthenticationException(new OAuth2Error(INVALID_NONCE_ERROR_CODE));
+				OAuth2Error oauth2Error = new OAuth2Error(INVALID_NONCE_ERROR_CODE);
+				throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
 			}
 			}
 		}
 		}
 
 
@@ -234,7 +235,7 @@ public class OidcAuthorizationCodeAuthenticationProvider implements Authenticati
 		return idToken;
 		return idToken;
 	}
 	}
 
 
-	private String createHash(String nonce) throws NoSuchAlgorithmException {
+	static String createHash(String nonce) throws NoSuchAlgorithmException {
 		MessageDigest md = MessageDigest.getInstance("SHA-256");
 		MessageDigest md = MessageDigest.getInstance("SHA-256");
 		byte[] digest = md.digest(nonce.getBytes(StandardCharsets.US_ASCII));
 		byte[] digest = md.digest(nonce.getBytes(StandardCharsets.US_ASCII));
 		return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);
 		return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);

+ 10 - 11
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeReactiveAuthenticationManager.java

@@ -65,6 +65,7 @@ import java.util.Map;
  * to complete the authentication.
  * to complete the authentication.
  *
  *
  * @author Rob Winch
  * @author Rob Winch
+ * @author Mark Heckler
  * @since 5.1
  * @since 5.1
  * @see OAuth2LoginAuthenticationToken
  * @see OAuth2LoginAuthenticationToken
  * @see ReactiveOAuth2AccessTokenResponseClient
  * @see ReactiveOAuth2AccessTokenResponseClient
@@ -199,30 +200,28 @@ public class OidcAuthorizationCodeReactiveAuthenticationManager implements
 				.map(jwt -> new OidcIdToken(jwt.getTokenValue(), jwt.getIssuedAt(), jwt.getExpiresAt(), jwt.getClaims()));
 				.map(jwt -> new OidcIdToken(jwt.getTokenValue(), jwt.getIssuedAt(), jwt.getExpiresAt(), jwt.getClaims()));
 	}
 	}
 
 
-	private Mono<OidcIdToken> validateNonce(OAuth2AuthorizationCodeAuthenticationToken authorizationCodeAuthentication, OidcIdToken idToken) {
-		String requestNonce = authorizationCodeAuthentication
-				.getAuthorizationExchange()
-				.getAuthorizationRequest()
-				.getAttribute(OidcParameterNames.NONCE);
+	private static Mono<OidcIdToken> validateNonce(OAuth2AuthorizationCodeAuthenticationToken authorizationCodeAuthentication, OidcIdToken idToken) {
+		String requestNonce = authorizationCodeAuthentication.getAuthorizationExchange()
+				.getAuthorizationRequest().getAttribute(OidcParameterNames.NONCE);
 		if (requestNonce != null) {
 		if (requestNonce != null) {
 			String nonceHash;
 			String nonceHash;
-
 			try {
 			try {
 				nonceHash = createHash(requestNonce);
 				nonceHash = createHash(requestNonce);
 			} catch (NoSuchAlgorithmException e) {
 			} catch (NoSuchAlgorithmException e) {
-				throw new OAuth2AuthenticationException(new OAuth2Error(INVALID_NONCE_ERROR_CODE));
+				OAuth2Error oauth2Error = new OAuth2Error(INVALID_NONCE_ERROR_CODE);
+				throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
 			}
 			}
-
-			String nonceHashClaim = idToken.getClaim(OidcParameterNames.NONCE);
+			String nonceHashClaim = idToken.getNonce();
 			if (nonceHashClaim == null || !nonceHashClaim.equals(nonceHash)) {
 			if (nonceHashClaim == null || !nonceHashClaim.equals(nonceHash)) {
-				throw new OAuth2AuthenticationException(new OAuth2Error(INVALID_NONCE_ERROR_CODE));
+				OAuth2Error oauth2Error = new OAuth2Error(INVALID_NONCE_ERROR_CODE);
+				throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
 			}
 			}
 		}
 		}
 
 
 		return Mono.just(idToken);
 		return Mono.just(idToken);
 	}
 	}
 
 
-	private String createHash(String nonce) throws NoSuchAlgorithmException {
+	static String createHash(String nonce) throws NoSuchAlgorithmException {
 		MessageDigest md = MessageDigest.getInstance("SHA-256");
 		MessageDigest md = MessageDigest.getInstance("SHA-256");
 		byte[] digest = md.digest(nonce.getBytes(StandardCharsets.US_ASCII));
 		byte[] digest = md.digest(nonce.getBytes(StandardCharsets.US_ASCII));
 		return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);
 		return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);

+ 19 - 17
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java

@@ -24,10 +24,12 @@ import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
 import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
 import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
 import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
 import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
+import org.springframework.security.oauth2.core.oidc.OidcScopes;
 import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames;
 import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames;
 import org.springframework.security.web.util.UrlUtils;
 import org.springframework.security.web.util.UrlUtils;
 import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 import org.springframework.util.Assert;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.web.util.UriComponents;
 import org.springframework.web.util.UriComponents;
 import org.springframework.web.util.UriComponentsBuilder;
 import org.springframework.web.util.UriComponentsBuilder;
@@ -63,7 +65,7 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 	private final ClientRegistrationRepository clientRegistrationRepository;
 	private final ClientRegistrationRepository clientRegistrationRepository;
 	private final AntPathRequestMatcher authorizationRequestMatcher;
 	private final AntPathRequestMatcher authorizationRequestMatcher;
 	private final StringKeyGenerator stateGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder());
 	private final StringKeyGenerator stateGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder());
-	private final StringKeyGenerator stringKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
+	private final StringKeyGenerator secureKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
 
 
 	/**
 	/**
 	 * Constructs a {@code DefaultOAuth2AuthorizationRequestResolver} using the provided parameters.
 	 * Constructs a {@code DefaultOAuth2AuthorizationRequestResolver} using the provided parameters.
@@ -121,13 +123,16 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 		if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
 		if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
 			builder = OAuth2AuthorizationRequest.authorizationCode();
 			builder = OAuth2AuthorizationRequest.authorizationCode();
 			Map<String, Object> additionalParameters = new HashMap<>();
 			Map<String, Object> additionalParameters = new HashMap<>();
-
-			addNonceParameters(attributes, additionalParameters);
-
+			if (!CollectionUtils.isEmpty(clientRegistration.getScopes()) &&
+					clientRegistration.getScopes().contains(OidcScopes.OPENID)) {
+				// Section 3.1.2.1 Authentication Request - https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
+				// scope
+				// 		REQUIRED. OpenID Connect requests MUST contain the "openid" scope value.
+				addNonceParameters(attributes, additionalParameters);
+			}
 			if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) {
 			if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) {
 				addPkceParameters(attributes, additionalParameters);
 				addPkceParameters(attributes, additionalParameters);
 			}
 			}
-
 			builder.additionalParameters(additionalParameters);
 			builder.additionalParameters(additionalParameters);
 		} else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) {
 		} else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) {
 			builder = OAuth2AuthorizationRequest.implicit();
 			builder = OAuth2AuthorizationRequest.implicit();
@@ -208,24 +213,21 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 	}
 	}
 
 
 	/**
 	/**
-	 * Creates nonce and its hash for use in OpenID Connect Authentication Requests
+	 * Creates nonce and its hash for use in OpenID Connect 1.0 Authentication Requests.
 	 *
 	 *
-	 * @param attributes where {@link OidcParameterNames#NONCE} is stored for the token request
-	 * @param additionalParameters where hash of {@link OidcParameterNames#NONCE} is added to the authentication request
+	 * @param attributes where the {@link OidcParameterNames#NONCE} is stored for the authentication request
+	 * @param additionalParameters where the {@link OidcParameterNames#NONCE} hash is added for the authentication request
 	 *
 	 *
 	 * @since 5.2
 	 * @since 5.2
-	 * @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes">15.5.2.  Nonce Implementation Notes</a>
-	 * @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation">3.1.3.7.  ID Token Validation</a>
+	 * @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest">3.1.2.1.  Authentication Request</a>
 	 */
 	 */
 	private void addNonceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
 	private void addNonceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
 		try {
 		try {
-			String nonce = this.stringKeyGenerator.generateKey();
-			attributes.put(OidcParameterNames.NONCE, nonce);
-			
+			String nonce = this.secureKeyGenerator.generateKey();
 			String nonceHash = createHash(nonce);
 			String nonceHash = createHash(nonce);
+			attributes.put(OidcParameterNames.NONCE, nonce);
 			additionalParameters.put(OidcParameterNames.NONCE, nonceHash);
 			additionalParameters.put(OidcParameterNames.NONCE, nonceHash);
-		} catch (NoSuchAlgorithmException ignored) {
-		}
+		} catch (NoSuchAlgorithmException e) { }
 	}
 	}
 
 
 	/**
 	/**
@@ -241,7 +243,7 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 	 * @see <a target="_blank" href="https://tools.ietf.org/html/rfc7636#section-4.2">4.2.  Client Creates the Code Challenge</a>
 	 * @see <a target="_blank" href="https://tools.ietf.org/html/rfc7636#section-4.2">4.2.  Client Creates the Code Challenge</a>
 	 */
 	 */
 	private void addPkceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
 	private void addPkceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
-		String codeVerifier = this.stringKeyGenerator.generateKey();
+		String codeVerifier = this.secureKeyGenerator.generateKey();
 		attributes.put(PkceParameterNames.CODE_VERIFIER, codeVerifier);
 		attributes.put(PkceParameterNames.CODE_VERIFIER, codeVerifier);
 		try {
 		try {
 			String codeChallenge = createHash(codeVerifier);
 			String codeChallenge = createHash(codeVerifier);
@@ -252,7 +254,7 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 		}
 		}
 	}
 	}
 
 
-	private String createHash(String value) throws NoSuchAlgorithmException {
+	private static String createHash(String value) throws NoSuchAlgorithmException {
 		MessageDigest md = MessageDigest.getInstance("SHA-256");
 		MessageDigest md = MessageDigest.getInstance("SHA-256");
 		byte[] digest = md.digest(value.getBytes(StandardCharsets.US_ASCII));
 		byte[] digest = md.digest(value.getBytes(StandardCharsets.US_ASCII));
 		return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);
 		return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);

+ 19 - 17
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java

@@ -27,10 +27,12 @@ import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
 import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
 import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
 import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
 import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
+import org.springframework.security.oauth2.core.oidc.OidcScopes;
 import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames;
 import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames;
 import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher;
 import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher;
 import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher;
 import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher;
 import org.springframework.util.Assert;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.ServerWebExchange;
@@ -77,7 +79,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolver
 
 
 	private final StringKeyGenerator stateGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder());
 	private final StringKeyGenerator stateGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder());
 
 
-	private final StringKeyGenerator stringKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
+	private final StringKeyGenerator secureKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
 
 
 	/**
 	/**
 	 * Creates a new instance
 	 * Creates a new instance
@@ -135,13 +137,16 @@ public class DefaultServerOAuth2AuthorizationRequestResolver
 		if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
 		if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
 			builder = OAuth2AuthorizationRequest.authorizationCode();
 			builder = OAuth2AuthorizationRequest.authorizationCode();
 			Map<String, Object> additionalParameters = new HashMap<>();
 			Map<String, Object> additionalParameters = new HashMap<>();
-
-			addNonceParameters(attributes, additionalParameters);
-
+			if (!CollectionUtils.isEmpty(clientRegistration.getScopes()) &&
+					clientRegistration.getScopes().contains(OidcScopes.OPENID)) {
+				// Section 3.1.2.1 Authentication Request - https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
+				// scope
+				// 		REQUIRED. OpenID Connect requests MUST contain the "openid" scope value.
+				addNonceParameters(attributes, additionalParameters);
+			}
 			if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) {
 			if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) {
 				addPkceParameters(attributes, additionalParameters);
 				addPkceParameters(attributes, additionalParameters);
 			}
 			}
-
 			builder.additionalParameters(additionalParameters);
 			builder.additionalParameters(additionalParameters);
 		} else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) {
 		} else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) {
 			builder = OAuth2AuthorizationRequest.implicit();
 			builder = OAuth2AuthorizationRequest.implicit();
@@ -212,24 +217,21 @@ public class DefaultServerOAuth2AuthorizationRequestResolver
 	}
 	}
 
 
 	/**
 	/**
-	 * Creates nonce and its hash for use in OpenID Connect Authentication Requests
+	 * Creates nonce and its hash for use in OpenID Connect 1.0 Authentication Requests.
 	 *
 	 *
-	 * @param attributes where {@link OidcParameterNames#NONCE} is stored for the token request
-	 * @param additionalParameters where hash of {@link OidcParameterNames#NONCE} is added to the authentication request
+	 * @param attributes where the {@link OidcParameterNames#NONCE} is stored for the authentication request
+	 * @param additionalParameters where the {@link OidcParameterNames#NONCE} hash is added for the authentication request
 	 *
 	 *
 	 * @since 5.2
 	 * @since 5.2
-	 * @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes">15.5.2.  Nonce Implementation Notes</a>
-	 * @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation">3.1.3.7.  ID Token Validation</a>
+	 * @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest">3.1.2.1.  Authentication Request</a>
 	 */
 	 */
 	private void addNonceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
 	private void addNonceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
 		try {
 		try {
-			String nonce = this.stringKeyGenerator.generateKey();
-			attributes.put(OidcParameterNames.NONCE, nonce);
-			
+			String nonce = this.secureKeyGenerator.generateKey();
 			String nonceHash = createHash(nonce);
 			String nonceHash = createHash(nonce);
+			attributes.put(OidcParameterNames.NONCE, nonce);
 			additionalParameters.put(OidcParameterNames.NONCE, nonceHash);
 			additionalParameters.put(OidcParameterNames.NONCE, nonceHash);
-		} catch (NoSuchAlgorithmException ignored) {
-		}
+		} catch (NoSuchAlgorithmException e) { }
 	}
 	}
 
 
 	/**
 	/**
@@ -245,7 +247,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolver
 	 * @see <a target="_blank" href="https://tools.ietf.org/html/rfc7636#section-4.2">4.2.  Client Creates the Code Challenge</a>
 	 * @see <a target="_blank" href="https://tools.ietf.org/html/rfc7636#section-4.2">4.2.  Client Creates the Code Challenge</a>
 	 */
 	 */
 	private void addPkceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
 	private void addPkceParameters(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
-		String codeVerifier = this.stringKeyGenerator.generateKey();
+		String codeVerifier = this.secureKeyGenerator.generateKey();
 		attributes.put(PkceParameterNames.CODE_VERIFIER, codeVerifier);
 		attributes.put(PkceParameterNames.CODE_VERIFIER, codeVerifier);
 		try {
 		try {
 			String codeChallenge = createHash(codeVerifier);
 			String codeChallenge = createHash(codeVerifier);
@@ -256,7 +258,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolver
 		}
 		}
 	}
 	}
 
 
-	private String createHash(String value) throws NoSuchAlgorithmException {
+	private static String createHash(String value) throws NoSuchAlgorithmException {
 		MessageDigest md = MessageDigest.getInstance("SHA-256");
 		MessageDigest md = MessageDigest.getInstance("SHA-256");
 		byte[] digest = md.digest(value.getBytes(StandardCharsets.US_ASCII));
 		byte[] digest = md.digest(value.getBytes(StandardCharsets.US_ASCII));
 		return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);
 		return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);

+ 31 - 79
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeAuthenticationProviderTests.java

@@ -46,8 +46,6 @@ import org.springframework.security.oauth2.jwt.Jwt;
 import org.springframework.security.oauth2.jwt.JwtDecoder;
 import org.springframework.security.oauth2.jwt.JwtDecoder;
 import org.springframework.security.oauth2.jwt.JwtException;
 import org.springframework.security.oauth2.jwt.JwtException;
 
 
-import java.nio.charset.StandardCharsets;
-import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.NoSuchAlgorithmException;
 import java.time.Instant;
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.Arrays;
@@ -64,10 +62,12 @@ import static org.hamcrest.CoreMatchers.containsString;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.when;
+import static org.springframework.security.oauth2.client.oidc.authentication.OidcAuthorizationCodeAuthenticationProvider.createHash;
 import static org.springframework.security.oauth2.client.registration.TestClientRegistrations.clientRegistration;
 import static org.springframework.security.oauth2.client.registration.TestClientRegistrations.clientRegistration;
 import static org.springframework.security.oauth2.core.endpoint.TestOAuth2AuthorizationRequests.request;
 import static org.springframework.security.oauth2.core.endpoint.TestOAuth2AuthorizationRequests.request;
 import static org.springframework.security.oauth2.core.endpoint.TestOAuth2AuthorizationResponses.error;
 import static org.springframework.security.oauth2.core.endpoint.TestOAuth2AuthorizationResponses.error;
 import static org.springframework.security.oauth2.core.endpoint.TestOAuth2AuthorizationResponses.success;
 import static org.springframework.security.oauth2.core.endpoint.TestOAuth2AuthorizationResponses.success;
+import static org.springframework.security.oauth2.jwt.TestJwts.jwt;
 
 
 /**
 /**
  * Tests for {@link OidcAuthorizationCodeAuthenticationProvider}.
  * Tests for {@link OidcAuthorizationCodeAuthenticationProvider}.
@@ -84,8 +84,7 @@ public class OidcAuthorizationCodeAuthenticationProviderTests {
 	private OAuth2AccessTokenResponse accessTokenResponse;
 	private OAuth2AccessTokenResponse accessTokenResponse;
 	private OAuth2UserService<OidcUserRequest, OidcUser> userService;
 	private OAuth2UserService<OidcUserRequest, OidcUser> userService;
 	private OidcAuthorizationCodeAuthenticationProvider authenticationProvider;
 	private OidcAuthorizationCodeAuthenticationProvider authenticationProvider;
-	private StringKeyGenerator stringKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
-	private String nonce = this.stringKeyGenerator.generateKey();
+	private StringKeyGenerator secureKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
 	private String nonceHash;
 	private String nonceHash;
 
 
 	@Rule
 	@Rule
@@ -94,16 +93,15 @@ public class OidcAuthorizationCodeAuthenticationProviderTests {
 	@Before
 	@Before
 	@SuppressWarnings("unchecked")
 	@SuppressWarnings("unchecked")
 	public void setUp() {
 	public void setUp() {
-		try {
-			nonceHash = createHash(nonce);
-		} catch (NoSuchAlgorithmException e) {
-			e.printStackTrace();
-		}
+		this.clientRegistration = clientRegistration().clientId("client1").build();
 		Map<String, Object> attributes = new HashMap<>();
 		Map<String, Object> attributes = new HashMap<>();
 		Map<String, Object> additionalParameters = new HashMap<>();
 		Map<String, Object> additionalParameters = new HashMap<>();
-		addNonceToRequest(attributes, additionalParameters);
-
-		this.clientRegistration = clientRegistration().clientId("client1").build();
+		try {
+			String nonce = this.secureKeyGenerator.generateKey();
+			this.nonceHash = createHash(nonce);
+			attributes.put(OidcParameterNames.NONCE, nonce);
+			additionalParameters.put(OidcParameterNames.NONCE, this.nonceHash);
+		} catch (NoSuchAlgorithmException e) { }
 		this.authorizationRequest = request()
 		this.authorizationRequest = request()
 				.scope("openid", "profile", "email")
 				.scope("openid", "profile", "email")
 				.attributes(attributes)
 				.attributes(attributes)
@@ -240,6 +238,23 @@ public class OidcAuthorizationCodeAuthenticationProviderTests {
 				new OAuth2LoginAuthenticationToken(this.clientRegistration, this.authorizationExchange));
 				new OAuth2LoginAuthenticationToken(this.clientRegistration, this.authorizationExchange));
 	}
 	}
 
 
+	@Test
+	public void authenticateWhenIdTokenInvalidNonceThenThrowOAuth2AuthenticationException() {
+		this.exception.expect(OAuth2AuthenticationException.class);
+		this.exception.expectMessage(containsString("[invalid_nonce]"));
+
+		Map<String, Object> claims = new HashMap<>();
+		claims.put(IdTokenClaimNames.ISS, "https://provider.com");
+		claims.put(IdTokenClaimNames.SUB, "subject1");
+		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client1", "client2"));
+		claims.put(IdTokenClaimNames.AZP, "client1");
+		claims.put(IdTokenClaimNames.NONCE, "invalid-nonce-hash");
+		this.setUpIdToken(claims);
+
+		this.authenticationProvider.authenticate(
+				new OAuth2LoginAuthenticationToken(this.clientRegistration, this.authorizationExchange));
+	}
+
 	@Test
 	@Test
 	public void authenticateWhenLoginSuccessThenReturnAuthentication() {
 	public void authenticateWhenLoginSuccessThenReturnAuthentication() {
 		Map<String, Object> claims = new HashMap<>();
 		Map<String, Object> claims = new HashMap<>();
@@ -247,7 +262,7 @@ public class OidcAuthorizationCodeAuthenticationProviderTests {
 		claims.put(IdTokenClaimNames.SUB, "subject1");
 		claims.put(IdTokenClaimNames.SUB, "subject1");
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client1", "client2"));
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client1", "client2"));
 		claims.put(IdTokenClaimNames.AZP, "client1");
 		claims.put(IdTokenClaimNames.AZP, "client1");
-		claims.put(IdTokenClaimNames.NONCE, nonceHash);
+		claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
 		this.setUpIdToken(claims);
 		this.setUpIdToken(claims);
 
 
 		OidcUser principal = mock(OidcUser.class);
 		OidcUser principal = mock(OidcUser.class);
@@ -277,7 +292,7 @@ public class OidcAuthorizationCodeAuthenticationProviderTests {
 		claims.put(IdTokenClaimNames.SUB, "subject1");
 		claims.put(IdTokenClaimNames.SUB, "subject1");
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client1", "client2"));
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client1", "client2"));
 		claims.put(IdTokenClaimNames.AZP, "client1");
 		claims.put(IdTokenClaimNames.AZP, "client1");
-		claims.put(IdTokenClaimNames.NONCE, nonceHash);
+		claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
 		this.setUpIdToken(claims);
 		this.setUpIdToken(claims);
 
 
 		OidcUser principal = mock(OidcUser.class);
 		OidcUser principal = mock(OidcUser.class);
@@ -307,7 +322,7 @@ public class OidcAuthorizationCodeAuthenticationProviderTests {
 		claims.put(IdTokenClaimNames.SUB, "subject1");
 		claims.put(IdTokenClaimNames.SUB, "subject1");
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client1", "client2"));
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client1", "client2"));
 		claims.put(IdTokenClaimNames.AZP, "client1");
 		claims.put(IdTokenClaimNames.AZP, "client1");
-		claims.put(IdTokenClaimNames.NONCE, nonceHash);
+		claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
 		this.setUpIdToken(claims);
 		this.setUpIdToken(claims);
 
 
 		OidcUser principal = mock(OidcUser.class);
 		OidcUser principal = mock(OidcUser.class);
@@ -324,49 +339,8 @@ public class OidcAuthorizationCodeAuthenticationProviderTests {
 				this.accessTokenResponse.getAdditionalParameters());
 				this.accessTokenResponse.getAdditionalParameters());
 	}
 	}
 
 
-	// gh-4442
-	@Test
-	public void authenticateWhenTokenSuccessResponseThenAdditionalParametersAddedToUserRequestNoNonce() {
-		OAuth2AuthorizationRequest authorizationRequestNoNonce = request()
-				.scope("openid", "profile", "email")
-				.attributes(new HashMap<>())
-				.additionalParameters(new HashMap<>())
-				.build();
-		OAuth2AuthorizationExchange authorizationExchangeNoNonce = new OAuth2AuthorizationExchange(authorizationRequestNoNonce, this.authorizationResponse);
-
-		Map<String, Object> claims = new HashMap<>();
-		claims.put(IdTokenClaimNames.ISS, "https://provider.com");
-		claims.put(IdTokenClaimNames.SUB, "subject1");
-		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client1", "client2"));
-		claims.put(IdTokenClaimNames.AZP, "client1");
-		this.setUpIdToken(claims);
-
-		OidcUser principal = mock(OidcUser.class);
-		List<GrantedAuthority> authorities = AuthorityUtils.createAuthorityList("ROLE_USER");
-		when(principal.getAuthorities()).thenAnswer(
-				(Answer<List<GrantedAuthority>>) invocation -> authorities);
-		ArgumentCaptor<OidcUserRequest> userRequestArgCaptor = ArgumentCaptor.forClass(OidcUserRequest.class);
-		when(this.userService.loadUser(userRequestArgCaptor.capture())).thenReturn(principal);
-
-		this.authenticationProvider.authenticate(new OAuth2LoginAuthenticationToken(
-				this.clientRegistration, authorizationExchangeNoNonce));
-
-		assertThat(userRequestArgCaptor.getValue().getAdditionalParameters()).containsAllEntriesOf(
-				this.accessTokenResponse.getAdditionalParameters());
-	}
-
 	private void setUpIdToken(Map<String, Object> claims) {
 	private void setUpIdToken(Map<String, Object> claims) {
-		Jwt idToken = Jwt.withTokenValue("token")
-				.header("alg", "none")
-				.audience(Collections.singletonList("https://audience.example.org"))
-				.expiresAt(Instant.MAX)
-				.issuedAt(Instant.MIN)
-				.issuer("https://issuer.example.org")
-				.jti("jti")
-				.notBefore(Instant.MIN)
-				.subject("mock-test-subject")
-				.claims(c -> c.putAll(claims))
-				.build();
+		Jwt idToken = jwt().claims(c -> c.putAll(claims)).build();
 		JwtDecoder jwtDecoder = mock(JwtDecoder.class);
 		JwtDecoder jwtDecoder = mock(JwtDecoder.class);
 		when(jwtDecoder.decode(anyString())).thenReturn(idToken);
 		when(jwtDecoder.decode(anyString())).thenReturn(idToken);
 		this.authenticationProvider.setJwtDecoderFactory(registration -> jwtDecoder);
 		this.authenticationProvider.setJwtDecoderFactory(registration -> jwtDecoder);
@@ -379,7 +353,6 @@ public class OidcAuthorizationCodeAuthenticationProviderTests {
 		additionalParameters.put("param1", "value1");
 		additionalParameters.put("param1", "value1");
 		additionalParameters.put("param2", "value2");
 		additionalParameters.put("param2", "value2");
 		additionalParameters.put(OidcParameterNames.ID_TOKEN, "id-token");
 		additionalParameters.put(OidcParameterNames.ID_TOKEN, "id-token");
-		additionalParameters.put(IdTokenClaimNames.NONCE, nonceHash);
 
 
 		return OAuth2AccessTokenResponse
 		return OAuth2AccessTokenResponse
 				.withToken("access-token-1234")
 				.withToken("access-token-1234")
@@ -391,25 +364,4 @@ public class OidcAuthorizationCodeAuthenticationProviderTests {
 				.build();
 				.build();
 
 
 	}
 	}
-
-	/**
-	 * Adds nonce for use in OpenID Connect Authentication Requests
-	 *
-	 * @param attributes where {@link IdTokenClaimNames#NONCE} is stored for the token request
-	 * @param additionalParameters where the hash of {@link IdTokenClaimNames#NONCE} is added to be used in the authentication request
-	 *
-	 * @since 5.2
-	 * @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes">15.5.2.  Nonce Implementation Notes</a>
-	 * @see <a target="_blank" href="https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation">3.1.3.7.  ID Token Validation</a>
-	 */
-	private void addNonceToRequest(Map<String, Object> attributes, Map<String, Object> additionalParameters) {
-		attributes.put(IdTokenClaimNames.NONCE, nonce);
-		additionalParameters.put(IdTokenClaimNames.NONCE, nonceHash);
-	}
-
-	private String createHash(String nonce) throws NoSuchAlgorithmException {
-		MessageDigest md = MessageDigest.getInstance("SHA-256");
-		byte[] digest = md.digest(nonce.getBytes(StandardCharsets.US_ASCII));
-		return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);
-	}
 }
 }

+ 70 - 16
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcAuthorizationCodeReactiveAuthenticationManagerTests.java

@@ -1,5 +1,5 @@
 /*
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 the original author or authors.
  *
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * you may not use this file except in compliance with the License.
@@ -16,22 +16,16 @@
 
 
 package org.springframework.security.oauth2.client.oidc.authentication;
 package org.springframework.security.oauth2.client.oidc.authentication;
 
 
-import java.time.Instant;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
 import org.junit.Before;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.mockito.junit.MockitoJUnitRunner;
-import reactor.core.publisher.Mono;
-
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.authority.AuthorityUtils;
+import org.springframework.security.crypto.keygen.Base64StringKeyGenerator;
+import org.springframework.security.crypto.keygen.StringKeyGenerator;
 import org.springframework.security.oauth2.client.authentication.OAuth2AuthorizationCodeAuthenticationToken;
 import org.springframework.security.oauth2.client.authentication.OAuth2AuthorizationCodeAuthenticationToken;
 import org.springframework.security.oauth2.client.authentication.OAuth2LoginAuthenticationToken;
 import org.springframework.security.oauth2.client.authentication.OAuth2LoginAuthenticationToken;
 import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest;
 import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest;
@@ -54,16 +48,25 @@ import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 import org.springframework.security.oauth2.jwt.Jwt;
 import org.springframework.security.oauth2.jwt.Jwt;
 import org.springframework.security.oauth2.jwt.JwtException;
 import org.springframework.security.oauth2.jwt.JwtException;
 import org.springframework.security.oauth2.jwt.ReactiveJwtDecoder;
 import org.springframework.security.oauth2.jwt.ReactiveJwtDecoder;
+import reactor.core.publisher.Mono;
+
+import java.security.NoSuchAlgorithmException;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Base64;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatCode;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.when;
+import static org.springframework.security.oauth2.client.oidc.authentication.OidcAuthorizationCodeReactiveAuthenticationManager.createHash;
 import static org.springframework.security.oauth2.jwt.TestJwts.jwt;
 import static org.springframework.security.oauth2.jwt.TestJwts.jwt;
 
 
 /**
 /**
  * @author Rob Winch
  * @author Rob Winch
+ * @author Joe Grandja
  * @since 5.1
  * @since 5.1
  */
  */
 @RunWith(MockitoJUnitRunner.class)
 @RunWith(MockitoJUnitRunner.class)
@@ -89,6 +92,10 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 
 
 	private OidcAuthorizationCodeReactiveAuthenticationManager manager;
 	private OidcAuthorizationCodeReactiveAuthenticationManager manager;
 
 
+	private StringKeyGenerator secureKeyGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96);
+
+	private String nonceHash;
+
 	@Before
 	@Before
 	public void setup() {
 	public void setup() {
 		this.manager = new OidcAuthorizationCodeReactiveAuthenticationManager(this.accessTokenResponseClient, this.userService);
 		this.manager = new OidcAuthorizationCodeReactiveAuthenticationManager(this.accessTokenResponseClient, this.userService);
@@ -164,6 +171,31 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 				.hasMessageContaining("[invalid_id_token] ID Token Validation Error");
 				.hasMessageContaining("[invalid_id_token] ID Token Validation Error");
 	}
 	}
 
 
+	@Test
+	public void authenticateWhenIdTokenInvalidNonceThenOAuth2AuthenticationException() {
+		OAuth2AccessTokenResponse accessTokenResponse = OAuth2AccessTokenResponse.withToken("foo")
+				.tokenType(OAuth2AccessToken.TokenType.BEARER)
+				.additionalParameters(Collections.singletonMap(OidcParameterNames.ID_TOKEN, this.idToken.getTokenValue()))
+				.build();
+
+		OAuth2AuthorizationCodeAuthenticationToken authorizationCodeAuthentication = loginToken();
+
+		Map<String, Object> claims = new HashMap<>();
+		claims.put(IdTokenClaimNames.ISS, "https://issuer.example.com");
+		claims.put(IdTokenClaimNames.SUB, "sub");
+		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id"));
+		claims.put(IdTokenClaimNames.NONCE, "invalid-nonce-hash");
+		Jwt idToken = jwt().claims(c -> c.putAll(claims)).build();
+
+		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
+		when(this.jwtDecoder.decode(any())).thenReturn(Mono.just(idToken));
+		this.manager.setJwtDecoderFactory(c -> this.jwtDecoder);
+
+		assertThatThrownBy(() -> this.manager.authenticate(authorizationCodeAuthentication).block())
+				.isInstanceOf(OAuth2AuthenticationException.class)
+				.hasMessageContaining("[invalid_nonce]");
+	}
+
 	@Test
 	@Test
 	public void authenticationWhenOAuth2UserNotFoundThenEmpty() {
 	public void authenticationWhenOAuth2UserNotFoundThenEmpty() {
 		OAuth2AccessTokenResponse accessTokenResponse = OAuth2AccessTokenResponse.withToken("foo")
 		OAuth2AccessTokenResponse accessTokenResponse = OAuth2AccessTokenResponse.withToken("foo")
@@ -171,17 +203,20 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 				.additionalParameters(Collections.singletonMap(OidcParameterNames.ID_TOKEN, "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ."))
 				.additionalParameters(Collections.singletonMap(OidcParameterNames.ID_TOKEN, "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ."))
 				.build();
 				.build();
 
 
+		OAuth2AuthorizationCodeAuthenticationToken authorizationCodeAuthentication = loginToken();
+
 		Map<String, Object> claims = new HashMap<>();
 		Map<String, Object> claims = new HashMap<>();
 		claims.put(IdTokenClaimNames.ISS, "https://issuer.example.com");
 		claims.put(IdTokenClaimNames.ISS, "https://issuer.example.com");
 		claims.put(IdTokenClaimNames.SUB, "rob");
 		claims.put(IdTokenClaimNames.SUB, "rob");
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id"));
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id"));
+		claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
 		Jwt idToken = jwt().claims(c -> c.putAll(claims)).build();
 		Jwt idToken = jwt().claims(c -> c.putAll(claims)).build();
 
 
 		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
 		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
 		when(this.userService.loadUser(any())).thenReturn(Mono.empty());
 		when(this.userService.loadUser(any())).thenReturn(Mono.empty());
 		when(this.jwtDecoder.decode(any())).thenReturn(Mono.just(idToken));
 		when(this.jwtDecoder.decode(any())).thenReturn(Mono.just(idToken));
 		this.manager.setJwtDecoderFactory(c -> this.jwtDecoder);
 		this.manager.setJwtDecoderFactory(c -> this.jwtDecoder);
-		assertThat(this.manager.authenticate(loginToken()).block()).isNull();
+		assertThat(this.manager.authenticate(authorizationCodeAuthentication).block()).isNull();
 	}
 	}
 
 
 	@Test
 	@Test
@@ -191,10 +226,13 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 				.additionalParameters(Collections.singletonMap(OidcParameterNames.ID_TOKEN, this.idToken.getTokenValue()))
 				.additionalParameters(Collections.singletonMap(OidcParameterNames.ID_TOKEN, this.idToken.getTokenValue()))
 				.build();
 				.build();
 
 
+		OAuth2AuthorizationCodeAuthenticationToken authorizationCodeAuthentication = loginToken();
+
 		Map<String, Object> claims = new HashMap<>();
 		Map<String, Object> claims = new HashMap<>();
 		claims.put(IdTokenClaimNames.ISS, "https://issuer.example.com");
 		claims.put(IdTokenClaimNames.ISS, "https://issuer.example.com");
 		claims.put(IdTokenClaimNames.SUB, "rob");
 		claims.put(IdTokenClaimNames.SUB, "rob");
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id"));
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id"));
+		claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
 		Jwt idToken = jwt().claims(c -> c.putAll(claims)).build();
 		Jwt idToken = jwt().claims(c -> c.putAll(claims)).build();
 
 
 		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
 		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
@@ -203,7 +241,7 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 		when(this.jwtDecoder.decode(any())).thenReturn(Mono.just(idToken));
 		when(this.jwtDecoder.decode(any())).thenReturn(Mono.just(idToken));
 		this.manager.setJwtDecoderFactory(c -> this.jwtDecoder);
 		this.manager.setJwtDecoderFactory(c -> this.jwtDecoder);
 
 
-		OAuth2LoginAuthenticationToken result = (OAuth2LoginAuthenticationToken) this.manager.authenticate(loginToken()).block();
+		OAuth2LoginAuthenticationToken result = (OAuth2LoginAuthenticationToken) this.manager.authenticate(authorizationCodeAuthentication).block();
 
 
 		assertThat(result.getPrincipal()).isEqualTo(user);
 		assertThat(result.getPrincipal()).isEqualTo(user);
 		assertThat(result.getAuthorities()).containsOnlyElementsOf(user.getAuthorities());
 		assertThat(result.getAuthorities()).containsOnlyElementsOf(user.getAuthorities());
@@ -218,10 +256,13 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 				.refreshToken("refresh-token")
 				.refreshToken("refresh-token")
 				.build();
 				.build();
 
 
+		OAuth2AuthorizationCodeAuthenticationToken authorizationCodeAuthentication = loginToken();
+
 		Map<String, Object> claims = new HashMap<>();
 		Map<String, Object> claims = new HashMap<>();
 		claims.put(IdTokenClaimNames.ISS, "https://issuer.example.com");
 		claims.put(IdTokenClaimNames.ISS, "https://issuer.example.com");
 		claims.put(IdTokenClaimNames.SUB, "rob");
 		claims.put(IdTokenClaimNames.SUB, "rob");
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id"));
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id"));
+		claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
 		Jwt idToken = jwt().claims(c -> c.putAll(claims)).build();
 		Jwt idToken = jwt().claims(c -> c.putAll(claims)).build();
 
 
 		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
 		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
@@ -230,7 +271,7 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 		when(this.jwtDecoder.decode(any())).thenReturn(Mono.just(idToken));
 		when(this.jwtDecoder.decode(any())).thenReturn(Mono.just(idToken));
 		this.manager.setJwtDecoderFactory(c -> this.jwtDecoder);
 		this.manager.setJwtDecoderFactory(c -> this.jwtDecoder);
 
 
-		OAuth2LoginAuthenticationToken result = (OAuth2LoginAuthenticationToken) this.manager.authenticate(loginToken()).block();
+		OAuth2LoginAuthenticationToken result = (OAuth2LoginAuthenticationToken) this.manager.authenticate(authorizationCodeAuthentication).block();
 
 
 		assertThat(result.getPrincipal()).isEqualTo(user);
 		assertThat(result.getPrincipal()).isEqualTo(user);
 		assertThat(result.getAuthorities()).containsOnlyElementsOf(user.getAuthorities());
 		assertThat(result.getAuthorities()).containsOnlyElementsOf(user.getAuthorities());
@@ -251,10 +292,13 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 				.additionalParameters(additionalParameters)
 				.additionalParameters(additionalParameters)
 				.build();
 				.build();
 
 
+		OAuth2AuthorizationCodeAuthenticationToken authorizationCodeAuthentication = loginToken();
+
 		Map<String, Object> claims = new HashMap<>();
 		Map<String, Object> claims = new HashMap<>();
 		claims.put(IdTokenClaimNames.ISS, "https://issuer.example.com");
 		claims.put(IdTokenClaimNames.ISS, "https://issuer.example.com");
 		claims.put(IdTokenClaimNames.SUB, "rob");
 		claims.put(IdTokenClaimNames.SUB, "rob");
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList(clientRegistration.getClientId()));
 		claims.put(IdTokenClaimNames.AUD, Arrays.asList(clientRegistration.getClientId()));
+		claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
 		Jwt idToken = jwt().claims(c -> c.putAll(claims)).build();
 		Jwt idToken = jwt().claims(c -> c.putAll(claims)).build();
 
 
 		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
 		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
@@ -264,7 +308,7 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 		when(this.jwtDecoder.decode(any())).thenReturn(Mono.just(idToken));
 		when(this.jwtDecoder.decode(any())).thenReturn(Mono.just(idToken));
 		this.manager.setJwtDecoderFactory(c -> this.jwtDecoder);
 		this.manager.setJwtDecoderFactory(c -> this.jwtDecoder);
 
 
-		this.manager.authenticate(loginToken()).block();
+		this.manager.authenticate(authorizationCodeAuthentication).block();
 
 
 		assertThat(userRequestArgCaptor.getValue().getAdditionalParameters())
 		assertThat(userRequestArgCaptor.getValue().getAdditionalParameters())
 				.containsAllEntriesOf(accessTokenResponse.getAdditionalParameters());
 				.containsAllEntriesOf(accessTokenResponse.getAdditionalParameters());
@@ -272,6 +316,14 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 
 
 	private OAuth2AuthorizationCodeAuthenticationToken loginToken() {
 	private OAuth2AuthorizationCodeAuthenticationToken loginToken() {
 		ClientRegistration clientRegistration = this.registration.build();
 		ClientRegistration clientRegistration = this.registration.build();
+		Map<String, Object> attributes = new HashMap<>();
+		Map<String, Object> additionalParameters = new HashMap<>();
+		try {
+			String nonce = this.secureKeyGenerator.generateKey();
+			this.nonceHash = createHash(nonce);
+			attributes.put(OidcParameterNames.NONCE, nonce);
+			additionalParameters.put(OidcParameterNames.NONCE, this.nonceHash);
+		} catch (NoSuchAlgorithmException e) { }
 		OAuth2AuthorizationRequest authorizationRequest = OAuth2AuthorizationRequest
 		OAuth2AuthorizationRequest authorizationRequest = OAuth2AuthorizationRequest
 				.authorizationCode()
 				.authorizationCode()
 				.state("state")
 				.state("state")
@@ -279,6 +331,8 @@ public class OidcAuthorizationCodeReactiveAuthenticationManagerTests {
 				.authorizationUri(clientRegistration.getProviderDetails().getAuthorizationUri())
 				.authorizationUri(clientRegistration.getProviderDetails().getAuthorizationUri())
 				.redirectUri(clientRegistration.getRedirectUriTemplate())
 				.redirectUri(clientRegistration.getRedirectUriTemplate())
 				.scopes(clientRegistration.getScopes())
 				.scopes(clientRegistration.getScopes())
+				.additionalParameters(additionalParameters)
+				.attributes(attributes)
 				.build();
 				.build();
 		OAuth2AuthorizationResponse authorizationResponse = this.authorizationResponseBldr
 		OAuth2AuthorizationResponse authorizationResponse = this.authorizationResponseBldr
 				.redirectUri(clientRegistration.getRedirectUriTemplate())
 				.redirectUri(clientRegistration.getRedirectUriTemplate())

+ 1 - 0
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenDecoderFactoryTests.java

@@ -59,6 +59,7 @@ public class OidcIdTokenDecoderFactoryTests {
 		Map<String, Converter<Object, ?>> claimTypeConverters = OidcIdTokenDecoderFactory.createDefaultClaimTypeConverters();
 		Map<String, Converter<Object, ?>> claimTypeConverters = OidcIdTokenDecoderFactory.createDefaultClaimTypeConverters();
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.ISS);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.ISS);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.AUD);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.AUD);
+		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.NONCE);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.EXP);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.EXP);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.IAT);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.IAT);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.AUTH_TIME);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.AUTH_TIME);

+ 1 - 0
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/ReactiveOidcIdTokenDecoderFactoryTests.java

@@ -59,6 +59,7 @@ public class ReactiveOidcIdTokenDecoderFactoryTests {
 		Map<String, Converter<Object, ?>> claimTypeConverters = ReactiveOidcIdTokenDecoderFactory.createDefaultClaimTypeConverters();
 		Map<String, Converter<Object, ?>> claimTypeConverters = ReactiveOidcIdTokenDecoderFactory.createDefaultClaimTypeConverters();
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.ISS);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.ISS);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.AUD);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.AUD);
+		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.NONCE);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.EXP);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.EXP);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.IAT);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.IAT);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.AUTH_TIME);
 		assertThat(claimTypeConverters).containsKey(IdTokenClaimNames.AUTH_TIME);

+ 54 - 27
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java

@@ -24,24 +24,26 @@ import org.springframework.security.oauth2.client.registration.InMemoryClientReg
 import org.springframework.security.oauth2.client.registration.TestClientRegistrations;
 import org.springframework.security.oauth2.client.registration.TestClientRegistrations;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
-import org.springframework.security.oauth2.core.endpoint.*;
-import org.springframework.security.oauth2.core.oidc.IdTokenClaimNames;
+import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
+import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponseType;
+import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
+import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
+import org.springframework.security.oauth2.core.oidc.OidcScopes;
+import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames;
 
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.assertj.core.api.Assertions.entry;
+import static org.assertj.core.api.Assertions.*;
 
 
 /**
 /**
  * Tests for {@link DefaultOAuth2AuthorizationRequestResolver}.
  * Tests for {@link DefaultOAuth2AuthorizationRequestResolver}.
  *
  *
  * @author Joe Grandja
  * @author Joe Grandja
- * @author Mark Heckler
  */
  */
 public class DefaultOAuth2AuthorizationRequestResolverTests {
 public class DefaultOAuth2AuthorizationRequestResolverTests {
 	private ClientRegistration registration1;
 	private ClientRegistration registration1;
 	private ClientRegistration registration2;
 	private ClientRegistration registration2;
 	private ClientRegistration fineRedirectUriTemplateRegistration;
 	private ClientRegistration fineRedirectUriTemplateRegistration;
 	private ClientRegistration pkceRegistration;
 	private ClientRegistration pkceRegistration;
+	private ClientRegistration oidcRegistration;
 	private ClientRegistrationRepository clientRegistrationRepository;
 	private ClientRegistrationRepository clientRegistrationRepository;
 	private final String authorizationRequestBaseUri = "/oauth2/authorization";
 	private final String authorizationRequestBaseUri = "/oauth2/authorization";
 	private DefaultOAuth2AuthorizationRequestResolver resolver;
 	private DefaultOAuth2AuthorizationRequestResolver resolver;
@@ -57,9 +59,12 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 				.clientAuthenticationMethod(ClientAuthenticationMethod.NONE)
 				.clientAuthenticationMethod(ClientAuthenticationMethod.NONE)
 				.clientSecret(null)
 				.clientSecret(null)
 				.build();
 				.build();
-
+		this.oidcRegistration = TestClientRegistrations.clientRegistration()
+				.registrationId("oidc-registration-id")
+				.scope(OidcScopes.OPENID).build();
 		this.clientRegistrationRepository = new InMemoryClientRegistrationRepository(
 		this.clientRegistrationRepository = new InMemoryClientRegistrationRepository(
-				this.registration1, this.registration2, this.fineRedirectUriTemplateRegistration, this.pkceRegistration);
+				this.registration1, this.registration2, this.fineRedirectUriTemplateRegistration,
+				this.pkceRegistration, this.oidcRegistration);
 		this.resolver = new DefaultOAuth2AuthorizationRequestResolver(
 		this.resolver = new DefaultOAuth2AuthorizationRequestResolver(
 				this.clientRegistrationRepository, this.authorizationRequestBaseUri);
 				this.clientRegistrationRepository, this.authorizationRequestBaseUri);
 	}
 	}
@@ -118,14 +123,12 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getState()).isNotNull();
 		assertThat(authorizationRequest.getState()).isNotNull();
 		assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey(OAuth2ParameterNames.REGISTRATION_ID);
 		assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey(OAuth2ParameterNames.REGISTRATION_ID);
 		assertThat(authorizationRequest.getAttributes())
 		assertThat(authorizationRequest.getAttributes())
-				.containsOnlyKeys(OAuth2ParameterNames.REGISTRATION_ID, IdTokenClaimNames.NONCE)
-				.contains(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
+				.containsExactly(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 				.matches("https://example.com/login/oauth/authorize\\?" +
 				.matches("https://example.com/login/oauth/authorize\\?" +
 						"response_type=code&client_id=client-id&" +
 						"response_type=code&client_id=client-id&" +
 						"scope=read:user&state=.{15,}&" +
 						"scope=read:user&state=.{15,}&" +
-						"redirect_uri=http://localhost/login/oauth2/code/registration-id&" +
-						"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+						"redirect_uri=http://localhost/login/oauth2/code/registration-id");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -138,8 +141,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request, clientRegistration.getRegistrationId());
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request, clientRegistration.getRegistrationId());
 		assertThat(authorizationRequest).isNotNull();
 		assertThat(authorizationRequest).isNotNull();
 		assertThat(authorizationRequest.getAttributes())
 		assertThat(authorizationRequest.getAttributes())
-				.containsOnlyKeys(OAuth2ParameterNames.REGISTRATION_ID, IdTokenClaimNames.NONCE)
-				.contains(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
+				.containsExactly(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
 	}
 	}
 
 
 	@Test
 	@Test
@@ -261,8 +263,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 				.matches("https://example.com/login/oauth/authorize\\?" +
 				.matches("https://example.com/login/oauth/authorize\\?" +
 						"response_type=code&client_id=client-id&" +
 						"response_type=code&client_id=client-id&" +
 						"scope=read:user&state=.{15,}&" +
 						"scope=read:user&state=.{15,}&" +
-						"redirect_uri=http://localhost/login/oauth2/code/registration-id&" +
-						"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+						"redirect_uri=http://localhost/login/oauth2/code/registration-id");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -280,8 +281,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 				.matches("https://example.com/login/oauth/authorize\\?" +
 				.matches("https://example.com/login/oauth/authorize\\?" +
 						"response_type=code&client_id=client-id&" +
 						"response_type=code&client_id=client-id&" +
 						"scope=read:user&state=.{15,}&" +
 						"scope=read:user&state=.{15,}&" +
-						"redirect_uri=https://example.com/login/oauth2/code/registration-id&" +
-						"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+						"redirect_uri=https://example.com/login/oauth2/code/registration-id");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -296,8 +296,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 				.matches("https://example.com/login/oauth/authorize\\?" +
 				.matches("https://example.com/login/oauth/authorize\\?" +
 						"response_type=code&client_id=client-id&" +
 						"response_type=code&client_id=client-id&" +
 						"scope=read:user&state=.{15,}&" +
 						"scope=read:user&state=.{15,}&" +
-						"redirect_uri=http://localhost/authorize/oauth2/code/registration-id&" +
-						"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+						"redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -312,8 +311,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 				.matches("https://example.com/login/oauth/authorize\\?" +
 				.matches("https://example.com/login/oauth/authorize\\?" +
 						"response_type=code&client_id=client-id-2&" +
 						"response_type=code&client_id=client-id-2&" +
 						"scope=read:user&state=.{15,}&" +
 						"scope=read:user&state=.{15,}&" +
-						"redirect_uri=http://localhost/login/oauth2/code/registration-id-2&" +
-						"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+						"redirect_uri=http://localhost/login/oauth2/code/registration-id-2");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -329,8 +327,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 				.matches("https://example.com/login/oauth/authorize\\?" +
 				.matches("https://example.com/login/oauth/authorize\\?" +
 						"response_type=code&client_id=client-id&" +
 						"response_type=code&client_id=client-id&" +
 						"scope=read:user&state=.{15,}&" +
 						"scope=read:user&state=.{15,}&" +
-						"redirect_uri=http://localhost/authorize/oauth2/code/registration-id&" +
-						"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+						"redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -346,8 +343,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 				.matches("https://example.com/login/oauth/authorize\\?" +
 				.matches("https://example.com/login/oauth/authorize\\?" +
 						"response_type=code&client_id=client-id-2&" +
 						"response_type=code&client_id=client-id-2&" +
 						"scope=read:user&state=.{15,}&" +
 						"scope=read:user&state=.{15,}&" +
-						"redirect_uri=http://localhost/login/oauth2/code/registration-id-2&" +
-						"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+						"redirect_uri=http://localhost/login/oauth2/code/registration-id-2");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -383,10 +379,41 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 						"scope=read:user&state=.{15,}&" +
 						"scope=read:user&state=.{15,}&" +
 						"redirect_uri=http://localhost/login/oauth2/code/pkce-client-registration-id&" +
 						"redirect_uri=http://localhost/login/oauth2/code/pkce-client-registration-id&" +
 						"code_challenge_method=S256&" +
 						"code_challenge_method=S256&" +
-						"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" +
 						"code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
 						"code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
 	}
 	}
 
 
+	@Test
+	public void resolveWhenAuthenticationRequestWithValidOidcClientThenResolves() {
+		ClientRegistration clientRegistration = this.oidcRegistration;
+		String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
+		request.setServletPath(requestUri);
+
+		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
+		assertThat(authorizationRequest).isNotNull();
+		assertThat(authorizationRequest.getAuthorizationUri()).isEqualTo(
+				clientRegistration.getProviderDetails().getAuthorizationUri());
+		assertThat(authorizationRequest.getGrantType()).isEqualTo(AuthorizationGrantType.AUTHORIZATION_CODE);
+		assertThat(authorizationRequest.getResponseType()).isEqualTo(OAuth2AuthorizationResponseType.CODE);
+		assertThat(authorizationRequest.getClientId()).isEqualTo(clientRegistration.getClientId());
+		assertThat(authorizationRequest.getRedirectUri())
+				.isEqualTo("http://localhost/login/oauth2/code/" + clientRegistration.getRegistrationId());
+		assertThat(authorizationRequest.getScopes()).isEqualTo(clientRegistration.getScopes());
+		assertThat(authorizationRequest.getState()).isNotNull();
+		assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey(OAuth2ParameterNames.REGISTRATION_ID);
+		assertThat(authorizationRequest.getAdditionalParameters()).containsKey(OidcParameterNames.NONCE);
+		assertThat(authorizationRequest.getAttributes())
+				.contains(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
+		assertThat(authorizationRequest.getAttributes()).containsKey(OidcParameterNames.NONCE);
+		assertThat((String) authorizationRequest.getAttribute(OidcParameterNames.NONCE)).matches("^([a-zA-Z0-9\\-\\.\\_\\~]){128}$");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" +
+						"response_type=code&client_id=client-id&" +
+						"scope=openid&state=.{15,}&" +
+						"redirect_uri=http://localhost/login/oauth2/code/oidc-registration-id&" +
+						"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+	}
+
 	private static ClientRegistration.Builder fineRedirectUriTemplateClientRegistration() {
 	private static ClientRegistration.Builder fineRedirectUriTemplateClientRegistration() {
 		return ClientRegistration.withRegistrationId("fine-redirect-uri-template-client-registration")
 		return ClientRegistration.withRegistrationId("fine-redirect-uri-template-client-registration")
 				.redirectUriTemplate("{baseScheme}://{baseHost}{basePort}{basePath}/{action}/oauth2/code/{registrationId}")
 				.redirectUriTemplate("{baseScheme}://{baseHost}{basePort}{basePath}/{action}/oauth2/code/{registrationId}")

+ 4 - 9
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java

@@ -1,5 +1,5 @@
 /*
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * you may not use this file except in compliance with the License.
@@ -49,7 +49,6 @@ import static org.mockito.Mockito.*;
  * Tests for {@link OAuth2AuthorizationRequestRedirectFilter}.
  * Tests for {@link OAuth2AuthorizationRequestRedirectFilter}.
  *
  *
  * @author Joe Grandja
  * @author Joe Grandja
- * @author Mark Heckler
  */
  */
 public class OAuth2AuthorizationRequestRedirectFilterTests {
 public class OAuth2AuthorizationRequestRedirectFilterTests {
 	private ClientRegistration registration1;
 	private ClientRegistration registration1;
@@ -155,8 +154,7 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
 				"response_type=code&client_id=client-id&" +
 				"response_type=code&client_id=client-id&" +
 				"scope=read:user&state=.{15,}&" +
 				"scope=read:user&state=.{15,}&" +
-				"redirect_uri=http://localhost/login/oauth2/code/registration-id&" +
-				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+				"redirect_uri=http://localhost/login/oauth2/code/registration-id");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -236,8 +234,7 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
 				"response_type=code&client_id=client-id&" +
 				"response_type=code&client_id=client-id&" +
 				"scope=read:user&state=.{15,}&" +
 				"scope=read:user&state=.{15,}&" +
-				"redirect_uri=http://localhost/login/oauth2/code/registration-id&" +
-				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+				"redirect_uri=http://localhost/login/oauth2/code/registration-id");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -258,8 +255,7 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
 				"response_type=code&client_id=client-id&" +
 				"response_type=code&client_id=client-id&" +
 				"scope=read:user&state=.{15,}&" +
 				"scope=read:user&state=.{15,}&" +
-				"redirect_uri=http://localhost/authorize/oauth2/code/registration-id&" +
-				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+				"redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
 		verify(this.requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class));
 		verify(this.requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class));
 	}
 	}
 
 
@@ -363,7 +359,6 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 				"response_type=code&client_id=client-id&" +
 				"response_type=code&client_id=client-id&" +
 				"scope=read:user&state=.{15,}&" +
 				"scope=read:user&state=.{15,}&" +
 				"redirect_uri=http://localhost/login/oauth2/code/registration-id&" +
 				"redirect_uri=http://localhost/login/oauth2/code/registration-id&" +
-				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" +
 				"login_hint=user@provider\\.com");
 				"login_hint=user@provider\\.com");
 	}
 	}
 }
 }

+ 27 - 11
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java

@@ -30,6 +30,8 @@ import org.springframework.security.oauth2.client.registration.TestClientRegistr
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
 import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
 import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
+import org.springframework.security.oauth2.core.oidc.OidcScopes;
+import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames;
 import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.ServerWebExchange;
 import reactor.core.publisher.Mono;
 import reactor.core.publisher.Mono;
@@ -41,7 +43,6 @@ import static org.mockito.Mockito.when;
 
 
 /**
 /**
  * @author Rob Winch
  * @author Rob Winch
- * @author Mark Heckler
  * @since 5.1
  * @since 5.1
  */
  */
 @RunWith(MockitoJUnitRunner.class)
 @RunWith(MockitoJUnitRunner.class)
@@ -83,13 +84,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		assertThat(request.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?" +
 		assertThat(request.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?" +
 				"response_type=code&client_id=client-id&" +
 				"response_type=code&client_id=client-id&" +
 				"scope=read:user&state=.*?&" +
 				"scope=read:user&state=.*?&" +
-				"redirect_uri=/login/oauth2/code/registration-id&" +
-				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
-	}
-
-	private OAuth2AuthorizationRequest resolve(String path) {
-		ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(path));
-		return this.resolver.resolve(exchange).block();
+				"redirect_uri=/login/oauth2/code/registration-id");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -103,8 +98,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		assertThat(request.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?" +
 		assertThat(request.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?" +
 				"response_type=code&client_id=client-id&" +
 				"response_type=code&client_id=client-id&" +
 				"scope=read:user&state=.*?&" +
 				"scope=read:user&state=.*?&" +
-				"redirect_uri=/login/oauth2/code/registration-id&" +
-				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+				"redirect_uri=/login/oauth2/code/registration-id");
 	}
 	}
 
 
 	@Test
 	@Test
@@ -124,7 +118,29 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 				"scope=read:user&state=.*?&" +
 				"scope=read:user&state=.*?&" +
 				"redirect_uri=/login/oauth2/code/registration-id&" +
 				"redirect_uri=/login/oauth2/code/registration-id&" +
 				"code_challenge_method=S256&" +
 				"code_challenge_method=S256&" +
-				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" +
 				"code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
 				"code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
 	}
 	}
+
+	@Test
+	public void resolveWhenAuthenticationRequestWithValidOidcClientThenResolves() {
+		when(this.clientRegistrationRepository.findByRegistrationId(any())).thenReturn(
+				Mono.just(TestClientRegistrations.clientRegistration()
+						.scope(OidcScopes.OPENID)
+						.build()));
+
+		OAuth2AuthorizationRequest request = resolve("/oauth2/authorization/registration-id");
+
+		assertThat((String) request.getAttribute(OidcParameterNames.NONCE)).matches("^([a-zA-Z0-9\\-\\.\\_\\~]){128}$");
+
+		assertThat(request.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?" +
+				"response_type=code&client_id=client-id&" +
+				"scope=openid&state=.*?&" +
+				"redirect_uri=/login/oauth2/code/registration-id&" +
+				"nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+	}
+
+	private OAuth2AuthorizationRequest resolve(String path) {
+		ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(path));
+		return this.resolver.resolve(exchange).block();
+	}
 }
 }

+ 1 - 1
oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/oidc/endpoint/OidcParameterNames.java

@@ -32,7 +32,7 @@ public interface OidcParameterNames {
 	String ID_TOKEN = "id_token";
 	String ID_TOKEN = "id_token";
 
 
 	/**
 	/**
-	 * {@code nonce} - used in the Access Token Request and Response.
+	 * {@code nonce} - used in the Authentication Request.
 	 */
 	 */
 	String NONCE = "nonce";
 	String NONCE = "nonce";