Skip to content
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
Expand Up @@ -156,7 +156,7 @@ public void configureWhenAuthorizationCodeRequestThenRedirectForAuthorization()
.andExpect(status().is3xxRedirection()).andReturn();
assertThat(mvcResult.getResponse().getRedirectedUrl())
.matches("https://provider.com/oauth2/authorize\\?" + "response_type=code&client_id=client-1&"
+ "scope=user&state=.{15,}&" + "redirect_uri=http://localhost/client-1");
+ "scope=user&state=.{15,}&" + "redirect_uri=http://localhost/client-1&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
// @formatter:on
}

Expand All @@ -166,9 +166,9 @@ public void configureWhenOauth2ClientInLambdaThenRedirectForAuthorization() thro
MvcResult mvcResult = this.mockMvc.perform(get("/oauth2/authorization/registration-1"))
.andExpect(status().is3xxRedirection())
.andReturn();
assertThat(mvcResult.getResponse().getRedirectedUrl())
.matches("https://provider.com/oauth2/authorize\\?" + "response_type=code&client_id=client-1&"
+ "scope=user&state=.{15,}&" + "redirect_uri=http://localhost/client-1");
assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?"
+ "response_type=code&client_id=client-1&" + "scope=user&state=.{15,}&"
+ "redirect_uri=http://localhost/client-1&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ public void requestWhenAuthorizeThenRedirect() throws Exception {
.andExpect(status().is3xxRedirection())
.andReturn();
// @formatter:on
assertThat(result.getResponse().getRedirectedUrl()).matches(
"https://accounts.google.com/o/oauth2/v2/auth\\?" + "response_type=code&client_id=google-client-id&"
+ "scope=scope1%20scope2&state=.{15,}&redirect_uri=http://localhost/callback/google");
assertThat(result.getResponse().getRedirectedUrl()).matches("https://accounts.google.com/o/oauth2/v2/auth\\?"
+ "response_type=code&client_id=google-client-id&"
+ "scope=scope1%20scope2&state=.{15,}&redirect_uri=http://localhost/callback/google&code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}&code_challenge_method=S256");
}

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

Expand Down
2 changes: 1 addition & 1 deletion docs/modules/ROOT/pages/servlet/oauth2/client/core.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ This information is available only if the Spring Boot property `spring.security.
<15> `(userInfoEndpoint)authenticationMethod`: The authentication method used when sending the access token to the UserInfo Endpoint.
The supported values are *header*, *form*, and *query*.
<16> `userNameAttributeName`: The name of the attribute returned in the UserInfo Response that references the Name or Identifier of the end-user.
<17> [[oauth2Client-client-registration-requireProofKey]]`requireProofKey`: If `true` or if `authorizationGrantType` is `none`, then PKCE will be enabled by default.
<17> [[oauth2Client-client-registration-requireProofKey]]`requireProofKey`: If `true` or if `clientAuthenticationMethod` is `none`, then PKCE will be enabled.

You can initially configure a `ClientRegistration` by using discovery of an OpenID Connect Provider's https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig[Configuration endpoint] or an Authorization Server's https://tools.ietf.org/html/rfc8414#section-3[Metadata endpoint].

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,10 @@ private ClientRegistration create() {
clientRegistration.clientName = StringUtils.hasText(this.clientName) ? this.clientName
: this.registrationId;
clientRegistration.clientSettings = this.clientSettings;
if (clientRegistration.clientSettings.requireProofKey) {
clientRegistration.clientSettings.requireProofKey = AuthorizationGrantType.AUTHORIZATION_CODE
.equals(this.authorizationGrantType);
}
return clientRegistration;
}

Expand Down Expand Up @@ -701,12 +705,6 @@ 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 Down Expand Up @@ -779,7 +777,7 @@ public static Builder builder() {

public static final class Builder {

private boolean requireProofKey;
private boolean requireProofKey = true;

private Builder() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@
import org.springframework.security.oauth2.core.AuthorizationGrantType;
import org.springframework.security.oauth2.core.ClientAuthenticationMethod;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.assertj.core.api.Assertions.*;

/**
* Tests for {@link ClientRegistration}.
Expand Down Expand Up @@ -683,7 +681,7 @@ void buildWhenDefaultClientSettingsThenDefaulted() {
// should not be null
assertThat(clientRegistration.getClientSettings()).isNotNull();
// proof key should be false for passivity
assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isFalse();
assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isTrue();
}

// gh-16382
Expand All @@ -701,28 +699,70 @@ void buildWhenNewAuthorizationCodeAndPkceThenBuilds() {
.tokenUri(TOKEN_URI)
.build();

// proof key should be false for passivity
assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isTrue();
}

@Test
void buildWhenNewAuthorizationCodeAndPkceDisabledThenBuilds() {
ClientRegistration.ClientSettings pkceDisabled = ClientRegistration.ClientSettings.builder()
.requireProofKey(false)
.build();
ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
.clientId(CLIENT_ID)
.clientSettings(pkceDisabled)
.authorizationGrantType(new AuthorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()))
.redirectUri(REDIRECT_URI)
.authorizationUri(AUTHORIZATION_URI)
.tokenUri(TOKEN_URI)
.build();

assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isFalse();
}

@Test
void buildWhenNewAuthorizationCodeAndPrivateClientThenPkceEnabledAndExceptionThrown() {
List<ClientAuthenticationMethod> clientAuthenticationMethods = Arrays
.stream(ClientAuthenticationMethod.class.getFields())
.filter((field) -> Modifier.isFinal(field.getModifiers())
&& field.getType() == ClientAuthenticationMethod.class)
.map((field) -> getStaticValue(field, ClientAuthenticationMethod.class))
.filter((authenticationMethod) -> authenticationMethod != ClientAuthenticationMethod.NONE)
.map((authenticationMethod) -> new ClientAuthenticationMethod(authenticationMethod.getValue()))
.toList();
for (ClientAuthenticationMethod clientAuthenticationMethod : clientAuthenticationMethods) {
ClientRegistration.ClientSettings pkceEnabled = ClientRegistration.ClientSettings.builder()
.requireProofKey(true)
.build();
ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
.clientId(CLIENT_ID)
.clientSettings(pkceEnabled)
.authorizationGrantType(
new AuthorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()))
.clientAuthenticationMethod(clientAuthenticationMethod)
.redirectUri(REDIRECT_URI)
.authorizationUri(AUTHORIZATION_URI)
.tokenUri(TOKEN_URI)
.build();
assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isTrue();
}
}

@ParameterizedTest
@MethodSource("invalidPkceGrantTypes")
void buildWhenInvalidGrantTypeForPkceThenException(AuthorizationGrantType invalidGrantType) {
ClientRegistration.ClientSettings pkceEnabled = ClientRegistration.ClientSettings.builder()
.requireProofKey(true)
.build();
ClientRegistration.Builder builder = ClientRegistration.withRegistrationId(REGISTRATION_ID)
ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
.clientId(CLIENT_ID)
.clientSettings(pkceEnabled)
.authorizationGrantType(invalidGrantType)
.redirectUri(REDIRECT_URI)
.authorizationUri(AUTHORIZATION_URI)
.tokenUri(TOKEN_URI);
.tokenUri(TOKEN_URI)
.build();

assertThatIllegalStateException().describedAs(
"clientSettings.isRequireProofKey=true is only valid with authorizationGrantType=AUTHORIZATION_CODE. Got authorizationGrantType={}",
invalidGrantType)
.isThrownBy(builder::build);
assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isFalse();
}

static List<AuthorizationGrantType> invalidPkceGrantTypes() {
Expand Down
Loading