keycloak-aplcache

KEYCLOAK-5827 Retrieve member attribute from LDAP on group/role

11/15/2017 11:51:17 AM

Details

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 962919b..a92dfb0 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
@@ -76,7 +76,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
 
     @Override
     public LDAPQuery createLDAPGroupQuery() {
-        return createGroupQuery();
+        return createGroupQuery(false);
     }
 
     @Override
@@ -88,7 +88,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
 
     // LDAP Group CRUD operations
 
-    public LDAPQuery createGroupQuery() {
+    public LDAPQuery createGroupQuery(boolean includeMemberAttribute) {
         LDAPQuery ldapQuery = new LDAPQuery(ldapProvider);
 
         // For now, use same search scope, which is configured "globally" and used for user's search.
@@ -107,7 +107,11 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
         }
 
         ldapQuery.addReturningLdapAttribute(config.getGroupNameLdapAttribute());
-        ldapQuery.addReturningLdapAttribute(config.getMembershipLdapAttribute());
+
+        // Performance improvement
+        if (includeMemberAttribute) {
+            ldapQuery.addReturningLdapAttribute(config.getMembershipLdapAttribute());
+        }
 
         for (String groupAttr : config.getGroupAttributes()) {
             ldapQuery.addReturningLdapAttribute(groupAttr);
@@ -125,7 +129,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
     }
 
     public LDAPObject loadLDAPGroupByName(String groupName) {
-        LDAPQuery ldapQuery = createGroupQuery();
+        LDAPQuery ldapQuery = createGroupQuery(true);
         Condition roleNameCondition = new LDAPQueryConditionsBuilder().equal(config.getGroupNameLdapAttribute(), groupName);
         ldapQuery.addWhereCondition(roleNameCondition);
         return ldapQuery.getFirstResult();
@@ -153,7 +157,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
         logger.debugf("Syncing groups from LDAP into Keycloak DB. Mapper is [%s], LDAP provider is [%s]", mapperModel.getName(), ldapProvider.getModel().getName());
 
         // Get all LDAP groups
-        List<LDAPObject> ldapGroups = getAllLDAPGroups();
+        List<LDAPObject> ldapGroups = getAllLDAPGroups(config.isPreserveGroupsInheritance());
 
         // Convert to internal format
         Map<String, LDAPObject> ldapGroupsMap = new HashMap<>();
@@ -163,12 +167,15 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
         for (LDAPObject ldapGroup : ldapGroups) {
             String groupName = ldapGroup.getAttributeAsString(groupsRdnAttr);
 
-            Set<String> subgroupNames = new HashSet<>();
-            for (LDAPDn groupDn : getLDAPSubgroups(ldapGroup)) {
-                subgroupNames.add(groupDn.getFirstRdnAttrValue());
+            if (config.isPreserveGroupsInheritance()) {
+                Set<String> subgroupNames = new HashSet<>();
+                for (LDAPDn groupDn : getLDAPSubgroups(ldapGroup)) {
+                    subgroupNames.add(groupDn.getFirstRdnAttrValue());
+                }
+
+                ldapGroupsRep.add(new GroupTreeResolver.Group(groupName, subgroupNames));
             }
 
-            ldapGroupsRep.add(new GroupTreeResolver.Group(groupName, subgroupNames));
             ldapGroupsMap.put(groupName, ldapGroup);
         }
 
@@ -342,8 +349,8 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
     }
 
     // Send LDAP query to retrieve all groups
-    protected List<LDAPObject> getAllLDAPGroups() {
-        LDAPQuery ldapGroupQuery = createGroupQuery();
+    protected List<LDAPObject> getAllLDAPGroups(boolean includeMemberAttribute) {
+        LDAPQuery ldapGroupQuery = createGroupQuery(includeMemberAttribute);
         return LDAPUtils.loadAllLDAPObjects(ldapGroupQuery, ldapProvider);
     }
 
@@ -368,7 +375,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
         logger.debugf("Syncing groups from Keycloak into LDAP. Mapper is [%s], LDAP provider is [%s]", mapperModel.getName(), ldapProvider.getModel().getName());
 
         // Query existing LDAP groups
-        LDAPQuery ldapQuery = createGroupQuery();
+        LDAPQuery ldapQuery = createGroupQuery(config.isPreserveGroupsInheritance());
         List<LDAPObject> ldapGroups = ldapQuery.getResultList();
 
         // Convert them to Map<String, LDAPObject>
@@ -615,7 +622,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
 
         @Override
         public void leaveGroup(GroupModel group) {
-            LDAPQuery ldapQuery = createGroupQuery();
+            LDAPQuery ldapQuery = createGroupQuery(true);
             LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder();
             Condition roleNameCondition = conditionsBuilder.equal(config.getGroupNameLdapAttribute(), group.getName());
 
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 2410a8f..36c1e0d 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
@@ -68,7 +68,7 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements 
 
     @Override
     public LDAPQuery createLDAPGroupQuery() {
-        return createRoleQuery();
+        return createRoleQuery(false);
     }
 
     @Override
@@ -124,7 +124,7 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements 
         logger.debugf("Syncing roles from LDAP into Keycloak DB. Mapper is [%s], LDAP provider is [%s]", mapperModel.getName(), ldapProvider.getModel().getName());
 
         // Send LDAP query to load all roles
-        LDAPQuery ldapRoleQuery = createRoleQuery();
+        LDAPQuery ldapRoleQuery = createRoleQuery(false);
         List<LDAPObject> ldapRoles = LDAPUtils.loadAllLDAPObjects(ldapRoleQuery, ldapProvider);
 
         RoleContainerModel roleContainer = getTargetRoleContainer(realm);
@@ -165,8 +165,8 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements 
         logger.debugf("Syncing roles from Keycloak into LDAP. Mapper is [%s], LDAP provider is [%s]", mapperModel.getName(), ldapProvider.getModel().getName());
 
         // Send LDAP query to see which roles exists there
-        LDAPQuery ldapQuery = createRoleQuery();
-        List<LDAPObject> ldapRoles = ldapQuery.getResultList();
+        LDAPQuery ldapQuery = createRoleQuery(false);
+        List<LDAPObject> ldapRoles = LDAPUtils.loadAllLDAPObjects(ldapQuery, ldapProvider);
 
         Set<String> ldapRoleNames = new HashSet<>();
         String rolesRdnAttr = config.getRoleNameLdapAttribute();
@@ -194,7 +194,7 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements 
     }
 
     // TODO: Possible to merge with GroupMapper and move to common class
-    public LDAPQuery createRoleQuery() {
+    public LDAPQuery createRoleQuery(boolean includeMemberAttribute) {
         LDAPQuery ldapQuery = new LDAPQuery(ldapProvider);
 
         // For now, use same search scope, which is configured "globally" and used for user's search.
@@ -214,9 +214,13 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements 
             ldapQuery.addWhereCondition(customFilterCondition);
         }
 
-        String membershipAttr = config.getMembershipLdapAttribute();
         ldapQuery.addReturningLdapAttribute(rolesRdnAttr);
-        ldapQuery.addReturningLdapAttribute(membershipAttr);
+
+        // Performance improvement
+        if (includeMemberAttribute) {
+            String membershipAttr = config.getMembershipLdapAttribute();
+            ldapQuery.addReturningLdapAttribute(membershipAttr);
+        }
 
         return ldapQuery;
     }
@@ -264,7 +268,7 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements 
     }
 
     public LDAPObject loadLDAPRoleByName(String roleName) {
-        LDAPQuery ldapQuery = createRoleQuery();
+        LDAPQuery ldapQuery = createRoleQuery(true);
         Condition roleNameCondition = new LDAPQueryConditionsBuilder().equal(config.getRoleNameLdapAttribute(), roleName);
         ldapQuery.addWhereCondition(roleNameCondition);
         return ldapQuery.getFirstResult();
@@ -430,7 +434,7 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements 
         public void deleteRoleMapping(RoleModel role) {
             if (role.getContainer().equals(roleContainer)) {
 
-                LDAPQuery ldapQuery = createRoleQuery();
+                LDAPQuery ldapQuery = createRoleQuery(true);
                 LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder();
                 Condition roleNameCondition = conditionsBuilder.equal(config.getRoleNameLdapAttribute(), role.getName());
 
diff --git a/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPTestUtils.java b/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPTestUtils.java
index 6a90636..83a3498 100644
--- a/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPTestUtils.java
+++ b/testsuite/integration-deprecated/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPTestUtils.java
@@ -281,7 +281,7 @@ public class LDAPTestUtils {
     public static void removeAllLDAPRoles(KeycloakSession session, RealmModel appRealm, ComponentModel ldapModel, String mapperName) {
         ComponentModel mapperModel = getSubcomponentByName(appRealm, ldapModel, mapperName);
         LDAPStorageProvider ldapProvider = LDAPTestUtils.getLdapProvider(session, ldapModel);
-        LDAPQuery roleQuery = getRoleMapper(mapperModel, ldapProvider, appRealm).createRoleQuery();
+        LDAPQuery roleQuery = getRoleMapper(mapperModel, ldapProvider, appRealm).createRoleQuery(false);
         List<LDAPObject> ldapRoles = roleQuery.getResultList();
         for (LDAPObject ldapRole : ldapRoles) {
             ldapProvider.getLdapIdentityStore().remove(ldapRole);
@@ -291,7 +291,7 @@ public class LDAPTestUtils {
     public static void removeAllLDAPGroups(KeycloakSession session, RealmModel appRealm, ComponentModel ldapModel, String mapperName) {
         ComponentModel mapperModel = getSubcomponentByName(appRealm, ldapModel, mapperName);
         LDAPStorageProvider ldapProvider = LDAPTestUtils.getLdapProvider(session, ldapModel);
-        LDAPQuery roleQuery = getGroupMapper(mapperModel, ldapProvider, appRealm).createGroupQuery();
+        LDAPQuery roleQuery = getGroupMapper(mapperModel, ldapProvider, appRealm).createGroupQuery(false);
         List<LDAPObject> ldapRoles = roleQuery.getResultList();
         for (LDAPObject ldapRole : ldapRoles) {
             ldapProvider.getLdapIdentityStore().remove(ldapRole);