Fix for ALF-19234 : An Editor (probably also Collaborator) can take ownership of a document (probably also folders)

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@55237 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/public-services-security-context.xml

@@ -199,6 +199,9 @@
        <property name="nodeService">
            <ref bean="nodeService"></ref>
        </property>
+       <property name="ownableService">
+           <ref bean="ownableService"></ref>
+       </property>
        <property name="authenticationService">
             <ref bean="authenticationService" />
        </property>
@@ -388,7 +391,7 @@
                org.alfresco.service.cmr.repository.NodeService.setChildAssociationIndex=ACL_PARENT.0.sys:base.WriteProperties
                org.alfresco.service.cmr.repository.NodeService.getType=ACL_ALLOW
                org.alfresco.service.cmr.repository.NodeService.setType=ACL_NODE.0.sys:base.WriteProperties
-               org.alfresco.service.cmr.repository.NodeService.addAspect=ACL_NODE.0.sys:base.WriteProperties
+               org.alfresco.service.cmr.repository.NodeService.addAspect=ACL_NODE.0.sys:base.WriteProperties,ACL_ITEM.0.cm:ownable.TakeOwnership
                org.alfresco.service.cmr.repository.NodeService.removeAspect=ACL_NODE.0.sys:base.WriteProperties
                org.alfresco.service.cmr.repository.NodeService.hasAspect=ACL_NODE.0.sys:base.ReadProperties
                org.alfresco.service.cmr.repository.NodeService.getAspects=ACL_NODE.0.sys:base.ReadProperties
@@ -398,9 +401,9 @@
                org.alfresco.service.cmr.repository.NodeService.removeChildAssociation=ACL_PARENT.0.sys:base.DeleteChildren,ACL_PRI_CHILD_ASSOC_ON_CHILD.0.sys:base.DeleteNode
                org.alfresco.service.cmr.repository.NodeService.getProperties=ACL_NODE.0.sys:base.ReadProperties
                org.alfresco.service.cmr.repository.NodeService.getProperty=ACL_NODE.0.sys:base.ReadProperties
-               org.alfresco.service.cmr.repository.NodeService.setProperties=ACL_NODE.0.sys:base.WriteProperties
+               org.alfresco.service.cmr.repository.NodeService.setProperties=ACL_NODE.0.sys:base.WriteProperties,ACL_ITEM.0.cm:ownable.TakeOwnership
-               org.alfresco.service.cmr.repository.NodeService.addProperties=ACL_NODE.0.sys:base.WriteProperties
+               org.alfresco.service.cmr.repository.NodeService.addProperties=ACL_NODE.0.sys:base.WriteProperties,ACL_ITEM.0.cm:ownable.TakeOwnership
-               org.alfresco.service.cmr.repository.NodeService.setProperty=ACL_NODE.0.sys:base.WriteProperties
+               org.alfresco.service.cmr.repository.NodeService.setProperty=ACL_NODE.0.sys:base.WriteProperties,ACL_ITEM.0.cm:ownable.TakeOwnership
                org.alfresco.service.cmr.repository.NodeService.removeProperty=ACL_NODE.0.sys:base.WriteProperties
                org.alfresco.service.cmr.repository.NodeService.getParentAssocs=ACL_NODE.0.sys:base.ReadProperties
                org.alfresco.service.cmr.repository.NodeService.getChildAssocs=ACL_NODE.0.sys:base.ReadChildren,AFTER_ACL_NODE.sys:base.ReadProperties

source/java/org/alfresco/repo/security/permissions/impl/acegi/ACLEntryVoter.java

@@ -18,11 +18,13 @@
  */
 package org.alfresco.repo.security.permissions.impl.acegi;
 
+import java.io.Serializable;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.StringTokenizer;
 
@@ -31,6 +33,7 @@ import net.sf.acegisecurity.ConfigAttribute;
 import net.sf.acegisecurity.ConfigAttributeDefinition;
 import net.sf.acegisecurity.vote.AccessDecisionVoter;
 
+import org.alfresco.model.ContentModel;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.permissions.impl.SimplePermissionReference;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
@@ -40,6 +43,7 @@ import org.alfresco.service.cmr.repository.StoreRef;
 import org.alfresco.service.cmr.security.AccessStatus;
 import org.alfresco.service.cmr.security.AuthenticationService;
 import org.alfresco.service.cmr.security.AuthorityService;
+import org.alfresco.service.cmr.security.OwnableService;
 import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.namespace.NamespacePrefixResolver;
 import org.alfresco.service.namespace.QName;
@@ -58,6 +62,8 @@ public class ACLEntryVoter implements AccessDecisionVoter, InitializingBean
 
     private static final String ACL_NODE = "ACL_NODE";
 
+    private static final String ACL_ITEM = "ACL_ITEM";
+
     private static final String ACL_PRI_CHILD_ASSOC_ON_CHILD = "ACL_PRI_CHILD_ASSOC_ON_CHILD";
 
     private static final String ACL_PARENT = "ACL_PARENT";
@@ -74,6 +80,8 @@ public class ACLEntryVoter implements AccessDecisionVoter, InitializingBean
 
     private NodeService nodeService;
 
+    private OwnableService ownableService;
+
     private AuthorityService authorityService;
     
     private Set<QName> abstainForClassQNames = new HashSet<QName>();
@@ -137,6 +145,15 @@ public class ACLEntryVoter implements AccessDecisionVoter, InitializingBean
         return nodeService;
     }
 
+    /**
+     * Get the ownable service
+     * @return the ownable service
+     */
+    public OwnableService getOwnableService()
+    {
+        return ownableService;
+    }
+
     /**
      * Set the node service
      * @param nodeService
@@ -146,6 +163,15 @@ public class ACLEntryVoter implements AccessDecisionVoter, InitializingBean
         this.nodeService = nodeService;
     }
 
+    /**
+     * Set the ownable service
+     * @param nodeService
+     */
+    public void setOwnableService(OwnableService ownableService)
+    {
+        this.ownableService = ownableService;
+    }
+
     /**
      * Set the authentication service
      * @param authenticationService
@@ -207,6 +233,7 @@ public class ACLEntryVoter implements AccessDecisionVoter, InitializingBean
     {
         if ((attribute.getAttribute() != null)
                 && (attribute.getAttribute().startsWith(ACL_NODE)
+                        || attribute.getAttribute().startsWith(ACL_ITEM)
                         || attribute.getAttribute().startsWith(ACL_PRI_CHILD_ASSOC_ON_CHILD)
                         || attribute.getAttribute().startsWith(ACL_PARENT)
                         || attribute.getAttribute().equals(ACL_ALLOW)
@@ -416,6 +443,75 @@ public class ACLEntryVoter implements AccessDecisionVoter, InitializingBean
                     throw new ACLEntryVoterException("The specified parameter is not a NodeRef or ChildAssociationRef");
                 }
             }
+            else if (cad.typeString.equals(ACL_ITEM))
+            {
+                if (NodeRef.class.isAssignableFrom(params[cad.parameter[0]]))
+                {
+                    if (Map.class.isAssignableFrom(params[1]) || Map.class.isAssignableFrom(params[2]))
+                    {
+                        Map<QName, Serializable> properties = (Map<QName, Serializable>) (Map.class.isAssignableFrom(params[1]) ? getArgument(invocation, 1) : getArgument(invocation, 2));
+                        if (properties != null && properties.containsKey(ContentModel.PROP_OWNER))
+                        {
+                            testNodeRef = getArgument(invocation, cad.parameter[0]);
+
+                            boolean isChanged = !properties.get(ContentModel.PROP_OWNER).toString().equals(ownableService.getOwner(testNodeRef));
+
+                            if (!isChanged)
+                            {
+                                testNodeRef = null;
+                            }
+
+                            if (log.isDebugEnabled())
+                            {
+                                if (nodeService.exists(testNodeRef))
+                                {
+                                    log.debug("\tPermission test on node " + nodeService.getPath(testNodeRef));
+                                }
+                                else
+                                {
+                                    log.debug("\tPermission test on non-existing node " + testNodeRef);
+                                }
+                            }
+                        }
+                    }
+                    else if (QName.class.isAssignableFrom(params[1]) && params[2] != null)
+                    {
+                        testNodeRef = getArgument(invocation, cad.parameter[0]);
+                        QName arg1 = getArgument(invocation, 1);
+                        boolean isOwnerProperty = ContentModel.PROP_OWNER.equals(arg1);
+                        if(isOwnerProperty)
+                        {
+                            Object arg2 = getArgument(invocation, 2);
+                            boolean isChanged = (arg2 != null && !arg2.toString().equals(ownableService.getOwner(testNodeRef)));
+
+                            if (!isChanged)
+                            {
+                                testNodeRef = null;
+                            }  
+                        }
+                        else
+                        {
+                            testNodeRef = null;
+                        }
+
+                        if (log.isDebugEnabled())
+                        {
+                            if (nodeService.exists(testNodeRef))
+                            {
+                                log.debug("\tPermission test on node " + nodeService.getPath(testNodeRef));
+                            }
+                            else
+                            {
+                                log.debug("\tPermission test on non-existing node " + testNodeRef);
+                            }
+                        }
+                    }
+                }
+                else
+                {
+                    throw new ACLEntryVoterException("The specified parameter is not a Item");
+                }
+            }
             else if (cad.typeString.equals(ACL_PARENT))
             {
                 // There is no point having parent permissions for store
@@ -573,14 +669,14 @@ public class ACLEntryVoter implements AccessDecisionVoter, InitializingBean
             }
             typeString = st.nextToken();
 
-            if (!(typeString.equals(ACL_NODE) || typeString.equals(ACL_PRI_CHILD_ASSOC_ON_CHILD)
+            if (!(typeString.equals(ACL_NODE) || typeString.equals(ACL_ITEM) || typeString.equals(ACL_PRI_CHILD_ASSOC_ON_CHILD)
                     || typeString.equals(ACL_PARENT) || typeString.equals(ACL_ALLOW) || typeString.equals(ACL_METHOD) || typeString
                     .equals(ACL_DENY)))
             {
-                throw new ACLEntryVoterException("Invalid type: must be ACL_NODE, ACL_PARENT or ACL_ALLOW");
+                throw new ACLEntryVoterException("Invalid type: must be ACL_NODE, ACL_ITEM, ACL_PARENT or ACL_ALLOW");
             }
 
-            if (typeString.equals(ACL_NODE) || typeString.equals(ACL_PRI_CHILD_ASSOC_ON_CHILD)
+            if (typeString.equals(ACL_NODE) || typeString.equals(ACL_ITEM) || typeString.equals(ACL_PRI_CHILD_ASSOC_ON_CHILD)
                     || typeString.equals(ACL_PARENT))
             {
                 int count = st.countTokens();

source/test-java/org/alfresco/repo/version/NodeServiceImplTest.java

@@ -29,14 +29,21 @@ import java.util.Set;
 import org.alfresco.model.ApplicationModel;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.cache.TransactionalCache;
+import org.alfresco.repo.security.authentication.AuthenticationComponent;
+import org.alfresco.repo.security.permissions.AccessDeniedException;
 import org.alfresco.service.cmr.repository.AssociationRef;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.repository.Path;
+import org.alfresco.service.cmr.security.AccessStatus;
+import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.cmr.version.Version;
+import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.namespace.RegexQNamePattern;
+import org.alfresco.util.GUID;
+import org.alfresco.util.TestWithUserUtils;
 import org.alfresco.util.debug.NodeStoreInspector;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -59,6 +66,11 @@ public class NodeServiceImplTest extends BaseVersionStoreTest
     private final static String MSG_ERR = 
         "This operation is not supported by a version store implementation of the node service.";
     
+    /**
+     * User password
+     */
+    private static final String PWD = "password";
+    
     /**
      * Dummy data used in failure tests
      */
@@ -715,4 +727,109 @@ public class NodeServiceImplTest extends BaseVersionStoreTest
         Version version = createVersion(newNode, this.versionProperties);
         assertNotNull(version);
     }
+    
+    public void testTakeOwnershipPermission()
+    {
+        NodeService proxyNodeService = (NodeService) applicationContext.getBean("NodeService");
+
+        // Authenticate as system user because the current user should not be node owner
+        AuthenticationComponent authenticationComponent = (AuthenticationComponent) this.applicationContext.getBean("authenticationComponent");
+        authenticationComponent.setSystemUserAsCurrentUser();
+
+        // Create folder
+        Map<QName, Serializable> folderProps = new HashMap<QName, Serializable>(1);
+        String folderName = "testFolder" + GUID.generate();
+        folderProps.put(ContentModel.PROP_NAME, folderName);
+        NodeRef folderRef = this.nodeService.createNode(this.rootNodeRef, ContentModel.ASSOC_CHILDREN, QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, folderName),
+                ContentModel.TYPE_FOLDER, folderProps).getChildRef();
+
+        // Create document
+        Map<QName, Serializable> docProps = new HashMap<QName, Serializable>(1);
+        String docName = "testDoc" + GUID.generate() + ".txt";
+        docProps.put(ContentModel.PROP_NAME, docName);
+        NodeRef docRef = this.nodeService.createNode(folderRef, ContentModel.ASSOC_CONTAINS, QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, docName),
+                ContentModel.TYPE_CONTENT, docProps).getChildRef();
+
+        // Create user
+        String userName = "testUser" + GUID.generate();
+        TestWithUserUtils.createUser(userName, PWD, this.rootNodeRef, this.nodeService, this.authenticationService);
+
+        // Apply editor permission to document
+        permissionService.deletePermissions(docRef);
+        permissionService.setInheritParentPermissions(docRef, false);
+        permissionService.setPermission(docRef, userName, PermissionService.EDITOR, true);
+
+        // Authenticate test user
+        TestWithUserUtils.authenticateUser(userName, PWD, this.rootNodeRef, this.authenticationService);
+
+        // Check if a user has not the "take ownership" permission directly through permissionService
+        boolean isAble = AccessStatus.ALLOWED == permissionService.hasPermission(docRef, PermissionService.TAKE_OWNERSHIP);
+        assertEquals("Incorrect TakeOwnership permission.", false, isAble);
+
+        // Add ownable aspect to the document
+        this.nodeService.addAspect(docRef, ContentModel.ASPECT_OWNABLE, null);
+
+        // Take ownership through addAspect method
+        Map<QName, Serializable> properties = new HashMap<QName, Serializable>(4, 1.0f);
+        properties.put(ContentModel.PROP_OWNER, (Serializable) userName);
+
+        try
+        {
+            proxyNodeService.addAspect(docRef, ContentModel.ASPECT_OWNABLE, properties);
+        }
+        catch (AccessDeniedException e)
+        {
+        }
+
+        // Retrieve the data directly from the node service to ensure its not been changed
+        String updatedOwner = (String) this.nodeService.getProperty(docRef, ContentModel.PROP_OWNER);
+
+        boolean isUserOwner = updatedOwner == null || !updatedOwner.equals(userName) ? false : true;
+        assertEquals("Ownership's rights to the document have been taken by the user that has Editor permissions (addAspect).", false, isUserOwner);
+
+        // Take ownership through addProperties method
+        try
+        {
+            proxyNodeService.addProperties(docRef, properties);
+        }
+        catch (AccessDeniedException e)
+        {
+        }
+
+        // Retrieve the data directly from the node service to ensure its not been changed
+        updatedOwner = (String) this.nodeService.getProperty(docRef, ContentModel.PROP_OWNER);
+
+        isUserOwner = updatedOwner == null || !updatedOwner.equals(userName) ? false : true;
+        assertEquals("Ownership's rights to the document have been taken by the user that has Editor permissions (addProperties).", false, isUserOwner);
+
+        // Take ownership through setProperties method
+        try
+        {
+            proxyNodeService.setProperties(docRef, properties);
+        }
+        catch (AccessDeniedException e)
+        {
+        }
+
+        // Retrieve the data directly from the node service to ensure its not been changed
+        updatedOwner = (String) this.nodeService.getProperty(docRef, ContentModel.PROP_OWNER);
+
+        isUserOwner = updatedOwner == null || !updatedOwner.equals(userName) ? false : true;
+        assertEquals("Ownership's rights to the document have been taken by the user that has Editor permissions (setProperties).", false, isUserOwner);
+
+        // Take ownership through setProperty method
+        try
+        {
+            proxyNodeService.setProperty(docRef, ContentModel.ASPECT_OWNABLE, (Serializable) userName);
+        }
+        catch (AccessDeniedException e)
+        {
+        }
+
+        // Retrieve the data directly from the node service to ensure its not been changed
+        updatedOwner = (String) this.nodeService.getProperty(docRef, ContentModel.PROP_OWNER);
+
+        isUserOwner = updatedOwner == null || !updatedOwner.equals(userName) ? false : true;
+        assertEquals("Ownership's rights to the document have been taken by the user that has Editor permissions (setProperty).", false, isUserOwner);
+    }
 }