Update client scopes in Client Update Request in DCR

Fix ClientScopesClientRegistrationPolicy.beforeUpdate because it was modifying the original clientRepresentation.
Add updateClientScopes method to set client scopes in Client Update Request in DCR.

Closes #24361

Signed-off-by: graziang <g.graziano94@gmail.com>

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java

@@ -464,28 +464,7 @@ public class RepresentationToModel {
             addClientScopeToClient(realm, client, clientTemplateName, true);
         }
 
-        if (resourceRep.getDefaultClientScopes() != null || resourceRep.getOptionalClientScopes() != null) {
+        updateClientScopes(resourceRep, client);
-            // First remove all default/built in client scopes
-            for (ClientScopeModel clientScope : client.getClientScopes(true).values()) {
-                client.removeClientScope(clientScope);
-            }
-
-            // First remove all default/built in client scopes
-            for (ClientScopeModel clientScope : client.getClientScopes(false).values()) {
-                client.removeClientScope(clientScope);
-            }
-        }
-
-        if (resourceRep.getDefaultClientScopes() != null) {
-            for (String clientScopeName : resourceRep.getDefaultClientScopes()) {
-                addClientScopeToClient(realm, client, clientScopeName, true);
-            }
-        }
-        if (resourceRep.getOptionalClientScopes() != null) {
-            for (String clientScopeName : resourceRep.getOptionalClientScopes()) {
-                addClientScopeToClient(realm, client, clientScopeName, false);
-            }
-        }
 
         if (resourceRep.isFullScopeAllowed() != null) {
             client.setFullScopeAllowed(resourceRep.isFullScopeAllowed());
@@ -656,6 +635,31 @@ public class RepresentationToModel {
             }
         }
     }
+    
+    public static void updateClientScopes(ClientRepresentation resourceRep, ClientModel client) {
+        if (resourceRep.getDefaultClientScopes() != null || resourceRep.getOptionalClientScopes() != null) {
+            // First remove all default/built in client scopes
+            for (ClientScopeModel clientScope : client.getClientScopes(true).values()) {
+                client.removeClientScope(clientScope);
+            }
+
+            // First remove all default/built in client scopes
+            for (ClientScopeModel clientScope : client.getClientScopes(false).values()) {
+                client.removeClientScope(clientScope);
+            }
+        }
+
+        if (resourceRep.getDefaultClientScopes() != null) {
+            for (String clientScopeName : resourceRep.getDefaultClientScopes()) {
+                addClientScopeToClient(client.getRealm(), client, clientScopeName, true);
+            }
+        }
+        if (resourceRep.getOptionalClientScopes() != null) {
+            for (String clientScopeName : resourceRep.getOptionalClientScopes()) {
+                addClientScopeToClient(client.getRealm(), client, clientScopeName, false);
+            }
+        }
+    }
 
     private static String generateProtocolNameKey(String protocol, String name) {
         return String.format("%s%%%s", protocol, name);

services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java

@@ -161,6 +161,7 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist
 
         RepresentationToModel.updateClient(rep, client, session);
         RepresentationToModel.updateClientProtocolMappers(rep, client);
+        RepresentationToModel.updateClientScopes(rep, client);
 
         if (rep.getDefaultRoles() != null) {
             updateDefaultRoles(client, rep.getDefaultRoles());

services/src/main/java/org/keycloak/services/clientregistration/policy/impl/ClientScopesClientRegistrationPolicy.java

@@ -68,16 +68,19 @@ public class ClientScopesClientRegistrationPolicy implements ClientRegistrationP
 
     @Override
     public void beforeUpdate(ClientRegistrationContext context, ClientModel clientModel) throws ClientRegistrationPolicyException {
-        List<String> requestedDefaultScopeNames = context.getClient().getDefaultClientScopes();
+        List<String> requestedDefaultScopeNames = new LinkedList<>();
-        List<String> requestedOptionalScopeNames = context.getClient().getOptionalClientScopes();
+        List<String> requestedOptionalScopeNames = new LinkedList<>();
+
+        if(context.getClient().getDefaultClientScopes() != null) {
+            requestedDefaultScopeNames.addAll(context.getClient().getDefaultClientScopes());
+        }
+        if(context.getClient().getOptionalClientScopes() != null) {
+            requestedOptionalScopeNames.addAll(context.getClient().getOptionalClientScopes());
+        }
 
         // Allow scopes, which were already presented before
-        if (requestedDefaultScopeNames != null) {
+        requestedDefaultScopeNames.removeAll(clientModel.getClientScopes(true).keySet());
-            requestedDefaultScopeNames.removeAll(clientModel.getClientScopes(true).keySet());
+        requestedOptionalScopeNames.removeAll(clientModel.getClientScopes(false).keySet());
-        }
-        if (requestedOptionalScopeNames != null) {
-            requestedOptionalScopeNames.removeAll(clientModel.getClientScopes(false).keySet());
-        }
 
         List<String> allowedDefaultScopeNames = getAllowedScopeNames(realm, true);
         List<String> allowedOptionalScopeNames = getAllowedScopeNames(realm, false);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java

@@ -242,6 +242,35 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest {
         assertThat(updatedClient.getDefaultRoles(), Matchers.arrayContainingInAnyOrder("test-default-role1","test-default-role2"));
     }
 
+    @Test
+    public void updateClientScopes() throws ClientRegistrationException {
+        authManageClients();
+        ClientRepresentation client = buildClient();
+        ArrayList<String> optionalClientScopes = new ArrayList<>(List.of("address"));
+        client.setOptionalClientScopes(optionalClientScopes);
+
+        ClientRepresentation createdClient = registerClient(client);
+        Set<String> requestedClientScopes = new HashSet<>(optionalClientScopes);
+        Set<String> registeredClientScopes = new HashSet<>(createdClient.getOptionalClientScopes());
+        assertEquals(requestedClientScopes, registeredClientScopes);
+        assertTrue(createdClient.getDefaultClientScopes().isEmpty());
+
+        authManageClients();
+        ClientRepresentation obtainedClient = reg.get(CLIENT_ID);
+        registeredClientScopes = new HashSet<>(obtainedClient.getOptionalClientScopes());
+        assertEquals(requestedClientScopes, registeredClientScopes);
+        assertTrue(obtainedClient.getDefaultClientScopes().isEmpty());
+
+
+        optionalClientScopes = new ArrayList<>(List.of("address", "phone"));
+        client.setOptionalClientScopes(optionalClientScopes);
+        ClientRepresentation updatedClient = reg.update(client);
+        requestedClientScopes = new HashSet<>(optionalClientScopes);
+        registeredClientScopes = new HashSet<>(updatedClient.getOptionalClientScopes());
+        assertEquals(requestedClientScopes, registeredClientScopes);
+        assertTrue(updatedClient.getDefaultClientScopes().isEmpty());
+    }
+
     @Test
     public void testInvalidUrlClientValidation() {
         testClientUriValidation("Root URL is not a valid URL",