From 2b1acb99a226f684024d01bb9e6b04bf4c465392 Mon Sep 17 00:00:00 2001
From: Benjamin Weimer <50862958+benjamin37@users.noreply.github.com>
Date: Mon, 23 Sep 2019 13:08:24 +0200
Subject: [PATCH] KEYCLAOK-9999 fix client import (#6136)

---
 .../models/utils/RepresentationToModel.java   | 14 ++++---
 .../admin/client/ClientScopeTest.java         | 37 +++++++++++++++++++
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java
index 9d870b3354..d9524ec0b2 100755
--- a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java
+++ b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java
@@ -1247,22 +1247,24 @@ public class RepresentationToModel {
             addClientScopeToClient(realm, client, clientTemplateName, true);
         }
 
-        if (resourceRep.getDefaultClientScopes() != null) {
+        if (resourceRep.getDefaultClientScopes() != null || resourceRep.getOptionalClientScopes() != null) {
             // First remove all default/built in client scopes
             for (ClientScopeModel clientScope : client.getClientScopes(true, false).values()) {
                 client.removeClientScope(clientScope);
             }
 
+            // First remove all default/built in client scopes
+            for (ClientScopeModel clientScope : client.getClientScopes(false, false).values()) {
+                client.removeClientScope(clientScope);
+            }
+        }
+
+        if (resourceRep.getDefaultClientScopes() != null) {
             for (String clientScopeName : resourceRep.getDefaultClientScopes()) {
                 addClientScopeToClient(realm, client, clientScopeName, true);
             }
         }
         if (resourceRep.getOptionalClientScopes() != null) {
-            // First remove all default/built in client scopes
-            for (ClientScopeModel clientScope : client.getClientScopes(false, false).values()) {
-                client.removeClientScope(clientScope);
-            }
-
             for (String clientScopeName : resourceRep.getOptionalClientScopes()) {
                 addClientScopeToClient(realm, client, clientScopeName, false);
             }
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 937028dbc6..6f34164e01 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
@@ -439,6 +439,43 @@ public class ClientScopeTest extends AbstractClientTest {
         Assert.assertFalse(clientOptionalScopes .contains("scope-opt"));
     }
 
+    // KEYCLOAK-9999
+    @Test
+    public void defaultOptionalClientScopeCanBeAssignedToClientAsDefaultScope() {
+
+        // Create optional client scope
+        ClientScopeRepresentation optionalClientScope = new ClientScopeRepresentation();
+        optionalClientScope.setName("optional-client-scope");
+        optionalClientScope.setProtocol("openid-connect");
+        String optionalClientScopeId = createClientScope(optionalClientScope);
+        getCleanup().addClientScopeId(optionalClientScopeId);
+
+        testRealmResource().addDefaultOptionalClientScope(optionalClientScopeId);
+        assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.defaultOptionalClientScopePath(optionalClientScopeId), ResourceType.CLIENT_SCOPE);
+
+        // Ensure that scope is optional
+        List<String> realmOptionalScopes = getClientScopeNames(testRealmResource().getDefaultOptionalClientScopes());
+        Assert.assertTrue(realmOptionalScopes.contains("optional-client-scope"));
+
+        // Create client
+        ClientRepresentation client = new ClientRepresentation();
+        client.setClientId("test-client");
+        client.setDefaultClientScopes(Collections.singletonList("optional-client-scope"));
+        String clientUuid = createClient(client);
+        getCleanup().addClientUuid(clientUuid);
+
+        // Ensure that default optional client scope is a default scope of the client
+        List<String> clientDefaultScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getDefaultClientScopes());
+        Assert.assertTrue(clientDefaultScopes.contains("optional-client-scope"));
+
+        // Ensure that no optional scopes are assigned to the client, even if there are default optional scopes!
+        List<String> clientOptionalScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getOptionalClientScopes());
+        Assert.assertTrue(clientOptionalScopes.isEmpty());
+
+        // Unassign optional client scope from realm for cleanup
+        testRealmResource().removeDefaultOptionalClientScope(optionalClientScopeId);
+        assertAdminEvents.assertEvent(getRealmId(), OperationType.DELETE, AdminEventPaths.defaultOptionalClientScopePath(optionalClientScopeId), ResourceType.CLIENT_SCOPE);
+    }
 
     // KEYCLOAK-5863
     @Test