From da112f3a33b1d9efa6d02b341956618a91e88147 Mon Sep 17 00:00:00 2001 From: Pavel Yurke Date: Tue, 10 Sep 2013 10:11:58 +0000 Subject: [PATCH] Merged DEV to HEAD (4.2) 54623: ALF-19234: An Editor (probably also Collaborator) can take ownership of a document (probably also folders) - The new "ACL_ITEM" cad was implemented. Now we check TakeOwnership permission if cm:owner property is going to change when NodeService.addProperties(), NodeService.addAspect(), NodeService.setProperties(), NodeService.setProperty() methods were invoked. Also testTakeOwnershipPermission test was added to check if we can take document ownership through NodeService without appropriate permissions. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@55167 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../public-services-security-context.xml | 11 +- .../permissions/impl/acegi/ACLEntryVoter.java | 94 +++++++++++++- .../repo/version/NodeServiceImplTest.java | 117 ++++++++++++++++++ 3 files changed, 214 insertions(+), 8 deletions(-) diff --git a/config/alfresco/public-services-security-context.xml b/config/alfresco/public-services-security-context.xml index f68a05d4ab..4764206686 100644 --- a/config/alfresco/public-services-security-context.xml +++ b/config/alfresco/public-services-security-context.xml @@ -199,6 +199,9 @@ + + + @@ -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.addProperties=ACL_NODE.0.sys:base.WriteProperties - org.alfresco.service.cmr.repository.NodeService.setProperty=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,ACL_ITEM.0.cm:ownable.TakeOwnership + 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 diff --git a/source/java/org/alfresco/repo/security/permissions/impl/acegi/ACLEntryVoter.java b/source/java/org/alfresco/repo/security/permissions/impl/acegi/ACLEntryVoter.java index 3397359f3e..f56a002262 100644 --- a/source/java/org/alfresco/repo/security/permissions/impl/acegi/ACLEntryVoter.java +++ b/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; @@ -57,7 +61,9 @@ public class ACLEntryVoter implements AccessDecisionVoter, InitializingBean private static Log log = LogFactory.getLog(ACLEntryVoter.class); 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 abstainForClassQNames = new HashSet(); @@ -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,65 @@ 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 properties = (Map) (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]); + boolean isChanged = !getArgument(invocation, 2).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 + { + 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 +659,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(); diff --git a/source/test-java/org/alfresco/repo/version/NodeServiceImplTest.java b/source/test-java/org/alfresco/repo/version/NodeServiceImplTest.java index 7b6d039323..a0f4ec2c5b 100644 --- a/source/test-java/org/alfresco/repo/version/NodeServiceImplTest.java +++ b/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 folderProps = new HashMap(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 docProps = new HashMap(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 properties = new HashMap(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); + } }