From cd1b10e620b59693bebf7815f47958e81fdf66ad Mon Sep 17 00:00:00 2001 From: matus Date: Mon, 27 Jul 2026 10:50:47 +0200 Subject: [PATCH 1/3] Add missing authorization to workflow step link subresources The getStep() link methods on WorkflowItemStepLinkRepository, PoolTaskStepLinkRepository and ClaimedTaskStepLinkRepository had no @PreAuthorize, unlike the sibling workflowitem link repositories (collection/item/submitter) and the parent findOne endpoints. Because @LinkRest methods are invoked directly on the link repository, the parent's authorization is never evaluated, so an unauthenticated caller reaches the handler and can disclose task existence and workflow step state (404-vs-401 divergence on a non-existent id; 200 with step state for a real task). Apply the same READ permission check the parent repositories already use, so each /step subresource enforces the same authorization as its parent. Co-Authored-By: Claude Opus 4.8 --- .../app/rest/repository/ClaimedTaskStepLinkRepository.java | 2 ++ .../dspace/app/rest/repository/PoolTaskStepLinkRepository.java | 2 ++ .../app/rest/repository/WorkflowItemStepLinkRepository.java | 2 ++ 3 files changed, 6 insertions(+) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClaimedTaskStepLinkRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClaimedTaskStepLinkRepository.java index 9ee277171e84..6e123dde7843 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClaimedTaskStepLinkRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClaimedTaskStepLinkRepository.java @@ -21,6 +21,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Pageable; import org.springframework.data.rest.webmvc.ResourceNotFoundException; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Component; /** @@ -44,6 +45,7 @@ public class ClaimedTaskStepLinkRepository extends AbstractDSpaceRestRepository * @return The {@link WorkflowStepRest} object related to the {@link ClaimedTask} specified by * the given ID */ + @PreAuthorize("hasPermission(#claimedTaskId, 'CLAIMEDTASK', 'READ')") public WorkflowStepRest getStep(@Nullable HttpServletRequest request, Integer claimedTaskId, @Nullable Pageable optionalPageable, diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/PoolTaskStepLinkRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/PoolTaskStepLinkRepository.java index 6e7f4f84ace3..807010228725 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/PoolTaskStepLinkRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/PoolTaskStepLinkRepository.java @@ -21,6 +21,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Pageable; import org.springframework.data.rest.webmvc.ResourceNotFoundException; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Component; /** @@ -44,6 +45,7 @@ public class PoolTaskStepLinkRepository extends AbstractDSpaceRestRepository imp * @return The {@link WorkflowStepRest} object related to the {@link PoolTask} specified by * the given ID */ + @PreAuthorize("hasPermission(#poolTaskId, 'POOLTASK', 'READ')") public WorkflowStepRest getStep(@Nullable HttpServletRequest request, Integer poolTaskId, @Nullable Pageable optionalPageable, diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemStepLinkRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemStepLinkRepository.java index 30aac1579c79..308e5fd2a878 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemStepLinkRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemStepLinkRepository.java @@ -26,6 +26,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Pageable; import org.springframework.data.rest.webmvc.ResourceNotFoundException; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Component; /** @@ -56,6 +57,7 @@ public class WorkflowItemStepLinkRepository extends AbstractDSpaceRestRepository * @return The {@link WorkflowStepRest} object related to the * {@link org.dspace.workflow.WorkflowItem} specified by the given ID */ + @PreAuthorize("hasPermission(#workflowItemId, 'WORKFLOWITEM', 'READ')") public WorkflowStepRest getStep(@Nullable HttpServletRequest request, Integer workflowItemId, @Nullable Pageable optionalPageable, From ba6ba4aaa454f40577fe6d27ab976bbbfb891d7c Mon Sep 17 00:00:00 2001 From: matus Date: Mon, 27 Jul 2026 13:28:07 +0200 Subject: [PATCH 2/3] Add authorization regression tests for workflow step link subresources Cover the /step subresource of pooltasks, claimedtasks and workflowitems: anonymous -> 401, authenticated user without READ permission -> 403, task owner / administrator -> 200. Guards against regressing the missing @PreAuthorize checks (internal security audit 2026_07_20_dq). Co-Authored-By: Claude Opus 4.8 --- .../rest/WorkflowStepLinkRepositoryIT.java | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepLinkRepositoryIT.java diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepLinkRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepLinkRepositoryIT.java new file mode 100644 index 000000000000..183daff543ce --- /dev/null +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepLinkRepositoryIT.java @@ -0,0 +1,151 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.rest; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import org.dspace.app.rest.test.AbstractControllerIntegrationTest; +import org.dspace.builder.ClaimedTaskBuilder; +import org.dspace.builder.CollectionBuilder; +import org.dspace.builder.CommunityBuilder; +import org.dspace.builder.EPersonBuilder; +import org.dspace.builder.PoolTaskBuilder; +import org.dspace.content.Collection; +import org.dspace.content.Community; +import org.dspace.eperson.EPerson; +import org.dspace.xmlworkflow.storedcomponents.ClaimedTask; +import org.dspace.xmlworkflow.storedcomponents.PoolTask; +import org.junit.Test; + +/** + * Authorization regression tests for the workflow "step" link subresources: + * {@code /api/workflow/{pooltasks,claimedtasks,workflowitems}/{id}/step}. + * + * These {@code @LinkRest} subresources previously had no method-level authorization, so the + * access control enforced on their parent endpoints was bypassed and an anonymous caller could + * reach the handler. Each /step subresource must now enforce the same READ permission as its + * parent. (Found during internal security audit 2026_07_20_dq.) + */ +public class WorkflowStepLinkRepositoryIT extends AbstractControllerIntegrationTest { + + @Test + public void poolTaskStepEnforcesAuthorization() throws Exception { + context.turnOffAuthorisationSystem(); + + EPerson reviewer = EPersonBuilder.createEPerson(context) + .withEmail("reviewer-step@example.com").withPassword(password).build(); + EPerson otherEPerson = EPersonBuilder.createEPerson(context) + .withEmail("other-step@example.com").withPassword(password).build(); + EPerson submitter = EPersonBuilder.createEPerson(context) + .withEmail("submitter-step@example.com").withPassword(password).build(); + + parentCommunity = CommunityBuilder.createCommunity(context).withName("Parent Community").build(); + Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) + .withName("Sub Community").build(); + Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1") + .withWorkflowGroup(1, reviewer).build(); + + context.setCurrentUser(submitter); + PoolTask poolTask = PoolTaskBuilder.createPoolTask(context, col1, reviewer) + .withTitle("Workflow Item Pool").withIssueDate("2017-10-17").build(); + + context.restoreAuthSystemState(); + + String reviewerToken = getAuthToken(reviewer.getEmail(), password); + String otherToken = getAuthToken(otherEPerson.getEmail(), password); + String adminToken = getAuthToken(admin.getEmail(), password); + + String stepPath = "/api/workflow/pooltasks/" + poolTask.getID() + "/step"; + + // anonymous caller must not reach the handler + getClient().perform(get(stepPath)).andExpect(status().isUnauthorized()); + // an authenticated user without READ permission on the task must be blocked + getClient(otherToken).perform(get(stepPath)).andExpect(status().isForbidden()); + // the task owner may read the step + getClient(reviewerToken).perform(get(stepPath)).andExpect(status().isOk()); + // an administrator may read the step + getClient(adminToken).perform(get(stepPath)).andExpect(status().isOk()); + } + + @Test + public void claimedTaskStepEnforcesAuthorization() throws Exception { + context.turnOffAuthorisationSystem(); + + EPerson reviewer = EPersonBuilder.createEPerson(context) + .withEmail("reviewer-step@example.com").withPassword(password).build(); + EPerson otherEPerson = EPersonBuilder.createEPerson(context) + .withEmail("other-step@example.com").withPassword(password).build(); + EPerson submitter = EPersonBuilder.createEPerson(context) + .withEmail("submitter-step@example.com").withPassword(password).build(); + + parentCommunity = CommunityBuilder.createCommunity(context).withName("Parent Community").build(); + Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) + .withName("Sub Community").build(); + Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1") + .withWorkflowGroup(1, reviewer).build(); + + context.setCurrentUser(submitter); + ClaimedTask claimedTask = ClaimedTaskBuilder.createClaimedTask(context, col1, reviewer) + .withTitle("Workflow Item Claimed").withIssueDate("2017-10-17").build(); + + context.restoreAuthSystemState(); + + String reviewerToken = getAuthToken(reviewer.getEmail(), password); + String otherToken = getAuthToken(otherEPerson.getEmail(), password); + String adminToken = getAuthToken(admin.getEmail(), password); + + String stepPath = "/api/workflow/claimedtasks/" + claimedTask.getID() + "/step"; + + // anonymous caller must not reach the handler + getClient().perform(get(stepPath)).andExpect(status().isUnauthorized()); + // an authenticated user without READ permission on the task must be blocked + getClient(otherToken).perform(get(stepPath)).andExpect(status().isForbidden()); + // the task owner may read the step + getClient(reviewerToken).perform(get(stepPath)).andExpect(status().isOk()); + // an administrator may read the step + getClient(adminToken).perform(get(stepPath)).andExpect(status().isOk()); + } + + @Test + public void workflowItemStepEnforcesAuthorization() throws Exception { + context.turnOffAuthorisationSystem(); + + EPerson reviewer = EPersonBuilder.createEPerson(context) + .withEmail("reviewer-step@example.com").withPassword(password).build(); + EPerson otherEPerson = EPersonBuilder.createEPerson(context) + .withEmail("other-step@example.com").withPassword(password).build(); + EPerson submitter = EPersonBuilder.createEPerson(context) + .withEmail("submitter-step@example.com").withPassword(password).build(); + + parentCommunity = CommunityBuilder.createCommunity(context).withName("Parent Community").build(); + Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) + .withName("Sub Community").build(); + Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1") + .withWorkflowGroup(1, reviewer).build(); + + context.setCurrentUser(submitter); + PoolTask poolTask = PoolTaskBuilder.createPoolTask(context, col1, reviewer) + .withTitle("Workflow Item Pool").withIssueDate("2017-10-17").build(); + + context.restoreAuthSystemState(); + + String otherToken = getAuthToken(otherEPerson.getEmail(), password); + String adminToken = getAuthToken(admin.getEmail(), password); + + String stepPath = "/api/workflow/workflowitems/" + poolTask.getWorkflowItem().getID() + "/step"; + + // anonymous caller must not reach the handler + getClient().perform(get(stepPath)).andExpect(status().isUnauthorized()); + // an authenticated user without READ permission on the workflow item must be blocked + getClient(otherToken).perform(get(stepPath)).andExpect(status().isForbidden()); + // an administrator may read the step + getClient(adminToken).perform(get(stepPath)).andExpect(status().isOk()); + } + +} From f27d7bc07a8276041322ccf47e4f6c43352e5b25 Mon Sep 17 00:00:00 2001 From: matus Date: Mon, 27 Jul 2026 14:55:19 +0200 Subject: [PATCH 3/3] Guard against null workflow item in the WORKFLOWITEM permission evaluator WorkflowRestPermissionEvaluatorPlugin dereferenced the result of workflowItemService.find() without a null check, so an unknown id from an authenticated non-admin threw a NullPointerException (HTTP 500) instead of 404. The sibling PoolTask/ClaimedTask evaluators already guard this case; mirror them by returning true for a null item so the handler can produce the proper 404. Now reachable via the newly authorized /step subresource. Add a regression test asserting an unknown workflowitem /step id yields 404. Co-Authored-By: Claude Opus 4.8 --- .../WorkflowRestPermissionEvaluatorPlugin.java | 4 ++++ .../app/rest/WorkflowStepLinkRepositoryIT.java | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowRestPermissionEvaluatorPlugin.java index 626290fdc3bb..f7084ccaa81d 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowRestPermissionEvaluatorPlugin.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowRestPermissionEvaluatorPlugin.java @@ -81,6 +81,10 @@ public boolean hasDSpacePermission(Authentication authentication, Serializable t } int dsoId = Integer.parseInt(targetId.toString()); XmlWorkflowItem workflowItem = workflowItemService.find(context, dsoId); + // If the workflow item is null then we give permission so we can throw another status code instead + if (workflowItem == null) { + return true; + } // submitter can see their inprogress submission if (ePerson.equals(workflowItem.getSubmitter())) { return true; diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepLinkRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepLinkRepositoryIT.java index 183daff543ce..a5a53750067f 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepLinkRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepLinkRepositoryIT.java @@ -148,4 +148,21 @@ public void workflowItemStepEnforcesAuthorization() throws Exception { getClient(adminToken).perform(get(stepPath)).andExpect(status().isOk()); } + @Test + public void workflowItemStepWithUnknownIdIsNotFound() throws Exception { + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withEmail("lookup-step@example.com").withPassword(password).build(); + + context.restoreAuthSystemState(); + + String token = getAuthToken(ePerson.getEmail(), password); + + // an authenticated (non-admin) user requesting an unknown workflow item id must get 404, not a 500 + // caused by an unchecked null in the WORKFLOWITEM permission evaluator + getClient(token).perform(get("/api/workflow/workflowitems/" + Integer.MAX_VALUE + "/step")) + .andExpect(status().isNotFound()); + } + }