handavid commented on code in PR #1236:
URL: https://github.com/apache/knox/pull/1236#discussion_r3304652319
##########
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);
Review Comment:
I find it strange to search for a group in a method called `getUser`. This
is a limitation of the `GroupSearchInterceptor` that will only call this method
for users. I have a change forthcoming to the interceptors and another planned
for handling general searches better. I understand that there is a problem that
you can't search for groups directly using the current implementation and hope
to solve that in a better way.
##########
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);
Review Comment:
I think we only need to recursively resolve the `nextLevelDns` and not the
full `allGroupDns`. Otherwise we are redundantly fetching the same groups
repeatedly. You'll probably need to pass in both the `nextLevelDns` and
`allGroupDns` if you want to filter out seen Dns in `fetchNextLevelDNs`
##########
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) {
Review Comment:
optimization: you can convert this from recursion to iteration to shorten
the call stack.
##########
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) {
Review Comment:
optimization: you can convert this from recursion to iteration to shorten
the call stack.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java:
##########
@@ -83,19 +83,20 @@ public String getName() {
@Override
public void initialize(Map<String, String> config) throws Exception {
// Proxy base DN is for entries created in the proxy LDAP server
Review Comment:
nit: comment redundant. you can remove the old comment.
##########
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 {
Review Comment:
this isn't only applicable to users after your change. you should rename the
`userDn` param to something more general
##########
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, "+")) {
Review Comment:
can we combine the groups into a single search instead of executing one
search per group? We can try constructing a search filter like
`connection.search(groupSearchBase, "(|(cn=Group1)(cn=Group2)(cn=Group3))",
SearchScope.SUBTREE, MEMBER_OF, "+")`.
--
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]
