From 010d99a7d0349ff47dec3b1d247b6af72a195ed4 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Tue, 14 Aug 2018 13:32:51 -0400 Subject: [PATCH] Make ClientRegistration.clientSecret optional Fixes gh-5652 --- .../registration/ClientRegistration.java | 5 +- .../registration/ClientRegistrationTests.java | 50 +++++++++---------- 2 files changed, 27 insertions(+), 28 deletions(-) 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 f6342fc06a..6b3df65ac5 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 @@ -20,6 +20,7 @@ import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.ClientAuthenticationMethod; import org.springframework.security.oauth2.core.oidc.OidcScopes; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import java.util.Arrays; import java.util.Collection; @@ -463,7 +464,7 @@ public final class ClientRegistration { clientRegistration.registrationId = this.registrationId; clientRegistration.clientId = this.clientId; - clientRegistration.clientSecret = this.clientSecret; + clientRegistration.clientSecret = StringUtils.hasText(this.clientSecret) ? this.clientSecret : ""; clientRegistration.clientAuthenticationMethod = this.clientAuthenticationMethod; clientRegistration.authorizationGrantType = this.authorizationGrantType; clientRegistration.redirectUriTemplate = this.redirectUriTemplate; @@ -488,7 +489,6 @@ public final class ClientRegistration { () -> "authorizationGrantType must be " + AuthorizationGrantType.AUTHORIZATION_CODE.getValue()); Assert.hasText(this.registrationId, "registrationId cannot be empty"); Assert.hasText(this.clientId, "clientId cannot be empty"); - Assert.hasText(this.clientSecret, "clientSecret cannot be empty"); Assert.notNull(this.clientAuthenticationMethod, "clientAuthenticationMethod cannot be null"); Assert.hasText(this.redirectUriTemplate, "redirectUriTemplate cannot be empty"); Assert.hasText(this.authorizationUri, "authorizationUri cannot be empty"); @@ -515,7 +515,6 @@ public final class ClientRegistration { () -> "authorizationGrantType must be " + AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()); Assert.hasText(this.registrationId, "registrationId cannot be empty"); Assert.hasText(this.clientId, "clientId cannot be empty"); - Assert.hasText(this.clientSecret, "clientSecret cannot be empty"); Assert.notNull(this.clientAuthenticationMethod, "clientAuthenticationMethod cannot be null"); Assert.hasText(this.tokenUri, "tokenUri cannot be empty"); } 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 b1218d3295..7c5758b4bd 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 @@ -124,21 +124,22 @@ public class ClientRegistrationTests { .build(); } - @Test(expected = IllegalArgumentException.class) - public void buildWhenAuthorizationCodeGrantClientSecretIsNullThenThrowIllegalArgumentException() { - ClientRegistration.withRegistrationId(REGISTRATION_ID) - .clientId(CLIENT_ID) - .clientSecret(null) - .clientAuthenticationMethod(ClientAuthenticationMethod.BASIC) - .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) - .redirectUriTemplate(REDIRECT_URI) - .scope(SCOPES.toArray(new String[0])) - .authorizationUri(AUTHORIZATION_URI) - .tokenUri(TOKEN_URI) - .userInfoAuthenticationMethod(AuthenticationMethod.FORM) - .jwkSetUri(JWK_SET_URI) - .clientName(CLIENT_NAME) - .build(); + @Test + public void buildWhenAuthorizationCodeGrantClientSecretIsNullThenDefaultToEmpty() { + ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID) + .clientId(CLIENT_ID) + .clientSecret(null) + .clientAuthenticationMethod(ClientAuthenticationMethod.BASIC) + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .redirectUriTemplate(REDIRECT_URI) + .scope(SCOPES.toArray(new String[0])) + .authorizationUri(AUTHORIZATION_URI) + .tokenUri(TOKEN_URI) + .userInfoAuthenticationMethod(AuthenticationMethod.FORM) + .jwkSetUri(JWK_SET_URI) + .clientName(CLIENT_NAME) + .build(); + assertThat(clientRegistration.getClientSecret()).isEqualTo(""); } @Test(expected = IllegalArgumentException.class) @@ -462,16 +463,15 @@ public class ClientRegistrationTests { } @Test - public void buildWhenClientCredentialsGrantClientSecretIsNullThenThrowIllegalArgumentException() { - assertThatThrownBy(() -> - ClientRegistration.withRegistrationId(REGISTRATION_ID) - .clientId(CLIENT_ID) - .clientSecret(null) - .clientAuthenticationMethod(ClientAuthenticationMethod.BASIC) - .authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS) - .tokenUri(TOKEN_URI) - .build() - ).isInstanceOf(IllegalArgumentException.class); + public void buildWhenClientCredentialsGrantClientSecretIsNullThenDefaultToEmpty() { + ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID) + .clientId(CLIENT_ID) + .clientSecret(null) + .clientAuthenticationMethod(ClientAuthenticationMethod.BASIC) + .authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS) + .tokenUri(TOKEN_URI) + .build(); + assertThat(clientRegistration.getClientSecret()).isEqualTo(""); } @Test