KEYCLOAK-18035 Fix update client with default default scope assigned as optional

vramik · hmlnarik · commit 95bf912dc969 · 2021-06-07T16:22:55.000+02:00
diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java
@@ -868,7 +868,8 @@ public void addClientScopes(RealmModel realm, ClientModel client, Set<ClientScop
         // Defaults to openid-connect
         String clientProtocol = client.getProtocol() == null ? OIDCLoginProtocol.LOGIN_PROTOCOL : client.getProtocol();
 
-        Map<String, ClientScopeModel> existingClientScopes = getClientScopes(realm, client, defaultScope);
+        Map<String, ClientScopeModel> existingClientScopes = getClientScopes(realm, client, true);
+        existingClientScopes.putAll(getClientScopes(realm, client, false));
 
         clientScopes.stream()
             .filter(clientScope -> ! existingClientScopes.containsKey(clientScope.getName()))
diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientProvider.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientProvider.java
@@ -298,7 +298,8 @@ public void addClientScopes(RealmModel realm, ClientModel client, Set<ClientScop
 
         LOG.tracef("addClientScopes(%s, %s, %s, %b)%s", realm, client, clientScopes, defaultScope, getShortStackTrace());
 
-        Map<String, ClientScopeModel> existingClientScopes = getClientScopes(realm, client, defaultScope);
+        Map<String, ClientScopeModel> existingClientScopes = getClientScopes(realm, client, true);
+        existingClientScopes.putAll(getClientScopes(realm, client, false));
 
         clientScopes.stream()
                 .filter(clientScope -> ! existingClientScopes.containsKey(clientScope.getName()))
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
@@ -506,6 +506,42 @@ public void testUpdateProtocolMappers() {
         clientScopes().get(scopeId).remove();
     }
 
+    @Test
+    public void updateClientWithDefaultScopeAssignedAsOptionalAndOpposite() {
+        // create client
+        ClientRepresentation clientRep = new ClientRepresentation();
+        clientRep.setClientId("bar-client");
+        clientRep.setProtocol("openid-connect");
+        String clientUuid = createClient(clientRep);
+        getCleanup().addClientUuid(clientUuid);
+
+        // Create 2 client scopes
+        ClientScopeRepresentation scopeRep = new ClientScopeRepresentation();
+        scopeRep.setName("scope-def");
+        scopeRep.setProtocol("openid-connect");
+        String scopeDefId = createClientScope(scopeRep);
+        getCleanup().addClientScopeId(scopeDefId);
+
+        scopeRep = new ClientScopeRepresentation();
+        scopeRep.setName("scope-opt");
+        scopeRep.setProtocol("openid-connect");
+        String scopeOptId = createClientScope(scopeRep);
+        getCleanup().addClientScopeId(scopeOptId);
+
+        // assign "scope-def" as optional client scope to client
+        testRealmResource().clients().get(clientUuid).addOptionalClientScope(scopeDefId);
+
+        // assign "scope-opt" as default client scope to client
+        testRealmResource().clients().get(clientUuid).addDefaultClientScope(scopeOptId);
+
+        // Add scope-def as default and scope-opt as optional client scope within the realm
+        testRealmResource().addDefaultDefaultClientScope(scopeDefId);
+        testRealmResource().addDefaultOptionalClientScope(scopeOptId);
+
+        //update client - check it passes (it used to throw ModelDuplicateException before)
+        clientRep.setDescription("new_description");
+        testRealmResource().clients().get(clientUuid).update(clientRep);
+    }
 
     private ClientScopesResource clientScopes() {
         return testRealmResource().clientScopes();
