[
https://issues.apache.org/jira/browse/HADOOP-18388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17599080#comment-17599080
]
ASF GitHub Bot commented on HADOOP-18388:
-----------------------------------------
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
> Allow dynamic groupSearchFilter in LdapGroupsMapping
> ----------------------------------------------------
>
> Key: HADOOP-18388
> URL: https://issues.apache.org/jira/browse/HADOOP-18388
> Project: Hadoop Common
> Issue Type: Improvement
> Components: security
> Reporter: Ayush Saxena
> Assignee: Ayush Saxena
> Priority: Major
> Labels: pull-request-available
> Attachments: dynamic-filter-idea.patch
>
>
> As of now the lookupGroup() method doesn't allow to have placeholders in
> groupSearchFilter, so that can not be dynamically adjusted.
> If we have placeholders for groupSearchFilter like:
> (&(|(XYZ=\{0})(ABC=\{1}))(objectClass=posixGroup))
> This fails here:
>
> {code:java}
> groupResults =
> c.search(groupbaseDN,
> "(&" + groupSearchFilter + "(" + groupMemberAttr + "={0}))",
> new Object[]{userDn},
> SEARCH_CONTROLS); {code}
> With
>
>
> {noformat}
> javax.naming.directory.InvalidSearchFilterException: number exceeds argument
> list: 1; remaining name {noformat}
>
> >>Dropped off or changed the details above which I thought won't be safe to
> >>disclose.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
