KNOX-3337: Add optional LDAP role lookup support#1247
Conversation
|
Cc. @handavid |
Test Results22 tests 22 ✅ 2s ⏱️ Results for commit 1c43561. ♻️ This comment has been updated with latest results. |
|
@handavid - I implemented what we discussed offline the other day. I'd to force push my changes due to |
| final List<Entry> entries = new ArrayList<>(); | ||
| try (EntryFilteringCursor cursor = next(ctx)) { | ||
| while (cursor.next()) { | ||
| entries.add(modifyEntry(cursor.get())); |
There was a problem hiding this comment.
I don't think it makes sense to do this entry-by-entry. this will be very inefficient if we have an entry set where many users contain the same groups. Instead, I think this should be done in 3 steps.
- collect all the users and groups
- map the users and groups to roles. you can re-use the
RoleMappingclass - replace the
memberOfgroups with roles and add any roles for the user.
There was a problem hiding this comment.
Thank you for the feedback regarding the batching strategy! I looked into implementing the 3-step approach (Collect -> Batch Map -> Merge), but I found that because our current REST API endpoint is designed for single-context lookups (one user + their associated groups), batching actually introduces more overhead rather than reducing it.
Specifically, since the API doesn't support a 'Bulk' request (e.g., fetching roles for multiple users at once), we must still make
Here is a comparison for a search result returning 100 users sharing the same 5 groups:
| Metric | Per-Entry Approach (Proposed) | 3-Step Strategy (Batching) |
|---|---|---|
| Total HTTP Requests | 100 | 105 |
| Backend DB Lookups | 600 | 105 |
| Network Bottleneck | 100 round-trips | 105 round-trips |
| Implementation | Simple & Stateless | Complex (manual result merging) |
Conclusion:
While the 3-step strategy reduces the number of lookups on the backend (database) side, it increases the number of HTTP round-trips, which is the primary latency bottleneck in this architecture. Furthermore, the REST API backend was specifically built to perform the User/Group 'OR' logic in a single query. By using the per-entry approach, we leverage the backend's design to get the final role set in exactly one trip per user.
If we ever introduce a 'Bulk' endpoint to the REST API, the 3-step strategy would absolutely be the way to go. But for the current API, the per-entry approach is both faster and simpler.
There was a problem hiding this comment.
thanks for looking into this and providing such a detailed analysis.
2 participants
KNOX-3337 - Introduce LDAP Role Lookup Service
What changes were proposed in this pull request?
This PR introduces a new LDAPRolesLookupService to Apache Knox, allowing for dynamic mapping of users and groups to specific roles during the authentication process. This is particularly useful for downstream services that require fine-grained role information (e.g.,
platform:admin,workspace:viewer) rather than just raw LDAP groups.The implementation has been evolved to decouple role resolution from the core LDAP service and move it into the request processing pipeline.
Key changes include:
LDAPRolesLookupServicewith a pluggable strategy pattern.FileBasedLdapRolesLookup: Maps roles based on a local JSON configuration file.RestApiLdapRolesLookup: Queries an external REST API to retrieve role assignments.LDAPRolesLookupInterceptorwithin the embedded LDAP server (ApacheDS). This interceptor dynamically intercepts LDAP search results and replaces memberOf group DNs with resolved role DNs. This ensures that any LDAP-client-based authentication receives roles instead of raw groups.AbstractAuthResource(the base forPreAuthandExtAuthz) to utilize theLDAPRolesLookupServicedirectly. When enabled, resolved roles are propagated in the X-Knox-Actor-Groups HTTP headers, seamlessly replacing raw groups for downstream services.GatewayConfigproperties to configure the lookup strategy, file paths, and API endpoints.How was this patch tested?
The changes were verified through both existing and newly added unit tests:
FileBasedLdapRolesLookupTest: Verified JSON mapping logic for users and groups.RestApiLdapRolesLookupTest: Verified REST API integration using mocked HTTP clients.LDAPRolesLookupInterceptorTest: Verified the logic of replacing LDAP attributes within the ApacheDS interceptor.KnoxLDAPServiceTest: Updated to ensure getUserGroups correctly returns raw groups while the interceptor handles role replacement for LDAP searches.PreAuthResourceTest&ExtAuthzResourceTest: AddedtestPopulatingGroupsWithRolesto verify that X-Knox-Actor-Groups headers are correctly populated with resolved roles.Integration Tests
No automated test added this time, but I configured Knox with MockServer as role lookup provider
and set the following configs in
gateway-reloadable.xml:Verified that the LDAP roles lookup service (as well as the Knox LDAP service) was reconfigured properly:
Then issued the following
curlcommand:Checked the logs:
Tested
ldapsearchtoo:As you can see, the
cnattribute was changed with the resolved roles.Logs:
UI changes
N/A