From 41c9da6dcb9c731a62fd32d2bf61e03c88a29d40 Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Thu, 3 Oct 2024 13:12:34 +0200 Subject: [PATCH 01/10] DSC-1960 fix wrong response code for UUIDLookupRestController --- .../app/rest/UUIDLookupRestController.java | 6 ++++ .../app/rest/UUIDLookupRestControllerIT.java | 35 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/UUIDLookupRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/UUIDLookupRestController.java index 40c0a79b97b..cf2d54266a1 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/UUIDLookupRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/UUIDLookupRestController.java @@ -20,6 +20,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.dspace.app.rest.converter.ConverterService; +import org.dspace.app.rest.exception.RESTAuthorizationException; import org.dspace.app.rest.model.DSpaceObjectRest; import org.dspace.app.rest.utils.ContextUtil; import org.dspace.app.rest.utils.DSpaceObjectUtils; @@ -93,6 +94,11 @@ public void getDSObyIdentifier(HttpServletRequest request, DSpaceObject dso = dspaceObjectUtil.findDSpaceObject(context, uuid); if (dso != null) { DSpaceObjectRest dsor = converter.toRest(dso, utils.obtainProjection()); + // if the user cannot access the item the converter will return null + if (dsor == null) { + throw new RESTAuthorizationException( + "The object with uuid " + uuid.toString() + " cannot be accessed"); + } URI link = linkTo(dsor.getController(), dsor.getCategory(), dsor.getTypePlural()).slash(dsor.getId()) .toUri(); response.setStatus(HttpServletResponse.SC_FOUND); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/UUIDLookupRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/UUIDLookupRestControllerIT.java index 8a6debce3ec..010593732df 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/UUIDLookupRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/UUIDLookupRestControllerIT.java @@ -23,11 +23,13 @@ import org.dspace.builder.GroupBuilder; import org.dspace.builder.ItemBuilder; import org.dspace.builder.SiteBuilder; +import org.dspace.builder.WorkspaceItemBuilder; import org.dspace.content.Bitstream; import org.dspace.content.Collection; import org.dspace.content.Community; import org.dspace.content.Item; import org.dspace.content.Site; +import org.dspace.content.WorkspaceItem; import org.dspace.eperson.Group; import org.junit.Ignore; import org.junit.Test; @@ -294,6 +296,39 @@ public void testInvalidUUID() throws Exception { .andExpect(status().isBadRequest()); } + @Test + /** + * Test that a request with a valid uuid related to a object not accessible to + * the current or anonymous user return a 401/403 error code as appropriate + * + * @throws Exception + */ + public void testUnauthorizedUUID() throws Exception { + context.turnOffAuthorisationSystem(); + // We create a community and a collection to get the uuid to lookup + context.setCurrentUser(admin); + Community community = CommunityBuilder.createCommunity(context) + .withName("A Community") + .build(); + + Collection collection = CollectionBuilder.createCollection(context, community) + .withName("A Collection") + .build(); + + WorkspaceItem wi = WorkspaceItemBuilder.createWorkspaceItem(context, collection).build(); + + context.restoreAuthSystemState(); + getClient().perform(get("/api/dso/find?uuid={uuid}", wi.getItem().getID().toString())) + .andExpect(status().isUnauthorized()); + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform(get("/api/dso/find?uuid={uuid}", wi.getItem().getID().toString())) + .andExpect(status().isForbidden()); + // admin should get the redirection + String tokenAdmin = getAuthToken(admin.getEmail(), password); + getClient(tokenAdmin).perform(get("/api/dso/find?uuid={uuid}", wi.getItem().getID().toString())) + .andExpect(status().is3xxRedirection()); + } + @Test @Ignore /** From 42247fdc8395024bfcbf1cc030f15ac357bc27ac Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Thu, 3 Oct 2024 15:20:23 +0200 Subject: [PATCH 02/10] DSC-1960 fix wrong response code for faceting with invalid solr queries --- .../app/rest/DiscoveryRestController.java | 19 ++++++++++++++----- .../repository/DiscoveryRestRepository.java | 3 ++- .../app/rest/DiscoveryRestControllerIT.java | 9 +++++++++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/DiscoveryRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/DiscoveryRestController.java index 947515ca54c..bb674accd30 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/DiscoveryRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/DiscoveryRestController.java @@ -121,13 +121,22 @@ public FacetsResource getFacets(@RequestParam(name = "query", required = false) + ", filters: " + Objects.toString(searchFilters)); } - SearchResultsRest searchResultsRest = discoveryRestRepository - .getAllFacets(query, dsoTypes, dsoScope, configuration, searchFilters); + try { + SearchResultsRest searchResultsRest = discoveryRestRepository + .getAllFacets(query, dsoTypes, dsoScope, configuration, searchFilters); - FacetsResource facetsResource = new FacetsResource(searchResultsRest, page); - halLinkService.addLinks(facetsResource, page); + FacetsResource facetsResource = new FacetsResource(searchResultsRest, page); + halLinkService.addLinks(facetsResource, page); - return facetsResource; + return facetsResource; + } catch (IllegalArgumentException e) { + boolean isParsingException = e.getMessage().contains(SOLR_PARSE_ERROR_CLASS); + if (isParsingException) { + throw new UnprocessableEntityException(e.getMessage()); + } else { + throw e; + } + } } @RequestMapping(method = RequestMethod.GET, value = "/search/objects") diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java index d11a634bc3b..7adf5534264 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DiscoveryRestRepository.java @@ -163,7 +163,7 @@ public FacetResultsRest getFacetObjects(String facetName, String prefix, String } } catch (SearchServiceException e) { log.error("Error while searching with Discovery", e); - //TODO TOM handle search exception + throw new IllegalArgumentException("Error while searching with Discovery: " + e.getMessage()); } FacetResultsRest facetResultsRest = discoverFacetResultsConverter.convert(context, facetName, prefix, query, @@ -200,6 +200,7 @@ public SearchResultsRest getAllFacets(String query, List dsoTypes, Strin } catch (SearchServiceException e) { log.error("Error while searching with Discovery", e); + throw new IllegalArgumentException("Error while searching with Discovery: " + e.getMessage()); } SearchResultsRest searchResultsRest = discoverFacetsConverter.convert(context, query, dsoTypes, diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java index 49ca6ae3f74..1e7eeafe1db 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java @@ -754,6 +754,15 @@ public void discoverFacetsTestWithSimpleQueryAndSearchFilter() throws Exception ; } + @Test + public void discoverFacetsWithInvalidQuery() throws Exception { + getClient().perform(get("/api/discover/facets").param("query", "title:")) + .andExpect(status().isUnprocessableEntity()); + + getClient().perform(get("/api/discover/facets/author_editor").param("query", "title:")) + .andExpect(status().isUnprocessableEntity()); + } + @Test public void discoverFacetsDateTest() throws Exception { //We turn off the authorization system in order to create the structure defined below From 79fa6d494a0a669ee60bacb73f2fbe7603a26694 Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Fri, 4 Oct 2024 14:49:27 +0200 Subject: [PATCH 03/10] DSC-1960 fix test expectation using the right endpoint --- .../java/org/dspace/app/rest/DiscoveryRestControllerIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java index 1e7eeafe1db..ff2ca384be0 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java @@ -756,7 +756,7 @@ public void discoverFacetsTestWithSimpleQueryAndSearchFilter() throws Exception @Test public void discoverFacetsWithInvalidQuery() throws Exception { - getClient().perform(get("/api/discover/facets").param("query", "title:")) + getClient().perform(get("/api/discover/search/facets").param("query", "title:")) .andExpect(status().isUnprocessableEntity()); getClient().perform(get("/api/discover/facets/author_editor").param("query", "title:")) From 81d16eb0ff156367c810dda9cdbd033052e552f2 Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Fri, 4 Oct 2024 19:17:40 +0200 Subject: [PATCH 04/10] DSC-1960 fix return code for patch/delete of not existing workspace and workflow items --- .../WorkflowItemRestRepository.java | 7 ++++-- .../WorkspaceItemRestRepository.java | 9 ++++++++ .../rest/WorkflowItemRestRepositoryIT.java | 22 +++++++++++++++++++ .../rest/WorkspaceItemRestRepositoryIT.java | 21 ++++++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemRestRepository.java index de39ff69fb9..e881d570ab6 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemRestRepository.java @@ -214,7 +214,10 @@ public WorkflowItemRest upload(HttpServletRequest request, String apiCategory, S public void patch(Context context, HttpServletRequest request, String apiCategory, String model, Integer id, Patch patch) throws SQLException, AuthorizeException { List operations = patch.getOperations(); - WorkflowItemRest wsi = findOne(context, id); + WorkflowItemRest wfi = findOne(context, id); + if (wfi == null) { + throw new ResourceNotFoundException("WorkflowItem ID " + id + " not found"); + } XmlWorkflowItem source = wis.find(context, id); this.checkIfEditMetadataAllowedInCurrentStep(context, source); @@ -224,7 +227,7 @@ public void patch(Context context, HttpServletRequest request, String apiCategor String[] path = op.getPath().substring(1).split("/", 3); if (OPERATION_PATH_SECTIONS.equals(path[0])) { String section = path[1]; - submissionService.evaluatePatchToInprogressSubmission(context, request, source, wsi, section, op); + submissionService.evaluatePatchToInprogressSubmission(context, request, source, wfi, section, op); } else { throw new DSpaceBadRequestException( "Patch path operation need to starts with '" + OPERATION_PATH_SECTIONS + "'"); diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkspaceItemRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkspaceItemRestRepository.java index 566ed14f333..3831f1c2efb 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkspaceItemRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkspaceItemRestRepository.java @@ -73,6 +73,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; +import org.springframework.data.rest.webmvc.ResourceNotFoundException; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Component; @@ -261,6 +262,9 @@ public void patch(Context context, HttpServletRequest request, String apiCategor Patch patch) throws SQLException, AuthorizeException { List operations = patch.getOperations(); WorkspaceItemRest wsi = findOne(context, id); + if (wsi == null) { + throw new ResourceNotFoundException(apiCategory + "." + model + " with id: " + id + " not found"); + } WorkspaceItem source = wis.find(context, id); for (Operation op : operations) { //the value in the position 0 is a null value @@ -286,6 +290,11 @@ protected void delete(Context context, Integer id) throws AuthorizeException { WorkspaceItem witem = null; try { witem = wis.find(context, id); + if (witem == null) { + throw new ResourceNotFoundException( + WorkspaceItemRest.CATEGORY + "." + WorkspaceItemRest.NAME + + " with id: " + id + " not found"); + } wis.deleteAll(context, witem); context.addEvent(new Event(Event.DELETE, Constants.ITEM, witem.getItem().getID(), null, itemService.getIdentifiers(context, witem.getItem()))); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowItemRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowItemRestRepositoryIT.java index ea4ed736a12..8814689fed1 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowItemRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowItemRestRepositoryIT.java @@ -686,6 +686,10 @@ public void deleteOneTest() throws Exception { getClient(token).perform(delete("/api/workflow/workflowitems/" + witem.getID())) .andExpect(status().is(204)); + // Delete the workflowitem a second time should result in 404 + getClient(token).perform(delete("/api/workflow/workflowitems/" + witem.getID())) + .andExpect(status().is(404)); + // Trying to get deleted workflowitem should fail with 404 getClient(token).perform(get("/api/workflow/workflowitems/" + witem.getID())) .andExpect(status().is(404)); @@ -1034,6 +1038,24 @@ public void stepAndGlobalValidationErrorsTest() throws Exception { contains(hasJsonPath("$.paths[1]", is("/sections/traditionalpageone/dc.identifier.doi")))))); } + @Test + public void patchNotExistingWorkflowItemTest() throws Exception { + List update = new ArrayList(); + List> values = new ArrayList<>(); + Map value = new HashMap(); + value.put("value", "Title"); + values.add(value); + update.add(new AddOperation("/sections/traditionalpageone/dc.title", values)); + + String patchBody = getPatchContent(update); + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(patch("/api/workflow/workflowitems/" + Integer.MAX_VALUE) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isNotFound()); + + } + @Test /** * Test the update of metadata diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkspaceItemRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkspaceItemRestRepositoryIT.java index 248915a8544..30d74129506 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkspaceItemRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkspaceItemRestRepositoryIT.java @@ -623,6 +623,9 @@ public void deleteOneTest() throws Exception { getClient(token).perform(delete("/api/submission/workspaceitems/" + witem.getID())) .andExpect(status().is(204)); + // a second attempt should return 404 + getClient(token).perform(delete("/api/submission/workspaceitems/" + witem.getID())) + .andExpect(status().is(404)); //Trying to get deleted item should fail with 404 getClient(token).perform(get("/api/submission/workspaceitems/" + witem.getID())) .andExpect(status().is(404)); @@ -2507,6 +2510,24 @@ public void globalValidationErrorsTest() throws Exception { contains(hasJsonPath("$.paths[1]", is("/sections/traditionalpageone/dc.identifier.doi")))))); } + @Test + public void patchNotExistingWorkspaceItemTest() throws Exception { + List update = new ArrayList(); + List> values = new ArrayList<>(); + Map value = new HashMap(); + value.put("value", "Title"); + values.add(value); + update.add(new AddOperation("/sections/traditionalpageone/dc.title", values)); + + String patchBody = getPatchContent(update); + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(patch("/api/submission/workspaceitems/" + Integer.MAX_VALUE) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isNotFound()); + + } + /** * Test the update of metadata for fields configured with type-bind * From 56d379cbcc87d9aef07a20aaf301383dd86c4dbb Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Sat, 5 Oct 2024 20:05:23 +0200 Subject: [PATCH 05/10] DSC-1960 fix error code response for WorkflowItem endpoint --- .../WorkflowItemRestRepository.java | 23 ++++++++++++++--- .../app/rest/TaskRestRepositoriesIT.java | 25 +++++++++++++------ .../rest/WorkflowItemRestRepositoryIT.java | 4 +-- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemRestRepository.java index e881d570ab6..2e80ba85216 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/WorkflowItemRestRepository.java @@ -196,6 +196,15 @@ public WorkflowItemRest upload(HttpServletRequest request, String apiCategory, S Context context = obtainContext(); WorkflowItemRest wsi = findOne(context, id); XmlWorkflowItem source = wis.find(context, id); + if (wsi == null) { + // we need to check the source as the rest resource could be null due to lack of authorization permission + if (source == null) { + throw new ResourceNotFoundException("WorkflowItem ID " + id + " not found"); + } else { + throw new RESTAuthorizationException( + "WorkflowItem ID " + id + " cannot be accessed by the current user"); + } + } this.checkIfEditMetadataAllowedInCurrentStep(context, source); List errors = submissionService.uploadFileToInprogressSubmission(context, request, wsi, source, @@ -215,10 +224,16 @@ public void patch(Context context, HttpServletRequest request, String apiCategor Patch patch) throws SQLException, AuthorizeException { List operations = patch.getOperations(); WorkflowItemRest wfi = findOne(context, id); + XmlWorkflowItem source = wis.find(context, id); if (wfi == null) { - throw new ResourceNotFoundException("WorkflowItem ID " + id + " not found"); + // we need to check the source as the rest resource could be null due to lack of authorization permission + if (source == null) { + throw new ResourceNotFoundException("WorkflowItem ID " + id + " not found"); + } else { + throw new RESTAuthorizationException( + "WorkflowItem ID " + id + " cannot be accessed by the current user"); + } } - XmlWorkflowItem source = wis.find(context, id); this.checkIfEditMetadataAllowedInCurrentStep(context, source); @@ -271,14 +286,14 @@ private void checkIfEditMetadataAllowedInCurrentStep(Context context, XmlWorkflo ClaimedTask claimedTask = claimedTaskService.findByWorkflowIdAndEPerson(context, xmlWorkflowItem, context.getCurrentUser()); if (claimedTask == null) { - throw new UnprocessableEntityException("WorkflowItem with id " + xmlWorkflowItem.getID() + throw new RESTAuthorizationException("WorkflowItem with id " + xmlWorkflowItem.getID() + " has not been claimed yet."); } Workflow workflow = workflowFactory.getWorkflow(claimedTask.getWorkflowItem().getCollection()); Step step = workflow.getStep(claimedTask.getStepID()); WorkflowActionConfig currentActionConfig = step.getActionConfig(claimedTask.getActionID()); if (!currentActionConfig.getProcessingAction().getOptions().contains(SUBMIT_EDIT_METADATA)) { - throw new UnprocessableEntityException(SUBMIT_EDIT_METADATA + " is not a valid option on this " + + throw new RESTAuthorizationException(SUBMIT_EDIT_METADATA + " is not a valid option on this " + "action (" + currentActionConfig.getProcessingAction().getClass() + ")."); } } catch (SQLException e) { diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/TaskRestRepositoriesIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/TaskRestRepositoriesIT.java index a9b5c6a582b..2e352489c27 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/TaskRestRepositoriesIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/TaskRestRepositoriesIT.java @@ -2656,7 +2656,7 @@ public void unclaimedTaskTest_Upload_EditMetadataOptionAllowed() throws Exceptio bibtex); getClient(reviewer1Token).perform(multipart("/api/workflow/workflowitems/" + witem.getID()) .file(bibtexFile)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); bibtex.close(); } @@ -2695,7 +2695,7 @@ public void unclaimedTaskTest_Patch_EditMetadataOptionAllowed() throws Exception context.setCurrentUser(submitter); - //3. create a workflowitem (so a pool task in step1) + //3. create a workflowitem (so a pool task in step2 as step1 is undefined) XmlWorkflowItem witem = WorkflowItemBuilder.createWorkflowItem(context, col1) .withTitle("Test item full workflow") .withIssueDate("2019-03-06") @@ -2725,9 +2725,6 @@ public void unclaimedTaskTest_Patch_EditMetadataOptionAllowed() throws Exception .andDo((result -> idRef .set(read(result.getResponse().getContentAsString(), "$._embedded.pooltasks[0].id")))); - // try to patch a workspace item while it is unclaimed - String authToken = getAuthToken(eperson.getEmail(), password); - // a simple patch to update an existent metadata List updateTitle = new ArrayList<>(); Map value = new HashMap<>(); @@ -2736,10 +2733,17 @@ public void unclaimedTaskTest_Patch_EditMetadataOptionAllowed() throws Exception String patchBody = getPatchContent(updateTitle); + getClient(reviewer2Token).perform(patch("/api/workflow/workflowitems/" + witem.getID()) + .content(patchBody) + .contentType(javax.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isForbidden()); + + // try to patch with an unrelated eperson while it is unclaimed + String authToken = getAuthToken(eperson.getEmail(), password); getClient(authToken).perform(patch("/api/workflow/workflowitems/" + witem.getID()) .content(patchBody) .contentType(javax.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); } @Test @@ -2797,7 +2801,7 @@ public void uploadTest_ClaimedTask_EditMetadataOptionNotAllowed() throws Excepti bibtex); getClient(authToken).perform(multipart("/api/workflow/workflowitems/" + witem.getID()) .file(bibtexFile)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); bibtex.close(); } @@ -2890,10 +2894,15 @@ public void patchTest_ClaimedTask_EditMetadataOptionNotAllowed() throws Exceptio String patchBody = getPatchContent(updateTitle); + getClient(reviewer1Token).perform(patch("/api/workflow/workflowitems/" + witem.getID()) + .content(patchBody) + .contentType(javax.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isForbidden()); + getClient(authToken).perform(patch("/api/workflow/workflowitems/" + witem.getID()) .content(patchBody) .contentType(javax.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); } @Test diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowItemRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowItemRestRepositoryIT.java index 8814689fed1..d472d78c128 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowItemRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowItemRestRepositoryIT.java @@ -77,7 +77,6 @@ import org.dspace.xmlworkflow.storedcomponents.XmlWorkflowItem; import org.hamcrest.Matchers; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.rest.webmvc.RestMediaTypes; @@ -1126,7 +1125,6 @@ public void patchUpdateMetadataTest() throws Exception { } @Test - @Ignore(value = "This demonstrate the bug logged in DS-4179") /** * Verify that update of metadata is forbidden in step 1. * @@ -1181,7 +1179,7 @@ public void patchUpdateMetadataStep1ForbiddenTest() throws Exception { .andExpect(status().isOk()) .andExpect(jsonPath("$", Matchers.is(WorkflowItemMatcher.matchItemWithTitleAndDateIssuedAndSubject(witem, - "New Title", "2017-10-17", "ExtraEntry")))) + "Workflow Item 1", "2017-10-17", "ExtraEntry")))) ; } From f7658df5b4f215fb840c84d808efe0f569885ca6 Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Sun, 6 Oct 2024 10:26:27 +0200 Subject: [PATCH 06/10] DSC-1960 update expectation for forbidden call in supervisor test --- .../org/dspace/app/rest/SupervisionOrderRestRepositoryIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/SupervisionOrderRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/SupervisionOrderRestRepositoryIT.java index fbc707d6a49..f11b96b7487 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/SupervisionOrderRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/SupervisionOrderRestRepositoryIT.java @@ -1413,7 +1413,7 @@ public void createSupervisionOnWorkspaceThenSubmitToWorkflowTest() throws Except getClient(authTokenA).perform(patch("/api/workflow/workflowitems/" + idRef.get()) .content(patchBody) .contentType(contentType)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); } finally { if (idRef.get() != null) { From f0a6dce98e11724b02c0c30dddd13dc4f8b99b12 Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Mon, 7 Oct 2024 15:13:03 +0200 Subject: [PATCH 07/10] DSC-1960 escape custom url before searching solr --- .../dspace/app/customurl/service/CustomUrlServiceImpl.java | 2 +- .../test/java/org/dspace/app/rest/ItemRestRepositoryIT.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dspace-api/src/main/java/org/dspace/app/customurl/service/CustomUrlServiceImpl.java b/dspace-api/src/main/java/org/dspace/app/customurl/service/CustomUrlServiceImpl.java index 9c9207f17c2..cceaa005f86 100644 --- a/dspace-api/src/main/java/org/dspace/app/customurl/service/CustomUrlServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/app/customurl/service/CustomUrlServiceImpl.java @@ -132,7 +132,7 @@ public Optional findItemByCustomUrl(Context context, String customUrl) { DiscoverQuery discoverQuery = new DiscoverQuery(); discoverQuery.addDSpaceObjectFilter(IndexableItem.TYPE); - discoverQuery.addFilterQueries("customurl:" + customUrl); + discoverQuery.addFilterQueries("customurl:" + searchService.escapeQueryChars(customUrl)); discoverQuery.setIncludeNotDiscoverableOrWithdrawn(true); List indexableObjects = findIndexableObjects(context, discoverQuery); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ItemRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ItemRestRepositoryIT.java index 576965e2d66..b5584f29512 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ItemRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ItemRestRepositoryIT.java @@ -5283,6 +5283,10 @@ public void testSearchItemByCustomUrlWithoutResult() throws Exception { .param("q", UUID.randomUUID().toString())) .andExpect(status().isNoContent()); + getClient(token).perform(get("/api/core/items/search/findByCustomURL") + .param("q", "http://example.com/sample")) + .andExpect(status().isNoContent()); + } @Test From 9255c0bda46e4483f602ed3b65fc50267f5c4b98 Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Fri, 11 Oct 2024 13:57:05 +0200 Subject: [PATCH 08/10] DSC-1960 reduce noice in log file --- .../org/dspace/app/rest/submit/step/DescribeStep.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/submit/step/DescribeStep.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/submit/step/DescribeStep.java index 98e62da012b..78b653820a3 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/submit/step/DescribeStep.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/submit/step/DescribeStep.java @@ -100,7 +100,7 @@ private void readField(InProgressSubmission obj, SubmissionStepConfig config, Da } } else if (StringUtils.equalsIgnoreCase(input.getInputType(), "group") || StringUtils.equalsIgnoreCase(input.getInputType(), "inline-group")) { - log.info("Called child form:" + config.getId() + "-" + + log.debug("Called child form:" + config.getId() + "-" + Utils.standardize(input.getSchema(), input.getElement(), input.getQualifier(), "-")); DCInputSet inputConfigChild = inputReader.getInputsByFormName(config.getId() + "-" + Utils .standardize(input.getSchema(), input.getElement(), input.getQualifier(), "-")); @@ -191,10 +191,6 @@ public void doPatchProcessing(Context context, HttpServletRequest currentRequest } } - private boolean isFromVocabulary(DCInput dcInput) { - return StringUtils.isNotBlank(dcInput.getVocabulary()); - } - private List getInputFieldsName(DCInputSet inputConfig, String configId) throws DCInputsReaderException { List fieldsName = new ArrayList(); for (DCInput[] row : inputConfig.getFields()) { @@ -205,7 +201,7 @@ private List getInputFieldsName(DCInputSet inputConfig, String configId) } } else if (StringUtils.equalsIgnoreCase(input.getInputType(), "group") || StringUtils.equalsIgnoreCase(input.getInputType(), "inline-group")) { - log.info("Called child form:" + configId + "-" + + log.debug("Called child form:" + configId + "-" + Utils.standardize(input.getSchema(), input.getElement(), input.getQualifier(), "-")); DCInputSet inputConfigChild = inputReader.getInputsByFormName(configId + "-" + Utils .standardize(input.getSchema(), input.getElement(), input.getQualifier(), "-")); From f58cc009ee6c7d7cbfe9f83ced52a4dac3841f6e Mon Sep 17 00:00:00 2001 From: Vincenzo Mecca Date: Mon, 14 Oct 2024 09:46:02 +0200 Subject: [PATCH 09/10] [DSC-1960] Fixes IT failures of SubmissionDeduplicationRestIT --- .../rest/SubmissionDeduplicationRestIT.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/SubmissionDeduplicationRestIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/SubmissionDeduplicationRestIT.java index b47e543aee1..01c31403b31 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/SubmissionDeduplicationRestIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/SubmissionDeduplicationRestIT.java @@ -986,7 +986,7 @@ public void workflowItemsAndItemTest() throws Exception { getClient(submitterTocken) .perform(patch("/api/workflow/workflowitems/" + witem.getID()).content(patchBody) .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); // execute the patch getClient(reviewerToken) @@ -1054,7 +1054,7 @@ public void workflowItemsAndItemTest() throws Exception { getClient(submitterTocken) .perform(patch("/api/workflow/workflowitems/" + witem.getID()).content(patchBody) .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); // patch operation getClient(reviewerToken) @@ -1216,7 +1216,7 @@ public void workflowItemsAndWorkspaceItemTest() throws Exception { // check security getClient(submitterToken).perform(patch("/api/workflow/workflowitems/" + witem.getID()).content(patchBody) - .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)).andExpect(status().isUnprocessableEntity()); + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)).andExpect(status().isForbidden()); // make patch getClient(reviewerToken) @@ -1289,7 +1289,7 @@ public void workflowItemsAndWorkspaceItemTest() throws Exception { getClient(submitterToken) .perform(patch("/api/workflow/workflowitems/" + witem.getID()).content(patchBody) .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); getClient(reviewerToken) .perform(patch("/api/workflow/workflowitems/" + witem.getID()) @@ -1454,7 +1454,7 @@ public void workflowItemCheckFailures() throws Exception { getClient(authToken) .perform(patch("/api/workflow/workflowitems/" + witem.getID()).content(patchBody) .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); // Ask for a patch with a number as UUID patchBody = null; @@ -1466,7 +1466,7 @@ public void workflowItemCheckFailures() throws Exception { getClient(authToken) .perform(patch("/api/workflow/workflowitems/" + witem.getID()).content(patchBody) .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); // Ask for a patch with an invalid operation value.clear(); @@ -1481,7 +1481,7 @@ public void workflowItemCheckFailures() throws Exception { getClient(authToken) .perform(patch("/api/workflow/workflowitems/" + witem.getID()).content(patchBody) .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); // Ask for a patch with wrong type value.clear(); @@ -1496,7 +1496,7 @@ public void workflowItemCheckFailures() throws Exception { getClient(authToken) .perform(patch("/api/workflow/workflowitems/" + witem.getID()).content(patchBody) .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); // Ask for a patch with the wrong decision type value.clear(); @@ -1510,7 +1510,7 @@ public void workflowItemCheckFailures() throws Exception { getClient(authToken) .perform(patch("/api/workflow/workflowitems/" + witem.getID()).content(patchBody) .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isForbidden()); } @Test From 2c537cc19603464a94190fc1615a8d685aadf2dd Mon Sep 17 00:00:00 2001 From: Vincenzo Mecca Date: Mon, 14 Oct 2024 11:11:09 +0200 Subject: [PATCH 10/10] [DSC-1960] Fixes DiscoveryRestControllerIT --- .../java/org/dspace/app/rest/DiscoveryRestControllerIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java index ff2ca384be0..3a40e14b0df 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java @@ -760,7 +760,7 @@ public void discoverFacetsWithInvalidQuery() throws Exception { .andExpect(status().isUnprocessableEntity()); getClient().perform(get("/api/discover/facets/author_editor").param("query", "title:")) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isBadRequest()); } @Test