Merge pull request #1493 from 4Science/DS-2895

DS-2895 authorization check for item not owned by collection

dspace-api/src/main/java/org/dspace/authorize/ResourcePolicyServiceImpl.java

@@ -118,15 +118,17 @@ public class ResourcePolicyServiceImpl implements ResourcePolicyService
      */
     @Override
     public void delete(Context context, ResourcePolicy resourcePolicy) throws SQLException, AuthorizeException {
+        // FIXME: authorizations
+        // Remove ourself
+        resourcePolicyDAO.delete(context, resourcePolicy);
+        
+        context.turnOffAuthorisationSystem();
         if(resourcePolicy.getdSpaceObject() != null)
         {
             //A policy for a DSpace Object has been modified, fire a modify event on the DSpace object
             contentServiceFactory.getDSpaceObjectService(resourcePolicy.getdSpaceObject()).updateLastModified(context, resourcePolicy.getdSpaceObject());
         }
-
+        context.restoreAuthSystemState();
-        // FIXME: authorizations
-        // Remove ourself
-        resourcePolicyDAO.delete(context, resourcePolicy);
     }
 
 
@@ -203,26 +205,34 @@ public class ResourcePolicyServiceImpl implements ResourcePolicyService
 
     @Override
     public void removeAllPolicies(Context c, DSpaceObject o) throws SQLException, AuthorizeException {
-        contentServiceFactory.getDSpaceObjectService(o).updateLastModified(c, o);
         resourcePolicyDAO.deleteByDso(c, o);
+        c.turnOffAuthorisationSystem();
+        contentServiceFactory.getDSpaceObjectService(o).updateLastModified(c, o);
+        c.restoreAuthSystemState();
     }
 
     @Override
     public void removePolicies(Context c, DSpaceObject o, String type) throws SQLException, AuthorizeException {
-        contentServiceFactory.getDSpaceObjectService(o).updateLastModified(c, o);
         resourcePolicyDAO.deleteByDsoAndType(c, o, type);
+        c.turnOffAuthorisationSystem();
+        contentServiceFactory.getDSpaceObjectService(o).updateLastModified(c, o);
+        c.restoreAuthSystemState();
     }
 
     @Override
     public void removeDsoGroupPolicies(Context context, DSpaceObject dso, Group group) throws SQLException, AuthorizeException {
-        contentServiceFactory.getDSpaceObjectService(dso).updateLastModified(context, dso);
         resourcePolicyDAO.deleteByDsoGroupPolicies(context, dso, group);
+        context.turnOffAuthorisationSystem();
+        contentServiceFactory.getDSpaceObjectService(dso).updateLastModified(context, dso);
+        context.restoreAuthSystemState();
     }
 
     @Override
     public void removeDsoEPersonPolicies(Context context, DSpaceObject dso, EPerson ePerson) throws SQLException, AuthorizeException {
-        contentServiceFactory.getDSpaceObjectService(dso).updateLastModified(context, dso);
         resourcePolicyDAO.deleteByDsoEPersonPolicies(context, dso, ePerson);
+        context.turnOffAuthorisationSystem();
+        contentServiceFactory.getDSpaceObjectService(dso).updateLastModified(context, dso);
+        context.restoreAuthSystemState();
 
     }
 
@@ -237,15 +247,19 @@ public class ResourcePolicyServiceImpl implements ResourcePolicyService
         {
             removeAllPolicies(c, o);
         }else{
-            contentServiceFactory.getDSpaceObjectService(o).updateLastModified(c, o);
             resourcePolicyDAO.deleteByDsoAndAction(c, o, actionId);
+            c.turnOffAuthorisationSystem();
+            contentServiceFactory.getDSpaceObjectService(o).updateLastModified(c, o);
+            c.restoreAuthSystemState();
         }
     }
 
     @Override
     public void removeDsoAndTypeNotEqualsToPolicies(Context c, DSpaceObject o, String type) throws SQLException, AuthorizeException {
-        contentServiceFactory.getDSpaceObjectService(o).updateLastModified(c, o);
         resourcePolicyDAO.deleteByDsoAndTypeNotEqualsTo(c, o, type);
+        c.turnOffAuthorisationSystem();
+        contentServiceFactory.getDSpaceObjectService(o).updateLastModified(c, o);
+        c.restoreAuthSystemState();
     }
 
 
@@ -279,10 +293,12 @@ public class ResourcePolicyServiceImpl implements ResourcePolicyService
             }
 
             //Update the last modified timestamp of all related DSpace Objects
+            context.turnOffAuthorisationSystem();
             for (DSpaceObject dSpaceObject : relatedDSpaceObjects) {
                 //A policy for a DSpace Object has been modified, fire a modify event on the DSpace object
             	contentServiceFactory.getDSpaceObjectService(dSpaceObject).updateLastModified(context, dSpaceObject);
             }
+            context.restoreAuthSystemState();
         }
     }
 }

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

@@ -30,6 +30,7 @@ import org.dspace.harvest.service.HarvestedItemService;
 import org.dspace.identifier.IdentifierException;
 import org.dspace.identifier.service.IdentifierService;
 import org.dspace.versioning.service.VersioningService;
+import org.dspace.workflow.WorkflowItemService;
 import org.springframework.beans.factory.annotation.Autowired;
 
 import java.io.IOException;
@@ -78,6 +79,11 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl<Item> implements It
     protected VersioningService versioningService;
     @Autowired(required=true)
     protected HarvestedItemService harvestedItemService;
+    @Autowired(required=true)
+    protected WorkspaceItemService workspaceItemService;
+    @Autowired(required=true)
+    protected WorkflowItemService workflowItemService;
+    
 
     protected ItemServiceImpl()
     {
@@ -881,12 +887,28 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl<Item> implements It
         // is this collection not yet created, and an item template is created
         if (item.getOwningCollection() == null)
         {
+        	if (!isInProgressSubmission(context, item)) {
         		return true;
         	}
+        	else {
+        		return false;
+        	}
+        }
 
         return collectionService.canEditBoolean(context, item.getOwningCollection(), false);
     }
 
+    /**
+     * Check if the item is an inprogress submission
+     * @param context
+     * @param item
+     * @return <code>true</code> if the item is an inprogress submission, i.e. a WorkspaceItem or WorkflowItem
+     * @throws SQLException
+     */
+    public boolean isInProgressSubmission(Context context, Item item) throws SQLException {
+		return workspaceItemService.findByItem(context, item) != null
+				|| workflowItemService.findByItem(context, item) != null;
+    }
     
     /*
     With every finished submission a bunch of resource policy entries with have null value for the dspace_object column are generated in the database.

dspace-api/src/main/java/org/dspace/content/service/ItemService.java

@@ -555,4 +555,12 @@ public interface ItemService extends DSpaceObjectService<Item>, DSpaceObjectLega
      * @throws SQLException if database error
      */
     int countWithdrawnItems(Context context) throws SQLException;
+
+	/**
+	 * Check if the supplied item is an inprogress submission
+	 * @param context
+	 * @param item
+	 * @return <code>true</code> if the item is linked to a workspaceitem or workflowitem
+	 */
+    boolean isInProgressSubmission(Context context, Item item) throws SQLException;
 }

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

@@ -1518,7 +1518,7 @@ public class ItemTest  extends AbstractDSpaceObjectTest
     }
 
     /**
-     * Test of canEditBoolean method, of class Collection.
+     * Test of canEdit method, of class Item.
      */
     @Test
     public void testCanEditBooleanAuth() throws Exception
@@ -1543,7 +1543,7 @@ public class ItemTest  extends AbstractDSpaceObjectTest
     }
 
     /**
-     * Test of canEditBoolean method, of class Collection.
+     * Test of canEdit method, of class Item.
      */
     @Test
     public void testCanEditBooleanAuth2() throws Exception
@@ -1568,7 +1568,7 @@ public class ItemTest  extends AbstractDSpaceObjectTest
     }
 
     /**
-     * Test of canEditBoolean method, of class Collection.
+     * Test of canEdit method, of class Item.
      */
     @Test
     public void testCanEditBooleanAuth3() throws Exception
@@ -1595,7 +1595,7 @@ public class ItemTest  extends AbstractDSpaceObjectTest
     }
 
     /**
-     * Test of canEditBoolean method, of class Collection.
+     * Test of canEdit method, of class Item.
      */
     @Test
     public void testCanEditBooleanAuth4() throws Exception
@@ -1617,11 +1617,33 @@ public class ItemTest  extends AbstractDSpaceObjectTest
         }};
 
         // Ensure person with WRITE perms on the Collection can edit item
-        assertTrue("testCanEditBooleanAuth43 0", itemService.canEdit(context, it));
+        assertTrue("testCanEditBooleanAuth4 0", itemService.canEdit(context, it));
     }
 
     /**
-     * Test of canEditBoolean method, of class Collection.
+     * Test of canEdit method, of class Item.
+     */
+    @Test
+    public void testCanEditBooleanAuth5() throws Exception
+    {
+        // Test Inheritance of permissions
+        new NonStrictExpectations(authorizeService.getClass())
+        {{
+            // Disallow Item WRITE perms
+        	authorizeService.authorizeAction((Context) any, (Item) any,
+                    Constants.WRITE); result = new AuthorizeException();
+            // Allow Collection WRITE perms
+            authorizeService.authorizeAction((Context) any, (Collection) any,
+                    Constants.WRITE,anyBoolean); result = null;
+        }};
+
+        collectionService.createTemplateItem(context, collection);
+        collectionService.update(context, collection);
+        assertTrue("testCanEditBooleanNoAuth5 0", itemService.canEdit(context, collection.getTemplateItem()));
+    }
+
+    /**
+     * Test of canEdit method, of class Item.
      */
     @Test
     public void testCanEditBooleanNoAuth() throws Exception
@@ -1650,6 +1672,79 @@ public class ItemTest  extends AbstractDSpaceObjectTest
         assertFalse("testCanEditBooleanNoAuth 0", itemService.canEdit(context, it));
     }
 
+    /**
+     * Test of canEdit method, of class Item.
+     */
+    @Test
+    public void testCanEditBooleanNoAuth2() throws Exception
+    {
+        context.turnOffAuthorisationSystem();
+        WorkspaceItem wi = workspaceItemService.create(context, collection, true);
+        context.restoreAuthSystemState();
+        // Test Inheritance of permissions
+        new NonStrictExpectations(authorizeService.getClass())
+        {{
+            // Disallow Item WRITE perms
+        	authorizeService.authorizeAction((Context) any, (Item) any,
+                    Constants.WRITE, anyBoolean); result = new AuthorizeException();
+        }};
+        assertFalse("testCanEditBooleanNoAuth2 0", itemService.canEdit(context, wi.getItem()));
+    }
+
+    /**
+     * Test of isInProgressSubmission method, of class Item.
+     * @throws AuthorizeException 
+     * @throws SQLException 
+     * @throws IOException 
+     * 
+     */
+    @Test
+    public void testIsInProgressSubmission() throws SQLException, AuthorizeException, IOException
+    {
+    	context.turnOffAuthorisationSystem();
+    	Collection c = createCollection();
+        WorkspaceItem wi = workspaceItemService.create(context, c, true);
+    	context.restoreAuthSystemState();
+        assertTrue("testIsInProgressSubmission 0", itemService.isInProgressSubmission(context, wi.getItem()));
+    }
+    
+    /**
+     * Test of isInProgressSubmission method, of class Item.
+     * @throws AuthorizeException 
+     * @throws SQLException 
+     * @throws IOException 
+     * 
+     */
+    @Test
+    public void testIsInProgressSubmissionFalse() throws SQLException, AuthorizeException, IOException
+    {
+    	context.turnOffAuthorisationSystem();
+    	Collection c = createCollection();
+        WorkspaceItem wi = workspaceItemService.create(context, c, true);
+        Item item = installItemService.installItem(context, wi);
+    	context.restoreAuthSystemState();
+        assertFalse("testIsInProgressSubmissionFalse 0", itemService.isInProgressSubmission(context, item));
+    }
+
+    /**
+     * Test of isInProgressSubmission method, of class Item.
+     * @throws AuthorizeException 
+     * @throws SQLException 
+     * @throws IOException 
+     * 
+     */
+    @Test
+    public void testIsInProgressSubmissionFalse2() throws SQLException, AuthorizeException, IOException
+    {
+    	context.turnOffAuthorisationSystem();
+    	Collection c = createCollection();
+        collectionService.createTemplateItem(context, c);
+        collectionService.update(context, c);
+        Item item = c.getTemplateItem();
+    	context.restoreAuthSystemState();
+        assertFalse("testIsInProgressSubmissionFalse2 0", itemService.isInProgressSubmission(context, item));
+    }
+    
     /**
      * Test of getName method, of class Item.
      */

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

@@ -295,11 +295,42 @@ public class WorkspaceItemTest extends AbstractUnitTest
      * Test of update method, of class WorkspaceItem.
      */
     @Test
-    public void testUpdate() throws Exception
+    public void testUpdateAuth() throws Exception
     {
-        //TODO: how can we verify it works?
+		// no need to mockup the authorization as we are the same user that have
+		// created the wi
+        boolean pBefore = wi.isPublishedBefore();
+        wi.setPublishedBefore(!pBefore);
         workspaceItemService.update(context, wi);
-        System.out.println("update");
+        context.commit();
+        // force to read the data from the database
+        context.clearCache();
+        // read all our test attributes objects from the fresh session 
+        // to avoid duplicate object in session issue
+        wi = workspaceItemService.find(context, wi.getID());
+        collection = wi.getCollection();
+        owningCommunity = collection.getCommunities().get(0);
+        assertTrue("testUpdate", pBefore != wi.isPublishedBefore());
+    }
+    
+    /**
+     * Test of update method, of class WorkspaceItem with no WRITE auth.
+     */
+    @Test(expected=AuthorizeException.class)
+    public void testUpdateNoAuth() throws Exception
+    {
+    	new NonStrictExpectations(authorizeService.getClass())
+        {{
+             // Remove Item WRITE perms
+        	authorizeService.authorizeActionBoolean((Context) any, (Item) any,
+                    Constants.WRITE); result = false; 
+                    authorizeService.authorizeAction((Context) any, (Item) any,
+                     Constants.WRITE); result = new AuthorizeException();
+        }};
+        boolean pBefore = wi.isPublishedBefore();
+        wi.setPublishedBefore(!pBefore);
+        workspaceItemService.update(context, wi);
+    	fail("Exception expected");
     }
 
     /**