From 717f65621e479b359096f7e966d90cb38a35d45a Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Mon, 7 Nov 2022 15:06:46 +0100 Subject: [PATCH] Allow a partial import to overwrite the default role (#15316) Closes #9891 --- .../models/cache/infinispan/RealmAdapter.java | 1 + .../partialimport/RolesPartialImport.java | 18 +++++++ .../partialimport/PartialImportTest.java | 51 ++++++++++++------- 3 files changed, 52 insertions(+), 18 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java index 03817848218..4337ef91159 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java @@ -1051,6 +1051,7 @@ public class RealmAdapter implements CachedRealmModel { @Override public RoleModel getDefaultRole() { + if (isUpdated()) return updated.getDefaultRole(); return cached.getDefaultRoleId() == null ? null : cacheSession.getRoleById(this, cached.getDefaultRoleId()); } diff --git a/services/src/main/java/org/keycloak/partialimport/RolesPartialImport.java b/services/src/main/java/org/keycloak/partialimport/RolesPartialImport.java index c4b295bfb59..58de4cab70e 100644 --- a/services/src/main/java/org/keycloak/partialimport/RolesPartialImport.java +++ b/services/src/main/java/org/keycloak/partialimport/RolesPartialImport.java @@ -18,6 +18,7 @@ package org.keycloak.partialimport; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; +import org.keycloak.models.RoleModel; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.RepresentationToModel; import org.keycloak.representations.idm.PartialImportRepresentation; @@ -29,6 +30,7 @@ import org.keycloak.services.ServicesLogger; import javax.ws.rs.core.Response; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; /** @@ -54,6 +56,7 @@ public class RolesPartialImport implements PartialImport { private final RealmRolesPartialImport realmRolesPI = new RealmRolesPartialImport(); private final ClientRolesPartialImport clientRolesPI = new ClientRolesPartialImport(); + private RoleRepresentation newDefaultRole; @Override public void prepare(PartialImportRepresentation rep, RealmModel realm, KeycloakSession session) throws ErrorResponseException { @@ -66,6 +69,16 @@ public class RolesPartialImport implements PartialImport { realmRolesPI.prepare(rep, realm, session); this.realmRolesToOverwrite = realmRolesPI.getToOverwrite(); + if (realmRolesToOverwrite.size() > 0) { + String defaultRoleName = realm.getDefaultRole().getName(); + for (RoleRepresentation representation : realmRolesToOverwrite) { + if (Objects.equals(defaultRoleName, representation.getName())) { + this.newDefaultRole = representation; + break; + } + } + } + this.realmRolesToSkip = realmRolesPI.getToSkip(); } @@ -94,6 +107,11 @@ public class RolesPartialImport implements PartialImport { if (rep.hasRealmRoles()) setUniqueIds(rep.getRoles().getRealm()); if (rep.hasClientRoles()) setUniqueIds(rep.getRoles().getClient()); + if (newDefaultRole != null) { + RoleModel defaultRole = RepresentationToModel.createRole(realm, newDefaultRole); + realm.setDefaultRole(defaultRole); + } + try { RepresentationToModel.importRoles(rep.getRoles(), realm); } catch (Exception e) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialimport/PartialImportTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialimport/PartialImportTest.java index 0a511989ea5..f6896be62a4 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialimport/PartialImportTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/partialimport/PartialImportTest.java @@ -49,6 +49,7 @@ import org.keycloak.testsuite.util.RealmBuilder; import javax.ws.rs.core.Response; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -268,7 +269,7 @@ public class PartialImportTest extends AbstractAuthTest { piRep.setGroups(groups); } - private void addClients(boolean withServiceAccounts) throws IOException { + private void addClients(boolean withServiceAccounts) { List clients = new ArrayList<>(); List serviceAccounts = new ArrayList<>(); @@ -440,7 +441,7 @@ public class PartialImportTest extends AbstractAuthTest { doImport(); UserRepresentation user = createUserRepresentation(USER_PREFIX + 999, USER_PREFIX + 1 + "@foo.com", "foo", "bar", true); - piRep.setUsers(Arrays.asList(user)); + piRep.setUsers(List.of(user)); PartialImportResults results = doImport(); assertEquals(1, results.getAdded()); @@ -479,7 +480,7 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testAddClients() throws IOException { + public void testAddClients() { setFail(); addClients(false); @@ -495,7 +496,7 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testAddClientsWithServiceAccountsAndAuthorization() throws IOException { + public void testAddClientsWithServiceAccountsAndAuthorization() { setFail(); addClients(true); @@ -621,7 +622,7 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testAddClientsFail() throws IOException { + public void testAddClientsFail() { addClients(false); testFail(); } @@ -676,13 +677,13 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testAddClientsSkip() throws IOException { + public void testAddClientsSkip() { addClients(false); testSkip(); } @Test - public void testAddClientsSkipWithServiceAccountsAndAuthorization() throws IOException { + public void testAddClientsSkipWithServiceAccountsAndAuthorization() { addClients(true); setSkip(); PartialImportResults results = doImport(); @@ -742,13 +743,13 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testAddClientsOverwrite() throws IOException { + public void testAddClientsOverwrite() { addClients(false); testOverwrite(); } @Test - public void testAddClientsOverwriteWithServiceAccountsAndAuthorization() throws IOException { + public void testAddClientsOverwriteWithServiceAccountsAndAuthorization() { addClients(true); setOverwrite(); PartialImportResults results = doImport(); @@ -759,7 +760,7 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testAddClientsOverwriteServiceAccountsWithNoServiceAccounts() throws IOException { + public void testAddClientsOverwriteServiceAccountsWithNoServiceAccounts() { addClients(true); setOverwrite(); PartialImportResults results = doImport(); @@ -806,7 +807,7 @@ public class PartialImportTest extends AbstractAuthTest { testOverwrite(); } - private void importEverything(boolean withServiceAccounts) throws IOException { + private void importEverything(boolean withServiceAccounts) { addUsers(); addGroups(); addClients(withServiceAccounts); @@ -824,7 +825,7 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testEverythingFail() throws IOException { + public void testEverythingFail() { setFail(); importEverything(false); PartialImportResults results = doImport(); // second import will fail because not allowed to skip or overwrite @@ -832,7 +833,7 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testEverythingSkip() throws IOException { + public void testEverythingSkip() { setSkip(); importEverything(false); PartialImportResults results = doImport(); @@ -840,7 +841,7 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testEverythingSkipWithServiceAccounts() throws IOException { + public void testEverythingSkipWithServiceAccounts() { setSkip(); importEverything(true); PartialImportResults results = doImport(); @@ -848,7 +849,7 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testEverythingOverwrite() throws IOException { + public void testEverythingOverwrite() { setOverwrite(); importEverything(false); PartialImportResults results = doImport(); @@ -856,7 +857,7 @@ public class PartialImportTest extends AbstractAuthTest { } @Test - public void testEverythingOverwriteWithServiceAccounts() throws IOException { + public void testEverythingOverwriteWithServiceAccounts() { setOverwrite(); importEverything(true); PartialImportResults results = doImport(); @@ -877,7 +878,7 @@ public class PartialImportTest extends AbstractAuthTest { RolesRepresentation roles = new RolesRepresentation(); roles.setClient(clients); - piRep.setClients(Arrays.asList(client)); + piRep.setClients(List.of(client)); piRep.setRoles(roles); doImport(); @@ -898,7 +899,7 @@ public class PartialImportTest extends AbstractAuthTest { @Test public void testOverwriteExistingClientWithServiceAccount() { setOverwrite(); - piRep.setClients(Arrays.asList(testRealmResource().clients().findByClientId(CLIENT_SERVICE_ACCOUNT).get(0))); + piRep.setClients(Collections.singletonList(testRealmResource().clients().findByClientId(CLIENT_SERVICE_ACCOUNT).get(0))); Assert.assertEquals(1, doImport().getOverwritten()); @@ -906,4 +907,18 @@ public class PartialImportTest extends AbstractAuthTest { testRealmResource().clients().get(client.getId()).getServiceAccountUser(); } + @Test + public void testOverwriteDefaultRole() { + setOverwrite(); + + RolesRepresentation roles = new RolesRepresentation(); + RoleRepresentation oldDefaultRole = testRealmResource().toRepresentation().getDefaultRole(); + roles.setRealm(Collections.singletonList(oldDefaultRole)); + piRep.setRoles(roles); + + Assert.assertEquals("default role should have been overwritten", 1, doImport().getOverwritten()); + Assert.assertNotEquals("when overwriting, the ID of the role changes", + testRealmResource().toRepresentation().getDefaultRole().getId(), oldDefaultRole.getId()); + } + }