From 52e53ba21997c2a2ddc2899db915d68589465139 Mon Sep 17 00:00:00 2001 From: Justin Richer Date: Fri, 6 Jun 2014 11:13:41 -0400 Subject: [PATCH] extracted validation exception, refactored protected resource registration endpoint to use this format --- .../exception/ValidationException.java | 47 +++++++ .../ClientDynamicRegistrationEndpoint.java | 38 +---- ...ProtectedResourceRegistrationEndpoint.java | 131 ++++++++++++------ 3 files changed, 135 insertions(+), 81 deletions(-) create mode 100644 openid-connect-server/src/main/java/org/mitre/openid/connect/exception/ValidationException.java diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/exception/ValidationException.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/exception/ValidationException.java new file mode 100644 index 000000000..3f121ff66 --- /dev/null +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/exception/ValidationException.java @@ -0,0 +1,47 @@ +package org.mitre.openid.connect.exception; + +import org.springframework.http.HttpStatus; + +/** + * Thrown by utility methods when a client fails to validate. Contains information + * to be returned. + * @author jricher + * + */ +public class ValidationException extends Exception { + private static final long serialVersionUID = 1820497072989294627L; + + private String error; + private String errorDescription; + private HttpStatus status; + public ValidationException(String error, String errorDescription, + HttpStatus status) { + this.error = error; + this.errorDescription = errorDescription; + this.status = status; + } + public String getError() { + return error; + } + public void setError(String error) { + this.error = error; + } + public String getErrorDescription() { + return errorDescription; + } + public void setErrorDescription(String errorDescription) { + this.errorDescription = errorDescription; + } + public HttpStatus getStatus() { + return status; + } + public void setStatus(HttpStatus status) { + this.status = status; + } + + @Override + public String toString() { + return "ValidationException [error=" + error + ", errorDescription=" + + errorDescription + ", status=" + status + "]"; + } +} \ No newline at end of file diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientDynamicRegistrationEndpoint.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientDynamicRegistrationEndpoint.java index 81a648e3b..4a5ff6c9f 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientDynamicRegistrationEndpoint.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientDynamicRegistrationEndpoint.java @@ -32,6 +32,7 @@ import org.mitre.oauth2.service.OAuth2TokenEntityService; import org.mitre.oauth2.service.SystemScopeService; import org.mitre.openid.connect.ClientDetailsEntityJsonProcessor; import org.mitre.openid.connect.config.ConfigurationPropertiesBean; +import org.mitre.openid.connect.exception.ValidationException; import org.mitre.openid.connect.service.BlacklistedSiteService; import org.mitre.openid.connect.service.OIDCTokenService; import org.slf4j.Logger; @@ -510,41 +511,4 @@ public class ClientDynamicRegistrationEndpoint { return newClient; } - /** - * Thrown by utility methods when a client fails to validate. Contains information - * to be returned. - * @author jricher - * - */ - private class ValidationException extends Exception { - private String error; - private String errorDescription; - private HttpStatus status; - public ValidationException(String error, String errorDescription, - HttpStatus status) { - this.error = error; - this.errorDescription = errorDescription; - this.status = status; - } - public String getError() { - return error; - } - public void setError(String error) { - this.error = error; - } - public String getErrorDescription() { - return errorDescription; - } - public void setErrorDescription(String errorDescription) { - this.errorDescription = errorDescription; - } - public HttpStatus getStatus() { - return status; - } - public void setStatus(HttpStatus status) { - this.status = status; - } - - } - } diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ProtectedResourceRegistrationEndpoint.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ProtectedResourceRegistrationEndpoint.java index 4cc056cfc..f99b0a37f 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ProtectedResourceRegistrationEndpoint.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ProtectedResourceRegistrationEndpoint.java @@ -31,6 +31,7 @@ import org.mitre.oauth2.service.OAuth2TokenEntityService; import org.mitre.oauth2.service.SystemScopeService; import org.mitre.openid.connect.ClientDetailsEntityJsonProcessor; import org.mitre.openid.connect.config.ConfigurationPropertiesBean; +import org.mitre.openid.connect.exception.ValidationException; import org.mitre.openid.connect.service.BlacklistedSiteService; import org.mitre.openid.connect.service.OIDCTokenService; import org.slf4j.Logger; @@ -110,40 +111,24 @@ public class ProtectedResourceRegistrationEndpoint { newClient.setClientId(null); newClient.setClientSecret(null); - // set of scopes that are OK for clients to dynamically register for - Set dynScopes = scopeService.getDynReg(); - - // scopes that the client is asking for - Set requestedScopes = scopeService.fromStrings(newClient.getScope()); - - // the scopes that the client can have must be a subset of the dynamically allowed scopes - Set allowedScopes = Sets.intersection(dynScopes, requestedScopes); - - // if the client didn't ask for any, give them the defaults - if (allowedScopes == null || allowedScopes.isEmpty()) { - allowedScopes = scopeService.getDefaults(); + // do validation on the fields + try { + newClient = validateScopes(newClient); + newClient = validateAuth(newClient); + } catch (ValidationException ve) { + // validation failed, return an error + m.addAttribute("error", ve.getError()); + m.addAttribute("errorMessage", ve.getErrorDescription()); + m.addAttribute("code", ve.getStatus()); + return "jsonErrorView"; } - newClient.setScope(scopeService.toStrings(allowedScopes)); - // no grant types are allowed newClient.setGrantTypes(new HashSet()); newClient.setResponseTypes(new HashSet()); newClient.setRedirectUris(new HashSet()); - if (newClient.getTokenEndpointAuthMethod() == null) { - newClient.setTokenEndpointAuthMethod(AuthMethod.SECRET_BASIC); - } - - if (newClient.getTokenEndpointAuthMethod() == AuthMethod.SECRET_BASIC || - newClient.getTokenEndpointAuthMethod() == AuthMethod.SECRET_JWT || - newClient.getTokenEndpointAuthMethod() == AuthMethod.SECRET_POST) { - - // we need to generate a secret - newClient = clientService.generateClientSecret(newClient); - } - // don't issue tokens to this client newClient.setAccessTokenValiditySeconds(0); newClient.setIdTokenValiditySeconds(0); @@ -207,6 +192,26 @@ public class ProtectedResourceRegistrationEndpoint { } + private ClientDetailsEntity validateScopes(ClientDetailsEntity newClient) throws ValidationException { + // set of scopes that are OK for clients to dynamically register for + Set dynScopes = scopeService.getDynReg(); + + // scopes that the client is asking for + Set requestedScopes = scopeService.fromStrings(newClient.getScope()); + + // the scopes that the client can have must be a subset of the dynamically allowed scopes + Set allowedScopes = Sets.intersection(dynScopes, requestedScopes); + + // if the client didn't ask for any, give them the defaults + if (allowedScopes == null || allowedScopes.isEmpty()) { + allowedScopes = scopeService.getDefaults(); + } + + newClient.setScope(scopeService.toStrings(allowedScopes)); + + return newClient; + } + /** * Get the meta information for a client. * @param clientId @@ -284,28 +289,51 @@ public class ProtectedResourceRegistrationEndpoint { // a client can't ask to update its own client secret to any particular value newClient.setClientSecret(oldClient.getClientSecret()); - // we need to copy over all of the local and SECOAUTH fields - newClient.setAccessTokenValiditySeconds(oldClient.getAccessTokenValiditySeconds()); - newClient.setIdTokenValiditySeconds(oldClient.getIdTokenValiditySeconds()); - newClient.setRefreshTokenValiditySeconds(oldClient.getRefreshTokenValiditySeconds()); - newClient.setDynamicallyRegistered(true); // it's still dynamically registered - newClient.setAllowIntrospection(oldClient.isAllowIntrospection()); - newClient.setAuthorities(oldClient.getAuthorities()); - newClient.setClientDescription(oldClient.getClientDescription()); - newClient.setCreatedAt(oldClient.getCreatedAt()); - newClient.setReuseRefreshToken(oldClient.isReuseRefreshToken()); + // no grant types are allowed + newClient.setGrantTypes(new HashSet()); + newClient.setResponseTypes(new HashSet()); + newClient.setRedirectUris(new HashSet()); - // set of scopes that are OK for clients to dynamically register for - Set dynScopes = scopeService.getDynReg(); + // don't issue tokens to this client + newClient.setAccessTokenValiditySeconds(0); + newClient.setIdTokenValiditySeconds(0); + newClient.setRefreshTokenValiditySeconds(0); - // scopes that the client is asking for - Set requestedScopes = scopeService.fromStrings(newClient.getScope()); + // clear out unused fields + newClient.setDefaultACRvalues(new HashSet()); + newClient.setDefaultMaxAge(null); + newClient.setIdTokenEncryptedResponseAlg(null); + newClient.setIdTokenEncryptedResponseEnc(null); + newClient.setIdTokenSignedResponseAlg(null); + newClient.setInitiateLoginUri(null); + newClient.setPostLogoutRedirectUri(null); + newClient.setRequestObjectSigningAlg(null); + newClient.setRequireAuthTime(null); + newClient.setReuseRefreshToken(false); + newClient.setSectorIdentifierUri(null); + newClient.setSubjectType(null); + newClient.setUserInfoEncryptedResponseAlg(null); + newClient.setUserInfoEncryptedResponseEnc(null); + newClient.setUserInfoSignedResponseAlg(null); - // the scopes that the client can have must be a subset of the dynamically allowed scopes - Set allowedScopes = Sets.intersection(dynScopes, requestedScopes); + // this client has been dynamically registered (obviously) + newClient.setDynamicallyRegistered(true); + + // this client has access to the introspection endpoint + newClient.setAllowIntrospection(true); + + // do validation on the fields + try { + newClient = validateScopes(newClient); + newClient = validateAuth(newClient); + } catch (ValidationException ve) { + // validation failed, return an error + m.addAttribute("error", ve.getError()); + m.addAttribute("errorMessage", ve.getErrorDescription()); + m.addAttribute("code", ve.getStatus()); + return "jsonErrorView"; + } - // make sure that the client doesn't ask for scopes it can't have - newClient.setScope(scopeService.toStrings(allowedScopes)); try { // save the client @@ -374,4 +402,19 @@ public class ProtectedResourceRegistrationEndpoint { } } + private ClientDetailsEntity validateAuth(ClientDetailsEntity newClient) throws ValidationException { + if (newClient.getTokenEndpointAuthMethod() == null) { + newClient.setTokenEndpointAuthMethod(AuthMethod.SECRET_BASIC); + } + + if (newClient.getTokenEndpointAuthMethod() == AuthMethod.SECRET_BASIC || + newClient.getTokenEndpointAuthMethod() == AuthMethod.SECRET_JWT || + newClient.getTokenEndpointAuthMethod() == AuthMethod.SECRET_POST) { + + // we need to generate a secret + newClient = clientService.generateClientSecret(newClient); + } + return newClient; + } + }