From 7336ff07acc09354e1940555fc3a5051a403e0ea Mon Sep 17 00:00:00 2001 From: rmartinc Date: Tue, 18 Jul 2023 08:52:45 +0200 Subject: [PATCH] Check RDN attribute for DN membership Closes https://github.com/keycloak/keycloak/issues/20718 --- .../ldap/mappers/membership/MembershipType.java | 16 +++++++--------- .../federation/ldap/LDAPGroupMapperSyncTest.java | 6 ++++++ 2 files changed, 13 insertions(+), 9 deletions(-) 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 68f1e54413..e3e1194e3c 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 @@ -50,18 +50,20 @@ public enum MembershipType { @Override public Set getLDAPSubgroups(CommonLDAPGroupMapper groupMapper, LDAPObject ldapGroup) { CommonLDAPGroupMapperConfig config = groupMapper.getConfig(); - return getLDAPMembersWithParent(groupMapper.getLdapProvider(), ldapGroup, config.getMembershipLdapAttribute(), LDAPDn.fromString(config.getLDAPGroupsDn())); + return getLDAPMembersWithParent(groupMapper.getLdapProvider(), ldapGroup, config.getMembershipLdapAttribute(), + LDAPDn.fromString(config.getLDAPGroupsDn()), config.getLDAPGroupNameLdapAttribute()); } // Get just those members of specified group, which are descendants of "requiredParentDn" - protected Set getLDAPMembersWithParent(LDAPStorageProvider ldapProvider, LDAPObject ldapGroup, String membershipLdapAttribute, LDAPDn requiredParentDn) { + protected Set getLDAPMembersWithParent(LDAPStorageProvider ldapProvider, LDAPObject ldapGroup, + String membershipLdapAttribute, LDAPDn requiredParentDn, String rdnAttr) { Set allMemberships = LDAPUtils.getExistingMemberships(ldapProvider, membershipLdapAttribute, ldapGroup); // Filter and keep just descendants of requiredParentDn Set result = new HashSet<>(); for (String membership : allMemberships) { LDAPDn childDn = LDAPDn.fromString(membership); - if (childDn.isDescendantOf(requiredParentDn)) { + if (childDn.getFirstRdn().getAttrValue(rdnAttr) != null && childDn.isDescendantOf(requiredParentDn)) { result.add(childDn); } } @@ -73,8 +75,9 @@ public enum MembershipType { LDAPStorageProvider ldapProvider = groupMapper.getLdapProvider(); CommonLDAPGroupMapperConfig config = groupMapper.getConfig(); + LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig(); LDAPDn usersDn = LDAPDn.fromString(ldapProvider.getLdapIdentityStore().getConfig().getUsersDn()); - Set userDns = getLDAPMembersWithParent(ldapProvider, ldapGroup, config.getMembershipLdapAttribute(), usersDn); + Set userDns = getLDAPMembersWithParent(ldapProvider, ldapGroup, config.getMembershipLdapAttribute(), usersDn, ldapConfig.getRdnLdapAttribute()); if (userDns == null) { return Collections.emptyList(); @@ -90,14 +93,9 @@ public enum MembershipType { // If usernameAttrName is same like DN, we can just retrieve usernames from DNs List usernames = new LinkedList<>(); - LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig(); if (ldapConfig.getUsernameLdapAttribute().equals(ldapConfig.getRdnLdapAttribute())) { for (LDAPDn userDn : dns) { String username = userDn.getFirstRdn().getAttrValue(ldapConfig.getRdnLdapAttribute()); - if (username == null) { - throw new ModelException("User returned from LDAP has null username! Check configuration of your LDAP mappings. Mapped username LDAP attribute: " + - ldapConfig.getRdnLdapAttribute() + ", user DN: " + userDn + ", attributes from LDAP: " + userDn.getFirstRdn().getAllKeys()); - } usernames.add(username); } } else { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncTest.java index 44f2c0f0b3..6bb4320273 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncTest.java @@ -36,6 +36,7 @@ import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.representations.idm.SynchronizationResultRepresentation; import org.keycloak.storage.ldap.LDAPStorageProvider; import org.keycloak.storage.ldap.LDAPUtils; +import org.keycloak.storage.ldap.idm.model.LDAPDn; import org.keycloak.storage.ldap.idm.model.LDAPObject; import org.keycloak.storage.ldap.mappers.membership.LDAPGroupMapperMode; import org.keycloak.storage.ldap.mappers.membership.MembershipType; @@ -94,6 +95,11 @@ public class LDAPGroupMapperSyncTest extends AbstractLDAPTest { LDAPUtils.addMember(ctx.getLdapProvider(), MembershipType.DN, LDAPConstants.MEMBER, "not-used", group1, group11); LDAPUtils.addMember(ctx.getLdapProvider(), MembershipType.DN, LDAPConstants.MEMBER, "not-used", group1, group12); + LDAPObject nonExistentChild = new LDAPObject(); + LDAPDn nonExistentChildDn = group1.getDn().getParentDn(); + nonExistentChildDn.addFirst(LDAPConstants.UID, "non-existent-child"); + nonExistentChild.setDn(nonExistentChildDn); + LDAPUtils.addMember(ctx.getLdapProvider(), MembershipType.DN, LDAPConstants.MEMBER, "not-used", group1, nonExistentChild); }); }