Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HADOOP-18074 - Partial/Incomplete groups list can be returned in LDAP. (#4503) #4570

Merged
merged 1 commit into from
Jul 17, 2022
Merged

HADOOP-18074 - Partial/Incomplete groups list can be returned in LDAP. (#4503) #4570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions ...mon-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
View file
Open in desktop
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
Open in desktop
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);
}
}
}