libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Fix update of group mappers on certain changes of the group path

Browse source 
The group reference in the mapper was not updated in the following cases:
- group rename: when an ancestor group was renamed
- (only for JpaRealmProvider, NOT for MapRealmProvider/MapGroupProvider) group move: when a group was converted from subgroup to top-level or when a subgroup's parent was changed

Closes #15614
This commit is contained in:
danielFesenmeyer 2022-11-22 08:17:33 +01:00 committed by Marek Posolda
parent fb315b57c3
commit 18381ecd2e
4 changed files with 110 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java
View file

@ -441,10 +441,12 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc
if (toParent != null && group.getId().equals(toParent.getId())) {
return;
}
GroupModel previousParent = group.getParent();
if (group.getParentId() != null) {
group.getParent().removeChild(group);
}
GroupModel previousParent = group.getParent();
group.setParent(toParent);
if (toParent != null) toParent.addChild(group);
else session.groups().addTopLevelGroup(realm, group);

11
server-spi-private/src/main/java/org/keycloak/broker/provider/mappersync/GroupConfigPropertyByPathSynchronizer.java
View file

@ -24,6 +24,8 @@ import org.keycloak.models.utils.KeycloakModelUtils;
import java.util.function.Consumer;
import static org.keycloak.models.utils.KeycloakModelUtils.GROUP_PATH_SEPARATOR;
/**
* Updates a group reference in a mapper config, when the path of a group changes.
*
@ -58,10 +60,17 @@ public class GroupConfigPropertyByPathSynchronizer extends AbstractConfigPropert
String configuredGroupPath = KeycloakModelUtils.normalizeGroupPath(currentPropertyValue);
String previousGroupPath = event.getPreviousPath();
if (previousGroupPath.equals(configuredGroupPath)) {
if (configuredGroupPath.equals(previousGroupPath)) {
String newPath = event.getNewPath();
propertyUpdater.accept(newPath);
} else if (isSubGroupOf(configuredGroupPath, previousGroupPath)) {
String newPath = event.getNewPath() + configuredGroupPath.substring(previousGroupPath.length());
propertyUpdater.accept(newPath);
}
}
private static boolean isSubGroupOf(String groupPath, String potentialParentGroupPath) {
return groupPath.startsWith(potentialParentGroupPath + GROUP_PATH_SEPARATOR);
}
}

4
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedGroupMapperTest.java
View file

@ -175,8 +175,8 @@ public abstract class AbstractAdvancedGroupMapperTest extends AbstractGroupMappe
}
@Override
protected String setupScenarioWithMatchingGroup() {
String mapperId = createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES, false, MAPPER_TEST_GROUP_PATH);
protected String setupScenarioWithGroupPath(String groupPath) {
String mapperId = createAdvancedGroupMapper(CLAIMS_OR_ATTRIBUTES, false, groupPath);
createUserInProviderRealm(createMatchingAttributes());
return mapperId;
}

116
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractGroupMapperTest.java
View file

@ -41,10 +41,10 @@ public abstract class AbstractGroupMapperTest extends AbstractIdentityProviderMa
IdentityProviderRepresentation idp, IdentityProviderMapperSyncMode syncMode, String groupPath);
/**
* Sets up a scenario with a matching group.
* Sets up a scenario with the given group path.
* @return the ID of the mapper
*/
protected abstract String setupScenarioWithMatchingGroup();
protected abstract String setupScenarioWithGroupPath(String groupPath);
protected abstract void setupScenarioWithNonExistingGroup();
@ -71,8 +71,8 @@ public abstract class AbstractGroupMapperTest extends AbstractIdentityProviderMa
}
@Test
public void mapperStillWorksWhenGroupIsMoved() {
final String mapperId = setupScenarioWithMatchingGroup();
public void mapperStillWorksWhenTopLevelGroupIsConvertedToSubGroup() {
final String mapperId = setupScenarioWithGroupPath(MAPPER_TEST_GROUP_PATH);
String newParentGroupName = "new-parent";
GroupRepresentation newParentGroup = new GroupRepresentation();
@ -84,21 +84,60 @@ public abstract class AbstractGroupMapperTest extends AbstractIdentityProviderMa
String expectedNewGroupPath = buildGroupPath(newParentGroupName, MAPPER_TEST_GROUP_NAME);
// mapper should have been updated to the new path of the group
IdentityProviderMapperRepresentation mapper =
realm.identityProviders().get(bc.getIDPAlias()).getMapperById(mapperId);
Map<String, String> config = mapper.getConfig();
assertThat(config.get(ConfigConstants.GROUP), equalTo(expectedNewGroupPath));
assertMapperHasExpectedPathAndSucceeds(mapperId, expectedNewGroupPath);
}
logInAsUserInIDPForFirstTimeAndAssertSuccess();
@Test
public void mapperStillWorksWhenSubGroupChangesParent() {
String parentGroupName = "parent-group";
GroupRepresentation parentGroup = new GroupRepresentation();
parentGroup.setName(parentGroupName);
String parentGroupId = CreatedResponseUtil.getCreatedId(realm.groups().add(parentGroup));
UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail());
assertThatUserHasBeenAssignedToGroup(user, expectedNewGroupPath);
GroupRepresentation mappedGroup = realm.groups().group(mapperGroupId).toRepresentation();
realm.groups().group(parentGroupId).subGroup(mappedGroup).close();
String initialGroupPath = buildGroupPath(parentGroupName, MAPPER_TEST_GROUP_NAME);
final String mapperId = setupScenarioWithGroupPath(initialGroupPath);
String newParentGroupName = "new-parent-group";
GroupRepresentation newParentGroup = new GroupRepresentation();
newParentGroup.setName(newParentGroupName);
String newParentGroupId = CreatedResponseUtil.getCreatedId(realm.groups().add(newParentGroup));
realm.groups().group(newParentGroupId).subGroup(mappedGroup).close();
String expectedNewGroupPath = buildGroupPath(newParentGroupName, MAPPER_TEST_GROUP_NAME);
assertMapperHasExpectedPathAndSucceeds(mapperId, expectedNewGroupPath);
}
@Test
public void mapperStillWorksWhenSubGroupIsConvertedToTopLevelGroup() {
String parentGroupName = "parent-group";
GroupRepresentation parentGroup = new GroupRepresentation();
parentGroup.setName(parentGroupName);
String parentGroupId = CreatedResponseUtil.getCreatedId(realm.groups().add(parentGroup));
GroupRepresentation mappedGroup = realm.groups().group(mapperGroupId).toRepresentation();
realm.groups().group(parentGroupId).subGroup(mappedGroup).close();
String initialGroupPath = buildGroupPath(parentGroupName, MAPPER_TEST_GROUP_NAME);
final String mapperId = setupScenarioWithGroupPath(initialGroupPath);
// convert the mapped group to a top-level group
realm.groups().add(realm.groups().group(mapperGroupId).toRepresentation());
String expectedNewGroupPath = buildGroupPath(MAPPER_TEST_GROUP_NAME);
assertMapperHasExpectedPathAndSucceeds(mapperId, expectedNewGroupPath);
}
@Test
public void mapperStillWorksWhenGroupIsRenamed() {
final String mapperId = setupScenarioWithMatchingGroup();
final String mapperId = setupScenarioWithGroupPath(MAPPER_TEST_GROUP_PATH);
String newGroupName = "new-name-" + MAPPER_TEST_GROUP_NAME;
GroupRepresentation mappedGroup = realm.groups().group(mapperGroupId).toRepresentation();
@ -107,16 +146,39 @@ public abstract class AbstractGroupMapperTest extends AbstractIdentityProviderMa
String expectedNewGroupPath = buildGroupPath(newGroupName);
// mapper should have been updated to the new path of the group
IdentityProviderMapperRepresentation mapper =
realm.identityProviders().get(bc.getIDPAlias()).getMapperById(mapperId);
Map<String, String> config = mapper.getConfig();
assertThat(config.get(ConfigConstants.GROUP), equalTo(expectedNewGroupPath));
assertMapperHasExpectedPathAndSucceeds(mapperId, expectedNewGroupPath);
}
logInAsUserInIDPForFirstTimeAndAssertSuccess();
@Test
public void mapperStillWorksWhenAncestorGroupIsRenamed() {
String topLevelGroupName = "top-level";
GroupRepresentation topLevelGroup = new GroupRepresentation();
topLevelGroup.setName(topLevelGroupName);
String topLevelGroupId = CreatedResponseUtil.getCreatedId(realm.groups().add(topLevelGroup));
UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail());
assertThatUserHasBeenAssignedToGroup(user, expectedNewGroupPath);
String midLevelGroupName = "mid-level";
GroupRepresentation midLevelGroup = new GroupRepresentation();
midLevelGroup.setName(midLevelGroupName);
String midLevelGroupId = CreatedResponseUtil.getCreatedId(realm.groups().add(midLevelGroup));
midLevelGroup = realm.groups().group(midLevelGroupId).toRepresentation();
realm.groups().group(topLevelGroupId).subGroup(midLevelGroup).close();
GroupRepresentation mappedGroup = realm.groups().group(mapperGroupId).toRepresentation();
realm.groups().group(midLevelGroupId).subGroup(mappedGroup).close();
String initialGroupPath = buildGroupPath(topLevelGroupName, midLevelGroupName, MAPPER_TEST_GROUP_NAME);
final String mapperId = setupScenarioWithGroupPath(initialGroupPath);
String newTopLevelGroupName = "new-name-" + topLevelGroupName;
topLevelGroup = realm.groups().group(topLevelGroupId).toRepresentation();
topLevelGroup.setName(newTopLevelGroupName);
realm.groups().group(topLevelGroupId).update(topLevelGroup);
String expectedNewGroupPath = buildGroupPath(newTopLevelGroupName, midLevelGroupName, MAPPER_TEST_GROUP_NAME);
assertMapperHasExpectedPathAndSucceeds(mapperId, expectedNewGroupPath);
}
protected UserRepresentation loginAsUserTwiceWithMapper(
@ -151,6 +213,18 @@ public abstract class AbstractGroupMapperTest extends AbstractIdentityProviderMa
return user;
}
private void assertMapperHasExpectedPathAndSucceeds(String mapperId, String expectedGroupPath) {
IdentityProviderMapperRepresentation mapper =
realm.identityProviders().get(bc.getIDPAlias()).getMapperById(mapperId);
Map<String, String> config = mapper.getConfig();
assertThat(config.get(ConfigConstants.GROUP), equalTo(expectedGroupPath));
logInAsUserInIDPForFirstTimeAndAssertSuccess();
UserRepresentation user = findUser(bc.consumerRealmName(), bc.getUserLogin(), bc.getUserEmail());
assertThatUserHasBeenAssignedToGroup(user, expectedGroupPath);
}
protected void assertThatUserHasBeenAssignedToGroup(UserRepresentation user) {
assertThatUserHasBeenAssignedToGroup(user, MAPPER_TEST_GROUP_PATH);
}