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