feat: Add reverse group lookup for LDAP servers that do not return memberOf - #19432
feat: Add reverse group lookup for LDAP servers that do not return memberOf#19432JWuCines wants to merge 3 commits into
Conversation
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
This is an automated review by Codex GPT-5
fc3d1cf to
a81ccf7
Compare
FrankChen021
left a comment
There was a problem hiding this comment.
Reviewed 8 of 8 changed files. The reverse group lookup now runs only after password verification, and I found no remaining PR-caused correctness issues in the LDAP reverse lookup changes.
This is an automated review by Codex GPT-5
|
Hey @FrankChen021, |
FrankChen021
left a comment
There was a problem hiding this comment.
I have reviewed the updated PR for correctness, edge cases, concurrency, and integration risks; no issues found.
Reviewed 8 of 8 changed files.
This is an automated review by Codex GPT-5.5
|
@FrankChen021 can you review and approve it? Can you also merge the PR as reviewer and committer? |
e641d7c to
6d33734
Compare
|
Hi @clintropolis, @gianm and @kfaraz, |
FrankChen021
left a comment
There was a problem hiding this comment.
I have reviewed the updated code for correctness, edge cases, concurrency, and integration risks; no issues found. The rebase introduced no PR behavior changes since the previous reviewed SHA.
Reviewed 8 of 8 changed files.
This is an automated review by Codex GPT-5.6-Sol
6d33734 to
b29310a
Compare
Description
When using LDAP servers that do not return the
memberOfattribute in user search results, group-based authorization denies all requests because Druid cannot determine the user's group memberships. This is a known issue with LDAP servers such as OpenLDAP, wherememberOfis either not enabled by default (it requires thememberofoverlay module) or is stored as an operational attribute that Java JNDI cannot retrieve — even whenldapsearchreturns it correctly.This PR adds an optional reverse group lookup mechanism to
LDAPCredentialsValidator. When configured, if the user search does not return amemberOfattribute, Druid searches group entries to find which groups contain the user's DN and injects the resolved group DNs as syntheticmemberOfvalues into theSearchResult. The existingLDAPRoleProvider(authorizer) then processes these groups as usual, requiring no changes.The feature is disabled by default. It activates only when both
groupBaseDnandgroupSearchare configured and the user search result lacksmemberOf.Added reverse group lookup to LDAPCredentialsValidator
Added
populateMemberOfFromGroupSearch()method that performs a reverse LDAP search (e.g.,(uniqueMember=<userDN>)) against the configured group base DN. The method:setReturningAttributes(new String[]{"1.1"})to request only the DN, minimizing response size.encodeForLDAP(userDn, true)to prevent LDAP filter injection.NamingExceptioninternally and logs an error, so a misconfigured group search does not block user authentication — the user proceeds without group memberships.Added group search configuration to BasicAuthLDAPConfig
Added two optional fields (
groupBaseDn,groupSearch) with a backward-compatible constructor that delegates to the new constructor withnulldefaults. AddedisGroupSearchConfigured()convenience method.Added helper methods for readability
hasMemberOfAttribute(SearchResult): checks ifmemberOfis present on the search result.isGroupSearchConfigured(): checks if both group search properties are set.Added embedded integration test for reverse group lookup
LdapReverseGroupLookupAuthResource: ExtendsLdapAuthResourceto addgroupBaseDnandgroupSearchproperties to the LDAP credentials validator configuration.BasicAuthLdapReverseGroupLookupTest: ExtendsBasicAuthLdapConfigurationTestto run the full LDAP auth test suite with reverse group lookup enabled. Verifies that group-based authorization works on OpenLDAP (which does not returnmemberOfby default).Updated documentation
docs/development/extensions-core/druid-basic-security.md: Added property reference entries forgroupBaseDnandgroupSearch.docs/operations/auth-ldap.md: Added "Group search reverse lookup configuration" section explaining the problem and configuration. Updated the existing info box to reference both manual role mapping and the new reverse lookup as alternatives.Updated spellcheck dictionary
website/.spelling: AddedDNandJNDIto the global dictionary to fix 5 spellcheck errors in the new documentation.Release note
Added support for LDAP group-based authorization on servers that do not return the
memberOfattribute (e.g., OpenLDAP). ConfiguregroupBaseDnandgroupSearchon the LDAP credentials validator to enable a reverse group lookup that resolves group memberships automatically.Key changed/added classes in this PR
BasicAuthLDAPConfigLDAPCredentialsValidatorLDAPCredentialsValidatorTestLdapReverseGroupLookupAuthResourceBasicAuthLdapReverseGroupLookupTestThis PR has: