ayushtkn commented on code in PR #4798:
URL: https://github.com/apache/hadoop/pull/4798#discussion_r960909797
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java:
##########
@@ -437,8 +443,14 @@ Set<String> lookupGroup(SearchResult result, DirContext c,
Set<String> groupDNs = new HashSet<>();
NamingEnumeration<SearchResult> groupResults;
- // perform the second LDAP query
- if (isPosix) {
+
+ String[] resolved = resolveCustomGroupFilterArgs(result);
+ // If custom group filter argument is supplied, use that!!!
+ if (resolved != null) {
Review Comment:
One of the filter that we checked internally had the object class as
posixGroup and it can still satisfy the isPosix condition, but if some user has
explicitly configured that replace my {0} with say X and {1} with say Y, so we
should do that irrespective of whether the filter anywhere contains posixGroup
or not.
Else if he has this `posixGroup` and stuff in the group filter, we can't use
custom filters at all and my use case will break.
Can we justify like if someone configured it use custom filter then we use
that because he explicitly asked us to do so. If he doesn't then we do our
normal logic and figure out if it is posix or non posix.
By default it will always be turned off, because there is no default value
set, so won't(or I should say shouldn't) bother any existing use case.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestLdapGroupsMapping.java:
##########
@@ -120,6 +124,50 @@ public void testGetGroupsWithDefaultBaseDN() throws
Exception {
doTestGetGroupsWithBaseDN(conf, baseDN.trim(), baseDN.trim());
}
+ @Test
+ public void testGetGroupsWithDynamicGroupFilter() throws Exception {
+ // Set basic mock stuff.
+ Configuration conf = getBaseConf(TEST_LDAP_URL);
+ String baseDN = "dc=xxx,dc=com";
+ conf.set(LdapGroupsMapping.BASE_DN_KEY, baseDN);
+ Attributes attributes = getAttributes();
+
+ // Set the groupFilter attributed to take the csv.
+ Attribute groupFilterAttr = mock(Attribute.class);
+ when(groupFilterAttr.get()).thenReturn("userDN,userName");
+ when(attributes.get(eq("groupfilter"))).thenReturn(groupFilterAttr);
+
+ // Set the value for userName attribute that is to be used as part of the
+ // group filter at argument 1.
+ final String userName = "some_user";
+ Attribute userNameAttr = mock(Attribute.class);
+ when(userNameAttr.get()).thenReturn(userName);
+ when(attributes.get(eq("userName"))).thenReturn(userNameAttr);
+
+ // Set the dynamic group search filter.
+ final String groupSearchFilter =
+ "(|(memberUid={0})(uname={1}))" + "(objectClass=group)";
+ conf.set(LdapGroupsMapping.GROUP_SEARCH_FILTER_KEY, groupSearchFilter);
+
+ final LdapGroupsMapping groupsMapping = getGroupsMapping();
+ groupsMapping.setConf(conf);
+
+ // The group search filter should be resolved and should be passed as the
+ // bellow.
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
