diff --git a/build.gradle b/build.gradle index 60089e6734f..4fd368b54a9 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") 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 bd002f31e8b..59def321a46 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 b6eee52585a..e1ca19df49f 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 4cab6a8472b..f2f14be45e9 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 375c8a12a89..04188773718 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 c6f44092c28..0fc13497b56 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 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 77b1fdd1217..d8fc1a099d6 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 0639a395f89..0b4179ff03e 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. @@ -26,6 +26,7 @@ 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; @@ -71,6 +72,8 @@ public final class ClientRegistration implements Serializable { private String clientName; + private ClientSettings clientSettings; + private ClientRegistration() { } @@ -162,6 +165,14 @@ public String getClientName() { 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 +186,7 @@ public String toString() { + '\'' + ", scopes=" + this.scopes + ", providerDetails=" + this.providerDetails + ", clientName='" + this.clientName + '\'' + + ", clientSettings='" + this.clientSettings + '\'' + '}'; // @formatter:on } @@ -367,6 +379,8 @@ public static final class Builder implements Serializable { private String clientName; + private ClientSettings clientSettings = ClientSettings.builder().build(); + private Builder(String registrationId) { this.registrationId = registrationId; } @@ -391,6 +405,7 @@ private Builder(ClientRegistration clientRegistration) { this.configurationMetadata = new HashMap<>(configurationMetadata); } this.clientName = clientRegistration.clientName; + this.clientSettings = clientRegistration.clientSettings; } /** @@ -594,6 +609,17 @@ public Builder clientName(String clientName) { return this; } + /** + * Sets the {@link ClientSettings client configuration settings}. + * @param clientSettings the client configuration settings + * @return the {@link Builder} + */ + public Builder clientSettings(ClientSettings clientSettings) { + Assert.notNull(clientSettings, "clientSettings cannot be null"); + this.clientSettings = clientSettings; + return this; + } + /** * Builds a new {@link ClientRegistration}. * @return a {@link ClientRegistration} @@ -627,12 +653,13 @@ private ClientRegistration create() { clientRegistration.providerDetails = createProviderDetails(clientRegistration); clientRegistration.clientName = StringUtils.hasText(this.clientName) ? this.clientName : this.registrationId; + clientRegistration.clientSettings = 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; @@ -685,6 +712,12 @@ private void validateAuthorizationGrantTypes() { "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); + } } } @@ -709,4 +742,76 @@ private static boolean withinTheRangeOf(int c, int min, int max) { } + /** + * 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/web/DefaultOAuth2AuthorizationRequestResolver.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java index c189317ec43..4909d0f730d 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 @@ private OAuth2AuthorizationRequest.Builder getBuilder(ClientRegistration clientR // 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/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 bb95dd20b73..0123a2aab7d 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 @@ private OAuth2AuthorizationRequest.Builder getBuilder(ClientRegistration clientR // 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 3f696c361c1..d6d0e819276 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 void deserializeWhenRequiredAttributesOnlyThenDeserializes() throws Excep 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" + @@ -276,7 +341,10 @@ private static String asJson(ClientRegistration clientRegistration) { " " + 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/registration/ClientRegistrationTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java index 070e2040bdb..9dbcbd5a5c8 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 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}. @@ -753,4 +760,86 @@ public void buildWhenCustomClientAuthenticationMethodProvidedThenSet() { 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(); + } + + // gh-16382 + @Test + void buildWhenNewAuthorizationCodeAndPkceThenBuilds() { + 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())) + .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) { + ClientRegistration.ClientSettings pkceEnabled = ClientRegistration.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); + } + } + } 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 c10a3f82cfc..a0abf7132e4 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. @@ -56,6 +56,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests { private ClientRegistration registration2; + private ClientRegistration pkceClientRegistration; + private ClientRegistration fineRedirectUriTemplateRegistration; private ClientRegistration publicClientRegistration; @@ -72,6 +74,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 +91,8 @@ public void setUp() { .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 +568,32 @@ public void resolveWhenAuthorizationRequestCustomizerOverridesParameterThenQuery + "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(ClientRegistration.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") 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 ec293997f5e..bf7ab096784 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 @@ -169,6 +169,22 @@ public void resolveWhenAuthorizationRequestApplyPkceToSpecificConfidentialClient assertPkceNotApplied(request, registration2); } + @Test + void resolveWhenRequireProofKeyTrueThenPkceEnabled() { + ClientRegistration.ClientSettings pkceEnabled = ClientRegistration.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); 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 e1321bd7595..433811a781a 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 int hashCode() { return this.getValue().hashCode(); } + @Override + public String toString() { + return "AuthorizationGrantType{" + "value='" + this.value + '\'' + '}'; + } + }