From cd698ca1f83170cfb4bde7b886bd8bacaa8d50bb Mon Sep 17 00:00:00 2001 From: MatusBeke Date: Mon, 27 Jul 2026 14:53:18 +0200 Subject: [PATCH 1/2] Add missing @PreAuthorize to ProcessRestRepository#delete DELETE /api/system/processes/{id} had no method-level authorization check, unlike its siblings (findOne, findAll, findByCurrentUser). The only check lived inside ProcessServiceImpl#delete's per-bitstream loop, so a process with zero bitstreams (e.g. status SCHEDULED) skipped the loop entirely and bypassed authorization, letting an anonymous DELETE remove any process record. Adds hasPermission(#integer, 'PROCESS', 'DELETE'), matching findOne. Reuses the existing ProcessRestPermissionEvaluatorPlugin (owner-or- admin check, independent of READ/WRITE/DELETE) - no new plugin needed. Originated from security audit (2026_07_20_dq) --- .../org/dspace/app/rest/repository/ProcessRestRepository.java | 1 + 1 file changed, 1 insertion(+) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ProcessRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ProcessRestRepository.java index 4c0bbf5e160..a1f246fcacd 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ProcessRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ProcessRestRepository.java @@ -164,6 +164,7 @@ public BitstreamRest getProcessBitstreamByType(Integer processId, String type) } @Override + @PreAuthorize("hasPermission(#integer, 'PROCESS', 'DELETE')") protected void delete(Context context, Integer integer) throws AuthorizeException, RepositoryMethodNotImplementedException { try { From 0fd6e5713b689ea3df53ff95f74c6958614a885f Mon Sep 17 00:00:00 2001 From: MatusBeke Date: Mon, 27 Jul 2026 15:14:47 +0200 Subject: [PATCH 2/2] Address review: rename param to id, add DELETE authz regression tests - Rename delete()'s parameter from `integer` to `id` for consistency with the rest of the repository and to make the SpEL expression read clearly (#id vs #integer). - Add ProcessRestRepositoryIT coverage for DELETE authorization on the bitstream-less/SCHEDULED process shape that previously bypassed the check entirely: admin succeeds (204, then 404 on re-fetch), anonymous is rejected (401), and a different authenticated user is rejected (403). Addresses both Copilot review comments on PR #1386. --- .../repository/ProcessRestRepository.java | 8 ++--- .../app/rest/ProcessRestRepositoryIT.java | 36 +++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ProcessRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ProcessRestRepository.java index a1f246fcacd..3ae8600f83a 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ProcessRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ProcessRestRepository.java @@ -164,13 +164,13 @@ public BitstreamRest getProcessBitstreamByType(Integer processId, String type) } @Override - @PreAuthorize("hasPermission(#integer, 'PROCESS', 'DELETE')") - protected void delete(Context context, Integer integer) + @PreAuthorize("hasPermission(#id, 'PROCESS', 'DELETE')") + protected void delete(Context context, Integer id) throws AuthorizeException, RepositoryMethodNotImplementedException { try { - processService.delete(context, processService.find(context, integer)); + processService.delete(context, processService.find(context, id)); } catch (SQLException | IOException e) { - log.error("Something went wrong trying to find Process with id: " + integer, e); + log.error("Something went wrong trying to find Process with id: " + id, e); throw new RuntimeException(e.getMessage(), e); } } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ProcessRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ProcessRestRepositoryIT.java index 6c018df6d07..e95d5b9a0cb 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ProcessRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ProcessRestRepositoryIT.java @@ -12,6 +12,7 @@ import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.is; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -126,6 +127,41 @@ public void getProcessForDifferentUserForbiddenException() throws Exception { } + @Test + public void deleteProcessAdmin() throws Exception { + // "process" (from setup) is SCHEDULED with no bitstreams - the exact shape that + // previously bypassed authorization entirely. + String token = getAuthToken(admin.getEmail(), password); + + getClient(token).perform(delete("/api/system/processes/" + process.getID())) + .andExpect(status().isNoContent()); + + getClient(token).perform(get("/api/system/processes/" + process.getID())) + .andExpect(status().isNotFound()); + } + + @Test + public void deleteProcessAnonymousUnauthorizedException() throws Exception { + getClient().perform(delete("/api/system/processes/" + process.getID())) + .andExpect(status().isUnauthorized()); + + String token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/system/processes/" + process.getID())) + .andExpect(status().isOk()); + } + + @Test + public void deleteProcessForDifferentUserForbiddenException() throws Exception { + String token = getAuthToken(eperson.getEmail(), password); + + getClient(token).perform(delete("/api/system/processes/" + process.getID())) + .andExpect(status().isForbidden()); + + String adminToken = getAuthToken(admin.getEmail(), password); + getClient(adminToken).perform(get("/api/system/processes/" + process.getID())) + .andExpect(status().isOk()); + } + @Test public void getProcessNotExisting() throws Exception { String token = getAuthToken(eperson.getEmail(), password);