Add checks to prevent GroupLDAPStorageMapper from performing operations on groups it does not manage

Closes #11008
Closes #17593
Closes #19652

Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>

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

@@ -569,6 +569,11 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
             return Collections.emptyList();
         }
 
+        if (!isGroupInGroupPath(realm, kcGroup)) {
+            // group being inspected is not managed by this mapper - return empty collection
+            return Collections.emptyList();
+        }
+
         // TODO: with ranged search in AD we can improve the search using the specific range (not done for the moment)
         LDAPObject ldapGroup = loadLDAPGroupByName(kcGroup.getName());
         if (ldapGroup == null) {
@@ -703,18 +708,18 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
         @Override
         public Stream<GroupModel> getGroupsStream() {
             Stream<GroupModel> ldapGroupMappings = getLDAPGroupMappingsConverted();
-            if (config.getMode() == LDAPGroupMapperMode.LDAP_ONLY) {
+            if (config.isTopLevelGroupsPath() && config.getMode() == LDAPGroupMapperMode.LDAP_ONLY) {
                 // Use just group mappings from LDAP
                 return ldapGroupMappings;
             } else {
-                // Merge mappings from both DB and LDAP
+                // Merge mappings from both DB and LDAP (including groups assigned from other group mappers)
                 return Stream.concat(ldapGroupMappings, super.getGroupsStream());
             }
         }
 
         @Override
         public void joinGroup(GroupModel group) {
-            if (config.getMode() == LDAPGroupMapperMode.LDAP_ONLY) {
+            if (config.getMode() == LDAPGroupMapperMode.LDAP_ONLY && isGroupInGroupPath(realm, group)) {
                 // We need to create new role mappings in LDAP
                 cachedLDAPGroupMappings = null;
                 addGroupMappingInLDAP(realm, group, ldapUser);
@@ -725,6 +730,11 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
 
         @Override
         public void leaveGroup(GroupModel group) {
+            // if user is leaving group not managed by this mapper, let the call proceed to the next mapper or to the DB.
+            if (!isGroupInGroupPath(realm, group)) {
+                super.leaveGroup(group);
+            }
+
             try (LDAPQuery ldapQuery = createGroupQuery(true)) {
                 LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder();
                 Condition roleNameCondition = conditionsBuilder.equal(config.getGroupNameLdapAttribute(), group.getName());
@@ -756,7 +766,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
 
         @Override
         public boolean isMemberOf(GroupModel group) {
-            return RoleUtils.isDirectMember(getGroupsStream(),group);
+            return isGroupInGroupPath(realm, group) && RoleUtils.isDirectMember(getGroupsStream(),group);
         }
 
         protected Stream<GroupModel> getLDAPGroupMappingsConverted() {
@@ -795,6 +805,23 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
         return config.isTopLevelGroupsPath() ? null : KeycloakModelUtils.findGroupByPath(session, realm, config.getGroupsPath());
     }
 
+    protected boolean isGroupInGroupPath(RealmModel realm, GroupModel group) {
+        if (config.isTopLevelGroupsPath()) {
+            return true; // any group is in the path of the top level path.
+        }
+        GroupModel groupPathGroup = KeycloakModelUtils.findGroupByPath(session, realm, config.getGroupsPath());
+        if (groupPathGroup != null) {
+            while(!groupPathGroup.getId().equals(group.getId())) {
+                group = group.getParent();
+                if (group == null) {
+                    return false; // we checked every ancestor group, and none matches the group path group.
+                }
+            }
+            return true;
+        }
+        return false;
+    }
+
     /**
      * Creates a new KC group from given LDAP group name in given KC parent group or the groups path.
      */

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/util/LDAPTestUtils.java

@@ -299,14 +299,18 @@ public class LDAPTestUtils {
     }
 
     public static void addOrUpdateGroupMapper(RealmModel realm, ComponentModel providerModel, LDAPGroupMapperMode mode, String descriptionAttrName, String... otherConfigOptions) {
-        ComponentModel mapperModel = getSubcomponentByName(realm, providerModel, "groupsMapper");
+        addOrUpdateGroupMapper("groupsMapper", realm, providerModel, mode, descriptionAttrName, otherConfigOptions);
+    }
+
+    public static void addOrUpdateGroupMapper(String mapperName, RealmModel realm, ComponentModel providerModel, LDAPGroupMapperMode mode, String descriptionAttrName, String... otherConfigOptions) {
+        ComponentModel mapperModel = getSubcomponentByName(realm, providerModel, mapperName);
         if (mapperModel != null) {
             mapperModel.getConfig().putSingle(GroupMapperConfig.MODE, mode.toString());
             updateGroupMapperConfigOptions(mapperModel, otherConfigOptions);
             realm.updateComponent(mapperModel);
         } else {
             String baseDn = providerModel.getConfig().getFirst(LDAPConstants.BASE_DN);
-            mapperModel = KeycloakModelUtils.createComponentModel("groupsMapper", providerModel.getId(), GroupLDAPStorageMapperFactory.PROVIDER_ID, LDAPStorageMapper.class.getName(),
+            mapperModel = KeycloakModelUtils.createComponentModel(mapperName, providerModel.getId(), GroupLDAPStorageMapperFactory.PROVIDER_ID, LDAPStorageMapper.class.getName(),
                     GroupMapperConfig.GROUPS_DN, "ou=Groups," + baseDn,
                     GroupMapperConfig.MAPPED_GROUP_ATTRIBUTES, descriptionAttrName,
                     GroupMapperConfig.PRESERVE_GROUP_INHERITANCE, "true",

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

@@ -40,6 +40,7 @@ import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery;
 import org.keycloak.storage.ldap.mappers.membership.LDAPGroupMapperMode;
 import org.keycloak.storage.ldap.mappers.membership.MembershipType;
 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.testsuite.util.LDAPRule;
 import org.keycloak.testsuite.util.LDAPTestUtils;
@@ -72,7 +73,6 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
     }
 
 
-
     @Test
     public void test01_ldapOnlyGroupMappings() {
         test01_ldapOnlyGroupMappings(true);
@@ -202,7 +202,7 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
             mary.leaveGroup(groupTeamChild20182019);
 
             Assert.assertEquals(0, john.getGroupsStream().count());
-            
+
             groupCount = john.getGroupsCount();
             Assert.assertEquals(0, groupCount);
         });
@@ -214,7 +214,6 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
     }
 
 
-
     protected void test02_readOnlyGroupMappings(boolean importEnabled) {
         testingClient.server().run(session -> {
             LDAPTestContext ctx = LDAPTestContext.init(session);
@@ -617,7 +616,7 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
 
             UserModel john = session.users().getUserByUsername(appRealm, "johnkeycloak");
 
-            GroupModel group4 =  KeycloakModelUtils.findGroupByPath(session, appRealm, "/group4");
+            GroupModel group4 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/group4");
             john.joinGroup(group4);
 
             GroupModel group31 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/group3/group31");
@@ -637,11 +636,11 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
 
             UserModel john = session.users().getUserByUsername(appRealm, "johnkeycloak");
 
-            GroupModel group14 =  KeycloakModelUtils.findGroupByPath(session, appRealm, "/group1/group14");
+            GroupModel group14 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/group1/group14");
-            GroupModel group3 =  KeycloakModelUtils.findGroupByPath(session, appRealm, "/group3");
+            GroupModel group3 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/group3");
             GroupModel group31 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/group3/group31");
             GroupModel group32 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/group3/group32");
-            GroupModel group4 =  KeycloakModelUtils.findGroupByPath(session, appRealm, "/group4");
+            GroupModel group4 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/group4");
 
             Set<GroupModel> groups = john.getGroupsStream().collect(Collectors.toSet());
             Assert.assertTrue(groups.contains(group14));
@@ -676,17 +675,17 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
 
             UserModel david = session.users().addUser(appRealm, "davidkeycloak");
 
-            GroupModel defaultGroup11 =  KeycloakModelUtils.findGroupByPath(session, appRealm, "/defaultGroup1/defaultGroup11");
+            GroupModel defaultGroup11 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/defaultGroup1/defaultGroup11");
             Assert.assertNotNull(defaultGroup11);
 
-            GroupModel defaultGroup12 =  KeycloakModelUtils.findGroupByPath(session, appRealm, "/defaultGroup1/defaultGroup12");
+            GroupModel defaultGroup12 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/defaultGroup1/defaultGroup12");
             Assert.assertNotNull(defaultGroup12);
 
             GroupModel group31 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/group3/group31");
             Assert.assertNotNull(group31);
             GroupModel group32 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/group3/group32");
             Assert.assertNotNull(group32);
-            GroupModel group4 =  KeycloakModelUtils.findGroupByPath(session, appRealm, "/group4");
+            GroupModel group4 = KeycloakModelUtils.findGroupByPath(session, appRealm, "/group4");
             Assert.assertNotNull(group4);
 
             Set<GroupModel> groups = david.getGroupsStream().collect(Collectors.toSet());
@@ -701,10 +700,10 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
 
     private static LDAPObject searchObjectInBase(LDAPStorageProvider ldapProvider, String dn, String... attrs) {
         LDAPQuery q = new LDAPQuery(ldapProvider)
-                            .setSearchDn(dn)
+                .setSearchDn(dn)
-                            .setSearchScope(SearchControls.OBJECT_SCOPE);
+                .setSearchScope(SearchControls.OBJECT_SCOPE);
         if (attrs != null) {
-            for (String attr: attrs) {
+            for (String attr : attrs) {
                 q.addReturningLdapAttribute(attr);
             }
         }
@@ -776,9 +775,9 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
             usernames.clear();
             for (int i = 0; i < membersToTest; i += 10) {
                 groupMembers = session.users().getGroupMembersStream(appRealm, kcBigGroup, i, 10)
-                    .collect(Collectors.toList());
+                        .collect(Collectors.toList());
                 usernames.addAll(groupMembers.stream().map(u -> u.getUsername()).collect(Collectors.toSet()));
-                Assert.assertEquals("Incorrect number of users after pagination " + i, membersToTest < i + 10? membersToTest : i + 10, usernames.size());
+                Assert.assertEquals("Incorrect number of users after pagination " + i, membersToTest < i + 10 ? membersToTest : i + 10, usernames.size());
             }
             for (int i = 0; i < membersToTest; i++) {
                 Assert.assertTrue("Group contains user after pagination " + i, usernames.contains(String.format("user%02d", i)));
@@ -839,4 +838,109 @@ public class LDAPGroupMapperTest extends AbstractLDAPTest {
             Assert.assertEquals(LDAPDn.fromString(LDAPConstants.EMPTY_MEMBER_ATTRIBUTE_VALUE), LDAPDn.fromString(deleteGroup.getAttributeAsString(LDAPConstants.MEMBER)));
         });
     }
+
+    @Test
+    public void test10_multipleGroupMappersInDifferentPaths() {
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel realm = ctx.getRealm();
+
+            // 1- Remove the group mapper already installed
+            ComponentModel mapperModel = LDAPTestUtils.getSubcomponentByName(realm, ctx.getLdapModel(), "groupsMapper");
+            realm.removeComponent(mapperModel);
+        });
+
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel realm = ctx.getRealm();
+
+            // 2- Add a couple of local groups
+            realm.createGroup("ldap-groups-1");
+            realm.createGroup("ldap-groups-2");
+
+            // 3- Add a couple of group mappers that map groups from different OUs into different group paths in Keycloak.
+            ComponentModel ldapModel = LDAPTestUtils.getLdapProviderModel(realm);
+            LDAPStorageProvider ldapProvider = LDAPTestUtils.getLdapProvider(session, ldapModel);
+            String baseDn = ldapModel.getConfig().getFirst(LDAPConstants.BASE_DN);
+            String descriptionName = ldapProvider.getLdapIdentityStore().getConfig().isActiveDirectory() ? "displayName" : "description";
+            LDAPTestUtils.addLdapOUinBaseDn(ldapProvider, "Groups1");
+            LDAPTestUtils.addOrUpdateGroupMapper("group-mapper-1", realm, ldapModel, LDAPGroupMapperMode.LDAP_ONLY, descriptionName,
+                    GroupMapperConfig.GROUPS_DN, "ou=Groups1," + baseDn, GroupMapperConfig.LDAP_GROUPS_PATH, "/ldap-groups-1");
+            LDAPTestUtils.addLdapOUinBaseDn(ldapProvider, "Groups2");
+            LDAPTestUtils.addOrUpdateGroupMapper("group-mapper-2", realm, ldapModel, LDAPGroupMapperMode.LDAP_ONLY, descriptionName,
+                    GroupMapperConfig.GROUPS_DN, "ou=Groups2," + baseDn, GroupMapperConfig.LDAP_GROUPS_PATH, "/ldap-groups-2");
+
+            LDAPTestUtils.createLDAPGroup("group-mapper-1", session, realm, ldapModel, "GroupA", descriptionName, "GroupA Description");
+            LDAPTestUtils.createLDAPGroup("group-mapper-1", session, realm, ldapModel, "GroupB", descriptionName, "GroupB Description");
+            LDAPTestUtils.createLDAPGroup("group-mapper-2", session, realm, ldapModel, "GroupC", descriptionName, "GroupC Description");
+
+            // 4- Sync LDAP groups from both mappers to Keycloak DB
+            ComponentModel mapperModel = LDAPTestUtils.getSubcomponentByName(realm, ldapModel, "group-mapper-1");
+            new GroupLDAPStorageMapperFactory().create(session, mapperModel).syncDataFromFederationProviderToKeycloak(realm);
+            mapperModel = LDAPTestUtils.getSubcomponentByName(realm, ldapModel, "group-mapper-2");
+            new GroupLDAPStorageMapperFactory().create(session, mapperModel).syncDataFromFederationProviderToKeycloak(realm);
+        });
+
+        testingClient.server(TEST_REALM_NAME).run(session -> {
+            RealmModel realm = session.getContext().getRealm();
+
+            GroupModel groupA = KeycloakModelUtils.findGroupByPath(session, realm, "/ldap-groups-1/GroupA");
+            GroupModel groupB = KeycloakModelUtils.findGroupByPath(session, realm, "/ldap-groups-1/GroupB");
+            GroupModel groupC = KeycloakModelUtils.findGroupByPath(session, realm, "/ldap-groups-2/GroupC");
+
+            UserModel john = session.users().getUserByUsername(realm, "johnkeycloak");
+            UserModel mary = session.users().getUserByUsername(realm, "marykeycloak");
+
+            // 5- Join some of the groups, and check that the correct mapper was applied (i.e. user was added as a member of the correct group)
+            john.joinGroup(groupA);
+            john.joinGroup(groupC);
+            mary.joinGroup(groupB);
+            mary.joinGroup(groupC);
+
+            // 5.1- Check that group mappings are in LDAP and hence available through federation
+            Set<GroupModel> johnGroups = john.getGroupsStream().collect(Collectors.toSet());
+            Assert.assertEquals(4, johnGroups.size());
+            Assert.assertTrue(johnGroups.contains(groupA));
+            Assert.assertFalse(johnGroups.contains(groupB));
+            Assert.assertTrue(johnGroups.contains(groupC));
+
+            Set<GroupModel> maryGroups = mary.getGroupsStream().collect(Collectors.toSet());
+            Assert.assertEquals(4, maryGroups.size());
+            Assert.assertFalse(maryGroups.contains(groupA));
+            Assert.assertTrue(maryGroups.contains(groupB));
+            Assert.assertTrue(maryGroups.contains(groupC));
+
+            // 5.2- Check through userProvider
+            List<String> groupAMembers = session.users().getGroupMembersStream(realm, groupA, 0, 10)
+                    .map(UserModel::getUsername).toList();
+            List<String> groupBMembers = session.users().getGroupMembersStream(realm, groupB, 0, 10)
+                    .map(UserModel::getUsername).toList();
+            List<String> groupCMembers = session.users().getGroupMembersStream(realm, groupC, 0, 10)
+                    .map(UserModel::getUsername).toList();
+
+            Assert.assertEquals(1, groupAMembers.size());
+            Assert.assertEquals("johnkeycloak", groupAMembers.get(0));
+            Assert.assertEquals(1, groupBMembers.size());
+            Assert.assertEquals("marykeycloak", groupBMembers.get(0));
+            Assert.assertEquals(2, groupCMembers.size());
+            Assert.assertTrue(groupCMembers.contains("johnkeycloak"));
+            Assert.assertTrue(groupCMembers.contains("marykeycloak"));
+        });
+
+        testingClient.server(TEST_REALM_NAME).run(session -> {
+            RealmModel realm = session.getContext().getRealm();
+
+            // 6- Create a local group with the same name of one of the LDAP groups, and add a local user to this group
+            GroupModel localGroup = realm.createGroup("GroupA");
+            UserModel localUser = UserStoragePrivateUtil.userLocalStorage(session).addUser(realm, "localuser");
+            localUser.joinGroup(localGroup);
+
+            // 7- Check that the members of the local group are not mixed with members of the group with same name from LDAP
+            List<String> localGroupMembers = session.users().getGroupMembersStream(realm, localGroup, 0, 10)
+                    .map(UserModel::getUsername).toList();
+            Assert.assertEquals(1, localGroupMembers.size());
+            Assert.assertEquals("localuser", localGroupMembers.get(0));
+        });
+    }
 }
+