KEYCLOAK-14892 NullPointerException when group mappings for LDAP users are accessed

This commit is contained in:
mposolda 2020-07-28 13:03:27 +02:00 committed by Marek Posolda
parent 330a3d8ff5
commit c4fca5895f
3 changed files with 46 additions and 4 deletions

View file

@ -17,6 +17,7 @@
package org.keycloak.storage.ldap.mappers.membership.group; package org.keycloak.storage.ldap.mappers.membership.group;
import org.keycloak.common.util.ObjectUtil;
import org.keycloak.component.ComponentModel; import org.keycloak.component.ComponentModel;
import org.keycloak.component.ComponentValidationException; import org.keycloak.component.ComponentValidationException;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
@ -264,6 +265,7 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact
ComponentModel parentModel = realm.getComponent(model.getParentId()); ComponentModel parentModel = realm.getComponent(model.getParentId());
UserStorageProviderModel parent = new UserStorageProviderModel(parentModel); UserStorageProviderModel parent = new UserStorageProviderModel(parentModel);
onParentUpdate(realm, parent, parent, model); onParentUpdate(realm, parent, parent, model);
setDefaultGroupsPath(realm, model);
} }
@ -272,6 +274,14 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact
ComponentModel parentModel = realm.getComponent(newModel.getParentId()); ComponentModel parentModel = realm.getComponent(newModel.getParentId());
UserStorageProviderModel parent = new UserStorageProviderModel(parentModel); UserStorageProviderModel parent = new UserStorageProviderModel(parentModel);
onParentUpdate(realm, parent, parent, newModel); 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 @Override
@ -293,9 +303,8 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact
LDAPUtils.validateCustomLdapFilter(config.getConfig().getFirst(GroupMapperConfig.GROUPS_LDAP_FILTER)); LDAPUtils.validateCustomLdapFilter(config.getConfig().getFirst(GroupMapperConfig.GROUPS_LDAP_FILTER));
checkMandatoryConfigAttribute(GroupMapperConfig.LDAP_GROUPS_PATH, "Groups Path", config); String group = new GroupMapperConfig(config).getGroupsPath();
String group = config.getConfig().getFirst(GroupMapperConfig.LDAP_GROUPS_PATH).trim(); if (!GroupMapperConfig.DEFAULT_LDAP_GROUPS_PATH.equals(group) && KeycloakModelUtils.findGroupByPath(realm, group) == null) {
if (!"/".equals(group) && KeycloakModelUtils.findGroupByPath(realm, group) == null) {
throw new ComponentValidationException("ldapErrorMissingGroupsPathGroup"); throw new ComponentValidationException("ldapErrorMissingGroupsPathGroup");
} }
} }

View file

@ -17,6 +17,7 @@
package org.keycloak.storage.ldap.mappers.membership.group; package org.keycloak.storage.ldap.mappers.membership.group;
import org.keycloak.common.util.ObjectUtil;
import org.keycloak.component.ComponentModel; import org.keycloak.component.ComponentModel;
import org.keycloak.models.LDAPConstants; import org.keycloak.models.LDAPConstants;
import org.keycloak.models.ModelException; 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 "/") // 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 LDAP_GROUPS_PATH = "groups.path";
public static final String DEFAULT_LDAP_GROUPS_PATH = "/";
public GroupMapperConfig(ComponentModel mapperModel) { public GroupMapperConfig(ComponentModel mapperModel) {
super(mapperModel); super(mapperModel);
@ -129,7 +131,8 @@ public class GroupMapperConfig extends CommonLDAPGroupMapperConfig {
} }
public String getGroupsPath() { 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() { public String getGroupsPathWithTrailingSlash() {

View file

@ -17,6 +17,8 @@
package org.keycloak.testsuite.federation.ldap; package org.keycloak.testsuite.federation.ldap;
import javax.ws.rs.core.Response;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
import org.junit.ClassRule; import org.junit.ClassRule;
@ -28,6 +30,7 @@ import org.keycloak.models.GroupModel;
import org.keycloak.models.LDAPConstants; import org.keycloak.models.LDAPConstants;
import org.keycloak.models.RealmModel; import org.keycloak.models.RealmModel;
import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.representations.idm.ComponentRepresentation;
import org.keycloak.storage.ldap.LDAPStorageProvider; import org.keycloak.storage.ldap.LDAPStorageProvider;
import org.keycloak.storage.ldap.LDAPUtils; import org.keycloak.storage.ldap.LDAPUtils;
import org.keycloak.storage.ldap.idm.model.LDAPObject; 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.GroupLDAPStorageMapperFactory;
import org.keycloak.storage.ldap.mappers.membership.group.GroupMapperConfig; import org.keycloak.storage.ldap.mappers.membership.group.GroupMapperConfig;
import org.keycloak.storage.user.SynchronizationResult; import org.keycloak.storage.user.SynchronizationResult;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.util.LDAPRule; import org.keycloak.testsuite.util.LDAPRule;
import org.keycloak.testsuite.util.LDAPTestUtils; import org.keycloak.testsuite.util.LDAPTestUtils;
@ -206,4 +210,30 @@ public class LDAPGroupMapperSyncWithGroupsPathTest extends AbstractLDAPTest {
Assert.assertNotNull(KeycloakModelUtils.findGroupByPath(realm, "/outside")); 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();
}
}
} }