GUACAMOLE-2224: Fix effective user group expansion for SAML/SSO groups

[tkuhlengel]

PR corresponding to https://issues.apache.org/jira/browse/GUACAMOLE-2224

Problem

When a user authenticates via an external SSO provider (e.g. SAML with Azure AD), the identity provider asserts group membership by name. These names are stored on the AuthenticatedUser and surfaced through getEffectiveUserGroups().

The previous implementation in ModeledAuthenticatedUser resolved this with a flat union:

return Sets.union(user.getEffectiveUserGroups(), super.getEffectiveUserGroups());

This combined the user's database group memberships with the raw SSO claims, but made no attempt to walk the database group hierarchy. If child-group (matched by SAML claim) was a member of parent-group in the database, parent-group was never included — and any permissions granted only to parent-group were invisible to the SSO user.

Database-native users were unaffected because their memberships are stored as entity_id references already handled by the existing recursive expansion in EntityService.

Changes

ModeledPermissions.expandEffectiveGroups() — guacamole-auth-jdbc-base

A new method delegates to EntityService.retrieveEffectiveGroups(), passing caller-provided external group names as additional seeds for recursive DB group expansion:

public Set<String> expandEffectiveGroups(Collection<String> externalEffectiveGroups) {
    return entityService.retrieveEffectiveGroups(this, externalEffectiveGroups);
}

This exposes the existing recursive CTE logic as a callable entry point on the permissions object.

ModeledAuthenticatedUser.getEffectiveUserGroups() — guacamole-auth-jdbc-base

The flat union is replaced with a call to expandEffectiveGroups(), passing the raw SSO claims as seeds:

@Override
public Set<String> getEffectiveUserGroups() {
    return user.expandEffectiveGroups(super.getEffectiveUserGroups());
}

super.getEffectiveUserGroups() returns the SSO group name claims from the originating AuthenticatedUser. These are now fed into the recursive expansion, so any ancestor groups of those claims in the database are included in the result — making inherited permissions visible to SSO users.

[Copilot]

Pull request overview

This pull request fixes a bug where SSO users (e.g., via SAML/Azure AD) did not receive permissions from parent groups in the database group hierarchy. When an SSO provider asserted group membership by name, the previous implementation simply unioned those names with database groups using Sets.union(), without recursively expanding parent groups. This meant that permissions granted only to parent groups were invisible to SSO users, while database-native users were unaffected because their memberships use entity_id references handled by existing recursive expansion logic.

Changes:

• Added ModeledPermissions.expandEffectiveGroups() method to expose the existing recursive group expansion logic from EntityService as a callable entry point
• Modified ModeledAuthenticatedUser.getEffectiveUserGroups() to call the new expansion method, passing SSO group claims as seeds for recursive database group expansion
• Removed unused Guava Sets import

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
ModeledPermissions.java	Adds new expandEffectiveGroups() method that delegates to EntityService.retrieveEffectiveGroups() to recursively expand external group names through the database group hierarchy
ModeledAuthenticatedUser.java	Updates getEffectiveUserGroups() to use the new expansion method instead of flat union, and removes unused Guava import

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[tkuhlengel]

This is ready for review.

[necouchman]

Thanks @tkuhlengel, I'll try to take a look. FYI, your "Typo fix" commit needs to either be properly tagged with the GUACAMOLE-2224 issue, or needs to be squashed into the other one.

[necouchman]

Rather than creating a new expandEffectiveGroups() method in the ModeledUser class, is there any reason not to just call the entityService.retrieveEffectiveGroups() method, here, and add it to the Sets.union() call?

[tkuhlengel]

I think ModeledAuthenticatedUser doesn't have access to entityService. It's injected in ModeledPermissions, which ModeledUser inherits from. The thin expandEffectiveGroups() wrapper keeps the service access encapsulated without changing any permissions or inheritance.

To call entityService directly here I'd need to either inject it into ModeledAuthenticatedUser or add a getter method on ModeledUser.

I can try to go that route if you want. The wrapper user.expandEffectiveGroups() seemed like the most compact option.

[necouchman]

I think injecting entityService into the ModeledAuthenticatedUser class would be the way I'd prefer to go - it seems to me that would result, overall, in easier-to-follow code, rather than having to jump from one class to another and then on to another.

[tkuhlengel]

Fixed the commit issue.

Replied to the review point.

[tkuhlengel]

Ready for followup.