From 00017b44a389958db2b64da627ad0c1b3d89e490 Mon Sep 17 00:00:00 2001 From: vramik Date: Fri, 9 Jul 2021 12:46:01 +0200 Subject: [PATCH] KEYCLOAK-18311 fix creation of roles during client registration --- .../keycloak/models/RoleContainerModel.java | 10 ++++++- .../AbstractClientRegistrationProvider.java | 26 +++++++++++++++++++ .../client/ClientRegistrationTest.java | 25 +++++++++++++++--- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/server-spi/src/main/java/org/keycloak/models/RoleContainerModel.java b/server-spi/src/main/java/org/keycloak/models/RoleContainerModel.java index 639a69117f..01ca540d1e 100755 --- a/server-spi/src/main/java/org/keycloak/models/RoleContainerModel.java +++ b/server-spi/src/main/java/org/keycloak/models/RoleContainerModel.java @@ -20,6 +20,7 @@ package org.keycloak.models; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import org.keycloak.provider.ProviderEvent; @@ -98,14 +99,21 @@ public interface RoleContainerModel { /** * @deprecated Default roles are now managed by {@link org.keycloak.models.RealmModel#getDefaultRole()}. This method will be removed. + * @return List of default roles names or empty list if there are none. Never returns {@code null}. */ @Deprecated default List getDefaultRoles() { - return getDefaultRolesStream().collect(Collectors.toList()); + Stream defaultRolesStream = getDefaultRolesStream(); + if (defaultRolesStream != null) { + return defaultRolesStream.collect(Collectors.toList()); + } else { + return Collections.emptyList(); + } } /** * @deprecated Default roles are now managed by {@link org.keycloak.models.RealmModel#getDefaultRole()}. This method will be removed. + * @return Stream of default roles names or empty stream if there are none. Never returns {@code null}. */ @Deprecated Stream getDefaultRolesStream(); 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 53a6e73815..32388731ba 100755 --- a/services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java @@ -17,6 +17,7 @@ package org.keycloak.services.clientregistration; +import java.util.stream.Stream; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; import org.keycloak.models.ClientInitialAccessModel; @@ -65,6 +66,12 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist RealmModel realm = session.getContext().getRealm(); ClientModel clientModel = ClientManager.createClient(session, realm, client); + if (client.getDefaultRoles() != null) { + for (String name : client.getDefaultRoles()) { + clientModel.addDefaultRole(name); + } + } + if (clientModel.isServiceAccountsEnabled()) { new ClientManager(new RealmManager(session)).enableServiceAccount(clientModel); } @@ -90,6 +97,11 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist client.setDirectAccessGrantsEnabled(false); + Stream defaultRolesNames = clientModel.getDefaultRolesStream(); + if (defaultRolesNames != null) { + client.setDefaultRoles(defaultRolesNames.toArray(String[]::new)); + } + event.client(client.getClientId()).success(); return client; } catch (ModelDuplicateException e) { @@ -114,6 +126,11 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist rep.setRegistrationAccessToken(registrationAccessToken); } + Stream defaultRolesNames = client.getDefaultRolesStream(); + if (defaultRolesNames != null) { + rep.setDefaultRoles(defaultRolesNames.toArray(String[]::new)); + } + event.client(client.getClientId()).success(); return rep; } @@ -133,8 +150,17 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist RepresentationToModel.updateClient(rep, client); RepresentationToModel.updateClientProtocolMappers(rep, client); + if (rep.getDefaultRoles() != null) { + client.updateDefaultRoles(rep.getDefaultRoles()); + } + rep = ModelToRepresentation.toRepresentation(client, session); + Stream defaultRolesNames = client.getDefaultRolesStream(); + if (defaultRolesNames != null) { + rep.setDefaultRoles(defaultRolesNames.toArray(String[]::new)); + } + if (auth.isRegistrationAccessToken()) { String registrationAccessToken = ClientRegistrationTokenUtils.updateRegistrationAccessToken(session, client, auth.getRegistrationAuth()); rep.setRegistrationAccessToken(registrationAccessToken); 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 7a0e54750e..4a09a8a449 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 @@ -17,7 +17,7 @@ package org.keycloak.testsuite.client; -import org.junit.Assert; +import org.hamcrest.Matchers; import org.junit.Test; import org.keycloak.client.registration.Auth; import org.keycloak.client.registration.ClientRegistration; @@ -26,7 +26,6 @@ import org.keycloak.client.registration.HttpErrorException; import org.keycloak.models.Constants; import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper; import org.keycloak.representations.idm.ClientRepresentation; -import org.keycloak.representations.idm.ClientScopeRepresentation; import org.keycloak.representations.idm.OAuth2ErrorRepresentation; import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -46,13 +45,13 @@ import java.util.Set; import java.util.stream.Collectors; import static java.util.Arrays.asList; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import org.keycloak.protocol.oidc.OIDCLoginProtocol; @@ -90,7 +89,7 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest { // Remove this client after test getCleanup().addClientUuid(createdClient.getId()); - return client; + return createdClient; } @Test @@ -174,6 +173,24 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest { assertEquals(name, createdClient.getName()); } + @Test + public void clientWithDefaultRoles() throws ClientRegistrationException { + authCreateClients(); + ClientRepresentation client = buildClient(); + client.setDefaultRoles(new String[]{"test-default-role"}); + + ClientRepresentation createdClient = registerClient(client); + assertThat(createdClient.getDefaultRoles(), Matchers.arrayContaining("test-default-role")); + + authManageClients(); + ClientRepresentation obtainedClient = reg.get(CLIENT_ID); + assertThat(obtainedClient.getDefaultRoles(), Matchers.arrayContaining("test-default-role")); + + client.setDefaultRoles(new String[]{"test-default-role1","test-default-role2"}); + ClientRepresentation updatedClient = reg.update(client); + assertThat(updatedClient.getDefaultRoles(), Matchers.arrayContainingInAnyOrder("test-default-role1","test-default-role2")); + } + @Test public void testInvalidUrlClientValidation() { testClientUriValidation("Root URL is not a valid URL",