فهرست منبع

Enable PKCE by default

Closes gh-17507

Signed-off-by: Rohan Naik <rohan.nn1203@gmail.com>
Rohan Naik 2 هفته پیش
والد
کامیت
8c65dc93f2

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

@@ -156,7 +156,7 @@ public class OAuth2ClientConfigurerTests {
 				.andExpect(status().is3xxRedirection()).andReturn();
 		assertThat(mvcResult.getResponse().getRedirectedUrl())
 				.matches("https://provider.com/oauth2/authorize\\?" + "response_type=code&client_id=client-1&"
-						+ "scope=user&state=.{15,}&" + "redirect_uri=http://localhost/client-1");
+						+ "scope=user&state=.{15,}&" + "redirect_uri=http://localhost/client-1&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 		// @formatter:on
 	}
 
@@ -166,9 +166,9 @@ public class OAuth2ClientConfigurerTests {
 		MvcResult mvcResult = this.mockMvc.perform(get("/oauth2/authorization/registration-1"))
 			.andExpect(status().is3xxRedirection())
 			.andReturn();
-		assertThat(mvcResult.getResponse().getRedirectedUrl())
-			.matches("https://provider.com/oauth2/authorize\\?" + "response_type=code&client_id=client-1&"
-					+ "scope=user&state=.{15,}&" + "redirect_uri=http://localhost/client-1");
+		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?"
+				+ "response_type=code&client_id=client-1&" + "scope=user&state=.{15,}&"
+				+ "redirect_uri=http://localhost/client-1&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -215,9 +215,9 @@ public class OAuth2ClientConfigurerTests {
 		MvcResult mvcResult = this.mockMvc.perform(get("/resource1").with(user("user1")))
 			.andExpect(status().is3xxRedirection())
 			.andReturn();
-		assertThat(mvcResult.getResponse().getRedirectedUrl())
-			.matches("https://provider.com/oauth2/authorize\\?" + "response_type=code&client_id=client-1&"
-					+ "scope=user&state=.{15,}&" + "redirect_uri=http://localhost/client-1");
+		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?"
+				+ "response_type=code&client_id=client-1&" + "scope=user&state=.{15,}&"
+				+ "redirect_uri=http://localhost/client-1&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 		verify(requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class));
 	}
 

+ 6 - 6
config/src/test/java/org/springframework/security/config/http/OAuth2ClientBeanDefinitionParserTests.java

@@ -112,9 +112,9 @@ public class OAuth2ClientBeanDefinitionParserTests {
 				.andExpect(status().is3xxRedirection())
 				.andReturn();
 		// @formatter:on
-		assertThat(result.getResponse().getRedirectedUrl()).matches(
-				"https://accounts.google.com/o/oauth2/v2/auth\\?" + "response_type=code&client_id=google-client-id&"
-						+ "scope=scope1%20scope2&state=.{15,}&redirect_uri=http://localhost/callback/google");
+		assertThat(result.getResponse().getRedirectedUrl()).matches("https://accounts.google.com/o/oauth2/v2/auth\\?"
+				+ "response_type=code&client_id=google-client-id&"
+				+ "scope=scope1%20scope2&state=.{15,}&redirect_uri=http://localhost/callback/google&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -134,9 +134,9 @@ public class OAuth2ClientBeanDefinitionParserTests {
 				.andExpect(status().is3xxRedirection())
 				.andReturn();
 		// @formatter:on
-		assertThat(result.getResponse().getRedirectedUrl()).matches(
-				"https://accounts.google.com/o/oauth2/v2/auth\\?" + "response_type=code&client_id=google-client-id&"
-						+ "scope=scope1%20scope2&state=.{15,}&redirect_uri=http://localhost/callback/google");
+		assertThat(result.getResponse().getRedirectedUrl()).matches("https://accounts.google.com/o/oauth2/v2/auth\\?"
+				+ "response_type=code&client_id=google-client-id&"
+				+ "scope=scope1%20scope2&state=.{15,}&redirect_uri=http://localhost/callback/google&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 		verify(this.clientRegistrationRepository).findByRegistrationId(any());
 	}
 

+ 1 - 1
docs/modules/ROOT/pages/servlet/oauth2/client/core.adoc

@@ -69,7 +69,7 @@ This information is available only if the Spring Boot property `spring.security.
 <15> `(userInfoEndpoint)authenticationMethod`: The authentication method used when sending the access token to the UserInfo Endpoint.
 The supported values are *header*, *form*, and *query*.
 <16> `userNameAttributeName`: The name of the attribute returned in the UserInfo Response that references the Name or Identifier of the end-user.
-<17> [[oauth2Client-client-registration-requireProofKey]]`requireProofKey`: If `true` or if `authorizationGrantType` is `none`, then PKCE will be enabled by default.
+<17> [[oauth2Client-client-registration-requireProofKey]]`requireProofKey`: If `true` or if `clientAuthenticationMethod` is `none`, then PKCE will be enabled.
 
 You can initially configure a `ClientRegistration` by using discovery of an OpenID Connect Provider's https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig[Configuration endpoint] or an Authorization Server's https://tools.ietf.org/html/rfc8414#section-3[Metadata endpoint].
 

+ 5 - 7
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java

@@ -651,6 +651,10 @@ public final class ClientRegistration implements Serializable {
 			clientRegistration.clientName = StringUtils.hasText(this.clientName) ? this.clientName
 					: this.registrationId;
 			clientRegistration.clientSettings = this.clientSettings;
+			if (clientRegistration.clientSettings.requireProofKey) {
+				clientRegistration.clientSettings.requireProofKey = AuthorizationGrantType.AUTHORIZATION_CODE
+					.equals(this.authorizationGrantType);
+			}
 			return clientRegistration;
 		}
 
@@ -701,12 +705,6 @@ public final class ClientRegistration implements Serializable {
 							"AuthorizationGrantType: %s does not match the pre-defined constant %s and won't match a valid OAuth2AuthorizedClientProvider",
 							this.authorizationGrantType, authorizationGrantType));
 				}
-				if (!AuthorizationGrantType.AUTHORIZATION_CODE.equals(this.authorizationGrantType)
-						&& this.clientSettings.isRequireProofKey()) {
-					throw new IllegalStateException(
-							"clientSettings.isRequireProofKey=true is only valid with authorizationGrantType=AUTHORIZATION_CODE. Got authorizationGrantType="
-									+ this.authorizationGrantType);
-				}
 			}
 		}
 
@@ -779,7 +777,7 @@ public final class ClientRegistration implements Serializable {
 
 		public static final class Builder {
 
-			private boolean requireProofKey;
+			private boolean requireProofKey = true;
 
 			private Builder() {
 			}

+ 51 - 11
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java

@@ -35,9 +35,7 @@ import org.springframework.security.oauth2.core.AuthenticationMethod;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
-import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
+import static org.assertj.core.api.Assertions.*;
 
 /**
  * Tests for {@link ClientRegistration}.
@@ -683,7 +681,7 @@ public class ClientRegistrationTests {
 		// should not be null
 		assertThat(clientRegistration.getClientSettings()).isNotNull();
 		// proof key should be false for passivity
-		assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isFalse();
+		assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isTrue();
 	}
 
 	// gh-16382
@@ -701,28 +699,70 @@ public class ClientRegistrationTests {
 			.tokenUri(TOKEN_URI)
 			.build();
 
-		// proof key should be false for passivity
 		assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isTrue();
 	}
 
+	@Test
+	void buildWhenNewAuthorizationCodeAndPkceDisabledThenBuilds() {
+		ClientRegistration.ClientSettings pkceDisabled = ClientRegistration.ClientSettings.builder()
+			.requireProofKey(false)
+			.build();
+		ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
+			.clientId(CLIENT_ID)
+			.clientSettings(pkceDisabled)
+			.authorizationGrantType(new AuthorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()))
+			.redirectUri(REDIRECT_URI)
+			.authorizationUri(AUTHORIZATION_URI)
+			.tokenUri(TOKEN_URI)
+			.build();
+
+		assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isFalse();
+	}
+
+	@Test
+	void buildWhenNewAuthorizationCodeAndPrivateClientThenPkceEnabledAndExceptionThrown() {
+		List<ClientAuthenticationMethod> clientAuthenticationMethods = Arrays
+			.stream(ClientAuthenticationMethod.class.getFields())
+			.filter((field) -> Modifier.isFinal(field.getModifiers())
+					&& field.getType() == ClientAuthenticationMethod.class)
+			.map((field) -> getStaticValue(field, ClientAuthenticationMethod.class))
+			.filter((authenticationMethod) -> authenticationMethod != ClientAuthenticationMethod.NONE)
+			.map((authenticationMethod) -> new ClientAuthenticationMethod(authenticationMethod.getValue()))
+			.toList();
+		for (ClientAuthenticationMethod clientAuthenticationMethod : clientAuthenticationMethods) {
+			ClientRegistration.ClientSettings pkceEnabled = ClientRegistration.ClientSettings.builder()
+				.requireProofKey(true)
+				.build();
+			ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
+				.clientId(CLIENT_ID)
+				.clientSettings(pkceEnabled)
+				.authorizationGrantType(
+						new AuthorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()))
+				.clientAuthenticationMethod(clientAuthenticationMethod)
+				.redirectUri(REDIRECT_URI)
+				.authorizationUri(AUTHORIZATION_URI)
+				.tokenUri(TOKEN_URI)
+				.build();
+			assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isTrue();
+		}
+	}
+
 	@ParameterizedTest
 	@MethodSource("invalidPkceGrantTypes")
 	void buildWhenInvalidGrantTypeForPkceThenException(AuthorizationGrantType invalidGrantType) {
 		ClientRegistration.ClientSettings pkceEnabled = ClientRegistration.ClientSettings.builder()
 			.requireProofKey(true)
 			.build();
-		ClientRegistration.Builder builder = ClientRegistration.withRegistrationId(REGISTRATION_ID)
+		ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
 			.clientId(CLIENT_ID)
 			.clientSettings(pkceEnabled)
 			.authorizationGrantType(invalidGrantType)
 			.redirectUri(REDIRECT_URI)
 			.authorizationUri(AUTHORIZATION_URI)
-			.tokenUri(TOKEN_URI);
+			.tokenUri(TOKEN_URI)
+			.build();
 
-		assertThatIllegalStateException().describedAs(
-				"clientSettings.isRequireProofKey=true is only valid with authorizationGrantType=AUTHORIZATION_CODE. Got authorizationGrantType={}",
-				invalidGrantType)
-			.isThrownBy(builder::build);
+		assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isFalse();
 	}
 
 	static List<AuthorizationGrantType> invalidPkceGrantTypes() {

+ 35 - 29
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java

@@ -60,6 +60,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 
 	private ClientRegistration pkceClientRegistration;
 
+	private ClientRegistration nonProofKeyPublicClientRegistration;
+
 	private ClientRegistration fineRedirectUriTemplateRegistration;
 
 	private ClientRegistration publicClientRegistration;
@@ -78,7 +80,11 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		this.registration2 = TestClientRegistrations.clientRegistration2().build();
 
 		this.pkceClientRegistration = pkceClientRegistration().build();
-
+		this.nonProofKeyPublicClientRegistration = TestClientRegistrations.clientRegistration()
+			.registrationId("invalid-public-client-registration-id")
+			.clientAuthenticationMethod(ClientAuthenticationMethod.NONE)
+			.clientSettings(ClientRegistration.ClientSettings.builder().requireProofKey(false).build())
+			.build();
 		this.fineRedirectUriTemplateRegistration = fineRedirectUriTemplateClientRegistration().build();
 		// @formatter:off
 		this.publicClientRegistration = TestClientRegistrations.clientRegistration()
@@ -94,7 +100,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		// @formatter:on
 		this.clientRegistrationRepository = new InMemoryClientRegistrationRepository(this.registration1,
 				this.registration2, this.pkceClientRegistration, this.fineRedirectUriTemplateRegistration,
-				this.publicClientRegistration, this.oidcRegistration);
+				this.publicClientRegistration, this.oidcRegistration, this.nonProofKeyPublicClientRegistration);
 		this.resolver = new DefaultOAuth2AuthorizationRequestResolver(this.clientRegistrationRepository,
 				this.authorizationRequestBaseUri);
 	}
@@ -178,12 +184,14 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getState()).isNotNull();
 		assertThat(authorizationRequest.getAdditionalParameters())
 			.doesNotContainKey(OAuth2ParameterNames.REGISTRATION_ID);
-		assertThat(authorizationRequest.getAttributes())
-			.containsExactly(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
+		assertThat(authorizationRequest.getAttributes()).containsExactly(
+				entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()),
+				entry(PkceParameterNames.CODE_VERIFIER,
+						authorizationRequest.getAttributes().get(PkceParameterNames.CODE_VERIFIER)));
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id");
+					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -194,8 +202,10 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request,
 				clientRegistration.getRegistrationId());
 		assertThat(authorizationRequest).isNotNull();
-		assertThat(authorizationRequest.getAttributes())
-			.containsExactly(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
+		assertThat(authorizationRequest.getAttributes()).containsExactly(
+				entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()),
+				entry(PkceParameterNames.CODE_VERIFIER,
+						authorizationRequest.getAttributes().get(PkceParameterNames.CODE_VERIFIER)));
 	}
 
 	@Test
@@ -282,7 +292,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id");
+					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id"
+					+ "&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -294,7 +305,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=https://example.com/login/oauth2/code/registration-id");
+					+ "redirect_uri=https://example.com/login/oauth2/code/registration-id"
+					+ "&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -307,7 +319,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
+					+ "redirect_uri=http://localhost/authorize/oauth2/code/registration-id&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -319,7 +331,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id-2&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id-2");
+					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id-2"
+					+ "&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -331,7 +344,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
+					+ "redirect_uri=http://localhost/authorize/oauth2/code/registration-id&"
+					+ "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -343,7 +357,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id-2&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id-2");
+					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id-2&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -419,7 +433,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
 		request = get(requestUri).build();
 		authorizationRequest = this.resolver.resolve(request);
-		assertPkceNotApplied(authorizationRequest, clientRegistration);
+		assertPkceApplied(authorizationRequest, clientRegistration);
 	}
 
 	private void assertPkceApplied(OAuth2AuthorizationRequest authorizationRequest,
@@ -477,7 +491,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 			.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}");
+					+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	// gh-7696
@@ -496,7 +510,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		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");
+					+ "redirect_uri=http://localhost/login/oauth2/code/oidc-registration-id&"
+					+ "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -513,7 +528,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 			.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}&" + "param1=value1");
+					+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}"
+					+ "&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256&param1=value1");
 	}
 
 	@Test
@@ -529,18 +545,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getAuthorizationRequestUri()).matches(
 				"https://example.com/login/oauth/authorize\\?" + "response_type=code&" + "scope=openid&state=.{15,}&"
 						+ "redirect_uri=http://localhost/login/oauth2/code/oidc-registration-id&"
-						+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "appid=client-id");
-	}
-
-	@Test
-	public void resolveWhenAuthorizationRequestNoProvideAuthorizationRequestBaseUri() {
-		OAuth2AuthorizationRequestResolver resolver = new DefaultOAuth2AuthorizationRequestResolver(
-				this.clientRegistrationRepository);
-		String requestUri = this.authorizationRequestBaseUri + "/" + this.registration2.getRegistrationId();
-		MockHttpServletRequest request = get(requestUri).build();
-		OAuth2AuthorizationRequest authorizationRequest = resolver.resolve(request);
-		assertThat(authorizationRequest.getRedirectUri())
-			.isEqualTo("http://localhost/login/oauth2/code/" + this.registration2.getRegistrationId());
+						+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}"
+						+ "&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256&appid=client-id");
 	}
 
 	@Test

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

@@ -183,7 +183,7 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 		verifyNoMoreInteractions(filterChain);
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?"
 				+ "response_type=code&client_id=client-id&" + "scope=read:user&state=.{15,}&"
-				+ "redirect_uri=http://localhost/login/oauth2/code/registration-id");
+				+ "redirect_uri=http://localhost/login/oauth2/code/registration-id&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -215,7 +215,7 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 		verifyNoMoreInteractions(filterChain);
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?"
 				+ "response_type=code&client_id=client-id&" + "scope=read:user&state=.{15,}&"
-				+ "redirect_uri=http://localhost/login/oauth2/code/registration-id");
+				+ "redirect_uri=http://localhost/login/oauth2/code/registration-id&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -231,7 +231,7 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 		verify(filterChain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?"
 				+ "response_type=code&client_id=client-id&" + "scope=read:user&state=.{15,}&"
-				+ "redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
+				+ "redirect_uri=http://localhost/authorize/oauth2/code/registration-id&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 		verify(this.requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class));
 	}
 
@@ -278,7 +278,7 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?"
 				+ "response_type=code&client_id=client-id&" + "scope=read:user&state=.{15,}&"
 				+ "redirect_uri=http://localhost/login/oauth2/code/registration-id&"
-				+ "idp=https://other.provider.com");
+				+ "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256&idp=https://other.provider.com");
 	}
 
 	// gh-4911, gh-5244
@@ -318,7 +318,7 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?"
 				+ "response_type=code&client_id=client-id&" + "scope=read:user&state=.{15,}&"
 				+ "redirect_uri=http://localhost/login/oauth2/code/registration-id&"
-				+ "login_hint=user@provider\\.com");
+				+ "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256&login_hint=user@provider\\.com");
 	}
 
 	@Test
@@ -344,7 +344,7 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 		assertThat(response.getContentAsString(StandardCharsets.UTF_8))
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=read:user&state=.{15,}&"
-					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id");
+					+ "redirect_uri=http://localhost/login/oauth2/code/registration-id&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	// gh-11602

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

@@ -59,11 +59,18 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 
 	private DefaultServerOAuth2AuthorizationRequestResolver resolver;
 
+	private ClientRegistration nonProofKeyPublicClientRegistration;
+
 	private ClientRegistration registration = TestClientRegistrations.clientRegistration().build();
 
 	@BeforeEach
 	public void setup() {
 		this.resolver = new DefaultServerOAuth2AuthorizationRequestResolver(this.clientRegistrationRepository);
+		this.nonProofKeyPublicClientRegistration = TestClientRegistrations.clientRegistration()
+			.registrationId("invalid-public-client-registration-id")
+			.clientAuthenticationMethod(ClientAuthenticationMethod.NONE)
+			.clientSettings(ClientRegistration.ClientSettings.builder().requireProofKey(false).build())
+			.build();
 	}
 
 	@Test
@@ -90,7 +97,8 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		OAuth2AuthorizationRequest request = resolve("/oauth2/authorization/not-found-id");
 		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");
+					+ "scope=read:user&state=.*?&" + "redirect_uri=/login/oauth2/code/registration-id"
+					+ "&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -105,7 +113,8 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		OAuth2AuthorizationRequest request = this.resolver.resolve(exchange).block();
 		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");
+					+ "scope=read:user&state=.*?&" + "redirect_uri=/login/oauth2/code/registration-id"
+					+ "&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
 	}
 
 	@Test
@@ -149,9 +158,9 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		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));
+		given(this.clientRegistrationRepository
+			.findByRegistrationId(eq(this.nonProofKeyPublicClientRegistration.getRegistrationId())))
+			.willReturn(Mono.just(this.nonProofKeyPublicClientRegistration));
 
 		this.resolver.setAuthorizationRequestCustomizer((builder) -> {
 			builder.attributes((attrs) -> {
@@ -165,8 +174,8 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		OAuth2AuthorizationRequest request = resolve("/oauth2/authorization/" + registration1.getRegistrationId());
 		assertPkceApplied(request, registration1);
 
-		request = resolve("/oauth2/authorization/" + registration2.getRegistrationId());
-		assertPkceNotApplied(request, registration2);
+		request = resolve("/oauth2/authorization/" + this.nonProofKeyPublicClientRegistration.getRegistrationId());
+		assertPkceApplied(request, this.nonProofKeyPublicClientRegistration);
 	}
 
 	@Test
@@ -220,7 +229,8 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		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}");
+				+ "redirect_uri=/login/oauth2/code/registration-id&" + "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&"
+				+ "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "code_challenge_method=S256");
 	}
 
 	// gh-7696
@@ -237,7 +247,8 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getAttributes()).containsKey(OAuth2ParameterNames.REGISTRATION_ID);
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
-					+ "scope=openid&state=.{15,}&" + "redirect_uri=/login/oauth2/code/registration-id");
+					+ "scope=openid&state=.{15,}&" + "redirect_uri=/login/oauth2/code/registration-id&"
+					+ "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "code_challenge_method=S256");
 	}
 
 	@Test
@@ -252,7 +263,8 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&client_id=client-id&"
 					+ "scope=openid&state=.{15,}&" + "redirect_uri=/login/oauth2/code/registration-id&"
-					+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "param1=value1");
+					+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&"
+					+ "code_challenge_method=S256&" + "param1=value1");
 	}
 
 	@Test
@@ -267,7 +279,8 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 			.matches("https://example.com/login/oauth/authorize\\?" + "response_type=code&"
 					+ "scope=openid&state=.{15,}&" + "redirect_uri=/login/oauth2/code/registration-id&"
-					+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "appid=client-id");
+					+ "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&"
+					+ "code_challenge_method=S256&" + "appid=client-id");
 	}
 
 	private OAuth2AuthorizationRequest resolve(String path) {