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/api/src/org/labkey/api/security/SecurityManager.java b/api/src/org/labkey/api/security/SecurityManager.java index 139a7ce4545..16a02ab4921 100644 --- a/api/src/org/labkey/api/security/SecurityManager.java +++ b/api/src/org/labkey/api/security/SecurityManager.java @@ -75,8 +75,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; @@ -314,7 +314,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); } @@ -2009,24 +2009,16 @@ public static Collection getFolderUserids(Container c) } /** - * @return an immutable list of Users who have been assigned all the requested permissions in the given container + * @return an immutable list of active Users who have been assigned all the requested permissions in the given container */ - public static List getUsersWithPermissions(Container c, boolean includeInactive, Set> perms) + public static List getUsersWithPermissions(Container c, Set> perms) { // 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(); } - /** - * @return an immutable list of active Users who have been assigned all the requested permissions in the given container - */ - public static List getUsersWithPermissions(Container c, Set> perms) - { - return getUsersWithPermissions(c, false, perms); - } - /** * @return an immutable list of Users who have been assigned any of the requested permissions in the given container */ @@ -2612,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() @@ -2655,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() { @@ -3078,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)) @@ -3287,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); @@ -3342,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 { diff --git a/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java b/api/src/org/labkey/api/security/impersonation/UserImpersonationContextFactory.java index 686c0c74f1e..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, true, 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 bc8bdcc2f0b..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(), true, Set.of(ApplicationAdminPermission.class)).stream() + SecurityManager.getUsersWithPermissions(ContainerManager.getRoot(), Set.of(ApplicationAdminPermission.class)).stream() .map(User::getUserId) .forEach(projectUserIds::add); _projectUserIds = projectUserIds; 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; } } 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); } } - }