From e7c1c6c7831cce8e5f3705b9410df749f163145c Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 7 Apr 2026 07:43:03 -0700 Subject: [PATCH 1/5] Give inactive users no permissions --- .../labkey/api/security/SecurityManager.java | 69 ++++++++++++++----- 1 file changed, 50 insertions(+), 19 deletions(-) diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index e6ccfc41323..f074a2d052b 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -313,7 +313,7 @@ public static void addGroupListener(GroupListener listener) public static void addGroupListener(GroupListener listener, boolean meFirst) { if (meFirst) - _listeners.add(0, listener); + _listeners.addFirst(listener); else _listeners.add(listener); } @@ -2604,16 +2604,20 @@ public boolean isValid(String[] error) public static class RegistrationEmailTemplate extends SecurityEmailTemplate { protected static final String DEFAULT_SUBJECT = - "Welcome to the ^organizationName^ ^siteShortName^ Web Site new user registration"; + "Welcome to the ^organizationName^ ^siteShortName^ Web Site new user registration"; protected static final String DEFAULT_BODY = - "^optionalMessage^\n\n" + - "You now have an account on the ^organizationName^ ^siteShortName^ web site. We are sending " + - "you this message to verify your email address and to allow you to create a password that will provide secure " + - "access to your data on the web site. To complete the registration process, simply click the link below or " + - "copy it to your browser's address bar. You will then be asked to choose a password.\n\n" + - "^verificationURL^\n\n" + - "The ^siteShortName^ home page is ^homePageURL^. If you have any questions don't hesitate to " + - "contact the ^siteShortName^ team at ^systemEmail^."; + """ + ^optionalMessage^ + + You now have an account on the ^organizationName^ ^siteShortName^ web site. We are sending \ + you this message to verify your email address and to allow you to create a password that will provide secure \ + access to your data on the web site. To complete the registration process, simply click the link below or \ + copy it to your browser's address bar. You will then be asked to choose a password. + + ^verificationURL^ + + The ^siteShortName^ home page is ^homePageURL^. If you have any questions don't hesitate to \ + contact the ^siteShortName^ team at ^systemEmail^."""; @SuppressWarnings("UnusedDeclaration") // Constructor called via reflection public RegistrationEmailTemplate() @@ -2647,14 +2651,17 @@ public RegistrationAdminEmailTemplate() public static class PasswordResetEmailTemplate extends SecurityEmailTemplate { protected static final String DEFAULT_SUBJECT = - "Reset Password Notification from the ^siteShortName^ Web Site"; + "Reset Password Notification from the ^siteShortName^ Web Site"; protected static final String DEFAULT_BODY = - "We have reset your password on the ^organizationName^ ^siteShortName^ web site. " + - "To sign in to the system you will need " + - "to specify a new password. Click the link below or copy it to your browser's address bar. You will then be " + - "asked to enter a new password.\n\n" + - "^verificationURL^\n\n" + - "The ^siteShortName^ home page is ^homePageURL^."; + """ + We have reset your password on the ^organizationName^ ^siteShortName^ web site. \ + To sign in to the system you will need \ + to specify a new password. Click the link below or copy it to your browser's address bar. You will then be \ + asked to enter a new password. + + ^verificationURL^ + + The ^siteShortName^ home page is ^homePageURL^."""; public PasswordResetEmailTemplate() { @@ -3070,7 +3077,7 @@ private static boolean hasPermissions(@Nullable String logMsg, SecurableResource */ public static Set> getPermissions(SecurableResource resource, UserPrincipal principal, Set contextualRoles) { - if (null == resource || null == principal) + if (null == resource || null == principal || !principal.isActive()) return Set.of(); if (principal instanceof User user && resource.getResourceContainer().isForbiddenProject(user, contextualRoles)) @@ -3279,7 +3286,7 @@ public void testAddMemberToGroup() throws InvalidGroupMembershipException for(Object[] groupMemberResponse : groupMemberResponses) { addMemberToGroupVerifyResponse((Group) groupMemberResponse[0], - (UserPrincipal) groupMemberResponse[1], (String) groupMemberResponse[2]); + (UserPrincipal) groupMemberResponse[1], (String) groupMemberResponse[2]); } addMember(groupA, groupB); @@ -3334,6 +3341,30 @@ public void testCreateUser() throws Exception User user2 = AuthenticationManager.authenticate(ViewServlet.mockRequest("GET", new ActionURL(), null, null, null), rawEmail, password); assertNotNull("\"" + rawEmail + "\" failed to authenticate with password \"" + password + "\"; check labkey.log around timestamp " + DateUtil.formatDateTime(new Date(), "HH:mm:ss,SSS") + " for the reason", user2); assertEquals(user, user2); + + // Now test setting that user to inactive + Container testContainer = JunitUtil.getTestContainer(); + if (!testContainer.hasPermission(user, ReadPermission.class)) + { + addRoleAssignment(new MutableSecurityPolicy(testContainer), user, ReaderRole.class, TestContext.get().getUser()); + assertTrue(testContainer.hasPermission(user, ReadPermission.class)); + } + // Set the user to inactive + UserManager.setUserActive(TestContext.get().getUser(), user, false); + // Refresh the user from the cache + user = UserManager.getUser(user.getUserId()); + assertNotNull(user); + assertFalse(user.isActive()); + try + { + user2 = AuthenticationManager.authenticate(ViewServlet.mockRequest("GET", new ActionURL(), null, null, null), rawEmail, password); + fail("Expected authenticate() to throw for inactive user, but it returned " + user2); + } + catch (UnauthorizedException ue) + { + // Expected that inactive user can't authenticate + } + assertFalse(testContainer.hasPermission(user, ReadPermission.class)); } finally { From 06557fb91028b634659ecc00efa04ce46bfe8570 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Fri, 17 Apr 2026 10:34:03 -0700 Subject: [PATCH 2/5] Fix FileLinkMetricsMaintenanceTask user. Rename LimitedUserImpersonatingContext -> LimitedUserPermissionContext. --- api/src/org/labkey/api/security/LimitedUser.java | 8 ++++---- .../experiment/FileLinkMetricsMaintenanceTask.java | 14 +++----------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/api/src/org/labkey/api/security/LimitedUser.java b/api/src/org/labkey/api/security/LimitedUser.java index e771d259bf5..be66f9b64bf 100644 --- a/api/src/org/labkey/api/security/LimitedUser.java +++ b/api/src/org/labkey/api/security/LimitedUser.java @@ -52,17 +52,17 @@ public class LimitedUser extends ClonedUser { // Must be a named class to allow Jackson deserialization (e.g., Evaluation Content loads folder archives via the pipeline using AdminUser) - private static class LimitedUserImpersonatingContext extends NormalPermissionsContext + private static class LimitedUserPermissionContext extends NormalPermissionsContext { private final Set _roles; @SuppressWarnings("unused") // Needed for deserialization - private LimitedUserImpersonatingContext() + private LimitedUserPermissionContext() { _roles = null; } - private LimitedUserImpersonatingContext(Set roles) + private LimitedUserPermissionContext(Set roles) { _roles = roles; } @@ -83,7 +83,7 @@ public Stream getAssignedRoles(User user, SecurableResource resource) @SafeVarargs public LimitedUser(User user, Class... roleClasses) { - super(user, new LimitedUserImpersonatingContext(getRoles(roleClasses))); + super(user, new LimitedUserPermissionContext(getRoles(roleClasses))); } @JsonCreator diff --git a/experiment/src/org/labkey/experiment/FileLinkMetricsMaintenanceTask.java b/experiment/src/org/labkey/experiment/FileLinkMetricsMaintenanceTask.java index b63d47f42ff..2e17d004a12 100644 --- a/experiment/src/org/labkey/experiment/FileLinkMetricsMaintenanceTask.java +++ b/experiment/src/org/labkey/experiment/FileLinkMetricsMaintenanceTask.java @@ -2,21 +2,17 @@ import org.apache.logging.log4j.Logger; import org.labkey.api.data.ContainerManager; -import org.labkey.api.security.LimitedUser; -import org.labkey.api.security.PrincipalType; import org.labkey.api.security.User; -import org.labkey.api.security.roles.ProjectAdminRole; -import org.labkey.api.util.SystemMaintenance; +import org.labkey.api.util.SystemMaintenance.MaintenanceTask; import org.labkey.experiment.api.ExperimentServiceImpl; import java.util.Date; import java.util.HashMap; import java.util.Map; -public class FileLinkMetricsMaintenanceTask implements SystemMaintenance.MaintenanceTask +public class FileLinkMetricsMaintenanceTask implements MaintenanceTask { public static final String NAME = "FileLinkMetricsMaintenanceTask"; - public static final String STARTUP_SCOPE = "FileLinkMetrics"; @Override public String getDescription() @@ -32,10 +28,7 @@ public String getName() private User getTaskUser() { - User taskUser = new User("FileLinkMetricsMaintenanceUser", -1); - taskUser.setPrincipalType(PrincipalType.SERVICE); - taskUser.setDisplayName("FileLinkMetricsMaintenanceUser"); - return new LimitedUser(taskUser, ProjectAdminRole.class); + return User.getAdminServiceUser(); } @Override @@ -77,5 +70,4 @@ public void run(Logger log) log.error("Unable to run missing files check task. {}", e); } } - } From 0e2351326944899f80a9cd4241a62c1d5433c156 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 22 Apr 2026 18:12:36 -0700 Subject: [PATCH 3/5] Throw if "includeInactive" is specified with getUsersWithPermissions() --- api/src/org/labkey/api/security/SecurityManager.java | 5 ++++- .../impersonation/UserImpersonationContextFactory.java | 2 +- core/src/org/labkey/core/query/CoreQuerySchema.java | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index f074a2d052b..042e34801d4 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -2005,8 +2005,11 @@ public static Collection getFolderUserids(Container c) */ public static List getUsersWithPermissions(Container c, boolean includeInactive, Set> perms) { + if (includeInactive) + throw new IllegalArgumentException("includeInactive parameter is no longer supported since inactive users have no permissions"); + // No cache right now, but performance seems fine. After the user list and policy are cached, no other queries occur. - return UserManager.getUsers(includeInactive).stream() + return UserManager.getUsers(false).stream() .filter(user -> hasAllPermissions(null, c, user, perms, Set.of())) .toList(); } diff --git a/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java b/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java index 686c0c74f1e..3fccb6bc0fe 100644 --- a/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java +++ b/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java @@ -163,7 +163,7 @@ public static Collection getValidImpersonationUsers(@Nullable Container pr else if (project != null && project.hasPermission(adminUser, ImpersonatePermission.class)) { // The read permission check is not for security; it just seems useless to offer impersonation on a user who lacks read. - validUsers = new ArrayList<>(SecurityManager.getUsersWithPermissions(project, true, Set.of(ReadPermission.class))); + validUsers = new ArrayList<>(SecurityManager.getUsersWithPermissions(project, false, Set.of(ReadPermission.class))); } else { diff --git a/core/src/org/labkey/core/query/CoreQuerySchema.java b/core/src/org/labkey/core/query/CoreQuerySchema.java index bc8bdcc2f0b..5b8c059e26e 100644 --- a/core/src/org/labkey/core/query/CoreQuerySchema.java +++ b/core/src/org/labkey/core/query/CoreQuerySchema.java @@ -481,7 +481,7 @@ public TableInfo getUsers() { Set projectUserIds = new HashSet<>(SecurityManager.getFolderUserids(getContainer())); // Add app admins and site admins (they both have ApplicationAdminPermission) - SecurityManager.getUsersWithPermissions(ContainerManager.getRoot(), true, Set.of(ApplicationAdminPermission.class)).stream() + SecurityManager.getUsersWithPermissions(ContainerManager.getRoot(), false, Set.of(ApplicationAdminPermission.class)).stream() .map(User::getUserId) .forEach(projectUserIds::add); _projectUserIds = projectUserIds; From c3490a99f428227ad16eb5092cbfb7ace6b507b3 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 22 Apr 2026 18:18:52 -0700 Subject: [PATCH 4/5] Stop calling the variant that takes the flag --- .../org/labkey/api/security/SecurityManager.java | 14 ++++++-------- .../UserImpersonationContextFactory.java | 2 +- .../src/org/labkey/core/query/CoreQuerySchema.java | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index 042e34801d4..f47ae7cdfa6 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -2000,18 +2000,13 @@ public static Collection getFolderUserids(Container c) return userIds; } - /** - * @return an immutable list of Users who have been assigned all the requested permissions in the given container - */ + @Deprecated // Call the other variant public static List getUsersWithPermissions(Container c, boolean includeInactive, Set> perms) { if (includeInactive) throw new IllegalArgumentException("includeInactive parameter is no longer supported since inactive users have no permissions"); - // No cache right now, but performance seems fine. After the user list and policy are cached, no other queries occur. - return UserManager.getUsers(false).stream() - .filter(user -> hasAllPermissions(null, c, user, perms, Set.of())) - .toList(); + return getUsersWithPermissions(c, perms); } /** @@ -2019,7 +2014,10 @@ public static List getUsersWithPermissions(Container c, boolean includeIna */ public static List getUsersWithPermissions(Container c, Set> perms) { - return getUsersWithPermissions(c, false, perms); + // No cache right now, but performance seems fine. After the user list and policy are cached, no other queries occur. + return UserManager.getUsers(false).stream() + .filter(user -> hasAllPermissions(null, c, user, perms, Set.of())) + .toList(); } /** diff --git a/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java b/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java index 3fccb6bc0fe..0f894a43a7b 100644 --- a/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java +++ b/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java @@ -163,7 +163,7 @@ public static Collection getValidImpersonationUsers(@Nullable Container pr else if (project != null && project.hasPermission(adminUser, ImpersonatePermission.class)) { // The read permission check is not for security; it just seems useless to offer impersonation on a user who lacks read. - validUsers = new ArrayList<>(SecurityManager.getUsersWithPermissions(project, false, Set.of(ReadPermission.class))); + validUsers = new ArrayList<>(SecurityManager.getUsersWithPermissions(project, Set.of(ReadPermission.class))); } else { diff --git a/core/src/org/labkey/core/query/CoreQuerySchema.java b/core/src/org/labkey/core/query/CoreQuerySchema.java index 5b8c059e26e..72aa3d8043c 100644 --- a/core/src/org/labkey/core/query/CoreQuerySchema.java +++ b/core/src/org/labkey/core/query/CoreQuerySchema.java @@ -481,7 +481,7 @@ public TableInfo getUsers() { Set projectUserIds = new HashSet<>(SecurityManager.getFolderUserids(getContainer())); // Add app admins and site admins (they both have ApplicationAdminPermission) - SecurityManager.getUsersWithPermissions(ContainerManager.getRoot(), false, Set.of(ApplicationAdminPermission.class)).stream() + SecurityManager.getUsersWithPermissions(ContainerManager.getRoot(), Set.of(ApplicationAdminPermission.class)).stream() .map(User::getUserId) .forEach(projectUserIds::add); _projectUserIds = projectUserIds; From 9976e9b7971aec57d5fdef2a84d21306c0fe17ec Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 28 Apr 2026 11:59:27 -0700 Subject: [PATCH 5/5] Stop supporting "inactive" flags on GetUsersWithPermissionsAction --- .../labkey/api/security/SecurityManager.java | 11 +-- .../org/labkey/core/user/UserController.java | 82 ++++++------------- 2 files changed, 25 insertions(+), 68 deletions(-) diff --git a/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index f47ae7cdfa6..9986346f841 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -74,8 +74,8 @@ 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.ImpersonatePermission; import org.labkey.api.security.permissions.DeletePermission; +import org.labkey.api.security.permissions.ImpersonatePermission; import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.permissions.ReadPermission; @@ -2000,15 +2000,6 @@ public static Collection getFolderUserids(Container c) return userIds; } - @Deprecated // Call the other variant - public static List getUsersWithPermissions(Container c, boolean includeInactive, Set> perms) - { - if (includeInactive) - throw new IllegalArgumentException("includeInactive parameter is no longer supported since inactive users have no permissions"); - - return getUsersWithPermissions(c, perms); - } - /** * @return an immutable list of active Users who have been assigned all the requested permissions in the given container */ diff --git a/core/src/org/labkey/core/user/UserController.java b/core/src/org/labkey/core/user/UserController.java index a780d0d9b8b..881ce5309ed 100644 --- a/core/src/org/labkey/core/user/UserController.java +++ b/core/src/org/labkey/core/user/UserController.java @@ -26,7 +26,6 @@ import org.json.JSONObject; import org.labkey.api.action.ApiResponse; import org.labkey.api.action.ApiSimpleResponse; -import org.labkey.api.action.ApiVersion; import org.labkey.api.action.FormViewAction; import org.labkey.api.action.MutatingApiAction; import org.labkey.api.action.QueryViewAction; @@ -107,7 +106,6 @@ import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.CanAccessLockedProjectsPermission; import org.labkey.api.security.permissions.DeleteUserPermission; -import org.labkey.api.security.permissions.ImpersonatePermission; import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.UpdateUserPermission; @@ -117,7 +115,6 @@ import org.labkey.api.security.roles.RoleManager; import org.labkey.api.settings.AdminConsole; import org.labkey.api.settings.AppProps; -import org.labkey.api.usageMetrics.SimpleMetricsService; import org.labkey.api.util.HtmlStringBuilder; import org.labkey.api.util.MailHelper; import org.labkey.api.util.Pair; @@ -142,7 +139,6 @@ import org.labkey.api.view.ViewContext; import org.labkey.api.view.WebPartView; import org.labkey.api.view.template.PageConfig; -import org.labkey.core.CoreModule; import org.labkey.core.login.DbLoginConfiguration; import org.labkey.core.login.DbLoginManager; import org.labkey.core.login.LoginController; @@ -2693,14 +2689,13 @@ protected void setUsersList(GetUsersForm form, Collection users, ApiSimple } /** - * Retrieves the set of users that have all of a specified set of permissions. A group - * may be provided and only users within that group will be returned. A name (prefix) may be - * provided and only users whose email or display name starts with the prefix will be returned. - * This will not return any deactivated users (since they do not have permissions of any sort). + * Retrieves the set of users that have all of a specified set of permissions. A group may be provided and only + * users within that group will be returned. A name (prefix) may be provided and only users whose email or display + * name starts with the prefix will be returned. This will not return any inactive users (since they do not have + * permissions of any sort). */ @RequiresLogin @RequiresPermission(ReadPermission.class) - @ApiVersion(23.10) public static class GetUsersWithPermissionsAction extends GetUsersAction { @Override @@ -2714,56 +2709,16 @@ else if (form.getPermissionClasses().isEmpty()) { errors.reject(ERROR_GENERIC, "No valid permission classes provided."); } - } - - /** - * The older 23.10 response format does not honor the newer includeInactive flag. It only honors the active - * flag when requesting users of a group, and ignores the flag (only ever returning active users) when not - * requesting a group. - */ - private ApiResponse response2310(ApiSimpleResponse response, GetUsersForm form) - { - SimpleMetricsService.get().increment(CoreModule.CORE_MODULE_NAME, "GetUsersWithPermissionsAction", "OldVersionCalls"); - Collection users; - - //if requesting users in a specific group... - if (null != StringUtils.trimToNull(form.getGroup()) || null != form.getGroupId()) - { - users = filterForPermissions(form, getProjectGroupUsers(form, response, !form.getActive())); - } - else - { - users = SecurityManager.getUsersWithPermissions(getContainer(), form.getPermissionClasses()); - } - - this.setUsersList(form, users, response); - - return response; - } - - /** - * The 23.11 response format does not honor the active flag, instead it honors the includeInactive flag, and - * it honors it when requesting users or groups. The flag defaults to false, so by default only active users - * will be returned. - */ - private ApiResponse response2311(ApiSimpleResponse response, GetUsersForm form) - { - boolean includeInactive = form.getIncludeInactive(); - Collection users; - - //if requesting users in a specific group... - if (null != StringUtils.trimToNull(form.getGroup()) || null != form.getGroupId()) + // 26.5 changes: stop supporting "active" and "includeInactive" parameters. Return errors for now. We could + // remove these checks in 26.11+. + else if (form.getActive()) { - users = filterForPermissions(form, getProjectGroupUsers(form, response, includeInactive)); + errors.reject(ERROR_GENERIC, "The active parameter is no longer supported. Inactive users have no permissions."); } - else + else if (form.getIncludeInactive()) { - users = SecurityManager.getUsersWithPermissions(getContainer(), includeInactive, form.getPermissionClasses()); + errors.reject(ERROR_GENERIC, "The includeInactive parameter is no longer supported. Inactive users have no permissions."); } - - this.setUsersList(form, users, response); - - return response; } @Override @@ -2778,10 +2733,21 @@ public ApiResponse execute(GetUsersForm form, BindException errors) ApiSimpleResponse response = new ApiSimpleResponse(); response.put("container", container.getPath()); - if (getRequestedApiVersion() <= 23.10) - return response2310(response, form); + Collection users; - return response2311(response, form); + if (null != StringUtils.trimToNull(form.getGroup()) || null != form.getGroupId()) + { + users = filterForPermissions(form, getProjectGroupUsers(form, response, false)); + } + else + { + // Active users only + users = SecurityManager.getUsersWithPermissions(getContainer(), form.getPermissionClasses()); + } + + setUsersList(form, users, response); + + return response; } }