HADOOP-12291. Add support for nested groups in LdapGroupsMapping. Contributed by Esther Kundin.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/6f0aa751 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/6f0aa751 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/6f0aa751 Branch: refs/heads/HDFS-7240 Commit: 6f0aa75121224589fe1e20630c597f851ef3bed2 Parents: 25064fb Author: Jitendra Pandey <jiten...@apache.org> Authored: Wed Jun 15 11:41:49 2016 -0700 Committer: Jitendra Pandey <jiten...@apache.org> Committed: Wed Jun 15 11:41:49 2016 -0700 ---------------------------------------------------------------------- .../hadoop/security/LdapGroupsMapping.java | 114 ++++++++++++++++--- .../src/main/resources/core-default.xml | 13 +++ .../hadoop/security/TestLdapGroupsMapping.java | 62 ++++++++-- .../security/TestLdapGroupsMappingBase.java | 33 +++++- .../TestLdapGroupsMappingWithPosixGroup.java | 2 +- 5 files changed, 198 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/6f0aa751/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java index da87369..5a0b1d9 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java @@ -25,6 +25,9 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Hashtable; import java.util.List; +import java.util.HashSet; +import java.util.Collection; +import java.util.Set; import javax.naming.Context; import javax.naming.NamingEnumeration; @@ -66,9 +69,11 @@ import org.apache.hadoop.conf.Configuration; * is used for searching users or groups which returns more results than are * allowed by the server, an exception will be thrown. * - * The implementation also does not attempt to resolve group hierarchies. In - * order to be considered a member of a group, the user must be an explicit - * member in LDAP. + * The implementation attempts to resolve group hierarchies, + * to a configurable limit. + * If the limit is 0, in order to be considered a member of a group, + * the user must be an explicit member in LDAP. Otherwise, it will traverse the + * group hierarchy n levels up. */ @InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"}) @InterfaceStability.Evolving @@ -157,6 +162,13 @@ public class LdapGroupsMapping public static final String GROUP_NAME_ATTR_DEFAULT = "cn"; /* + * How many levels to traverse when checking for groups in the org hierarchy + */ + public static final String GROUP_HIERARCHY_LEVELS_KEY = + LDAP_CONFIG_PREFIX + ".search.group.hierarchy.levels"; + public static final int GROUP_HIERARCHY_LEVELS_DEFAULT = 0; + + /* * LDAP attribute names to use when doing posix-like lookups */ public static final String POSIX_UID_ATTR_KEY = LDAP_CONFIG_PREFIX + ".posix.attr.uid.name"; @@ -208,6 +220,7 @@ public class LdapGroupsMapping private String memberOfAttr; private String groupMemberAttr; private String groupNameAttr; + private int groupHierarchyLevels; private String posixUidAttr; private String posixGidAttr; private boolean isPosix; @@ -234,7 +247,7 @@ public class LdapGroupsMapping */ for(int retry = 0; retry < RECONNECT_RETRY_COUNT; retry++) { try { - return doGetGroups(user); + return doGetGroups(user, groupHierarchyLevels); } catch (NamingException e) { LOG.warn("Failed to get groups for user " + user + " (retry=" + retry + ") by " + e); @@ -324,9 +337,11 @@ public class LdapGroupsMapping * @return a list of strings representing group names of the user. * @throws NamingException if unable to find group names */ - private List<String> lookupGroup(SearchResult result, DirContext c) + private List<String> lookupGroup(SearchResult result, DirContext c, + int goUpHierarchy) throws NamingException { List<String> groups = new ArrayList<String>(); + Set<String> groupDNs = new HashSet<String>(); NamingEnumeration<SearchResult> groupResults = null; // perform the second LDAP query @@ -345,12 +360,14 @@ public class LdapGroupsMapping if (groupResults != null) { while (groupResults.hasMoreElements()) { SearchResult groupResult = groupResults.nextElement(); - Attribute groupName = groupResult.getAttributes().get(groupNameAttr); - if (groupName == null) { - throw new NamingException("The group object does not have " + - "attribute '" + groupNameAttr + "'."); - } - groups.add(groupName.get().toString()); + getGroupNames(groupResult, groups, groupDNs, goUpHierarchy > 0); + } + if (goUpHierarchy > 0 && !isPosix) { + // convert groups to a set to ensure uniqueness + Set<String> groupset = new HashSet<String>(groups); + goUpGroupHierarchy(groupDNs, goUpHierarchy, groupset); + // convert set back to list for compatibility + groups = new ArrayList<String>(groupset); } } return groups; @@ -369,7 +386,8 @@ public class LdapGroupsMapping * return an empty string array. * @throws NamingException if unable to get group names */ - List<String> doGetGroups(String user) throws NamingException { + List<String> doGetGroups(String user, int goUpHierarchy) + throws NamingException { DirContext c = getDirContext(); // Search for the user. We'll only ever need to look at the first result @@ -378,7 +396,7 @@ public class LdapGroupsMapping // return empty list if the user can not be found. if (!results.hasMoreElements()) { if (LOG.isDebugEnabled()) { - LOG.debug("doGetGroups(" + user + ") return no groups because the " + + LOG.debug("doGetGroups(" + user + ") returned no groups because the " + "user is not found."); } return new ArrayList<String>(); @@ -411,15 +429,76 @@ public class LdapGroupsMapping "the second LDAP query using the user's DN.", e); } } - if (groups == null || groups.isEmpty()) { - groups = lookupGroup(result, c); + if (groups == null || groups.isEmpty() || goUpHierarchy > 0) { + groups = lookupGroup(result, c, goUpHierarchy); } if (LOG.isDebugEnabled()) { - LOG.debug("doGetGroups(" + user + ") return " + groups); + LOG.debug("doGetGroups(" + user + ") returned " + groups); } return groups; } + /* Helper function to get group name from search results. + */ + void getGroupNames(SearchResult groupResult, Collection<String> groups, + Collection<String> groupDNs, boolean doGetDNs) + throws NamingException { + Attribute groupName = groupResult.getAttributes().get(groupNameAttr); + if (groupName == null) { + throw new NamingException("The group object does not have " + + "attribute '" + groupNameAttr + "'."); + } + groups.add(groupName.get().toString()); + if (doGetDNs) { + groupDNs.add(groupResult.getNameInNamespace()); + } + } + + /* Implementation for walking up the ldap hierarchy + * This function will iteratively find the super-group memebership of + * groups listed in groupDNs and add them to + * the groups set. It will walk up the hierarchy goUpHierarchy levels. + * Note: This is an expensive operation and settings higher than 1 + * are NOT recommended as they will impact both the speed and + * memory usage of all operations. + * The maximum time for this function will be bounded by the ldap query + * timeout and the number of ldap queries that it will make, which is + * max(Recur Depth in LDAP, goUpHierarcy) * DIRECTORY_SEARCH_TIMEOUT + * + * @param ctx - The context for contacting the ldap server + * @param groupDNs - the distinguished name of the groups whose parents we + * want to look up + * @param goUpHierarchy - the number of levels to go up, + * @param groups - Output variable to store all groups that will be added + */ + void goUpGroupHierarchy(Set<String> groupDNs, + int goUpHierarchy, + Set<String> groups) + throws NamingException { + if (goUpHierarchy <= 0 || groups.isEmpty()) { + return; + } + DirContext context = getDirContext(); + Set<String> nextLevelGroups = new HashSet<String>(); + StringBuilder filter = new StringBuilder(); + filter.append("(&").append(groupSearchFilter).append("(|"); + for (String dn : groupDNs) { + filter.append("(").append(groupMemberAttr).append("=") + .append(dn).append(")"); + } + filter.append("))"); + LOG.debug("Ldap group query string: " + filter.toString()); + NamingEnumeration<SearchResult> groupResults = + context.search(baseDN, + filter.toString(), + SEARCH_CONTROLS); + while (groupResults.hasMoreElements()) { + SearchResult groupResult = groupResults.nextElement(); + getGroupNames(groupResult, groups, nextLevelGroups, true); + } + goUpGroupHierarchy(nextLevelGroups, goUpHierarchy - 1, groups); + } + DirContext getDirContext() throws NamingException { if (ctx == null) { // Set up the initial environment for LDAP connectivity @@ -446,7 +525,6 @@ public class LdapGroupsMapping ctx = new InitialDirContext(env); } - return ctx; } @@ -513,6 +591,8 @@ public class LdapGroupsMapping conf.get(GROUP_MEMBERSHIP_ATTR_KEY, GROUP_MEMBERSHIP_ATTR_DEFAULT); groupNameAttr = conf.get(GROUP_NAME_ATTR_KEY, GROUP_NAME_ATTR_DEFAULT); + groupHierarchyLevels = + conf.getInt(GROUP_HIERARCHY_LEVELS_KEY, GROUP_HIERARCHY_LEVELS_DEFAULT); posixUidAttr = conf.get(POSIX_UID_ATTR_KEY, POSIX_UID_ATTR_DEFAULT); posixGidAttr = http://git-wip-us.apache.org/repos/asf/hadoop/blob/6f0aa751/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml index f1d77dd..089a2c1 100644 --- a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml +++ b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml @@ -316,6 +316,19 @@ </property> <property> + <name>hadoop.security.group.mapping.ldap.search.group.hierarchy.levels</name> + <value>0</value> + <description> + The number of levels to go up the group hierarchy when determining + which groups a user is part of. 0 Will represent checking just the + group that the user belongs to. Each additional level will raise the + time it takes to exectue a query by at most + hadoop.security.group.mapping.ldap.directory.search.timeout. + The default will usually be appropriate for all LDAP systems. + </description> +</property> + +<property> <name>hadoop.security.group.mapping.ldap.posix.attr.uid.name</name> <value>uidNumber</value> <description> http://git-wip-us.apache.org/repos/asf/hadoop/blob/6f0aa751/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java index 9f9f994..131b4e6 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java @@ -39,6 +39,7 @@ import java.net.Socket; import java.util.Arrays; import java.util.List; import java.util.concurrent.CountDownLatch; +import java.util.HashSet; import javax.naming.CommunicationException; import javax.naming.NamingException; @@ -91,8 +92,23 @@ public class TestLdapGroupsMapping extends TestLdapGroupsMappingBase { when(getContext().search(anyString(), anyString(), any(Object[].class), any(SearchControls.class))) .thenReturn(getUserNames(), getGroupNames()); - - doTestGetGroups(Arrays.asList(testGroups), 2); + doTestGetGroups(Arrays.asList(getTestGroups()), 2); + } + + @Test + public void testGetGroupsWithHierarchy() throws IOException, NamingException { + // The search functionality of the mock context is reused, so we will + // return the user NamingEnumeration first, and then the group + // The parent search is run once for each level, and is a different search + // The parent group is returned once for each group, yet the final list + // should be unique + when(getContext().search(anyString(), anyString(), any(Object[].class), + any(SearchControls.class))) + .thenReturn(getUserNames(), getGroupNames()); + when(getContext().search(anyString(), anyString(), + any(SearchControls.class))) + .thenReturn(getParentGroupNames()); + doTestGetGroupsWithParent(Arrays.asList(getTestParentGroups()), 2, 1); } @Test @@ -104,8 +120,10 @@ public class TestLdapGroupsMapping extends TestLdapGroupsMappingBase { .thenThrow(new CommunicationException("Connection is closed")) .thenReturn(getUserNames(), getGroupNames()); - // Although connection is down but after reconnected it still should retrieve the result groups - doTestGetGroups(Arrays.asList(testGroups), 1 + 2); // 1 is the first failure call + // Although connection is down but after reconnected + // it still should retrieve the result groups + // 1 is the first failure call + doTestGetGroups(Arrays.asList(getTestGroups()), 1 + 2); } @Test @@ -139,7 +157,37 @@ public class TestLdapGroupsMapping extends TestLdapGroupsMappingBase { any(Object[].class), any(SearchControls.class)); } - + + private void doTestGetGroupsWithParent(List<String> expectedGroups, + int searchTimesGroup, int searchTimesParentGroup) + throws IOException, NamingException { + Configuration conf = new Configuration(); + // Set this, so we don't throw an exception + conf.set(LdapGroupsMapping.LDAP_URL_KEY, "ldap://test";); + // Set the config to get parents 1 level up + conf.setInt(LdapGroupsMapping.GROUP_HIERARCHY_LEVELS_KEY, 1); + + LdapGroupsMapping groupsMapping = getGroupsMapping(); + groupsMapping.setConf(conf); + // Username is arbitrary, since the spy is mocked to respond the same, + // regardless of input + List<String> groups = groupsMapping.getGroups("some_user"); + + // compare lists, ignoring the order + Assert.assertEquals(new HashSet<String>(expectedGroups), + new HashSet<String>(groups)); + + // We should have searched for a user, and group + verify(getContext(), times(searchTimesGroup)).search(anyString(), + anyString(), + any(Object[].class), + any(SearchControls.class)); + // One groups search for the parent group should have been done + verify(getContext(), times(searchTimesParentGroup)).search(anyString(), + anyString(), + any(SearchControls.class)); + } + @Test public void testExtractPassword() throws IOException { File testDir = GenericTestUtils.getTestDir(); @@ -246,7 +294,7 @@ public class TestLdapGroupsMapping extends TestLdapGroupsMappingBase { mapping.setConf(conf); try { - mapping.doGetGroups("hadoop"); + mapping.doGetGroups("hadoop", 1); fail("The LDAP query should have timed out!"); } catch (NamingException ne) { LOG.debug("Got the exception while LDAP querying: ", ne); @@ -302,7 +350,7 @@ public class TestLdapGroupsMapping extends TestLdapGroupsMappingBase { mapping.setConf(conf); try { - mapping.doGetGroups("hadoop"); + mapping.doGetGroups("hadoop", 1); fail("The LDAP query should have timed out!"); } catch (NamingException ne) { LOG.debug("Got the exception while LDAP querying: ", ne); http://git-wip-us.apache.org/repos/asf/hadoop/blob/6f0aa751/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingBase.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingBase.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingBase.java index 75e3bf1..51d3673 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingBase.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingBase.java @@ -46,13 +46,17 @@ public class TestLdapGroupsMappingBase { @Mock private NamingEnumeration<SearchResult> groupNames; @Mock + private NamingEnumeration<SearchResult> parentGroupNames; + @Mock private SearchResult userSearchResult; @Mock private Attributes attributes; @Spy private LdapGroupsMapping groupsMapping = new LdapGroupsMapping(); - protected String[] testGroups = new String[] {"group1", "group2"}; + private String[] testGroups = new String[] {"group1", "group2"}; + private String[] testParentGroups = + new String[] {"group1", "group2", "group1_1"}; @Before public void setupMocksBase() throws NamingException { @@ -93,6 +97,24 @@ public class TestLdapGroupsMappingBase { thenReturn(getUserSearchResult()); when(getUserSearchResult().getAttributes()).thenReturn(getAttributes()); + // Define results for groups 1 level up + SearchResult parentGroupResult = mock(SearchResult.class); + + // only one parent group + when(parentGroupNames.hasMoreElements()).thenReturn(true, false); + when(parentGroupNames.nextElement()). + thenReturn(parentGroupResult); + + // Define the attribute for the parent group + Attribute parentGroup1Attr = new BasicAttribute("cn"); + parentGroup1Attr.add(testParentGroups[2]); + Attributes parentGroup1Attrs = new BasicAttributes(); + parentGroup1Attrs.put(parentGroup1Attr); + + // attach the attributes to the result + when(parentGroupResult.getAttributes()).thenReturn(parentGroup1Attrs); + when(parentGroupResult.getNameInNamespace()). + thenReturn("CN=some_group,DC=test,DC=com"); } protected DirContext getContext() { @@ -117,4 +139,13 @@ public class TestLdapGroupsMappingBase { protected LdapGroupsMapping getGroupsMapping() { return groupsMapping; } + protected String[] getTestGroups() { + return testGroups; + } + protected NamingEnumeration getParentGroupNames() { + return parentGroupNames; + } + protected String[] getTestParentGroups() { + return testParentGroups; + } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/6f0aa751/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithPosixGroup.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithPosixGroup.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithPosixGroup.java index 332eed4..17d28a5 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithPosixGroup.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithPosixGroup.java @@ -69,7 +69,7 @@ public class TestLdapGroupsMappingWithPosixGroup any(Object[].class), any(SearchControls.class))) .thenReturn(getUserNames(), getGroupNames()); - doTestGetGroups(Arrays.asList(testGroups), 2); + doTestGetGroups(Arrays.asList(getTestGroups()), 2); } private void doTestGetGroups(List<String> expectedGroups, int searchTimes) --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
