KEYCLOAK-2154 Group mapper fixes

federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java

@@ -206,25 +206,9 @@ public class LDAPFederationProvider implements UserFederationProvider {
         return Collections.emptyList();
     }
 
-    public List<UserModel> loadUsersByLDAPDns(Collection<LDAPDn> userDns, RealmModel realm) {
+    public List<UserModel> loadUsersByUsernames(List<String> usernames, RealmModel realm) {
-        // We have dns of users, who are members of our group. Load them now
+        List<UserModel> result = new ArrayList<>();
-        LDAPQuery query = LDAPUtils.createQueryForUserSearch(this, realm);
+        for (String username : usernames) {
-        LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder();
-        Condition[] orSubconditions = new Condition[userDns.size()];
-        int index = 0;
-        for (LDAPDn userDn : userDns) {
-            Condition condition = conditionsBuilder.equal(userDn.getFirstRdnAttrName(), userDn.getFirstRdnAttrValue());
-            orSubconditions[index] = condition;
-            index++;
-        }
-        Condition orCondition = conditionsBuilder.orCondition(orSubconditions);
-        query.addWhereCondition(orCondition);
-        List<LDAPObject> ldapUsers = query.getResultList();
-
-        // We have ldapUsers, Need to load users from KC DB or import them here
-        List<UserModel> result = new LinkedList<>();
-        for (LDAPObject ldapUser : ldapUsers) {
-            String username = LDAPUtils.getUsername(ldapUser, getLdapIdentityStore().getConfig());
             UserModel kcUser = session.users().getUserByUsername(username, realm);
             if (!model.getId().equals(kcUser.getFederationLink())) {
                 logger.warnf("Incorrect federation provider of user %s" + kcUser.getUsername());

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/AbstractLDAPFederationMapper.java

@@ -75,4 +75,12 @@ public abstract class AbstractLDAPFederationMapper {
         String paramm = mapperModel.getConfig().get(paramName);
         return Boolean.parseBoolean(paramm);
     }
+
+    public LDAPFederationProvider getLdapProvider() {
+        return ldapProvider;
+    }
+
+    public RealmModel getRealm() {
+        return realm;
+    }
 }

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/MembershipType.java

@@ -1,5 +1,24 @@
 package org.keycloak.federation.ldap.mappers.membership;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Set;
+
+import org.keycloak.federation.ldap.LDAPConfig;
+import org.keycloak.federation.ldap.LDAPFederationProvider;
+import org.keycloak.federation.ldap.LDAPUtils;
+import org.keycloak.federation.ldap.idm.model.LDAPDn;
+import org.keycloak.federation.ldap.idm.model.LDAPObject;
+import org.keycloak.federation.ldap.idm.query.Condition;
+import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery;
+import org.keycloak.federation.ldap.idm.query.internal.LDAPQueryConditionsBuilder;
+import org.keycloak.federation.ldap.mappers.membership.group.GroupLDAPFederationMapper;
+import org.keycloak.models.RealmModel;
+import org.keycloak.models.UserModel;
+
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
@@ -8,10 +27,114 @@ public enum MembershipType {
     /**
      * Used if LDAP role has it's members declared in form of their full DN. For example ( "member: uid=john,ou=users,dc=example,dc=com" )
      */
-    DN,
+    DN {
+
+        @Override
+        public Set<LDAPDn> getLDAPSubgroups(GroupLDAPFederationMapper groupMapper, LDAPObject ldapGroup) {
+            CommonLDAPGroupMapperConfig config = groupMapper.getConfig();
+            return getLDAPMembersWithParent(ldapGroup, config.getMembershipLdapAttribute(), LDAPDn.fromString(config.getLDAPGroupsDn()));
+        }
+
+        // Get just those members of specified group, which are descendants of "requiredParentDn"
+        protected Set<LDAPDn> getLDAPMembersWithParent(LDAPObject ldapGroup, String membershipLdapAttribute, LDAPDn requiredParentDn) {
+            Set<String> allMemberships = LDAPUtils.getExistingMemberships(membershipLdapAttribute, ldapGroup);
+
+            // Filter and keep just groups
+            Set<LDAPDn> result = new HashSet<>();
+            for (String membership : allMemberships) {
+                LDAPDn childDn = LDAPDn.fromString(membership);
+                if (childDn.isDescendantOf(requiredParentDn)) {
+                    result.add(childDn);
+                }
+            }
+            return result;
+        }
+
+        @Override
+        public List<UserModel> getGroupMembers(GroupLDAPFederationMapper groupMapper, LDAPObject ldapGroup, int firstResult, int maxResults) {
+            RealmModel realm = groupMapper.getRealm();
+            LDAPFederationProvider ldapProvider = groupMapper.getLdapProvider();
+            CommonLDAPGroupMapperConfig config = groupMapper.getConfig();
+
+            LDAPDn usersDn = LDAPDn.fromString(ldapProvider.getLdapIdentityStore().getConfig().getUsersDn());
+            Set<LDAPDn> userDns = getLDAPMembersWithParent(ldapGroup, config.getMembershipLdapAttribute(), usersDn);
+
+            if (userDns == null) {
+                return Collections.emptyList();
+            }
+
+            if (userDns.size() <= firstResult) {
+                return Collections.emptyList();
+            }
+
+            List<LDAPDn> dns = new ArrayList<>(userDns);
+            int max = Math.min(dns.size(), firstResult + maxResults);
+            dns = dns.subList(firstResult, max);
+
+            // If usernameAttrName is same like DN, we can just retrieve usernames from DNs
+            List<String> usernames = new LinkedList<>();
+            LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig();
+            if (ldapConfig.getUsernameLdapAttribute().equals(ldapConfig.getRdnLdapAttribute())) {
+                for (LDAPDn userDn : dns) {
+                    String username = userDn.getFirstRdnAttrValue();
+                    usernames.add(username);
+                }
+            } else {
+                LDAPQuery query = LDAPUtils.createQueryForUserSearch(ldapProvider, realm);
+                LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder();
+                Condition[] orSubconditions = new Condition[dns.size()];
+                int index = 0;
+                for (LDAPDn userDn : dns) {
+                    Condition condition = conditionsBuilder.equal(userDn.getFirstRdnAttrName(), userDn.getFirstRdnAttrValue());
+                    orSubconditions[index] = condition;
+                    index++;
+                }
+                Condition orCondition = conditionsBuilder.orCondition(orSubconditions);
+                query.addWhereCondition(orCondition);
+                List<LDAPObject> ldapUsers = query.getResultList();
+                for (LDAPObject ldapUser : ldapUsers) {
+                    String username = LDAPUtils.getUsername(ldapUser, ldapConfig);
+                    usernames.add(username);
+                }
+            }
+
+            // We have dns of users, who are members of our group. Load them now
+            return ldapProvider.loadUsersByUsernames(usernames, realm);
+        }
+
+    },
+
 
     /**
      * Used if LDAP role has it's members declared in form of pure user uids. For example ( "memberUid: john" )
      */
-    UID
+    UID {
+
+        // Group inheritance not supported for this config
+        @Override
+        public Set<LDAPDn> getLDAPSubgroups(GroupLDAPFederationMapper groupMapper, LDAPObject ldapGroup) {
+            return Collections.emptySet();
+        }
+
+        @Override
+        public List<UserModel> getGroupMembers(GroupLDAPFederationMapper groupMapper, LDAPObject ldapGroup, int firstResult, int maxResults) {
+            String memberAttrName = groupMapper.getConfig().getMembershipLdapAttribute();
+            Set<String> memberUids = LDAPUtils.getExistingMemberships(memberAttrName, ldapGroup);
+
+            if (memberUids == null || memberUids.size() <= firstResult) {
+                return Collections.emptyList();
+            }
+
+            List<String> uids = new ArrayList<>(memberUids);
+            int max = Math.min(memberUids.size(), firstResult + maxResults);
+            uids = uids.subList(firstResult, max);
+
+            return groupMapper.getLdapProvider().loadUsersByUsernames(uids, groupMapper.getRealm());
+        }
+
+    };
+
+    public abstract Set<LDAPDn> getLDAPSubgroups(GroupLDAPFederationMapper groupMapper, LDAPObject ldapGroup);
+
+    public abstract List<UserModel> getGroupMembers(GroupLDAPFederationMapper groupMapper, LDAPObject ldapGroup, int firstResult, int maxResults);
 }

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java

@@ -1,6 +1,5 @@
 package org.keycloak.federation.ldap.mappers.membership.group;
 
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -27,8 +26,6 @@ import org.keycloak.federation.ldap.mappers.membership.UserRolesRetrieveStrategy
 import org.keycloak.models.GroupModel;
 import org.keycloak.models.ModelException;
 import org.keycloak.models.RealmModel;
-import org.keycloak.models.RoleContainerModel;
-import org.keycloak.models.RoleModel;
 import org.keycloak.models.UserFederationMapperModel;
 import org.keycloak.models.UserFederationSyncResult;
 import org.keycloak.models.UserModel;
@@ -115,22 +112,8 @@ public class GroupLDAPFederationMapper extends AbstractLDAPFederationMapper impl
     }
 
     protected Set<LDAPDn> getLDAPSubgroups(LDAPObject ldapGroup) {
-        return getLDAPMembersWithParent(ldapGroup, LDAPDn.fromString(config.getGroupsDn()));
+        MembershipType membershipType = config.getMembershipTypeLdapAttribute();
-    }
+        return membershipType.getLDAPSubgroups(this, ldapGroup);
-
-    // Get just those members of specified group, which are descendants of "requiredParentDn"
-    protected Set<LDAPDn> getLDAPMembersWithParent(LDAPObject ldapGroup, LDAPDn requiredParentDn) {
-        Set<String> allMemberships = LDAPUtils.getExistingMemberships(config.getMembershipLdapAttribute(), ldapGroup);
-
-        // Filter and keep just groups
-        Set<LDAPDn> result = new HashSet<>();
-        for (String membership : allMemberships) {
-            LDAPDn childDn = LDAPDn.fromString(membership);
-            if (childDn.isDescendantOf(requiredParentDn)) {
-                result.add(childDn);
-            }
-        }
-        return result;
     }
 
 
@@ -461,23 +444,8 @@ public class GroupLDAPFederationMapper extends AbstractLDAPFederationMapper impl
             return Collections.emptyList();
         }
 
-        LDAPDn usersDn = LDAPDn.fromString(ldapProvider.getLdapIdentityStore().getConfig().getUsersDn());
+        MembershipType membershipType = config.getMembershipTypeLdapAttribute();
-        Set<LDAPDn> userDns = getLDAPMembersWithParent(ldapGroup, usersDn);
+        return membershipType.getGroupMembers(this, ldapGroup, firstResult, maxResults);
-
-        if (userDns == null) {
-            return Collections.emptyList();
-        }
-
-        if (userDns.size() <= firstResult) {
-            return Collections.emptyList();
-        }
-
-        List<LDAPDn> dns = new ArrayList<>(userDns);
-        int max = Math.min(dns.size(), firstResult + maxResults);
-        dns = dns.subList(firstResult, max);
-
-        // We have dns of users, who are members of our group. Load them now
-        return ldapProvider.loadUsersByLDAPDns(dns, realm);
     }
 
     public void addGroupMappingInLDAP(String groupName, LDAPObject ldapUser) {

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapperFactory.java

@@ -11,6 +11,7 @@ import org.keycloak.federation.ldap.LDAPConfig;
 import org.keycloak.federation.ldap.LDAPFederationProvider;
 import org.keycloak.federation.ldap.mappers.AbstractLDAPFederationMapper;
 import org.keycloak.federation.ldap.mappers.AbstractLDAPFederationMapperFactory;
+import org.keycloak.federation.ldap.mappers.membership.CommonLDAPGroupMapperConfig;
 import org.keycloak.federation.ldap.mappers.membership.LDAPGroupMapperMode;
 import org.keycloak.federation.ldap.mappers.membership.MembershipType;
 import org.keycloak.federation.ldap.mappers.membership.UserRolesRetrieveStrategy;
@@ -170,6 +171,13 @@ public class GroupLDAPFederationMapperFactory extends AbstractLDAPFederationMapp
     public void validateConfig(UserFederationMapperModel mapperModel) throws MapperConfigValidationException {
         checkMandatoryConfigAttribute(GroupMapperConfig.GROUPS_DN, "LDAP Groups DN", mapperModel);
         checkMandatoryConfigAttribute(GroupMapperConfig.MODE, "Mode", mapperModel);
+
+        String mt = mapperModel.getConfig().get(CommonLDAPGroupMapperConfig.MEMBERSHIP_ATTRIBUTE_TYPE);
+        MembershipType membershipType = mt==null ? MembershipType.DN : Enum.valueOf(MembershipType.class, mt);
+        boolean preserveGroupInheritance = Boolean.parseBoolean(mapperModel.getConfig().get(GroupMapperConfig.PRESERVE_GROUP_INHERITANCE));
+        if (preserveGroupInheritance && membershipType != MembershipType.DN) {
+            throw new MapperConfigValidationException("Not possible to preserve group inheritance and use UID membership type together");
+        }
     }
 
     @Override

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPGroupMapperSyncTest.java

@@ -59,6 +59,9 @@ public class LDAPGroupMapperSyncTest {
             // Add group mapper
             FederationTestUtils.addOrUpdateGroupMapper(appRealm, ldapModel, LDAPGroupMapperMode.LDAP_ONLY, descriptionAttrName);
 
+            // Remove all LDAP groups
+            FederationTestUtils.removeAllLDAPGroups(session, appRealm, ldapModel, "groupsMapper");
+
             // Add some groups for testing
             LDAPObject group1 = FederationTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group1", descriptionAttrName, "group1 - description");
             LDAPObject group11 = FederationTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group11");

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPGroupMapperTest.java

@@ -64,6 +64,9 @@ public class LDAPGroupMapperTest {
             // Add group mapper
             FederationTestUtils.addOrUpdateGroupMapper(appRealm, ldapModel, LDAPGroupMapperMode.LDAP_ONLY, descriptionAttrName);
 
+            // Remove all LDAP groups
+            FederationTestUtils.removeAllLDAPGroups(session, appRealm, ldapModel, "groupsMapper");
+
             // Add some groups for testing
             LDAPObject group1 = FederationTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group1", descriptionAttrName, "group1 - description");
             LDAPObject group11 = FederationTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group11");