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


##########
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 {

Review Comment:
   can we rename this to `fetchNextLevelDNsViaMemberOf` to preserve the usage 
of this method in the memberOf codepath only?



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