Merged in task/dspace-cris-2023_02_x/DSC-1960 (pull request DSpace#2864)

Task/dspace cris 2023 02 x/DSC-1960

Approved-by: Vincenzo Mecca

dspace-api/src/main/java/org/dspace/app/customurl/service/CustomUrlServiceImpl.java

@@ -132,7 +132,7 @@ public Optional<Item> 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<IndexableObject> indexableObjects = findIndexableObjects(context, discoverQuery);

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")

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);

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<String> 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,

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<ErrorRest> errors = submissionService.uploadFileToInprogressSubmission(context, request, wsi, source,
@@ -214,8 +223,17 @@ 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<Operation> operations = patch.getOperations();
-        WorkflowItemRest wsi = findOne(context, id);
+        WorkflowItemRest wfi = findOne(context, id);
         XmlWorkflowItem source = wis.find(context, id);
+        if (wfi == 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);
 
@@ -224,7 +242,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 + "'");
@@ -268,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) {

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<Operation> 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())));

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<String> getInputFieldsName(DCInputSet inputConfig, String configId) throws DCInputsReaderException {
         List<String> fieldsName = new ArrayList<String>();
         for (DCInput[] row : inputConfig.getFields()) {
@@ -205,7 +201,7 @@ private List<String> 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(), "-"));

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/search/facets").param("query", "title:"))
+                .andExpect(status().isUnprocessableEntity());
+
+        getClient().perform(get("/api/discover/facets/author_editor").param("query", "title:"))
+                .andExpect(status().isBadRequest());
+    }
+
     @Test
     public void discoverFacetsDateTest() throws Exception {
         //We turn off the authorization system in order to create the structure defined below

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

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

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) {