diff --git a/openid-connect-common/src/main/java/org/mitre/oauth2/model/SystemScope.java b/openid-connect-common/src/main/java/org/mitre/oauth2/model/SystemScope.java index 59bc403c7..5901c6001 100644 --- a/openid-connect-common/src/main/java/org/mitre/oauth2/model/SystemScope.java +++ b/openid-connect-common/src/main/java/org/mitre/oauth2/model/SystemScope.java @@ -46,8 +46,8 @@ public class SystemScope { private String value; // scope value private String description; // human-readable description private String icon; // class of the icon to display on the auth page - private boolean allowDynReg = false; // can a dynamically registered client ask for this scope? private boolean defaultScope = false; // is this a default scope for newly-registered clients? + private boolean restricted = false; // is this scope restricted to admin-only registration access? private boolean structured = false; // is this a default scope for newly-registered clients? private String structuredParamDescription; private String structuredValue; @@ -124,20 +124,6 @@ public class SystemScope { public void setIcon(String icon) { this.icon = icon; } - /** - * @return the allowDynReg - */ - @Basic - @Column(name = "allow_dyn_reg") - public boolean isAllowDynReg() { - return allowDynReg; - } - /** - * @param allowDynReg the allowDynReg to set - */ - public void setAllowDynReg(boolean allowDynReg) { - this.allowDynReg = allowDynReg; - } /** * @return the defaultScope @@ -155,6 +141,22 @@ public class SystemScope { this.defaultScope = defaultScope; } + /** + * @return the restricted + */ + @Basic + @Column(name = "restricted") + public boolean isRestricted() { + return restricted; + } + + /** + * @param restricted the restricted to set + */ + public void setRestricted(boolean restricted) { + this.restricted = restricted; + } + /** * @return the isStructured status */ @@ -208,14 +210,19 @@ public class SystemScope { public int hashCode() { final int prime = 31; int result = 1; - result = prime * result + (allowDynReg ? 1231 : 1237); result = prime * result + (defaultScope ? 1231 : 1237); - result = prime * result + ((description == null) ? 0 : description.hashCode()); + result = prime * result + + ((description == null) ? 0 : description.hashCode()); result = prime * result + ((icon == null) ? 0 : icon.hashCode()); result = prime * result + ((id == null) ? 0 : id.hashCode()); + result = prime * result + (restricted ? 1231 : 1237); result = prime * result + (structured ? 1231 : 1237); - result = prime * result + ((structuredParamDescription == null) ? 0 : structuredParamDescription.hashCode()); - result = prime * result + ((structuredValue == null) ? 0 : structuredValue.hashCode()); + result = prime + * result + + ((structuredParamDescription == null) ? 0 + : structuredParamDescription.hashCode()); + result = prime * result + + ((structuredValue == null) ? 0 : structuredValue.hashCode()); result = prime * result + ((value == null) ? 0 : value.hashCode()); return result; } @@ -231,13 +238,10 @@ public class SystemScope { if (obj == null) { return false; } - if (!(obj instanceof SystemScope)) { + if (getClass() != obj.getClass()) { return false; } SystemScope other = (SystemScope) obj; - if (allowDynReg != other.allowDynReg) { - return false; - } if (defaultScope != other.defaultScope) { return false; } @@ -262,6 +266,9 @@ public class SystemScope { } else if (!id.equals(other.id)) { return false; } + if (restricted != other.restricted) { + return false; + } if (structured != other.structured) { return false; } @@ -269,7 +276,8 @@ public class SystemScope { if (other.structuredParamDescription != null) { return false; } - } else if (!structuredParamDescription.equals(other.structuredParamDescription)) { + } else if (!structuredParamDescription + .equals(other.structuredParamDescription)) { return false; } if (structuredValue == null) { @@ -294,7 +302,11 @@ public class SystemScope { */ @Override public String toString() { - return "SystemScope [id=" + id + ", value=" + value + ", description=" + description + ", icon=" + icon + ", allowDynReg=" + allowDynReg + ", defaultScope=" + defaultScope + ", structured=" + structured + ", structuredParamDescription=" + structuredParamDescription + ", structuredValue=" + return "SystemScope [id=" + id + ", value=" + value + ", description=" + + description + ", icon=" + icon + ", defaultScope=" + + defaultScope + ", restricted=" + restricted + ", structured=" + + structured + ", structuredParamDescription=" + + structuredParamDescription + ", structuredValue=" + structuredValue + "]"; } diff --git a/openid-connect-common/src/main/java/org/mitre/oauth2/service/SystemScopeService.java b/openid-connect-common/src/main/java/org/mitre/oauth2/service/SystemScopeService.java index a08467db7..4d2f35d30 100644 --- a/openid-connect-common/src/main/java/org/mitre/oauth2/service/SystemScopeService.java +++ b/openid-connect-common/src/main/java/org/mitre/oauth2/service/SystemScopeService.java @@ -23,6 +23,8 @@ import java.util.Set; import org.mitre.oauth2.model.SystemScope; +import com.google.common.collect.Sets; + /** * @author jricher * @@ -34,6 +36,13 @@ public interface SystemScopeService { public static final String ID_TOKEN_SCOPE = "id-token"; public static final String REGISTRATION_TOKEN_SCOPE = "registration-token"; public static final String RESOURCE_TOKEN_SCOPE = "resource-token"; + + public static final Set reservedScopes = + Sets.newHashSet( + new SystemScope(ID_TOKEN_SCOPE), + new SystemScope(REGISTRATION_TOKEN_SCOPE), + new SystemScope(RESOURCE_TOKEN_SCOPE) + ); public Set getAll(); @@ -42,13 +51,28 @@ public interface SystemScopeService { * @return */ public Set getDefaults(); - + /** - * Get all scopes that are allowed for dynamic registration on this system + * Get all the reserved system scopes. These can't be used + * by clients directly, but are instead tied to special system + * tokens like id tokens and registration access tokens. + * * @return */ - public Set getDynReg(); + public Set getReserved(); + + /** + * Get all the registered scopes that are restricted. + * @return + */ + public Set getRestricted(); + /** + * Get all the registered scopes that aren't restricted. + * @return + */ + public Set getUnrestricted(); + public SystemScope getById(Long id); public SystemScope getByValue(String value); @@ -82,9 +106,18 @@ public interface SystemScopeService { public boolean scopesMatch(Set expected, Set actual); /** - * Remove any system-restricted scopes from the set and return the result. + * Remove any system-reserved or registered restricted scopes from the + * set and return the result. * @param scopes * @return */ - public Set removeRestrictedScopes(Set scopes); + public Set removeRestrictedAndReservedScopes(Set scopes); + + /** + * Remove any system-reserved scopes from the set and return the result. + * @param scopes + * @return + */ + public Set removeReservedScopes(Set scopes); + } diff --git a/openid-connect-server-webapp/src/main/resources/db/scopes.sql b/openid-connect-server-webapp/src/main/resources/db/scopes.sql index 8b03afe2b..27792880f 100644 --- a/openid-connect-server-webapp/src/main/resources/db/scopes.sql +++ b/openid-connect-server-webapp/src/main/resources/db/scopes.sql @@ -10,23 +10,23 @@ START TRANSACTION; -- Insert scope information into the temporary tables. -- -INSERT INTO system_scope_TEMP (scope, description, icon, allow_dyn_reg, default_scope, structured, structured_param_description) VALUES - ('openid', 'log in using your identity', 'user', true, true, false, null), - ('profile', 'basic profile information', 'list-alt', true, true, false, null), - ('email', 'email address', 'envelope', true, true, false, null), - ('address', 'physical address', 'home', true, true, false, null), - ('phone', 'telephone number', 'bell', true, true, false, null), - ('offline_access', 'offline access', 'time', true, false, false, null); +INSERT INTO system_scope_TEMP (scope, description, icon, restricted, default_scope, structured, structured_param_description) VALUES + ('openid', 'log in using your identity', 'user', false, true, false, null), + ('profile', 'basic profile information', 'list-alt', false, true, false, null), + ('email', 'email address', 'envelope', false, true, false, null), + ('address', 'physical address', 'home', false, true, false, null), + ('phone', 'telephone number', 'bell', false, true, false, null), + ('offline_access', 'offline access', 'time', false, false, false, null); -- -- Merge the temporary scopes safely into the database. This is a two-step process to keep scopes from being created on every startup with a persistent store. -- MERGE INTO system_scope - USING (SELECT scope, description, icon, allow_dyn_reg, default_scope, structured, structured_param_description FROM system_scope_TEMP) AS vals(scope, description, icon, allow_dyn_reg, default_scope, structured, structured_param_description) + USING (SELECT scope, description, icon, restricted, default_scope, structured, structured_param_description FROM system_scope_TEMP) AS vals(scope, description, icon, restricted, default_scope, structured, structured_param_description) ON vals.scope = system_scope.scope WHEN NOT MATCHED THEN - INSERT (scope, description, icon, allow_dyn_reg, default_scope, structured, structured_param_description) VALUES(vals.scope, vals.description, vals.icon, vals.allow_dyn_reg, vals.default_scope, vals.structured, vals.structured_param_description); + INSERT (scope, description, icon, restricted, default_scope, structured, structured_param_description) VALUES(vals.scope, vals.description, vals.icon, vals.restricted, vals.default_scope, vals.structured, vals.structured_param_description); COMMIT; diff --git a/openid-connect-server-webapp/src/main/resources/db/tables/hsql_database_tables.sql b/openid-connect-server-webapp/src/main/resources/db/tables/hsql_database_tables.sql index c663f5021..ea932fec3 100644 --- a/openid-connect-server-webapp/src/main/resources/db/tables/hsql_database_tables.sql +++ b/openid-connect-server-webapp/src/main/resources/db/tables/hsql_database_tables.sql @@ -170,7 +170,7 @@ CREATE TABLE IF NOT EXISTS system_scope ( scope VARCHAR(256) NOT NULL, description VARCHAR(4096), icon VARCHAR(256), - allow_dyn_reg BOOLEAN DEFAULT false NOT NULL, + restricted BOOLEAN DEFAULT false NOT NULL, default_scope BOOLEAN DEFAULT false NOT NULL, structured BOOLEAN DEFAULT false NOT NULL, structured_param_description VARCHAR(256), diff --git a/openid-connect-server-webapp/src/main/resources/db/tables/loading_temp_tables.sql b/openid-connect-server-webapp/src/main/resources/db/tables/loading_temp_tables.sql index c1e746530..a2cfdeee3 100644 --- a/openid-connect-server-webapp/src/main/resources/db/tables/loading_temp_tables.sql +++ b/openid-connect-server-webapp/src/main/resources/db/tables/loading_temp_tables.sql @@ -69,7 +69,7 @@ CREATE TEMPORARY TABLE IF NOT EXISTS system_scope_TEMP ( scope VARCHAR(256), description VARCHAR(4096), icon VARCHAR(256), - allow_dyn_reg BOOLEAN, + restricted BOOLEAN, default_scope BOOLEAN, structured BOOLEAN, structured_param_description VARCHAR(256) diff --git a/openid-connect-server-webapp/src/main/resources/db/tables/mysql_database_tables.sql b/openid-connect-server-webapp/src/main/resources/db/tables/mysql_database_tables.sql index e6afeed19..4adddd772 100644 --- a/openid-connect-server-webapp/src/main/resources/db/tables/mysql_database_tables.sql +++ b/openid-connect-server-webapp/src/main/resources/db/tables/mysql_database_tables.sql @@ -170,7 +170,7 @@ CREATE TABLE IF NOT EXISTS system_scope ( scope VARCHAR(256) NOT NULL, description VARCHAR(4096), icon VARCHAR(256), - allow_dyn_reg BOOLEAN NOT NULL DEFAULT 0, + restricted BOOLEAN NOT NULL DEFAULT 0, default_scope BOOLEAN NOT NULL DEFAULT 0, structured BOOLEAN NOT NULL DEFAULT 0, structured_param_description VARCHAR(256), diff --git a/openid-connect-server/src/main/java/org/mitre/discovery/web/DiscoveryEndpoint.java b/openid-connect-server/src/main/java/org/mitre/discovery/web/DiscoveryEndpoint.java index 28c4c8cf4..f643a15b2 100644 --- a/openid-connect-server/src/main/java/org/mitre/discovery/web/DiscoveryEndpoint.java +++ b/openid-connect-server/src/main/java/org/mitre/discovery/web/DiscoveryEndpoint.java @@ -274,7 +274,7 @@ public class DiscoveryEndpoint { //end_session_endpoint m.put("jwks_uri", baseUrl + "jwk"); m.put("registration_endpoint", baseUrl + "register"); - m.put("scopes_supported", scopeService.toStrings(scopeService.getDynReg())); // these are the scopes that you can dynamically register for, which is what matters for discovery + m.put("scopes_supported", scopeService.toStrings(scopeService.getUnrestricted())); // these are the scopes that you can dynamically register for, which is what matters for discovery m.put("response_types_supported", Lists.newArrayList("code", "token")); // we don't support these yet: , "id_token", "id_token token")); m.put("grant_types_supported", Lists.newArrayList("authorization_code", "implicit", "urn:ietf:params:oauth:grant-type:jwt-bearer", "client_credentials", "urn:ietf:params:oauth:grant_type:redelegate")); //acr_values_supported @@ -311,6 +311,7 @@ public class DiscoveryEndpoint { "email", "email_verified", "phone_number", + "phone_number_verified", "address" )); m.put("service_documentation", baseUrl + "about"); diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ClientDetailsEntityService.java b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ClientDetailsEntityService.java index 8f59cd2bd..b17497de1 100644 --- a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ClientDetailsEntityService.java +++ b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ClientDetailsEntityService.java @@ -121,8 +121,7 @@ public class DefaultOAuth2ClientDetailsEntityService implements ClientDetailsEnt checkSectorIdentifierUri(client); - // make sure a client doesn't get any special system scopes - client.setScope(scopeService.removeRestrictedScopes(client.getScope())); + ensureNoReservedScopes(client); ClientDetailsEntity c = clientRepository.saveClient(client); @@ -131,6 +130,15 @@ public class DefaultOAuth2ClientDetailsEntityService implements ClientDetailsEnt return c; } + private void ensureNoReservedScopes(ClientDetailsEntity client) { + // make sure a client doesn't get any special system scopes + Set requestedScope = scopeService.fromStrings(client.getScope()); + + requestedScope = scopeService.removeReservedScopes(requestedScope); + + client.setScope(scopeService.toStrings(requestedScope)); + } + private void checkSectorIdentifierUri(ClientDetailsEntity client) { if (!Strings.isNullOrEmpty(client.getSectorIdentifierUri())) { try { @@ -246,7 +254,7 @@ public class DefaultOAuth2ClientDetailsEntityService implements ClientDetailsEnt checkSectorIdentifierUri(newClient); // make sure a client doesn't get any special system scopes - newClient.setScope(scopeService.removeRestrictedScopes(newClient.getScope())); + ensureNoReservedScopes(newClient); return clientRepository.updateClient(oldClient.getId(), newClient); } diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ProviderTokenService.java b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ProviderTokenService.java index 92e91dedf..af7e553ca 100644 --- a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ProviderTokenService.java +++ b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ProviderTokenService.java @@ -30,6 +30,7 @@ import org.mitre.oauth2.model.AuthenticationHolderEntity; import org.mitre.oauth2.model.ClientDetailsEntity; import org.mitre.oauth2.model.OAuth2AccessTokenEntity; import org.mitre.oauth2.model.OAuth2RefreshTokenEntity; +import org.mitre.oauth2.model.SystemScope; import org.mitre.oauth2.repository.AuthenticationHolderRepository; import org.mitre.oauth2.repository.OAuth2TokenRepository; import org.mitre.oauth2.service.ClientDetailsEntityService; @@ -144,10 +145,12 @@ public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityServi // inherit the scope from the auth, but make a new set so it is //not unmodifiable. Unmodifiables don't play nicely with Eclipselink, which //wants to use the clone operation. - Set scopes = Sets.newHashSet(clientAuth.getScope()); + Set scopes = scopeService.fromStrings(clientAuth.getScope()); + // remove any of the special system scopes - scopes = scopeService.removeRestrictedScopes(scopes); - token.setScope(scopes); + scopes = scopeService.removeRestrictedAndReservedScopes(scopes); + + token.setScope(scopeService.toStrings(scopes)); // make it expire if necessary if (client.getAccessTokenValiditySeconds() != null && client.getAccessTokenValiditySeconds() > 0) { @@ -263,19 +266,22 @@ public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityServi OAuth2AccessTokenEntity token = new OAuth2AccessTokenEntity(); // get the stored scopes from the authentication holder's authorization request; these are the scopes associated with the refresh token - Set refreshScopes = new HashSet(refreshToken.getAuthenticationHolder().getAuthentication().getOAuth2Request().getScope()); + Set refreshScopesRequested = new HashSet(refreshToken.getAuthenticationHolder().getAuthentication().getOAuth2Request().getScope()); + Set refreshScopes = scopeService.fromStrings(refreshScopesRequested); // remove any of the special system scopes - refreshScopes = scopeService.removeRestrictedScopes(refreshScopes); + refreshScopes = scopeService.removeRestrictedAndReservedScopes(refreshScopes); - Set scope = authRequest.getScope() == null ? new HashSet() : new HashSet(authRequest.getScope()); + Set scopeRequested = authRequest.getScope() == null ? new HashSet() : new HashSet(authRequest.getScope()); + Set scope = scopeService.fromStrings(scopeRequested); + // remove any of the special system scopes - scope = scopeService.removeRestrictedScopes(scope); + scope = scopeService.removeRestrictedAndReservedScopes(scope); if (scope != null && !scope.isEmpty()) { // ensure a proper subset of scopes if (refreshScopes != null && refreshScopes.containsAll(scope)) { // set the scope of the new access token if requested - token.setScope(scope); + token.setScope(scopeService.toStrings(scope)); } else { String errorMsg = "Up-scoping is not allowed."; logger.error(errorMsg); @@ -283,7 +289,7 @@ public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityServi } } else { // otherwise inherit the scope of the refresh token (if it's there -- this can return a null scope set) - token.setScope(refreshScopes); + token.setScope(scopeService.toStrings(refreshScopes)); } token.setClient(client); diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultSystemScopeService.java b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultSystemScopeService.java index 5452ec2db..ac30910c9 100644 --- a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultSystemScopeService.java +++ b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultSystemScopeService.java @@ -56,21 +56,17 @@ public class DefaultSystemScopeService implements SystemScopeService { } }; - - private Predicate isDynReg = new Predicate() { + private Predicate isRestricted = new Predicate() { @Override public boolean apply(SystemScope input) { - return (input != null && input.isAllowDynReg()); + return (input != null && input.isRestricted()); } }; - - private Predicate isRestricted = new Predicate() { + + private Predicate isReserved = new Predicate() { @Override - public boolean apply(String input) { - return (input != null && - !input.equals(ID_TOKEN_SCOPE) && - !input.equals(REGISTRATION_TOKEN_SCOPE) && - !input.equals(RESOURCE_TOKEN_SCOPE)); + public boolean apply(SystemScope input) { + return (input != null && getReserved().contains(input)); } }; @@ -124,22 +120,6 @@ public class DefaultSystemScopeService implements SystemScopeService { return repository.getAll(); } - /* (non-Javadoc) - * @see org.mitre.oauth2.service.SystemScopeService#getDefaults() - */ - @Override - public Set getDefaults() { - return Sets.filter(getAll(), isDefault); - } - - /* (non-Javadoc) - * @see org.mitre.oauth2.service.SystemScopeService#getDynReg() - */ - @Override - public Set getDynReg() { - return Sets.filter(getAll(), isDynReg); - } - /* (non-Javadoc) * @see org.mitre.oauth2.service.SystemScopeService#getById(java.lang.Long) */ @@ -170,7 +150,11 @@ public class DefaultSystemScopeService implements SystemScopeService { */ @Override public SystemScope save(SystemScope scope) { - return repository.save(scope); + if (!isReserved.apply(scope)) { // don't allow saving of reserved scopes + return repository.save(scope); + } else { + return null; + } } /* (non-Javadoc) @@ -241,10 +225,34 @@ public class DefaultSystemScopeService implements SystemScopeService { } @Override - public Set removeRestrictedScopes(Set scopes) { - return new LinkedHashSet(Collections2.filter(scopes, isRestricted)); + public Set getDefaults() { + return Sets.filter(getAll(), isDefault); } + @Override + public Set getReserved() { + return reservedScopes; + } + + @Override + public Set getRestricted() { + return Sets.filter(getAll(), isRestricted); + } + + @Override + public Set getUnrestricted() { + return Sets.filter(getAll(), Predicates.not(isRestricted)); + } + + @Override + public Set removeRestrictedAndReservedScopes(Set scopes) { + return Sets.filter(scopes, Predicates.not(Predicates.or(isRestricted, isReserved))); + } + + @Override + public Set removeReservedScopes(Set scopes) { + return Sets.filter(scopes, Predicates.not(isReserved)); + } } 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/DynamicClientRegistrationEndpoint.java similarity index 98% rename from openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientDynamicRegistrationEndpoint.java rename to openid-connect-server/src/main/java/org/mitre/openid/connect/web/DynamicClientRegistrationEndpoint.java index d41fcbd58..38b62293d 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/DynamicClientRegistrationEndpoint.java @@ -62,7 +62,7 @@ import com.google.gson.JsonSyntaxException; @Controller @RequestMapping(value = "register") -public class ClientDynamicRegistrationEndpoint { +public class DynamicClientRegistrationEndpoint { @Autowired private ClientDetailsEntityService clientService; @@ -85,7 +85,7 @@ public class ClientDynamicRegistrationEndpoint { @Autowired private OIDCTokenService connectTokenService; - private static Logger logger = LoggerFactory.getLogger(ClientDynamicRegistrationEndpoint.class); + private static Logger logger = LoggerFactory.getLogger(DynamicClientRegistrationEndpoint.class); /** * Create a new Client, issue a client ID, and create a registration access token. @@ -361,14 +361,11 @@ public class ClientDynamicRegistrationEndpoint { } 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); + Set allowedScopes = scopeService.removeRestrictedAndReservedScopes(requestedScopes); // if the client didn't ask for any, give them the defaults if (allowedScopes == null || allowedScopes.isEmpty()) { 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 e4725b3c6..e950a2387 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 @@ -201,18 +201,18 @@ public class ProtectedResourceRegistrationEndpoint { } private ClientDetailsEntity validateScopes(ClientDetailsEntity newClient) throws ValidationException { - - // note that protected resources can register for any scopes, even ones not used by the sysadmin - // scopes that the client is asking for Set requestedScopes = scopeService.fromStrings(newClient.getScope()); - // if the client didn't ask for any, give them the defaults - if (requestedScopes == null || requestedScopes.isEmpty()) { - requestedScopes = scopeService.getDefaults(); - } + // the scopes that the client can have must be a subset of the dynamically allowed scopes + Set allowedScopes = scopeService.removeRestrictedAndReservedScopes(requestedScopes); - newClient.setScope(scopeService.toStrings(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; } diff --git a/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ClientDetailsEntityService.java b/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ClientDetailsEntityService.java index b27929aad..97d4258f8 100644 --- a/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ClientDetailsEntityService.java +++ b/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ClientDetailsEntityService.java @@ -99,7 +99,7 @@ public class TestDefaultOAuth2ClientDetailsEntityService { } }); - Mockito.when(scopeService.removeRestrictedScopes(Matchers.anySet())).thenAnswer(new Answer>() { + Mockito.when(scopeService.removeRestrictedAndReservedScopes(Matchers.anySet())).thenAnswer(new Answer>() { @Override public Set answer(InvocationOnMock invocation) throws Throwable { Object[] args = invocation.getArguments(); diff --git a/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ProviderTokenService.java b/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ProviderTokenService.java index b3a2c776a..68ffe9c8c 100644 --- a/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ProviderTokenService.java +++ b/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ProviderTokenService.java @@ -144,7 +144,7 @@ public class TestDefaultOAuth2ProviderTokenService { Mockito.when(authenticationHolderRepository.save(Matchers.any(AuthenticationHolderEntity.class))).thenReturn(storedAuthHolder); - Mockito.when(scopeService.removeRestrictedScopes(Matchers.anySet())).then(AdditionalAnswers.returnsFirstArg()); + Mockito.when(scopeService.removeRestrictedAndReservedScopes(Matchers.anySet())).then(AdditionalAnswers.returnsFirstArg()); Mockito.when(tokenEnhancer.enhance(Matchers.any(OAuth2AccessTokenEntity.class), Matchers.any(OAuth2Authentication.class))) .thenAnswer(new Answer(){ diff --git a/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultSystemScopeService.java b/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultSystemScopeService.java index 784549e19..cd97a74ba 100644 --- a/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultSystemScopeService.java +++ b/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultSystemScopeService.java @@ -21,6 +21,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; import static org.junit.Assert.assertThat; +import java.util.HashSet; import java.util.Set; import org.junit.Before; @@ -50,7 +51,7 @@ public class TestDefaultSystemScopeService { private SystemScope defaultScope1; private SystemScope defaultScope2; private SystemScope dynScope1; - private SystemScope extraScope1; + private SystemScope restrictedScope1; private SystemScope structuredScope1; private SystemScope structuredScope1Value; @@ -59,12 +60,14 @@ public class TestDefaultSystemScopeService { private String defaultScope1String = "defaultScope1"; private String defaultScope2String = "defaultScope2"; private String dynScope1String = "dynScope1"; - private String extraScope1String = "extraScope1"; + private String restrictedScope1String = "restrictedScope1"; private String structuredScope1String = "structuredScope1"; private String structuredValue = "structuredValue"; private Set allScopes; private Set allScopeStrings; + private Set allScopesWithValue; + private Set allScopeStringsWithValue; @Mock private SystemScopeRepository repository; @@ -80,27 +83,28 @@ public class TestDefaultSystemScopeService { Mockito.reset(repository); - // two default and dynamically registerable scopes + // two default and dynamically registerable scopes (unrestricted) defaultDynScope1 = new SystemScope(defaultDynScope1String); defaultDynScope2 = new SystemScope(defaultDynScope2String); - defaultDynScope1.setAllowDynReg(true); - defaultDynScope2.setAllowDynReg(true); defaultDynScope1.setDefaultScope(true); defaultDynScope2.setDefaultScope(true); - // two strictly default scopes (isAllowDynReg false) + // two strictly default scopes (restricted) defaultScope1 = new SystemScope(defaultScope1String); defaultScope2 = new SystemScope(defaultScope2String); + defaultScope1.setRestricted(true); + defaultScope2.setRestricted(true); defaultScope1.setDefaultScope(true); defaultScope2.setDefaultScope(true); // one strictly dynamically registerable scope (isDefault false) dynScope1 = new SystemScope(dynScope1String); - dynScope1.setAllowDynReg(true); - // extraScope1 : extra scope that is neither (defaults to false/false) - extraScope1 = new SystemScope(extraScope1String); + // extraScope1 : extra scope that is neither restricted nor default (defaults to false/false) + restrictedScope1 = new SystemScope(restrictedScope1String); + restrictedScope1.setRestricted(true); + // structuredScope1 : structured scope structuredScope1 = new SystemScope(structuredScope1String); structuredScope1.setStructured(true); @@ -110,15 +114,18 @@ public class TestDefaultSystemScopeService { structuredScope1Value.setStructured(true); structuredScope1Value.setStructuredValue(structuredValue); - allScopes = Sets.newHashSet(defaultDynScope1, defaultDynScope2, defaultScope1, defaultScope2, dynScope1, extraScope1, structuredScope1, structuredScope1Value); - allScopeStrings = Sets.newHashSet(defaultDynScope1String, defaultDynScope2String, defaultScope1String, defaultScope2String, dynScope1String, extraScope1String, structuredScope1String, structuredScope1String + ":" + structuredValue); + allScopes = Sets.newHashSet(defaultDynScope1, defaultDynScope2, defaultScope1, defaultScope2, dynScope1, restrictedScope1, structuredScope1); + allScopeStrings = Sets.newHashSet(defaultDynScope1String, defaultDynScope2String, defaultScope1String, defaultScope2String, dynScope1String, restrictedScope1String, structuredScope1String); + + allScopesWithValue = Sets.newHashSet(defaultDynScope1, defaultDynScope2, defaultScope1, defaultScope2, dynScope1, restrictedScope1, structuredScope1, structuredScope1Value); + allScopeStringsWithValue = Sets.newHashSet(defaultDynScope1String, defaultDynScope2String, defaultScope1String, defaultScope2String, dynScope1String, restrictedScope1String, structuredScope1String, structuredScope1String + ":" + structuredValue); Mockito.when(repository.getByValue(defaultDynScope1String)).thenReturn(defaultDynScope1); Mockito.when(repository.getByValue(defaultDynScope2String)).thenReturn(defaultDynScope2); Mockito.when(repository.getByValue(defaultScope1String)).thenReturn(defaultScope1); Mockito.when(repository.getByValue(defaultScope2String)).thenReturn(defaultScope2); Mockito.when(repository.getByValue(dynScope1String)).thenReturn(dynScope1); - Mockito.when(repository.getByValue(extraScope1String)).thenReturn(extraScope1); + Mockito.when(repository.getByValue(restrictedScope1String)).thenReturn(restrictedScope1); // we re-use this value so we've got to use thenAnswer instead Mockito.when(repository.getByValue(structuredScope1String)).thenAnswer(new Answer() { @Override @@ -148,13 +155,21 @@ public class TestDefaultSystemScopeService { } @Test - public void getDynReg() { + public void getUnrestricted() { - Set dynReg = Sets.newHashSet(defaultDynScope1, defaultDynScope2, dynScope1); + Set unrestricted = Sets.newHashSet(defaultDynScope1, defaultDynScope2, dynScope1, structuredScope1); - assertThat(service.getDynReg(), equalTo(dynReg)); + assertThat(service.getUnrestricted(), equalTo(unrestricted)); } + @Test + public void getRestricted() { + Set restricted = Sets.newHashSet(defaultScope1, defaultScope2, restrictedScope1); + + assertThat(service.getRestricted(), equalTo(restricted)); + + } + @Test public void fromStrings() { @@ -162,6 +177,8 @@ public class TestDefaultSystemScopeService { assertThat(service.fromStrings(null), is(nullValue())); assertThat(service.fromStrings(allScopeStrings), equalTo(allScopes)); + + assertThat(service.fromStrings(allScopeStringsWithValue), equalTo(allScopesWithValue)); } @Test @@ -171,6 +188,8 @@ public class TestDefaultSystemScopeService { assertThat(service.toStrings(null), is(nullValue())); assertThat(service.toStrings(allScopes), equalTo(allScopeStrings)); + + assertThat(service.toStrings(allScopesWithValue), equalTo(allScopeStringsWithValue)); } @Test diff --git a/openid-connect-server/src/test/java/org/mitre/openid/connect/service/impl/TestMITREidDataService_1_0.java b/openid-connect-server/src/test/java/org/mitre/openid/connect/service/impl/TestMITREidDataService_1_0.java index 17611f681..7c8967bcf 100644 --- a/openid-connect-server/src/test/java/org/mitre/openid/connect/service/impl/TestMITREidDataService_1_0.java +++ b/openid-connect-server/src/test/java/org/mitre/openid/connect/service/impl/TestMITREidDataService_1_0.java @@ -80,6 +80,7 @@ import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonWriter; @RunWith(MockitoJUnitRunner.class) +@SuppressWarnings(value = {"rawtypes", "unchecked"}) public class TestMITREidDataService_1_0 { @Mock