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
This commit is contained in:
parent
9b1d3579b4
commit
e11ba5930e
@ -59,6 +59,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;
|
||||
@ -428,7 +429,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 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 @@ Set<String> doGetGroups(String user, int goUpHierarchy)
|
||||
}
|
||||
} 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);
|
||||
}
|
||||
|
@ -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 @@
|
||||
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;
|
||||
}
|
||||
Set<String> lookupGroup(SearchResult result, DirContext c,
|
||||
int goUpHierarchy) throws NamingException {
|
||||
secondaryQueryCalled = true;
|
||||
return super.lookupGroup(result, c, goUpHierarchy);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user