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 962919b86b..a92dfb0ef1 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 ldapGroups = getAllLDAPGroups(); + List ldapGroups = getAllLDAPGroups(config.isPreserveGroupsInheritance()); // Convert to internal format Map ldapGroupsMap = new HashMap<>(); @@ -163,12 +167,15 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements for (LDAPObject ldapGroup : ldapGroups) { String groupName = ldapGroup.getAttributeAsString(groupsRdnAttr); - Set subgroupNames = new HashSet<>(); - for (LDAPDn groupDn : getLDAPSubgroups(ldapGroup)) { - subgroupNames.add(groupDn.getFirstRdnAttrValue()); + if (config.isPreserveGroupsInheritance()) { + Set 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 getAllLDAPGroups() { - LDAPQuery ldapGroupQuery = createGroupQuery(); + protected List 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 ldapGroups = ldapQuery.getResultList(); // Convert them to Map @@ -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 2410a8fdc4..36c1e0d567 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 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 ldapRoles = ldapQuery.getResultList(); + LDAPQuery ldapQuery = createRoleQuery(false); + List ldapRoles = LDAPUtils.loadAllLDAPObjects(ldapQuery, ldapProvider); Set 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 6a906369bd..83a3498d59 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 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 ldapRoles = roleQuery.getResultList(); for (LDAPObject ldapRole : ldapRoles) { ldapProvider.getLdapIdentityStore().remove(ldapRole);