From ac00f7fee2007a9c0bcae54df903aac05f575706 Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 19 Dec 2016 15:35:45 +0100 Subject: [PATCH] KEYCLOAK-4087 LDAP group mapping should be possible via uidNumber in memberUid mode --- .../org/keycloak/storage/ldap/LDAPUtils.java | 20 +++++++---- .../mappers/AbstractLDAPStorageMapper.java | 1 + .../CommonLDAPGroupMapperConfig.java | 9 +++++ .../mappers/membership/MembershipType.java | 34 +++++++++++++++++-- .../membership/UserRolesRetrieveStrategy.java | 10 +++--- .../group/GroupLDAPStorageMapper.java | 29 ++++++++++++---- .../group/GroupLDAPStorageMapperFactory.java | 15 +++++++- .../membership/group/GroupMapperConfig.java | 5 --- .../role/RoleLDAPStorageMapper.java | 23 ++++++++++--- .../role/RoleLDAPStorageMapperFactory.java | 14 +++++++- .../resources/admin/UsersResource.java | 1 + .../storage/ldap/LDAPGroupMapperSyncTest.java | 8 ++--- .../storage/ldap/LDAPGroupMapperTest.java | 8 ++--- .../src/test/resources/log4j.properties | 5 ++- 14 files changed, 141 insertions(+), 41 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java index 3a391d3aa25..87754f59353 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java @@ -153,11 +153,12 @@ public class LDAPUtils { * @param ldapProvider * @param membershipType how is 'member' attribute saved (full DN or just uid) * @param memberAttrName usually 'member' + * @param memberChildAttrName used just if membershipType is UID. Usually 'uid' * @param ldapParent role or group * @param ldapChild usually user (or child group or child role) * @param sendLDAPUpdateRequest if true, the method will send LDAP update request too. Otherwise it will skip it */ - public static void addMember(LDAPStorageProvider ldapProvider, MembershipType membershipType, String memberAttrName, LDAPObject ldapParent, LDAPObject ldapChild, boolean sendLDAPUpdateRequest) { + public static void addMember(LDAPStorageProvider ldapProvider, MembershipType membershipType, String memberAttrName, String memberChildAttrName, LDAPObject ldapParent, LDAPObject ldapChild, boolean sendLDAPUpdateRequest) { Set memberships = getExistingMemberships(memberAttrName, ldapParent); @@ -171,7 +172,7 @@ public class LDAPUtils { } } - String membership = getMemberValueOfChildObject(ldapChild, membershipType); + String membership = getMemberValueOfChildObject(ldapChild, membershipType, memberChildAttrName); memberships.add(membership); ldapParent.setAttribute(memberAttrName, memberships); @@ -187,13 +188,14 @@ public class LDAPUtils { * @param ldapProvider * @param membershipType how is 'member' attribute saved (full DN or just uid) * @param memberAttrName usually 'member' + * @param memberChildAttrName used just if membershipType is UID. Usually 'uid' * @param ldapParent role or group * @param ldapChild usually user (or child group or child role) */ - public static void deleteMember(LDAPStorageProvider ldapProvider, MembershipType membershipType, String memberAttrName, LDAPObject ldapParent, LDAPObject ldapChild) { + public static void deleteMember(LDAPStorageProvider ldapProvider, MembershipType membershipType, String memberAttrName, String memberChildAttrName, LDAPObject ldapParent, LDAPObject ldapChild) { Set memberships = getExistingMemberships(memberAttrName, ldapParent); - String userMembership = getMemberValueOfChildObject(ldapChild, membershipType); + String userMembership = getMemberValueOfChildObject(ldapChild, membershipType, memberChildAttrName); memberships.remove(userMembership); @@ -222,10 +224,14 @@ public class LDAPUtils { } /** - * Get value to be used as attribute 'member' in some parent ldapObject + * Get value to be used as attribute 'member' or 'memberUid' in some parent ldapObject */ - public static String getMemberValueOfChildObject(LDAPObject ldapUser, MembershipType membershipType) { - return membershipType == MembershipType.DN ? ldapUser.getDn().toString() : ldapUser.getAttributeAsString(ldapUser.getRdnAttributeName()); + public static String getMemberValueOfChildObject(LDAPObject ldapUser, MembershipType membershipType, String memberChildAttrName) { + if (membershipType == MembershipType.DN) { + return ldapUser.getDn().toString(); + } else { + return ldapUser.getAttributeAsString(memberChildAttrName); + } } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/AbstractLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/AbstractLDAPStorageMapper.java index fb09d1df090..dedf8d83e6e 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/AbstractLDAPStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/AbstractLDAPStorageMapper.java @@ -22,6 +22,7 @@ import org.keycloak.models.GroupModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.storage.ldap.LDAPConfig; import org.keycloak.storage.ldap.LDAPStorageProvider; import org.keycloak.storage.ldap.idm.model.LDAPObject; import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery; diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/CommonLDAPGroupMapperConfig.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/CommonLDAPGroupMapperConfig.java index a4ff1756b4a..01bcbb13461 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/CommonLDAPGroupMapperConfig.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/CommonLDAPGroupMapperConfig.java @@ -20,6 +20,7 @@ package org.keycloak.storage.ldap.mappers.membership; import org.keycloak.component.ComponentModel; import org.keycloak.models.LDAPConstants; import org.keycloak.models.ModelException; +import org.keycloak.storage.ldap.LDAPConfig; import java.util.HashSet; import java.util.Set; @@ -35,6 +36,9 @@ public abstract class CommonLDAPGroupMapperConfig { // See docs for MembershipType enum public static final String MEMBERSHIP_ATTRIBUTE_TYPE = "membership.attribute.type"; + // Used just for membershipType=UID. Name of LDAP attribute on user, which is used for membership mappings. Usually it will be "uid" + public static final String MEMBERSHIP_USER_LDAP_ATTRIBUTE = "membership.user.ldap.attribute"; + // See docs for Mode enum public static final String MODE = "mode"; @@ -58,6 +62,11 @@ public abstract class CommonLDAPGroupMapperConfig { return (membershipType!=null && !membershipType.isEmpty()) ? Enum.valueOf(MembershipType.class, membershipType) : MembershipType.DN; } + public String getMembershipUserLdapAttribute(LDAPConfig ldapConfig) { + String membershipUserAttrName = mapperModel.getConfig().getFirst(MEMBERSHIP_USER_LDAP_ATTRIBUTE); + return membershipUserAttrName!=null ? membershipUserAttrName : ldapConfig.getUsernameLdapAttribute(); + } + public LDAPGroupMapperMode getMode() { String modeString = mapperModel.getConfig().getFirst(MODE); if (modeString == null || modeString.isEmpty()) { diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/MembershipType.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/MembershipType.java index f2df5cd49fe..894b2b4d449 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/MembershipType.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/MembershipType.java @@ -57,7 +57,7 @@ public enum MembershipType { protected Set getLDAPMembersWithParent(LDAPObject ldapGroup, String membershipLdapAttribute, LDAPDn requiredParentDn) { Set allMemberships = LDAPUtils.getExistingMemberships(membershipLdapAttribute, ldapGroup); - // Filter and keep just groups + // Filter and keep just descendants of requiredParentDn Set result = new HashSet<>(); for (String membership : allMemberships) { LDAPDn childDn = LDAPDn.fromString(membership); @@ -135,6 +135,9 @@ public enum MembershipType { @Override public List getGroupMembers(RealmModel realm, GroupLDAPStorageMapper groupMapper, LDAPObject ldapGroup, int firstResult, int maxResults) { + LDAPStorageProvider ldapProvider = groupMapper.getLdapProvider(); + LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig(); + String memberAttrName = groupMapper.getConfig().getMembershipLdapAttribute(); Set memberUids = LDAPUtils.getExistingMemberships(memberAttrName, ldapGroup); @@ -146,7 +149,34 @@ public enum MembershipType { int max = Math.min(memberUids.size(), firstResult + maxResults); uids = uids.subList(firstResult, max); - return groupMapper.getLdapProvider().loadUsersByUsernames(uids, realm); + String membershipUserAttrName = groupMapper.getConfig().getMembershipUserLdapAttribute(ldapConfig); + + List usernames; + if (membershipUserAttrName.equals(ldapConfig.getUsernameLdapAttribute())) { + usernames = uids; // Optimized version. No need to + } else { + usernames = new LinkedList<>(); + + LDAPQuery query = LDAPUtils.createQueryForUserSearch(ldapProvider, realm); + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + + Condition[] orSubconditions = new Condition[uids.size()]; + int index = 0; + for (String memberUid : uids) { + Condition condition = conditionsBuilder.equal(membershipUserAttrName, memberUid, EscapeStrategy.DEFAULT); + orSubconditions[index] = condition; + index++; + } + Condition orCondition = conditionsBuilder.orCondition(orSubconditions); + query.addWhereCondition(orCondition); + List ldapUsers = query.getResultList(); + for (LDAPObject ldapUser : ldapUsers) { + String username = LDAPUtils.getUsername(ldapUser, ldapConfig); + usernames.add(username); + } + } + + return groupMapper.getLdapProvider().loadUsersByUsernames(usernames, realm); } }; diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/UserRolesRetrieveStrategy.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/UserRolesRetrieveStrategy.java index 29dd1d42430..3ce4a42532b 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/UserRolesRetrieveStrategy.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/UserRolesRetrieveStrategy.java @@ -19,6 +19,7 @@ package org.keycloak.storage.ldap.mappers.membership; import org.keycloak.models.LDAPConstants; +import org.keycloak.storage.ldap.LDAPConfig; import org.keycloak.storage.ldap.LDAPUtils; import org.keycloak.storage.ldap.idm.model.LDAPDn; import org.keycloak.storage.ldap.idm.model.LDAPObject; @@ -39,7 +40,7 @@ import java.util.Set; public interface UserRolesRetrieveStrategy { - List getLDAPRoleMappings(CommonLDAPGroupMapper roleOrGroupMapper, LDAPObject ldapUser); + List getLDAPRoleMappings(CommonLDAPGroupMapper roleOrGroupMapper, LDAPObject ldapUser, LDAPConfig ldapConfig); void beforeUserLDAPQuery(LDAPQuery query); @@ -52,11 +53,12 @@ public interface UserRolesRetrieveStrategy { class LoadRolesByMember implements UserRolesRetrieveStrategy { @Override - public List getLDAPRoleMappings(CommonLDAPGroupMapper roleOrGroupMapper, LDAPObject ldapUser) { + public List getLDAPRoleMappings(CommonLDAPGroupMapper roleOrGroupMapper, LDAPObject ldapUser, LDAPConfig ldapConfig) { LDAPQuery ldapQuery = roleOrGroupMapper.createLDAPGroupQuery(); String membershipAttr = roleOrGroupMapper.getConfig().getMembershipLdapAttribute(); - String userMembership = LDAPUtils.getMemberValueOfChildObject(ldapUser, roleOrGroupMapper.getConfig().getMembershipTypeLdapAttribute()); + String membershipUserAttrName = roleOrGroupMapper.getConfig().getMembershipUserLdapAttribute(ldapConfig); + String userMembership = LDAPUtils.getMemberValueOfChildObject(ldapUser, roleOrGroupMapper.getConfig().getMembershipTypeLdapAttribute(), membershipUserAttrName); Condition membershipCondition = getMembershipCondition(membershipAttr, userMembership); ldapQuery.addWhereCondition(membershipCondition); @@ -79,7 +81,7 @@ public interface UserRolesRetrieveStrategy { class GetRolesFromUserMemberOfAttribute implements UserRolesRetrieveStrategy { @Override - public List getLDAPRoleMappings(CommonLDAPGroupMapper roleOrGroupMapper, LDAPObject ldapUser) { + public List getLDAPRoleMappings(CommonLDAPGroupMapper roleOrGroupMapper, LDAPObject ldapUser, LDAPConfig ldapConfig) { Set memberOfValues = ldapUser.getAttributeAsSet(LDAPConstants.MEMBER_OF); if (memberOfValues == null) { return Collections.emptyList(); diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java index fe4930caf8f..fcdae3c012f 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java @@ -27,6 +27,7 @@ import org.keycloak.models.UserModel; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.RoleUtils; import org.keycloak.models.utils.UserModelDelegate; +import org.keycloak.storage.ldap.LDAPConfig; import org.keycloak.storage.ldap.LDAPStorageProvider; import org.keycloak.storage.ldap.LDAPUtils; import org.keycloak.storage.ldap.idm.model.LDAPDn; @@ -450,11 +451,13 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements LDAPObject ldapGroup = ldapGroupsMap.get(kcGroup.getName()); Set toRemoveSubgroupsDNs = getLDAPSubgroups(ldapGroup); + String membershipUserLdapAttrName = getMembershipUserLdapAttribute(); // Not applicable for groups, but needs to be here + // Add LDAP subgroups, which are KC subgroups Set kcSubgroups = kcGroup.getSubGroups(); for (GroupModel kcSubgroup : kcSubgroups) { LDAPObject ldapSubgroup = ldapGroupsMap.get(kcSubgroup.getName()); - LDAPUtils.addMember(ldapProvider, MembershipType.DN, config.getMembershipLdapAttribute(), ldapGroup, ldapSubgroup, false); + LDAPUtils.addMember(ldapProvider, MembershipType.DN, config.getMembershipLdapAttribute(), membershipUserLdapAttrName, ldapGroup, ldapSubgroup, false); toRemoveSubgroupsDNs.remove(ldapSubgroup.getDn()); } @@ -462,7 +465,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements for (LDAPDn toRemoveDN : toRemoveSubgroupsDNs) { LDAPObject fakeGroup = new LDAPObject(); fakeGroup.setDn(toRemoveDN); - LDAPUtils.deleteMember(ldapProvider, MembershipType.DN, config.getMembershipLdapAttribute(), ldapGroup, fakeGroup); + LDAPUtils.deleteMember(ldapProvider, MembershipType.DN, config.getMembershipLdapAttribute(), membershipUserLdapAttrName, ldapGroup, fakeGroup); } // Update group to LDAP @@ -497,17 +500,22 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements ldapGroup = loadLDAPGroupByName(groupName); } - LDAPUtils.addMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), ldapGroup, ldapUser, true); + String membershipUserLdapAttrName = getMembershipUserLdapAttribute(); + + LDAPUtils.addMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), membershipUserLdapAttrName, ldapGroup, ldapUser, true); } public void deleteGroupMappingInLDAP(LDAPObject ldapUser, LDAPObject ldapGroup) { - LDAPUtils.deleteMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), ldapGroup, ldapUser); + String membershipUserLdapAttrName = getMembershipUserLdapAttribute(); + LDAPUtils.deleteMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), membershipUserLdapAttrName, ldapGroup, ldapUser); } protected List getLDAPGroupMappings(LDAPObject ldapUser) { String strategyKey = config.getUserGroupsRetrieveStrategy(); UserRolesRetrieveStrategy strategy = factory.getUserGroupsRetrieveStrategy(strategyKey); - return strategy.getLDAPRoleMappings(this, ldapUser); + + LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig(); + return strategy.getLDAPRoleMappings(this, ldapUser, ldapConfig); } @Override @@ -555,6 +563,12 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements } + protected String getMembershipUserLdapAttribute() { + LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig(); + return config.getMembershipUserLdapAttribute(ldapConfig); + } + + public class LDAPGroupMappingsUserDelegate extends UserModelDelegate { private final RealmModel realm; @@ -604,8 +618,11 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements LDAPQuery ldapQuery = createGroupQuery(); LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); Condition roleNameCondition = conditionsBuilder.equal(config.getGroupNameLdapAttribute(), group.getName()); - String membershipUserAttr = LDAPUtils.getMemberValueOfChildObject(ldapUser, config.getMembershipTypeLdapAttribute()); + + String membershipUserLdapAttrName = getMembershipUserLdapAttribute(); + String membershipUserAttr = LDAPUtils.getMemberValueOfChildObject(ldapUser, config.getMembershipTypeLdapAttribute(), membershipUserLdapAttrName); Condition membershipCondition = conditionsBuilder.equal(config.getMembershipLdapAttribute(), membershipUserAttr); + ldapQuery.addWhereCondition(roleNameCondition).addWhereCondition(membershipCondition); LDAPObject ldapGroup = ldapQuery.getFirstResult(); 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 3f018b70222..3473372631b 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 @@ -34,6 +34,7 @@ import org.keycloak.storage.ldap.mappers.membership.CommonLDAPGroupMapperConfig; import org.keycloak.storage.ldap.mappers.membership.LDAPGroupMapperMode; import org.keycloak.storage.ldap.mappers.membership.MembershipType; import org.keycloak.storage.ldap.mappers.membership.UserRolesRetrieveStrategy; +import org.keycloak.storage.ldap.mappers.membership.role.RoleMapperConfig; import java.util.HashMap; import java.util.LinkedHashMap; @@ -74,11 +75,14 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact private static List getProps(ComponentModel parent) { String roleObjectClasses = LDAPConstants.GROUP_OF_NAMES; String mode = LDAPGroupMapperMode.LDAP_ONLY.toString(); + String membershipUserAttribute = LDAPConstants.UID; if (parent != null) { LDAPConfig config = new LDAPConfig(parent.getConfig()); roleObjectClasses = config.isActiveDirectory() ? LDAPConstants.GROUP : LDAPConstants.GROUP_OF_NAMES; mode = config.getEditMode() == UserStorageProvider.EditMode.WRITABLE ? LDAPGroupMapperMode.LDAP_ONLY.toString() : LDAPGroupMapperMode.READ_ONLY.toString(); + membershipUserAttribute = config.getUsernameLdapAttribute(); } + return ProviderConfigurationBuilder.create() .property().name(GroupMapperConfig.GROUPS_DN) .label("LDAP Groups DN") @@ -106,7 +110,8 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact .add() .property().name(GroupMapperConfig.MEMBERSHIP_LDAP_ATTRIBUTE) .label("Membership LDAP Attribute") - .helpText("Name of LDAP attribute on group, which is used for membership mappings. Usually it will be 'member' ") + .helpText("Name of LDAP attribute on group, which is used for membership mappings. Usually it will be 'member' ." + + "However when 'Membership Attribute Type' is 'UID' then 'Membership LDAP Attribute' could be typically 'memberUid' .") .type(ProviderConfigProperty.STRING_TYPE) .defaultValue(LDAPConstants.MEMBER) .add() @@ -118,6 +123,14 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact .options(MEMBERSHIP_TYPES) .defaultValue(MembershipType.DN.toString()) .add() + .property().name(RoleMapperConfig.MEMBERSHIP_USER_LDAP_ATTRIBUTE) + .label("Membership User LDAP Attribute") + .helpText("Used just if Membership Attribute Type is UID. It is name of LDAP attribute on user, which is used for membership mappings. Usually it will be 'uid' . For example if value of " + + "'Membership User LDAP Attribute' is 'uid' and " + + " LDAP group has 'memberUid: john', then it is expected that particular LDAP user will have attribute 'uid: john' .") + .type(ProviderConfigProperty.STRING_TYPE) + .defaultValue(membershipUserAttribute) + .add() .property().name(GroupMapperConfig.GROUPS_LDAP_FILTER) .label("LDAP Filter") .helpText("LDAP Filter adds additional custom filter to the whole query for retrieve LDAP groups. Leave this empty if no additional filtering is needed and you want to retrieve all groups from LDAP. Otherwise make sure that filter starts with '(' and ends with ')'") 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 99cde4de7d4..b305cb4eb53 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 @@ -90,11 +90,6 @@ public class GroupMapperConfig extends CommonLDAPGroupMapperConfig { return AbstractLDAPStorageMapper.parseBooleanParameter(mapperModel, PRESERVE_GROUP_INHERITANCE); } - public String getMembershipLdapAttribute() { - String membershipAttrName = mapperModel.getConfig().getFirst(MEMBERSHIP_LDAP_ATTRIBUTE); - return membershipAttrName!=null ? membershipAttrName : LDAPConstants.MEMBER; - } - public Collection getGroupObjectClasses(LDAPStorageProvider ldapProvider) { String objectClasses = mapperModel.getConfig().getFirst(GROUP_OBJECT_CLASSES); if (objectClasses == null) { diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java index fd788772220..1a4a5f90564 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java @@ -27,6 +27,7 @@ import org.keycloak.models.RoleModel; import org.keycloak.models.UserModel; import org.keycloak.models.utils.RoleUtils; import org.keycloak.models.utils.UserModelDelegate; +import org.keycloak.storage.ldap.LDAPConfig; import org.keycloak.storage.ldap.LDAPStorageProvider; import org.keycloak.storage.ldap.LDAPUtils; import org.keycloak.storage.ldap.idm.model.LDAPObject; @@ -252,11 +253,14 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements ldapRole = createLDAPRole(roleName); } - LDAPUtils.addMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), ldapRole, ldapUser, true); + String membershipUserAttrName = getMembershipUserLdapAttribute(); + + LDAPUtils.addMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), membershipUserAttrName, ldapRole, ldapUser, true); } public void deleteRoleMappingInLDAP(LDAPObject ldapUser, LDAPObject ldapRole) { - LDAPUtils.deleteMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), ldapRole, ldapUser); + String membershipUserAttrName = getMembershipUserLdapAttribute(); + LDAPUtils.deleteMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), membershipUserAttrName, ldapRole, ldapUser); } public LDAPObject loadLDAPRoleByName(String roleName) { @@ -269,7 +273,9 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements protected List getLDAPRoleMappings(LDAPObject ldapUser) { String strategyKey = config.getUserRolesRetrieveStrategy(); UserRolesRetrieveStrategy strategy = factory.getUserRolesRetrieveStrategy(strategyKey); - return strategy.getLDAPRoleMappings(this, ldapUser); + + LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig(); + return strategy.getLDAPRoleMappings(this, ldapUser, ldapConfig); } @Override @@ -292,6 +298,11 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements } + protected String getMembershipUserLdapAttribute() { + LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig(); + return config.getMembershipUserLdapAttribute(ldapConfig); + } + public class LDAPRoleMappingsUserDelegate extends UserModelDelegate { @@ -422,8 +433,12 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements LDAPQuery ldapQuery = createRoleQuery(); LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); Condition roleNameCondition = conditionsBuilder.equal(config.getRoleNameLdapAttribute(), role.getName()); - String membershipUserAttr = LDAPUtils.getMemberValueOfChildObject(ldapUser, config.getMembershipTypeLdapAttribute()); + + String membershipUserAttrName = getMembershipUserLdapAttribute(); + String membershipUserAttr = LDAPUtils.getMemberValueOfChildObject(ldapUser, config.getMembershipTypeLdapAttribute(), membershipUserAttrName); + Condition membershipCondition = conditionsBuilder.equal(config.getMembershipLdapAttribute(), membershipUserAttr); + ldapQuery.addWhereCondition(roleNameCondition).addWhereCondition(membershipCondition); LDAPObject ldapRole = ldapQuery.getFirstResult(); diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapperFactory.java index f5c8dfe7527..2af7116419c 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapperFactory.java @@ -74,11 +74,14 @@ public class RoleLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFacto private static List getProps(ComponentModel parent) { String roleObjectClasses = LDAPConstants.GROUP_OF_NAMES; String mode = LDAPGroupMapperMode.LDAP_ONLY.toString(); + String membershipUserAttribute = LDAPConstants.UID; if (parent != null) { LDAPConfig config = new LDAPConfig(parent.getConfig()); roleObjectClasses = config.isActiveDirectory() ? LDAPConstants.GROUP : LDAPConstants.GROUP_OF_NAMES; mode = config.getEditMode() == UserStorageProvider.EditMode.WRITABLE ? LDAPGroupMapperMode.LDAP_ONLY.toString() : LDAPGroupMapperMode.READ_ONLY.toString(); + membershipUserAttribute = config.getUsernameLdapAttribute(); } + return ProviderConfigurationBuilder.create() .property().name(RoleMapperConfig.ROLES_DN) .label("LDAP Roles DN") @@ -99,7 +102,8 @@ public class RoleLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFacto .add() .property().name(RoleMapperConfig.MEMBERSHIP_LDAP_ATTRIBUTE) .label("Membership LDAP Attribute") - .helpText("Name of LDAP attribute on role, which is used for membership mappings. Usually it will be 'member' ") + .helpText("Name of LDAP attribute on role, which is used for membership mappings. Usually it will be 'member' ." + + "However when 'Membership Attribute Type' is 'UID' then 'Membership LDAP Attribute' could be typically 'memberUid' .") .type(ProviderConfigProperty.STRING_TYPE) .defaultValue(LDAPConstants.MEMBER) .add() @@ -111,6 +115,14 @@ public class RoleLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFacto .options(MEMBERSHIP_TYPES) .defaultValue(MembershipType.DN.toString()) .add() + .property().name(RoleMapperConfig.MEMBERSHIP_USER_LDAP_ATTRIBUTE) + .label("Membership User LDAP Attribute") + .helpText("Used just if Membership Attribute Type is UID. It is name of LDAP attribute on user, which is used for membership mappings. Usually it will be 'uid' . For example if value of " + + "'Membership User LDAP Attribute' is 'uid' and " + + " LDAP group has 'memberUid: john', then it is expected that particular LDAP user will have attribute 'uid: john' .") + .type(ProviderConfigProperty.STRING_TYPE) + .defaultValue(membershipUserAttribute) + .add() .property().name(RoleMapperConfig.ROLES_LDAP_FILTER) .label("LDAP Filter") .helpText("LDAP Filter adds additional custom filter to the whole query for retrieve LDAP roles. Leave this empty if no additional filtering is needed and you want to retrieve all roles from LDAP. Otherwise make sure that filter starts with '(' and ends with ')'") diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java index 49e719b8316..d1f1d3ab69e 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java @@ -233,6 +233,7 @@ public class UsersResource { if (session.getTransactionManager().isActive()) { session.getTransactionManager().setRollbackOnly(); } + logger.warn("Could not create user", me); return ErrorResponse.exists("Could not create user"); } } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperSyncTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperSyncTest.java index 29e79c45fd5..746b3021ee9 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperSyncTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperSyncTest.java @@ -104,8 +104,8 @@ public class LDAPGroupMapperSyncTest { LDAPObject group11 = LDAPTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group11"); LDAPObject group12 = LDAPTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group12", descriptionAttrName, "group12 - description"); - LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, group1, group11, false); - LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, group1, group12, true); + LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", group1, group11, false); + LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", group1, group12, true); } }); @@ -144,7 +144,7 @@ public class LDAPGroupMapperSyncTest { // Add recursive group mapping to LDAP. Check that sync with preserve group inheritance will fail LDAPObject group1 = groupMapper.loadLDAPGroupByName("group1"); LDAPObject group12 = groupMapper.loadLDAPGroupByName("group12"); - LDAPUtils.addMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, group12, group1, true); + LDAPUtils.addMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", group12, group1, true); try { new GroupLDAPStorageMapperFactory().create(session, mapperModel).syncDataFromFederationProviderToKeycloak(realm); @@ -171,7 +171,7 @@ public class LDAPGroupMapperSyncTest { Assert.assertEquals("group12 - description", kcGroup12.getFirstAttribute(descriptionAttrName)); // Cleanup - remove recursive mapping in LDAP - LDAPUtils.deleteMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, group12, group1); + LDAPUtils.deleteMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", group12, group1); } finally { keycloakRule.stopSession(session, false); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperTest.java index 022a0760f2b..e36f8e855d6 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperTest.java @@ -111,8 +111,8 @@ public class LDAPGroupMapperTest { LDAPObject group12 = LDAPTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group12", descriptionAttrName, "group12 - description"); LDAPObject groupSpecialCharacters = LDAPTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group-spec,ia*l_characžter)s", descriptionAttrName, "group-special-characters"); - LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, group1, group11, false); - LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, group1, group12, true); + LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", group1, group11, false); + LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", group1, group12, true); // Sync LDAP groups to Keycloak DB ComponentModel mapperModel = LDAPTestUtils.getSubcomponentByName(appRealm, ldapModel, "groupsMapper"); @@ -366,14 +366,14 @@ public class LDAPGroupMapperTest { // 2 - Add one existing user rob to LDAP group LDAPObject jamesLdap = ldapProvider.loadLDAPUserByUsername(appRealm, "jameskeycloak"); - LDAPUtils.addMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, group2, jamesLdap, false); + LDAPUtils.addMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", group2, jamesLdap, false); // 3 - Add non-existing user to LDAP group LDAPDn nonExistentDn = LDAPDn.fromString(ldapProvider.getLdapIdentityStore().getConfig().getUsersDn()); nonExistentDn.addFirst(jamesLdap.getRdnAttributeName(), "nonexistent"); LDAPObject nonExistentLdapUser = new LDAPObject(); nonExistentLdapUser.setDn(nonExistentDn); - LDAPUtils.addMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, group2, nonExistentLdapUser, true); + LDAPUtils.addMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, "not-used", group2, nonExistentLdapUser, true); // 4 - Check group members. Just existing user rob should be present groupMapper.syncDataFromFederationProviderToKeycloak(appRealm); diff --git a/testsuite/integration/src/test/resources/log4j.properties b/testsuite/integration/src/test/resources/log4j.properties index b7922eec8b4..22a890dafb6 100755 --- a/testsuite/integration/src/test/resources/log4j.properties +++ b/testsuite/integration/src/test/resources/log4j.properties @@ -58,7 +58,7 @@ log4j.logger.org.keycloak.connections.jpa.DefaultJpaConnectionProviderFactory=${ log4j.logger.org.keycloak.connections.jpa.HibernateStatsReporter=debug # Enable to view ldap logging -# log4j.logger.org.keycloak.federation.ldap=trace +# log4j.logger.org.keycloak.storage.ldap=trace # Enable to view kerberos/spnego logging # log4j.logger.org.keycloak.federation.kerberos=trace @@ -78,5 +78,4 @@ log4j.logger.org.apache.directory.server.ldap.LdapProtocolHandler=error #log4j.logger.org.apache.http.impl.conn=debug # Enable to view details from identity provider authenticator -# log4j.logger.org.keycloak.authentication.authenticators.browser.IdentityProviderAuthenticator=trace -log4j.logger.org.keycloak.storage.ldap.mappers.UserAttributeLDAPStorageMapper=debug \ No newline at end of file +# log4j.logger.org.keycloak.authentication.authenticators.browser.IdentityProviderAuthenticator=trace \ No newline at end of file