You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by st...@apache.org on 2022/07/14 12:54:22 UTC
[hadoop] branch branch-3.3 updated: HADOOP-18074 - Partial/Incomplete groups list can be returned in LDAP. (#4503)
This is an automated email from the ASF dual-hosted git repository.
stevel pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/branch-3.3 by this push:
new f015c7f1a7c HADOOP-18074 - Partial/Incomplete groups list can be returned in LDAP. (#4503)
f015c7f1a7c is described below
commit f015c7f1a7cd1df92c85b3f043245168d181f271
Author: lmccay <lm...@apache.org>
AuthorDate: Thu Jul 14 08:54:15 2022 -0400
HADOOP-18074 - Partial/Incomplete groups list can be returned in LDAP. (#4503)
Partial/Incomplete groups list can be returned in LDAP groups lookup.
Backported in #4550; minor tuning of parameters needed.
Contributed by larry mccay
---
.../apache/hadoop/security/LdapGroupsMapping.java | 13 +--
.../TestLdapGroupsMappingWithOneQuery.java | 99 +++++++++++++++++++---
2 files changed, 95 insertions(+), 17 deletions(-)
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 e4d6225fe87..e751ed6a373 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
@@ -58,6 +58,7 @@ import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.TrustManager;
import javax.net.ssl.TrustManagerFactory;
+import org.apache.hadoop.classification.VisibleForTesting;
import org.apache.hadoop.thirdparty.com.google.common.collect.Iterators;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
@@ -458,7 +459,8 @@ 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,
+ @VisibleForTesting
+ List<String> lookupGroup(SearchResult result, DirContext c,
int goUpHierarchy)
throws NamingException {
List<String> groups = new ArrayList<>();
@@ -510,6 +512,7 @@ public class LdapGroupsMapping
List<String> doGetGroups(String user, int goUpHierarchy)
throws NamingException {
DirContext c = getDirContext();
+ List<String> groups = new ArrayList<>();
// Search for the user. We'll only ever need to look at the first result
NamingEnumeration<SearchResult> results = c.search(userbaseDN,
@@ -518,11 +521,10 @@ public class LdapGroupsMapping
if (!results.hasMoreElements()) {
LOG.debug("doGetGroups({}) returned no groups because the " +
"user is not found.", user);
- return new ArrayList<>();
+ return groups;
}
SearchResult result = results.nextElement();
- List<String> groups = null;
if (useOneQuery) {
try {
/**
@@ -536,7 +538,6 @@ public class LdapGroupsMapping
memberOfAttr + "' attribute." +
"Returned user object: " + result.toString());
}
- groups = new ArrayList<>();
NamingEnumeration groupEnumeration = groupDNAttr.getAll();
while (groupEnumeration.hasMore()) {
String groupDN = groupEnumeration.next().toString();
@@ -544,11 +545,13 @@ public class LdapGroupsMapping
}
} catch (NamingException e) {
// If the first lookup failed, fall back to the typical scenario.
+ // In order to force the fallback, we need to reset groups collection.
+ groups.clear();
LOG.info("Failed to get groups from the first lookup. Initiating " +
"the second LDAP query using the user's DN.", e);
}
}
- if (groups == null || groups.isEmpty() || goUpHierarchy > 0) {
+ if (groups.isEmpty() || goUpHierarchy > 0) {
groups = lookupGroup(result, c, goUpHierarchy);
}
LOG.debug("doGetGroups({}) returned {}", user, groups);
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithOneQuery.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithOneQuery.java
index 7ae802e26d3..c86f1768b7f 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithOneQuery.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithOneQuery.java
@@ -18,19 +18,21 @@
package org.apache.hadoop.security;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import javax.naming.NamingEnumeration;
import javax.naming.NamingException;
import javax.naming.directory.Attribute;
+import javax.naming.directory.DirContext;
import javax.naming.directory.SearchControls;
import javax.naming.directory.SearchResult;
import org.apache.hadoop.conf.Configuration;
import org.junit.Assert;
-import org.junit.Before;
import org.junit.Test;
+import org.mockito.stubbing.Stubber;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
@@ -49,48 +51,121 @@ import static org.mockito.Mockito.when;
public class TestLdapGroupsMappingWithOneQuery
extends TestLdapGroupsMappingBase {
- @Before
- public void setupMocks() throws NamingException {
+ public void setupMocks(List<String> listOfDNs) throws NamingException {
Attribute groupDN = mock(Attribute.class);
NamingEnumeration<SearchResult> groupNames = getGroupNames();
doReturn(groupNames).when(groupDN).getAll();
- String groupName1 = "CN=abc,DC=foo,DC=bar,DC=com";
- String groupName2 = "CN=xyz,DC=foo,DC=bar,DC=com";
- String groupName3 = "CN=sss,CN=foo,DC=bar,DC=com";
- doReturn(groupName1).doReturn(groupName2).doReturn(groupName3).
- when(groupNames).next();
- when(groupNames.hasMore()).thenReturn(true).thenReturn(true).
- thenReturn(true).thenReturn(false);
+ buildListOfGroupDNs(listOfDNs).when(groupNames).next();
+ when(groupNames.hasMore()).
+ thenReturn(true).thenReturn(true).
+ thenReturn(true).thenReturn(false);
when(getAttributes().get(eq("memberOf"))).thenReturn(groupDN);
}
+ /**
+ * Build and return a list of individually added group DNs such
+ * that calls to .next() will result in a single value each time.
+ *
+ * @param listOfDNs
+ * @return the stubber to use for the .when().next() call
+ */
+ private Stubber buildListOfGroupDNs(List<String> listOfDNs) {
+ Stubber stubber = null;
+ for (String s : listOfDNs) {
+ if (stubber != null) {
+ stubber.doReturn(s);
+ } else {
+ stubber = doReturn(s);
+ }
+ }
+ return stubber;
+ }
+
@Test
public void testGetGroups() throws NamingException {
// given a user whose ldap query returns a user object with three "memberOf"
// properties, return an array of strings representing its groups.
String[] testGroups = new String[] {"abc", "xyz", "sss"};
doTestGetGroups(Arrays.asList(testGroups));
+
+ // test fallback triggered by NamingException
+ doTestGetGroupsWithFallback();
}
private void doTestGetGroups(List<String> expectedGroups)
throws NamingException {
+ List<String> groupDns = new ArrayList<>();
+ groupDns.add("CN=abc,DC=foo,DC=bar,DC=com");
+ groupDns.add("CN=xyz,DC=foo,DC=bar,DC=com");
+ groupDns.add("CN=sss,DC=foo,DC=bar,DC=com");
+
+ setupMocks(groupDns);
String ldapUrl = "ldap://test";
Configuration conf = getBaseConf(ldapUrl);
// enable single-query lookup
conf.set(LdapGroupsMapping.MEMBEROF_ATTR_KEY, "memberOf");
- LdapGroupsMapping groupsMapping = getGroupsMapping();
+ TestLdapGroupsMapping groupsMapping = new TestLdapGroupsMapping();
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");
Assert.assertEquals(expectedGroups, groups);
+ Assert.assertFalse("Second LDAP query should NOT have been called.",
+ groupsMapping.isSecondaryQueryCalled());
// We should have only made one query because single-query lookup is enabled
verify(getContext(), times(1)).search(anyString(), anyString(),
any(Object[].class), any(SearchControls.class));
}
-}
\ No newline at end of file
+
+ private void doTestGetGroupsWithFallback()
+ throws NamingException {
+ List<String> groupDns = new ArrayList<>();
+ groupDns.add("CN=abc,DC=foo,DC=bar,DC=com");
+ groupDns.add("CN=xyz,DC=foo,DC=bar,DC=com");
+ groupDns.add("ipaUniqueID=e4a9a634-bb24-11ec-aec1-06ede52b5fe1," +
+ "CN=sudo,DC=foo,DC=bar,DC=com");
+ setupMocks(groupDns);
+ String ldapUrl = "ldap://test";
+ Configuration conf = getBaseConf(ldapUrl);
+ // enable single-query lookup
+ conf.set(LdapGroupsMapping.MEMBEROF_ATTR_KEY, "memberOf");
+ conf.set(LdapGroupsMapping.LDAP_NUM_ATTEMPTS_KEY, "1");
+
+ TestLdapGroupsMapping groupsMapping = new TestLdapGroupsMapping();
+ 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");
+
+ // expected to be empty due to invalid memberOf
+ Assert.assertEquals(0, groups.size());
+
+ // expect secondary query to be called: getGroups()
+ Assert.assertTrue("Second LDAP query should have been called.",
+ groupsMapping.isSecondaryQueryCalled());
+
+ // We should have fallen back to the second query because first threw
+ // NamingException expected count is 3 since testGetGroups calls
+ // doTestGetGroups and doTestGetGroupsWithFallback in succession and
+ // the count is across both test scenarios.
+ verify(getContext(), times(3)).search(anyString(), anyString(),
+ any(Object[].class), any(SearchControls.class));
+ }
+
+ private static final class TestLdapGroupsMapping extends LdapGroupsMapping {
+ private boolean secondaryQueryCalled = false;
+ public boolean isSecondaryQueryCalled() {
+ return secondaryQueryCalled;
+ }
+ List<String> lookupGroup(SearchResult result, DirContext c,
+ int goUpHierarchy) throws NamingException {
+ secondaryQueryCalled = true;
+ return super.lookupGroup(result, c, goUpHierarchy);
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org