diff --git a/server-spi/src/main/java/org/keycloak/models/ClientScopeModel.java b/server-spi/src/main/java/org/keycloak/models/ClientScopeModel.java index 22e542bad6..8424c1a3ee 100755 --- a/server-spi/src/main/java/org/keycloak/models/ClientScopeModel.java +++ b/server-spi/src/main/java/org/keycloak/models/ClientScopeModel.java @@ -112,7 +112,7 @@ public interface ClientScopeModel extends ProtocolMapperContainerModel, ScopeCon } default boolean isDynamicScope() { - return Optional.ofNullable(getAttribute(IS_DYNAMIC_SCOPE)).isPresent(); + return Boolean.parseBoolean(getAttribute(IS_DYNAMIC_SCOPE)); } default void setIsDynamicScope(boolean isDynamicScope) { diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java index 5b425a48b8..a38946deb0 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java @@ -348,6 +348,9 @@ public class ClientResource { if (clientScope == null) { throw new javax.ws.rs.NotFoundException("Client scope not found"); } + if (defaultScope && clientScope.isDynamicScope()) { + throw new ErrorResponseException("invalid_request", "Can't assign a Dynamic Scope to a Client as a Default Scope", Response.Status.BAD_REQUEST); + } client.addClientScope(clientScope, defaultScope); adminEvent.operation(OperationType.CREATE).resource(ResourceType.CLIENT_SCOPE_CLIENT_MAPPING).resourcePath(session.getContext().getUri()).success(); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientScopeResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientScopeResource.java index abf27dc9a2..187614e661 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientScopeResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientScopeResource.java @@ -20,7 +20,6 @@ import org.jboss.logging.Logger; import org.jboss.resteasy.annotations.cache.NoCache; import org.jboss.resteasy.spi.ResteasyProviderFactory; import org.keycloak.common.Profile; -import org.keycloak.events.Errors; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; import org.keycloak.models.ClientScopeModel; @@ -45,7 +44,6 @@ import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; -import java.util.Map; import java.util.Optional; import java.util.regex.Pattern; @@ -105,7 +103,7 @@ public class ClientScopeResource { @Consumes(MediaType.APPLICATION_JSON) public Response update(final ClientScopeRepresentation rep) { auth.clients().requireManageClientScopes(); - validateDynamicClientScope(rep); + validateDynamicScopeUpdate(rep); try { RepresentationToModel.updateClientScope(rep, clientScope); adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()).representation(rep).success(); @@ -137,7 +135,6 @@ public class ClientScopeResource { /** * Delete the client scope - * */ @DELETE @NoCache @@ -160,33 +157,56 @@ public class ClientScopeResource { * @throws ErrorResponseException */ public static void validateDynamicClientScope(ClientScopeRepresentation clientScope) throws ErrorResponseException { - if(clientScope.getAttributes() == null) { + if (clientScope.getAttributes() == null) { return; } boolean isDynamic = Boolean.parseBoolean(clientScope.getAttributes().get(ClientScopeModel.IS_DYNAMIC_SCOPE)); String regexp = clientScope.getAttributes().get(ClientScopeModel.DYNAMIC_SCOPE_REGEXP); - if(Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) { + if (Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) { // if the scope is dynamic but the regexp is empty, it's not considered valid - if(isDynamic && StringUtil.isNullOrEmpty(regexp)) { + if (isDynamic && StringUtil.isNullOrEmpty(regexp)) { throw new ErrorResponseException(ErrorResponse.error("Dynamic scope regexp must not be null or empty", Response.Status.BAD_REQUEST)); } // Always validate the dynamic scope regexp to avoid inserting a wrong value even when the feature is disabled - if(!StringUtil.isNullOrEmpty(regexp) && !dynamicScreenPattern.matcher(regexp).matches()) { + if (!StringUtil.isNullOrEmpty(regexp) && !dynamicScreenPattern.matcher(regexp).matches()) { throw new ErrorResponseException(ErrorResponse.error(String.format("Invalid format for the Dynamic Scope regexp %1s", regexp), Response.Status.BAD_REQUEST)); } } else { // if the value is not null or empty we won't accept the request as the feature is disabled Optional.ofNullable(regexp).ifPresent(s -> { - if(!s.isEmpty()) { + if (!s.isEmpty()) { throw new ErrorResponseException(ErrorResponse.error(String.format("Unexpected value \"%1s\" for attribute %2s in ClientScope", regexp, ClientScopeModel.DYNAMIC_SCOPE_REGEXP), Response.Status.BAD_REQUEST)); } }); // If isDynamic is true, we won't accept the request as the feature is disabled - if(isDynamic) { + if (isDynamic) { throw new ErrorResponseException(ErrorResponse.error(String.format("Unexpected value \"%1s\" for attribute %2s in ClientScope", isDynamic, ClientScopeModel.IS_DYNAMIC_SCOPE), Response.Status.BAD_REQUEST)); } } } + + /** + * Makes sure that an update that makes a Client Scope Dynamic is rejected if the Client Scope is assigned to a client + * as a default scope. + * @param rep the {@link ClientScopeRepresentation} with the changes from the frontend. + */ + public void validateDynamicScopeUpdate(ClientScopeRepresentation rep) { + // Only check this if the representation has been sent to make it dynamic + if (rep.getAttributes() != null && rep.getAttributes().getOrDefault(ClientScopeModel.IS_DYNAMIC_SCOPE, "false").equalsIgnoreCase("true")) { + Optional scopeModelOpt = realm.getClientsStream() + .flatMap(clientModel -> clientModel.getClientScopes(true).values().stream()) + .map(ClientScopeModel::getId) + .filter(scopeId -> scopeId.equalsIgnoreCase(this.clientScope.getId())) + .findAny(); + // if it's present, it means that a client has this scope assigned as a default scope, so this scope can't be made dynamic + if (scopeModelOpt.isPresent()) { + throw new ErrorResponseException(ErrorResponse.error("This Client Scope can't be made dynamic as it's assigned to a Client as a Default Scope", + Response.Status.BAD_REQUEST)); + } + } + // after the previous validation, run the usual Dynamic Scope validations. + validateDynamicClientScope(rep); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java index 8cbd43db1a..e7f7ad68b1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java @@ -17,6 +17,7 @@ package org.keycloak.testsuite.admin.client; +import org.hamcrest.MatcherAssert; import org.junit.Assert; import org.junit.Test; import org.keycloak.admin.client.resource.ClientResource; @@ -739,6 +740,65 @@ public class ClientScopeTest extends AbstractClientTest { handleExpectedCreateFailure(scopeRep, 400, "Invalid format for the Dynamic Scope regexp dynamic-scope-def:*:*"); } + @Test + @EnableFeature(value = Profile.Feature.DYNAMIC_SCOPES, skipRestart = true) + public void updateAssignedDefaultClientScopeToDynamicScope() { + ClientRepresentation clientRep = new ClientRepresentation(); + clientRep.setClientId("dyn-scope-client"); + clientRep.setProtocol("openid-connect"); + String clientUuid = createClient(clientRep); + getCleanup().addClientUuid(clientUuid); + + ClientScopeRepresentation scopeRep = new ClientScopeRepresentation(); + scopeRep.setName("dynamic-scope-def"); + scopeRep.setProtocol("openid-connect"); + String scopeDefId = createClientScope(scopeRep); + getCleanup().addClientScopeId(scopeDefId); + + testRealmResource().clients().get(clientUuid).addDefaultClientScope(scopeDefId); + + scopeRep.setAttributes(new HashMap() {{ + put(ClientScopeModel.IS_DYNAMIC_SCOPE, "true"); + put(ClientScopeModel.DYNAMIC_SCOPE_REGEXP, "dynamic-scope-def:*:*"); + }}); + + try { + clientScopes().get(scopeDefId).update(scopeRep); + Assert.fail("This update should fail"); + } catch (ClientErrorException ex) { + MatcherAssert.assertThat(ex.getResponse(), Matchers.statusCodeIs(Status.BAD_REQUEST)); + } + } + + @Test + @EnableFeature(value = Profile.Feature.DYNAMIC_SCOPES, skipRestart = true) + public void dynamicClientScopeCannotBeAssignedAsDefaultClientScope() { + ClientRepresentation clientRep = new ClientRepresentation(); + clientRep.setClientId("dyn-scope-client"); + clientRep.setProtocol("openid-connect"); + String clientUuid = createClient(clientRep); + getCleanup().addClientUuid(clientUuid); + + ClientScopeRepresentation optionalClientScope = new ClientScopeRepresentation(); + optionalClientScope.setName("optional-dynamic-client-scope"); + optionalClientScope.setProtocol("openid-connect"); + optionalClientScope.setAttributes(new HashMap() {{ + put(ClientScopeModel.IS_DYNAMIC_SCOPE, "true"); + put(ClientScopeModel.DYNAMIC_SCOPE_REGEXP, "dynamic-scope-def:*"); + }}); + String optionalClientScopeId = createClientScope(optionalClientScope); + getCleanup().addClientScopeId(optionalClientScopeId); + + try { + ClientResource clientResource = testRealmResource().clients().get(clientUuid); + clientResource.addDefaultClientScope(optionalClientScopeId); + Assert.fail("A Dynamic Scope shouldn't not be assigned as a default scope to a client"); + } catch (ClientErrorException ex) { + MatcherAssert.assertThat(ex.getResponse(), Matchers.statusCodeIs(Status.BAD_REQUEST)); + } + + } + private void handleExpectedCreateFailure(ClientScopeRepresentation scopeRep, int expectedErrorCode, String expectedErrorMessage) { try(Response resp = clientScopes().create(scopeRep)) { Assert.assertEquals(expectedErrorCode, resp.getStatus()); diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/client-scopes-setup.html b/themes/src/main/resources/theme/base/admin/resources/partials/client-scopes-setup.html index db39cf0ab4..50f10b6a4e 100644 --- a/themes/src/main/resources/theme/base/admin/resources/partials/client-scopes-setup.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/client-scopes-setup.html @@ -48,7 +48,7 @@