HADOOP-12291. Add support for nested groups in LdapGroupsMapping. Contributed by Esther Kundin.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java

@@ -25,6 +25,9 @@
 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 @@
  * 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
+ * The implementation attempts to resolve group hierarchies,
- * order to be considered a member of a group, the user must be an explicit
+ * to a configurable limit.
- * member in LDAP.
+ * 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
@@ -156,6 +161,13 @@ public class LdapGroupsMapping
   public static final String GROUP_NAME_ATTR_KEY = LDAP_CONFIG_PREFIX + ".search.attr.group.name";
   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
    */
@@ -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 synchronized List<String> getGroups(String user) {
      */
     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 @@ private NamingEnumeration<SearchResult> lookupPosixGroup(SearchResult result,
    * @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 @@ private List<String> lookupGroup(SearchResult result, DirContext c)
     if (groupResults != null) {
       while (groupResults.hasMoreElements()) {
         SearchResult groupResult = groupResults.nextElement();
-        Attribute groupName = groupResult.getAttributes().get(groupNameAttr);
+        getGroupNames(groupResult, groups, groupDNs, goUpHierarchy > 0);
-        if (groupName == null) {
+      }
-          throw new NamingException("The group object does not have " +
+      if (goUpHierarchy > 0 && !isPosix) {
-              "attribute '" + groupNameAttr + "'.");
+        // convert groups to a set to ensure uniqueness
-        }
+        Set<String> groupset = new HashSet<String>(groups);
-        groups.add(groupName.get().toString());
+        goUpGroupHierarchy(groupDNs, goUpHierarchy, groupset);
+        // convert set back to list for compatibility
+        groups = new ArrayList<String>(groupset);
       }
     }
     return groups;
@@ -369,7 +386,8 @@ private List<String> lookupGroup(SearchResult result, DirContext c)
    * 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 @@ List<String> doGetGroups(String user) throws NamingException {
     // 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 @@ List<String> doGetGroups(String user) throws NamingException {
                 "the second LDAP query using the user's DN.", e);
       }
     }
-    if (groups == null || groups.isEmpty()) {
+    if (groups == null || groups.isEmpty() || goUpHierarchy > 0) {
-      groups = lookupGroup(result, c);
+      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 @@ DirContext getDirContext() throws NamingException {
 
       ctx = new InitialDirContext(env);
     }
-
     return ctx;
   }
   
@@ -513,6 +591,8 @@ public synchronized void setConf(Configuration conf) {
         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 =

hadoop-common-project/hadoop-common/src/main/resources/core-default.xml

@@ -324,6 +324,19 @@
   </description>
 </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>

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java

@@ -39,6 +39,7 @@
 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 void testGetGroups() throws IOException, NamingException {
     when(getContext().search(anyString(), anyString(), any(Object[].class),
         any(SearchControls.class)))
         .thenReturn(getUserNames(), getGroupNames());
-    
+    doTestGetGroups(Arrays.asList(getTestGroups()), 2);
-    doTestGetGroups(Arrays.asList(testGroups), 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 void testGetGroupsWithConnectionClosed() throws IOException, NamingExcept
         .thenThrow(new CommunicationException("Connection is closed"))
         .thenReturn(getUserNames(), getGroupNames());
     
-    // Although connection is down but after reconnected it still should retrieve the result groups
+    // Although connection is down but after reconnected
-    doTestGetGroups(Arrays.asList(testGroups), 1 + 2); // 1 is the first failure call 
+    // it still should retrieve the result groups
+    // 1 is the first failure call
+    doTestGetGroups(Arrays.asList(getTestGroups()), 1 + 2);
   }
 
   @Test
@@ -139,7 +157,37 @@ private void doTestGetGroups(List<String> expectedGroups, int searchTimes) throw
                                          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 void run() {
       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 void run() {
       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);

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 void setupMocksBase() throws NamingException {
         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 @@ protected Attributes getAttributes() {
   protected LdapGroupsMapping getGroupsMapping() {
     return groupsMapping;
   }
+  protected String[] getTestGroups() {
+    return testGroups;
+  }
+  protected NamingEnumeration getParentGroupNames() {
+    return parentGroupNames;
+  }
+  protected String[] getTestParentGroups() {
+    return testParentGroups;
+  }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithPosixGroup.java

@@ -69,7 +69,7 @@ public void testGetGroups() throws IOException, NamingException {
         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)