diff --git a/dspace-api/src/main/java/org/dspace/app/bulkaccesscontrol/BulkAccessControlScriptConfiguration.java b/dspace-api/src/main/java/org/dspace/app/bulkaccesscontrol/BulkAccessControlScriptConfiguration.java index 5196247f94cb..443cc8ad8206 100644 --- a/dspace-api/src/main/java/org/dspace/app/bulkaccesscontrol/BulkAccessControlScriptConfiguration.java +++ b/dspace-api/src/main/java/org/dspace/app/bulkaccesscontrol/BulkAccessControlScriptConfiguration.java @@ -9,12 +9,14 @@ import java.io.InputStream; import java.sql.SQLException; +import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.UUID; -import java.util.stream.Collectors; +import org.apache.commons.cli.DefaultParser; import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; import org.dspace.app.util.DSpaceObjectUtilsImpl; import org.dspace.app.util.service.DSpaceObjectUtils; import org.dspace.content.DSpaceObject; @@ -41,13 +43,14 @@ public boolean isAllowedToExecute(Context context, List dspaceObjectIDs = - commandLineParameters.stream() - .filter(parameter -> "-u".equals(parameter.getName())) - .map(DSpaceCommandLineParameter::getValue) - .collect(Collectors.toList()); + } + String[] dspaceObjectIDs = getTargetUuids(commandLineParameters); + if (Objects.isNull(dspaceObjectIDs)) { + // Without target uuids there is nothing to check per object, so fail closed: only a site admin + // may go on. The run would fail before modifying anything, but this check must not rely on that. + return authorizeService.isAdmin(context); + } else { DSpaceObjectUtils dSpaceObjectUtils = new DSpace().getServiceManager().getServiceByName( DSpaceObjectUtilsImpl.class.getName(), DSpaceObjectUtilsImpl.class); @@ -71,6 +74,29 @@ public boolean isAllowedToExecute(Context context, List commandLineParameters) { + List args = new ArrayList<>(); + for (DSpaceCommandLineParameter parameter : commandLineParameters) { + args.add(parameter.getName()); + if (parameter.getValue() != null) { + args.add(parameter.getValue()); + } + } + + try { + return new DefaultParser().parse(getOptions(), args.toArray(new String[0])).getOptionValues('u'); + } catch (ParseException e) { + return null; + } + } + @Override public Options getOptions() { if (options == null) { diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/bulkaccesscontrol/BulkAccessControlScriptIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/bulkaccesscontrol/BulkAccessControlScriptIT.java index da0f90ca97c7..f328f25b1877 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/bulkaccesscontrol/BulkAccessControlScriptIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/bulkaccesscontrol/BulkAccessControlScriptIT.java @@ -50,6 +50,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.MediaType; import org.springframework.mock.web.MockMultipartFile; +import org.springframework.test.web.servlet.ResultActions; /** * Basic integration testing for the bulk access Import feature via UI {@link BulkAccessControl}. @@ -431,6 +432,21 @@ public void bulkAccessScriptWithMultipleTargetUuidsWithAdminUserTest() throws Ex @Test public void bulkAccessScriptWithoutTargetUUIDParameterTest() throws Exception { + String token = getAuthToken(admin.getEmail(), password); + + getClient(token) + .perform( + multipart(CURATE_SCRIPT_ENDPOINT) + .param("properties", new ObjectMapper().writeValueAsString(List.of())) + ) + .andExpect(status().isInternalServerError()) + .andExpect(result -> assertTrue(result.getResolvedException() + .getMessage() + .contains("At least one target uuid must be provided"))); + } + + @Test + public void bulkAccessScriptWithoutTargetUUIDParameterWithAdminUserOfCommunityTest() throws Exception { context.turnOffAuthorisationSystem(); parentCommunity = CommunityBuilder.createCommunity(context) @@ -447,10 +463,7 @@ public void bulkAccessScriptWithoutTargetUUIDParameterTest() throws Exception { multipart(CURATE_SCRIPT_ENDPOINT) .param("properties", new ObjectMapper().writeValueAsString(List.of())) ) - .andExpect(status().isInternalServerError()) - .andExpect(result -> assertTrue(result.getResolvedException() - .getMessage() - .contains("At least one target uuid must be provided"))); + .andExpect(status().isForbidden()); } @Test @@ -499,4 +512,70 @@ public void bulkAccessScriptWithNormalUserTest() throws Exception { .andExpect(status().isForbidden()); } + @Test + public void bulkAccessScriptWithAlternativeUuidSpellingsOfNotAdministeredCommunityTest() throws Exception { + context.turnOffAuthorisationSystem(); + CommunityBuilder.createCommunity(context) + .withName("Administered Community") + .withAdminGroup(eperson) + .build(); + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Other Community") + .build(); + context.restoreAuthSystemState(); + + String uuid = parentCommunity.getID().toString(); + for (DSpaceCommandLineParameter uuidParameter : List.of( + new DSpaceCommandLineParameter("--uuid", uuid), + new DSpaceCommandLineParameter("--uu", uuid), + new DSpaceCommandLineParameter("--uuid=" + uuid, null), + new DSpaceCommandLineParameter("-u" + uuid, null))) { + performBulkAccessScript(eperson.getEmail(), uuidParameter) + .andExpect(status().isForbidden()); + } + } + + @Test + public void bulkAccessScriptWithLongUuidOptionOfAdministeredCommunityTest() throws Exception { + context.turnOffAuthorisationSystem(); + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Administered Community") + .withAdminGroup(eperson) + .build(); + context.restoreAuthSystemState(); + + performBulkAccessScript(eperson.getEmail(), + new DSpaceCommandLineParameter("--uuid", parentCommunity.getID().toString())) + .andExpect(status().isAccepted()); + } + + private ResultActions performBulkAccessScript(String email, DSpaceCommandLineParameter uuidParameter) + throws Exception { + String json = "{ \"item\": {\n" + + " \"mode\": \"replace\",\n" + + " \"accessConditions\": [\n" + + " {\n" + + " \"name\": \"openaccess\"\n" + + " }\n" + + " ]\n" + + " }}\n"; + + MockMultipartFile bitstreamFile = new MockMultipartFile("file", "test.json", MediaType.TEXT_PLAIN_VALUE, + new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8))); + + List list = + List.of(uuidParameter, new DSpaceCommandLineParameter("-f", "test.json")).stream() + .map(dSpaceCommandLineParameter -> dSpaceRunnableParameterConverter + .convert(dSpaceCommandLineParameter, Projection.DEFAULT)) + .collect(Collectors.toList()); + + String token = getAuthToken(email, password); + + return getClient(token) + .perform( + multipart(CURATE_SCRIPT_ENDPOINT) + .file(bitstreamFile) + .param("properties", new ObjectMapper().writeValueAsString(list))); + } + }