keycloak-aplcache

diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
index 010870e..4ddd142 100755
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
@@ -179,8 +179,8 @@ public class LDAPFederationProvider implements UserFederationProvider {
     @Override
     public boolean removeUser(RealmModel realm, UserModel user) {
         if (editMode == EditMode.READ_ONLY || editMode == EditMode.UNSYNCED) {
-            logger.warnf("User '%s' can't be deleted in LDAP as editMode is '%s'", user.getUsername(), editMode.toString());
-            return false;
+            logger.warnf("User '%s' can't be deleted in LDAP as editMode is '%s'. Deleting user just from Keycloak DB, but he will be re-imported from LDAP again once searched in Keycloak", user.getUsername(), editMode.toString());
+            return true;
         }
 
         LDAPObject ldapObject = loadAndValidateUser(realm, user);

diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java
index b075d19..e406c11 100755
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java
@@ -19,6 +19,7 @@ package org.keycloak.federation.ldap;
 
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -229,6 +230,45 @@ public class LDAPUtils {
     }
 
 
+    /**
+     * Load all LDAP objects corresponding to given query. We will load them paginated, so we allow to bypass the limitation of 1000
+     * maximum loaded objects in single query in MSAD
+     *
+     * @param ldapQuery
+     * @param ldapProvider
+     * @return
+     */
+    public static List<LDAPObject> loadAllLDAPObjects(LDAPQuery ldapQuery, LDAPFederationProvider ldapProvider) {
+        LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig();
+        boolean pagination = ldapConfig.isPagination();
+        if (pagination) {
+            // For now reuse globally configured batch size in LDAP provider page
+            int pageSize = ldapConfig.getBatchSizeForSync();
+
+            List<LDAPObject> result = new LinkedList<>();
+            boolean nextPage = true;
+
+            while (nextPage) {
+                ldapQuery.setLimit(pageSize);
+                final List<LDAPObject> currentPageGroups = ldapQuery.getResultList();
+                result.addAll(currentPageGroups);
+                nextPage = ldapQuery.getPaginationContext() != null;
+            }
+
+            return result;
+        } else {
+            // LDAP pagination not available. Do everything in single transaction
+            return ldapQuery.getResultList();
+        }
+    }
+
+
+    /**
+     * Validate configured customFilter matches the requested format
+     *
+     * @param customFilter
+     * @throws FederationConfigValidationException
+     */
     public static void validateCustomLdapFilter(String customFilter) throws FederationConfigValidationException {
         if (customFilter != null) {
 

diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java
index 00108bc..cd220b2 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java
@@ -343,28 +343,7 @@ public class GroupLDAPFederationMapper extends AbstractLDAPFederationMapper impl
     // Send LDAP query to retrieve all groups
     protected List<LDAPObject> getAllLDAPGroups() {
         LDAPQuery ldapGroupQuery = createGroupQuery();
-
-        LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig();
-        boolean pagination = ldapConfig.isPagination();
-        if (pagination) {
-            // For now reuse globally configured batch size in LDAP provider page
-            int pageSize = ldapConfig.getBatchSizeForSync();
-
-            List<LDAPObject> result = new LinkedList<>();
-            boolean nextPage = true;
-
-            while (nextPage) {
-                ldapGroupQuery.setLimit(pageSize);
-                final List<LDAPObject> currentPageGroups = ldapGroupQuery.getResultList();
-                result.addAll(currentPageGroups);
-                nextPage = ldapGroupQuery.getPaginationContext() != null;
-            }
-
-            return result;
-        } else {
-            // LDAP pagination not available. Do everything in single transaction
-            return ldapGroupQuery.getResultList();
-        }
+        return LDAPUtils.loadAllLDAPObjects(ldapGroupQuery, ldapProvider);
     }
 
 

diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapperFactory.java
index fb6056b..169875d 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapperFactory.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapperFactory.java
@@ -88,9 +88,9 @@ public class GroupLDAPFederationMapperFactory extends AbstractLDAPFederationMapp
         for (MembershipType membershipType : MembershipType.values()) {
             membershipTypes.add(membershipType.toString());
         }
-        ProviderConfigProperty membershipType = createConfigProperty(RoleMapperConfig.MEMBERSHIP_ATTRIBUTE_TYPE, "Membership Attribute Type",
-                "DN means that LDAP role has it's members declared in form of their full DN. For example 'member: uid=john,ou=users,dc=example,dc=com' . " +
-                        "UID means that LDAP role has it's members declared in form of pure user uids. For example 'memberUid: john' .",
+        ProviderConfigProperty membershipType = createConfigProperty(GroupMapperConfig.MEMBERSHIP_ATTRIBUTE_TYPE, "Membership Attribute Type",
+                "DN means that LDAP group has it's members declared in form of their full DN. For example 'member: uid=john,ou=users,dc=example,dc=com' . " +
+                        "UID means that LDAP group has it's members declared in form of pure user uids. For example 'memberUid: john' .",
                 ProviderConfigProperty.LIST_TYPE, membershipTypes);
         configProperties.add(membershipType);
 
@@ -165,6 +165,7 @@ public class GroupLDAPFederationMapperFactory extends AbstractLDAPFederationMapp
 
         defaultValues.put(GroupMapperConfig.PRESERVE_GROUP_INHERITANCE, "true");
         defaultValues.put(GroupMapperConfig.MEMBERSHIP_LDAP_ATTRIBUTE, LDAPConstants.MEMBER);
+        defaultValues.put(GroupMapperConfig.MEMBERSHIP_ATTRIBUTE_TYPE, MembershipType.DN.toString());
 
         String mode = config.getEditMode() == UserFederationProvider.EditMode.WRITABLE ? LDAPGroupMapperMode.LDAP_ONLY.toString() : LDAPGroupMapperMode.READ_ONLY.toString();
         defaultValues.put(GroupMapperConfig.MODE, mode);

diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapper.java
index cbafcc4..9dbeec9 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapper.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapper.java
@@ -122,9 +122,9 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper imple
 
         logger.debugf("Syncing roles from LDAP into Keycloak DB. Mapper is [%s], LDAP provider is [%s]", mapperModel.getName(), ldapProvider.getModel().getDisplayName());
 
-        // Send LDAP query
-        LDAPQuery ldapQuery = createRoleQuery();
-        List<LDAPObject> ldapRoles = ldapQuery.getResultList();
+        // Send LDAP query to load all roles
+        LDAPQuery ldapRoleQuery = createRoleQuery();
+        List<LDAPObject> ldapRoles = LDAPUtils.loadAllLDAPObjects(ldapRoleQuery, ldapProvider);
 
         RoleContainerModel roleContainer = getTargetRoleContainer();
         String rolesRdnAttr = config.getRoleNameLdapAttribute();

diff --git a/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java b/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java
index 8f809f3..ef8d182 100755
--- a/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java
+++ b/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java
@@ -96,7 +96,7 @@ public class UserFederationManager implements UserProvider {
                 boolean localRemoved = session.userStorage().removeUser(realm, user);
                 managedUsers.remove(user.getId());
                 if (!localRemoved) {
-                    logger.warn("User removed from federation provider, but failed to remove him from keycloak model");
+                    logger.warn("User possibly removed from federation provider, but failed to remove him from keycloak model");
                 }
                 return localRemoved;
             } else {

diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java
index 3ee6b70..be8c97b 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java
@@ -500,6 +500,8 @@ public class FederationProvidersIntegrationTest {
         }
     }
 
+
+    // TODO: Rather separate test for fullNameMapper to better test all the possibilities
     @Test
     public void testFullNameMapper() {
         KeycloakSession session = keycloakRule.startSession();
@@ -691,7 +693,7 @@ public class FederationProvidersIntegrationTest {
 
             }
 
-            Assert.assertFalse(session.users().removeUser(appRealm, user));
+            Assert.assertTrue(session.users().removeUser(appRealm, user));
         } finally {
             keycloakRule.stopSession(session, false);
         }
@@ -826,8 +828,12 @@ public class FederationProvidersIntegrationTest {
             LDAPObject ldapUser = ldapProvider.loadLDAPUserByUsername(appRealm, "johnkeycloak");
             ldapProvider.getLdapIdentityStore().validatePassword(ldapUser, "Password1");
 
-            // ATM it's not permitted to delete user in unsynced mode. Should be user deleted just locally instead?
-            Assert.assertFalse(session.users().removeUser(appRealm, user));
+            // User is deleted just locally
+            Assert.assertTrue(session.users().removeUser(appRealm, user));
+
+            // Assert user not available locally, but will be reimported from LDAP once searched
+            Assert.assertNull(session.userStorage().getUserByUsername("johnkeycloak", appRealm));
+            Assert.assertNotNull(session.users().getUserByUsername("johnkeycloak", appRealm));
         } finally {
             keycloakRule.stopSession(session, false);
         }