Client scope assignment for client registration

Closes #31062

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

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

@@ -37,6 +37,7 @@ import org.keycloak.models.RealmModel;
 import org.keycloak.models.RoleModel;
 import org.keycloak.models.utils.ModelToRepresentation;
 import org.keycloak.models.utils.RepresentationToModel;
+import org.keycloak.protocol.oidc.OIDCLoginProtocolFactory;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.oidc.OIDCClientRepresentation;
 import org.keycloak.services.ErrorResponseException;
@@ -67,6 +68,9 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist
 
     public ClientRepresentation create(ClientRegistrationContext context) {
         ClientRepresentation client = context.getClient();
+        if(client.getOptionalClientScopes() != null && client.getDefaultClientScopes() == null) {
+            client.setDefaultClientScopes(List.of(OIDCLoginProtocolFactory.BASIC_SCOPE));
+        }
 
         event.event(EventType.CLIENT_REGISTER);
 

services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java

@@ -55,6 +55,7 @@ import java.net.URI;
 import java.security.PublicKey;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -82,7 +83,9 @@ public class DescriptionConverter {
         client.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL);
 
         String scopeParam = clientOIDC.getScope();
-        if (scopeParam != null) client.setOptionalClientScopes(new ArrayList<>(Arrays.asList(scopeParam.split(" "))));
+        if (scopeParam != null) {
+            client.setOptionalClientScopes(scopeParam.isEmpty() ? Collections.emptyList() : Arrays.asList(scopeParam.split(" ")));
+        }
 
         List<String> oidcResponseTypes = clientOIDC.getResponseTypes();
         if (oidcResponseTypes == null || oidcResponseTypes.isEmpty()) {

services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java

@@ -29,6 +29,7 @@ import org.keycloak.models.utils.ModelToRepresentation;
 import org.keycloak.models.utils.RepresentationToModel;
 import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
+import org.keycloak.protocol.oidc.OIDCLoginProtocolFactory;
 import org.keycloak.protocol.oidc.mappers.AbstractPairwiseSubMapper;
 import org.keycloak.protocol.oidc.mappers.PairwiseSubMapperHelper;
 import org.keycloak.protocol.oidc.mappers.SHA256PairwiseSubMapper;
@@ -56,6 +57,8 @@ import jakarta.ws.rs.core.Response;
 import org.keycloak.urls.UrlType;
 
 import java.net.URI;
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
@@ -129,6 +132,13 @@ public class OIDCClientRegistrationProvider extends AbstractClientRegistrationPr
     public Response updateOIDC(@PathParam("clientId") String clientId, OIDCClientRepresentation clientOIDC) {
         try {
             ClientRepresentation client = DescriptionConverter.toInternal(session, clientOIDC);
+
+            if (clientOIDC.getScope() != null) {
+                ClientModel oldClient = session.getContext().getRealm().getClientById(clientOIDC.getClientId());
+                Collection<String> defaultClientScopes = oldClient.getClientScopes(true).keySet();
+                client.setDefaultClientScopes(new ArrayList<>(defaultClientScopes));
+            }
+
             OIDCClientRegistrationContext oidcContext = new OIDCClientRegistrationContext(session, client, this, clientOIDC);
             client = update(clientId, oidcContext);
 

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

@@ -17,6 +17,7 @@
 
 package org.keycloak.services.clientregistration.policy.impl;
 
+import java.util.ArrayList;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.stream.Collectors;
@@ -54,11 +55,13 @@ public class ClientScopesClientRegistrationPolicy implements ClientRegistrationP
         List<String> requestedDefaultScopeNames = context.getClient().getDefaultClientScopes();
         List<String> requestedOptionalScopeNames = context.getClient().getOptionalClientScopes();
 
-        List<String> allowedDefaultScopeNames = getAllowedScopeNames(realm, true);
+        List<String> allowedScopeNames = new ArrayList<>();
-        List<String> allowedOptionalScopeNames = getAllowedScopeNames(realm, false);
+        allowedScopeNames.addAll(getAllowedScopeNames(realm, true));
+        allowedScopeNames.addAll(getAllowedScopeNames(realm, false));
 
-        checkClientScopesAllowed(requestedDefaultScopeNames, allowedDefaultScopeNames);
+
-        checkClientScopesAllowed(requestedOptionalScopeNames, allowedOptionalScopeNames);
+        checkClientScopesAllowed(requestedDefaultScopeNames, allowedScopeNames);
+        checkClientScopesAllowed(requestedOptionalScopeNames, allowedScopeNames);
     }
 
     @Override
@@ -82,11 +85,12 @@ public class ClientScopesClientRegistrationPolicy implements ClientRegistrationP
         requestedDefaultScopeNames.removeAll(clientModel.getClientScopes(true).keySet());
         requestedOptionalScopeNames.removeAll(clientModel.getClientScopes(false).keySet());
 
-        List<String> allowedDefaultScopeNames = getAllowedScopeNames(realm, true);
+        List<String> allowedScopeNames = new ArrayList<>();
-        List<String> allowedOptionalScopeNames = getAllowedScopeNames(realm, false);
+        allowedScopeNames.addAll(getAllowedScopeNames(realm, true));
+        allowedScopeNames.addAll(getAllowedScopeNames(realm, false));
 
-        checkClientScopesAllowed(requestedDefaultScopeNames, allowedDefaultScopeNames);
+        checkClientScopesAllowed(requestedDefaultScopeNames, allowedScopeNames);
-        checkClientScopesAllowed(requestedOptionalScopeNames, allowedOptionalScopeNames);
+        checkClientScopesAllowed(requestedOptionalScopeNames, allowedScopeNames);
     }
 
     @Override

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

@@ -33,6 +33,7 @@ import org.keycloak.client.registration.Auth;
 import org.keycloak.client.registration.ClientRegistration;
 import org.keycloak.client.registration.ClientRegistrationException;
 import org.keycloak.client.registration.HttpErrorException;
+import org.keycloak.common.util.CollectionUtil;
 import org.keycloak.events.Errors;
 import org.keycloak.models.Constants;
 import org.keycloak.models.utils.KeycloakModelUtils;
@@ -262,27 +263,26 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest {
         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());
+        assertTrue(CollectionUtil.collectionEquals(createdClient.getDefaultClientScopes(), Set.of("basic")));
 
         authManageClients();
         ClientRepresentation obtainedClient = reg.get(CLIENT_ID);
         registeredClientScopes = new HashSet<>(obtainedClient.getOptionalClientScopes());
         assertEquals(requestedClientScopes, registeredClientScopes);
-        assertTrue(obtainedClient.getDefaultClientScopes().isEmpty());
+        assertTrue(CollectionUtil.collectionEquals(obtainedClient.getDefaultClientScopes(), Set.of("basic")));
 
 
         optionalClientScopes = new ArrayList<>(List.of("address", "phone"));
-        client.setOptionalClientScopes(optionalClientScopes);
+        obtainedClient.setOptionalClientScopes(optionalClientScopes);
-        ClientRepresentation updatedClient = reg.update(client);
+        ClientRepresentation updatedClient = reg.update(obtainedClient);
         requestedClientScopes = new HashSet<>(optionalClientScopes);
         registeredClientScopes = new HashSet<>(updatedClient.getOptionalClientScopes());
         assertEquals(requestedClientScopes, registeredClientScopes);
-        assertTrue(updatedClient.getDefaultClientScopes().isEmpty());
+        assertTrue(CollectionUtil.collectionEquals(updatedClient.getDefaultClientScopes(), Set.of("basic")));
     }
 
     @Test
@@ -741,7 +741,7 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest {
         Set<String> requestedClientScopes = new HashSet<>(optionalClientScopes);
         Set<String> registeredClientScopes = new HashSet<>(client.getOptionalClientScopes());
         assertTrue(requestedClientScopes.equals(registeredClientScopes));
-        assertTrue(client.getDefaultClientScopes().isEmpty());
+        assertTrue(CollectionUtil.collectionEquals(client.getDefaultClientScopes(), Set.of("basic")));
     }
 
     @Test

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

@@ -40,6 +40,7 @@ import org.keycloak.representations.idm.ClientInitialAccessCreatePresentation;
 import org.keycloak.representations.idm.ClientInitialAccessPresentation;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
+import org.keycloak.representations.idm.ClientScopeRepresentation;
 import org.keycloak.representations.oidc.OIDCClientRepresentation;
 import org.keycloak.testsuite.Assert;
 import org.keycloak.testsuite.admin.ApiUtil;
@@ -179,19 +180,27 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest {
 
     @Test
     public void updateClient() throws ClientRegistrationException {
+        Set<String> realmDefaultClientScopes = adminClient.realm(REALM_NAME).getDefaultDefaultClientScopes().stream()
+                .filter(scope -> Objects.equals(scope.getProtocol(), OIDCLoginProtocol.LOGIN_PROTOCOL))
+                .map(ClientScopeRepresentation::getName).collect(Collectors.toSet());
+
         OIDCClientRepresentation response = create();
         reg.auth(Auth.token(response));
 
         response.setRedirectUris(Collections.singletonList("http://newredirect"));
         response.setResponseTypes(Arrays.asList("code", "id_token token", "code id_token token"));
         response.setGrantTypes(Arrays.asList(OAuth2Constants.AUTHORIZATION_CODE, OAuth2Constants.REFRESH_TOKEN, OAuth2Constants.PASSWORD));
-
         OIDCClientRepresentation updated = reg.oidc().update(response);
 
+        ClientResource clientResource = adminClient.realm(REALM_NAME).clients().get(response.getClientId());
+        ClientRepresentation rep = clientResource.toRepresentation();
+        Set<String> registeredDefaultClientScopes = new HashSet<>(rep.getDefaultClientScopes());
+
         assertEquals(AUTH_SERVER_ROOT + "/realms/" + REALM_NAME + "/clients-registrations/openid-connect/" + updated.getClientId(), updated.getRegistrationClientUri());
         assertTrue(CollectionUtil.collectionEquals(Collections.singletonList("http://newredirect"), updated.getRedirectUris()));
         assertTrue(CollectionUtil.collectionEquals(Arrays.asList(OAuth2Constants.AUTHORIZATION_CODE, OAuth2Constants.IMPLICIT, OAuth2Constants.REFRESH_TOKEN, OAuth2Constants.PASSWORD), updated.getGrantTypes()));
         assertTrue(CollectionUtil.collectionEquals(Arrays.asList(OAuth2Constants.CODE, OIDCResponseType.NONE, OIDCResponseType.ID_TOKEN, "id_token token", "code id_token", "code token", "code id_token token"), updated.getResponseTypes()));
+        assertTrue(CollectionUtil.collectionEquals(realmDefaultClientScopes, registeredDefaultClientScopes));
     }
 
     @Test
@@ -743,8 +752,7 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest {
         assertTrue(clientScopes.equals(registeredClientScopes));
 
         ClientResource clientResource = adminClient.realm(REALM_NAME).clients().get(response.getClientId());
-        assertTrue(clientResource.toRepresentation().getDefaultClientScopes().isEmpty());
+        assertTrue(CollectionUtil.collectionEquals(clientResource.toRepresentation().getDefaultClientScopes(), Set.of("basic")));
-
     }
 
     @Test