diff --git a/api/src/org/labkey/api/data/NestedRenderContext.java b/api/src/org/labkey/api/data/NestedRenderContext.java index 85c5ff6a9b1..40da20f1945 100644 --- a/api/src/org/labkey/api/data/NestedRenderContext.java +++ b/api/src/org/labkey/api/data/NestedRenderContext.java @@ -131,7 +131,7 @@ public SimpleFilter buildFilter(TableInfo tinfo, List displayColumns fullSQL.append(" ) Limited )"); // Apply a filter that restricts the group ids to the right "page" of data - result.addClause(new SimpleFilter.SQLClause(fullSQL.getSQL(), fullSQL.getParamsArray())); + result.addClause(new SimpleFilter.SQLClause(fullSQL)); } return result; } diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index 2c262ebf8c7..84059487c84 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -55,7 +55,6 @@ import org.labkey.api.data.PropertyManager.WritablePropertyMap; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; -import org.labkey.api.data.Selector; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.SqlSelector; @@ -1883,126 +1882,6 @@ public static String getMembershipPathwayHTMLDisplay(Set> pa sb.append("
"); } return sb.toString(); - } - - // TODO: Redundant with getProjectUsers() -- this approach should be more efficient for simple cases - // TODO: Also redundant with getFolderUserids() - // TODO: Cache this set - public static Set getProjectUsersIds(Container c) - { - SQLFragment sql = getProjectUsersSQL(c.getProject()); - sql.insert(0, "SELECT DISTINCT members.UserId "); - - Selector selector = new SqlSelector(core.getSchema(), sql); - return new HashSet<>(selector.getCollection(Integer.class)); - } - - // True fragment -- need to prepend SELECT DISTINCT() or IN () for this to be valid SQL - public static SQLFragment getProjectUsersSQL(Container c) - { - return new SQLFragment("FROM " + core.getTableInfoMembers() + " members INNER JOIN " + core.getTableInfoUsers() + " users ON members.UserId = users.UserId\n" + - "INNER JOIN " + core.getTableInfoPrincipals() + " groups ON members.GroupId = groups.UserId\n" + - "WHERE (groups.Container = ?)", c); - } - - public static @NotNull List getProjectUsers(Container c) - { - return getProjectUsers(c, false, true); - } - - public static @NotNull List getProjectUsers(Container c, boolean includeGlobal, boolean includeInactive) - { - if (c != null && !c.isProject()) - c = c.getProject(); - - List groups = getGroups(c, includeGlobal); - Set emails = new HashSet<>(); - - //get members for each group - ArrayList projectUsers = new ArrayList<>(); - Set members; - - for (Group g : groups) - { - if (g.isGuests() || g.isUsers()) - continue; - - // TODO: currently only getting members that are users (no groups). should this be changed to get users of member groups? - members = getGroupMembers(g, includeInactive ? MemberType.ACTIVE_AND_INACTIVE_USERS : MemberType.ACTIVE_USERS); - - //add this group's members to hashset - if (!members.isEmpty()) - { - //get list of users from email - for (UserPrincipal member : members) - { - User user = UserManager.getUser(member.getUserId()); - if (null != user && emails.add(user.getEmail())) - projectUsers.add(user); - } - } - } - - return projectUsers; - } - - public static Collection getFolderUserids(Container c) - { - Container project = (c.isProject() || c.isRoot()) ? c : c.getProject(); - SecurityPolicy policy = c.getPolicy(); - - //don't filter if all site users is playing a role - Group allSiteUsers = getGroup(Group.groupUsers); - if (policy.getAssignedRoles(allSiteUsers).findAny().isPresent()) - { - // Just select all users - SQLFragment sql = new SQLFragment("SELECT u.UserId FROM "); - sql.append(core.getTableInfoPrincipals(), "u"); - sql.append(" WHERE u.type='u'"); - - return new SqlSelector(core.getSchema(), sql).getCollection(Integer.class); - } - - //users "in the project" consists of: - // - users who are members of a project group - // - users who belong to a site group that has a role assignment in the policy for the specified folder - // - users who have a direct role assignment in the policy for the specified folder - - Set userIds = new HashSet<>(); - - // Add all project groups - Set groupsToExpand = new HashSet<>(getGroups(project, false)); - - // Look for users and site groups that have direct assignment to the container - for (RoleAssignment roleAssignment : c.getPolicy().getAssignments()) - { - User user = UserManager.getUser(roleAssignment.getUserId()); - if (user != null) - { - userIds.add(user.getUserId()); - } - else - { - Group assignedGroup = getGroup(roleAssignment.getUserId()); - if (assignedGroup != null && !assignedGroup.isProjectGroup()) - { - // Add all site groups - groupsToExpand.add(assignedGroup); - } - } - } - - // Find the users who are members of all the relevant site groups - for (Group group : groupsToExpand) - { - Set groupMembers = getAllGroupMembers(group, MemberType.ACTIVE_AND_INACTIVE_USERS); - for (User groupMember : groupMembers) - { - userIds.add(groupMember.getUserId()); - } - } - - return userIds; } /** diff --git a/core/src/org/labkey/core/query/CoreQuerySchema.java b/core/src/org/labkey/core/query/CoreQuerySchema.java index 74164a19b6d..6fbd9d9fd8c 100644 --- a/core/src/org/labkey/core/query/CoreQuerySchema.java +++ b/core/src/org/labkey/core/query/CoreQuerySchema.java @@ -62,6 +62,7 @@ import org.labkey.api.security.UserPrincipal; import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.ApplicationAdminPermission; +import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.SeeGroupDetailsPermission; import org.labkey.api.security.permissions.SeeUserDetailsPermission; import org.labkey.api.security.permissions.TroubleshooterPermission; @@ -76,14 +77,13 @@ import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; public class CoreQuerySchema extends UserSchema { - private Set _projectUserIds; + private Set _folderUserIds; private final boolean _mustCheckPermissions; public static final String NAME = "core"; @@ -483,17 +483,15 @@ public TableInfo getUsers() } else { - if (_projectUserIds == null) + // All users with read permissions in this folder + if (_folderUserIds == null) { - Set projectUserIds = new HashSet<>(SecurityManager.getFolderUserids(getContainer())); - // Add app admins and site admins (they both have ApplicationAdminPermission) - SecurityManager.getUsersWithPermissions(ContainerManager.getRoot(), Set.of(ApplicationAdminPermission.class)).stream() - .map(User::getUserId) - .forEach(projectUserIds::add); - _projectUserIds = projectUserIds; + _folderUserIds = SecurityManager.getUsersWithPermissions(getContainer(), Set.of(ReadPermission.class)).stream() + .map(UserPrincipal::getUserId) + .collect(Collectors.toSet()); } ColumnInfo userid = users.getRealTable().getColumn("userid"); - users.addInClause(userid, _projectUserIds); + users.addInClause(userid, _folderUserIds); addGroupsColumn(users); addAvatarColumn(users); diff --git a/core/src/org/labkey/core/query/UsersTable.java b/core/src/org/labkey/core/query/UsersTable.java index fabd86cbd3e..c4e130cbb17 100644 --- a/core/src/org/labkey/core/query/UsersTable.java +++ b/core/src/org/labkey/core/query/UsersTable.java @@ -28,10 +28,9 @@ import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.NullColumnInfo; -import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.SimpleFilter.InClause; import org.labkey.api.data.TableInfo; -import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.exp.PropertyColumn; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.property.Domain; @@ -92,10 +91,6 @@ import static org.labkey.api.util.IntegerUtils.asInteger; -/** -* User: klum -* Date: 9/19/12 -*/ public class UsersTable extends SimpleUserSchema.SimpleTable { private Set _illegalColumns; @@ -116,7 +111,7 @@ public UsersTable(UserSchema schema, TableInfo table) { super(schema, table, null); - setDescription("Contains all users who are members of the current project." + + setDescription("Contains all users with read permissions in the current project." + " The data in this table are available only to users who are signed-in (not guests). Guests see no rows." + " Signed-in users see the columns UserId, EntityId, and DisplayName." + " Users granted the '" + SeeUserAndGroupDetailsRole.NAME + "' role see all standard and custom columns."); @@ -414,6 +409,7 @@ public boolean hasPermission(@NotNull UserPrincipal user, @NotNull Class columnMap, SqlDialect dialect) - { - ColumnInfo col = columnMap.get(userIdColumnFieldKey); - - // NOTE: Ideally we would use col.getValueSql() here instead - SQLFragment sql = new SQLFragment(); - - if (col != null) - sql.appendIdentifier(col.getAlias()); - else - sql.append(userIdColumnFieldKey); - sql.append(" IN (SELECT members.UserId "); - sql.append(super.toSQLFragment(columnMap, dialect)); - sql.append(")"); - return sql; - } - }); + // Consider: could short-circuit optimize if guests or all site users have read permissions + InClause clause = new InClause(userIdColumnFieldKey, SecurityManager.getUsersWithPermissions(c, Set.of(ReadPermission.class))); + filter.addClause(clause); } return filter; } diff --git a/core/src/org/labkey/core/security/SecurityApiActions.java b/core/src/org/labkey/core/security/SecurityApiActions.java index c0ecb706f29..cfd5aaa03b0 100644 --- a/core/src/org/labkey/core/security/SecurityApiActions.java +++ b/core/src/org/labkey/core/security/SecurityApiActions.java @@ -1578,8 +1578,7 @@ public ApiResponse execute(IdForm form, BindException errors) throws Exception Container c = getContainer(); if (!c.isRoot()) { - List projectUsers = SecurityManager.getProjectUsers(c); - if (!projectUsers.contains(user)) + if (!c.hasPermission(user, ReadPermission.class)) throw new IllegalArgumentException("User id " + form.getId() + " does not exist in the folder: " + c.getPath()); } diff --git a/core/src/org/labkey/core/user/UserController.java b/core/src/org/labkey/core/user/UserController.java index d1985e4215d..41e08919390 100644 --- a/core/src/org/labkey/core/user/UserController.java +++ b/core/src/org/labkey/core/user/UserController.java @@ -843,7 +843,7 @@ public void addNavTrail(NavTree root) // Site admins and Application admins can act on any user // Project admins can only act on users who are project users - private void authorizeUserAction(Integer targetUserId, String action, boolean allowFolderAdmins) throws UnauthorizedException + private void authorizeUserAction(User targetUser, String action, boolean allowFolderAdmins) throws UnauthorizedException { User user = getUser(); @@ -863,8 +863,8 @@ private void authorizeUserAction(Integer targetUserId, String action, boolean al if (!allowFolderAdmins) requiresProjectAdminOrBetter(); - // ...and user must be a project user - if (!SecurityManager.getProjectUsersIds(c.getProject()).contains(targetUserId)) + // ...and user have read permission in this folder + if (!c.hasPermission(user, ReadPermission.class)) throw new UnauthorizedException("You can only " + action + " project users"); } } @@ -1584,7 +1584,7 @@ public ModelAndView getView(UserQueryForm form, BindException errors) // Anyone can view their own record; otherwise, make sure current user can view the details of this user if (!isOwnRecord) - authorizeUserAction(_detailsUserId, "view details of", true); + authorizeUserAction(detailsUser, "view details of", true); Container c = getContainer(); ActionURL currentUrl = getViewContext().getActionURL(); @@ -2554,27 +2554,17 @@ public void setIncludeInactive(boolean includeInactive) /** - * Collects a set of users either from a particular group or from any of the project groups of the current container. - * Optionally filters for those users who have a given set of permissions. Can also include deactivated users (though if + * Collects a set of users either from a particular group or from all users in the system. Optionally filters for + * those users who have a given set of permissions. Can also include deactivated users (though if * checking for permissions, no deactivated users will be included). - * - * N.B. Users that have permissions within the current project but are not part of any project group WILL NOT be returned unless - * the user is in one of the global groups (such as SiteAdmins) and you set allMembers=true. In other words, this is probably - * not the API you're looking for. Consider using GetUsersWithPermissions instead. */ - @RequiresLogin - @RequiresPermission(ReadPermission.class) + @RequiresPermission(AdminPermission.class) public static class GetUsersAction extends ReadOnlyApiAction { @Override public ApiResponse execute(GetUsersForm form, BindException errors) { Container container = getContainer(); - User currentUser = getUser(); - - if (container.isRoot() && !currentUser.hasRootPermission(UserManagementPermission.class)) - throw new UnauthorizedException("Only site/application administrators may see users in the root container!"); - ApiSimpleResponse response = new ApiSimpleResponse(); response.put("container", container.getPath()); @@ -2583,24 +2573,18 @@ public ApiResponse execute(GetUsersForm form, BindException errors) //if requesting users in a specific group... if (null != StringUtils.trimToNull(form.getGroup()) || null != form.getGroupId()) { - users = getProjectGroupUsers(form, response, !form.getActive()); + users = getGroupUsers(form, response, !form.getActive()); } else { - //special-case: if container is root, return all active users - //else, return all users in the current project - //we've already checked above that the current user is a system admin - if (container.isRoot()) - users = UserManager.getUsers(!form.getActive()); - else - users = SecurityManager.getProjectUsers(container, form.isAllMembers(), !form.getActive()); + users = UserManager.getUsers(!form.getActive()); } - this.setUsersList(form, filterForPermissions(form, users), response); + setUsersList(form, filterForPermissions(form, users), response); return response; } - // Filter the collection of users to those that have all of the permissions provided in the form. + // Filter the collection of users to those that have all the permissions provided in the form. // If no permissions are provided, no filtering will occur. protected Set filterForPermissions(GetUsersForm form, Collection users) { @@ -2620,7 +2604,7 @@ protected Set filterForPermissions(GetUsersForm form, Collection use } @NotNull - protected Collection getProjectGroupUsers(GetUsersForm form, ApiSimpleResponse response, boolean includeInactive) + protected Collection getGroupUsers(GetUsersForm form, ApiSimpleResponse response, boolean includeInactive) { Container project = getContainer().getProject(); @@ -2737,11 +2721,10 @@ public ApiResponse execute(GetUsersForm form, BindException errors) if (null != StringUtils.trimToNull(form.getGroup()) || null != form.getGroupId()) { - users = filterForPermissions(form, getProjectGroupUsers(form, response, false)); + users = filterForPermissions(form, getGroupUsers(form, response, false)); } else { - // Active users only users = SecurityManager.getUsersWithPermissions(getContainer(), form.getPermissionClasses()); } @@ -3179,23 +3162,23 @@ public void testActionPermissions() UserController controller = new UserController(); assertForNoPermission(user, - new GetImpersonationUsersAction(), - new ImpersonateUserAction(), new GetImpersonationGroupsAction(), - new ImpersonateGroupAction(), new GetImpersonationRolesAction(), - new ImpersonateRolesAction() + new GetImpersonationUsersAction(), + new ImpersonateGroupAction(), + new ImpersonateRolesAction(), + new ImpersonateUserAction() ); // @RequiresPermission(ReadPermission.class) assertForReadPermission(user, false, new BeginAction(), - new GetUsersAction(), new GetUsersWithPermissionsAction() ); // @RequiresPermission(AdminPermission.class) assertForAdminPermission(user, + new GetUsersAction(), controller.new ShowUsersAction() //TODO controller.new ShowUserHistoryAction(), //TODO controller.new UserAccessAction(), diff --git a/issues/src/org/labkey/issue/IssuesController.java b/issues/src/org/labkey/issue/IssuesController.java index 97d59cbcf86..b35a7ec7ac7 100644 --- a/issues/src/org/labkey/issue/IssuesController.java +++ b/issues/src/org/labkey/issue/IssuesController.java @@ -93,7 +93,6 @@ import org.labkey.api.search.SearchService; import org.labkey.api.search.SearchUrls; import org.labkey.api.security.Group; -import org.labkey.api.security.MemberType; import org.labkey.api.security.RequiresLogin; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.SecurityManager; @@ -102,7 +101,6 @@ import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.ReadPermission; -import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.security.roles.OwnerRole; import org.labkey.api.security.roles.RoleManager; import org.labkey.api.util.ButtonBuilder; @@ -1739,43 +1737,19 @@ public static class GetUsersForGroupAction extends ReadOnlyApiAction users = new ArrayList<>(); - - if (null != form.getGroupId()) - { - Group group = SecurityManager.getGroup(form.getGroupId()); - if (group != null) - { - for (User user : SecurityManager.getAllGroupMembers(group, MemberType.ACTIVE_USERS, group.isUsers())) - { - if (getContainer().hasPermission(user, UpdatePermission.class)) - { - UserGroupForm usergrp = new UserGroupForm(); - usergrp.setGroupId(form.groupId); - usergrp.setUserId(user.getUserId()); - usergrp.setDisplayName(user.getDisplayName(getUser())); - users.add(usergrp); - } - } - } - } - else - { - // all project users - for (User user : SecurityManager.getProjectUsers(getContainer())) - { - if (getContainer().hasPermission(user, UpdatePermission.class)) - { - UserGroupForm projectUsers = new UserGroupForm(); - projectUsers.setUserId(user.getUserId()); - projectUsers.setDisplayName(user.getDisplayName(getUser())); - users.add(projectUsers); - } - } - } - - users.sort(Comparator.comparing(UserGroupForm::getDisplayName, String.CASE_INSENSITIVE_ORDER)); - return users; + Integer groupId = form.getGroupId(); + Group group = null != groupId ? SecurityManager.getGroup(groupId) : null; + return IssueManager.getUsersForGroup(getContainer(), group) + .map(user -> { + UserGroupForm userGroupForm = new UserGroupForm(); + if (groupId != null) + userGroupForm.setGroupId(groupId); + userGroupForm.setUserId(user.getUserId()); + userGroupForm.setDisplayName(user.getDisplayName(getUser())); + return userGroupForm; + }) + .sorted(Comparator.comparing(UserGroupForm::getDisplayName, String.CASE_INSENSITIVE_ORDER)) + .collect(Collectors.toList()); } } diff --git a/issues/src/org/labkey/issue/model/IssueManager.java b/issues/src/org/labkey/issue/model/IssueManager.java index bb7143b2e4d..087da9c9a22 100644 --- a/issues/src/org/labkey/issue/model/IssueManager.java +++ b/issues/src/org/labkey/issue/model/IssueManager.java @@ -72,6 +72,7 @@ import org.labkey.api.security.UserManager; import org.labkey.api.security.ValidEmail; import org.labkey.api.security.permissions.AdminPermission; +import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.security.roles.ReaderRole; @@ -121,10 +122,12 @@ import java.util.Set; import java.util.TreeSet; import java.util.function.Consumer; +import java.util.stream.Collectors; +import java.util.stream.Stream; -import static org.labkey.api.util.IntegerUtils.asInteger; import static org.labkey.api.search.SearchService.PROPERTY.categories; import static org.labkey.api.security.UserManager.USER_DISPLAY_NAME_COMPARATOR; +import static org.labkey.api.util.IntegerUtils.asInteger; public class IssueManager { @@ -324,8 +327,7 @@ public static List getCommentsForRelatedIssues(IssueO } /** - * Determine if the parameter issue has related issues. Returns true if the issue has related - * issues and false otherwise. + * Determine if the parameter issue has related issues. Returns true if the issue has related issues and false otherwise. * * @param issue The issue to query * @return boolean return value @@ -472,6 +474,12 @@ public static Collection> getSummary(Container c, User user, return Collections.emptyList(); } + private static final Class ASSIGNED_TO_PERMISSION = UpdatePermission.class; + + private static boolean canAssignTo(Container c, User u) + { + return c.hasPermission(u, ASSIGNED_TO_PERMISSION); + } public static @NotNull Collection getAssignedToList(Container c, @Nullable String issueDefName, @Nullable IssueObject issue) { @@ -494,7 +502,6 @@ public static Collection> getSummary(Container c, User user, return initialAssignedTo; } - private static final BlockingCache> ASSIGNED_TO_CACHE = DatabaseCache.get(IssuesSchema.getInstance().getSchema().getScope(), 1000, "Issues assigned-to lists", (key, argument) -> { assert argument != null; @@ -504,62 +511,50 @@ public static Collection> getSummary(Container c, User user, Group group = getAssignedToGroup(c, issueDefName); - if (null != group) - return createAssignedToList(c, SecurityManager.getAllGroupMembers(group, MemberType.ACTIVE_USERS, true)); - else - return createAssignedToList(c, SecurityManager.getProjectUsers(c.getProject())); + // Stream active users from the selected group or from all users. Set is filtered for update permissions in this container. + // Then collect users into an unmodifiable TreeSet with our special comparator. + return getUsersForGroup(c, group) + .collect(Collectors.collectingAndThen( + Collectors.toCollection(() -> new TreeSet<>(USER_DISPLAY_NAME_COMPARATOR)), + Collections::unmodifiableSet + )); }); - // Returns the assigned to list that is used for every new issue in this container. We can cache it and share it - // across requests. The collection is unmodifiable. - private static @NotNull Collection getInitialAssignedToList(final Container c, @Nullable String issueDefName) - { - issueDefName = issueDefName != null ? issueDefName : IssueListDef.DEFAULT_ISSUE_LIST_NAME; - String cacheKey = getCacheKey(c, issueDefName); - - return ASSIGNED_TO_CACHE.get(cacheKey, Pair.of(c, issueDefName)); - } - - - public static String getCacheKey(@Nullable Container c, String issueDefName) + public static Stream getUsersForGroup(Container c, @Nullable Group group) { - String key = "AssignedTo-" + issueDefName; - return null != c ? key + c.getId() : key; + return null != group ? + SecurityManager.getAllGroupMembers(group, MemberType.ACTIVE_USERS, true).stream().filter(u -> c.hasPermission(u, ASSIGNED_TO_PERMISSION)) : + SecurityManager.getUsersWithPermissions(c, Set.of(ASSIGNED_TO_PERMISSION)).stream(); } - - private static Set createAssignedToList(Container c, Collection candidates) - { - Set assignedTo = new TreeSet<>(USER_DISPLAY_NAME_COMPARATOR); - - for (User candidate : candidates) - if (canAssignTo(c, candidate)) - assignedTo.add(candidate); - - // Cache an unmodifiable version - return Collections.unmodifiableSet(assignedTo); - } - - - private static boolean canAssignTo(Container c, @NotNull User user) - { - return user.isActive() && c.hasPermission(user, UpdatePermission.class); - } - - static @Nullable Integer validateAssignedTo(Container c, Integer candidate) { if (null != candidate) { User user = UserManager.getUser(candidate); - if (null != user && canAssignTo(c, user)) + if (null != user && c.hasPermission(user, ASSIGNED_TO_PERMISSION)) return candidate; } return null; } + // Returns the assigned to list that is used for every new issue in this container. We can cache it and share it + // across requests. The collection is unmodifiable. + private static @NotNull Collection getInitialAssignedToList(final Container c, @Nullable String issueDefName) + { + issueDefName = issueDefName != null ? issueDefName : IssueListDef.DEFAULT_ISSUE_LIST_NAME; + String cacheKey = getCacheKey(c, issueDefName); + + return ASSIGNED_TO_CACHE.get(cacheKey, Pair.of(c, issueDefName)); + } + + public static String getCacheKey(@Nullable Container c, String issueDefName) + { + String key = "AssignedTo-" + issueDefName; + return null != c ? key + c.getId() : key; + } public static int getUserEmailPreferences(Container c, Integer userId) { diff --git a/mothership/src/org/labkey/mothership/MothershipController.java b/mothership/src/org/labkey/mothership/MothershipController.java index c5d3a0c4c2f..993d53d1db5 100644 --- a/mothership/src/org/labkey/mothership/MothershipController.java +++ b/mothership/src/org/labkey/mothership/MothershipController.java @@ -1471,7 +1471,7 @@ public boolean handlePost(BulkUpdateForm form, BindException errors) if (form.getUserId() != null) { user = UserManager.getUser(form.getUserId()); - if (!MothershipManager.get().getAssignedToList(getContainer()).contains(user)) + if (!MothershipManager.get().canAssignTo(getContainer(), user)) { throw new NotFoundException("User not available to assign to: " + form.getUserId()); } diff --git a/mothership/src/org/labkey/mothership/MothershipManager.java b/mothership/src/org/labkey/mothership/MothershipManager.java index 212887eb2c7..2eb11e11f37 100644 --- a/mothership/src/org/labkey/mothership/MothershipManager.java +++ b/mothership/src/org/labkey/mothership/MothershipManager.java @@ -35,7 +35,10 @@ import org.labkey.api.data.TableSelector; import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.query.FieldKey; +import org.labkey.api.security.SecurityManager; import org.labkey.api.security.User; +import org.labkey.api.security.permissions.Permission; +import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.util.DateUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.JsonUtil; @@ -50,6 +53,7 @@ import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.locks.ReentrantLock; import static org.labkey.api.security.UserManager.USER_DISPLAY_NAME_COMPARATOR; @@ -609,18 +613,17 @@ public ServerInstallation getServerInstallation(int id, Container c) return new TableSelector(getTableInfoServerInstallation(), filter, null).getObject(ServerInstallation.class); } + private static final Class ASSIGNED_TO_PERM = UpdatePermission.class; + + public boolean canAssignTo(Container container, User possibleAssignee) + { + return container.hasPermission(possibleAssignee, ASSIGNED_TO_PERM); + } + public List getAssignedToList(Container container) { - List projectUsers = org.labkey.api.security.SecurityManager.getProjectUsers(container.getProject()); - List list = new ArrayList<>(); - // Filter list to only show active users - for (User user : projectUsers) - { - if (user.isActive()) - { - list.add(user); - } - } + List projectUsers = SecurityManager.getUsersWithPermissions(container.getProject(), Set.of(ASSIGNED_TO_PERM)); + List list = new ArrayList<>(projectUsers); // Make mutable so we can sort list.sort(USER_DISPLAY_NAME_COMPARATOR); return list; }