From d4dac21ca585ca13d4309eb4341a2115fcc41b19 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Thu, 19 Oct 2017 14:15:59 -0400 Subject: [PATCH] Make ClientRegistration.Builder constructor private Fixes gh-4656 --- .../oauth2/client/CommonOAuth2Provider.java | 2 +- .../registration/ClientRegistration.java | 116 +++++------------- ...AuthorizationCodeAuthenticationFilter.java | 2 +- .../security/oauth2/client/web/TestUtil.java | 4 +- 4 files changed, 37 insertions(+), 87 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/oauth2/client/CommonOAuth2Provider.java b/config/src/main/java/org/springframework/security/config/oauth2/client/CommonOAuth2Provider.java index 9d823735f2..8d6fc308f6 100644 --- a/config/src/main/java/org/springframework/security/config/oauth2/client/CommonOAuth2Provider.java +++ b/config/src/main/java/org/springframework/security/config/oauth2/client/CommonOAuth2Provider.java @@ -97,7 +97,7 @@ public enum CommonOAuth2Provider { protected final ClientRegistration.Builder getBuilder(String registrationId, ClientAuthenticationMethod method, String redirectUri) { - ClientRegistration.Builder builder = new ClientRegistration.Builder(registrationId); + ClientRegistration.Builder builder = ClientRegistration.withRegistrationId(registrationId); builder.clientAuthenticationMethod(method); builder.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE); builder.redirectUri(redirectUri); 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 ca4957001d..1f03281723 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 @@ -33,7 +33,7 @@ import java.util.Set; * @since 5.0 * @see Section 2 Client Registration */ -public class ClientRegistration { +public final class ClientRegistration { private String registrationId; private String clientId; private String clientSecret; @@ -44,147 +44,97 @@ public class ClientRegistration { private ProviderDetails providerDetails = new ProviderDetails(); private String clientName; - protected ClientRegistration() { + private ClientRegistration() { } public String getRegistrationId() { return this.registrationId; } - protected void setRegistrationId(String registrationId) { - this.registrationId = registrationId; - } - public String getClientId() { return this.clientId; } - protected void setClientId(String clientId) { - this.clientId = clientId; - } - public String getClientSecret() { return this.clientSecret; } - protected void setClientSecret(String clientSecret) { - this.clientSecret = clientSecret; - } - public ClientAuthenticationMethod getClientAuthenticationMethod() { return this.clientAuthenticationMethod; } - protected void setClientAuthenticationMethod(ClientAuthenticationMethod clientAuthenticationMethod) { - this.clientAuthenticationMethod = clientAuthenticationMethod; - } - public AuthorizationGrantType getAuthorizationGrantType() { return this.authorizationGrantType; } - protected void setAuthorizationGrantType(AuthorizationGrantType authorizationGrantType) { - this.authorizationGrantType = authorizationGrantType; - } - public String getRedirectUri() { return this.redirectUri; } - protected void setRedirectUri(String redirectUri) { - this.redirectUri = redirectUri; - } - public Set getScopes() { return this.scopes; } - protected void setScopes(Set scopes) { - this.scopes = scopes; - } - public ProviderDetails getProviderDetails() { return this.providerDetails; } - protected void setProviderDetails(ProviderDetails providerDetails) { - this.providerDetails = providerDetails; - } - public String getClientName() { return this.clientName; } - protected void setClientName(String clientName) { - this.clientName = clientName; - } - public class ProviderDetails { private String authorizationUri; private String tokenUri; private UserInfoEndpoint userInfoEndpoint = new UserInfoEndpoint(); private String jwkSetUri; - protected ProviderDetails() { + private ProviderDetails() { } public String getAuthorizationUri() { return this.authorizationUri; } - protected void setAuthorizationUri(String authorizationUri) { - this.authorizationUri = authorizationUri; - } - public String getTokenUri() { return this.tokenUri; } - protected void setTokenUri(String tokenUri) { - this.tokenUri = tokenUri; - } - public UserInfoEndpoint getUserInfoEndpoint() { return this.userInfoEndpoint; } - protected void setUserInfoEndpoint(UserInfoEndpoint userInfoEndpoint) { - this.userInfoEndpoint = userInfoEndpoint; - } - public String getJwkSetUri() { return this.jwkSetUri; } - protected void setJwkSetUri(String jwkSetUri) { - this.jwkSetUri = jwkSetUri; - } - public class UserInfoEndpoint { private String uri; private String userNameAttributeName; - protected UserInfoEndpoint() { + private UserInfoEndpoint() { } public String getUri() { return this.uri; } - protected void setUri(String uri) { - this.uri = uri; - } - public String getUserNameAttributeName() { return this.userNameAttributeName; } - - protected void setUserNameAttributeName(String userNameAttributeName) { - this.userNameAttributeName = userNameAttributeName; - } } } + public static Builder withRegistrationId(String registrationId) { + Assert.hasText(registrationId, "registrationId cannot be empty"); + return new Builder(registrationId); + } + + public static Builder from(ClientRegistration clientRegistration) { + Assert.notNull(clientRegistration, "clientRegistration cannot be null"); + return new Builder(clientRegistration); + } + public static class Builder { private String registrationId; private String clientId; @@ -200,11 +150,11 @@ public class ClientRegistration { private String jwkSetUri; private String clientName; - public Builder(String registrationId) { + private Builder(String registrationId) { this.registrationId = registrationId; } - public Builder(ClientRegistration clientRegistration) { + private Builder(ClientRegistration clientRegistration) { this(clientRegistration.getRegistrationId()); this.clientId(clientRegistration.getClientId()); this.clientSecret(clientRegistration.getClientSecret()); @@ -295,31 +245,31 @@ public class ClientRegistration { return this.create(); } - protected ClientRegistration create() { + private ClientRegistration create() { ClientRegistration clientRegistration = new ClientRegistration(); - clientRegistration.setRegistrationId(this.registrationId); - clientRegistration.setClientId(this.clientId); - clientRegistration.setClientSecret(this.clientSecret); - clientRegistration.setClientAuthenticationMethod(this.clientAuthenticationMethod); - clientRegistration.setAuthorizationGrantType(this.authorizationGrantType); - clientRegistration.setRedirectUri(this.redirectUri); - clientRegistration.setScopes(this.scopes); + clientRegistration.registrationId = this.registrationId; + clientRegistration.clientId = this.clientId; + clientRegistration.clientSecret = this.clientSecret; + clientRegistration.clientAuthenticationMethod = this.clientAuthenticationMethod; + clientRegistration.authorizationGrantType = this.authorizationGrantType; + clientRegistration.redirectUri = this.redirectUri; + clientRegistration.scopes = this.scopes; ProviderDetails providerDetails = clientRegistration.new ProviderDetails(); - providerDetails.setAuthorizationUri(this.authorizationUri); - providerDetails.setTokenUri(this.tokenUri); - providerDetails.getUserInfoEndpoint().setUri(this.userInfoUri); - providerDetails.getUserInfoEndpoint().setUserNameAttributeName(this.userNameAttributeName); - providerDetails.setJwkSetUri(this.jwkSetUri); - clientRegistration.setProviderDetails(providerDetails); + providerDetails.authorizationUri = this.authorizationUri; + providerDetails.tokenUri = this.tokenUri; + providerDetails.userInfoEndpoint.uri = this.userInfoUri; + providerDetails.userInfoEndpoint.userNameAttributeName = this.userNameAttributeName; + providerDetails.jwkSetUri = this.jwkSetUri; + clientRegistration.providerDetails = providerDetails; - clientRegistration.setClientName(this.clientName); + clientRegistration.clientName = this.clientName; return clientRegistration; } - protected void validateAuthorizationCodeGrantType() { + private void validateAuthorizationCodeGrantType() { Assert.isTrue(AuthorizationGrantType.AUTHORIZATION_CODE.equals(this.authorizationGrantType), "authorizationGrantType must be " + AuthorizationGrantType.AUTHORIZATION_CODE.getValue()); Assert.hasText(this.registrationId, "registrationId cannot be empty"); @@ -337,7 +287,7 @@ public class ClientRegistration { Assert.hasText(this.clientName, "clientName cannot be empty"); } - protected void validateImplicitGrantType() { + private void validateImplicitGrantType() { Assert.isTrue(AuthorizationGrantType.IMPLICIT.equals(this.authorizationGrantType), "authorizationGrantType must be " + AuthorizationGrantType.IMPLICIT.getValue()); Assert.hasText(this.registrationId, "registrationId cannot be empty"); diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java index d840ad0d7f..5504702a8d 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java @@ -117,7 +117,7 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio // MUST BE the same one used to complete the authorization code flow. // Therefore, we'll create a copy of the clientRegistration and override the redirectUri // with the one contained in authorizationRequest. - clientRegistration = new ClientRegistration.Builder(clientRegistration) + clientRegistration = ClientRegistration.from(clientRegistration) .redirectUri(authorizationRequest.getRedirectUri()) .build(); diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/TestUtil.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/TestUtil.java index b1c993b717..76b7f37b4e 100644 --- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/TestUtil.java +++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/TestUtil.java @@ -44,7 +44,7 @@ class TestUtil { } static ClientRegistration googleClientRegistration(String redirectUri) { - return new ClientRegistration.Builder(GOOGLE_REGISTRATION_ID) + return ClientRegistration.withRegistrationId(GOOGLE_REGISTRATION_ID) .clientId("google-client-id") .clientSecret("secret") .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) @@ -63,7 +63,7 @@ class TestUtil { } static ClientRegistration githubClientRegistration(String redirectUri) { - return new ClientRegistration.Builder(GITHUB_REGISTRATION_ID) + return ClientRegistration.withRegistrationId(GITHUB_REGISTRATION_ID) .clientId("github-client-id") .clientSecret("secret") .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)