[ 
https://issues.apache.org/jira/browse/KNOX-3328?focusedWorklogId=1022399&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1022399
 ]

ASF GitHub Bot logged work on KNOX-3328:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 27/May/26 13:44
            Start Date: 27/May/26 13:44
    Worklog Time Spent: 10m 
      Work Description: handavid commented on code in PR #1236:
URL: https://github.com/apache/knox/pull/1236#discussion_r3311174268


##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java:
##########
@@ -258,213 +264,251 @@ public Entry getUser(String username, SchemaManager 
schemaManager) throws Except
         LdapConnection connection = null;
         try {
             connection = getConnection();
-            // Search for user using configurable attribute
-            String filter = userSearchFilter.replace("{username}", username);
-            EntryCursor cursor = connection.search(userSearchBase, filter, 
SearchScope.SUBTREE, "*");
+            // 1. Try search in user base
+            final String filter = userSearchFilter.replace("{username}", 
username);
+            final Entry userBaseEntry = fetchEntry(username, schemaManager, 
connection, userSearchBase, filter);
 
-            if (cursor.next()) {
-                Entry sourceEntry = cursor.get();
-                Entry entry = createProxyEntry(sourceEntry, username, 
connection, schemaManager);
-                cursor.close();
-                return entry;
+            if (userBaseEntry != null) {
+                return userBaseEntry;
+            } else {
+                // 2. Try search in group base if not found in user base.
+                // This allows the proxy to enrich group entries with 
recursive 'memberOf' attributes
+                // when a client searches for a group name.
+                String groupFilter = "(" + groupIdentifierAttribute + "=" + 
username + ")";
+                return fetchEntry(username, schemaManager, connection, 
groupSearchBase, groupFilter);
             }
-            cursor.close();
-            return null;
         } finally {
             releaseConnection(connection);
         }
     }
 
+    private Entry fetchEntry(String username, SchemaManager schemaManager, 
LdapConnection connection, String searchBase, String searchFilter) throws 
Exception {
+        try (EntryCursor cursor = connection.search(searchBase, searchFilter, 
SearchScope.SUBTREE, "*", "+")) {
+            final Entry sourceEntry = cursor.next() ? cursor.get() : null;
+            return sourceEntry == null ? null : createProxyEntry(sourceEntry, 
username, connection, schemaManager);
+        }
+    }
+
     @Override
     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));
+                try (EntryCursor cursor = connection.search(userSearchBase, 
filter, SearchScope.SUBTREE, "dn")) {
+                    if (cursor.next()) {
+                        String userDn = cursor.get().getDn().toString();
+                        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);
+                        }
+                    }
                 }
+            }
 
-                cursor.close();
+            List<String> groupNames = new ArrayList<>();
+            for (String dn : allGroupDns) {
+                String name = extractGroupNameFromDn(dn);
+                if (name != null) {
+                    groupNames.add(name);
+                }
             }
-            return Collections.emptyList();
+            return groupNames;
         } finally {
             releaseConnection(connection);
         }
     }
 
-    private List<String> getUserGroupsViaMemberOf(LdapConnection connection, 
String username) throws LdapException, CursorException, IOException {
-        List<String> groups = new ArrayList<>();
+    private List<String> getUserGroupDnsViaMemberOf(LdapConnection connection, 
String username) throws LdapException, CursorException, IOException {
+        final String filter = userSearchFilter.replace("{username}", username);
+        try (EntryCursor cursor = connection.search(userSearchBase, filter, 
SearchScope.SUBTREE, MEMBER_OF, "+")) {
+            List<String> dns = new ArrayList<>();
+            if (cursor.next()) {
+                Entry userEntry = cursor.get();
+                Attribute memberOfAttr = userEntry.get(MEMBER_OF);
+                if (memberOfAttr != null) {
+                    for (org.apache.directory.api.ldap.model.entry.Value value 
: memberOfAttr) {
+                        dns.add(value.getString());
+                    }
+                }
+            }
+            return dns;
+        }
+    }
+
+    private void resolveRecursiveGroupDnsViaMemberOf(LdapConnection 
connection, Set<String> allGroupDns, int depth) throws LdapException, 
CursorException, IOException {
+        if (depth >= groupMaxDepth) {
+            return;
+        }
 
-        // Search for user and retrieve memberOf attribute
-        String filter = userSearchFilter.replace("{username}", username);
-        EntryCursor cursor = connection.search(userSearchBase, filter, 
SearchScope.SUBTREE, "memberOf");
+        final Set<String> nextLevelDns = fetchNextLevelDNs(connection, 
allGroupDns);
 
-        if (cursor.next()) {
-            Entry userEntry = cursor.get();
-            Attribute memberOfAttr = userEntry.get("memberOf");
+        if (!nextLevelDns.isEmpty()) {
+            allGroupDns.addAll(nextLevelDns);
+            resolveRecursiveGroupDnsViaMemberOf(connection, allGroupDns, depth 
+ 1);
+        }
+    }
 
-            if (memberOfAttr != null) {
-                // Extract group names from DNs
-                for (org.apache.directory.api.ldap.model.entry.Value value : 
memberOfAttr) {
-                    String groupDn = value.getString();
-                    String groupName = extractGroupNameFromDn(groupDn);
-                    if (groupName != null) {
-                        groups.add(groupName);
+    private Set<String> fetchNextLevelDNs(LdapConnection connection, 
Set<String> allGroupDns) throws IOException, LdapException, CursorException {
+        final Set<String> nextLevelDns = new HashSet<>();
+        for (String dn : new HashSet<>(allGroupDns)) {
+            try (EntryCursor cursor = connection.search(dn, "(objectClass=*)", 
SearchScope.OBJECT, MEMBER_OF, "+")) {
+                if (cursor.next()) {
+                    Attribute memberOfAttr = cursor.get().get(MEMBER_OF);
+                    if (memberOfAttr != null) {
+                        for (org.apache.directory.api.ldap.model.entry.Value 
value : memberOfAttr) {
+                            String parentDn = value.getString();
+                            if (!allGroupDns.contains(parentDn)) {
+                                nextLevelDns.add(parentDn);
+                            }
+                        }
                     }
                 }
             }
         }
+        return nextLevelDns;
+    }
+
+    private void resolveRecursiveGroupDnsInternal(LdapConnection connection, 
Set<String> allGroupDns, Set<String> visited, int depth) throws LdapException, 
CursorException, IOException {
+        if (depth >= groupMaxDepth) {
+            return;
+        }
 
-        cursor.close();
-        return groups;
+        Set<String> nextLevelDns = new HashSet<>();
+        for (String dn : new HashSet<>(allGroupDns)) {
+            if (visited.contains(dn)) {
+                continue;
+            }
+            visited.add(dn);
+
+            List<Entry> parents = getUserGroupsInternal(connection, dn, null);
+            for (Entry parent : parents) {
+                String parentDn = parent.getDn().toString();
+                if (!allGroupDns.contains(parentDn)) {
+                    nextLevelDns.add(parentDn);
+                }
+            }
+        }
+
+        if (!nextLevelDns.isEmpty()) {
+            allGroupDns.addAll(nextLevelDns);
+            resolveRecursiveGroupDnsInternal(connection, allGroupDns, visited, 
depth + 1);
+        }
     }
 
     private String extractGroupNameFromDn(String groupDn) {
-        // Extract CN from DN like "CN=Domain 
Admins,CN=Users,DC=company,DC=com"
         if (groupDn.toLowerCase(Locale.ROOT).startsWith("cn=")) {
             int commaIdx = groupDn.indexOf(',');
             if (commaIdx > 0) {
                 return groupDn.substring(3, commaIdx);
             }
+            return groupDn.substring(3);
         }
         return null;
     }
 
     private List<Entry> getUserGroupsInternal(LdapConnection connection, 
String userDn, String username) throws LdapException, CursorException, 
IOException {
-        List<Entry> groups = new ArrayList<>();
-
-        // Search for groups where user is a member - build filter based on 
configuration
-        String filter;
+        final String filter;
         if ("member".equals(groupMemberAttribute)) {
-            // AD style - uses full DN
-            filter = "(|" +
-                    "(member=" + userDn + ")" +
-                    "(uniqueMember=" + userDn + ")" +
-                    ")";
+            filter = "(|(member=" + userDn + ")(uniqueMember=" + userDn + "))";
         } else {
-            // POSIX style - uses username
-            filter = "(|" +
-                    "(memberUid=" + username + ")" +
-                    "(member=" + userDn + ")" +
-                    "(uniqueMember=" + userDn + ")" +
-                    ")";
+            filter = "(|(memberUid=" + (username != null ? username : "") + 
")(member=" + userDn + ")(uniqueMember=" + userDn + "))";
         }
 
-        EntryCursor cursor = connection.search(groupSearchBase, filter, 
SearchScope.SUBTREE, "cn");
-
-        while (cursor.next()) {
-            groups.add(cursor.get());
-        }
-
-        cursor.close();
-        return groups;
-    }
-
-    private List<String> getCnsFromEntries(Collection<Entry> entries) throws 
LdapException {
-        List<String> cns = new ArrayList<>();
-        for (Entry entry : entries) {
-            Attribute cnAttr = entry.get("cn");
-            if (cnAttr != null) {
-                cns.add(cnAttr.getString());
+        try (EntryCursor cursor = connection.search(groupSearchBase, filter, 
SearchScope.SUBTREE, "cn") ) {
+            List<Entry> groups = new ArrayList<>();
+            while (cursor.next()) {
+                groups.add(cursor.get());
             }
+            return groups;
         }
-        return cns;
     }
 
     @Override
     public List<Entry> searchUsers(String filter, SchemaManager schemaManager) 
throws Exception {
-        List<Entry> results = new ArrayList<>();
         LdapConnection connection = null;
-
         try {
             connection = getConnection();
-            String ldapFilter = "(" + userIdentifierAttribute + "=" + 
filter.trim() + ")";
-            EntryCursor cursor = connection.search(userSearchBase, ldapFilter, 
SearchScope.SUBTREE, "*");
-
-            while (cursor.next()) {
-                Entry sourceEntry = cursor.get();
-                Attribute idAttr = sourceEntry.get(userIdentifierAttribute);
-                if (idAttr != null) {
-                    String username = idAttr.getString();
-                    Entry entry = createProxyEntry(sourceEntry, username, 
connection, schemaManager);
-                    results.add(entry);
+            final String userSearchFilter = "(" + userIdentifierAttribute + 
"=" + filter.trim() + ")";
+            try (EntryCursor cursor = connection.search(userSearchBase, 
userSearchFilter, SearchScope.SUBTREE, "*", "+")) {
+                final List<Entry> users = new ArrayList<>();
+                while (cursor.next()) {
+                    Entry sourceEntry = cursor.get();
+                    Attribute idAttr = 
sourceEntry.get(userIdentifierAttribute);
+                    if (idAttr != null) {
+                        Entry entry = createProxyEntry(sourceEntry, 
idAttr.getString(), connection, schemaManager);

Review Comment:
   this can lead to many redundant calls to the backend because the groups are 
retrieved on a per-user basis. If we have 10 users that are members of the same 
group, we'll do the same recursive group lookup 10 times.





Issue Time Tracking
-------------------

    Worklog Id:     (was: 1022399)
    Time Spent: 2h 50m  (was: 2h 40m)

> Support recursive group resolution in LDAP Proxy Service
> --------------------------------------------------------
>
>                 Key: KNOX-3328
>                 URL: https://issues.apache.org/jira/browse/KNOX-3328
>             Project: Apache Knox
>          Issue Type: Improvement
>          Components: Server
>    Affects Versions: 3.0.0
>            Reporter: Sandor Molnar
>            Assignee: Sandor Molnar
>            Priority: Major
>             Fix For: 3.0.0
>
>          Time Spent: 2h 50m
>  Remaining Estimate: 0h
>
> The current Knox LDAP proxy implementation only supports direct group 
> memberships for users. When a client queries for a user's {{memberOf}} 
> attribute, Knox only returns the groups where the user is an explicit member, 
> ignoring any transitive or nested group memberships.
> Additionally, group entries themselves are not enriched with the {{memberOf}} 
> attribute, making it difficult for clients to walk the hierarchy manually.
> This task implements:
> # Recursive group resolution for both search-based and {{memberOf}}-based 
> lookups.
> # Strict cycle detection to prevent infinite loops in misconfigured LDAP 
> environments.
> # Configurable depth limits to prevent performance degradation.
> # Enrichment of group-type entries with the {{memberOf}} attribute, allowing 
> clients to see parent group memberships.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to