LabKey · labkey-adam · Jan 17, 2026
diff --git a/api/src/org/labkey/api/security/Group.java b/api/src/org/labkey/api/security/Group.java
@@ -111,7 +111,7 @@ public boolean isInGroup(int group)
     public Stream<Role> getAssignedRoles(SecurableResource resource)
     {
         SecurityPolicy policy = SecurityPolicyManager.getPolicy(resource);
-        return policy.getRoles(getGroups()).stream();
+        return policy.streamRoles(getGroups());
     }
 
     @Override

diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java
@@ -3376,7 +3376,7 @@ public PrincipalArray getGroups()
                 public Stream<Role> getAssignedRoles(SecurableResource resource)
                 {
                     SecurityPolicy policy = SecurityPolicyManager.getPolicy(resource);
-                    return policy.getRoles(getGroups()).stream();
+                    return policy.streamRoles(getGroups());
                 }
 
                 @Override

diff --git a/api/src/org/labkey/api/security/SecurityPolicy.java b/api/src/org/labkey/api/security/SecurityPolicy.java
@@ -38,15 +38,19 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.SortedSet;
+import java.util.Spliterators;
 import java.util.TreeSet;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
 
 /**
- * Represents a security policy for a {@link org.labkey.api.security.SecurableResource}. You can get a security policy for a resource
- * using SecurityManager.getPolicy(). Note that this class is immutable once constructed, so it may
- * be used by multiple threads at the same time. To make changes to an existing policy, construct a new
+ * Represents a security policy for a {@link org.labkey.api.security.SecurableResource}. You can get a security policy
+ * for a resource using SecurityManager.getPolicy(). Note that this class is immutable once constructed, so it may be
+ * used by multiple threads at the same time. To make changes to an existing policy, construct a new
  * {@link MutableSecurityPolicy} passing the existing SecurityPolicy instance in the constructor.
- * Note: intentionally does not implement HasPermission, use that interface for things that have a SecurityPolicy
+ * Note: intentionally does not implement HasPermission; use that interface for things that have a SecurityPolicy.
  */
 public class SecurityPolicy
 {
@@ -211,6 +215,12 @@ private Set<Class<? extends Permission>> getOwnPermissions(PrincipalArray princi
         Set<Class<? extends Permission>> permClasses = new HashSet<>();
         handleRoles(principalArray, role -> permClasses.addAll(role.getPermissions()));
 
+        Set<Class<? extends Permission>> permClasses2 = streamRoles(principalArray)
+            .flatMap(role -> role.getPermissions().stream())
+            .collect(Collectors.toSet());
+
+        assert permClasses2.equals(permClasses);
+
         return permClasses;
     }
 
@@ -221,13 +231,18 @@ public Set<Role> getRoles(PrincipalArray principalArray)
         Set<Role> roles = new HashSet<>();
         handleRoles(principalArray, roles::add);
 
+        Set<Role> roles2 = streamRoles(principalArray).collect(Collectors.toSet());
+
+        assert roles.equals(roles2);
+
         return roles;
     }
 
     /* Does not inspect any contextual roles, just the roles explicitly given by this SecurityPolicy */
     public boolean hasRole(UserPrincipal principal, Class<? extends Role> roleClass)
     {
-        return getRoles(principal.getGroups()).contains(RoleManager.getRole(roleClass));
+        return streamRoles(principal.getGroups())
+            .anyMatch(r -> r.equals(RoleManager.getRole(roleClass)));
     }
 
     private void handleRoles(PrincipalArray principalArray, Consumer<Role> consumer)
@@ -259,6 +274,70 @@ else if (assignment.getUserId() < principalId)
         }
     }
 
+    /* Does not return any contextual roles, just the roles explicitly granted by this SecurityPolicy */
+    @NotNull
+    public Stream<Role> streamRoles(PrincipalArray principalArray)
+    {
+        return StreamSupport.stream(
+            Spliterators.spliteratorUnknownSize(
+                new RoleIterator(principalArray, getAssignments()),
+                0
+            ),
+            false
+        );
+    }
+
+    private static class RoleIterator implements Iterator<Role>
+    {
+        private final Iterator<RoleAssignment> _assignmentIterator;
+        private final List<Integer> _principals;
+
+        private RoleAssignment _assignment;
+        private int _principalsIdx = 0;
+        private Role _nextRole = null;
+
+        public RoleIterator(PrincipalArray principals, SortedSet<RoleAssignment> roleAssignments)
+        {
+            _assignmentIterator = roleAssignments.iterator();
+            _principals = principals.getList();
+            _assignment = _assignmentIterator.hasNext() ? _assignmentIterator.next() : null;
+        }
+
+        private @Nullable Role getNextRole()
+        {
+            while (null != _assignment && _principalsIdx < _principals.size())
+            {
+                int principalId = _principals.get(_principalsIdx);
+                if (_assignment.getUserId() == principalId)
+                {
+                    Role role = _assignment.getRole();
+                    _assignment = _assignmentIterator.hasNext() ? _assignmentIterator.next() : null;
+                    if (null != role)
+                        return role;
+                }
+                else if (_assignment.getUserId() < principalId)
+                    _assignment = _assignmentIterator.hasNext() ? _assignmentIterator.next() : null;
+                else
+                    ++_principalsIdx;
+            }
+
+            return null;
+        }
+
+        @Override
+        public boolean hasNext()
+        {
+            _nextRole = getNextRole();
+            return _nextRole != null;
+        }
+
+        @Override
+        public Role next()
+        {
+            return _nextRole;
+        }
+    }
+
     @Nullable
     public Date getModified()
     {
@@ -345,12 +424,18 @@ public Map<String, Map<PrincipalType, List<UserPrincipal>>> getAssignmentsAsMap(
 
     public boolean hasNonInheritedPermission(@NotNull UserPrincipal principal, Class<? extends Permission> perm)
     {
+        boolean ret = streamRoles(new PrincipalArray(List.of(principal.getUserId()))).anyMatch(role -> role.getPermissions().contains(perm));
+
         for (Role role : getRoles(new PrincipalArray(List.of(principal.getUserId()))))
         {
             if (role.getPermissions().contains(perm))
+            {
+                assert ret;
                 return true;
+            }
         }
 
+        assert !ret;
         return false;
     }
 }
diff --git a/api/src/org/labkey/api/security/User.java b/api/src/org/labkey/api/security/User.java
@@ -362,6 +362,11 @@ public final Stream<Role> getSiteRoles()
         return _impersonationContext.getSiteRoles(this);
     }
 
+    public final Stream<Role> getSiteRoles(SecurableResource resource)
+    {
+        return _impersonationContext.getSiteRoles(this, resource);
+    }
+
     @Override
     public Object clone() throws CloneNotSupportedException
     {

diff --git a/api/src/org/labkey/api/security/UserPrincipal.java b/api/src/org/labkey/api/security/UserPrincipal.java
@@ -146,7 +146,8 @@ public JdbcType getJdbcParameterType()
     public boolean hasPrivilegedRole()
     {
         // Check for any privileged role assigned to this principal at the root
-        return ContainerManager.getRoot().getPolicy().getRoles(getGroups()).stream().anyMatch(Role::isPrivileged);
+        return ContainerManager.getRoot().getPolicy().streamRoles(getGroups())
+            .anyMatch(Role::isPrivileged);
     }
 
     @Override

diff --git a/api/src/org/labkey/api/security/impersonation/ImpersonationContext.java b/api/src/org/labkey/api/security/impersonation/ImpersonationContext.java
@@ -25,6 +25,7 @@
 import org.labkey.api.security.SecurityPolicyManager;
 import org.labkey.api.security.User;
 import org.labkey.api.security.permissions.Permission;
+import org.labkey.api.security.roles.AbstractRootContainerRole;
 import org.labkey.api.security.roles.NoPermissionsRole;
 import org.labkey.api.security.roles.Role;
 import org.labkey.api.security.roles.RoleManager;
@@ -58,15 +59,16 @@ public interface ImpersonationContext extends Serializable
      */
     default Stream<Role> getAssignedRoles(User user, SecurableResource resource)
     {
-        Stream<Role> roles = getSiteRoles(user);
+        Stream<Role> roles = getSiteRoles(user, resource);
         SecurityPolicy policy = SecurityPolicyManager.getPolicy(resource);
-        return Streams.concat(roles, policy.getRoles(user.getGroups()).stream()).distinct();
+        return Streams.concat(roles, policy.streamRoles(user.getGroups())).distinct();
     }
 
     /**
      * @return The roles assigned to this user in the root. The roles may be modified and/or filtered by the
      * impersonation context.
      */
+    @Deprecated
     default Stream<Role> getSiteRoles(User user)
     {
         Container root = ContainerManager.getRoot();
@@ -79,9 +81,29 @@ default Stream<Role> getSiteRoles(User user)
         return roles.stream();
     }
 
+    /**
+     * @return The roles assigned to this user in the root that are applicable to the passed in resource. The roles may
+     * be modified and/or filtered by the impersonation context.
+     */
+    default Stream<Role> getSiteRoles(User user, SecurableResource resource)
+    {
+        Container root = ContainerManager.getRoot();
+        SecurityPolicy policy = root.getPolicy();
+        return policy.streamRoles(getGroups(user))
+            .filter(role -> !role.equals(RoleManager.getRole(NoPermissionsRole.class)))
+            .filter(role -> {
+                if (!role.isApplicable(policy, root))
+                    throw new IllegalStateException("Root role " + role.getName() + " is not applicable");
+                if (!(role instanceof AbstractRootContainerRole siteRole))
+                    throw new IllegalStateException("Root roles should all be AbstractRootContainerRole");
+
+                return siteRole.isAvailableEverywhere() || resource.equals(ContainerManager.getRoot());
+            });
+    }
+
     ImpersonationContextFactory getFactory();
 
-    /** Responsible for adding menu items to allow the user to initiate or stop impersonating, based on the current state */
+    /** Responsible for adding menu items to allow the user to initiate, adjust, or stop impersonating, based on the current state */
     void addMenu(NavTree menu, Container c, User user, ActionURL currentURL);
 
     // restrict the permissions this user is allowed

diff --git a/api/src/org/labkey/api/security/roles/AbstractRootContainerRole.java b/api/src/org/labkey/api/security/roles/AbstractRootContainerRole.java
@@ -60,4 +60,9 @@ public boolean isApplicable(SecurityPolicy policy, SecurableResource resource)
     {
         return resource instanceof Container && ((Container)resource).isRoot();
     }
+
+    public boolean isAvailableEverywhere()
+    {
+        return true;
+    }
 }
diff --git a/api/src/org/labkey/api/security/roles/TroubleshooterRole.java b/api/src/org/labkey/api/security/roles/TroubleshooterRole.java
@@ -17,6 +17,7 @@
 
 import org.labkey.api.audit.permissions.CanSeeAuditLogPermission;
 import org.labkey.api.security.permissions.Permission;
+import org.labkey.api.security.permissions.ReadPermission;
 import org.labkey.api.security.permissions.SeeUserDetailsPermission;
 import org.labkey.api.security.permissions.TroubleshooterPermission;
 
@@ -28,11 +29,18 @@ public class TroubleshooterRole extends AbstractRootContainerRole
     static Collection<Class<? extends Permission>> PERMISSIONS = Set.of(
         TroubleshooterPermission.class,
         SeeUserDetailsPermission.class,
-        CanSeeAuditLogPermission.class
+        CanSeeAuditLogPermission.class,
+        ReadPermission.class
     );
 
     public TroubleshooterRole()
     {
         super("Troubleshooter", "Troubleshooters may view administration settings but may not change them.", PERMISSIONS);
     }
+
+    @Override
+    public boolean isAvailableEverywhere()
+    {
+        return false; // This ensures troubleshooters get these permissions (esp. ReadPermission) only in the root
+    }
 }
diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java
@@ -2640,7 +2640,8 @@ public void addNavTrail(NavTree root)
         }
     }
 
-    @AdminConsoleAction
+    // This allows Troubleshooters to GET and POST to the action, supporting export to Excel and script, e.g.
+    @RequiresPermission(TroubleshooterPermission.class)
     public class PostgresStatActivityAction extends AbstractPostgresAction
     {
         public PostgresStatActivityAction()
@@ -2649,7 +2650,7 @@ public PostgresStatActivityAction()
         }
     }
 
-    @AdminConsoleAction
+    @RequiresPermission(TroubleshooterPermission.class)
     public class PostgresLocksAction extends AbstractPostgresAction
     {
         public PostgresLocksAction()
@@ -2658,7 +2659,7 @@ public PostgresLocksAction()
         }
     }
 
-    @AdminConsoleAction
+    @RequiresPermission(TroubleshooterPermission.class)
     public class PostgresTableSizesAction extends AbstractPostgresAction
     {
         public PostgresTableSizesAction()
@@ -3579,16 +3580,6 @@ protected AbstractAdminQueryAction(String schemaName, String queryName)
             _queryName = queryName;
         }
 
-        @Override
-        public void setViewContext(ViewContext context)
-        {
-            // Troubleshooters don't have read permissions but DataRegion requires it. I don't love poking an elevated
-            // user into the ViewContext, but this is the only way I could get DataRegion to see read permission on
-            // tables that are wrapped by a query (e.g., core.Documents used by DocumentsGroupedByParentType.sql).
-            context.setUser(ElevatedUser.getElevatedUser(context.getUser(), ReaderRole.class));
-            super.setViewContext(context);
-        }
-
         @Override
         protected QueryView createQueryView(QueryExportForm form, BindException errors, boolean forExport, @Nullable String dataRegion) throws Exception
         {
@@ -3609,7 +3600,8 @@ protected String getQueryName()
         abstract protected UserSchema getUserSchema();
     }
 
-    @AdminConsoleAction
+    // This allows Troubleshooters to GET and POST to the action, supporting export to Excel and script, e.g.
+    @RequiresPermission(TroubleshooterPermission.class)
     public class AttachmentsAction extends AbstractAdminQueryAction
     {
         @SuppressWarnings("unused") // Invoked via reflection
@@ -3632,7 +3624,7 @@ public void addNavTrail(NavTree root)
     }
 
     @SuppressWarnings("unused") // Linked from core.DocumentsGroupedByParentTypeAdmin
-    @AdminConsoleAction
+    @RequiresPermission(TroubleshooterPermission.class)
     public class AttachmentsForTypeAction extends AbstractAdminQueryAction
     {
         @SuppressWarnings("unused") // Invoked via reflection

diff --git a/core/src/org/labkey/core/query/CoreQuerySchema.java b/core/src/org/labkey/core/query/CoreQuerySchema.java
@@ -142,8 +142,7 @@ public Set<String> getTableNames()
             CONTAINERS_TABLE_NAME, WORKBOOKS_TABLE_NAME, QCSTATE_TABLE_NAME, DATA_STATES_TABLE_NAME,
             VIEW_CATEGORY_TABLE_NAME, MISSING_VALUE_INDICATOR_TABLE_NAME);
 
-        // Don't show troubleshooters the query in the schema browser since query-execute.view requires read permissions
-        if (getUser().hasRootPermission(ApplicationAdminPermission.class))
+        if (getUser().hasRootPermission(TroubleshooterPermission.class))
             names.add(DOCUMENTS_TABLE_NAME);
 
         if (getUser().hasRootPermission(UserManagementPermission.class))
@@ -204,7 +203,6 @@ public TableInfo createTable(String name, ContainerFilter cf)
             return getMVIndicatorTable(cf);
         if (SHORT_URL_TABLE_NAME.equalsIgnoreCase(name) && ShortUrlTableInfo.canDisplayTable(getUser(), getContainer()))
             return new ShortUrlTableInfo(this);
-        // Allow troubleshooters to view this query from the admin console
         if (DOCUMENTS_TABLE_NAME.equalsIgnoreCase(name) && getUser().hasRootPermission(TroubleshooterPermission.class))
             return new DocumentsTable(this, cf);
 

diff --git a/core/src/org/labkey/core/security/SecurityController.java b/core/src/org/labkey/core/security/SecurityController.java
@@ -77,7 +77,6 @@
 import org.labkey.api.security.LoginManager;
 import org.labkey.api.security.MemberType;
 import org.labkey.api.security.MutableSecurityPolicy;
-import org.labkey.api.security.PrincipalArray;
 import org.labkey.api.security.RequiresLogin;
 import org.labkey.api.security.RequiresNoPermission;
 import org.labkey.api.security.RequiresPermission;
@@ -721,14 +720,6 @@ private ModelAndView renderGroup(Group group, BindException errors, List<HtmlStr
         return view;
     }
 
-    private void verifySystemGroupIsAssignedRole(Group group, Role role, BindException errors)
-    {
-        Set<Role> roles = ContainerManager.getRoot().getPolicy().getRoles(new PrincipalArray(List.of(group.getUserId())));
-        if (!roles.contains(role))
-            errors.reject(ERROR_MSG, "Warning: This group is not assigned its standard role, "
-                + role.getDisplayName() + "! Consider assigning it on the Site Permissions page.");
-    }
-
     @RequiresPermission(AdminPermission.class)
     public class GroupAction extends FormViewAction<UpdateMembersForm>
     {