소스 검색

Allow configuring PKCE for confidential clients

Closes gh-6548
Joe Grandja 3 년 전
부모
커밋
54b033078b

+ 3 - 0
docs/modules/ROOT/pages/reactive/oauth2/client/authorization-grants.adoc

@@ -72,6 +72,9 @@ If the client is running in an untrusted environment (eg. native application or
 . `client-secret` is omitted (or empty)
 . `client-authentication-method` is set to "none" (`ClientAuthenticationMethod.NONE`)
 
+[TIP]
+If the OAuth 2.0 Provider supports PKCE for https://tools.ietf.org/html/rfc6749#section-2.1[Confidential Clients], you may (optionally) configure it using `DefaultServerOAuth2AuthorizationRequestResolver.setAuthorizationRequestCustomizer(OAuth2AuthorizationRequestCustomizers.withPkce())`.
+
 [[oauth2Client-auth-code-redirect-uri]]
 The `DefaultServerOAuth2AuthorizationRequestResolver` also supports `URI` template variables for the `redirect-uri` using `UriComponentsBuilder`.
 

+ 3 - 0
docs/modules/ROOT/pages/servlet/oauth2/client/authorization-grants.adoc

@@ -83,6 +83,9 @@ If the client is running in an untrusted environment (such as a native applicati
 . `client-secret` is omitted (or empty)
 . `client-authentication-method` is set to `none` (`ClientAuthenticationMethod.NONE`)
 
+[TIP]
+If the OAuth 2.0 Provider supports PKCE for https://tools.ietf.org/html/rfc6749#section-2.1[Confidential Clients], you may (optionally) configure it using `DefaultOAuth2AuthorizationRequestResolver.setAuthorizationRequestCustomizer(OAuth2AuthorizationRequestCustomizers.withPkce())`.
+
 [[oauth2Client-auth-code-redirect-uri]]
 The `DefaultOAuth2AuthorizationRequestResolver` also supports `URI` template variables for the `redirect-uri` by using `UriComponentsBuilder`.
 

+ 28 - 58
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -34,7 +34,6 @@ import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
 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 org.springframework.security.web.util.UrlUtils;
@@ -70,14 +69,18 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 
 	private static final char PATH_DELIMITER = '/';
 
-	private final ClientRegistrationRepository clientRegistrationRepository;
+	private static final StringKeyGenerator DEFAULT_STATE_GENERATOR = new Base64StringKeyGenerator(
+			Base64.getUrlEncoder());
 
-	private final AntPathRequestMatcher authorizationRequestMatcher;
+	private static final StringKeyGenerator DEFAULT_SECURE_KEY_GENERATOR = new Base64StringKeyGenerator(
+			Base64.getUrlEncoder().withoutPadding(), 96);
 
-	private final StringKeyGenerator stateGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder());
+	private static final Consumer<OAuth2AuthorizationRequest.Builder> DEFAULT_PKCE_APPLIER = OAuth2AuthorizationRequestCustomizers
+			.withPkce();
 
-	private final StringKeyGenerator secureKeyGenerator = new Base64StringKeyGenerator(
-			Base64.getUrlEncoder().withoutPadding(), 96);
+	private final ClientRegistrationRepository clientRegistrationRepository;
+
+	private final AntPathRequestMatcher authorizationRequestMatcher;
 
 	private Consumer<OAuth2AuthorizationRequest.Builder> authorizationRequestCustomizer = (customizer) -> {
 	};
@@ -100,7 +103,7 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 
 	@Override
 	public OAuth2AuthorizationRequest resolve(HttpServletRequest request) {
-		String registrationId = this.resolveRegistrationId(request);
+		String registrationId = resolveRegistrationId(request);
 		if (registrationId == null) {
 			return null;
 		}
@@ -123,6 +126,7 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 	 * @param authorizationRequestCustomizer the {@code Consumer} to be provided the
 	 * {@link OAuth2AuthorizationRequest.Builder}
 	 * @since 5.3
+	 * @see OAuth2AuthorizationRequestCustomizers
 	 */
 	public void setAuthorizationRequestCustomizer(
 			Consumer<OAuth2AuthorizationRequest.Builder> authorizationRequestCustomizer) {
@@ -147,9 +151,7 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 		if (clientRegistration == null) {
 			throw new IllegalArgumentException("Invalid Client Registration with Id: " + registrationId);
 		}
-		Map<String, Object> attributes = new HashMap<>();
-		attributes.put(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId());
-		OAuth2AuthorizationRequest.Builder builder = getBuilder(clientRegistration, attributes);
+		OAuth2AuthorizationRequest.Builder builder = getBuilder(clientRegistration);
 
 		String redirectUriStr = expandRedirectUri(request, clientRegistration, redirectUriAction);
 
@@ -158,8 +160,7 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 				.authorizationUri(clientRegistration.getProviderDetails().getAuthorizationUri())
 				.redirectUri(redirectUriStr)
 				.scopes(clientRegistration.getScopes())
-				.state(this.stateGenerator.generateKey())
-				.attributes(attributes);
+				.state(DEFAULT_STATE_GENERATOR.generateKey());
 		// @formatter:on
 
 		this.authorizationRequestCustomizer.accept(builder);
@@ -167,23 +168,24 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 		return builder.build();
 	}
 
-	private OAuth2AuthorizationRequest.Builder getBuilder(ClientRegistration clientRegistration,
-			Map<String, Object> attributes) {
+	private OAuth2AuthorizationRequest.Builder getBuilder(ClientRegistration clientRegistration) {
 		if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
-			OAuth2AuthorizationRequest.Builder builder = OAuth2AuthorizationRequest.authorizationCode();
-			Map<String, Object> additionalParameters = new HashMap<>();
+			// @formatter:off
+			OAuth2AuthorizationRequest.Builder builder = OAuth2AuthorizationRequest.authorizationCode()
+					.attributes((attrs) ->
+							attrs.put(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
+			// @formatter:on
 			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);
+				applyNonce(builder);
 			}
 			if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) {
-				addPkceParameters(attributes, additionalParameters);
+				DEFAULT_PKCE_APPLIER.accept(builder);
 			}
-			builder.additionalParameters(additionalParameters);
 			return builder;
 		}
 		if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) {
@@ -252,54 +254,22 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au
 
 	/**
 	 * Creates nonce and its hash for use in OpenID Connect 1.0 Authentication Requests.
-	 * @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
+	 * @param builder where the {@link OidcParameterNames#NONCE} and hash is stored for
+	 * the authentication request
 	 *
 	 * @since 5.2
 	 * @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 static void applyNonce(OAuth2AuthorizationRequest.Builder builder) {
 		try {
-			String nonce = this.secureKeyGenerator.generateKey();
+			String nonce = DEFAULT_SECURE_KEY_GENERATOR.generateKey();
 			String nonceHash = createHash(nonce);
-			attributes.put(OidcParameterNames.NONCE, nonce);
-			additionalParameters.put(OidcParameterNames.NONCE, nonceHash);
-		}
-		catch (NoSuchAlgorithmException ex) {
-		}
-	}
-
-	/**
-	 * Creates and adds additional PKCE parameters for use in the OAuth 2.0 Authorization
-	 * and Access Token Requests
-	 * @param attributes where {@link PkceParameterNames#CODE_VERIFIER} is stored for the
-	 * token request
-	 * @param additionalParameters where {@link PkceParameterNames#CODE_CHALLENGE} and,
-	 * usually, {@link PkceParameterNames#CODE_CHALLENGE_METHOD} are added to be used in
-	 * the authorization request.
-	 *
-	 * @since 5.2
-	 * @see <a target="_blank" href="https://tools.ietf.org/html/rfc7636#section-1.1">1.1.
-	 * Protocol Flow</a>
-	 * @see <a target="_blank" href="https://tools.ietf.org/html/rfc7636#section-4.1">4.1.
-	 * Client Creates a Code Verifier</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) {
-		String codeVerifier = this.secureKeyGenerator.generateKey();
-		attributes.put(PkceParameterNames.CODE_VERIFIER, codeVerifier);
-		try {
-			String codeChallenge = createHash(codeVerifier);
-			additionalParameters.put(PkceParameterNames.CODE_CHALLENGE, codeChallenge);
-			additionalParameters.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");
+			builder.attributes((attrs) -> attrs.put(OidcParameterNames.NONCE, nonce));
+			builder.additionalParameters((params) -> params.put(OidcParameterNames.NONCE, nonceHash));
 		}
 		catch (NoSuchAlgorithmException ex) {
-			additionalParameters.put(PkceParameterNames.CODE_CHALLENGE, codeVerifier);
 		}
 	}
 

+ 111 - 0
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestCustomizers.java

@@ -0,0 +1,111 @@
+/*
+ * Copyright 2002-2022 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
+ *
+ *      https://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.client.web;
+
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.Base64;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Consumer;
+
+import org.springframework.security.crypto.keygen.Base64StringKeyGenerator;
+import org.springframework.security.crypto.keygen.StringKeyGenerator;
+import org.springframework.security.oauth2.client.web.server.DefaultServerOAuth2AuthorizationRequestResolver;
+import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
+import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
+
+/**
+ * A factory of customizers that customize the {@link OAuth2AuthorizationRequest OAuth 2.0
+ * Authorization Request} via the {@link OAuth2AuthorizationRequest.Builder}.
+ *
+ * @author Joe Grandja
+ * @since 5.7
+ * @see OAuth2AuthorizationRequest.Builder
+ * @see DefaultOAuth2AuthorizationRequestResolver#setAuthorizationRequestCustomizer(Consumer)
+ * @see DefaultServerOAuth2AuthorizationRequestResolver#setAuthorizationRequestCustomizer(Consumer)
+ */
+public final class OAuth2AuthorizationRequestCustomizers {
+
+	private static final StringKeyGenerator DEFAULT_SECURE_KEY_GENERATOR = new Base64StringKeyGenerator(
+			Base64.getUrlEncoder().withoutPadding(), 96);
+
+	private OAuth2AuthorizationRequestCustomizers() {
+	}
+
+	/**
+	 * Returns a {@code Consumer} to be provided the
+	 * {@link OAuth2AuthorizationRequest.Builder} that adds the
+	 * {@link PkceParameterNames#CODE_CHALLENGE code_challenge} and, usually,
+	 * {@link PkceParameterNames#CODE_CHALLENGE_METHOD code_challenge_method} parameters
+	 * to the OAuth 2.0 Authorization Request. The {@code code_verifier} is stored in
+	 * {@link OAuth2AuthorizationRequest#getAttribute(String)} under the key
+	 * {@link PkceParameterNames#CODE_VERIFIER code_verifier} for subsequent use in the
+	 * OAuth 2.0 Access Token Request.
+	 * @return a {@code Consumer} to be provided the
+	 * {@link OAuth2AuthorizationRequest.Builder} that adds the PKCE parameters
+	 * @see <a target="_blank" href=
+	 * "https://datatracker.ietf.org/doc/html/rfc7636#section-1.1">1.1. Protocol Flow</a>
+	 * @see <a target="_blank" href=
+	 * "https://datatracker.ietf.org/doc/html/rfc7636#section-4.1">4.1. Client Creates a
+	 * Code Verifier</a>
+	 * @see <a target="_blank" href=
+	 * "https://datatracker.ietf.org/doc/html/rfc7636#section-4.2">4.2. Client Creates the
+	 * Code Challenge</a>
+	 */
+	public static Consumer<OAuth2AuthorizationRequest.Builder> withPkce() {
+		return OAuth2AuthorizationRequestCustomizers::applyPkce;
+	}
+
+	private static void applyPkce(OAuth2AuthorizationRequest.Builder builder) {
+		if (isPkceAlreadyApplied(builder)) {
+			return;
+		}
+
+		String codeVerifier = DEFAULT_SECURE_KEY_GENERATOR.generateKey();
+
+		builder.attributes((attrs) -> attrs.put(PkceParameterNames.CODE_VERIFIER, codeVerifier));
+
+		builder.additionalParameters((params) -> {
+			try {
+				String codeChallenge = createHash(codeVerifier);
+				params.put(PkceParameterNames.CODE_CHALLENGE, codeChallenge);
+				params.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");
+			}
+			catch (NoSuchAlgorithmException ex) {
+				params.put(PkceParameterNames.CODE_CHALLENGE, codeVerifier);
+			}
+		});
+	}
+
+	private static boolean isPkceAlreadyApplied(OAuth2AuthorizationRequest.Builder builder) {
+		AtomicBoolean pkceApplied = new AtomicBoolean(false);
+		builder.additionalParameters((params) -> {
+			if (params.containsKey(PkceParameterNames.CODE_CHALLENGE)) {
+				pkceApplied.set(true);
+			}
+		});
+		return pkceApplied.get();
+	}
+
+	private static String createHash(String value) throws NoSuchAlgorithmException {
+		MessageDigest md = MessageDigest.getInstance("SHA-256");
+		byte[] digest = md.digest(value.getBytes(StandardCharsets.US_ASCII));
+		return Base64.getUrlEncoder().withoutPadding().encodeToString(digest);
+	}
+
+}

+ 30 - 58
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -32,11 +32,11 @@ import org.springframework.security.crypto.keygen.Base64StringKeyGenerator;
 import org.springframework.security.crypto.keygen.StringKeyGenerator;
 import org.springframework.security.oauth2.client.registration.ClientRegistration;
 import org.springframework.security.oauth2.client.registration.ReactiveClientRegistrationRepository;
+import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestCustomizers;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
 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 org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher;
@@ -59,6 +59,7 @@ import org.springframework.web.util.UriComponentsBuilder;
  *
  * @author Rob Winch
  * @author Mark Heckler
+ * @author Joe Grandja
  * @since 5.1
  */
 public class DefaultServerOAuth2AuthorizationRequestResolver implements ServerOAuth2AuthorizationRequestResolver {
@@ -78,14 +79,18 @@ public class DefaultServerOAuth2AuthorizationRequestResolver implements ServerOA
 
 	private static final char PATH_DELIMITER = '/';
 
-	private final ServerWebExchangeMatcher authorizationRequestMatcher;
+	private static final StringKeyGenerator DEFAULT_STATE_GENERATOR = new Base64StringKeyGenerator(
+			Base64.getUrlEncoder());
 
-	private final ReactiveClientRegistrationRepository clientRegistrationRepository;
+	private static final StringKeyGenerator DEFAULT_SECURE_KEY_GENERATOR = new Base64StringKeyGenerator(
+			Base64.getUrlEncoder().withoutPadding(), 96);
 
-	private final StringKeyGenerator stateGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder());
+	private static final Consumer<OAuth2AuthorizationRequest.Builder> DEFAULT_PKCE_APPLIER = OAuth2AuthorizationRequestCustomizers
+			.withPkce();
 
-	private final StringKeyGenerator secureKeyGenerator = new Base64StringKeyGenerator(
-			Base64.getUrlEncoder().withoutPadding(), 96);
+	private final ServerWebExchangeMatcher authorizationRequestMatcher;
+
+	private final ReactiveClientRegistrationRepository clientRegistrationRepository;
 
 	private Consumer<OAuth2AuthorizationRequest.Builder> authorizationRequestCustomizer = (customizer) -> {
 	};
@@ -133,7 +138,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolver implements ServerOA
 
 	@Override
 	public Mono<OAuth2AuthorizationRequest> resolve(ServerWebExchange exchange, String clientRegistrationId) {
-		return this.findByRegistrationId(exchange, clientRegistrationId)
+		return findByRegistrationId(exchange, clientRegistrationId)
 				.map((clientRegistration) -> authorizationRequest(exchange, clientRegistration));
 	}
 
@@ -143,6 +148,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolver implements ServerOA
 	 * @param authorizationRequestCustomizer the {@code Consumer} to be provided the
 	 * {@link OAuth2AuthorizationRequest.Builder}
 	 * @since 5.3
+	 * @see OAuth2AuthorizationRequestCustomizers
 	 */
 	public final void setAuthorizationRequestCustomizer(
 			Consumer<OAuth2AuthorizationRequest.Builder> authorizationRequestCustomizer) {
@@ -159,17 +165,14 @@ public class DefaultServerOAuth2AuthorizationRequestResolver implements ServerOA
 
 	private OAuth2AuthorizationRequest authorizationRequest(ServerWebExchange exchange,
 			ClientRegistration clientRegistration) {
+		OAuth2AuthorizationRequest.Builder builder = getBuilder(clientRegistration);
 		String redirectUriStr = expandRedirectUri(exchange.getRequest(), clientRegistration);
-		Map<String, Object> attributes = new HashMap<>();
-		attributes.put(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId());
-		OAuth2AuthorizationRequest.Builder builder = getBuilder(clientRegistration, attributes);
 		// @formatter:off
 		builder.clientId(clientRegistration.getClientId())
 				.authorizationUri(clientRegistration.getProviderDetails().getAuthorizationUri())
 				.redirectUri(redirectUriStr)
 				.scopes(clientRegistration.getScopes())
-				.state(this.stateGenerator.generateKey())
-				.attributes(attributes);
+				.state(DEFAULT_STATE_GENERATOR.generateKey());
 		// @formatter:on
 
 		this.authorizationRequestCustomizer.accept(builder);
@@ -177,11 +180,13 @@ public class DefaultServerOAuth2AuthorizationRequestResolver implements ServerOA
 		return builder.build();
 	}
 
-	private OAuth2AuthorizationRequest.Builder getBuilder(ClientRegistration clientRegistration,
-			Map<String, Object> attributes) {
+	private OAuth2AuthorizationRequest.Builder getBuilder(ClientRegistration clientRegistration) {
 		if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
-			OAuth2AuthorizationRequest.Builder builder = OAuth2AuthorizationRequest.authorizationCode();
-			Map<String, Object> additionalParameters = new HashMap<>();
+			// @formatter:off
+			OAuth2AuthorizationRequest.Builder builder = OAuth2AuthorizationRequest.authorizationCode()
+					.attributes((attrs) ->
+							attrs.put(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
+			// @formatter:on
 			if (!CollectionUtils.isEmpty(clientRegistration.getScopes())
 					&& clientRegistration.getScopes().contains(OidcScopes.OPENID)) {
 				// Section 3.1.2.1 Authentication Request -
@@ -189,12 +194,11 @@ public class DefaultServerOAuth2AuthorizationRequestResolver implements ServerOA
 				// scope
 				// REQUIRED. OpenID Connect requests MUST contain the "openid" scope
 				// value.
-				addNonceParameters(attributes, additionalParameters);
+				applyNonce(builder);
 			}
 			if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) {
-				addPkceParameters(attributes, additionalParameters);
+				DEFAULT_PKCE_APPLIER.accept(builder);
 			}
-			builder.additionalParameters(additionalParameters);
 			return builder;
 		}
 		if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizationGrantType())) {
@@ -261,54 +265,22 @@ public class DefaultServerOAuth2AuthorizationRequestResolver implements ServerOA
 
 	/**
 	 * Creates nonce and its hash for use in OpenID Connect 1.0 Authentication Requests.
-	 * @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
+	 * @param builder where the {@link OidcParameterNames#NONCE} and hash is stored for
+	 * the authentication request
 	 *
 	 * @since 5.2
 	 * @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 static void applyNonce(OAuth2AuthorizationRequest.Builder builder) {
 		try {
-			String nonce = this.secureKeyGenerator.generateKey();
+			String nonce = DEFAULT_SECURE_KEY_GENERATOR.generateKey();
 			String nonceHash = createHash(nonce);
-			attributes.put(OidcParameterNames.NONCE, nonce);
-			additionalParameters.put(OidcParameterNames.NONCE, nonceHash);
-		}
-		catch (NoSuchAlgorithmException ex) {
-		}
-	}
-
-	/**
-	 * Creates and adds additional PKCE parameters for use in the OAuth 2.0 Authorization
-	 * and Access Token Requests
-	 * @param attributes where {@link PkceParameterNames#CODE_VERIFIER} is stored for the
-	 * token request
-	 * @param additionalParameters where {@link PkceParameterNames#CODE_CHALLENGE} and,
-	 * usually, {@link PkceParameterNames#CODE_CHALLENGE_METHOD} are added to be used in
-	 * the authorization request.
-	 *
-	 * @since 5.2
-	 * @see <a target="_blank" href="https://tools.ietf.org/html/rfc7636#section-1.1">1.1.
-	 * Protocol Flow</a>
-	 * @see <a target="_blank" href="https://tools.ietf.org/html/rfc7636#section-4.1">4.1.
-	 * Client Creates a Code Verifier</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) {
-		String codeVerifier = this.secureKeyGenerator.generateKey();
-		attributes.put(PkceParameterNames.CODE_VERIFIER, codeVerifier);
-		try {
-			String codeChallenge = createHash(codeVerifier);
-			additionalParameters.put(PkceParameterNames.CODE_CHALLENGE, codeChallenge);
-			additionalParameters.put(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256");
+			builder.attributes((attrs) -> attrs.put(OidcParameterNames.NONCE, nonce));
+			builder.additionalParameters((params) -> params.put(OidcParameterNames.NONCE, nonceHash));
 		}
 		catch (NoSuchAlgorithmException ex) {
-			additionalParameters.put(PkceParameterNames.CODE_CHALLENGE, codeVerifier);
 		}
 	}
 

+ 92 - 19
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -57,7 +57,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 
 	private ClientRegistration fineRedirectUriTemplateRegistration;
 
-	private ClientRegistration pkceRegistration;
+	private ClientRegistration publicClientRegistration;
 
 	private ClientRegistration oidcRegistration;
 
@@ -73,9 +73,9 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		this.registration2 = TestClientRegistrations.clientRegistration2().build();
 		this.fineRedirectUriTemplateRegistration = fineRedirectUriTemplateClientRegistration().build();
 		// @formatter:off
-		this.pkceRegistration = TestClientRegistrations.clientRegistration()
-				.registrationId("pkce-client-registration-id")
-				.clientId("pkce-client-id")
+		this.publicClientRegistration = TestClientRegistrations.clientRegistration()
+				.registrationId("public-client-registration-id")
+				.clientId("public-client-id")
 				.clientAuthenticationMethod(ClientAuthenticationMethod.NONE)
 				.clientSecret(null)
 				.build();
@@ -85,7 +85,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 				.build();
 		// @formatter:on
 		this.clientRegistrationRepository = new InMemoryClientRegistrationRepository(this.registration1,
-				this.registration2, this.fineRedirectUriTemplateRegistration, this.pkceRegistration,
+				this.registration2, this.fineRedirectUriTemplateRegistration, this.publicClientRegistration,
 				this.oidcRegistration);
 		this.resolver = new DefaultOAuth2AuthorizationRequestResolver(this.clientRegistrationRepository,
 				this.authorizationRequestBaseUri);
@@ -371,8 +371,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 	}
 
 	@Test
-	public void resolveWhenAuthorizationRequestWithValidPkceClientThenResolves() {
-		ClientRegistration clientRegistration = this.pkceRegistration;
+	public void resolveWhenAuthorizationRequestWithValidPublicClientThenResolves() {
+		ClientRegistration clientRegistration = this.publicClientRegistration;
 		String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
 		request.setServletPath(requestUri);
@@ -398,10 +398,84 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat((String) authorizationRequest.getAttribute(PkceParameterNames.CODE_VERIFIER))
 				.matches("^([a-zA-Z0-9\\-\\.\\_\\~]){128}$");
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
-				.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=pkce-client-id&"
-						+ "scope=read:user&state=.{15,}&"
-						+ "redirect_uri=http://localhost/login/oauth2/code/pkce-client-registration-id&"
-						+ "code_challenge_method=S256&" + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+				.matches("https://example.com/login/oauth/authorize\\?"
+						+ "response_type=code&client_id=public-client-id&" + "scope=read:user&state=.{15,}&"
+						+ "redirect_uri=http://localhost/login/oauth2/code/public-client-registration-id&"
+						+ "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "code_challenge_method=S256");
+	}
+
+	// gh-6548
+	@Test
+	public void resolveWhenAuthorizationRequestApplyPkceToConfidentialClientsThenApplied() {
+		this.resolver.setAuthorizationRequestCustomizer(OAuth2AuthorizationRequestCustomizers.withPkce());
+
+		ClientRegistration clientRegistration = this.registration1;
+		String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
+		request.setServletPath(requestUri);
+		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
+		assertPkceApplied(authorizationRequest, clientRegistration);
+
+		clientRegistration = this.registration2;
+		requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
+		request = new MockHttpServletRequest("GET", requestUri);
+		request.setServletPath(requestUri);
+		authorizationRequest = this.resolver.resolve(request);
+		assertPkceApplied(authorizationRequest, clientRegistration);
+	}
+
+	// gh-6548
+	@Test
+	public void resolveWhenAuthorizationRequestApplyPkceToSpecificConfidentialClientThenApplied() {
+		this.resolver.setAuthorizationRequestCustomizer((builder) -> {
+			builder.attributes((attrs) -> {
+				String registrationId = (String) attrs.get(OAuth2ParameterNames.REGISTRATION_ID);
+				if (this.registration1.getRegistrationId().equals(registrationId)) {
+					OAuth2AuthorizationRequestCustomizers.withPkce().accept(builder);
+				}
+			});
+		});
+
+		ClientRegistration clientRegistration = this.registration1;
+		String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
+		request.setServletPath(requestUri);
+		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
+		assertPkceApplied(authorizationRequest, clientRegistration);
+
+		clientRegistration = this.registration2;
+		requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
+		request = new MockHttpServletRequest("GET", requestUri);
+		request.setServletPath(requestUri);
+		authorizationRequest = this.resolver.resolve(request);
+		assertPkceNotApplied(authorizationRequest, clientRegistration);
+	}
+
+	private void assertPkceApplied(OAuth2AuthorizationRequest authorizationRequest,
+			ClientRegistration clientRegistration) {
+		assertThat(authorizationRequest.getAdditionalParameters()).containsKey(PkceParameterNames.CODE_CHALLENGE);
+		assertThat(authorizationRequest.getAdditionalParameters())
+				.contains(entry(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256"));
+		assertThat(authorizationRequest.getAttributes()).containsKey(PkceParameterNames.CODE_VERIFIER);
+		assertThat((String) authorizationRequest.getAttribute(PkceParameterNames.CODE_VERIFIER))
+				.matches("^([a-zA-Z0-9\\-\\.\\_\\~]){128}$");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&" + "client_id="
+						+ clientRegistration.getClientId() + "&" + "scope=read:user&" + "state=.{15,}&"
+						+ "redirect_uri=http://localhost/login/oauth2/code/" + clientRegistration.getRegistrationId()
+						+ "&" + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "code_challenge_method=S256");
+	}
+
+	private void assertPkceNotApplied(OAuth2AuthorizationRequest authorizationRequest,
+			ClientRegistration clientRegistration) {
+		assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey(PkceParameterNames.CODE_CHALLENGE);
+		assertThat(authorizationRequest.getAdditionalParameters())
+				.doesNotContainKey(PkceParameterNames.CODE_CHALLENGE_METHOD);
+		assertThat(authorizationRequest.getAttributes()).doesNotContainKey(PkceParameterNames.CODE_VERIFIER);
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&" + "client_id="
+						+ clientRegistration.getClientId() + "&" + "scope=read:user&" + "state=.{15,}&"
+						+ "redirect_uri=http://localhost/login/oauth2/code/" + clientRegistration.getRegistrationId());
 	}
 
 	@Test
@@ -444,7 +518,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
 		request.setServletPath(requestUri);
 		this.resolver.setAuthorizationRequestCustomizer(
-				(customizer) -> customizer.additionalParameters((params) -> params.remove(OidcParameterNames.NONCE))
+				(builder) -> builder.additionalParameters((params) -> params.remove(OidcParameterNames.NONCE))
 						.attributes((attrs) -> attrs.remove(OidcParameterNames.NONCE)));
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
 		assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey(OidcParameterNames.NONCE);
@@ -462,11 +536,10 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
 		request.setServletPath(requestUri);
-		this.resolver
-				.setAuthorizationRequestCustomizer((customizer) -> customizer.authorizationRequestUri((uriBuilder) -> {
-					uriBuilder.queryParam("param1", "value1");
-					return uriBuilder.build();
-				}));
+		this.resolver.setAuthorizationRequestCustomizer((builder) -> builder.authorizationRequestUri((uriBuilder) -> {
+			uriBuilder.queryParam("param1", "value1");
+			return uriBuilder.build();
+		}));
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 				.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
@@ -481,7 +554,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
 		request.setServletPath(requestUri);
-		this.resolver.setAuthorizationRequestCustomizer((customizer) -> customizer.parameters((params) -> {
+		this.resolver.setAuthorizationRequestCustomizer((builder) -> builder.parameters((params) -> {
 			params.put("appid", params.get("client_id"));
 			params.remove("client_id");
 		}));

+ 84 - 10
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -29,6 +29,7 @@ import org.springframework.mock.web.server.MockServerWebExchange;
 import org.springframework.security.oauth2.client.registration.ClientRegistration;
 import org.springframework.security.oauth2.client.registration.ReactiveClientRegistrationRepository;
 import org.springframework.security.oauth2.client.registration.TestClientRegistrations;
+import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestCustomizers;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
 import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
@@ -41,7 +42,9 @@ import org.springframework.web.server.ServerWebExchange;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.assertj.core.api.Assertions.entry;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.BDDMockito.given;
 
 /**
@@ -106,7 +109,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 	}
 
 	@Test
-	public void resolveWhenAuthorizationRequestWithValidPkceClientThenResolves() {
+	public void resolveWhenAuthorizationRequestWithValidPublicClientThenResolves() {
 		given(this.clientRegistrationRepository.findByRegistrationId(any()))
 				.willReturn(Mono.just(TestClientRegistrations.clientRegistration()
 						.clientAuthenticationMethod(ClientAuthenticationMethod.NONE).clientSecret(null).build()));
@@ -116,7 +119,79 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		assertThat(request.getAuthorizationRequestUri())
 				.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 						+ "scope=read:user&state=.*?&" + "redirect_uri=/login/oauth2/code/registration-id&"
-						+ "code_challenge_method=S256&" + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
+						+ "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "code_challenge_method=S256");
+	}
+
+	// gh-6548
+	@Test
+	public void resolveWhenAuthorizationRequestApplyPkceToConfidentialClientsThenApplied() {
+		ClientRegistration registration1 = TestClientRegistrations.clientRegistration().build();
+		given(this.clientRegistrationRepository.findByRegistrationId(eq(registration1.getRegistrationId())))
+				.willReturn(Mono.just(registration1));
+		ClientRegistration registration2 = TestClientRegistrations.clientRegistration2().build();
+		given(this.clientRegistrationRepository.findByRegistrationId(eq(registration2.getRegistrationId())))
+				.willReturn(Mono.just(registration2));
+
+		this.resolver.setAuthorizationRequestCustomizer(OAuth2AuthorizationRequestCustomizers.withPkce());
+
+		OAuth2AuthorizationRequest request = resolve("/oauth2/authorization/" + registration1.getRegistrationId());
+		assertPkceApplied(request, registration1);
+
+		request = resolve("/oauth2/authorization/" + registration2.getRegistrationId());
+		assertPkceApplied(request, registration2);
+	}
+
+	// gh-6548
+	@Test
+	public void resolveWhenAuthorizationRequestApplyPkceToSpecificConfidentialClientThenApplied() {
+		ClientRegistration registration1 = TestClientRegistrations.clientRegistration().build();
+		given(this.clientRegistrationRepository.findByRegistrationId(eq(registration1.getRegistrationId())))
+				.willReturn(Mono.just(registration1));
+		ClientRegistration registration2 = TestClientRegistrations.clientRegistration2().build();
+		given(this.clientRegistrationRepository.findByRegistrationId(eq(registration2.getRegistrationId())))
+				.willReturn(Mono.just(registration2));
+
+		this.resolver.setAuthorizationRequestCustomizer((builder) -> {
+			builder.attributes((attrs) -> {
+				String registrationId = (String) attrs.get(OAuth2ParameterNames.REGISTRATION_ID);
+				if (registration1.getRegistrationId().equals(registrationId)) {
+					OAuth2AuthorizationRequestCustomizers.withPkce().accept(builder);
+				}
+			});
+		});
+
+		OAuth2AuthorizationRequest request = resolve("/oauth2/authorization/" + registration1.getRegistrationId());
+		assertPkceApplied(request, registration1);
+
+		request = resolve("/oauth2/authorization/" + registration2.getRegistrationId());
+		assertPkceNotApplied(request, registration2);
+	}
+
+	private void assertPkceApplied(OAuth2AuthorizationRequest authorizationRequest,
+			ClientRegistration clientRegistration) {
+		assertThat(authorizationRequest.getAdditionalParameters()).containsKey(PkceParameterNames.CODE_CHALLENGE);
+		assertThat(authorizationRequest.getAdditionalParameters())
+				.contains(entry(PkceParameterNames.CODE_CHALLENGE_METHOD, "S256"));
+		assertThat(authorizationRequest.getAttributes()).containsKey(PkceParameterNames.CODE_VERIFIER);
+		assertThat((String) authorizationRequest.getAttribute(PkceParameterNames.CODE_VERIFIER))
+				.matches("^([a-zA-Z0-9\\-\\.\\_\\~]){128}$");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&" + "client_id="
+						+ clientRegistration.getClientId() + "&" + "scope=read:user&" + "state=.{15,}&"
+						+ "redirect_uri=/login/oauth2/code/" + clientRegistration.getRegistrationId() + "&"
+						+ "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "code_challenge_method=S256");
+	}
+
+	private void assertPkceNotApplied(OAuth2AuthorizationRequest authorizationRequest,
+			ClientRegistration clientRegistration) {
+		assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey(PkceParameterNames.CODE_CHALLENGE);
+		assertThat(authorizationRequest.getAdditionalParameters())
+				.doesNotContainKey(PkceParameterNames.CODE_CHALLENGE_METHOD);
+		assertThat(authorizationRequest.getAttributes()).doesNotContainKey(PkceParameterNames.CODE_VERIFIER);
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&" + "client_id="
+						+ clientRegistration.getClientId() + "&" + "scope=read:user&" + "state=.{15,}&"
+						+ "redirect_uri=/login/oauth2/code/" + clientRegistration.getRegistrationId());
 	}
 
 	@Test
@@ -136,7 +211,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		given(this.clientRegistrationRepository.findByRegistrationId(any()))
 				.willReturn(Mono.just(TestClientRegistrations.clientRegistration().scope(OidcScopes.OPENID).build()));
 		this.resolver.setAuthorizationRequestCustomizer(
-				(customizer) -> customizer.additionalParameters((params) -> params.remove(OidcParameterNames.NONCE))
+				(builder) -> builder.additionalParameters((params) -> params.remove(OidcParameterNames.NONCE))
 						.attributes((attrs) -> attrs.remove(OidcParameterNames.NONCE)));
 		OAuth2AuthorizationRequest authorizationRequest = resolve("/oauth2/authorization/registration-id");
 		assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey(OidcParameterNames.NONCE);
@@ -151,11 +226,10 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 	public void resolveWhenAuthorizationRequestCustomizerAddsParameterThenQueryIncludesParameter() {
 		given(this.clientRegistrationRepository.findByRegistrationId(any()))
 				.willReturn(Mono.just(TestClientRegistrations.clientRegistration().scope(OidcScopes.OPENID).build()));
-		this.resolver
-				.setAuthorizationRequestCustomizer((customizer) -> customizer.authorizationRequestUri((uriBuilder) -> {
-					uriBuilder.queryParam("param1", "value1");
-					return uriBuilder.build();
-				}));
+		this.resolver.setAuthorizationRequestCustomizer((builder) -> builder.authorizationRequestUri((uriBuilder) -> {
+			uriBuilder.queryParam("param1", "value1");
+			return uriBuilder.build();
+		}));
 		OAuth2AuthorizationRequest authorizationRequest = resolve("/oauth2/authorization/registration-id");
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 				.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
@@ -167,7 +241,7 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 	public void resolveWhenAuthorizationRequestCustomizerOverridesParameterThenQueryIncludesParameter() {
 		given(this.clientRegistrationRepository.findByRegistrationId(any()))
 				.willReturn(Mono.just(TestClientRegistrations.clientRegistration().scope(OidcScopes.OPENID).build()));
-		this.resolver.setAuthorizationRequestCustomizer((customizer) -> customizer.parameters((params) -> {
+		this.resolver.setAuthorizationRequestCustomizer((builder) -> builder.parameters((params) -> {
 			params.put("appid", params.get("client_id"));
 			params.remove("client_id");
 		}));