[fixes #9750] Make sure a Dynamic scope isn't assignable to a client as a default scope, and only show non-dynamic scopes in the available client scopes client menu

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) {

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();

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<String> 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);
+    }
 }

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<String, String>() {{
+            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<String, String>() {{
+            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());

themes/src/main/resources/theme/base/admin/resources/partials/client-scopes-setup.html

@@ -48,7 +48,7 @@
                         <select id="available" class="form-control overflow-select" multiple size="5"
                                 ng-multiple="true"
                                 ng-model="selectedDefaultClientScopes">
-                            <option ng-repeat="r in availableClientScopes | orderBy:'name'"
+                            <option ng-repeat="r in availableClientScopes | orderBy:'name' | filter: {attributes: {'is.dynamic.scope': '!true'}}"
                                     value="{{r}}" title="{{r.name}}">
                                 {{r.name}}
                             </option>