From 12732333c80db8478aa70556c62e3f71ede44cbd Mon Sep 17 00:00:00 2001 From: Giuseppe Graziano Date: Wed, 24 Jul 2024 17:40:16 +0200 Subject: [PATCH] Client scope assignment for client registration Closes #31062 Signed-off-by: Giuseppe Graziano --- .../AbstractClientRegistrationProvider.java | 4 ++++ .../oidc/DescriptionConverter.java | 5 ++++- .../oidc/OIDCClientRegistrationProvider.java | 10 ++++++++++ .../ClientScopesClientRegistrationPolicy.java | 20 +++++++++++-------- .../client/ClientRegistrationTest.java | 14 ++++++------- .../client/OIDCClientRegistrationTest.java | 14 ++++++++++--- .../OIDCPairwiseClientRegistrationTest.java | 2 +- .../keycloak/testsuite/oauth/par/ParTest.java | 2 +- 8 files changed, 50 insertions(+), 21 deletions(-) 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 caaf1bb070..8929bbd7dd 100755 --- a/services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java +++ b/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); diff --git a/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java b/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java index 951f561d00..30b0b726f9 100755 --- a/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java +++ b/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 oidcResponseTypes = clientOIDC.getResponseTypes(); if (oidcResponseTypes == null || oidcResponseTypes.isEmpty()) { diff --git a/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java b/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java index 893b29d98b..8acf0500ae 100755 --- a/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java +++ b/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 defaultClientScopes = oldClient.getClientScopes(true).keySet(); + client.setDefaultClientScopes(new ArrayList<>(defaultClientScopes)); + } + OIDCClientRegistrationContext oidcContext = new OIDCClientRegistrationContext(session, client, this, clientOIDC); client = update(clientId, oidcContext); 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 a9837dc874..d4356c33b5 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 @@ -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 requestedDefaultScopeNames = context.getClient().getDefaultClientScopes(); List requestedOptionalScopeNames = context.getClient().getOptionalClientScopes(); - List allowedDefaultScopeNames = getAllowedScopeNames(realm, true); - List allowedOptionalScopeNames = getAllowedScopeNames(realm, false); + List allowedScopeNames = new ArrayList<>(); + 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 allowedDefaultScopeNames = getAllowedScopeNames(realm, true); - List allowedOptionalScopeNames = getAllowedScopeNames(realm, false); + List allowedScopeNames = new ArrayList<>(); + allowedScopeNames.addAll(getAllowedScopeNames(realm, true)); + allowedScopeNames.addAll(getAllowedScopeNames(realm, false)); - checkClientScopesAllowed(requestedDefaultScopeNames, allowedDefaultScopeNames); - checkClientScopesAllowed(requestedOptionalScopeNames, allowedOptionalScopeNames); + checkClientScopesAllowed(requestedDefaultScopeNames, allowedScopeNames); + checkClientScopesAllowed(requestedOptionalScopeNames, allowedScopeNames); } @Override 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 e57a43335e..b505301e9d 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 @@ -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 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()); + 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); - ClientRepresentation updatedClient = reg.update(client); + obtainedClient.setOptionalClientScopes(optionalClientScopes); + 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 requestedClientScopes = new HashSet<>(optionalClientScopes); Set registeredClientScopes = new HashSet<>(client.getOptionalClientScopes()); assertTrue(requestedClientScopes.equals(registeredClientScopes)); - assertTrue(client.getDefaultClientScopes().isEmpty()); + assertTrue(CollectionUtil.collectionEquals(client.getDefaultClientScopes(), Set.of("basic"))); } @Test diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java index 8e3ba20f33..2d27f8939b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java +++ b/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 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 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 diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java index 59fd747fd0..860b4d1748 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCPairwiseClientRegistrationTest.java @@ -508,4 +508,4 @@ public class OIDCPairwiseClientRegistrationTest extends AbstractClientRegistrati ApiUtil.findClientResourceByClientId(adminClient.realm(REALM_NAME), clientId).removeDefaultClientScope(scope.getId()); }); } -} \ No newline at end of file +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/par/ParTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/par/ParTest.java index ca53bdb944..a3a8fc905c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/par/ParTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/par/ParTest.java @@ -1334,4 +1334,4 @@ public class ParTest extends AbstractClientPoliciesTest { assertEquals(0, response.getHeaders("Access-Control-Expose-Headers").length); } -} \ No newline at end of file +}