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 4c3ee05b93..4112506804 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 @@ -464,28 +464,7 @@ public class RepresentationToModel { addClientScopeToClient(realm, client, clientTemplateName, true); } - 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(realm, client, clientScopeName, true); - } - } - if (resourceRep.getOptionalClientScopes() != null) { - for (String clientScopeName : resourceRep.getOptionalClientScopes()) { - addClientScopeToClient(realm, client, clientScopeName, false); - } - } + updateClientScopes(resourceRep, client); 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); diff --git a/services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java b/services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java index c9b0af1fb3..17324c65b8 100755 --- a/services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java +++ b/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()); diff --git a/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/ClientScopesClientRegistrationPolicy.java b/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/ClientScopesClientRegistrationPolicy.java index 08c751fe94..a9837dc874 100644 --- a/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/ClientScopesClientRegistrationPolicy.java +++ b/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 requestedDefaultScopeNames = context.getClient().getDefaultClientScopes(); - List requestedOptionalScopeNames = context.getClient().getOptionalClientScopes(); + List requestedDefaultScopeNames = new LinkedList<>(); + List 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()); - } - if (requestedOptionalScopeNames != null) { - requestedOptionalScopeNames.removeAll(clientModel.getClientScopes(false).keySet()); - } + requestedDefaultScopeNames.removeAll(clientModel.getClientScopes(true).keySet()); + requestedOptionalScopeNames.removeAll(clientModel.getClientScopes(false).keySet()); List allowedDefaultScopeNames = getAllowedScopeNames(realm, true); List allowedOptionalScopeNames = getAllowedScopeNames(realm, false); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java index 924cf96dd9..bff7d220d6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java +++ b/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 optionalClientScopes = new ArrayList<>(List.of("address")); + client.setOptionalClientScopes(optionalClientScopes); + + ClientRepresentation createdClient = registerClient(client); + Set requestedClientScopes = new HashSet<>(optionalClientScopes); + Set 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",