Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ClientRegistration.clientSettings.requireProofKey to Enable PKCE #16386

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -71,6 +71,8 @@ public final class ClientRegistration implements Serializable {

private String clientName;

private ClientSettings clientSettings;

private ClientRegistration() {
}

Expand Down Expand Up @@ -162,6 +164,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
Expand All @@ -175,6 +185,7 @@ public String toString() {
+ '\'' + ", scopes=" + this.scopes
+ ", providerDetails=" + this.providerDetails
+ ", clientName='" + this.clientName + '\''
+ ", clientSettings='" + this.clientSettings + '\''
+ '}';
// @formatter:on
}
Expand Down Expand Up @@ -367,6 +378,8 @@ public static final class Builder implements Serializable {

private String clientName;

private ClientSettings clientSettings;

private Builder(String registrationId) {
this.registrationId = registrationId;
}
Expand All @@ -391,6 +404,7 @@ private Builder(ClientRegistration clientRegistration) {
this.configurationMetadata = new HashMap<>(configurationMetadata);
}
this.clientName = clientRegistration.clientName;
this.clientSettings = clientRegistration.clientSettings;
}

/**
Expand Down Expand Up @@ -594,6 +608,16 @@ 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) {
this.clientSettings = clientSettings;
return this;
}

/**
* Builds a new {@link ClientRegistration}.
* @return a {@link ClientRegistration}
Expand Down Expand Up @@ -627,12 +651,14 @@ private ClientRegistration create() {
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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}

}

}
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -28,6 +28,7 @@
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;
Expand Down Expand Up @@ -56,6 +57,8 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {

private ClientRegistration registration2;

private ClientRegistration pkceClientRegistration;

private ClientRegistration fineRedirectUriTemplateRegistration;

private ClientRegistration publicClientRegistration;
Expand All @@ -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()
Expand All @@ -86,8 +92,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);
}
Expand Down Expand Up @@ -563,6 +569,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(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")
Expand Down
Loading