Skip to content

Allow a partial import to overwrite the default role #15316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stianst merged 1 commit into from
Nov 7, 2022
Merged

Allow a partial import to overwrite the default role #15316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,7 @@ public void setDefaultRole(RoleModel role) {

@Override
public RoleModel getDefaultRole() {
if (isUpdated()) return updated.getDefaultRole();
return cached.getDefaultRoleId() == null ? null : cacheSession.getRoleById(this, cached.getDefaultRoleId());
}

Expand Down
18 changes: 18 additions & 0 deletions services/src/main/java/org/keycloak/partialimport/RolesPartialImport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

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;
Expand All @@ -29,6 +30,7 @@
import javax.ws.rs.core.Response;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

/**
Expand All @@ -54,6 +56,7 @@ public class RolesPartialImport implements PartialImport<RolesRepresentation> {

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 {
Expand All @@ -66,6 +69,16 @@ private void prepareRealmRoles(PartialImportRepresentation rep, RealmModel realm

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();
}

Expand Down Expand Up @@ -94,6 +107,11 @@ public PartialImportResults doImport(PartialImportRepresentation rep, RealmModel
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) {
Expand Down
51 changes: 33 additions & 18 deletions ...ests/base/src/test/java/org/keycloak/testsuite/admin/partialimport/PartialImportTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
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;
Expand Down Expand Up @@ -268,7 +269,7 @@ private void addGroups() {
piRep.setGroups(groups);
}

private void addClients(boolean withServiceAccounts) throws IOException {
private void addClients(boolean withServiceAccounts) {
List<ClientRepresentation> clients = new ArrayList<>();
List<UserRepresentation> serviceAccounts = new ArrayList<>();

Expand Down Expand Up @@ -440,7 +441,7 @@ public void testAddUsersWithDuplicateEmailsAllowed() {
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());
Expand Down Expand Up @@ -479,7 +480,7 @@ public void testAddUsersWithTermsAndConditions() {
}

@Test
public void testAddClients() throws IOException {
public void testAddClients() {
setFail();
addClients(false);

Expand All @@ -495,7 +496,7 @@ public void testAddClients() throws IOException {
}

@Test
public void testAddClientsWithServiceAccountsAndAuthorization() throws IOException {
public void testAddClientsWithServiceAccountsAndAuthorization() {
setFail();
addClients(true);

Expand Down Expand Up @@ -621,7 +622,7 @@ public void testAddGroupsFail() {
}

@Test
public void testAddClientsFail() throws IOException {
public void testAddClientsFail() {
addClients(false);
testFail();
}
Expand Down Expand Up @@ -676,13 +677,13 @@ public void testAddGroupsSkip() {
}

@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();
Expand Down Expand Up @@ -742,13 +743,13 @@ public void testAddGroupsOverwrite() {
}

@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();
Expand All @@ -759,7 +760,7 @@ public void testAddClientsOverwriteWithServiceAccountsAndAuthorization() throws
}

@Test
public void testAddClientsOverwriteServiceAccountsWithNoServiceAccounts() throws IOException {
public void testAddClientsOverwriteServiceAccountsWithNoServiceAccounts() {
addClients(true);
setOverwrite();
PartialImportResults results = doImport();
Expand Down Expand Up @@ -806,7 +807,7 @@ public void testAddClientRolesOverwrite() {
testOverwrite();
}

private void importEverything(boolean withServiceAccounts) throws IOException {
private void importEverything(boolean withServiceAccounts) {
addUsers();
addGroups();
addClients(withServiceAccounts);
Expand All @@ -824,39 +825,39 @@ private void importEverything(boolean withServiceAccounts) throws IOException {
}

@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
assertNotNull(results.getErrorMessage());
}

@Test
public void testEverythingSkip() throws IOException {
public void testEverythingSkip() {
setSkip();
importEverything(false);
PartialImportResults results = doImport();
assertEquals(NUM_ENTITIES * NUM_RESOURCE_TYPES, results.getSkipped());
}

@Test
public void testEverythingSkipWithServiceAccounts() throws IOException {
public void testEverythingSkipWithServiceAccounts() {
setSkip();
importEverything(true);
PartialImportResults results = doImport();
assertEquals(NUM_ENTITIES * (NUM_RESOURCE_TYPES + 1), results.getSkipped());
}

@Test
public void testEverythingOverwrite() throws IOException {
public void testEverythingOverwrite() {
setOverwrite();
importEverything(false);
PartialImportResults results = doImport();
assertEquals(NUM_ENTITIES * NUM_RESOURCE_TYPES, results.getOverwritten());
}

@Test
public void testEverythingOverwriteWithServiceAccounts() throws IOException {
public void testEverythingOverwriteWithServiceAccounts() {
setOverwrite();
importEverything(true);
PartialImportResults results = doImport();
Expand All @@ -877,7 +878,7 @@ public void testOverwriteExistingClientWithRoles() {
RolesRepresentation roles = new RolesRepresentation();
roles.setClient(clients);

piRep.setClients(Arrays.asList(client));
piRep.setClients(List.of(client));
piRep.setRoles(roles);

doImport();
Expand All @@ -898,12 +899,26 @@ public void testOverwriteExistingInternalClient() {
@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());

ClientRepresentation client = testRealmResource().clients().findByClientId(CLIENT_SERVICE_ACCOUNT).get(0);
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());
}

}