[
https://issues.apache.org/jira/browse/KNOX-3490?focusedWorklogId=1044298&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1044298
]
ASF GitHub Bot logged work on KNOX-3490:
----------------------------------------
Author: ASF GitHub Bot
Created on: 28/Sep/26 06:47
Start Date: 28/Sep/26 06:47
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #1431:
URL: https://github.com/apache/knox/pull/1431#discussion_r4119304940
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/interceptor/LDAPRolesLookupInterceptor.java:
##########
@@ -81,7 +81,15 @@ public EntryFilteringCursor search(SearchOperationContext
ctx) throws LdapExcept
if (!entries.isEmpty()) {
for (Entry entry : entries) {
try {
- final String username =
LdapUtils.extractUsernameFromEntry(entry, "uid", "cn");
+ // Key the role lookup on the stable uid, never a display
name. The entry's DN
+ // (uid=...,ou=people,...) is always present, whereas the
uid *attribute* is
+ // stripped when the LDAP client (e.g. Hadoop
LdapGroupsMapping) does not request
+ // it, leaving only cn and resolving the wrong roles.
Prefer the uid from the DN,
+ // falling back to entry attributes for non-uid-based DNs.
+ String username =
LdapUtils.extractUsernameFromDn(entry.getDn());
Review Comment:
Good catch, @handavid! You're right that the DN-first approach breaks on AD.
The AD DN is `CN=<display name>,…,` the real id is `sAMAccountName` (which
LdapProxyBackend maps onto `uid`), so keying off the RDN would resolve the
display name and look up the wrong roles.
I've dropped the DN-first change and gone with your suggestion: force `uid`
into the search's returning attributes, key the lookup on it, then strip it
back out of the entries when the client didn't ask for it. Restoring the
original returning attributes afterwards too, so downstream sees exactly what
it requested. A `*` (all-user-attributes) request or one that already names
`uid` is left untouched.
I also generalized the plumbing rather than hard-coding `uid`: there's now a
small interceptor-owned `REQUIRED_ATTRIBUTES` list, and the augment/strip logic
adds whichever of those the client omitted and strips exactly those back out
(per-attribute, using `ctx.contains(...)` so the `*`/`+`/`1.1` flags are
honored). It's deliberately a code constant, not topology config: an attribute
only earns a place there if code in the interceptor actually reads it, so the
augment set and its consumer can't drift apart. That makes the `objectClass`
case you mentioned (telling user vs group entries apart, and the overlap with
#1428) a one-line addition here plus the consuming logic, whenever we want to
tackle it.
Issue Time Tracking
-------------------
Worklog Id: (was: 1044298)
Time Spent: 50m (was: 40m)
> LDAPRolesLookupInterceptor resolves the wrong username when the LDAP client
> does not request the uid attribute, causing incorrect role lookups
> ----------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: KNOX-3490
> URL: https://issues.apache.org/jira/browse/KNOX-3490
> Project: Apache Knox
> Issue Type: Bug
> Components: Server
> Affects Versions: 3.0.0
> Reporter: Sandor Molnar
> Assignee: Sandor Molnar
> Priority: Major
> Fix For: 3.1.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> *Background*
> Knox's embedded LDAP server supports a roles-lookup feature:
> {{LDAPRolesLookupInterceptor}} intercepts search results, derives the user's
> identity from the returned entry, calls the configured roles-lookup service
> with that identity, and rewrites the entry's {{memberOf}} values to reflect
> the resolved roles.
> In a federated topology this embedded LDAP on a _central_ cluster is queried
> by a _local_ cluster. The _local_ cluster resolves groups/roles via Hadoop's
> {{LdapGroupsMapping}} (through {{{}HadoopGroupProviderFilter{}}}), connecting
> to the central cluster's embedded LDAP over LDAPS.
> *The bug*
> The interceptor derives the username with:
> {code:java}
> final String username = LdapUtils.extractUsernameFromEntry(entry, "uid",
> "cn");{code}
> {{extractUsernameFromEntry}} reads attribute values from the entry, but the
> entry has already been trimmed to the attributes the client requested.
> {{LdapGroupsMapping}} requests only the attributes it needs for group
> resolution (e.g. {{{}memberOf{}}}/{{{}group-name{}}} attributes) and does
> *not* request {{{}uid{}}}. As a result, by the time the interceptor runs, the
> {{uid}} attribute is gone and the code falls back to {{{}cn{}}}, which is a
> human-readable display name rather than the stable login id.
> The roles-lookup service is then called with the display name instead of the
> {{{}uid{}}}, and returns the wrong set of roles (or none).
> *Observed behavior*
> For the same user, two paths produce different results against the same
> roles-lookup service:
> ||Path 1||Attributes requested||user_id sent to lookup||Roles returned||
> |_Central_ cluster (requests all attributes, *)|includes uid|<uid> (e.g.
> [email protected])|correct (e.g. 4 roles)|
> |_Local_ cluster via {{LdapGroupsMapping}}|uid *not* requested|cn display
> name (e.g. John Doe)|wrong (e.g. 1 role)|
> *Root cause*
> The username is read from an attribute that may be absent depending on what
> the client requested. The entry's DN ({{{}uid=…,ou=…{}}}), however, is always
> present regardless of the requested attribute set, and already carries the
> stable {{uid}} in its RDN.
> *Steps to Reproduce*
> 1. Configure Knox's embedded LDAP with roles lookup enabled.
> 2. Perform an LDAP search that requests only group-related attributes and
> omits {{uid}} (as Hadoop {{LdapGroupsMapping}} does).
> 3. Observe that the roles-lookup call is keyed on the {{cn}} display name,
> returning the wrong roles.
> 4. Repeat the search requesting * (all attributes) and observe the correct
> roles - confirming the discrepancy is driven purely by which attributes the
> client requested.
> *Expected Behavior*
> Role lookup is keyed on the stable {{uid}} regardless of which attributes the
> client requested, so all clients receive a consistent set of roles for the
> same user.
> *Proposed Fix*
> Derive the username from the entry's DN first (always present), falling back
> to entry attributes only if the DN does not yield a uid:
> {code:java}
> String username = LdapUtils.extractUsernameFromDn(entry.getDn());
> if (username == null) {
> username = LdapUtils.extractUsernameFromEntry(entry, "uid", "cn");
> }{code}
> {{LdapUtils.extractUsernameFromDn}} returns the RDN value when the RDN type
> is {{{}uid{}}}, which is immune to attribute trimming. The fallback preserves
> existing behavior for entries whose DN is not uid-based.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
