Merge pull request #3264 from 4Science/CST-4122-ExposewWithdrawnItemsToAnonymousUsersFilteringOutTheMetadata

Need a way to identify withdrawn items via REST API

dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ItemConverter.java

@@ -8,8 +8,10 @@
 package org.dspace.app.rest.converter;
 
 import java.sql.SQLException;
+import java.util.ArrayList;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Objects;
 
 import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.lang3.StringUtils;
@@ -73,6 +75,10 @@ public class ItemConverter
         List<MetadataValue> fullList = itemService.getMetadata(obj, Item.ANY, Item.ANY, Item.ANY, Item.ANY, true);
         List<MetadataValue> returnList = new LinkedList<>();
         try {
+            if (obj.isWithdrawn() && (Objects.isNull(context) ||
+                                      Objects.isNull(context.getCurrentUser()) || !authorizeService.isAdmin(context))) {
+                return new MetadataValueList(new ArrayList<MetadataValue>());
+            }
             if (context != null && authorizeService.isAdmin(context)) {
                 return new MetadataValueList(fullList);
             }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemRestRepository.java

@@ -100,7 +100,7 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
     }
 
     @Override
-    @PreAuthorize("hasPermission(#id, 'ITEM', 'READ')")
+    @PreAuthorize("hasPermission(#id, 'ITEM', 'STATUS') || hasPermission(#id, 'ITEM', 'READ')")
     public ItemRest findOne(Context context, UUID id) {
         Item item = null;
         try {

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemVersionLinkRepository.java

@@ -23,6 +23,7 @@ import org.dspace.versioning.service.VersioningService;
 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;
 
 /**
@@ -48,6 +49,7 @@ public class ItemVersionLinkRepository extends AbstractDSpaceRestRepository
      *                          itemUuid param as UUID
      * @throws SQLException     If something goes wrong
      */
+    @PreAuthorize("hasPermission(#itemUuid, 'ITEM', 'READ')")
     public VersionRest getItemVersion(@Nullable HttpServletRequest request,
                                       UUID itemUuid,
                                       @Nullable Pageable optionalPageable,

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthorizeServicePermissionEvaluatorPlugin.java

@@ -86,11 +86,16 @@ public class AuthorizeServicePermissionEvaluatorPlugin extends RestObjectPermiss
                         return true;
                     }
 
-                    // If the item is still inprogress we can process here only the READ permission.
+
-                    // Other actions need to be evaluated against the wrapper object (workspace or workflow item)
                     if (dSpaceObject instanceof Item) {
-                        if (!DSpaceRestPermission.READ.equals(restPermission)
+                        Item item = (Item) dSpaceObject;
-                            && !((Item) dSpaceObject).isArchived() && !((Item) dSpaceObject).isWithdrawn()) {
+                        if (DSpaceRestPermission.STATUS.equals(restPermission) && item.isWithdrawn()) {
+                            return true;
+                        }
+                        // If the item is still inprogress we can process here only the READ permission.
+                        // Other actions need to be evaluated against the wrapper object (workspace or workflow item)
+                        if (!DSpaceRestPermission.READ.equals(restPermission) &&
+                                   !item.isArchived() && !item.isWithdrawn()) {
                             return false;
                         }
                     }

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/DSpaceRestPermission.java

@@ -18,7 +18,11 @@ public enum DSpaceRestPermission {
     WRITE(Constants.WRITE),
     DELETE(Constants.DELETE),
     ADD(Constants.ADD),
-    ADMIN(Constants.ADMIN);
+    ADMIN(Constants.ADMIN),
+    // STATUS permissions allows someone to see the status of an object, without necessarily being able to fully READ it
+    // For example, STATUS is used to allow withdrawn Items to be seen as withdrawn,
+    // while hiding all their metadata/links.
+    STATUS(Constants.READ);
 
     private int dspaceApiActionId;
 

dspace-server-webapp/src/test/java/org/dspace/app/rest/ItemRestRepositoryIT.java

@@ -3730,4 +3730,142 @@ public class ItemRestRepositoryIT extends AbstractControllerIntegrationTest {
         context.restoreAuthSystemState();
     }
 
+    @Test
+    public void findWithdrawnItemTest() throws Exception {
+        context.turnOffAuthorisationSystem();
+
+        parentCommunity = CommunityBuilder.createCommunity(context)
+                                          .withName("Parent Community")
+                                          .build();
+
+        Collection col1 = CollectionBuilder.createCollection(context, parentCommunity)
+                                           .withName("Collection 1").build();
+
+        Item item = ItemBuilder.createItem(context, col1)
+                               .withTitle("Public item 1")
+                               .withIssueDate("2017-10-17")
+                               .withAuthor("Doe, John")
+                               .withSubject("ExtraEntry")
+                               .build();
+
+        context.restoreAuthSystemState();
+
+        String tokenAdmin = getAuthToken(admin.getEmail(), password);
+        String tokenEperson = getAuthToken(eperson.getEmail(), password);
+
+        List<Operation> ops = new ArrayList<Operation>();
+        ReplaceOperation replaceOperation = new ReplaceOperation("/withdrawn", true);
+        ops.add(replaceOperation);
+        String patchBody = getPatchContent(ops);
+
+        // check item status
+        getClient(tokenAdmin).perform(get("/api/core/items/" + item.getID()))
+                             .andExpect(status().isOk())
+                             .andExpect(jsonPath("$.uuid", Matchers.is(item.getID().toString())))
+                             .andExpect(jsonPath("$.withdrawn", Matchers.is(false)))
+                             .andExpect(jsonPath("$.inArchive", Matchers.is(true)));
+
+        // withdraw item
+        getClient(tokenAdmin).perform(patch("/api/core/items/" + item.getID())
+                             .content(patchBody)
+                             .contentType(MediaType.APPLICATION_JSON_PATCH_JSON))
+                             .andExpect(status().isOk())
+                             .andExpect(jsonPath("$.uuid", Matchers.is(item.getID().toString())))
+                             .andExpect(jsonPath("$.withdrawn", Matchers.is(true)))
+                             .andExpect(jsonPath("$.inArchive", Matchers.is(false)));
+
+        // admins should be able to get the full information about withdrawn items
+        getClient(tokenAdmin).perform(get("/api/core/items/" + item.getID())
+                 .param("projection", "full"))
+                 .andExpect(status().isOk())
+                 .andExpect(jsonPath("$", ItemMatcher.matchFullEmbeds()))
+                 .andExpect(jsonPath("$.uuid", Matchers.is(item.getID().toString())))
+                 .andExpect(jsonPath("$.withdrawn", Matchers.is(true)))
+                 .andExpect(jsonPath("$.inArchive", Matchers.is(false)))
+                 .andExpect(jsonPath("$._links.self.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString())))
+                 .andExpect(jsonPath("$._links.bundles.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/bundles")))
+                 .andExpect(jsonPath("$._links.mappedCollections.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/mappedCollections")))
+                 .andExpect(jsonPath("$._links.owningCollection.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/owningCollection")))
+                 .andExpect(jsonPath("$._links.relationships.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/relationships")))
+                 .andExpect(jsonPath("$._links.version.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/version")))
+                 .andExpect(jsonPath("$._links.templateItemOf.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/templateItemOf")));
+
+        getClient(tokenAdmin).perform(get("/api/core/items/" + item.getID() + "/owningCollection")
+                             .param("projection", "full"))
+                             .andExpect(status().isOk())
+                             .andExpect(jsonPath("$", CollectionMatcher.matchCollectionEntryFullProjection(
+                                        col1.getName(), col1.getID(), col1.getHandle())));;
+
+        // try to spoof information as a logged in eperson using embedding, verify that no embedds are included
+        getClient(tokenEperson).perform(get("/api/core/items/" + item.getID())
+                 .param("projection", "full"))
+                 .andExpect(status().isOk())
+                 .andExpect(jsonPath("$", HalMatcher.matchNoEmbeds()))
+                 .andExpect(jsonPath("$.uuid", Matchers.is(item.getID().toString())))
+                 .andExpect(jsonPath("$.name", Matchers.is(item.getName())))
+                 .andExpect(jsonPath("$.handle", Matchers.is(item.getHandle())))
+                 .andExpect(jsonPath("$.metadata").isEmpty())
+                 .andExpect(jsonPath("$.withdrawn", Matchers.is(true)))
+                 .andExpect(jsonPath("$.inArchive", Matchers.is(false)))
+                 .andExpect(jsonPath("$._links.self.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString())))
+                 .andExpect(jsonPath("$._links.bundles.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/bundles")))
+                 .andExpect(jsonPath("$._links.mappedCollections.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/mappedCollections")))
+                 .andExpect(jsonPath("$._links.owningCollection.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/owningCollection")))
+                 .andExpect(jsonPath("$._links.relationships.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/relationships")))
+                 .andExpect(jsonPath("$._links.version.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/version")))
+                 .andExpect(jsonPath("$._links.templateItemOf.href",
+                     Matchers.containsString("/api/core/items/" + item.getID().toString() + "/templateItemOf")));
+
+
+        // access to linked resources should be denied
+        getClient(tokenEperson).perform(get("/api/core/items/" + item.getID() + "/owningCollection"))
+                               .andExpect(status().isForbidden());
+
+        // try to spoof information as anonymous user using embedding, verify that no embedds are included
+        getClient().perform(get("/api/core/items/" + item.getID())
+                   .param("projection", "full"))
+                   .andExpect(status().isOk())
+                   .andExpect(jsonPath("$", HalMatcher.matchNoEmbeds()))
+                   .andExpect(jsonPath("$.uuid", Matchers.is(item.getID().toString())))
+                   .andExpect(jsonPath("$.name", Matchers.is(item.getName())))
+                   .andExpect(jsonPath("$.handle", Matchers.is(item.getHandle())))
+                   .andExpect(jsonPath("$.metadata").isEmpty())
+                   .andExpect(jsonPath("$.withdrawn", Matchers.is(true)))
+                   .andExpect(jsonPath("$.inArchive", Matchers.is(false)))
+                   .andExpect(jsonPath("$._links.self.href",
+                       Matchers.containsString("/api/core/items/" + item.getID().toString())))
+                   .andExpect(jsonPath("$._links.self.href",
+                       Matchers.containsString("/api/core/items/" + item.getID().toString())))
+                   .andExpect(jsonPath("$._links.bundles.href",
+                       Matchers.containsString("/api/core/items/" + item.getID().toString() + "/bundles")))
+                   .andExpect(jsonPath("$._links.mappedCollections.href",
+                       Matchers.containsString("/api/core/items/" + item.getID().toString() + "/mappedCollections")))
+                   .andExpect(jsonPath("$._links.owningCollection.href",
+                       Matchers.containsString("/api/core/items/" + item.getID().toString() + "/owningCollection")))
+                   .andExpect(jsonPath("$._links.relationships.href",
+                       Matchers.containsString("/api/core/items/" + item.getID().toString() + "/relationships")))
+                   .andExpect(jsonPath("$._links.version.href",
+                       Matchers.containsString("/api/core/items/" + item.getID().toString() + "/version")))
+                   .andExpect(jsonPath("$._links.templateItemOf.href",
+                       Matchers.containsString("/api/core/items/" + item.getID().toString() + "/templateItemOf")));
+
+        // access to linked resources should be denied
+        getClient().perform(get("/api/core/items/" + item.getID() + "/owningCollection"))
+                   .andExpect(status().isUnauthorized());
+
+    }
+
 }