Skip to content

[ZEPPELIN-6171] Resolve nested LDAP groups on FreeIPA via memberOf - #5505

Merged
tbonelee merged 3 commits into
apache:masterfrom
HwangRock:ZEPPELIN-6171-freeipa-memberof-nested-groups
Oct 5, 2026
Merged

tbonelee merged 3 commits into
apache:masterfrom
HwangRock:ZEPPELIN-6171-freeipa-memberof-nested-groups

Conversation

@HwangRock

Copy link
Copy Markdown
Contributor

What is this PR for?

LdapRealm resolves a user's groups in one of two ways today: the Active Directory LDAP_MATCHING_RULE_IN_CHAIN operator, or a group search over the member attribute. Neither works for nested groups on FreeIPA / 389 Directory Server — the AD matching rule isn't supported there, and the plain member search only sees direct members. So if a user is in dev and dev is a member of eng, the user's eng membership (and any role mapped to it) silently disappears.

This adds a third path. When ldapRealm.groupSearchEnableMemberOf = true, the realm reads the user entry's own memberOf attribute instead of walking the group tree. FreeIPA/389 DS already flattens direct and nested membership onto memberOf, so a single lookup of the user entry gives you the full set, nested groups included.

While adding it I split rolesFor() into three small per-strategy methods (matching-rule / group-membership / memberOf) so each path is readable on its own. The two existing paths are moved as-is, no behavior change.

New settings:

# resolve nested groups via the user's memberOf attribute (e.g. FreeIPA / 389 DS)
ldapRealm.groupSearchEnableMemberOf = true
ldapRealm.memberOfAttribute = memberOf

A few decisions worth calling out:

  • Precedence when both are on. If groupSearchEnableMatchingRuleInChain and groupSearchEnableMemberOf are both set, the matching-rule path wins and we log a one-time warning, rather than refusing to start. This keeps existing AD setups behaving exactly as before; the cost is that a contradictory config isn't hard-rejected, just warned.
  • Group name comes from the leaf RDN only. A memberOf value is a full group DN, and we take the group name from its leaf RDN without scanning ancestors. On FreeIPA a container on the path (cn=groups,cn=accounts,...) has the same cn= type as the group itself, so scanning would pick the wrong one. A malformed DN is skipped (and logged) instead of failing the login.
  • No behavior change for the existing paths. The matching-rule branch still doesn't populate the session group-name set the way the default branch does — that's pre-existing and I left it alone on purpose rather than "fixing" it in a refactor.

One prerequisite (documented in shiro_authentication.md): memberOf is only returned to an authenticated bind, which Zeppelin already does via systemUsername/systemPassword. Deployments that split groups and members across separate backends may also need server-side memberOf scope configuration.

What type of PR is it?

Improvement

Todos

  • memberOf-based group resolution path
  • split rolesFor() into per-strategy methods
  • unit tests
  • docs

What is the Jira issue?

ZEPPELIN-6171

How should this be tested?

LdapRealmTest (11 tests, mock-based) covers the memberOf path: nested resolution, matching-rule precedence, leaf-RDN fallback, and the empty / missing memberOf cases. The existing tests pass unchanged, which is what pins down that the two old paths still behave the same.

I also ran it end-to-end against a real 389 Directory Server (FreeIPA's LDAP engine) with a nested setup — alice ∈ dev, and dev ∈ eng:

# alice's memberOf at the directory level (server-flattened, ground truth)
$ ldapsearch ... -b 'uid=alice,ou=people,dc=example,dc=com' -s base memberOf
dn: uid=alice,ou=people,dc=example,dc=com
memberOf: cn=dev,ou=groups,dc=example,dc=com
memberOf: cn=eng,ou=groups,dc=example,dc=com

# LdapRealm.rolesFor(alice) against that server, only the new flag differs:
case A  groupSearchEnableMemberOf=false  ->  [dev]        # nested cn=eng missing
case B  groupSearchEnableMemberOf=true   ->  [dev, eng]   # nested cn=eng resolved

So the nested group is recovered only with the new flag on.

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? Yes — docs/setup/security/shiro_authentication.md is updated.

On FreeIPA (389 Directory Server) the AD LDAP_MATCHING_RULE_IN_CHAIN
operator isn't supported, and the default member-attribute search only
sees direct members, so a user's nested group memberships and their
roles were dropped.

Add a third resolution path: when groupSearchEnableMemberOf is set, read
the user entry's own memberOf attribute, which the directory pre-flattens
with nested membership. Split rolesFor() into three per-strategy private
methods so each path stays readable; the two existing paths are moved
unchanged and matchingRuleInChain keeps precedence when both are enabled.
try {
while (memberOfValues.hasMore()) {
String groupDn = memberOfValues.next().toString();
String groupName = groupNameFromMemberOfDn(groupDn);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On FreeIPA, memberOf also holds entries that aren't groups. FreeIPA configures the MemberOf plugin with memberUser, memberHost and ipaOwner in addition to member (install/share/memberof-conf.ldif), so a user's memberOf also carries the DNs of HBAC rules, sudo rules and roles (cn=roles). The current code treats all of them as groups.

I checked this by setting up a 389 DS container with the same plugin config and DN layout as FreeIPA (cn=groups,cn=accounts, etc.) and logging in to Zeppelin built from this branch. alice ∈ dev ∈ eng, and alice also has the helpdesk role, one HBAC rule and one sudo rule:

  • groupSearchEnableMemberOf=false: ["user_role"]
  • groupSearchEnableMemberOf=true: ["user_role","admin_role","helpdesk","aaaa-1111","bbbb-2222"]

The nested group is resolved as intended, but the ipaUniqueIDs of the HBAC and sudo rules and the role name also became roles. If such a name matches a key in rolesByGroup, a roles[...] in [urls], or a role name used in note permissions, it grants permissions nobody intended.

How about accepting only DNs under groupSearchBase, like the other two paths? It can be checked with new LdapName(groupDn).startsWith(new LdapName(groupSearchBase)), and in the same setup the result became ["user_role","admin_role"].

When groupSearchBase isn't set, it falls back to searchBase (usually the root), so the filter wouldn't filter anything. Since this flag isn't in any release yet, how about starting strict: in memberOf mode, accept no memberOf values when groupSearchBase isn't set, and warn in onInit()? Relaxing that later stays compatible, but tightening it later would change existing users' roles.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this, it's a critical one!
Fixed it to only accept DNs under groupSearchBase like you suggested.

}
Rdn leafRdn = rdns.get(rdns.size() - 1);
if (!getGroupIdAttribute().equalsIgnoreCase(leafRdn.getType())) {
LOGGER.warn("memberOf value '{}' leaf RDN type '{}' does not match groupIdAttribute "

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the memberOf path doesn't look up the group entry, it can only get the name from the configured groupIdAttribute when the leaf RDN type matches it. When the type differs, how about skipping the value instead of using another attribute's value? For example, with groupIdAttribute = gidNumber and rolesByGroup keyed by gid numbers, turning on memberOf mode now silently adds the cn value as an unmapped role. When skipping, a DEBUG log plus a line in the docs ("in memberOf mode, the leaf RDN type of group DNs must match groupIdAttribute") would be enough.

Such a setup can still use memberOf mode by switching to groupIdAttribute = cn and keying rolesByGroup by cn. With the current code rolesByGroup has to be re-keyed by cn anyway, so the only extra requirement is that groupIdAttribute matches what is actually used. If a setup that needs groupIdAttribute read from the group entry shows up later, a lookup only for mismatched types can be added then. Skipping now means that addition won't change existing users' behavior.

This WARN also repeats on every request. In the setup from the comment above, the HBAC and sudo rule DNs went through this path, and 10 calls to the notebook API grew this WARN from 2 lines to 42. With the scope filter this path would mostly see group DNs, but a mismatched config would repeat the same way.

With this change, the first assertion in testGroupNameFromMemberOfDnFallback would expect null.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied this as you suggested.

String userDn = getUserDnForSearch(userName);

if (groupSearchEnableMatchingRuleInChain && groupSearchEnableMemberOf
&& !warnedBothGroupSearchModes) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both flags are fixed at configuration time, so checking once in onInit() seems enough. That would remove the warnedBothGroupSearchModes field and the per-request check.

Also, testWarnBothGroupSearchModesLogsOnlyOnce doesn't check the log count; it only compares the roles from the two calls. It still passes after removing warnedBothGroupSearchModes = true;, which makes the warning go from 1 to 2 within this test. With the check in onInit(), "only once" is guaranteed by structure, and the precedence is already covered by testRolesForMatchingRuleInChainTakesPrecedenceOverMemberOf, so I think this test could be dropped.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied your suggestion. Also dropped that test you pointed out.

return numResults;
}

// Default path: search groups and check the member attribute for the user DN.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method name already says this, so this comment could be removed.

Suggested change
// Default path: search groups and check the member attribute for the user DN.

On FreeIPA the memberOf attribute also carries HBAC rules, sudo rules and
roles, not just groups. Only accept memberOf DNs under groupSearchBase so
those non-group entries don't become roles; when groupSearchBase is unset,
resolve nothing and warn in onInit().

Also skip memberOf values whose leaf RDN type doesn't match groupIdAttribute
instead of using the value, and move the both-flags warning into onInit() so
it is logged once (dropping the per-request field).
@tbonelee
tbonelee merged commit 943a3fb into apache:master Oct 5, 2026
22 of 25 checks passed
@tbonelee

tbonelee commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Merged into master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants