KNOX-3328: Implement recursive group resolution for LDAP proxy#1236
KNOX-3328: Implement recursive group resolution for LDAP proxy#1236smolnar82 wants to merge 5 commits into
Conversation
|
Cc. @handavid |
Test Results21 tests 21 ✅ 1s ⏱️ Results for commit 529e44d. ♻️ This comment has been updated with latest results. |
lmccay
left a comment
lmccay
left a comment
There was a problem hiding this comment.
@smolnar82 - thanks for this improvement!
Mostly looks good.
I question the default depth of 10 and am also wondering about cursor leaks.
Cursor leaks may have been an issue prior to this PR but we should be sure that things do get cleaned up properly.
| cursor.close(); | ||
|
|
||
| // 2. Try search in group base if not found in user base | ||
| String groupFilter = "(cn=" + username + ")"; |
There was a problem hiding this comment.
Do we know for sure that this will always be common name?
Also, I think a little more description of what this groupFilter's affect on the search will be in the comment would go a long way for future readers.
There was a problem hiding this comment.
I'll introduce a configurable groupIdentifierAttribute, defaulting to cn, and add descriptive comments to clarify this search.
| } | ||
| cursor.close(); | ||
|
|
||
| return null; |
There was a problem hiding this comment.
do we not need any logging here?
What affect does not getting an Entry for the getUser method have?
There was a problem hiding this comment.
The return null is actually not new in this PR:
According to the JavaDoc of the parent class, this is perfectly ok:
On the other hand, I agree that it's good to have some DEBUG log.
2 participants
KNOX-3328 - Support recursive group resolution in LDAP Proxy Service
What changes were proposed in this pull request?
This PR introduces recursive group resolution to the
LdapProxyBackend. Key changes include:LdapProxyBackend.getUserto search both the user and group search bases. This allows group entries to be returned as "proxy entries" enriched with their ownmemberOfattributes.(*, +)to ensure complete profile data, while recursive steps specifically request only thememberOfand operational attributes(+)to minimize network payload and processing overhead.How was this patch tested?
The patch was tested using the LdapProxyBackendTest suite with an embedded ApacheDS server.
testGetUserGroupsRecursive: Verifies 3-level deep nesting is resolved correctly.testGetUserGroupsRecursiveCircular: Verifies that circular references are handled without errors.testGetUserGroupsRecursiveMaxDepth: Verifies that the recursion stops at the configured depth.testGetUserGroupsRecursiveUseMemberOf: Verifies recursive resolution whenuseMemberOfis set totrue.ldap-proxy-backend-test.ldifto include nested groups, circular groups, and groups withmemberOfattributes.Integration Tests
Automated unit tests were added to gateway-server. These tests use an embedded LDAP server to simulate a real-world proxy scenario, which provides high-fidelity verification of the recursive logic and LDAP protocol handling. No changes were required to .github/workflows/tests as the standard test suite covers these new unit tests.
UI changes
N/A