From 8e6ce738bff1c40503379ea22582e4665a43b255 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 12 Apr 2025 16:06:03 -0700 Subject: [PATCH] Adjust permissions page for Site Administrators and Developers group changes --- .../core/security/SecurityApiActions.java | 17 ++++++++++++----- core/webapp/Security/util/SecurityCache.js | 7 +------ core/webapp/Security/window/UserInfoPopup.js | 10 ++++------ .../test/tests/study/StudySecurityTest.java | 2 -- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/core/src/org/labkey/core/security/SecurityApiActions.java b/core/src/org/labkey/core/security/SecurityApiActions.java index c780ec52b97..f60543a0206 100644 --- a/core/src/org/labkey/core/security/SecurityApiActions.java +++ b/core/src/org/labkey/core/security/SecurityApiActions.java @@ -970,14 +970,14 @@ public void setName(String name) public static class IdForm { - private int _id = -1; + private Integer _id = null; - public int getId() + public Integer getId() { return _id; } - public void setId(int id) + public void setId(Integer id) { _id = id; } @@ -1515,9 +1515,13 @@ public static class DeleteGroupAction extends MutatingApiAction @Override public ApiResponse execute(IdForm form, BindException errors) { - if (form.getId() < 0) + if (form.getId() == null) throw new IllegalArgumentException("You must specify an id parameter!"); + // Allow deleting of legacy Site Administrators (-1) and Developers (-4) groups, if they exist + if (form.getId() < 0 && form.getId() != -1 && form.getId() != -4) + throw new IllegalArgumentException("You must specify a valid id parameter!"); + Group group = SecurityManager.getGroup(form.getId()); Container c = getContainer(); if (null == group || (c.isRoot() && null != group.getContainer()) || (!c.isRoot() && !c.getId().equals(group.getContainer()))) @@ -1540,9 +1544,12 @@ public static class DeleteUserAction extends MutatingApiAction @Override public ApiResponse execute(IdForm form, BindException errors) throws Exception { - if (form.getId() < 0) + if (form.getId() == null) throw new IllegalArgumentException("You must specify an id parameter!"); + if (form.getId() < 0) + throw new IllegalArgumentException("You must specify a valid id parameter!"); + User user = UserManager.getUser(form.getId()); if (null == user) throw new IllegalArgumentException("User id " + form.getId() + " does not exist"); diff --git a/core/webapp/Security/util/SecurityCache.js b/core/webapp/Security/util/SecurityCache.js index 95eab7ada60..c716007bbea 100644 --- a/core/webapp/Security/util/SecurityCache.js +++ b/core/webapp/Security/util/SecurityCache.js @@ -16,10 +16,8 @@ Ext4.define('Security.util.SecurityCache', { }, statics : { - groupAdministrators : -1, groupUsers : -2, groupGuests : -3, - groupDevelopers : -4, global : null, getGlobalCache : function() { @@ -575,9 +573,6 @@ Ext4.define('Security.util.SecurityCache', { Principals_onReady : function() { - var admin = this.principalsStore.getById(Security.util.SecurityCache.groupAdministrators); - if (admin) - admin.data.Name = 'Site Administrators'; var users = this.principalsStore.getById(Security.util.SecurityCache.groupUsers); if (users) users.data.Name = 'All Site Users'; @@ -596,7 +591,7 @@ Ext4.define('Security.util.SecurityCache', { _applyPrincipalsSortOrder : function(item) { var data = item.data; - var major = data.Type == 'u' ? '4' : data.Container ? '3' : data.UserId > 0 ? '2' : '1'; // Put system groups at the top + var major = data.Type === 'u' ? '4' : data.Container ? '3' : data.UserId > 0 ? '2' : '1'; // Put system groups at the top var minor = (data.Name||'').toLowerCase(); data.sortOrder = major + "." + minor; }, diff --git a/core/webapp/Security/window/UserInfoPopup.js b/core/webapp/Security/window/UserInfoPopup.js index 8be598814b0..bfd26a5e3ba 100644 --- a/core/webapp/Security/window/UserInfoPopup.js +++ b/core/webapp/Security/window/UserInfoPopup.js @@ -57,9 +57,6 @@ Ext4.define('Security.window.UserInfoPopup', { if (this.user.UserId === Security.util.SecurityCache.groupUsers || this.user.UserId === Security.util.SecurityCache.groupGuests) this.canEdit = false; - if (!LABKEY.user.isSystemAdmin && this.user.UserId === Security.util.SecurityCache.groupAdministrators) - this.canEdit = false; - if (this.canEdit) { const userContainer = this.user.Container === this.cache.projectId ? container : this.user.Container; @@ -169,10 +166,11 @@ Ext4.define('Security.window.UserInfoPopup', { { toAdd.push({html: '

Site Users represents all signed-in users.

', border: false, frame: false}); } - else { + else + { toAdd.push({html: '

Members

', border: false, frame: false}); if (this.canEdit) { - if (users.length == 0 && this.userId > 0) { + if (users.length === 0 && this.userId !== Security.util.SecurityCache.groupGuests) { toAdd.push(Ext4.create('Ext.Button', { text: "Delete Empty Group", handler: Ext4.bind(this.DeleteGroup_onClick, this) @@ -181,7 +179,7 @@ Ext4.define('Security.window.UserInfoPopup', { } let items = []; - const canRemove = this.canEdit && (this.userId !== Security.util.SecurityCache.groupAdministrators || users.length > 1); + const canRemove = this.canEdit && users.length > 1; for (i = 0; i < users.length; i++) { user = users[i]; diff --git a/study/test/src/org/labkey/test/tests/study/StudySecurityTest.java b/study/test/src/org/labkey/test/tests/study/StudySecurityTest.java index 400c1586e90..db89fd850a7 100644 --- a/study/test/src/org/labkey/test/tests/study/StudySecurityTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudySecurityTest.java @@ -62,7 +62,6 @@ public class StudySecurityTest extends BaseWebDriverTest private static final String GROUP_NONE = "No Access"; // General system groups. - private static final String GROUP_DEVELOPER = "Developers"; private static final String GROUP_GUESTS = "Guests"; private static final String GROUP_ALL_USERS = "All site users"; private static final String GROUP_USERS = "Users"; @@ -696,7 +695,6 @@ public void setTestDefaultStudySecurity() GROUP_AUTHORS, GroupSecuritySetting.PER_DATASET, GROUP_LIMITED, GroupSecuritySetting.PER_DATASET, GROUP_NONE, GroupSecuritySetting.NONE, - GROUP_DEVELOPER, GroupSecuritySetting.NONE, GROUP_GUESTS, GroupSecuritySetting.NONE, GROUP_ALL_USERS, GroupSecuritySetting.NONE, GROUP_USERS, GroupSecuritySetting.NONE);