From 8acd1d3f5143cbb0067ac4244550110221eb9792 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 17 Jan 2025 17:14:16 -0600 Subject: [PATCH 01/10] Fix checkstyleNohttp OutOfMemoryError --- build.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build.gradle b/build.gradle index 60089e6734..4fd368b54a 100644 --- a/build.gradle +++ b/build.gradle @@ -110,6 +110,10 @@ nohttp { source.builtBy(project(':spring-security-config').tasks.withType(RncToXsd)) } +tasks.named('checkstyleNohttp') { + maxHeapSize = '1g' +} + tasks.register('cloneRepository', IncludeRepoTask) { repository = project.getProperties().get("repositoryName") ref = project.getProperties().get("ref") From 8d3e0844c50e424ff3696190afca354a2eaadf73 Mon Sep 17 00:00:00 2001 From: DingHao Date: Thu, 9 Jan 2025 17:32:25 +0800 Subject: [PATCH 02/10] Add ClientRegistration.clientSettings.requireProofKey to Enable PKCE Closes gh-16382 Signed-off-by: DingHao --- .../ClientRegistrationDeserializer.java | 2 +- .../registration/ClientRegistration.java | 30 +++++++- .../client/registration/ClientSettings.java | 68 +++++++++++++++++++ ...ultOAuth2AuthorizationRequestResolver.java | 5 +- .../OAuth2AuthorizedClientMixinTests.java | 5 +- ...uth2AuthorizationRequestResolverTests.java | 38 ++++++++++- 6 files changed, 139 insertions(+), 9 deletions(-) create mode 100644 oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/ClientRegistrationDeserializer.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/ClientRegistrationDeserializer.java index 77b1fdd121..d8fc1a099d 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/ClientRegistrationDeserializer.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/ClientRegistrationDeserializer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2025 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. diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java index 0639a395f8..c8fa34e682 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2025 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. @@ -71,6 +71,8 @@ public final class ClientRegistration implements Serializable { private String clientName; + private ClientSettings clientSettings; + private ClientRegistration() { } @@ -162,6 +164,14 @@ public final class ClientRegistration implements Serializable { return this.clientName; } + /** + * Returns the {@link ClientSettings client configuration settings}. + * @return the {@link ClientSettings} + */ + public ClientSettings getClientSettings() { + return this.clientSettings; + } + @Override public String toString() { // @formatter:off @@ -175,6 +185,7 @@ public final class ClientRegistration implements Serializable { + '\'' + ", scopes=" + this.scopes + ", providerDetails=" + this.providerDetails + ", clientName='" + this.clientName + '\'' + + ", clientSettings='" + this.clientSettings + '\'' + '}'; // @formatter:on } @@ -367,6 +378,8 @@ public final class ClientRegistration implements Serializable { private String clientName; + private ClientSettings clientSettings; + private Builder(String registrationId) { this.registrationId = registrationId; } @@ -391,6 +404,7 @@ public final class ClientRegistration implements Serializable { this.configurationMetadata = new HashMap<>(configurationMetadata); } this.clientName = clientRegistration.clientName; + this.clientSettings = clientRegistration.clientSettings; } /** @@ -594,6 +608,16 @@ public final class ClientRegistration implements Serializable { return this; } + /** + * Sets the {@link ClientSettings client configuration settings}. + * @param clientSettings the client configuration settings + * @return the {@link Builder} + */ + public Builder clientSettings(ClientSettings clientSettings) { + this.clientSettings = clientSettings; + return this; + } + /** * Builds a new {@link ClientRegistration}. * @return a {@link ClientRegistration} @@ -627,12 +651,14 @@ public final class ClientRegistration implements Serializable { clientRegistration.providerDetails = createProviderDetails(clientRegistration); clientRegistration.clientName = StringUtils.hasText(this.clientName) ? this.clientName : this.registrationId; + clientRegistration.clientSettings = (this.clientSettings == null) ? ClientSettings.builder().build() + : this.clientSettings; return clientRegistration; } private ClientAuthenticationMethod deduceClientAuthenticationMethod(ClientRegistration clientRegistration) { if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(this.authorizationGrantType) - && !StringUtils.hasText(this.clientSecret)) { + && (!StringUtils.hasText(this.clientSecret))) { return ClientAuthenticationMethod.NONE; } return ClientAuthenticationMethod.CLIENT_SECRET_BASIC; diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java new file mode 100644 index 0000000000..de9c4bf7b8 --- /dev/null +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java @@ -0,0 +1,68 @@ +/* + * Copyright 2002-2025 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.registration; + +/** + * A facility for client configuration settings. + * + * @author DingHao + * @since 6.5 + */ +public final class ClientSettings { + + private boolean requireProofKey; + + private ClientSettings() { + + } + + public boolean isRequireProofKey() { + return this.requireProofKey; + } + + public static Builder builder() { + return new Builder(); + } + + public static final class Builder { + + private boolean requireProofKey; + + private Builder() { + } + + /** + * Set to {@code true} if the client is required to provide a proof key challenge + * and verifier when performing the Authorization Code Grant flow. + * @param requireProofKey {@code true} if the client is required to provide a + * proof key challenge and verifier, {@code false} otherwise + * @return the {@link Builder} for further configuration + */ + public Builder requireProofKey(boolean requireProofKey) { + this.requireProofKey = requireProofKey; + return this; + } + + public ClientSettings build() { + ClientSettings clientSettings = new ClientSettings(); + clientSettings.requireProofKey = this.requireProofKey; + return clientSettings; + } + + } + +} diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java index c189317ec4..4909d0f730 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2025 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. @@ -183,7 +183,8 @@ public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2Au // value. applyNonce(builder); } - if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) { + if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod()) + || clientRegistration.getClientSettings().isRequireProofKey()) { DEFAULT_PKCE_APPLIER.accept(builder); } return builder; diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthorizedClientMixinTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthorizedClientMixinTests.java index 3f696c361c..47e4a2d2e3 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthorizedClientMixinTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthorizedClientMixinTests.java @@ -276,7 +276,10 @@ public class OAuth2AuthorizedClientMixinTests { " " + configurationMetadata + "\n" + " }\n" + " },\n" + - " \"clientName\": \"" + clientRegistration.getClientName() + "\"\n" + + " \"clientName\": \"" + clientRegistration.getClientName() + "\",\n" + + " \"clientSettings\": {\n" + + " \"requireProofKey\": " + clientRegistration.getClientSettings().isRequireProofKey() + "\n" + + " }\n" + "}"; // @formatter:on } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java index c10a3f82cf..d0d260922c 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2025 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. @@ -28,6 +28,7 @@ import org.mockito.Mockito; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; +import org.springframework.security.oauth2.client.registration.ClientSettings; import org.springframework.security.oauth2.client.registration.InMemoryClientRegistrationRepository; import org.springframework.security.oauth2.client.registration.TestClientRegistrations; import org.springframework.security.oauth2.core.AuthorizationGrantType; @@ -56,6 +57,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests { private ClientRegistration registration2; + private ClientRegistration pkceClientRegistration; + private ClientRegistration fineRedirectUriTemplateRegistration; private ClientRegistration publicClientRegistration; @@ -72,6 +75,9 @@ public class DefaultOAuth2AuthorizationRequestResolverTests { public void setUp() { this.registration1 = TestClientRegistrations.clientRegistration().build(); this.registration2 = TestClientRegistrations.clientRegistration2().build(); + + this.pkceClientRegistration = pkceClientRegistration().build(); + this.fineRedirectUriTemplateRegistration = fineRedirectUriTemplateClientRegistration().build(); // @formatter:off this.publicClientRegistration = TestClientRegistrations.clientRegistration() @@ -86,8 +92,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests { .build(); // @formatter:on this.clientRegistrationRepository = new InMemoryClientRegistrationRepository(this.registration1, - this.registration2, this.fineRedirectUriTemplateRegistration, this.publicClientRegistration, - this.oidcRegistration); + this.registration2, this.pkceClientRegistration, this.fineRedirectUriTemplateRegistration, + this.publicClientRegistration, this.oidcRegistration); this.resolver = new DefaultOAuth2AuthorizationRequestResolver(this.clientRegistrationRepository, this.authorizationRequestBaseUri); } @@ -563,6 +569,32 @@ public class DefaultOAuth2AuthorizationRequestResolverTests { + "nonce=([a-zA-Z0-9\\-\\.\\_\\~]){43}&" + "appid=client-id"); } + @Test + public void resolveWhenAuthorizationRequestProvideCodeChallengeMethod() { + ClientRegistration clientRegistration = this.pkceClientRegistration; + String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId(); + MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri); + request.setServletPath(requestUri); + OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request); + assertThat(authorizationRequest.getAdditionalParameters().containsKey(PkceParameterNames.CODE_CHALLENGE_METHOD)) + .isTrue(); + } + + private static ClientRegistration.Builder pkceClientRegistration() { + return ClientRegistration.withRegistrationId("pkce") + .redirectUri("{baseUrl}/{action}/oauth2/code/{registrationId}") + .clientSettings(ClientSettings.builder().requireProofKey(true).build()) + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .scope("read:user") + .authorizationUri("https://example.com/login/oauth/authorize") + .tokenUri("https://example.com/login/oauth/access_token") + .userInfoUri("https://api.example.com/user") + .userNameAttributeName("id") + .clientName("Client Name") + .clientId("client-id-3") + .clientSecret("client-secret"); + } + private static ClientRegistration.Builder fineRedirectUriTemplateClientRegistration() { // @formatter:off return ClientRegistration.withRegistrationId("fine-redirect-uri-template-client-registration") From 0ed7b18f423827aee7df47c8db9759d132b89201 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 17 Jan 2025 14:14:55 -0600 Subject: [PATCH 03/10] DefaultServerOAuth2AuthorizationRequestResolver requireProofKey support When requireProofKey=true, DefaultServerOAuth2AuthorizationRequestResolver enables PKCE support. Issue gh-16382 --- ...tServerOAuth2AuthorizationRequestResolver.java | 3 ++- ...erOAuth2AuthorizationRequestResolverTests.java | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java index bb95dd20b7..0123a2aab7 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java @@ -196,7 +196,8 @@ public class DefaultServerOAuth2AuthorizationRequestResolver implements ServerOA // value. applyNonce(builder); } - if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod())) { + if (ClientAuthenticationMethod.NONE.equals(clientRegistration.getClientAuthenticationMethod()) + || clientRegistration.getClientSettings().isRequireProofKey()) { DEFAULT_PKCE_APPLIER.accept(builder); } return builder; diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java index ec293997f5..9772ed1b61 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java @@ -27,6 +27,7 @@ import org.springframework.http.HttpStatus; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.web.server.MockServerWebExchange; import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.client.registration.ClientSettings; import org.springframework.security.oauth2.client.registration.ReactiveClientRegistrationRepository; import org.springframework.security.oauth2.client.registration.TestClientRegistrations; import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestCustomizers; @@ -169,6 +170,20 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests { assertPkceNotApplied(request, registration2); } + @Test + void resolveWhenRequireProofKeyTrueThenPkceEnabled() { + ClientSettings pkceEnabled = ClientSettings.builder().requireProofKey(true).build(); + ClientRegistration clientWithPkceEnabled = TestClientRegistrations.clientRegistration() + .clientSettings(pkceEnabled) + .build(); + given(this.clientRegistrationRepository.findByRegistrationId(any())) + .willReturn(Mono.just(clientWithPkceEnabled)); + + OAuth2AuthorizationRequest request = resolve( + "/oauth2/authorization/" + clientWithPkceEnabled.getRegistrationId()); + assertPkceApplied(request, clientWithPkceEnabled); + } + private void assertPkceApplied(OAuth2AuthorizationRequest authorizationRequest, ClientRegistration clientRegistration) { assertThat(authorizationRequest.getAdditionalParameters()).containsKey(PkceParameterNames.CODE_CHALLENGE); From 2665a92107b85170ca5ddabcc9aeb975bae8886a Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 17 Jan 2025 10:56:42 -0600 Subject: [PATCH 04/10] Ensure that ClientSettings cannot be null This ensures that ClientRegistration.Builder.ClientSettings cannot be null. This has a slight advantage in terms of null safety to making this check happen in the build method since the Builder does not have a null field either. Issue gh-16382 --- .../registration/ClientRegistration.java | 6 ++--- .../registration/ClientRegistrationTests.java | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java index c8fa34e682..8c5c09c140 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java @@ -378,7 +378,7 @@ public final class ClientRegistration implements Serializable { private String clientName; - private ClientSettings clientSettings; + private ClientSettings clientSettings = ClientSettings.builder().build(); private Builder(String registrationId) { this.registrationId = registrationId; @@ -614,6 +614,7 @@ public final class ClientRegistration implements Serializable { * @return the {@link Builder} */ public Builder clientSettings(ClientSettings clientSettings) { + Assert.notNull(clientSettings, "clientSettings cannot be null"); this.clientSettings = clientSettings; return this; } @@ -651,8 +652,7 @@ public final class ClientRegistration implements Serializable { clientRegistration.providerDetails = createProviderDetails(clientRegistration); clientRegistration.clientName = StringUtils.hasText(this.clientName) ? this.clientName : this.registrationId; - clientRegistration.clientSettings = (this.clientSettings == null) ? ClientSettings.builder().build() - : this.clientSettings; + clientRegistration.clientSettings = this.clientSettings; return clientRegistration; } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java index 070e2040bd..2c4ee41a07 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java @@ -753,4 +753,27 @@ public class ClientRegistrationTests { assertThat(clientRegistration.getClientAuthenticationMethod()).isEqualTo(clientAuthenticationMethod); } + @Test + void clientSettingsWhenNullThenThrowIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> ClientRegistration.withRegistrationId(REGISTRATION_ID).clientSettings(null)); + } + + // gh-16382 + @Test + void buildWhenDefaultClientSettingsThenDefaulted() { + ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID) + .clientId(CLIENT_ID) + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .redirectUri(REDIRECT_URI) + .authorizationUri(AUTHORIZATION_URI) + .tokenUri(TOKEN_URI) + .build(); + + // should not be null + assertThat(clientRegistration.getClientSettings()).isNotNull(); + // proof key should be false for passivity + assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isFalse(); + } + } From b0a4dcb89e3dc264279ea8042b69f91ca2d29eaf Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 17 Jan 2025 14:13:30 -0600 Subject: [PATCH 05/10] ClientSettings equals, hashCode, toString Issue gh-16382 --- .../client/registration/ClientSettings.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java index de9c4bf7b8..92c5f4b491 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java @@ -16,6 +16,8 @@ package org.springframework.security.oauth2.client.registration; +import java.util.Objects; + /** * A facility for client configuration settings. * @@ -34,6 +36,27 @@ public final class ClientSettings { return this.requireProofKey; } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof ClientSettings that)) { + return false; + } + return this.requireProofKey == that.requireProofKey; + } + + @Override + public int hashCode() { + return Objects.hashCode(this.requireProofKey); + } + + @Override + public String toString() { + return "ClientSettings{" + "requireProofKey=" + this.requireProofKey + '}'; + } + public static Builder builder() { return new Builder(); } From ab629cc1cad533501c5c6ad3db5ae1128ef7dfc4 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 17 Jan 2025 10:58:08 -0600 Subject: [PATCH 06/10] Add AuthorizationGrantType.toString() This adds AuthorizationGrantType.toString() which makes debuging easier. In particular, it will help when performing unit tests which validate the AuthorizationGrantType. Issue gh-16382 --- .../security/oauth2/core/AuthorizationGrantType.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/AuthorizationGrantType.java b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/AuthorizationGrantType.java index e1321bd759..433811a781 100644 --- a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/AuthorizationGrantType.java +++ b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/AuthorizationGrantType.java @@ -111,4 +111,9 @@ public final class AuthorizationGrantType implements Serializable { return this.getValue().hashCode(); } + @Override + public String toString() { + return "AuthorizationGrantType{" + "value='" + this.value + '\'' + '}'; + } + } From f9498d38853172ee73ddc91e6eb2dc4f13e3cccb Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 17 Jan 2025 10:59:48 -0600 Subject: [PATCH 07/10] PKCE cannot be true and AuthorizationGrantType != AUTHORIZATION_CODE PKCE is only valid for AuthorizationGrantType.AUTHORIZATION_CODE so the code should validate this. Issue gh-16382 --- .../registration/ClientRegistration.java | 6 ++ .../registration/ClientRegistrationTests.java | 62 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java index 8c5c09c140..199a54dca2 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java @@ -711,6 +711,12 @@ 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); + } } } diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java index 2c4ee41a07..1816843286 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java @@ -16,14 +16,20 @@ package org.springframework.security.oauth2.client.registration; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import org.springframework.security.oauth2.core.AuthenticationMethod; import org.springframework.security.oauth2.core.AuthorizationGrantType; @@ -31,6 +37,7 @@ 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; /** * Tests for {@link ClientRegistration}. @@ -776,4 +783,59 @@ public class ClientRegistrationTests { assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isFalse(); } + // gh-16382 + @Test + void buildWhenNewAuthorizationCodeAndPkceThenBuilds() { + ClientSettings pkceEnabled = ClientSettings.builder().requireProofKey(true).build(); + ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID) + .clientId(CLIENT_ID) + .clientSettings(pkceEnabled) + .authorizationGrantType(new AuthorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue())) + .redirectUri(REDIRECT_URI) + .authorizationUri(AUTHORIZATION_URI) + .tokenUri(TOKEN_URI) + .build(); + + // proof key should be false for passivity + assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isTrue(); + } + + @ParameterizedTest + @MethodSource("invalidPkceGrantTypes") + void buildWhenInvalidGrantTypeForPkceThenException(AuthorizationGrantType invalidGrantType) { + ClientSettings pkceEnabled = ClientSettings.builder().requireProofKey(true).build(); + ClientRegistration.Builder builder = ClientRegistration.withRegistrationId(REGISTRATION_ID) + .clientId(CLIENT_ID) + .clientSettings(pkceEnabled) + .authorizationGrantType(invalidGrantType) + .redirectUri(REDIRECT_URI) + .authorizationUri(AUTHORIZATION_URI) + .tokenUri(TOKEN_URI); + + assertThatIllegalStateException().describedAs( + "clientSettings.isRequireProofKey=true is only valid with authorizationGrantType=AUTHORIZATION_CODE. Got authorizationGrantType={}", + invalidGrantType) + .isThrownBy(builder::build); + } + + static List invalidPkceGrantTypes() { + return Arrays.stream(AuthorizationGrantType.class.getFields()) + .filter((field) -> Modifier.isFinal(field.getModifiers()) + && field.getType() == AuthorizationGrantType.class) + .map((field) -> getStaticValue(field, AuthorizationGrantType.class)) + .filter((grantType) -> grantType != AuthorizationGrantType.AUTHORIZATION_CODE) + // ensure works with .equals + .map((grantType) -> new AuthorizationGrantType(grantType.getValue())) + .collect(Collectors.toList()); + } + + private static T getStaticValue(Field field, Class clazz) { + try { + return (T) field.get(null); + } + catch (IllegalAccessException ex) { + throw new RuntimeException(ex); + } + } + } From 4c533569bba801f84f4184f45c4dea2944e71e98 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:19:52 -0600 Subject: [PATCH 08/10] Ensure missing ClientRegistration.clientSettings JSON node works Issue gh-16382 --- .../OAuth2AuthorizedClientMixinTests.java | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthorizedClientMixinTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthorizedClientMixinTests.java index 47e4a2d2e3..d6d0e81927 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthorizedClientMixinTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthorizedClientMixinTests.java @@ -214,6 +214,71 @@ public class OAuth2AuthorizedClientMixinTests { assertThat(authorizedClient.getRefreshToken()).isNull(); } + @Test + void deserializeWhenClientSettingsPropertyDoesNotExistThenDefaulted() throws JsonProcessingException { + // ClientRegistration.clientSettings was added later, so old values will be + // serialized without that property + // this test checks for passivity + ClientRegistration clientRegistration = this.clientRegistrationBuilder.build(); + ClientRegistration.ProviderDetails providerDetails = clientRegistration.getProviderDetails(); + ClientRegistration.ProviderDetails.UserInfoEndpoint userInfoEndpoint = providerDetails.getUserInfoEndpoint(); + String scopes = ""; + if (!CollectionUtils.isEmpty(clientRegistration.getScopes())) { + scopes = StringUtils.collectionToDelimitedString(clientRegistration.getScopes(), ",", "\"", "\""); + } + String configurationMetadata = "\"@class\": \"java.util.Collections$UnmodifiableMap\""; + if (!CollectionUtils.isEmpty(providerDetails.getConfigurationMetadata())) { + configurationMetadata += "," + providerDetails.getConfigurationMetadata() + .keySet() + .stream() + .map((key) -> "\"" + key + "\": \"" + providerDetails.getConfigurationMetadata().get(key) + "\"") + .collect(Collectors.joining(",")); + } + // @formatter:off + String json = "{\n" + + " \"@class\": \"org.springframework.security.oauth2.client.registration.ClientRegistration\",\n" + + " \"registrationId\": \"" + clientRegistration.getRegistrationId() + "\",\n" + + " \"clientId\": \"" + clientRegistration.getClientId() + "\",\n" + + " \"clientSecret\": \"" + clientRegistration.getClientSecret() + "\",\n" + + " \"clientAuthenticationMethod\": {\n" + + " \"value\": \"" + clientRegistration.getClientAuthenticationMethod().getValue() + "\"\n" + + " },\n" + + " \"authorizationGrantType\": {\n" + + " \"value\": \"" + clientRegistration.getAuthorizationGrantType().getValue() + "\"\n" + + " },\n" + + " \"redirectUri\": \"" + clientRegistration.getRedirectUri() + "\",\n" + + " \"scopes\": [\n" + + " \"java.util.Collections$UnmodifiableSet\",\n" + + " [" + scopes + "]\n" + + " ],\n" + + " \"providerDetails\": {\n" + + " \"@class\": \"org.springframework.security.oauth2.client.registration.ClientRegistration$ProviderDetails\",\n" + + " \"authorizationUri\": \"" + providerDetails.getAuthorizationUri() + "\",\n" + + " \"tokenUri\": \"" + providerDetails.getTokenUri() + "\",\n" + + " \"userInfoEndpoint\": {\n" + + " \"@class\": \"org.springframework.security.oauth2.client.registration.ClientRegistration$ProviderDetails$UserInfoEndpoint\",\n" + + " \"uri\": " + ((userInfoEndpoint.getUri() != null) ? "\"" + userInfoEndpoint.getUri() + "\"" : null) + ",\n" + + " \"authenticationMethod\": {\n" + + " \"value\": \"" + userInfoEndpoint.getAuthenticationMethod().getValue() + "\"\n" + + " },\n" + + " \"userNameAttributeName\": " + ((userInfoEndpoint.getUserNameAttributeName() != null) ? "\"" + userInfoEndpoint.getUserNameAttributeName() + "\"" : null) + "\n" + + " },\n" + + " \"jwkSetUri\": " + ((providerDetails.getJwkSetUri() != null) ? "\"" + providerDetails.getJwkSetUri() + "\"" : null) + ",\n" + + " \"issuerUri\": " + ((providerDetails.getIssuerUri() != null) ? "\"" + providerDetails.getIssuerUri() + "\"" : null) + ",\n" + + " \"configurationMetadata\": {\n" + + " " + configurationMetadata + "\n" + + " }\n" + + " },\n" + + " \"clientName\": \"" + clientRegistration.getClientName() + "\"\n" + + "}"; + // @formatter:on + // validate the test input + assertThat(json).doesNotContain("clientSettings"); + ClientRegistration registration = this.mapper.readValue(json, ClientRegistration.class); + // the default value of requireProofKey is false + assertThat(registration.getClientSettings().isRequireProofKey()).isFalse(); + } + private static String asJson(OAuth2AuthorizedClient authorizedClient) { // @formatter:off return "{\n" + From 004f38639d0b0e5cb619a2582085933edb9497de Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 17 Jan 2025 17:18:30 -0600 Subject: [PATCH 09/10] Move ClientSettings to ClientRegistration Initially it was proposed to put ClientSettings as a top level class, but to be consistent with ProviderDetails, this commit moves ClientSettings to be an inner class of ClientRegistration Issue gh-16382 # Conflicts: # oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java --- .../registration/ClientRegistration.java | 73 +++++++++++++++ .../client/registration/ClientSettings.java | 91 ------------------- .../registration/ClientRegistrationTests.java | 8 +- ...uth2AuthorizationRequestResolverTests.java | 3 +- ...uth2AuthorizationRequestResolverTests.java | 5 +- 5 files changed, 83 insertions(+), 97 deletions(-) delete mode 100644 oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java index 199a54dca2..0b4179ff03 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java @@ -26,6 +26,7 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import org.apache.commons.logging.Log; @@ -741,4 +742,76 @@ public final class ClientRegistration implements Serializable { } + /** + * A facility for client configuration settings. + * + * @author DingHao + * @since 6.5 + */ + public static final class ClientSettings { + + private boolean requireProofKey; + + private ClientSettings() { + + } + + public boolean isRequireProofKey() { + return this.requireProofKey; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof ClientSettings that)) { + return false; + } + return this.requireProofKey == that.requireProofKey; + } + + @Override + public int hashCode() { + return Objects.hashCode(this.requireProofKey); + } + + @Override + public String toString() { + return "ClientSettings{" + "requireProofKey=" + this.requireProofKey + '}'; + } + + public static Builder builder() { + return new Builder(); + } + + public static final class Builder { + + private boolean requireProofKey; + + private Builder() { + } + + /** + * Set to {@code true} if the client is required to provide a proof key + * challenge and verifier when performing the Authorization Code Grant flow. + * @param requireProofKey {@code true} if the client is required to provide a + * proof key challenge and verifier, {@code false} otherwise + * @return the {@link Builder} for further configuration + */ + public Builder requireProofKey(boolean requireProofKey) { + this.requireProofKey = requireProofKey; + return this; + } + + public ClientSettings build() { + ClientSettings clientSettings = new ClientSettings(); + clientSettings.requireProofKey = this.requireProofKey; + return clientSettings; + } + + } + + } + } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java deleted file mode 100644 index 92c5f4b491..0000000000 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientSettings.java +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Copyright 2002-2025 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.registration; - -import java.util.Objects; - -/** - * A facility for client configuration settings. - * - * @author DingHao - * @since 6.5 - */ -public final class ClientSettings { - - private boolean requireProofKey; - - private ClientSettings() { - - } - - public boolean isRequireProofKey() { - return this.requireProofKey; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (!(o instanceof ClientSettings that)) { - return false; - } - return this.requireProofKey == that.requireProofKey; - } - - @Override - public int hashCode() { - return Objects.hashCode(this.requireProofKey); - } - - @Override - public String toString() { - return "ClientSettings{" + "requireProofKey=" + this.requireProofKey + '}'; - } - - public static Builder builder() { - return new Builder(); - } - - public static final class Builder { - - private boolean requireProofKey; - - private Builder() { - } - - /** - * Set to {@code true} if the client is required to provide a proof key challenge - * and verifier when performing the Authorization Code Grant flow. - * @param requireProofKey {@code true} if the client is required to provide a - * proof key challenge and verifier, {@code false} otherwise - * @return the {@link Builder} for further configuration - */ - public Builder requireProofKey(boolean requireProofKey) { - this.requireProofKey = requireProofKey; - return this; - } - - public ClientSettings build() { - ClientSettings clientSettings = new ClientSettings(); - clientSettings.requireProofKey = this.requireProofKey; - return clientSettings; - } - - } - -} diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java index 1816843286..9dbcbd5a5c 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java @@ -786,7 +786,9 @@ public class ClientRegistrationTests { // gh-16382 @Test void buildWhenNewAuthorizationCodeAndPkceThenBuilds() { - ClientSettings pkceEnabled = ClientSettings.builder().requireProofKey(true).build(); + ClientRegistration.ClientSettings pkceEnabled = ClientRegistration.ClientSettings.builder() + .requireProofKey(true) + .build(); ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID) .clientId(CLIENT_ID) .clientSettings(pkceEnabled) @@ -803,7 +805,9 @@ public class ClientRegistrationTests { @ParameterizedTest @MethodSource("invalidPkceGrantTypes") void buildWhenInvalidGrantTypeForPkceThenException(AuthorizationGrantType invalidGrantType) { - ClientSettings pkceEnabled = ClientSettings.builder().requireProofKey(true).build(); + ClientRegistration.ClientSettings pkceEnabled = ClientRegistration.ClientSettings.builder() + .requireProofKey(true) + .build(); ClientRegistration.Builder builder = ClientRegistration.withRegistrationId(REGISTRATION_ID) .clientId(CLIENT_ID) .clientSettings(pkceEnabled) diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java index d0d260922c..a0abf7132e 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java @@ -28,7 +28,6 @@ import org.mockito.Mockito; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; -import org.springframework.security.oauth2.client.registration.ClientSettings; import org.springframework.security.oauth2.client.registration.InMemoryClientRegistrationRepository; import org.springframework.security.oauth2.client.registration.TestClientRegistrations; import org.springframework.security.oauth2.core.AuthorizationGrantType; @@ -583,7 +582,7 @@ public class DefaultOAuth2AuthorizationRequestResolverTests { private static ClientRegistration.Builder pkceClientRegistration() { return ClientRegistration.withRegistrationId("pkce") .redirectUri("{baseUrl}/{action}/oauth2/code/{registrationId}") - .clientSettings(ClientSettings.builder().requireProofKey(true).build()) + .clientSettings(ClientRegistration.ClientSettings.builder().requireProofKey(true).build()) .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) .scope("read:user") .authorizationUri("https://example.com/login/oauth/authorize") diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java index 9772ed1b61..bf7ab09678 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java @@ -27,7 +27,6 @@ import org.springframework.http.HttpStatus; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.web.server.MockServerWebExchange; import org.springframework.security.oauth2.client.registration.ClientRegistration; -import org.springframework.security.oauth2.client.registration.ClientSettings; import org.springframework.security.oauth2.client.registration.ReactiveClientRegistrationRepository; import org.springframework.security.oauth2.client.registration.TestClientRegistrations; import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestCustomizers; @@ -172,7 +171,9 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests { @Test void resolveWhenRequireProofKeyTrueThenPkceEnabled() { - ClientSettings pkceEnabled = ClientSettings.builder().requireProofKey(true).build(); + ClientRegistration.ClientSettings pkceEnabled = ClientRegistration.ClientSettings.builder() + .requireProofKey(true) + .build(); ClientRegistration clientWithPkceEnabled = TestClientRegistrations.clientRegistration() .clientSettings(pkceEnabled) .build(); From 85d7cc13355493cfad52893b9cb9b9098d21f636 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Fri, 17 Jan 2025 16:43:27 -0600 Subject: [PATCH 10/10] Document requireProofKey Issue gh-16386 --- .../pages/reactive/oauth2/client/authorization-grants.adoc | 4 ++++ docs/modules/ROOT/pages/reactive/oauth2/client/core.adoc | 5 +++++ .../pages/servlet/oauth2/client/authorization-grants.adoc | 7 ++++++- docs/modules/ROOT/pages/servlet/oauth2/client/core.adoc | 5 +++++ docs/modules/ROOT/pages/whats-new.adoc | 4 ++++ 5 files changed, 24 insertions(+), 1 deletion(-) diff --git a/docs/modules/ROOT/pages/reactive/oauth2/client/authorization-grants.adoc b/docs/modules/ROOT/pages/reactive/oauth2/client/authorization-grants.adoc index bd002f31e8..59def321a4 100644 --- a/docs/modules/ROOT/pages/reactive/oauth2/client/authorization-grants.adoc +++ b/docs/modules/ROOT/pages/reactive/oauth2/client/authorization-grants.adoc @@ -79,6 +79,10 @@ 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`) +or + +. When `ClientRegistration.clientSettings.requireProofKey` is `true` (in this case `ClientRegistration.authorizationGrantType` must be `authorization_code`) + [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())`. diff --git a/docs/modules/ROOT/pages/reactive/oauth2/client/core.adoc b/docs/modules/ROOT/pages/reactive/oauth2/client/core.adoc index b6eee52585..e1ca19df49 100644 --- a/docs/modules/ROOT/pages/reactive/oauth2/client/core.adoc +++ b/docs/modules/ROOT/pages/reactive/oauth2/client/core.adoc @@ -39,6 +39,10 @@ public final class ClientRegistration { } } + + public static final class ClientSettings { + private boolean requireProofKey; // <17> + } } ---- <1> `registrationId`: The ID that uniquely identifies the `ClientRegistration`. @@ -64,6 +68,7 @@ The name may be used in certain scenarios, such as when displaying the name of t <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. A `ClientRegistration` can be initially configured 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]. diff --git a/docs/modules/ROOT/pages/servlet/oauth2/client/authorization-grants.adoc b/docs/modules/ROOT/pages/servlet/oauth2/client/authorization-grants.adoc index 4cab6a8472..f2f14be45e 100644 --- a/docs/modules/ROOT/pages/servlet/oauth2/client/authorization-grants.adoc +++ b/docs/modules/ROOT/pages/servlet/oauth2/client/authorization-grants.adoc @@ -77,9 +77,14 @@ spring: Public Clients are supported by using https://tools.ietf.org/html/rfc7636[Proof Key for Code Exchange] (PKCE). If the client is running in an untrusted environment (such as a native application or web browser-based application) and is therefore incapable of maintaining the confidentiality of its credentials, PKCE is automatically used when the following conditions are true: -. `client-secret` is omitted (or empty) +. `client-secret` is omitted (or empty) and . `client-authentication-method` is set to `none` (`ClientAuthenticationMethod.NONE`) +or + +. When `ClientRegistration.clientSettings.requireProofKey` is `true` (in this case `ClientRegistration.authorizationGrantType` must be `authorization_code`) + + [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())`. diff --git a/docs/modules/ROOT/pages/servlet/oauth2/client/core.adoc b/docs/modules/ROOT/pages/servlet/oauth2/client/core.adoc index 375c8a12a8..0418877371 100644 --- a/docs/modules/ROOT/pages/servlet/oauth2/client/core.adoc +++ b/docs/modules/ROOT/pages/servlet/oauth2/client/core.adoc @@ -40,6 +40,10 @@ public final class ClientRegistration { } } + + public static final class ClientSettings { + private boolean requireProofKey; // <17> + } } ---- <1> `registrationId`: The ID that uniquely identifies the `ClientRegistration`. @@ -65,6 +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. 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]. diff --git a/docs/modules/ROOT/pages/whats-new.adoc b/docs/modules/ROOT/pages/whats-new.adoc index c6f44092c2..0fc13497b5 100644 --- a/docs/modules/ROOT/pages/whats-new.adoc +++ b/docs/modules/ROOT/pages/whats-new.adoc @@ -10,3 +10,7 @@ Below are the highlights of the release, or you can view https://github.com/spri The `security.security.reached.filter.section` key name was corrected to `spring.security.reached.filter.section`. Note that this may affect reports that operate on this key name. + +== OAuth + +* https://github.com/spring-projects/spring-security/pull/16386[gh-16386] - Enable PKCE for confidential clients using `ClientRegistration.clientSettings.requireProofKey=true` for xref:servlet/oauth2/client/core.adoc#oauth2Client-client-registration-requireProofKey[servlet] and xref:reactive/oauth2/client/core.adoc#oauth2Client-client-registration-requireProofKey[reactive] applications