Skip to content

Commit

Permalink
HADOOP-18074 - Partial/Incomplete groups list can be returned in LDAP. (
Browse files Browse the repository at this point in the history 
#4503)


Partial/Incomplete groups list can be returned in LDAP groups lookup.

Backported in #4550; minor tuning of parameters needed.

Contributed by larry mccay
  • Loading branch information
@lmccay
lmccay authored Jul 14, 2022
1 parent 3a6d865 commit f015c7f
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 17 deletions.
13 changes: 8 additions & 5 deletions ...mon-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
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;
Expand Down Expand Up @@ -458,7 +459,8 @@ 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,
@VisibleForTesting
List<String> lookupGroup(SearchResult result, DirContext c,
int goUpHierarchy)
throws NamingException {
List<String> groups = new ArrayList<>();
Expand Down Expand Up @@ -510,6 +512,7 @@ private List<String> lookupGroup(SearchResult result, DirContext c,
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,
Expand All @@ -518,11 +521,10 @@ List<String> doGetGroups(String user, int goUpHierarchy)
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 {
/**
Expand All @@ -536,19 +538,20 @@ List<String> doGetGroups(String user, int goUpHierarchy)
memberOfAttr + "' attribute." +
"Returned user object: " + result.toString());
}
groups = new ArrayList<>();
NamingEnumeration groupEnumeration = groupDNAttr.getAll();
while (groupEnumeration.hasMore()) {
String groupDN = groupEnumeration.next().toString();
groups.add(getRelativeDistinguishedName(groupDN));
}
} 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);
Expand Down
99 changes: 87 additions & 12 deletions ...op-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMappingWithOneQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -49,48 +51,121 @@
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));
}
}

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);
}
}
}

0 comments on commit f015c7f

Please sign in to comment.