Merge remote-tracking branch 'upstream/master'

This commit is contained in:
Bill Burke 2017-09-22 11:48:41 -04:00
commit 9d452b4bc3
7 changed files with 45 additions and 16 deletions

View file

@ -175,7 +175,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
// Now we have list of LDAP groups. Let's form the tree (if needed) // Now we have list of LDAP groups. Let's form the tree (if needed)
if (config.isPreserveGroupsInheritance()) { if (config.isPreserveGroupsInheritance()) {
try { try {
List<GroupTreeResolver.GroupTreeEntry> groupTrees = new GroupTreeResolver().resolveGroupTree(ldapGroupsRep); List<GroupTreeResolver.GroupTreeEntry> groupTrees = new GroupTreeResolver().resolveGroupTree(ldapGroupsRep, config.isIgnoreMissingGroups());
updateKeycloakGroupTree(realm, groupTrees, ldapGroupsMap, syncResult); updateKeycloakGroupTree(realm, groupTrees, ldapGroupsMap, syncResult);
} catch (GroupTreeResolver.GroupTreeResolveException gre) { } catch (GroupTreeResolver.GroupTreeResolveException gre) {

View file

@ -114,6 +114,12 @@ public class GroupLDAPStorageMapperFactory extends AbstractLDAPStorageMapperFact
.type(ProviderConfigProperty.BOOLEAN_TYPE) .type(ProviderConfigProperty.BOOLEAN_TYPE)
.defaultValue("true") .defaultValue("true")
.add() .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) .property().name(GroupMapperConfig.MEMBERSHIP_LDAP_ATTRIBUTE)
.label("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' ." + .helpText("Name of LDAP attribute on group, which is used for membership mappings. Usually it will be 'member' ." +

View file

@ -44,6 +44,9 @@ public class GroupMapperConfig extends CommonLDAPGroupMapperConfig {
// Flag whether group inheritance from LDAP should be propagated to Keycloak group inheritance. // Flag whether group inheritance from LDAP should be propagated to Keycloak group inheritance.
public static final String PRESERVE_GROUP_INHERITANCE = "preserve.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 // Customized LDAP filter which is added to the whole LDAP query
public static final String GROUPS_LDAP_FILTER = "groups.ldap.filter"; 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); return AbstractLDAPStorageMapper.parseBooleanParameter(mapperModel, PRESERVE_GROUP_INHERITANCE);
} }
public boolean isIgnoreMissingGroups() {
return AbstractLDAPStorageMapper.parseBooleanParameter(mapperModel, IGNORE_MISSING_GROUPS);
}
public Collection<String> getGroupObjectClasses(LDAPStorageProvider ldapProvider) { public Collection<String> getGroupObjectClasses(LDAPStorageProvider ldapProvider) {
String objectClasses = mapperModel.getConfig().getFirst(GROUP_OBJECT_CLASSES); String objectClasses = mapperModel.getConfig().getFirst(GROUP_OBJECT_CLASSES);
if (objectClasses == null) { if (objectClasses == null) {

View file

@ -17,6 +17,8 @@
package org.keycloak.storage.ldap.mappers.membership.group; package org.keycloak.storage.ldap.mappers.membership.group;
import org.jboss.logging.Logger;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collection; import java.util.Collection;
import java.util.LinkedList; import java.util.LinkedList;
@ -31,6 +33,8 @@ import java.util.TreeSet;
*/ */
public class GroupTreeResolver { 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 * 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) * 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 groups
* @param ignoreMissingGroups
* @return * @return
* @throws GroupTreeResolveException * @throws GroupTreeResolveException
*/ */
public List<GroupTreeEntry> resolveGroupTree(List<Group> groups) throws GroupTreeResolveException { public List<GroupTreeEntry> resolveGroupTree(List<Group> groups, boolean ignoreMissingGroups) throws GroupTreeResolveException {
// 1- Get parents of each group // 1- Get parents of each group
Map<String, List<String>> parentsTree = getParentsTree(groups); Map<String, List<String>> parentsTree = getParentsTree(groups, ignoreMissingGroups);
// 2 - Get rootGroups (groups without parent) and check if there is no group with multiple parents // 2 - Get rootGroups (groups without parent) and check if there is no group with multiple parents
List<String> rootGroups = new LinkedList<>(); List<String> rootGroups = new LinkedList<>();
@ -96,7 +101,7 @@ public class GroupTreeResolver {
return finalResult; return finalResult;
} }
private Map<String, List<String>> getParentsTree(List<Group> groups) throws GroupTreeResolveException { private Map<String, List<String>> getParentsTree(List<Group> groups, boolean ignoreMissingGroups) throws GroupTreeResolveException {
Map<String, List<String>> result = new TreeMap<>(); Map<String, List<String>> result = new TreeMap<>();
for (Group group : groups) { for (Group group : groups) {
@ -106,10 +111,15 @@ public class GroupTreeResolver {
for (Group group : groups) { for (Group group : groups) {
for (String child : group.getChildrenNames()) { for (String child : group.getChildrenNames()) {
List<String> list = result.get(child); List<String> 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"); throw new GroupTreeResolveException("Group '" + child + "' referenced as member of group '" + group.getGroupName() + "' doesn't exists");
} }
list.add(group.getGroupName());
} }
} }
return result; return result;

View file

@ -41,7 +41,7 @@ public class GroupTreeResolverTest {
List<GroupTreeResolver.Group> groups = Arrays.asList(group1, group2, group3, group4, group5, group6, group7); List<GroupTreeResolver.Group> groups = Arrays.asList(group1, group2, group3, group4, group5, group6, group7);
GroupTreeResolver resolver = new GroupTreeResolver(); GroupTreeResolver resolver = new GroupTreeResolver();
List<GroupTreeResolver.GroupTreeEntry> groupTree = resolver.resolveGroupTree(groups); List<GroupTreeResolver.GroupTreeEntry> groupTree = resolver.resolveGroupTree(groups, false);
Assert.assertEquals(1, groupTree.size()); Assert.assertEquals(1, groupTree.size());
Assert.assertEquals("{ group1 -> [ { group2 -> [ { group4 -> [ ]}{ group5 -> [ ]} ]}{ group3 -> [ { group6 -> [ { group7 -> [ ]} ]} ]} ]}", groupTree.get(0).toString()); Assert.assertEquals("{ group1 -> [ { group2 -> [ { group4 -> [ ]}{ group5 -> [ ]} ]}{ group3 -> [ { group6 -> [ { group7 -> [ ]} ]} ]} ]}", groupTree.get(0).toString());
} }
@ -60,7 +60,7 @@ public class GroupTreeResolverTest {
List<GroupTreeResolver.Group> groups = Arrays.asList(group1, group2, group3, group4, group5, group6, group7, group8, group9); List<GroupTreeResolver.Group> groups = Arrays.asList(group1, group2, group3, group4, group5, group6, group7, group8, group9);
GroupTreeResolver resolver = new GroupTreeResolver(); GroupTreeResolver resolver = new GroupTreeResolver();
List<GroupTreeResolver.GroupTreeEntry> groupTree = resolver.resolveGroupTree(groups); List<GroupTreeResolver.GroupTreeEntry> groupTree = resolver.resolveGroupTree(groups, false);
Assert.assertEquals(2, groupTree.size()); Assert.assertEquals(2, groupTree.size());
Assert.assertEquals("{ group3 -> [ { group2 -> [ ]} ]}", groupTree.get(0).toString()); Assert.assertEquals("{ group3 -> [ { group2 -> [ ]} ]}", groupTree.get(0).toString());
@ -81,7 +81,7 @@ public class GroupTreeResolverTest {
GroupTreeResolver resolver = new GroupTreeResolver(); GroupTreeResolver resolver = new GroupTreeResolver();
try { try {
resolver.resolveGroupTree(groups); resolver.resolveGroupTree(groups, false);
Assert.fail("Exception expected because of recursion"); Assert.fail("Exception expected because of recursion");
} catch (GroupTreeResolver.GroupTreeResolveException gre) { } catch (GroupTreeResolver.GroupTreeResolveException gre) {
Assert.assertTrue(gre.getMessage().startsWith("Recursion detected")); Assert.assertTrue(gre.getMessage().startsWith("Recursion detected"));
@ -99,7 +99,7 @@ public class GroupTreeResolverTest {
GroupTreeResolver resolver = new GroupTreeResolver(); GroupTreeResolver resolver = new GroupTreeResolver();
try { try {
resolver.resolveGroupTree(groups); resolver.resolveGroupTree(groups, false);
Assert.fail("Exception expected because of some groups have multiple parents"); Assert.fail("Exception expected because of some groups have multiple parents");
} catch (GroupTreeResolver.GroupTreeResolveException gre) { } catch (GroupTreeResolver.GroupTreeResolveException gre) {
Assert.assertTrue(gre.getMessage().contains("detected to have multiple parents")); Assert.assertTrue(gre.getMessage().contains("detected to have multiple parents"));
@ -108,7 +108,7 @@ public class GroupTreeResolverTest {
@Test @Test
public void testGroupResolvingMissingGroup() { public void testGroupResolvingMissingGroup() throws GroupTreeResolver.GroupTreeResolveException {
GroupTreeResolver.Group group1 = new GroupTreeResolver.Group("group1", "group2"); GroupTreeResolver.Group group1 = new GroupTreeResolver.Group("group1", "group2");
GroupTreeResolver.Group group2 = new GroupTreeResolver.Group("group2", "group3"); GroupTreeResolver.Group group2 = new GroupTreeResolver.Group("group2", "group3");
GroupTreeResolver.Group group4 = new GroupTreeResolver.Group("group4"); GroupTreeResolver.Group group4 = new GroupTreeResolver.Group("group4");
@ -116,10 +116,16 @@ public class GroupTreeResolverTest {
GroupTreeResolver resolver = new GroupTreeResolver(); GroupTreeResolver resolver = new GroupTreeResolver();
try { try {
resolver.resolveGroupTree(groups); resolver.resolveGroupTree(groups, false);
Assert.fail("Exception expected because of missing referenced group"); Assert.fail("Exception expected because of missing referenced group");
} catch (GroupTreeResolver.GroupTreeResolveException gre) { } catch (GroupTreeResolver.GroupTreeResolveException gre) {
Assert.assertEquals("Group 'group3' referenced as member of group 'group2' doesn't exists", gre.getMessage()); Assert.assertEquals("Group 'group3' referenced as member of group 'group2' doesn't exists", gre.getMessage());
} }
List<GroupTreeResolver.GroupTreeEntry> 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());
} }
} }

View file

@ -138,7 +138,7 @@ public class SetPasswordCmd extends AbstractAuthOptionsCmd {
public static String usage() { public static String usage() {
StringWriter sb = new StringWriter(); StringWriter sb = new StringWriter();
PrintWriter out = new PrintWriter(sb); PrintWriter out = new PrintWriter(sb);
out.println("Usage: " + CMD + " set-password (--username USERNAME | --userid ID) [--password PASSWORD] [ARGUMENTS]"); out.println("Usage: " + CMD + " set-password (--username USERNAME | --userid ID) [--new-password PASSWORD] [ARGUMENTS]");
out.println(); out.println();
out.println("Command to reset user's password."); out.println("Command to reset user's password.");
out.println(); out.println();
@ -167,7 +167,7 @@ public class SetPasswordCmd extends AbstractAuthOptionsCmd {
out.println("Examples:"); out.println("Examples:");
out.println(); out.println();
out.println("Set new temporary password for the user:"); out.println("Set new temporary password for the user:");
out.println(" " + PROMPT + " " + CMD + " set-password -r demorealm --username testuser --password NEWPASS -t"); out.println(" " + PROMPT + " " + CMD + " set-password -r demorealm --username testuser --new-password NEWPASS -t");
out.println(); out.println();
out.println(); out.println();
out.println("Use '" + CMD + " help' for general information and a list of commands"); out.println("Use '" + CMD + " help' for general information and a list of commands");

View file

@ -123,7 +123,7 @@ public class KcAdmTest extends AbstractAdmCliTest {
exe = KcAdmExec.execute("set-password"); exe = KcAdmExec.execute("set-password");
assertExitCodeAndStdErrSize(exe, 1, 0); assertExitCodeAndStdErrSize(exe, 1, 0);
Assert.assertTrue("help message returned", exe.stdoutLines().size() > 10); Assert.assertTrue("help message returned", exe.stdoutLines().size() > 10);
Assert.assertEquals("help message", "Usage: " + CMD + " set-password (--username USERNAME | --userid ID) [--password PASSWORD] [ARGUMENTS]", exe.stdoutLines().get(0)); Assert.assertEquals("help message", "Usage: " + CMD + " set-password (--username USERNAME | --userid ID) [--new-password PASSWORD] [ARGUMENTS]", exe.stdoutLines().get(0));
//Assert.assertEquals("error message", "CLIENT not specified", exe.stderrLines().get(0)); //Assert.assertEquals("error message", "CLIENT not specified", exe.stderrLines().get(0));
exe = KcAdmExec.execute("help"); exe = KcAdmExec.execute("help");
@ -174,7 +174,7 @@ public class KcAdmTest extends AbstractAdmCliTest {
exe = KcAdmExec.execute("set-password --help"); exe = KcAdmExec.execute("set-password --help");
assertExitCodeAndStdErrSize(exe, 0, 0); assertExitCodeAndStdErrSize(exe, 0, 0);
Assert.assertEquals("stdout first line", "Usage: " + CMD + " set-password (--username USERNAME | --userid ID) [--password PASSWORD] [ARGUMENTS]", exe.stdoutLines().get(0)); Assert.assertEquals("stdout first line", "Usage: " + CMD + " set-password (--username USERNAME | --userid ID) [--new-password PASSWORD] [ARGUMENTS]", exe.stdoutLines().get(0));
exe = KcAdmExec.execute("config --help"); exe = KcAdmExec.execute("config --help");
assertExitCodeAndStdErrSize(exe, 0, 0); assertExitCodeAndStdErrSize(exe, 0, 0);