Impersonating Troubleshooters can now impersonate users and groups

api/src/org/labkey/api/ApiModule.java

@@ -135,6 +135,7 @@
 import org.labkey.api.security.SecurityManager;
 import org.labkey.api.security.UserManager;
 import org.labkey.api.security.ValidEmail;
+import org.labkey.api.security.impersonation.ImpersonationTest;
 import org.labkey.api.settings.AppProps;
 import org.labkey.api.settings.AppPropsTestCase;
 import org.labkey.api.settings.BaseServerProperties;
@@ -398,6 +399,7 @@ public void registerServlets(ServletContext servletCtx)
             FileUtil.TestCase.class,
             GenerateUniqueDataIterator.TestCase.class,
             HelpTopic.TestCase.class,
+            ImpersonationTest.class,
             InlineInClauseGenerator.TestCase.class,
             JSONDataLoader.HeaderMatchTest.class,
             JSONDataLoader.MetadataTest.class,

api/src/org/labkey/api/security/NormalPermissionsContext.java

@@ -22,8 +22,7 @@
 import org.labkey.api.security.impersonation.ImpersonationContextFactory;
 import org.labkey.api.security.impersonation.RoleImpersonationContextFactory;
 import org.labkey.api.security.impersonation.UserImpersonationContextFactory;
-import org.labkey.api.security.permissions.AdminPermission;
-import org.labkey.api.security.permissions.CanImpersonatePrivilegedSiteRolesPermission;
+import org.labkey.api.security.permissions.ImpersonatePermission;
 import org.labkey.api.util.PageFlowUtil;
 import org.labkey.api.view.ActionURL;
 import org.labkey.api.view.NavTree;
@@ -93,22 +92,16 @@ public void addMenu(NavTree menu, Container c, User user, ActionURL currentURL)
         {
             @Nullable Container project = c.getProject();
 
-            // Must be site or project admin (folder admins can't impersonate)
-            if (user.hasRootAdminPermission() || (null != project && project.hasPermission(user, AdminPermission.class)))
+            // Site admin, app admin, and impersonating troubleshooter can impersonate anywhere; project admin can
+            // impersonate in that project. Folder admins can't impersonate.
+            if (user.hasRootPermission(ImpersonatePermission.class) || (project != null && project.hasPermission(user, ImpersonatePermission.class)))
             {
                 NavTree impersonateMenu = new NavTree("Impersonate");
                 UserImpersonationContextFactory.addMenu(impersonateMenu);
                 GroupImpersonationContextFactory.addMenu(impersonateMenu);
                 RoleImpersonationContextFactory.addMenu(impersonateMenu);
                 menu.addChild(impersonateMenu);
             }
-            // Or Impersonating Troubleshooter to impersonate site roles only
-            else if (null == project && user.hasRootPermission(CanImpersonatePrivilegedSiteRolesPermission.class))
-            {
-                NavTree impersonateMenu = new NavTree("Impersonate");
-                RoleImpersonationContextFactory.addMenu(impersonateMenu);
-                menu.addChild(impersonateMenu);
-            }
         }
 
         NavTree signOut = new NavTree("Sign Out", PageFlowUtil.urlProvider(LoginUrls.class).getLogoutURL(c));

api/src/org/labkey/api/security/PermissionsContext.java

@@ -64,7 +64,7 @@ default Stream<Role> getAssignedRoles(User user, SecurableResource resource)
                 if (!(role instanceof AbstractRootContainerRole siteRole))
                     throw new IllegalStateException("Root roles should all be AbstractRootContainerRole");
 
-                return siteRole.isAvailableEverywhere() || resource.equals(root);
+                return siteRole.isApplicableOutsideRoot() || resource.equals(root);
             });
 
         if (!resource.equals(root))

api/src/org/labkey/api/security/SecurityManager.java

@@ -74,7 +74,7 @@
 import org.labkey.api.security.permissions.AbstractPermission;
 import org.labkey.api.security.permissions.AddUserPermission;
 import org.labkey.api.security.permissions.AdminPermission;
-import org.labkey.api.security.permissions.CanImpersonateSiteRolesPermission;
+import org.labkey.api.security.permissions.ImpersonatePermission;
 import org.labkey.api.security.permissions.DeletePermission;
 import org.labkey.api.security.permissions.InsertPermission;
 import org.labkey.api.security.permissions.Permission;
@@ -822,7 +822,7 @@ public static void impersonateUser(ViewContext viewContext, User impersonatedUse
         @Nullable Container project = viewContext.getContainer().getProject();
         User user = viewContext.getUser();
 
-        if (user.hasRootAdminPermission())
+        if (user.hasRootPermission(ImpersonatePermission.class))
             project = null;
 
         impersonate(viewContext, new UserImpersonationContextFactory(project, user, impersonatedUser, returnUrl));
@@ -839,7 +839,7 @@ public static void impersonateRoles(ViewContext viewContext, Collection<Role> ne
         @Nullable Container project = viewContext.getContainer().getProject();
         User user = viewContext.getUser();
 
-        if (user.hasRootPermission(CanImpersonateSiteRolesPermission.class))
+        if (user.hasRootPermission(ImpersonatePermission.class))
             project = null;
 
         impersonate(viewContext, new RoleImpersonationContextFactory(project, user, newImpersonationRoles, currentImpersonationRoles, returnUrl));
@@ -1412,7 +1412,7 @@ public static void ensureAtLeastOneRootAdminExists()
     }
 
     // A permission class that uniquely identifies the root admins, of which we insist there must be at least one
-    public static final Class<? extends Permission> ROOT_ADMIN_PERMISSION = CanImpersonateSiteRolesPermission.class;
+    public static final Class<? extends Permission> ROOT_ADMIN_PERMISSION = ImpersonatePermission.class;
 
     public static boolean isRootAdmin(User user)
     {

api/src/org/labkey/api/security/User.java

@@ -33,7 +33,7 @@
 import org.labkey.api.security.permissions.AnalystPermission;
 import org.labkey.api.security.permissions.ApplicationAdminPermission;
 import org.labkey.api.security.permissions.BrowserDeveloperPermission;
-import org.labkey.api.security.permissions.CanImpersonateSiteRolesPermission;
+import org.labkey.api.security.permissions.ImpersonatePermission;
 import org.labkey.api.security.permissions.DeletePermission;
 import org.labkey.api.security.permissions.InsertPermission;
 import org.labkey.api.security.permissions.Permission;
@@ -594,7 +594,7 @@ public static JSONObject getUserProps(User user, User currentUser, @Nullable Con
             props.put("isAdmin", nonNullContainer && container.hasPermission(user, AdminPermission.class));
             props.put("isRootAdmin", user.hasRootAdminPermission());
             props.put("isSystemAdmin", user.hasSiteAdminPermission());
-            props.put("canImpersonateSiteRoles", user.hasRootPermission(CanImpersonateSiteRolesPermission.class));
+            props.put("canImpersonateSiteRoles", user.hasRootPermission(ImpersonatePermission.class));
             props.put("isGuest", user.isGuest());
             props.put("isDeveloper", user.isBrowserDev());
             props.put("isAnalyst", user.hasRootPermission(AnalystPermission.class));

api/src/org/labkey/api/security/UserCache.java

@@ -83,8 +83,8 @@ private UserCache()
         return null != user ? user.cloneUser() : null;
     }
 
-    // Returns a deep copy of the active users list, allowing callers to interrogate user permissions without affecting
-    // cached users. Collection is ordered by email... maybe it should be ordered by display name?
+    // Returns a mutable deep copy of the active users list, allowing callers to interrogate user permissions without
+    // affecting cached users. Collection is ordered by email... maybe it should be ordered by display name?
     static @NotNull Collection<User> getActiveUsers()
     {
         List<User> activeUsers = getUserCollections().getActiveUsers();
@@ -95,7 +95,7 @@ private UserCache()
             .collect(Collectors.toCollection(LinkedList::new));
     }
 
-    // Returns a deep copy of the users list including deactivated accounts, allowing callers to interrogate user permissions
+    // Returns a mutable deep copy of the users list including deactivated accounts, allowing callers to interrogate user permissions
     // without affecting cached users. Collection is ordered randomly... maybe it should be ordered by display name?
     static @NotNull Collection<User> getActiveAndInactiveUsers()
     {
@@ -104,7 +104,7 @@ private UserCache()
         return users
             .stream()
             .map(User::cloneUser)
-            .collect(Collectors.toList());
+            .collect(Collectors.toCollection(LinkedList::new));
     }
 
     static @NotNull List<Integer> getUserIds()
@@ -114,7 +114,7 @@ private UserCache()
 
     static @NotNull Map<ValidEmail, User> getUserEmailMap()
     {
-        return Collections.unmodifiableMap(getUserCollections().getEmailMap());
+        return getUserCollections().getEmailMap();
     }
 
     static int getActiveUserCount()

api/src/org/labkey/api/security/UserManager.java

@@ -59,7 +59,7 @@
 import org.labkey.api.security.SecurityManager.UserManagementException;
 import org.labkey.api.security.permissions.AbstractActionPermissionTest;
 import org.labkey.api.security.permissions.ApplicationAdminPermission;
-import org.labkey.api.security.permissions.CanImpersonatePrivilegedSiteRolesPermission;
+import org.labkey.api.security.permissions.ImpersonatePrivilegedSiteRolesPermission;
 import org.labkey.api.security.permissions.SiteAdminPermission;
 import org.labkey.api.security.roles.ApplicationAdminRole;
 import org.labkey.api.security.roles.SiteAdminRole;
@@ -162,7 +162,7 @@ public static void addUserListener(UserListener listener)
     public static void addUserListener(UserListener listener, boolean meFirst)
     {
         if (meFirst)
-            _listeners.add(0, listener);
+            _listeners.addFirst(listener);
         else
             _listeners.add(listener);
     }
@@ -670,12 +670,14 @@ public static String getEmailForId(Integer userId)
     }
 
     @NotNull
+    // Returns a mutable collection
     public static Collection<User> getActiveUsers()
     {
         return getUsers(false);
     }
 
     @NotNull
+    // Returns a mutable collection
     public static Collection<User> getUsers(boolean includeInactive)
     {
         return includeInactive ? UserCache.getActiveAndInactiveUsers() : UserCache.getActiveUsers() ;
@@ -1305,7 +1307,7 @@ public void testPermissionsRetrieval()
             assertTrue("Expected all SiteAdmins to be in the AppAdmins list",
                 appAdmins.containsAll(siteAdmins));
 
-            List<User> privilegedUsers = SecurityManager.getUsersWithPermissions(ContainerManager.getRoot(), Set.of(CanImpersonatePrivilegedSiteRolesPermission.class));
+            List<User> privilegedUsers = SecurityManager.getUsersWithPermissions(ContainerManager.getRoot(), Set.of(ImpersonatePrivilegedSiteRolesPermission.class));
             assertTrue("Expected all users with privileged role impersonation permissions to have a privileged role",
                 privilegedUsers.stream().allMatch(User::hasPrivilegedRole));
 

api/src/org/labkey/api/security/impersonation/AbstractImpersonationContext.java

@@ -21,7 +21,7 @@
 import org.labkey.api.security.LoginUrls;
 import org.labkey.api.security.PermissionsContext;
 import org.labkey.api.security.User;
-import org.labkey.api.security.permissions.CanImpersonatePrivilegedSiteRolesPermission;
+import org.labkey.api.security.permissions.ImpersonatePrivilegedSiteRolesPermission;
 import org.labkey.api.security.roles.Role;
 import org.labkey.api.util.PageFlowUtil;
 import org.labkey.api.view.ActionURL;
@@ -88,7 +88,7 @@ public ImpersonationContextFactory getFactory()
      */
     protected Stream<Role> getFilteredRoles(Stream<Role> roles)
     {
-        if (getAdminUser() != null && !getAdminUser().hasRootPermission(CanImpersonatePrivilegedSiteRolesPermission.class))
+        if (getAdminUser() != null && !getAdminUser().hasRootPermission(ImpersonatePrivilegedSiteRolesPermission.class))
             return roles.filter(role -> !role.isPrivileged());
 
         return roles;

api/src/org/labkey/api/security/impersonation/GroupImpersonationContextFactory.java

@@ -31,7 +31,7 @@
 import org.labkey.api.security.SecurityManager;
 import org.labkey.api.security.User;
 import org.labkey.api.security.UserManager;
-import org.labkey.api.security.permissions.AdminPermission;
+import org.labkey.api.security.permissions.ImpersonatePermission;
 import org.labkey.api.security.roles.Role;
 import org.labkey.api.util.GUID;
 import org.labkey.api.view.ActionURL;
@@ -126,28 +126,26 @@ public User getAdminUser()
         return UserManager.getUser(_adminUserId);
     }
 
-    private static boolean canImpersonateGroup(@Nullable Container project, User user, Group group)
+    private static boolean canImpersonateGroup(@Nullable Container project, User adminUser, Group group)
     {
         // Impersonating the "Site: Guests" group leads to confusion and is not useful. Better to just log out. See #20140.
         if (group.isGuests())
             return false;
 
-        // Impersonating "Site: Administrators" or any other group assigned a privileged role by a non-site admin is confusing as well.
-        if (group.hasPrivilegedRole() && !user.hasSiteAdminPermission())
-            return false;
-
-        // Site/app admin can impersonate any other group
-        if (user.hasRootAdminPermission())
+        // Site admin, app admin, and impersonating troubleshooter can impersonate any group, even those assigned
+        // privileged roles. However, in the case where an app admin impersonates such a group, the privileged roles
+        // are filtered out (see GroupImpersonationContext.getAssignedRoles() below).
+        if (adminUser.hasRootPermission(ImpersonatePermission.class))
             return true;
 
         // Project admin...
-        if (null != project && project.hasPermission(user, AdminPermission.class))
+        if (null != project && project.hasPermission(adminUser, ImpersonatePermission.class))
         {
             // ...can impersonate any project group but must be a member of a site group to impersonate it
             if (group.isProjectGroup())
                 return group.getContainer().equals(project.getId());
             else
-                return user.isInGroup(group.getUserId());
+                return adminUser.isInGroup(group.getUserId());
         }
 
         return false;
@@ -160,11 +158,12 @@ public static void addMenu(NavTree menu)
         menu.addChild(groupMenu);
     }
 
+    // Returns the groups this user is allowed to impersonate. Empty for users who aren't allowed to impersonate.
     public static Collection<Group> getValidImpersonationGroups(Container c, User user)
     {
         LinkedList<Group> validGroups = new LinkedList<>();
-        List<Group> groups = SecurityManager.getGroups(c.getProject(), true);
         Container project = c.getProject();
+        List<Group> groups = SecurityManager.getGroups(project, true);
 
         // Site groups are always first, followed by project groups
         for (Group group : groups)
@@ -181,13 +180,13 @@ private static class GroupImpersonationContext extends AbstractImpersonationCont
 
         @JsonCreator
         protected GroupImpersonationContext(
-                @JsonProperty("_project") @Nullable Container project,
-                @JsonProperty("_adminUser") User adminUser,
-                @JsonProperty("_group") Group group,
-                @JsonProperty("_groups") PrincipalArray groups,
-                @JsonProperty("_returnUrl") ActionURL returnUrl,
-                @JsonProperty("_factory") ImpersonationContextFactory factory)
-
+            @JsonProperty("_project") @Nullable Container project,
+            @JsonProperty("_adminUser") User adminUser,
+            @JsonProperty("_group") Group group,
+            @JsonProperty("_groups") PrincipalArray groups,
+            @JsonProperty("_returnUrl") ActionURL returnUrl,
+            @JsonProperty("_factory") ImpersonationContextFactory factory
+        )
         {
             super(adminUser, project, returnUrl, factory);
             _group = group;