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 6b3df65ac5..411cd36712 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 @@ -18,7 +18,6 @@ package org.springframework.security.oauth2.client.registration; import org.springframework.security.oauth2.core.AuthenticationMethod; 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; @@ -479,7 +478,8 @@ public final class ClientRegistration { providerDetails.jwkSetUri = this.jwkSetUri; clientRegistration.providerDetails = providerDetails; - clientRegistration.clientName = this.clientName; + clientRegistration.clientName = StringUtils.hasText(this.clientName) ? + this.clientName : this.registrationId; return clientRegistration; } @@ -489,15 +489,9 @@ 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.notNull(this.clientAuthenticationMethod, "clientAuthenticationMethod cannot be null"); Assert.hasText(this.redirectUriTemplate, "redirectUriTemplate cannot be empty"); Assert.hasText(this.authorizationUri, "authorizationUri cannot be empty"); Assert.hasText(this.tokenUri, "tokenUri cannot be empty"); - if (this.scopes != null && this.scopes.contains(OidcScopes.OPENID)) { - // OIDC Clients need to verify/validate the ID Token - Assert.hasText(this.jwkSetUri, "jwkSetUri cannot be empty"); - } - Assert.hasText(this.clientName, "clientName cannot be empty"); } private void validateImplicitGrantType() { @@ -507,7 +501,6 @@ public final class ClientRegistration { Assert.hasText(this.clientId, "clientId cannot be empty"); Assert.hasText(this.redirectUriTemplate, "redirectUriTemplate cannot be empty"); Assert.hasText(this.authorizationUri, "authorizationUri cannot be empty"); - Assert.hasText(this.clientName, "clientName cannot be empty"); } private void validateClientCredentialsGrantType() { @@ -515,7 +508,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.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 7c5758b4bd..438894fab5 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 @@ -142,21 +142,21 @@ public class ClientRegistrationTests { assertThat(clientRegistration.getClientSecret()).isEqualTo(""); } - @Test(expected = IllegalArgumentException.class) - public void buildWhenAuthorizationCodeGrantClientAuthenticationMethodIsNullThenThrowIllegalArgumentException() { - ClientRegistration.withRegistrationId(REGISTRATION_ID) - .clientId(CLIENT_ID) - .clientSecret(CLIENT_SECRET) - .clientAuthenticationMethod(null) - .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 buildWhenAuthorizationCodeGrantClientAuthenticationMethodNotProvidedThenDefaultToBasic() { + ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID) + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .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.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.BASIC); } @Test(expected = IllegalArgumentException.class) @@ -228,38 +228,21 @@ public class ClientRegistrationTests { .build(); } - @Test(expected = IllegalArgumentException.class) - public void buildWhenAuthorizationCodeGrantJwkSetUriIsNullThenThrowIllegalArgumentException() { - ClientRegistration.withRegistrationId(REGISTRATION_ID) - .clientId(CLIENT_ID) - .clientSecret(CLIENT_SECRET) - .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(null) - .clientName(CLIENT_NAME) - .build(); - } - - @Test(expected = IllegalArgumentException.class) - public void buildWhenAuthorizationCodeGrantClientNameIsNullThenThrowIllegalArgumentException() { - ClientRegistration.withRegistrationId(REGISTRATION_ID) - .clientId(CLIENT_ID) - .clientSecret(CLIENT_SECRET) - .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(null) - .build(); + @Test + public void buildWhenAuthorizationCodeGrantClientNameNotProvidedThenDefaultToRegistrationId() { + ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID) + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .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) + .build(); + assertThat(clientRegistration.getClientName()).isEqualTo(clientRegistration.getRegistrationId()); } @Test @@ -381,17 +364,17 @@ public class ClientRegistrationTests { .build(); } - @Test(expected = IllegalArgumentException.class) - public void buildWhenImplicitGrantClientNameIsNullThenThrowIllegalArgumentException() { - ClientRegistration.withRegistrationId(REGISTRATION_ID) - .clientId(CLIENT_ID) - .authorizationGrantType(AuthorizationGrantType.IMPLICIT) - .redirectUriTemplate(REDIRECT_URI) - .scope(SCOPES.toArray(new String[0])) - .authorizationUri(AUTHORIZATION_URI) - .userInfoAuthenticationMethod(AuthenticationMethod.FORM) - .clientName(null) - .build(); + @Test + public void buildWhenImplicitGrantClientNameNotProvidedThenDefaultToRegistrationId() { + ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID) + .clientId(CLIENT_ID) + .authorizationGrantType(AuthorizationGrantType.IMPLICIT) + .redirectUriTemplate(REDIRECT_URI) + .scope(SCOPES.toArray(new String[0])) + .authorizationUri(AUTHORIZATION_URI) + .userInfoAuthenticationMethod(AuthenticationMethod.FORM) + .build(); + assertThat(clientRegistration.getClientName()).isEqualTo(clientRegistration.getRegistrationId()); } @Test @@ -475,16 +458,14 @@ public class ClientRegistrationTests { } @Test - public void buildWhenClientCredentialsGrantClientAuthenticationMethodIsNullThenThrowIllegalArgumentException() { - assertThatThrownBy(() -> - ClientRegistration.withRegistrationId(REGISTRATION_ID) - .clientId(CLIENT_ID) - .clientSecret(CLIENT_SECRET) - .clientAuthenticationMethod(null) - .authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS) - .tokenUri(TOKEN_URI) - .build() - ).isInstanceOf(IllegalArgumentException.class); + public void buildWhenClientCredentialsGrantClientAuthenticationMethodNotProvidedThenDefaultToBasic() { + ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID) + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS) + .tokenUri(TOKEN_URI) + .build(); + assertThat(clientRegistration.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.BASIC); } @Test