Skip to content

Adjust permissions page for site group changes #6560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 13, 2025
+17 −19
Merged

Adjust permissions page for site group changes #6560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions core/src/org/labkey/core/security/SecurityApiActions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -1515,9 +1515,13 @@ public static class DeleteGroupAction extends MutatingApiAction<IdForm>
@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())))
Expand All @@ -1540,9 +1544,12 @@ public static class DeleteUserAction extends MutatingApiAction<IdForm>
@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");
Expand Down
7 changes: 1 addition & 6 deletions core/webapp/Security/util/SecurityCache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@ Ext4.define('Security.util.SecurityCache', {
},

statics : {
groupAdministrators : -1,
groupUsers : -2,
groupGuests : -3,
groupDevelopers : -4,

global : null,
getGlobalCache : function() {
Expand Down Expand Up @@ -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';
Expand All @@ -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;
},
Expand Down
10 changes: 4 additions & 6 deletions core/webapp/Security/window/UserInfoPopup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -169,10 +166,11 @@ Ext4.define('Security.window.UserInfoPopup', {
{
toAdd.push({html: '<p>Site Users represents all signed-in users.</p>', border: false, frame: false});
}
else {
else
{
toAdd.push({html: '<p class="userinfoHdr">Members</p>', 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)
Expand All @@ -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];

Expand Down
2 changes: 0 additions & 2 deletions study/test/src/org/labkey/test/tests/study/StudySecurityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down