Skip to content

GitHub Issue 1300: BulkUpgradeGroupAction scoping changes #3099

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

Open
labkey-jeckels wants to merge 2 commits into
base: release25.7-SNAPSHOT
Choose a base branch
from
Open

GitHub Issue 1300: BulkUpgradeGroupAction scoping changes #3099

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be2e984
GitHub Issue 1300: BulkUpgradeGroupAction scoping changes
labkey-jeckels Jul 10, 2026
9b3dfff
Can't use getFirst() yet
labkey-jeckels Jul 10, 2026
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
62 changes: 62 additions & 0 deletions src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.labkey.test.util.APIUserHelper;
import org.labkey.test.util.ApiPermissionsHelper;
import org.labkey.test.util.DataRegionTable;
import org.labkey.test.util.PermissionsHelper;
import org.labkey.test.util.PermissionsHelper.PrincipalType;

import java.util.ArrayList;
Expand Down Expand Up @@ -61,11 +62,15 @@ public class BulkUpdateGroupApiTest extends BaseWebDriverTest
private static Integer group1Id;
private static Integer group2Id;
private static final String siteGroup = "createdSiteGroup";
private static final String OTHER_PROJECT = "BulkUpdateGroupApiTest Other Project";
private static final String OTHER_GROUP = "otherProjectGroup";
private static Integer otherProjectGroupId;

@Override
protected void doCleanup(boolean afterTest) throws TestTimeoutException
{
_containerHelper.deleteProject(getProjectName(), afterTest);
_containerHelper.deleteProject(OTHER_PROJECT, afterTest);
deleteTestUsers(EMAIL_SUFFIX);
_permissionsHelper.deleteGroup(siteGroup);
}
Expand Down Expand Up @@ -101,6 +106,9 @@ private void doSetup()
user2Id = _userHelper.createUser(USER2).getUserId();
group1Id = _permissionsHelper.createProjectGroup(GROUP1, getProjectName());
group2Id = _permissionsHelper.createProjectGroup(GROUP2, getProjectName());

_containerHelper.createProject(OTHER_PROJECT, null);
otherProjectGroupId = _permissionsHelper.createProjectGroup(OTHER_GROUP, OTHER_PROJECT);
}

@Test
Expand Down Expand Up @@ -499,6 +507,60 @@ public void testBulkUpdateAuditing() throws Exception
AuditLogTest.verifyAuditEvent(this, AuditLogTest.GROUP_AUDIT_EVENT, "Comment", GROUP1, 2);
}

// A project group may only be modified through the container it belongs to; targeting another project's group must be rejected.
@Test
public void testGroupFromDifferentContainerError() throws Exception
{
// otherProjectGroupId belongs to OTHER_PROJECT, but the command targets this test's project
BulkUpdateGroupCommand command = new BulkUpdateGroupCommand(otherProjectGroupId);
command.addMemberUser(USER1);
Connection connection = createDefaultConnection();

try
{
String message = command.execute(connection, getProjectName()).getText();
fail("Expected CommandException modifying a group from another container\nResponse:\n" + message);
}
catch (CommandException e)
{
assertTrue("Expected cross-container error. Actual error: " + e.getMessage(), e.getMessage().contains("does not belong to this project"));
}

_permissionsHelper.assertUserNotInGroup(USER1, OTHER_GROUP, OTHER_PROJECT, PrincipalType.USER);
}

// Adding an existing user to a group requires only AdminPermission, but creating a brand-new user additionally requires AddUserPermission.
@Test
public void testCreateUserWithoutAddUserPermission() throws Exception
{
// A Folder Administrator has AdminPermission (so the action's @RequiresPermission passes) but lacks AddUserPermission
String folderAdmin = genTestEmail("folderadminnoadduser");
String newUser = genTestEmail("shouldnotbecreated");
_userHelper.createUser(folderAdmin);
_permissionsHelper.addMemberToRole(folderAdmin, PermissionsHelper.FOLDER_ADMIN_ROLE, PermissionsHelper.MemberType.user, getProjectName());

Connection connection = createDefaultConnection();
connection.impersonate(folderAdmin, getProjectName());
try
{
BulkUpdateGroupCommand command = new BulkUpdateGroupCommand(group1Id);
command.addMemberUser(USER1); // pre-existing user: should be added successfully
command.addMemberUser(newUser); // new user: folder admin lacks AddUserPermission
BulkUpdateGroupResponse response = command.execute(connection, getProjectName());

List<String> errors = collectErrors(response);
assertEquals("Expected exactly one member error:\n" + String.join("\n", errors), 1, errors.size());
assertTrue("Expected AddUserPermission error. Actual error: " + errors.get(0), errors.get(0).contains("do not have permission to create new users"));
}
finally
{
connection.stopImpersonating();
}

assertNull("New user was created despite the caller lacking AddUserPermission", _userHelper.getUserId(newUser));
_permissionsHelper.assertUserInGroup(USER1, GROUP1, getProjectName(), PrincipalType.USER);
}

protected List<String> collectErrors(BulkUpdateGroupResponse response)
{
Map<String, Object> errors = response.getErrors();
Expand Down