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