diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java index 0e502d0da62..dfa6251ca7a 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java @@ -17,6 +17,7 @@ package org.keycloak.storage.ldap.mappers.membership.group; +import org.keycloak.common.util.ObjectUtil; import org.keycloak.component.ComponentModel; import org.keycloak.component.ComponentValidationException; import org.keycloak.models.KeycloakSession; @@ -264,6 +265,7 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact ComponentModel parentModel = realm.getComponent(model.getParentId()); UserStorageProviderModel parent = new UserStorageProviderModel(parentModel); onParentUpdate(realm, parent, parent, model); + setDefaultGroupsPath(realm, model); } @@ -272,6 +274,14 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact ComponentModel parentModel = realm.getComponent(newModel.getParentId()); UserStorageProviderModel parent = new UserStorageProviderModel(parentModel); onParentUpdate(realm, parent, parent, newModel); + setDefaultGroupsPath(realm, newModel); + } + + private void setDefaultGroupsPath(RealmModel realm, ComponentModel mapperModel) { + if (ObjectUtil.isBlank(mapperModel.getConfig().getFirst(GroupMapperConfig.LDAP_GROUPS_PATH))) { + mapperModel.getConfig().putSingle(GroupMapperConfig.LDAP_GROUPS_PATH, GroupMapperConfig.DEFAULT_LDAP_GROUPS_PATH); + realm.updateComponent(mapperModel); + } } @Override @@ -293,9 +303,8 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact LDAPUtils.validateCustomLdapFilter(config.getConfig().getFirst(GroupMapperConfig.GROUPS_LDAP_FILTER)); - checkMandatoryConfigAttribute(GroupMapperConfig.LDAP_GROUPS_PATH, "Groups Path", config); - String group = config.getConfig().getFirst(GroupMapperConfig.LDAP_GROUPS_PATH).trim(); - if (!"/".equals(group) && KeycloakModelUtils.findGroupByPath(realm, group) == null) { + String group = new GroupMapperConfig(config).getGroupsPath(); + if (!GroupMapperConfig.DEFAULT_LDAP_GROUPS_PATH.equals(group) && KeycloakModelUtils.findGroupByPath(realm, group) == null) { throw new ComponentValidationException("ldapErrorMissingGroupsPathGroup"); } } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupMapperConfig.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupMapperConfig.java index 399651a050e..f5067a53604 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupMapperConfig.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupMapperConfig.java @@ -17,6 +17,7 @@ package org.keycloak.storage.ldap.mappers.membership.group; +import org.keycloak.common.util.ObjectUtil; import org.keycloak.component.ComponentModel; import org.keycloak.models.LDAPConstants; import org.keycloak.models.ModelException; @@ -63,6 +64,7 @@ public class GroupMapperConfig extends CommonLDAPGroupMapperConfig { // Keycloak group path the LDAP groups are added to (default: top level "/") public static final String LDAP_GROUPS_PATH = "groups.path"; + public static final String DEFAULT_LDAP_GROUPS_PATH = "/"; public GroupMapperConfig(ComponentModel mapperModel) { super(mapperModel); @@ -129,7 +131,8 @@ public class GroupMapperConfig extends CommonLDAPGroupMapperConfig { } public String getGroupsPath() { - return mapperModel.getConfig().getFirst(LDAP_GROUPS_PATH); + String groupsPath = mapperModel.getConfig().getFirst(LDAP_GROUPS_PATH); + return ObjectUtil.isBlank(groupsPath) ? DEFAULT_LDAP_GROUPS_PATH : groupsPath.trim(); } public String getGroupsPathWithTrailingSlash() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncWithGroupsPathTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncWithGroupsPathTest.java index ae8f6bcd0ca..20c6bcbfaf3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncWithGroupsPathTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncWithGroupsPathTest.java @@ -17,6 +17,8 @@ package org.keycloak.testsuite.federation.ldap; +import javax.ws.rs.core.Response; + import org.junit.Assert; import org.junit.Before; import org.junit.ClassRule; @@ -28,6 +30,7 @@ import org.keycloak.models.GroupModel; import org.keycloak.models.LDAPConstants; import org.keycloak.models.RealmModel; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.storage.ldap.LDAPStorageProvider; import org.keycloak.storage.ldap.LDAPUtils; import org.keycloak.storage.ldap.idm.model.LDAPObject; @@ -37,6 +40,7 @@ import org.keycloak.storage.ldap.mappers.membership.group.GroupLDAPStorageMapper import org.keycloak.storage.ldap.mappers.membership.group.GroupLDAPStorageMapperFactory; import org.keycloak.storage.ldap.mappers.membership.group.GroupMapperConfig; import org.keycloak.storage.user.SynchronizationResult; +import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.util.LDAPRule; import org.keycloak.testsuite.util.LDAPTestUtils; @@ -206,4 +210,30 @@ public class LDAPGroupMapperSyncWithGroupsPathTest extends AbstractLDAPTest { Assert.assertNotNull(KeycloakModelUtils.findGroupByPath(realm, "/outside")); }); } + + // KEYCLOAK-14892 + @Test + public void test03_createConfigurationWithoutGroupPath() throws Exception { + ComponentRepresentation groupMapperRep = findMapperRepByName("groupsMapper"); + + groupMapperRep.setId(null); + groupMapperRep.setName("different"); + groupMapperRep.getConfig().remove(GroupMapperConfig.LDAP_GROUPS_PATH); + groupMapperRep.getConfig().remove(GroupMapperConfig.LDAP_GROUPS_PATH); + + // Test that attempt to create configuration without LDAP_GROUPS_PATH configured should be still allowed due the backwards compatibility + Response response = adminClient.realm("test").components().add(groupMapperRep); + String newMapperId = ApiUtil.getCreatedId(response); + + try { + response.close(); + + // Load the mapper and assert default group path set + ComponentRepresentation newMapper = adminClient.realm("test").components().component(newMapperId).toRepresentation(); + Assert.assertEquals(GroupMapperConfig.DEFAULT_LDAP_GROUPS_PATH, newMapper.getConfig().getFirst(GroupMapperConfig.LDAP_GROUPS_PATH)); + } finally { + // revert new mapper + adminClient.realm("test").components().component(newMapperId).remove(); + } + } }