Skip to content

Commit

Permalink
Add ClientRegistration.clientSettings.requireProofKey
Browse files Browse the repository at this point in the history
Setting ClientRegistration.clientSettings.requireProofKey=true will
enable PKCE for clients using authorization_code grant type.

Closes gh-16386
  • Loading branch information
rwinch committed Jan 17, 2025
2 parents c2a5709 + 85d7cc1 commit 4fc99aa
Show file tree
Hide file tree
Showing 15 changed files with 354 additions and 11 deletions.
4 changes: 4 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())`.
Expand Down
5 changes: 5 additions & 0 deletions docs/modules/ROOT/pages/reactive/oauth2/client/core.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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].

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())`.
Expand Down
5 changes: 5 additions & 0 deletions docs/modules/ROOT/pages/servlet/oauth2/client/core.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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].

Expand Down
4 changes: 4 additions & 0 deletions docs/modules/ROOT/pages/whats-new.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 All @@ -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;
Expand Down Expand Up @@ -71,6 +72,8 @@ public final class ClientRegistration implements Serializable {

private String clientName;

private ClientSettings clientSettings;

private ClientRegistration() {
}

Expand Down Expand Up @@ -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
Expand All @@ -175,6 +186,7 @@ public String toString() {
+ '\'' + ", scopes=" + this.scopes
+ ", providerDetails=" + this.providerDetails
+ ", clientName='" + this.clientName + '\''
+ ", clientSettings='" + this.clientSettings + '\''
+ '}';
// @formatter:on
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -391,6 +405,7 @@ private Builder(ClientRegistration clientRegistration) {
this.configurationMetadata = new HashMap<>(configurationMetadata);
}
this.clientName = clientRegistration.clientName;
this.clientSettings = clientRegistration.clientSettings;
}

/**
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}

Expand All @@ -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;
}

}

}

}
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 @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" +
Expand Down Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 4fc99aa

Please sign in to comment.