From be2e9845486483eee0468b0c1951fc49982ac199 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 10 Jul 2026 15:07:07 -0700 Subject: [PATCH 1/2] GitHub Issue 1300: BulkUpgradeGroupAction scoping changes --- .../remoteapi/BulkUpdateGroupApiTest.java | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java b/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java index bdef4aabf0..309559a3e5 100644 --- a/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java +++ b/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java @@ -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; @@ -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); } @@ -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 @@ -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 errors = collectErrors(response); + assertEquals("Expected exactly one member error:\n" + String.join("\n", errors), 1, errors.size()); + assertTrue("Expected AddUserPermission error. Actual error: " + errors.getFirst(), errors.getFirst().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 collectErrors(BulkUpdateGroupResponse response) { Map errors = response.getErrors(); From 9b3dfff198359035bdc0c5e3e84d92f151f4d800 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 10 Jul 2026 15:59:03 -0700 Subject: [PATCH 2/2] Can't use getFirst() yet --- src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java b/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java index 309559a3e5..720a3f335d 100644 --- a/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java +++ b/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java @@ -550,7 +550,7 @@ public void testCreateUserWithoutAddUserPermission() throws Exception List errors = collectErrors(response); assertEquals("Expected exactly one member error:\n" + String.join("\n", errors), 1, errors.size()); - assertTrue("Expected AddUserPermission error. Actual error: " + errors.getFirst(), errors.getFirst().contains("do not have permission to create new users")); + assertTrue("Expected AddUserPermission error. Actual error: " + errors.get(0), errors.get(0).contains("do not have permission to create new users")); } finally {