This is an automated email from the ASF dual-hosted git repository.
lmccay pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/trunk by this push:
new e11ba5930e2 HADOOP-18074 - Partial/Incomplete groups list can be
returned in LDAP… (#4503)
e11ba5930e2 is described below
commit e11ba5930e29a370f07685eac888bdb95aefc54f
Author: lmccay <[email protected]>
AuthorDate: Mon Jul 11 01:03:44 2022 -0400
HADOOP-18074 - Partial/Incomplete groups list can be returned in LDAP…
(#4503)
* HADOOP-18074 - Partial/Incomplete groups list can be returned in LDAP
groups lookup
---
.../apache/hadoop/security/LdapGroupsMapping.java | 6 +-
.../TestLdapGroupsMappingWithOneQuery.java | 100 ++++++++++++++++++---
2 files changed, 93 insertions(+), 13 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 4eb3d865ec7..2c388107b62 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
@@ -59,6 +59,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;
@@ -428,7 +429,8 @@ public class LdapGroupsMapping
* @return a list of strings representing group names of the user.
* @throws NamingException if unable to find group names
*/
- private Set<String> lookupGroup(SearchResult result, DirContext c,
+ @VisibleForTesting
+ Set<String> lookupGroup(SearchResult result, DirContext c,
int goUpHierarchy)
throws NamingException {
Set<String> groups = new LinkedHashSet<>();
@@ -510,6 +512,8 @@ 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);
}
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..8686d5c6e3b 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,22 @@
package org.apache.hadoop.security;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
+import java.util.Set;
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 +52,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;
+ }
+ Set<String> lookupGroup(SearchResult result, DirContext c,
+ int goUpHierarchy) throws NamingException {
+ secondaryQueryCalled = true;
+ return super.lookupGroup(result, c, goUpHierarchy);
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
