Merge pull request #8901 from kshepherd/default_bundle_permissions

Fix to adjusting bundle and bitstream policies (item install, embargo lift)

dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java

@@ -930,16 +930,23 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl<Item> implements It
     @Override
     public void adjustBundleBitstreamPolicies(Context context, Item item, Collection collection)
         throws SQLException, AuthorizeException {
-        List<ResourcePolicy> defaultCollectionPolicies = authorizeService
+        // Bundles should inherit from DEFAULT_ITEM_READ so that if the item is readable, the files
+        // can be listed (even if they are themselves not readable as per DEFAULT_BITSTREAM_READ or other
+        // policies or embargos applied
+        List<ResourcePolicy> defaultCollectionBundlePolicies = authorizeService
+                .getPoliciesActionFilter(context, collection, Constants.DEFAULT_ITEM_READ);
+        // Bitstreams should inherit from DEFAULT_BITSTREAM_READ
+        List<ResourcePolicy> defaultCollectionBitstreamPolicies = authorizeService
             .getPoliciesActionFilter(context, collection, Constants.DEFAULT_BITSTREAM_READ);
 
         List<ResourcePolicy> defaultItemPolicies = authorizeService.findPoliciesByDSOAndType(context, item,
                 ResourcePolicy.TYPE_CUSTOM);
-        if (defaultCollectionPolicies.size() < 1) {
+        if (defaultCollectionBitstreamPolicies.size() < 1) {
             throw new SQLException("Collection " + collection.getID()
                                        + " (" + collection.getHandle() + ")"
                                        + " has no default bitstream READ policies");
         }
+        // TODO: should we also throw an exception if no DEFAULT_ITEM_READ?
 
         // remove all policies from bundles, add new ones
         // Remove bundles
@@ -950,11 +957,12 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl<Item> implements It
             authorizeService.removeAllPoliciesByDSOAndType(context, mybundle, ResourcePolicy.TYPE_SUBMISSION);
             authorizeService.removeAllPoliciesByDSOAndType(context, mybundle, ResourcePolicy.TYPE_WORKFLOW);
             addCustomPoliciesNotInPlace(context, mybundle, defaultItemPolicies);
-            addDefaultPoliciesNotInPlace(context, mybundle, defaultCollectionPolicies);
+            addDefaultPoliciesNotInPlace(context, mybundle, defaultCollectionBundlePolicies);
 
             for (Bitstream bitstream : mybundle.getBitstreams()) {
                 // if come from InstallItem: remove all submission/workflow policies
-                removeAllPoliciesAndAddDefault(context, bitstream, defaultItemPolicies, defaultCollectionPolicies);
+                removeAllPoliciesAndAddDefault(context, bitstream, defaultItemPolicies,
+                                               defaultCollectionBitstreamPolicies);
             }
         }
     }

dspace-api/src/test/java/org/dspace/content/ItemTest.java

@@ -1393,6 +1393,78 @@ public class ItemTest extends AbstractDSpaceObjectTest {
         assertTrue("testInheritCollectionDefaultPolicies 2", equals);
     }
 
+    // Test to verify DEFAULT_*_READ policies on collection inherit properly to Item/Bundle/Bitstream
+    @Test
+    public void testInheritCollectionDefaultPolicies_custom_default_groups() throws Exception {
+        context.turnOffAuthorisationSystem();
+        // Create a new collection
+        Collection c = createCollection();
+        // Create a custom group with DEFAULT_ITEM_READ privileges in this Collection
+        Group item_read_role = collectionService.createDefaultReadGroup(context, c, "ITEM",
+                                                                        Constants.DEFAULT_ITEM_READ);
+        // Create a custom group with DEFAULT_BITSTREAM_READ privileges in this Collection
+        Group bitstream_read_role = collectionService.createDefaultReadGroup(context, c, "BITSTREAM",
+                                                                        Constants.DEFAULT_BITSTREAM_READ);
+        context.restoreAuthSystemState();
+
+        // Verify that Collection's DEFAULT_ITEM_READ now uses the newly created group.
+        List<ResourcePolicy> defaultItemReadPolicies =
+            authorizeService.getPoliciesActionFilter(context, c, Constants.DEFAULT_ITEM_READ);
+        assertEquals("One DEFAULT_ITEM_READ policy", 1, defaultItemReadPolicies.size());
+        assertEquals("DEFAULT_ITEM_READ group", item_read_role.getName(),
+                     defaultItemReadPolicies.get(0).getGroup().getName());
+
+        // Verify that Collection's DEFAULT_BITSTREAM_READ now uses the newly created group.
+        List<ResourcePolicy> defaultBitstreamReadPolicies =
+            authorizeService.getPoliciesActionFilter(context, c, Constants.DEFAULT_BITSTREAM_READ);
+        assertEquals("One DEFAULT_BITSTREAM_READ policy on Collection", 1, defaultBitstreamReadPolicies.size());
+        assertEquals("DEFAULT_BITSTREAM_READ group", bitstream_read_role.getName(),
+                     defaultBitstreamReadPolicies.get(0).getGroup().getName());
+
+        context.turnOffAuthorisationSystem();
+        // Create a new Item in this Collection
+        WorkspaceItem workspaceItem = workspaceItemService.create(context, c, false);
+        Item item = workspaceItem.getItem();
+        // Add a single Bitstream to the ORIGINAL bundle
+        File f = new File(testProps.get("test.bitstream").toString());
+        Bitstream bitstream = itemService.createSingleBitstream(context, new FileInputStream(f), item);
+        context.restoreAuthSystemState();
+
+        // Allow Item WRITE perms
+        doNothing().when(authorizeServiceSpy).authorizeAction(context, item, Constants.WRITE, true);
+        // Inherit all default policies from Collection down to new Item
+        itemService.inheritCollectionDefaultPolicies(context, item, c);
+
+        // Verify Item inherits DEFAULT_ITEM_READ group from Collection
+        List<ResourcePolicy> itemReadPolicies = authorizeService.getPoliciesActionFilter(context, item, Constants.READ);
+        assertEquals("One READ policy on Item", 1, itemReadPolicies.size());
+        assertEquals("Item's READ group", item_read_role.getName(),
+                     itemReadPolicies.get(0).getGroup().getName());
+
+        // Verify Bitstream inherits DEFAULT_BITSTREAM_READ group from Collection
+        List<ResourcePolicy> bitstreamReadPolicies = authorizeService.getPoliciesActionFilter(context, bitstream,
+                                                                                              Constants.READ);
+        assertEquals("One READ policy on Bitstream", 1, bitstreamReadPolicies.size());
+        assertEquals("Bitstream's READ group", bitstream_read_role.getName(),
+                     bitstreamReadPolicies.get(0).getGroup().getName());
+
+        // Verify ORIGINAL Bundle inherits DEFAULT_ITEM_READ group from Collection
+        // Bundles should inherit from DEFAULT_ITEM_READ so that if the item is readable, the files
+        // can be listed (even if files are access restricted or embargoed)
+        List<Bundle> bundles = item.getBundles(Constants.DEFAULT_BUNDLE_NAME);
+        Bundle originalBundle = bundles.get(0);
+        List<ResourcePolicy> bundleReadPolicies = authorizeService.getPoliciesActionFilter(context, originalBundle,
+                                                                                           Constants.READ);
+        assertEquals("One READ policy on Bundle", 1, bundleReadPolicies.size());
+        assertEquals("Bundles's READ group", item_read_role.getName(),
+                     bundleReadPolicies.get(0).getGroup().getName());
+
+        // Cleanup after ourselves. Delete created collection & all content under it
+        context.turnOffAuthorisationSystem();
+        collectionService.delete(context, c);
+        context.restoreAuthSystemState();
+    }
+
     /**
      * Test of move method, of class Item.
      */