Skip to content

Commit 2665a92

Browse files
committed
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
1 parent 0ed7b18 commit 2665a92

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ public static final class Builder implements Serializable {
378378

379379
private String clientName;
380380

381-
private ClientSettings clientSettings;
381+
private ClientSettings clientSettings = ClientSettings.builder().build();
382382

383383
private Builder(String registrationId) {
384384
this.registrationId = registrationId;
@@ -614,6 +614,7 @@ public Builder clientName(String clientName) {
614614
* @return the {@link Builder}
615615
*/
616616
public Builder clientSettings(ClientSettings clientSettings) {
617+
Assert.notNull(clientSettings, "clientSettings cannot be null");
617618
this.clientSettings = clientSettings;
618619
return this;
619620
}
@@ -651,8 +652,7 @@ private ClientRegistration create() {
651652
clientRegistration.providerDetails = createProviderDetails(clientRegistration);
652653
clientRegistration.clientName = StringUtils.hasText(this.clientName) ? this.clientName
653654
: this.registrationId;
654-
clientRegistration.clientSettings = (this.clientSettings == null) ? ClientSettings.builder().build()
655-
: this.clientSettings;
655+
clientRegistration.clientSettings = this.clientSettings;
656656
return clientRegistration;
657657
}
658658

oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,4 +753,27 @@ public void buildWhenCustomClientAuthenticationMethodProvidedThenSet() {
753753
assertThat(clientRegistration.getClientAuthenticationMethod()).isEqualTo(clientAuthenticationMethod);
754754
}
755755

756+
@Test
757+
void clientSettingsWhenNullThenThrowIllegalArgumentException() {
758+
assertThatIllegalArgumentException()
759+
.isThrownBy(() -> ClientRegistration.withRegistrationId(REGISTRATION_ID).clientSettings(null));
760+
}
761+
762+
// gh-16382
763+
@Test
764+
void buildWhenDefaultClientSettingsThenDefaulted() {
765+
ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
766+
.clientId(CLIENT_ID)
767+
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
768+
.redirectUri(REDIRECT_URI)
769+
.authorizationUri(AUTHORIZATION_URI)
770+
.tokenUri(TOKEN_URI)
771+
.build();
772+
773+
// should not be null
774+
assertThat(clientRegistration.getClientSettings()).isNotNull();
775+
// proof key should be false for passivity
776+
assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isFalse();
777+
}
778+
756779
}

0 commit comments

Comments
 (0)