From 79c51a6a8064c2cba39f1e206aeaf268a750ca8d Mon Sep 17 00:00:00 2001 From: Markus Heberling Date: Thu, 21 Sep 2017 13:11:49 +0200 Subject: [PATCH] KEYCLOAK-5510 Allow import of groups with missing subgroups. --- .../group/GroupLDAPStorageMapper.java | 2 +- .../group/GroupLDAPStorageMapperFactory.java | 6 ++++++ .../membership/group/GroupMapperConfig.java | 7 +++++++ .../membership/group/GroupTreeResolver.java | 20 ++++++++++++++----- .../ldap/idm/model/GroupTreeResolverTest.java | 18 +++++++++++------ 5 files changed, 41 insertions(+), 12 deletions(-) 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 fcdae3c012..0ae1627f73 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 @@ -175,7 +175,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements // Now we have list of LDAP groups. Let's form the tree (if needed) if (config.isPreserveGroupsInheritance()) { try { - List groupTrees = new GroupTreeResolver().resolveGroupTree(ldapGroupsRep); + List groupTrees = new GroupTreeResolver().resolveGroupTree(ldapGroupsRep, config.isIgnoreMissingGroups()); updateKeycloakGroupTree(realm, groupTrees, ldapGroupsMap, syncResult); } catch (GroupTreeResolver.GroupTreeResolveException gre) { diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java index b093bcb787..14c5ef2322 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java @@ -114,6 +114,12 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact .type(ProviderConfigProperty.BOOLEAN_TYPE) .defaultValue("true") .add() + .property().name(GroupMapperConfig.IGNORE_MISSING_GROUPS) + .label("Ignore Missing Groups") + .helpText("Ignore missing groups in the group hierarchy") + .type(ProviderConfigProperty.BOOLEAN_TYPE) + .defaultValue("false") + .add() .property().name(GroupMapperConfig.MEMBERSHIP_LDAP_ATTRIBUTE) .label("Membership LDAP Attribute") .helpText("Name of LDAP attribute on group, which is used for membership mappings. Usually it will be 'member' ." + diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupMapperConfig.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupMapperConfig.java index b305cb4eb5..36a2fc62db 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupMapperConfig.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupMapperConfig.java @@ -44,6 +44,9 @@ public class GroupMapperConfig extends CommonLDAPGroupMapperConfig { // Flag whether group inheritance from LDAP should be propagated to Keycloak group inheritance. public static final String PRESERVE_GROUP_INHERITANCE = "preserve.group.inheritance"; + // Flag whether missing groups should be ignored. + public static final String IGNORE_MISSING_GROUPS = "ignore.missing.groups"; + // Customized LDAP filter which is added to the whole LDAP query public static final String GROUPS_LDAP_FILTER = "groups.ldap.filter"; @@ -90,6 +93,10 @@ public class GroupMapperConfig extends CommonLDAPGroupMapperConfig { return AbstractLDAPStorageMapper.parseBooleanParameter(mapperModel, PRESERVE_GROUP_INHERITANCE); } + public boolean isIgnoreMissingGroups() { + return AbstractLDAPStorageMapper.parseBooleanParameter(mapperModel, IGNORE_MISSING_GROUPS); + } + public Collection getGroupObjectClasses(LDAPStorageProvider ldapProvider) { String objectClasses = mapperModel.getConfig().getFirst(GROUP_OBJECT_CLASSES); if (objectClasses == null) { diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupTreeResolver.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupTreeResolver.java index d38f361cc3..b28374ec72 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupTreeResolver.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupTreeResolver.java @@ -17,6 +17,8 @@ package org.keycloak.storage.ldap.mappers.membership.group; +import org.jboss.logging.Logger; + import java.util.Arrays; import java.util.Collection; import java.util.LinkedList; @@ -31,6 +33,8 @@ import java.util.TreeSet; */ public class GroupTreeResolver { + private static final Logger logger = Logger.getLogger(GroupTreeResolver.class); + /** * Fully resolves list of group trees to be used in Keycloak. The input is group info (usually from LDAP) where each "Group" object contains @@ -39,12 +43,13 @@ public class GroupTreeResolver { * The operation also performs validation as rules for LDAP are less strict than for Keycloak (In LDAP, the recursion is possible and multiple parents of single group is also allowed) * * @param groups + * @param ignoreMissingGroups * @return * @throws GroupTreeResolveException */ - public List resolveGroupTree(List groups) throws GroupTreeResolveException { + public List resolveGroupTree(List groups, boolean ignoreMissingGroups) throws GroupTreeResolveException { // 1- Get parents of each group - Map> parentsTree = getParentsTree(groups); + Map> parentsTree = getParentsTree(groups, ignoreMissingGroups); // 2 - Get rootGroups (groups without parent) and check if there is no group with multiple parents List rootGroups = new LinkedList<>(); @@ -96,7 +101,7 @@ public class GroupTreeResolver { return finalResult; } - private Map> getParentsTree(List groups) throws GroupTreeResolveException { + private Map> getParentsTree(List groups, boolean ignoreMissingGroups) throws GroupTreeResolveException { Map> result = new TreeMap<>(); for (Group group : groups) { @@ -106,10 +111,15 @@ public class GroupTreeResolver { for (Group group : groups) { for (String child : group.getChildrenNames()) { List list = result.get(child); - if (list == null) { + if (list != null) { + list.add(group.getGroupName()); + } else if(ignoreMissingGroups){ + // Need to remove the missing group + group.getChildrenNames().remove(child); + logger.debug("Group '" + child + "' referenced as member of group '" + group.getGroupName() + "' doesn't exists. Ignoring."); + } else { throw new GroupTreeResolveException("Group '" + child + "' referenced as member of group '" + group.getGroupName() + "' doesn't exists"); } - list.add(group.getGroupName()); } } return result; diff --git a/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/GroupTreeResolverTest.java b/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/GroupTreeResolverTest.java index 78974a2fc1..82009716c0 100644 --- a/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/GroupTreeResolverTest.java +++ b/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/GroupTreeResolverTest.java @@ -41,7 +41,7 @@ public class GroupTreeResolverTest { List groups = Arrays.asList(group1, group2, group3, group4, group5, group6, group7); GroupTreeResolver resolver = new GroupTreeResolver(); - List groupTree = resolver.resolveGroupTree(groups); + List groupTree = resolver.resolveGroupTree(groups, false); Assert.assertEquals(1, groupTree.size()); Assert.assertEquals("{ group1 -> [ { group2 -> [ { group4 -> [ ]}{ group5 -> [ ]} ]}{ group3 -> [ { group6 -> [ { group7 -> [ ]} ]} ]} ]}", groupTree.get(0).toString()); } @@ -60,7 +60,7 @@ public class GroupTreeResolverTest { List groups = Arrays.asList(group1, group2, group3, group4, group5, group6, group7, group8, group9); GroupTreeResolver resolver = new GroupTreeResolver(); - List groupTree = resolver.resolveGroupTree(groups); + List groupTree = resolver.resolveGroupTree(groups, false); Assert.assertEquals(2, groupTree.size()); Assert.assertEquals("{ group3 -> [ { group2 -> [ ]} ]}", groupTree.get(0).toString()); @@ -81,7 +81,7 @@ public class GroupTreeResolverTest { GroupTreeResolver resolver = new GroupTreeResolver(); try { - resolver.resolveGroupTree(groups); + resolver.resolveGroupTree(groups, false); Assert.fail("Exception expected because of recursion"); } catch (GroupTreeResolver.GroupTreeResolveException gre) { Assert.assertTrue(gre.getMessage().startsWith("Recursion detected")); @@ -99,7 +99,7 @@ public class GroupTreeResolverTest { GroupTreeResolver resolver = new GroupTreeResolver(); try { - resolver.resolveGroupTree(groups); + resolver.resolveGroupTree(groups, false); Assert.fail("Exception expected because of some groups have multiple parents"); } catch (GroupTreeResolver.GroupTreeResolveException gre) { Assert.assertTrue(gre.getMessage().contains("detected to have multiple parents")); @@ -108,7 +108,7 @@ public class GroupTreeResolverTest { @Test - public void testGroupResolvingMissingGroup() { + public void testGroupResolvingMissingGroup() throws GroupTreeResolver.GroupTreeResolveException { GroupTreeResolver.Group group1 = new GroupTreeResolver.Group("group1", "group2"); GroupTreeResolver.Group group2 = new GroupTreeResolver.Group("group2", "group3"); GroupTreeResolver.Group group4 = new GroupTreeResolver.Group("group4"); @@ -116,10 +116,16 @@ public class GroupTreeResolverTest { GroupTreeResolver resolver = new GroupTreeResolver(); try { - resolver.resolveGroupTree(groups); + resolver.resolveGroupTree(groups, false); Assert.fail("Exception expected because of missing referenced group"); } catch (GroupTreeResolver.GroupTreeResolveException gre) { Assert.assertEquals("Group 'group3' referenced as member of group 'group2' doesn't exists", gre.getMessage()); } + + List groupTree = resolver.resolveGroupTree(groups, true); + + Assert.assertEquals(2, groupTree.size()); + Assert.assertEquals("{ group1 -> [ { group2 -> [ ]} ]}", groupTree.get(0).toString()); + Assert.assertEquals("{ group4 -> [ ]}", groupTree.get(1).toString()); } }