necouchman commented on code in PR #1170:
URL: https://github.com/apache/guacamole-client/pull/1170#discussion_r3118772774


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledAuthenticatedUser.java:
##########
@@ -170,11 +169,21 @@ public String getIdentifier() {
     public void setIdentifier(String identifier) {
         user.setIdentifier(identifier);
     }
-    
+
+
+    /**
+     * Expands a user's groups through the parents in database group hierarchy
+     * so that parent groups of external groups (e.g. SAML/SSO group claims)
+     * are included. This also covers the user's own direct
+     * DB memberships (via entity_id) and skeleton users with null entity_id.
+     *
+     * @return
+     *     The set of effective groups for this user, whether inherited or
+     *     direct.
+     */
     @Override
     public Set<String> getEffectiveUserGroups() {
-        return Sets.union(user.getEffectiveUserGroups(),
-                super.getEffectiveUserGroups());
+        return user.expandEffectiveGroups(super.getEffectiveUserGroups());

Review Comment:
   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?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to