[
https://issues.apache.org/jira/browse/KNOX-3341?focusedWorklogId=1026787&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1026787
]
ASF GitHub Bot logged work on KNOX-3341:
----------------------------------------
Author: ASF GitHub Bot
Created on: 25/Jun/26 09:56
Start Date: 25/Jun/26 09:56
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #1274:
URL: https://github.com/apache/knox/pull/1274#discussion_r3473284050
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java:
##########
@@ -302,10 +326,12 @@ public Entry getUser(String username, SchemaManager
schemaManager) throws Except
connection = getConnection();
// Search for user using configurable attribute
String filter = userSearchFilter.replace("{username}", username);
- try (EntryCursor cursor = connection.search(userSearchBase,
filter, SearchScope.SUBTREE, "*")) {
+ try (EntryCursor cursor = connection.search(remoteUserSearchBase,
filter, SearchScope.SUBTREE, "*")) {
if (cursor.next()) {
Entry sourceEntry = cursor.get();
- return createProxyEntry(sourceEntry, username, connection,
schemaManager, createResolvedParentsCache());
+ addGroupMemberships(sourceEntry, connection,
createEntryCache(), createResolvedParentsCache());
+ return
remoteSchemaConverter.convertRemoteEntryToProxyEntry(sourceEntry,
schemaManager);
Review Comment:
Attribute copying changed from an allowlist to copy-everything.
Assessed by Claude:
```
The old createProxyEntry copied a fixed set (cn, dn, mail, description,
uid, objectClass, identifier, memberOf). The new
RemoteSchemaConverter.convertRemoteEntryToProxyEntry copies all attributes from
the remote entry.
Two concerns:
- Potential exposure of sensitive/operational attributes (e.g.
userPassword) through the proxy that previously weren't surfaced.
Please confirm the remote search ("*") can't return those, or re-introduce
an allowlist/denylist.
- copyAttribute runs convertRemoteDnToProxyDn on every attribute value,
not just DN-valued ones.
A description/mail value that happens to contain the remote base DN
substring would be rewritten. Low probability, but it's blind string rewriting.
```
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/FileBackend.java:
##########
@@ -145,9 +153,13 @@ public List<String> getUserGroups(String username) throws
Exception {
public List<Entry> searchUsers(String filter, SchemaManager schemaManager)
throws Exception {
List<Entry> results = new ArrayList<>();
+ String userFilter = extractUser(filter).toLowerCase(Locale.ROOT);
Review Comment:
Possible NPE here: `extractUser` returns `null` when the filter has no
`uid=/cn=/sAMAccountName=` term.
I'm aware that the file-based backend implementation is for tests only, but
I'd like this to fix.
Analysis by Claude code:
```
extractUser returns null when the filter has no uid=/cn=/sAMAccountName=
term (e.g. (objectclass=inetOrgPerson)). Since the new FileBackend.search()
delegates every filter to searchUsers, and the new UserSearchInterceptor now
forwards all searches to the backend, a general search against a file backend
throws NPE.
It's swallowed by UserSearchInterceptor's catch (Exception e) and logged as
a search failure, so the file backend silently returns nothing for exactly the
"general searches" this PR is about. Should guard the null (return empty or
fall back to wildcard match).
```
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/RemoteSchemaConverter.java:
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.ldap.backend;
+
+import org.apache.directory.api.ldap.model.entry.Attribute;
+import org.apache.directory.api.ldap.model.entry.DefaultEntry;
+import org.apache.directory.api.ldap.model.entry.Entry;
+import org.apache.directory.api.ldap.model.entry.Value;
+import org.apache.directory.api.ldap.model.exception.LdapException;
+import org.apache.directory.api.ldap.model.filter.ExprNode;
+import org.apache.directory.api.ldap.model.filter.FilterParser;
+import org.apache.directory.api.ldap.model.schema.SchemaManager;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.ldap.LdapMessages;
+
+public class RemoteSchemaConverter {
+ private static final LdapMessages LOG =
MessagesFactory.get(LdapMessages.class);
+
+ // Proxy configuration
+ private final String proxyBaseDn; // Base DN for proxy entries (e.g.,
dc=proxy,dc=com)
+ private final String proxyUserSearchBase;
+ private final String proxyGroupSearchBase;
+
+ // Backend configuration
+ private final String remoteBaseDn; // Base DN for remote server searches
(e.g., dc=hadoop,dc=apache,dc=org)
+ private final String remoteUserSearchBase;
+ private final String remoteGroupSearchBase;
+ private final String remoteUserIdentifierAttribute;
+ private final String remoteUserObjectClass;
+ private final String remoteGroupObjectClass;
+
+ public RemoteSchemaConverter(String proxyBaseDn,
+ String proxyUserSearchBase,
+ String proxyGroupSearchBase,
+ String remoteBaseDn,
+ String remoteUserSearchBase,
+ String remoteGroupSearchBase,
+ String remoteUserIdentifierAttribute,
+ String remoteUserObjectClass,
+ String remoteGroupObjectClass) {
+ this.proxyBaseDn = proxyBaseDn;
+ this.proxyUserSearchBase = proxyUserSearchBase;
+ this.proxyGroupSearchBase = proxyGroupSearchBase;
+ this.remoteBaseDn = remoteBaseDn;
+ this.remoteUserSearchBase = remoteUserSearchBase;
+ this.remoteGroupSearchBase = remoteGroupSearchBase;
+ this.remoteUserIdentifierAttribute = remoteUserIdentifierAttribute;
+ this.remoteUserObjectClass = remoteUserObjectClass;
+ this.remoteGroupObjectClass = remoteGroupObjectClass;
+ }
+
+ /**
+ * Creates a proxy entry from a backend source entry with all required
attributes.
+ * This method standardizes the conversion of backend LDAP entries to
proxy entries,
+ * preserving the backend DN and copying all standard user attributes.
+ *
+ * @param sourceEntry The entry from the backend LDAP server
+ * @param schemaManager The schemaManager
+ * @return A new Entry with backend DN and all copied attributes
+ * @throws LdapException if entry creation or attribute copying fails
+ */
+ public Entry convertRemoteEntryToProxyEntry(Entry sourceEntry,
SchemaManager schemaManager) throws LdapException {
+ // Standard proxy approach: return entry with backend DN unchanged
+ // This preserves DN integrity for bind operations and DN references
+ Entry entry = new DefaultEntry(schemaManager);
+ entry.setDn(sourceEntry.getDn());
+
+ // Copy all known AttributeTypes as-is from backend response
+ for (Attribute attribute : sourceEntry.getAttributes()) {
+ copyAttribute(sourceEntry, entry, attribute.getId());
+ }
+
+ // Map identifier attribute to uid for consistency if needed
+ if (!"uid".equals(remoteUserIdentifierAttribute)) {
+ Attribute idAttr = sourceEntry.get(remoteUserIdentifierAttribute);
+ if (idAttr != null) {
+ entry.add("uid", idAttr.getString());
+ }
+ }
+
+ // replace userObjectClass and groupObjectClass object classes
+ Attribute objectClassAttribute = sourceEntry.get("objectclass");
+ if (objectClassAttribute.contains(remoteGroupObjectClass)) {
Review Comment:
Possible NPE if `sourceEntry.get("objectclass")` returns `null`.
##########
knox-site/docs/service_ldap_server.md:
##########
@@ -120,23 +128,27 @@ The proxy backend delegates lookups to a remote LDAP or
Active Directory server.
| `gateway.ldap.interceptor.<name>.url` | N/A | Remote LDAP URL (e.g.,
`ldap://remote-host:389`). |
| `gateway.ldap.interceptor.<name>.host` | N/A | Host of remote LDAP (used if
`url` is not provided). |
| `gateway.ldap.interceptor.<name>.port` | N/A | Port of remote LDAP (used if
`url` is not provided). |
+| `gateway.ldap.interceptor.<name>.baseDn` | N/A | **Required**. The base DN
of the LDAP proxy server. |
| `gateway.ldap.interceptor.<name>.remoteBaseDn` | N/A | **Required**. The
base DN of the remote LDAP server. |
| `gateway.ldap.interceptor.<name>.systemUsername` | N/A | Bind DN for the
remote server (alias: `bindDn`). |
| `gateway.ldap.interceptor.<name>.systemPassword` | N/A | Password for the
bind DN (alias: `bindPassword`). |
| `gateway.ldap.interceptor.<name>.userSearchBase` |
`ou=people,{remoteBaseDn}` | Base DN for user searches on the remote server. |
| `gateway.ldap.interceptor.<name>.groupSearchBase` |
`ou=groups,{remoteBaseDn}` | Base DN for group searches on the remote server. |
| `gateway.ldap.interceptor.<name>.userIdentifierAttribute` | `uid` |
Attribute used for user lookup (e.g., `sAMAccountName` for AD). |
+| `gateway.ldap.interceptor.<name>.userObjectClass` | `inetOrgPerson` |
Objectclass for identifying users in remote server (e.g., `person` for AD). |
+| `gateway.ldap.interceptor.<name>.groupObjectClass` | `groupOfNames` |
Objectclass for identifying groups in remote server (e.g., `group` for AD). |
| `gateway.ldap.interceptor.<name>.groupMemberAttribute` | `memberUid` |
Attribute used for group membership (e.g., `member` for AD). |
| `gateway.ldap.interceptor.<name>.useMemberOf` | `false` | If `true`, use the
`memberOf` attribute for efficient group lookups. |
-| `gateway.interceptor.<name>.proxy.poolMaxActive` | `8` | Maximum number of
active connections in the pool. |
+| `gateway.ldap.interceptor.<name>.proxy.poolMaxActive` | `8` | Maximum number
of active connections in the pool. |
+| `gateway.ldap.interceptor.<name>.proxy.poolMaxActive` | `8` | Maximum number
of active connections in the pool. |
Review Comment:
Duplicated row -> has to be eliminated.
##########
gateway-server/src/test/resources/ldap-recursive-test.ldif:
##########
@@ -93,3 +105,77 @@ objectclass:top
objectclass:groupOfNames
cn: cycleGroupB
member: cn=cycleGroupA,ou=recursiveGroups,dc=hadoop,dc=apache,dc=org
+
+# entry for sample user memberOfUser using memberOf
+dn: uid=memberOfUser,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass:person
+objectclass:organizationalPerson
+objectclass:inetOrgPerson
+cn: MemberOf
+sn: User
+uid: memberOfUser
+userPassword:memberOfUser-password
+memberOf:
cn=memberOflevel1,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+
+# entry for sample user memberOfUser using memberOf
+dn: uid=memberOfUser2,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass:person
+objectclass:organizationalPerson
+objectclass:inetOrgPerson
+cn: MemberOf2
+sn: User
+uid: memberOfUser2
+userPassword:memberOfUser2-password
+memberOf:
cn=memberOflevel1,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+
+# memberOf Level 1 Group
+dn: cn=memberOflevel1,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass: groupofnames
+cn: memberOflevel1
+memberOf:
cn=memberOflevel2,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+member: uid=memberOfUser,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+member: uid=memberOfUser2,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+
+# memberOf Level 2 Group (Member is Level 1)
+dn: cn=memberOflevel2,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass: groupofnames
+cn: memberOflevel2level2
Review Comment:
Wrong `cn`: it should be `cn: memberOflevel2` (otherwise it'll be a
mismatched RDN).
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java:
##########
@@ -540,27 +683,16 @@ private String extractGroupNameFromDn(String groupDn) {
return null;
}
- private List<Entry> getUserGroupsInternal(LdapConnection connection,
String userDn, String username) throws LdapException, CursorException,
IOException {
+ // tODO reuse this method in recursive calls
Review Comment:
nit: typo
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/RemoteSchemaConverter.java:
##########
@@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.ldap.backend;
+
+import org.apache.directory.api.ldap.model.entry.Attribute;
+import org.apache.directory.api.ldap.model.entry.DefaultEntry;
+import org.apache.directory.api.ldap.model.entry.Entry;
+import org.apache.directory.api.ldap.model.entry.Value;
+import org.apache.directory.api.ldap.model.exception.LdapException;
+import org.apache.directory.api.ldap.model.filter.ExprNode;
+import org.apache.directory.api.ldap.model.filter.FilterParser;
+import org.apache.directory.api.ldap.model.schema.SchemaManager;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.ldap.LdapMessages;
+
+public class RemoteSchemaConverter {
+ private static final LdapMessages LOG =
MessagesFactory.get(LdapMessages.class);
+
+ // Proxy configuration
+ private final String proxyBaseDn; // Base DN for proxy entries (e.g.,
dc=proxy,dc=com)
+ private final String proxyUserSearchBase;
+ private final String proxyGroupSearchBase;
+
+ // Backend configuration
+ private final String remoteBaseDn; // Base DN for remote server searches
(e.g., dc=hadoop,dc=apache,dc=org)
+ private final String remoteUserSearchBase;
+ private final String remoteGroupSearchBase;
+ private final String remoteUserIdentifierAttribute;
+ private final String remoteUserObjectClass;
+ private final String remoteGroupObjectClass;
+
+ public RemoteSchemaConverter(String proxyBaseDn,
+ String proxyUserSearchBase,
+ String proxyGroupSearchBase,
+ String remoteBaseDn,
+ String remoteUserSearchBase,
+ String remoteGroupSearchBase,
+ String remoteUserIdentifierAttribute,
+ String remoteUserObjectClass,
+ String remoteGroupObjectClass) {
+ this.proxyBaseDn = proxyBaseDn;
+ this.proxyUserSearchBase = proxyUserSearchBase;
+ this.proxyGroupSearchBase = proxyGroupSearchBase;
+ this.remoteBaseDn = remoteBaseDn;
+ this.remoteUserSearchBase = remoteUserSearchBase;
+ this.remoteGroupSearchBase = remoteGroupSearchBase;
+ this.remoteUserIdentifierAttribute = remoteUserIdentifierAttribute;
+ this.remoteUserObjectClass = remoteUserObjectClass;
+ this.remoteGroupObjectClass = remoteGroupObjectClass;
+ }
+
+ /**
+ * Creates a proxy entry from a backend source entry with all required
attributes.
+ * This method standardizes the conversion of backend LDAP entries to
proxy entries,
+ * preserving the backend DN and copying all standard user attributes.
+ *
+ * @param sourceEntry The entry from the backend LDAP server
+ * @param schemaManager The schemaManager
+ * @return A new Entry with backend DN and all copied attributes
+ * @throws LdapException if entry creation or attribute copying fails
+ */
+ public Entry convertRemoteEntryToProxyEntry(Entry sourceEntry,
SchemaManager schemaManager) throws LdapException {
+ // Standard proxy approach: return entry with backend DN unchanged
+ // This preserves DN integrity for bind operations and DN references
+ Entry entry = new DefaultEntry(schemaManager);
+ entry.setDn(sourceEntry.getDn());
+
+ // Copy all known AttributeTypes as-is from backend response
+ for (Attribute attribute : sourceEntry.getAttributes()) {
+ copyAttribute(sourceEntry, entry, attribute.getId());
+ }
+
+ // Map identifier attribute to uid for consistency if needed
+ if (!"uid".equals(remoteUserIdentifierAttribute)) {
+ Attribute idAttr = sourceEntry.get(remoteUserIdentifierAttribute);
+ if (idAttr != null) {
+ entry.add("uid", idAttr.getString());
+ }
+ }
+
+ // replace userObjectClass and groupObjectClass object classes
+ Attribute objectClassAttribute = sourceEntry.get("objectclass");
+ if (objectClassAttribute.contains(remoteGroupObjectClass)) {
+ entry.remove("objectclass", remoteGroupObjectClass);
+ entry.add("objectclass", "groupofnames");
+ }
+ if (objectClassAttribute.contains(remoteUserObjectClass)) {
+ entry.remove("objectclass", remoteUserObjectClass);
+ entry.add("objectclass", "inetOrgPerson");
+ }
+
+ return entry;
+ }
+
+ /**
+ * Converts an LDAP search filter from proxy attributes to remote
attributes.
+ * @param filter the filter
+ * @param schemaManager the schema manager
+ * @return the converted filter
+ * @throws Exception if the filter cannot be parsed
+ */
+ public String convertProxyFilterToRemoteFilter(String filter,
SchemaManager schemaManager) throws Exception {
+ FilterMappingVisitor filterMappingVisitor = new
FilterMappingVisitor(remoteUserIdentifierAttribute, remoteUserObjectClass,
remoteGroupObjectClass, schemaManager);
+
+ // Filter likely has already been annotated by other interceptors.
+ // Clean the filter by removing any modifications or annotations
+ String rawFilter = filter.replaceAll(":\\[.*?\\]",
"").replaceAll("=\\s+", "=");
+ ExprNode cleanNode = FilterParser.parse(schemaManager, rawFilter);
+
+ return cleanNode.accept(filterMappingVisitor).toString();
+ }
+
+ /**
+ * Copy attributes from a remote entry to a proxy entry. DN values are
converted from
+ * the remote base to the proxy base
+ * @param source The remote entry
+ * @param target The proxy entry
+ * @param attributeName The name of the attribute to copy
+ */
+ public void copyAttribute(Entry source, Entry target, String
attributeName) {
+ final Attribute attribute = source.get(attributeName);
+ if (attribute != null) {
+ // Copy all values of the attribute (important for multi-valued
attributes like objectClass)
+ for (Value value : attribute) {
+ String valueString =
convertRemoteDnToProxyDn(value.toString());
+ if (!target.contains(attributeName, valueString)) {
+ try {
+ target.add(attributeName, valueString);
+ } catch (LdapException e) {
+ LOG.ldapAttributeCopyError(e);
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * Converts a remote dn string to a proxy dn string. This also works for
search base strings.
+ * @param string the remote dn or search base
+ * @return the proxy search base
+ */
+ public String convertRemoteDnToProxyDn(String string) {
Review Comment:
DN/filter conversion via String.replaceAll is regex-fragile.
Assessed by Claude:
```
convertRemoteDnToProxyDn/convertProxyDnToRemoteDn use the base DNs as regex
patterns and the targets as replacement strings (where $/\ are special).
Standard DNs are safe, but this will misbehave if a DN contains regex
meta-characters.
Pattern.quote + Matcher.quoteReplacement (or proper DN parsing) would be
more robust. Same fragility in convertProxyFilterToRemoteFilter's
filter.replaceAll(":\\[.*?\\]", "")... cleanup, which is a bit of a hack
(acknowledged in the comment).
```
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java:
##########
@@ -315,72 +341,182 @@ public Entry getUser(String username, SchemaManager
schemaManager) throws Except
}
@Override
- public List<String> getUserGroups(String username) throws Exception {
+ public List<String> getUserGroups(String username, SchemaManager
schemaManager) throws Exception {
Review Comment:
Claude's assessment:
```
`getUserGroups()` with `useMemberOf=true` + `recursiveGroupResolution=true`
likely returns an incomplete CN list.
In the recursive memberOf path, populateParentCacheViaMemberOf fetches group
entries via connection.lookup(groupDn, "memberOf") — requesting only memberOf,
so the cached entries have no cn. Those entries flow back through
getUserGroupsEntries
→ getCnsFromEntries, which does entry.get("cn") and drops anything without a
cn. So the recursively-discovered (and even the directly-resolved) groups get
dropped from the returned name list.
This path has no test — all the new recursive-memberOf tests go through
search() and assert on the memberOf attribute (DN strings via
addGroupMemberships), not on getUserGroups() CNs. Fix: request "cn", "memberOf"
in the lookup, or derive the CN from the DN.
```
##########
gateway-server/src/test/resources/ldap-recursive-test.ldif:
##########
@@ -93,3 +105,77 @@ objectclass:top
objectclass:groupOfNames
cn: cycleGroupB
member: cn=cycleGroupA,ou=recursiveGroups,dc=hadoop,dc=apache,dc=org
+
+# entry for sample user memberOfUser using memberOf
+dn: uid=memberOfUser,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass:person
+objectclass:organizationalPerson
+objectclass:inetOrgPerson
+cn: MemberOf
+sn: User
+uid: memberOfUser
+userPassword:memberOfUser-password
+memberOf:
cn=memberOflevel1,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+
+# entry for sample user memberOfUser using memberOf
+dn: uid=memberOfUser2,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass:person
+objectclass:organizationalPerson
+objectclass:inetOrgPerson
+cn: MemberOf2
+sn: User
+uid: memberOfUser2
+userPassword:memberOfUser2-password
+memberOf:
cn=memberOflevel1,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+
+# memberOf Level 1 Group
+dn: cn=memberOflevel1,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass: groupofnames
+cn: memberOflevel1
+memberOf:
cn=memberOflevel2,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+member: uid=memberOfUser,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+member: uid=memberOfUser2,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+
+# memberOf Level 2 Group (Member is Level 1)
+dn: cn=memberOflevel2,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass: groupofnames
+cn: memberOflevel2level2
+memberOf:
cn=memberOflevel3,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+memberOf:
cn=memberOfCycleA,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+member: cn=memberOflevel1,ou=recursiveGroups,dc=hadoop,dc=apache,dc=org
Review Comment:
points to `ou=recursiveGroups` instead of `ou=recursiveMemberOfGroups`.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/interceptor/DisabledUserInterceptor.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.ldap.interceptor;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.directory.api.ldap.model.cursor.CursorException;
+import org.apache.directory.api.ldap.model.cursor.ListCursor;
+import org.apache.directory.api.ldap.model.entry.Attribute;
+import org.apache.directory.api.ldap.model.entry.Entry;
+import org.apache.directory.api.ldap.model.exception.LdapException;
+import org.apache.directory.server.core.api.filtering.EntryFilteringCursor;
+import org.apache.directory.server.core.api.filtering.EntryFilteringCursorImpl;
+import org.apache.directory.server.core.api.interceptor.BaseInterceptor;
+import
org.apache.directory.server.core.api.interceptor.context.SearchOperationContext;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+public class DisabledUserInterceptor extends BaseInterceptor {
+
+ private static final String UAC_ATTRIBUTENAME = "useraccountcontrol";
+ // Active Directory UserAccountControl Bitmasks
+ private static final int UAC_ACCOUNTDISABLE = 0x0002;
+
+ private static final String NSACCOUNTLOCK_ATTRIBUTE = "nsaccountlock";
+ private static final String NSACCOUNTLOCK_DISABLED_VALUE = "true";
+
+ private boolean removeDisabledUsers;
+
+ public DisabledUserInterceptor(String name, Map<String, String> config) {
+ this(name,
Boolean.parseBoolean(config.getOrDefault("removeDisabledUsers", "false")));
+ }
+
+ protected DisabledUserInterceptor(String name, boolean
removeDisabledUsers) {
+ super(name);
+ this.removeDisabledUsers = removeDisabledUsers;
+ }
+
+ @Override
+ public EntryFilteringCursor search(SearchOperationContext ctx) throws
LdapException {
+ // First execute the interceptor chain to get the results
+ List<Entry> filteredEntries = List.of();
+ try (EntryFilteringCursor originalResults = next(ctx)) {
+ List<Entry> originalEntries = new ArrayList<>();
+ try {
+ while (originalResults.next()) {
+ originalEntries.add(originalResults.get());
+ }
+ } catch (CursorException e) {
+ // rethrow exception on incomplete iteration
+ throw new LdapException(e);
+ }
+ filteredEntries = filterDisabledUsers(originalEntries);
+ } catch (IOException e) {
+ // IOException would only occur after finishing iterating over
results
+ // we can ignore this exception and return the filtered entries
+ } catch (Exception e) {
+ throw e;
+ }
Review Comment:
This catch does nothing, we can remove it.
##########
gateway-server/src/test/resources/ldap-recursive-test.ldif:
##########
@@ -93,3 +105,77 @@ objectclass:top
objectclass:groupOfNames
cn: cycleGroupB
member: cn=cycleGroupA,ou=recursiveGroups,dc=hadoop,dc=apache,dc=org
+
+# entry for sample user memberOfUser using memberOf
+dn: uid=memberOfUser,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass:person
+objectclass:organizationalPerson
+objectclass:inetOrgPerson
+cn: MemberOf
+sn: User
+uid: memberOfUser
+userPassword:memberOfUser-password
+memberOf:
cn=memberOflevel1,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+
+# entry for sample user memberOfUser using memberOf
+dn: uid=memberOfUser2,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass:person
+objectclass:organizationalPerson
+objectclass:inetOrgPerson
+cn: MemberOf2
+sn: User
+uid: memberOfUser2
+userPassword:memberOfUser2-password
+memberOf:
cn=memberOflevel1,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+
+# memberOf Level 1 Group
+dn: cn=memberOflevel1,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass: groupofnames
+cn: memberOflevel1
+memberOf:
cn=memberOflevel2,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+member: uid=memberOfUser,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+member: uid=memberOfUser2,ou=recursiveMemberOfPeople,dc=hadoop,dc=apache,dc=org
+
+# memberOf Level 2 Group (Member is Level 1)
+dn: cn=memberOflevel2,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+objectclass:top
+objectclass: groupofnames
+cn: memberOflevel2level2
+memberOf:
cn=memberOflevel3,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+memberOf:
cn=memberOfCycleA,ou=recursiveMemberOfGroups,dc=hadoop,dc=apache,dc=org
+member: cn=memberOflevel1,ou=recursiveGroups,dc=hadoop,dc=apache,dc=org
+
Review Comment:
These 2 issues above don't fail the tests (the `memberOf` recursion reads
the `memberOf` attribute, not `cn/member`), which is precisely why issue with
`getUserGroups()` above slipped through untested. I'd fix the data and add a
`getUserGroups()` assertion for the `useMemberOf+recursive` combo.
Issue Time Tracking
-------------------
Worklog Id: (was: 1026787)
Time Spent: 40m (was: 0.5h)
> Enhance LDAP Proxy to support more general search parameters
> ------------------------------------------------------------
>
> Key: KNOX-3341
> URL: https://issues.apache.org/jira/browse/KNOX-3341
> Project: Apache Knox
> Issue Type: Improvement
> Components: Server
> Reporter: David Han
> Assignee: David Han
> Priority: Major
> Fix For: 3.0.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> LDAP search with the LDAP Proxy is focused around user retrieval based on
> uid. This should be enhanced to better support the search filter and returned
> attribute parameters commonly used in LDAP search.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
