Cwikius-Spring/spring-security
1
0
Fork 0
mirror of https://github.com/spring-projects/spring-security.git synced 2025-03-06 13:29:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

Relax validation on ClientRegistration

Browse Source 
Fixes gh-5667
This commit is contained in:
Joe Grandja 2018-08-14 14:05:45 -04:00
parent 010d99a7d0
commit cbdc7ee4b3
2 changed files with 51 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -18,7 +18,6 @@ package org.springframework.security.oauth2.client.registration;
import org.springframework.security.oauth2.core.AuthenticationMethod; import org.springframework.security.oauth2.core.AuthenticationMethod;
import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.AuthorizationGrantType;
import org.springframework.security.oauth2.core.ClientAuthenticationMethod; import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
import org.springframework.security.oauth2.core.oidc.OidcScopes;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
@ -479,7 +478,8 @@ public final class ClientRegistration {
providerDetails.jwkSetUri = this.jwkSetUri; providerDetails.jwkSetUri = this.jwkSetUri;
clientRegistration.providerDetails = providerDetails; clientRegistration.providerDetails = providerDetails;
clientRegistration.clientName = this.clientName; clientRegistration.clientName = StringUtils.hasText(this.clientName) ?
this.clientName : this.registrationId;
return clientRegistration; return clientRegistration;
} }
@ -489,15 +489,9 @@ public final class ClientRegistration {
() -> "authorizationGrantType must be " + AuthorizationGrantType.AUTHORIZATION_CODE.getValue()); () -> "authorizationGrantType must be " + AuthorizationGrantType.AUTHORIZATION_CODE.getValue());
Assert.hasText(this.registrationId, "registrationId cannot be empty"); Assert.hasText(this.registrationId, "registrationId cannot be empty");
Assert.hasText(this.clientId, "clientId 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.redirectUriTemplate, "redirectUriTemplate cannot be empty");
Assert.hasText(this.authorizationUri, "authorizationUri cannot be empty"); Assert.hasText(this.authorizationUri, "authorizationUri cannot be empty");
Assert.hasText(this.tokenUri, "tokenUri 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() { private void validateImplicitGrantType() {
@ -507,7 +501,6 @@ public final class ClientRegistration {
Assert.hasText(this.clientId, "clientId cannot be empty"); Assert.hasText(this.clientId, "clientId cannot be empty");
Assert.hasText(this.redirectUriTemplate, "redirectUriTemplate cannot be empty"); Assert.hasText(this.redirectUriTemplate, "redirectUriTemplate cannot be empty");
Assert.hasText(this.authorizationUri, "authorizationUri cannot be empty"); Assert.hasText(this.authorizationUri, "authorizationUri cannot be empty");
Assert.hasText(this.clientName, "clientName cannot be empty");
} }
private void validateClientCredentialsGrantType() { private void validateClientCredentialsGrantType() {
@ -515,7 +508,6 @@ public final class ClientRegistration {
() -> "authorizationGrantType must be " + AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()); () -> "authorizationGrantType must be " + AuthorizationGrantType.CLIENT_CREDENTIALS.getValue());
Assert.hasText(this.registrationId, "registrationId cannot be empty"); Assert.hasText(this.registrationId, "registrationId cannot be empty");
Assert.hasText(this.clientId, "clientId 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"); Assert.hasText(this.tokenUri, "tokenUri cannot be empty");
} }
} }

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

@ -142,12 +142,11 @@ public class ClientRegistrationTests {
assertThat(clientRegistration.getClientSecret()).isEqualTo(""); assertThat(clientRegistration.getClientSecret()).isEqualTo("");
} }
@Test(expected = IllegalArgumentException.class) @Test
public void buildWhenAuthorizationCodeGrantClientAuthenticationMethodIsNullThenThrowIllegalArgumentException() { public void buildWhenAuthorizationCodeGrantClientAuthenticationMethodNotProvidedThenDefaultToBasic() {
ClientRegistration.withRegistrationId(REGISTRATION_ID) ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
.clientId(CLIENT_ID) .clientId(CLIENT_ID)
.clientSecret(CLIENT_SECRET) .clientSecret(CLIENT_SECRET)
.clientAuthenticationMethod(null)
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
.redirectUriTemplate(REDIRECT_URI) .redirectUriTemplate(REDIRECT_URI)
.scope(SCOPES.toArray(new String[0])) .scope(SCOPES.toArray(new String[0]))
@ -157,6 +156,7 @@ public class ClientRegistrationTests {
.jwkSetUri(JWK_SET_URI) .jwkSetUri(JWK_SET_URI)
.clientName(CLIENT_NAME) .clientName(CLIENT_NAME)
.build(); .build();
assertThat(clientRegistration.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.BASIC);
} }
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
@ -228,26 +228,9 @@ public class ClientRegistrationTests {
.build(); .build();
} }
@Test(expected = IllegalArgumentException.class) @Test
public void buildWhenAuthorizationCodeGrantJwkSetUriIsNullThenThrowIllegalArgumentException() { public void buildWhenAuthorizationCodeGrantClientNameNotProvidedThenDefaultToRegistrationId() {
ClientRegistration.withRegistrationId(REGISTRATION_ID) 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(null)
.clientName(CLIENT_NAME)
.build();
}
@Test(expected = IllegalArgumentException.class)
public void buildWhenAuthorizationCodeGrantClientNameIsNullThenThrowIllegalArgumentException() {
ClientRegistration.withRegistrationId(REGISTRATION_ID)
.clientId(CLIENT_ID) .clientId(CLIENT_ID)
.clientSecret(CLIENT_SECRET) .clientSecret(CLIENT_SECRET)
.clientAuthenticationMethod(ClientAuthenticationMethod.BASIC) .clientAuthenticationMethod(ClientAuthenticationMethod.BASIC)
@ -258,8 +241,8 @@ public class ClientRegistrationTests {
.tokenUri(TOKEN_URI) .tokenUri(TOKEN_URI)
.userInfoAuthenticationMethod(AuthenticationMethod.FORM) .userInfoAuthenticationMethod(AuthenticationMethod.FORM)
.jwkSetUri(JWK_SET_URI) .jwkSetUri(JWK_SET_URI)
.clientName(null)
.build(); .build();
assertThat(clientRegistration.getClientName()).isEqualTo(clientRegistration.getRegistrationId());
} }
@Test @Test
@ -381,17 +364,17 @@ public class ClientRegistrationTests {
.build(); .build();
} }
@Test(expected = IllegalArgumentException.class) @Test
public void buildWhenImplicitGrantClientNameIsNullThenThrowIllegalArgumentException() { public void buildWhenImplicitGrantClientNameNotProvidedThenDefaultToRegistrationId() {
ClientRegistration.withRegistrationId(REGISTRATION_ID) ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
.clientId(CLIENT_ID) .clientId(CLIENT_ID)
.authorizationGrantType(AuthorizationGrantType.IMPLICIT) .authorizationGrantType(AuthorizationGrantType.IMPLICIT)
.redirectUriTemplate(REDIRECT_URI) .redirectUriTemplate(REDIRECT_URI)
.scope(SCOPES.toArray(new String[0])) .scope(SCOPES.toArray(new String[0]))
.authorizationUri(AUTHORIZATION_URI) .authorizationUri(AUTHORIZATION_URI)
.userInfoAuthenticationMethod(AuthenticationMethod.FORM) .userInfoAuthenticationMethod(AuthenticationMethod.FORM)
.clientName(null)
.build(); .build();
assertThat(clientRegistration.getClientName()).isEqualTo(clientRegistration.getRegistrationId());
} }
@Test @Test
@ -475,16 +458,14 @@ public class ClientRegistrationTests {
} }
@Test @Test
public void buildWhenClientCredentialsGrantClientAuthenticationMethodIsNullThenThrowIllegalArgumentException() { public void buildWhenClientCredentialsGrantClientAuthenticationMethodNotProvidedThenDefaultToBasic() {
assertThatThrownBy(() -> ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
ClientRegistration.withRegistrationId(REGISTRATION_ID)
.clientId(CLIENT_ID) .clientId(CLIENT_ID)
.clientSecret(CLIENT_SECRET) .clientSecret(CLIENT_SECRET)
.clientAuthenticationMethod(null)
.authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS) .authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS)
.tokenUri(TOKEN_URI) .tokenUri(TOKEN_URI)
.build() .build();
).isInstanceOf(IllegalArgumentException.class); assertThat(clientRegistration.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.BASIC);
} }
@Test @Test