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 9ee277171e8..6e123dde784 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 6e7f4f84ace..80701022872 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 30aac1579c7..308e5fd2a87 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, 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 626290fdc3b..f7084ccaa81 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 new file mode 100644 index 00000000000..a5a53750067 --- /dev/null +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepLinkRepositoryIT.java @@ -0,0 +1,168 @@ +/** + * 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()); + } + + @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()); + } + +}