smolnar82 commented on code in PR #1236:
URL: https://github.com/apache/knox/pull/1236#discussion_r3279902293


##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java:
##########
@@ -280,191 +297,218 @@ public List<String> getUserGroups(String username) 
throws Exception {
         LdapConnection connection = null;
         try {
             connection = getConnection();
+            Set<String> allGroupDns = new HashSet<>();
             if (useMemberOf) {
-                // Use memberOf attribute for efficient AD lookups
-                return getUserGroupsViaMemberOf(connection, username);
+                allGroupDns.addAll(getUserGroupDnsViaMemberOf(connection, 
username));
+                if (recursiveGroupResolution && !allGroupDns.isEmpty()) {
+                    resolveRecursiveGroupDnsViaMemberOf(connection, 
allGroupDns, 0);
+                }
             } else {
-                // Use traditional group search approach
                 String filter = userSearchFilter.replace("{username}", 
username);
                 EntryCursor cursor = connection.search(userSearchBase, filter, 
SearchScope.SUBTREE, "dn");
-
                 if (cursor.next()) {
                     String userDn = cursor.get().getDn().toString();
                     cursor.close();
-                    return getCnsFromEntries(getUserGroupsInternal(connection, 
userDn, username));
+                    Set<String> visited = new HashSet<>();
+                    visited.add(userDn);
+                    List<Entry> initialGroups = 
getUserGroupsInternal(connection, userDn, username);
+                    for (Entry group : initialGroups) {
+                        allGroupDns.add(group.getDn().toString());
+                    }
+                    if (recursiveGroupResolution && !allGroupDns.isEmpty()) {
+                        resolveRecursiveGroupDnsInternal(connection, 
allGroupDns, visited, 0);
+                    }
+                } else {
+                    cursor.close();

Review Comment:
   The cursor was actually closed a couple line above when the `if` confition 
is true.
   However, reflecting to your comment below, I'll rework the code to use 
try-with-resources around cursors as they are `Closeable`.



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

Reply via email to