Merge pull request #2726 from atmire/w2p-69976_subresource-permissions

Subresources should obey access restrictions
DSpace · May 22, 2020 · 19ba3dd · 19ba3dd
2 parents 1376563 + 68b91aa
commit 19ba3dd
Show file tree

Hide file tree

Showing 87 changed files with 1,776 additions and 178 deletions.
diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/RelationshipTypeRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/RelationshipTypeRestController.java
@@ -81,7 +81,7 @@ public PagedModel<RelationshipTypeResource> retrieve(@PathVariable Integer id,
         List<RelationshipType> list = relationshipTypeService.findByEntityType(context, entityType, -1, -1);
 
         Page<RelationshipTypeRest> relationshipTypeRestPage = converter
-            .toRestPage(list, pageable, list.size(), utils.obtainProjection());
+            .toRestPage(list, pageable, utils.obtainProjection());
 
         Page<RelationshipTypeResource> relationshipTypeResources = relationshipTypeRestPage
             .map(relationshipTypeRest -> new RelationshipTypeResource(relationshipTypeRest, utils));

diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/ScriptProcessesController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/ScriptProcessesController.java
@@ -14,6 +14,9 @@
 import org.dspace.app.rest.model.ScriptRest;
 import org.dspace.app.rest.model.hateoas.ProcessResource;
 import org.dspace.app.rest.repository.ScriptRestRepository;
+import org.dspace.app.rest.utils.ContextUtil;
+import org.dspace.core.Context;
+import org.dspace.services.RequestService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.rest.webmvc.ControllerUtils;
 import org.springframework.hateoas.RepresentationModel;
@@ -41,6 +44,9 @@ public class ScriptProcessesController {
     @Autowired
     private ScriptRestRepository scriptRestRepository;
 
+    @Autowired
+    private RequestService requestService;
+
     /**
      * This method can be called by sending a POST request to the system/scripts/{name}/processes endpoint
      * This will start a process for the script that matches the given name
@@ -55,8 +61,10 @@ public ResponseEntity<RepresentationModel<?>> startProcess(@PathVariable(name =
         if (log.isTraceEnabled()) {
             log.trace("Starting Process for Script with name: " + scriptName);
         }
-        ProcessRest processRest = scriptRestRepository.startProcess(scriptName);
+        Context context = ContextUtil.obtainContext(requestService.getCurrentRequest().getServletRequest());
+        ProcessRest processRest = scriptRestRepository.startProcess(context, scriptName);
         ProcessResource processResource = converter.toResource(processRest);
+        context.complete();
         return ControllerUtils.toResponseEntity(HttpStatus.ACCEPTED, new HttpHeaders(), processResource);
     }
 

diff --git a/...webapp/src/main/java/org/dspace/app/rest/WorkflowDefinitionCollectionsLinkRepository.java b/...webapp/src/main/java/org/dspace/app/rest/WorkflowDefinitionCollectionsLinkRepository.java
@@ -72,7 +72,7 @@ public Page<CollectionRest> getCollections(@Nullable HttpServletRequest request,
             collectionsMappedToWorkflow.addAll(xmlWorkflowFactory.getCollectionHandlesMappedToWorklow(context,
                 workflowName));
             Pageable pageable = optionalPageable != null ? optionalPageable : PageRequest.of(0, 20);
-            return converter.toRestPage(utils.getPage(collectionsMappedToWorkflow, pageable),
+            return converter.toRestPage(collectionsMappedToWorkflow, pageable,
                 projection);
         } else {
             throw new ResourceNotFoundException("No workflow with name " + workflowName + " is configured");

diff --git a/...erver-webapp/src/main/java/org/dspace/app/rest/WorkflowDefinitionStepsLinkRepository.java b/...erver-webapp/src/main/java/org/dspace/app/rest/WorkflowDefinitionStepsLinkRepository.java
@@ -55,7 +55,7 @@ public Page<WorkflowStepRest> getSteps(@Nullable HttpServletRequest request,
         try {
             List<Step> steps = xmlWorkflowFactory.getWorkflowByName(workflowName).getSteps();
             Pageable pageable = optionalPageable != null ? optionalPageable : PageRequest.of(0, 20);
-            return converter.toRestPage(utils.getPage(steps, pageable), projection);
+            return converter.toRestPage(steps, pageable, projection);
         } catch (WorkflowConfigurationException e) {
             throw new ResourceNotFoundException("No workflow with name " + workflowName + " is configured");
         }

diff --git a/...ce-server-webapp/src/main/java/org/dspace/app/rest/WorkflowStepActionsLinkRepository.java b/...ce-server-webapp/src/main/java/org/dspace/app/rest/WorkflowStepActionsLinkRepository.java
@@ -52,6 +52,6 @@ public Page<WorkflowActionRest> getActions(@Nullable HttpServletRequest request,
                                                Projection projection) {
         List<WorkflowActionConfig> actions = xmlWorkflowFactory.getStepByName(workflowStepName).getActions();
         Pageable pageable = optionalPageable != null ? optionalPageable : PageRequest.of(0, 20);
-        return converter.toRestPage(utils.getPage(actions, pageable), projection);
+        return converter.toRestPage(actions, pageable, projection);
     }
 }
diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java
@@ -10,6 +10,7 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.util.HashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -19,11 +20,13 @@
 import org.apache.log4j.Logger;
 import org.dspace.app.rest.link.HalLinkFactory;
 import org.dspace.app.rest.link.HalLinkService;
+import org.dspace.app.rest.model.BaseObjectRest;
 import org.dspace.app.rest.model.RestAddressableModel;
 import org.dspace.app.rest.model.RestModel;
 import org.dspace.app.rest.model.hateoas.HALResource;
 import org.dspace.app.rest.projection.DefaultProjection;
 import org.dspace.app.rest.projection.Projection;
+import org.dspace.app.rest.security.DSpacePermissionEvaluator;
 import org.dspace.app.rest.utils.Utils;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.config.BeanDefinition;
@@ -34,6 +37,7 @@
 import org.springframework.data.domain.Pageable;
 import org.springframework.hateoas.EntityModel;
 import org.springframework.hateoas.Link;
+import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Component;
 import org.springframework.stereotype.Service;
 
@@ -64,6 +68,9 @@ public class ConverterService {
     @Autowired
     private List<Projection> projections;
 
+    @Autowired
+    private DSpacePermissionEvaluator dSpacePermissionEvaluator;
+
     /**
      * Converts the given model object to a rest object, using the appropriate {@link DSpaceConverter} and
      * the given projection.
@@ -86,6 +93,14 @@ public <M, R> R toRest(M modelObject, Projection projection) {
         M transformedModel = projection.transformModel(modelObject);
         DSpaceConverter<M, R> converter = requireConverter(modelObject.getClass());
         R restObject = converter.convert(transformedModel, projection);
+        if (restObject instanceof BaseObjectRest) {
+            if (!dSpacePermissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(),
+                                                         restObject, "READ")) {
+                log.debug("Access denied on " + restObject.getClass() + " with id: " +
+                              ((BaseObjectRest) restObject).getId());
+                return null;
+            }
+        }
         if (restObject instanceof RestModel) {
             return (R) projection.transformRest((RestModel) restObject);
         }
@@ -97,33 +112,55 @@ public <M, R> R toRest(M modelObject, Projection projection) {
      *
      * @param modelObjects the list of model objects.
      * @param pageable the pageable.
-     * @param total the total number of items.
      * @param projection the projection to use.
      * @param <M> the model object class.
      * @param <R> the rest object class.
      * @return the page.
      * @throws IllegalArgumentException if there is no compatible converter.
      * @throws ClassCastException if the converter's return type is not compatible with the inferred return type.
      */
-    public <M, R> Page<R> toRestPage(List<M> modelObjects, Pageable pageable, long total, Projection projection) {
-        return new PageImpl<>(modelObjects, pageable, total).map((object) -> toRest(object, projection));
+    public <M, R> Page<R> toRestPage(List<M> modelObjects, Pageable pageable, Projection projection) {
+        List<R> transformedList = new LinkedList<>();
+        for (M modelObject : modelObjects) {
+            R transformedObject = toRest(modelObject, projection);
+            if (transformedObject != null) {
+                transformedList.add(transformedObject);
+            }
+        }
+        if (pageable == null) {
+            pageable = utils.getPageable(pageable);
+        }
+        return utils.getPage(transformedList, pageable);
     }
 
     /**
-     * Converts a list of model objects to a page of rest objects using the given {@link Projection}.
-     *
-     * @param modelObjects the page of model objects.
+     * Converts a list of ModelObjects to a page of Rest Objects using the given {@link Projection}
+     * This method differences in the sense that we define a total here instead of the size of the list because
+     * this method will be called if the list is limited through a DB call already and thus we need to give the
+     * total amount of records in the DB; not the size of the given list
+     * @param modelObjects the list of model objects.
+     * @param pageable the pageable.
+     * @param total The total amount of objects
      * @param projection the projection to use.
      * @param <M> the model object class.
      * @param <R> the rest object class.
      * @return the page.
-     * @throws IllegalArgumentException if there is no compatible converter.
-     * @throws ClassCastException if the converter's return type is not compatible with the inferred return type.
      */
-    public <M, R> Page<R> toRestPage(Page<M> modelObjects, Projection projection) {
-        return modelObjects.map((object) -> toRest(object, projection));
+    public <M, R> Page<R> toRestPage(List<M> modelObjects, Pageable pageable, long total, Projection projection) {
+        List<R> transformedList = new LinkedList<>();
+        for (M modelObject : modelObjects) {
+            R transformedObject = toRest(modelObject, projection);
+            if (transformedObject != null) {
+                transformedList.add(transformedObject);
+            }
+        }
+        if (pageable == null) {
+            pageable = utils.getPageable(pageable);
+        }
+        return new PageImpl(transformedList, pageable, total);
     }
 
+
     /**
      * Gets the converter supporting the given class as input.
      *
@@ -177,6 +214,9 @@ public <T extends HALResource> T toResource(RestModel restObject) {
      * @return the fully converted resource, with all automatic links and embeds applied.
      */
     public <T extends HALResource> T toResource(RestModel restObject, Link... oldLinks) {
+        if (restObject == null) {
+            return null;
+        }
         T halResource = getResource(restObject);
         if (restObject instanceof RestAddressableModel) {
             utils.embedOrLinkClassLevelRels(halResource, oldLinks);

diff --git a/...bapp/src/main/java/org/dspace/app/rest/repository/AuthorizationFeatureRestRepository.java b/...bapp/src/main/java/org/dspace/app/rest/repository/AuthorizationFeatureRestRepository.java
@@ -45,8 +45,7 @@ public Class<AuthorizationFeatureRest> getDomainClass() {
     @PreAuthorize("hasAuthority('ADMIN')")
     @Override
     public Page<AuthorizationFeatureRest> findAll(Context context, Pageable pageable) {
-        return converter.toRestPage(utils.getPage(authorizationFeatureService.findAll(),
-                pageable), utils.obtainProjection());
+        return converter.toRestPage(authorizationFeatureService.findAll(), pageable, utils.obtainProjection());
     }
 
     @PreAuthorize("hasAuthority('ADMIN')")
@@ -64,6 +63,6 @@ public AuthorizationFeatureRest findOne(Context context, String id) {
     public Page<AuthorizationFeatureRest> findByResourceType(@Parameter(value = "type", required = true) String type,
             Pageable pageable) {
         List<AuthorizationFeature> foundFeatures = authorizationFeatureService.findByResourceType(type);
-        return converter.toRestPage(utils.getPage(foundFeatures, pageable), utils.obtainProjection());
+        return converter.toRestPage(foundFeatures, pageable, utils.obtainProjection());
     }
 }
diff --git a/...rver-webapp/src/main/java/org/dspace/app/rest/repository/AuthorizationRestRepository.java b/...rver-webapp/src/main/java/org/dspace/app/rest/repository/AuthorizationRestRepository.java
@@ -174,7 +174,7 @@ public Page<AuthorizationRest> findByObject(@Parameter(value = "uri", required =
             // restore the real current user
             context.restoreContextUser();
         }
-        return converter.toRestPage(utils.getPage(authorizations, pageable), utils.obtainProjection());
+        return converter.toRestPage(authorizations, pageable, utils.obtainProjection());
     }
 
     /**

diff --git a/...er-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamFormatRestRepository.java b/...er-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamFormatRestRepository.java
@@ -42,6 +42,7 @@ public class BitstreamFormatRestRepository extends DSpaceRestRepository<Bitstrea
     BitstreamFormatService bitstreamFormatService;
 
     @Override
+    @PreAuthorize("permitAll()")
     public BitstreamFormatRest findOne(Context context, Integer id) {
         BitstreamFormat bit = null;
         try {
@@ -59,7 +60,7 @@ public BitstreamFormatRest findOne(Context context, Integer id) {
     public Page<BitstreamFormatRest> findAll(Context context, Pageable pageable) {
         try {
             List<BitstreamFormat> bit = bitstreamFormatService.findAll(context);
-            return converter.toRestPage(utils.getPage(bit, pageable), utils.obtainProjection());
+            return converter.toRestPage(bit, pageable, utils.obtainProjection());
         } catch (SQLException e) {
             throw new RuntimeException(e.getMessage(), e);
         }

diff --git a/...server-webapp/src/main/java/org/dspace/app/rest/repository/BrowseIndexRestRepository.java b/...server-webapp/src/main/java/org/dspace/app/rest/repository/BrowseIndexRestRepository.java
@@ -16,6 +16,7 @@
 import org.dspace.core.Context;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
+import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.stereotype.Component;
 
 /**
@@ -27,6 +28,7 @@
 public class BrowseIndexRestRepository extends DSpaceRestRepository<BrowseIndexRest, String> {
 
     @Override
+    @PreAuthorize("permitAll()")
     public BrowseIndexRest findOne(Context context, String name) {
         BrowseIndexRest bi = null;
         BrowseIndex bix;

diff --git a/...er-webapp/src/main/java/org/dspace/app/rest/repository/BundleBitstreamLinkRepository.java b/...er-webapp/src/main/java/org/dspace/app/rest/repository/BundleBitstreamLinkRepository.java
@@ -15,7 +15,6 @@
 import org.dspace.app.rest.model.BitstreamRest;
 import org.dspace.app.rest.model.BundleRest;
 import org.dspace.app.rest.projection.Projection;
-import org.dspace.content.Bitstream;
 import org.dspace.content.Bundle;
 import org.dspace.content.service.BitstreamService;
 import org.dspace.content.service.BundleService;
@@ -52,8 +51,7 @@ public Page<BitstreamRest> getBitstreams(@Nullable HttpServletRequest request,
                 throw new ResourceNotFoundException("No such bundle: " + bundleId);
             }
             Pageable pageable = utils.getPageable(optionalPageable);
-            Page<Bitstream> page = utils.getPage(bundle.getBitstreams(), pageable);
-            return converter.toRestPage(page, projection);
+            return converter.toRestPage(bundle.getBitstreams(), pageable, projection);
         } catch (SQLException e) {
             throw new RuntimeException(e);
         }

diff --git a/...server-webapp/src/main/java/org/dspace/app/rest/repository/ClaimedTaskRestRepository.java b/...server-webapp/src/main/java/org/dspace/app/rest/repository/ClaimedTaskRestRepository.java
@@ -108,7 +108,7 @@ public Page<ClaimedTaskRest> findByUser(@Parameter(value = "uuid", required = tr
             if (authorizeService.isAdmin(context) || userID.equals(currentUser.getID())) {
                 EPerson ep = epersonService.find(context, userID);
                 List<ClaimedTask> tasks = claimedTaskService.findByEperson(context, ep);
-                return converter.toRestPage(utils.getPage(tasks, pageable), utils.obtainProjection());
+                return converter.toRestPage(tasks, pageable, utils.obtainProjection());
             } else {
                 throw new RESTAuthorizationException("Only administrators can search for claimed tasks of other users");
             }

diff --git a/.../main/java/org/dspace/app/rest/repository/CollectionBitstreamReadGroupLinkRepository.java b/.../main/java/org/dspace/app/rest/repository/CollectionBitstreamReadGroupLinkRepository.java
@@ -72,6 +72,9 @@ public GroupRest getBitstreamReadGroup(@Nullable HttpServletRequest request,
             }
             List<Group> bitstreamGroups = authorizeService
                 .getAuthorizedGroups(context, collection, Constants.DEFAULT_BITSTREAM_READ);
+            if (bitstreamGroups == null || bitstreamGroups.isEmpty()) {
+                return null;
+            }
             Group bitstreamReadGroup = bitstreamGroups.get(0);
 
             if (bitstreamReadGroup == null) {

diff --git a/...-server-webapp/src/main/java/org/dspace/app/rest/repository/CollectionRestRepository.java b/...-server-webapp/src/main/java/org/dspace/app/rest/repository/CollectionRestRepository.java
@@ -175,7 +175,7 @@ public Page<CollectionRest> findAuthorizedByCommunity(
                         + " not found");
             }
             List<Collection> collections = cs.findAuthorized(context, com, Constants.ADD);
-            return converter.toRestPage(utils.getPage(collections, pageable), utils.obtainProjection());
+            return converter.toRestPage(collections, pageable, utils.obtainProjection());
         } catch (SQLException e) {
             throw new RuntimeException(e.getMessage(), e);
         }
@@ -186,7 +186,7 @@ public Page<CollectionRest> findAuthorized(Pageable pageable) {
         try {
             Context context = obtainContext();
             List<Collection> collections = cs.findAuthorizedOptimized(context, Constants.ADD);
-            return converter.toRestPage(utils.getPage(collections, pageable), utils.obtainProjection());
+            return converter.toRestPage(collections, pageable, utils.obtainProjection());
         } catch (SQLException e) {
             throw new RuntimeException(e.getMessage(), e);
         }

diff --git a/...e-server-webapp/src/main/java/org/dspace/app/rest/repository/CommunityRestRepository.java b/...e-server-webapp/src/main/java/org/dspace/app/rest/repository/CommunityRestRepository.java
@@ -201,7 +201,7 @@ public Page<CommunityRest> findAll(Context context, Pageable pageable) {
     public Page<CommunityRest> findAllTop(Pageable pageable) {
         try {
             List<Community> communities = cs.findAllTop(obtainContext());
-            return converter.toRestPage(utils.getPage(communities, pageable), utils.obtainProjection());
+            return converter.toRestPage(communities, pageable, utils.obtainProjection());
         } catch (SQLException e) {
             throw new RuntimeException(e.getMessage(), e);
         }

diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DSpaceRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/DSpaceRestRepository.java
@@ -32,6 +32,7 @@
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.repository.PagingAndSortingRepository;
+import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.multipart.MultipartFile;
 
 /**
@@ -122,6 +123,7 @@ public Optional<T> findById(ID id) {
      *            the rest object id
      * @return the REST object identified by its ID
      */
+    @PreAuthorize("hasAuthority('ADMIN')")
     public abstract T findOne(Context context, ID id);
 
     @Override

diff --git a/...erver-webapp/src/main/java/org/dspace/app/rest/repository/EPersonGroupLinkRepository.java b/...erver-webapp/src/main/java/org/dspace/app/rest/repository/EPersonGroupLinkRepository.java
@@ -17,7 +17,6 @@
 import org.dspace.app.rest.projection.Projection;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
-import org.dspace.eperson.Group;
 import org.dspace.eperson.service.EPersonService;
 import org.dspace.eperson.service.GroupService;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -51,8 +50,7 @@ public Page<GroupRest> getGroups(@Nullable HttpServletRequest request,
             if (eperson == null) {
                 throw new ResourceNotFoundException("No such eperson: " + epersonId);
             }
-            Page<Group> groups = utils.getPage(eperson.getGroups(), optionalPageable);
-            return converter.toRestPage(groups, projection);
+            return converter.toRestPage(eperson.getGroups(), optionalPageable, projection);
         } catch (SQLException e) {
             throw new RuntimeException(e);
         }

diff --git a/...-server-webapp/src/main/java/org/dspace/app/rest/repository/EntityTypeRestRepository.java b/...-server-webapp/src/main/java/org/dspace/app/rest/repository/EntityTypeRestRepository.java
@@ -18,6 +18,7 @@
 import org.springframework.data.domain.Page;
 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;
 
 /**
@@ -29,6 +30,7 @@ public class EntityTypeRestRepository extends DSpaceRestRepository<EntityTypeRes
     @Autowired
     private EntityTypeService entityTypeService;
 
+    @PreAuthorize("permitAll()")
     public EntityTypeRest findOne(Context context, Integer integer) {
         try {
             EntityType entityType = entityTypeService.find(context, integer);
@@ -44,7 +46,7 @@ public EntityTypeRest findOne(Context context, Integer integer) {
     public Page<EntityTypeRest> findAll(Context context, Pageable pageable) {
         try {
             List<EntityType> entityTypes = entityTypeService.findAll(context);
-            return converter.toRestPage(utils.getPage(entityTypes, pageable), utils.obtainProjection());
+            return converter.toRestPage(entityTypes, pageable, utils.obtainProjection());
         } catch (SQLException e) {
             throw new RuntimeException(e.getMessage(), e);
         }