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

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

                Author: ASF GitHub Bot
            Created on: 01/Jun/26 20:01
            Start Date: 01/Jun/26 20:01
    Worklog Time Spent: 10m 
      Work Description: handavid commented on code in PR #1236:
URL: https://github.com/apache/knox/pull/1236#discussion_r3336807051


##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java:
##########
@@ -410,19 +508,17 @@ public List<Entry> searchUsers(String filter, 
SchemaManager schemaManager) throw
         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);
+            try (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);

Review Comment:
   we still have the problem of redundant searches if multiple users are 
members of the same groups.



##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java:
##########
@@ -301,55 +307,148 @@ public List<String> getUserGroups(String username) 
throws Exception {
         LdapConnection connection = null;
         try {
             connection = getConnection();
-            if (useMemberOf) {
-                // Use memberOf attribute for efficient AD lookups
-                return getUserGroupsViaMemberOf(connection, username);
-            } else {
-                // Use traditional group search approach
-                String filter = userSearchFilter.replace("{username}", 
username);
-                EntryCursor cursor = connection.search(userSearchBase, filter, 
SearchScope.SUBTREE, "dn");
+            List<Entry> groups = getUserGroupsEntries(connection, username);
+            return getCnsFromEntries(groups);
+        } finally {
+            releaseConnection(connection);
+        }
+    }
 
+    private List<Entry> getUserGroupsEntries(LdapConnection connection, String 
username) throws Exception {
+        List<Entry> groups;
+        if (useMemberOf) {
+            // Use memberOf attribute for efficient AD lookups
+            groups = getUserGroupsViaMemberOfInternal(connection, username);
+        } else {
+            // Use traditional group search approach
+            String filter = userSearchFilter.replace("{username}", username);
+            try (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));
+                    groups = getUserGroupsInternal(connection, userDn, 
username);
+                } else {
+                    groups = Collections.emptyList();
                 }
-
-                cursor.close();
             }
-            return Collections.emptyList();
-        } finally {
-            releaseConnection(connection);
         }
+
+        if (recursiveGroupResolution && !groups.isEmpty()) {
+            groups = resolveGroupsRecursive(connection, groups, username);
+        }
+        return groups;
     }
 
-    private List<String> getUserGroupsViaMemberOf(LdapConnection connection, 
String username) throws LdapException, CursorException, IOException {
-        List<String> groups = new ArrayList<>();
+    private List<Entry> getUserGroupsViaMemberOfInternal(LdapConnection 
connection, String username) throws LdapException, CursorException, IOException 
{

Review Comment:
   we only need the `Entry` if we are doing recursive group resolution. I think 
we should still return `List<String>` here.  `getUserGroupsEntries` can do the 
`connection.lookup` calls to transform the `String`s into `Entry`s if needed.





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

    Worklog Id:     (was: 1023184)
    Time Spent: 4h 20m  (was: 4h 10m)

> 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: 4h 20m
>  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