lmccay commented on code in PR #4798:
URL: https://github.com/apache/hadoop/pull/4798#discussion_r960739138


##########
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:
   Is it not the case that this condition is a subset of the non-posix 
condition check? I think that we should only be looking for 
customGroupFilterArgs if it is non-posix not before the check of isPosix. Am I 
missing something that makes this the appropriate place for this?



##########
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:
   sic. 'below'



-- 
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]

Reply via email to