You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by "smolnar82 (via GitHub)" <gi...@apache.org> on 2026/07/22 11:20:44 UTC
[PR] KNOX-3389: Embedded LDAP getUserGroups drops roles-lookup roles that have no backing group [knox]
smolnar82 opened a new pull request, #1318:
URL: https://github.com/apache/knox/pull/1318
[KNOX-3389](https://issues.apache.org/jira/browse/KNOX-3389) - Embedded LDAP getUserGroups drops roles-lookup roles that have no backing group
## What changes were proposed in this pull request?
When the roles lookup interceptor is enabled on the embedded LDAP server, resolved roles are stored on the user entry as `memberOf` values. If the user has no existing groups, there is no template group DN available, so `LDAPRolesLookupInterceptor.addRoleAttribute` stores each role as a bare RDN (`cn=<role>`) with no trailing DN components.
`KnoxLDAPServerManager.getUserGroups` extracted the group name as the substring between `cn=` and the first comma.
For a bare RDN there is no comma, so`indexOf(',')` returned `-1` and the value was silently dropped, thus the resolved roles never surfaced as groups.
This PR makes `getUserGroups` handle the bare-RDN case by taking everything after `cn=`, gated on `hasRolesLookupInterceptor` so the behavior only relaxes for the path that actually produces bare RDNs:
- Full DN (`cn=role,ou=groups,dc=...`): unchanged, extract between `cn=` and the first comma.
- Bare RDN (`cn=role`): only when the roles lookup interceptor is active, take everything after `cn=`.
- No-comma value without the interceptor: still skipped as unexpected/malformed data.
## How was this patch tested?
Added two unit tests to `KnoxLDAPServerManagerTest` that start the embedded server, seed a user via the admin session, and call `getUserGroups` directly:
- `testGetUserGroupsResolvesBareRdnRolesWhenRolesLookupActive` - user with `memberOf: cn=platform:awc-admin-sam` and `cn=ml-workspace-abc:viewer-sam`, interceptor flag on: both roles resolve as groups (reproduces the reported scenario).
- `testGetUserGroupsIgnoresBareRdnWhenRolesLookupInactive` - a full-DN group plus a bare RDN, interceptor flag off: only the full-DN group resolves, proving the lenient parse is scoped to the roles-lookup case.
Manually tested as well (using the same setup as described in the JIRA). After my changes applied, I could see the resolved groups:
```
~ $ curl -iku sam:sam-password http://localhost:8443/gateway/sandbox/auth/api/v1/pre
HTTP/1.1 200 OK
Date: Wed, 22 Jul 2026 11:10:35 GMT
Set-Cookie: KNOXSESSIONID=node01qag1cyt0x3sat9xtybpnjtlb0.node0; Path=/gateway/sandbox; Secure; HttpOnly
Expires: Thu, 01 Jan 1970 00:00:00 GMT
Set-Cookie: rememberMe=deleteMe; Path=/gateway/sandbox; Max-Age=0; Expires=Tue, 21-Jul-2026 11:10:35 GMT; SameSite=lax
x-knox-username: sam
x-knox-roles: ml-workspace-abc:viewer-sam,platform:awc-admin-sam
Content-Length: 0
```
## Integration Tests
N/A
## UI changes
N/A
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3389: Embedded LDAP getUserGroups drops roles-lookup roles that have no backing group [knox]
Posted by "handavid (via GitHub)" <gi...@apache.org>.
handavid commented on code in PR #1318:
URL: https://github.com/apache/knox/pull/1318#discussion_r3630825141
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java:
##########
@@ -470,6 +470,13 @@ public List<String> getUserGroups(String username) throws Exception {
int commaIdx = groupDn.indexOf(',');
if (commaIdx > 0) {
groups.add(groupDn.substring(3, commaIdx));
+ } else if (hasRolesLookupInterceptor) {
Review Comment:
This looks ok, but I was wondering if we could convert the value to a `Dn` and pull the `Rdn` instead of string splitting. e.g., something like:
```
Dn groupDn = new Dn(value.getString());
String groupName = groupDn.getRdn().getValue();
```
I feel that would be more resilient regardless of what RDN is used
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3389: Embedded LDAP getUserGroups drops roles-lookup roles that have no backing group [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 merged PR #1318:
URL: https://github.com/apache/knox/pull/1318
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3389: Embedded LDAP getUserGroups drops roles-lookup roles that have no backing group [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #1318:
URL: https://github.com/apache/knox/pull/1318#discussion_r3631079730
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java:
##########
@@ -470,6 +470,13 @@ public List<String> getUserGroups(String username) throws Exception {
int commaIdx = groupDn.indexOf(',');
if (commaIdx > 0) {
groups.add(groupDn.substring(3, commaIdx));
+ } else if (hasRolesLookupInterceptor) {
Review Comment:
Thanks, @handavid , for your review. It's actually a really good idea so I submitted a new commit that uses an already existing group parser util (by `Dn`) instead of string operations.
I tested the updated code manually as well:
- backend LDAP with 0 groups + roles lookup enabled
- backend LDAP with 1+ groups + roles lookup enabled
- backend LDAP with 0 groups + roles lookup disabled
- backend LDAP with 1+ groups + roles lookup disabled
In all cases I got the desired output.
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3389: Embedded LDAP getUserGroups drops roles-lookup roles that have no backing group [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on PR #1318:
URL: https://github.com/apache/knox/pull/1318#issuecomment-5045196198
Cc. @handavid
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3389: Embedded LDAP getUserGroups drops roles-lookup roles that have no backing group [knox]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1318:
URL: https://github.com/apache/knox/pull/1318#issuecomment-5045645387
## Test Results
40 tests 40 ✅ 4s ⏱️
3 suites 0 💤
3 files 0 ❌
Results for commit 1e259629.
[test-results]:data:application/gzip;base64,H4sIAEi0YGoC/12Myw6CMBBFf4V07aIPQOrPmLFOk4lATR8r4r87oPJwd8+5yZmEpx6TuFTmVIlUKK9wLxEyhZGxZuQjz1ctf3BNxbk/86Anm014oP4gMMYQvyaWcS3O+xD8iK238C638L7mwjBQZhAKdWNbbXWL/mxAobSInbpJ8F4a3aF0jQKD4vUG8xd2QP8AAAA=
--
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: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org