diff --git a/repository/src/main/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java b/repository/src/main/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java index e902f5a261..375dc969cf 100644 --- a/repository/src/main/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java +++ b/repository/src/main/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java @@ -1483,7 +1483,17 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO // Update ACLs for moved tree Long newParentAclId = newParentNode.getAclId(); - accessControlListDAO.updateInheritance(newChildNodeId, oldParentAclId, newParentAclId); + + // Verify if parent has aspect applied and ACL's are pending + if (hasNodeAspect(oldParentNodeId, ContentModel.ASPECT_PENDING_FIX_ACL)) + { + Long oldParentSharedAclId = (Long) this.getNodeProperty(oldParentNodeId, ContentModel.PROP_SHARED_ACL_TO_REPLACE); + accessControlListDAO.updateInheritance(newChildNodeId, oldParentSharedAclId, newParentAclId); + } + else + { + accessControlListDAO.updateInheritance(newChildNodeId, oldParentAclId, newParentAclId); + } } // Done diff --git a/repository/src/main/java/org/alfresco/repo/domain/permissions/ADMAccessControlListDAO.java b/repository/src/main/java/org/alfresco/repo/domain/permissions/ADMAccessControlListDAO.java index b32ccaf972..b0d0f708b7 100644 --- a/repository/src/main/java/org/alfresco/repo/domain/permissions/ADMAccessControlListDAO.java +++ b/repository/src/main/java/org/alfresco/repo/domain/permissions/ADMAccessControlListDAO.java @@ -337,6 +337,13 @@ public class ADMAccessControlListDAO implements AccessControlListDAO setFixedAcls(getNodeIdNotNull(parent), inheritFrom, null, sharedAclToReplace, changes, false, asyncCall, true); return changes; } + + public List setInheritanceForChildren(NodeRef parent, Long inheritFrom, Long sharedAclToReplace, boolean asyncCall, boolean forceSharedACL) + { + List changes = new ArrayList(); + setFixedAcls(getNodeIdNotNull(parent), inheritFrom, null, sharedAclToReplace, changes, false, asyncCall, true, forceSharedACL); + return changes; + } public void updateChangedAcls(NodeRef startingPoint, List changes) { @@ -362,6 +369,29 @@ public class ADMAccessControlListDAO implements AccessControlListDAO setFixedAcls(nodeId, inheritFrom, mergeFrom, sharedAclToReplace, changes, set, false, true); } + /** + * Support to set a shared ACL on a node and all of its children + * + * @param nodeId + * the parent node + * @param inheritFrom + * the parent node's ACL + * @param mergeFrom + * the shared ACL, if already known. If null, will be retrieved / created lazily + * @param changes + * the list in which to record changes + * @param set + * set the shared ACL on the parent ? + * @param asyncCall + * function may require asynchronous call depending the execution time; if time exceeds configured fixedAclMaxTransactionTime value, + * recursion is stopped using propagateOnChildren parameter(set on false) and those nodes for which the method execution was not finished + * in the classical way, will have ASPECT_PENDING_FIX_ACL, which will be used in {@link FixedAclUpdater} for later processing + */ + public void setFixedAcls(Long nodeId, Long inheritFrom, Long mergeFrom, Long sharedAclToReplace, List changes, boolean set, boolean asyncCall, boolean propagateOnChildren) + { + setFixedAcls(nodeId, inheritFrom, mergeFrom, sharedAclToReplace, changes, set, false, true, false); + } + /** * Support to set a shared ACL on a node and all of its children * @@ -379,8 +409,10 @@ public class ADMAccessControlListDAO implements AccessControlListDAO * function may require asynchronous call depending the execution time; if time exceeds configured fixedAclMaxTransactionTime value, * recursion is stopped using propagateOnChildren parameter(set on false) and those nodes for which the method execution was not finished * in the classical way, will have ASPECT_PENDING_FIX_ACL, which will be used in {@link FixedAclUpdater} for later processing + * @param forceSharedACL + * When a child node has an unexpected ACL, force it to assume the new shared ACL instead of throwing a concurrency exception. */ - public void setFixedAcls(Long nodeId, Long inheritFrom, Long mergeFrom, Long sharedAclToReplace, List changes, boolean set, boolean asyncCall, boolean propagateOnChildren) + public void setFixedAcls(Long nodeId, Long inheritFrom, Long mergeFrom, Long sharedAclToReplace, List changes, boolean set, boolean asyncCall, boolean propagateOnChildren, boolean forceSharedACL) { if (log.isDebugEnabled()) { @@ -431,14 +463,14 @@ public class ADMAccessControlListDAO implements AccessControlListDAO if (acl == null) { - propagateOnChildren = setFixAclPending(child.getId(), inheritFrom, mergeFrom, sharedAclToReplace, changes, false, asyncCall, propagateOnChildren); + propagateOnChildren = setFixAclPending(child.getId(), inheritFrom, mergeFrom, sharedAclToReplace, changes, false, asyncCall, propagateOnChildren, forceSharedACL); } else { // Still has old shared ACL or already replaced if(acl.equals(sharedAclToReplace) || acl.equals(mergeFrom) || acl.equals(currentAcl)) { - propagateOnChildren = setFixAclPending(child.getId(), inheritFrom, mergeFrom, sharedAclToReplace, changes, false, asyncCall, propagateOnChildren); + propagateOnChildren = setFixAclPending(child.getId(), inheritFrom, mergeFrom, sharedAclToReplace, changes, false, asyncCall, propagateOnChildren, forceSharedACL); } else { @@ -457,7 +489,20 @@ public class ADMAccessControlListDAO implements AccessControlListDAO } else if (dbAcl.getAclType() == ACLType.SHARED) { - throw new ConcurrencyFailureException("setFixedAcls: unexpected shared acl: "+dbAcl); + if (forceSharedACL) + { + log.warn("Forcing shared ACL on node: " + child.getId() + " ( " + + nodeDAO.getNodePair(child.getId()).getSecond() + ") - " + dbAcl); + sharedAclToReplace = acl; + propagateOnChildren = setFixAclPending(child.getId(), inheritFrom, mergeFrom, sharedAclToReplace, + changes, false, asyncCall, propagateOnChildren, forceSharedACL); + } + else + { + throw new ConcurrencyFailureException( + "setFixedAcls: unexpected shared acl: " + dbAcl + " on node " + child.getId() + " ( " + + nodeDAO.getNodePair(child.getId()).getSecond() + ")"); + } } } } @@ -506,7 +551,7 @@ public class ADMAccessControlListDAO implements AccessControlListDAO * */ private boolean setFixAclPending(Long nodeId, Long inheritFrom, Long mergeFrom, Long sharedAclToReplace, - List changes, boolean set, boolean asyncCall, boolean propagateOnChildren) + List changes, boolean set, boolean asyncCall, boolean propagateOnChildren, boolean forceSharedACL) { // check transaction time long transactionStartTime = AlfrescoTransactionSupport.getTransactionStartTime(); @@ -514,7 +559,7 @@ public class ADMAccessControlListDAO implements AccessControlListDAO if (transactionTime < fixedAclMaxTransactionTime) { // make regular method call if time is under max transaction configured time - setFixedAcls(nodeId, inheritFrom, mergeFrom, sharedAclToReplace, changes, set, asyncCall, propagateOnChildren); + setFixedAcls(nodeId, inheritFrom, mergeFrom, sharedAclToReplace, changes, set, asyncCall, propagateOnChildren, forceSharedACL); return true; } diff --git a/repository/src/main/java/org/alfresco/repo/domain/permissions/AccessControlListDAO.java b/repository/src/main/java/org/alfresco/repo/domain/permissions/AccessControlListDAO.java index 700f280c80..3d55dfb426 100644 --- a/repository/src/main/java/org/alfresco/repo/domain/permissions/AccessControlListDAO.java +++ b/repository/src/main/java/org/alfresco/repo/domain/permissions/AccessControlListDAO.java @@ -91,6 +91,11 @@ public interface AccessControlListDAO */ public List setInheritanceForChildren(NodeRef parent, Long inheritFrom, Long sharedAclToReplace, boolean asyncCall); + /** + * Set the inheritance on a given node and it's children. If an unexpected ACL occurs in a child, it can be overriden by setting forceSharedACL + */ + public List setInheritanceForChildren(NodeRef parent, Long inheritFrom, Long sharedAclToReplace, boolean asyncCall, boolean forceSharedACL); + public Long getIndirectAcl(NodeRef nodeRef); public Long getInheritedAcl(NodeRef nodeRef); diff --git a/repository/src/main/java/org/alfresco/repo/domain/permissions/FixedAclUpdater.java b/repository/src/main/java/org/alfresco/repo/domain/permissions/FixedAclUpdater.java index 8812ff5028..2aa3d8b691 100644 --- a/repository/src/main/java/org/alfresco/repo/domain/permissions/FixedAclUpdater.java +++ b/repository/src/main/java/org/alfresco/repo/domain/permissions/FixedAclUpdater.java @@ -38,6 +38,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.alfresco.model.ContentModel; import org.alfresco.repo.batch.BatchProcessWorkProvider; import org.alfresco.repo.batch.BatchProcessor; +import org.alfresco.repo.batch.BatchProcessor.BatchProcessWorker; import org.alfresco.repo.domain.node.NodeDAO; import org.alfresco.repo.domain.node.NodeDAO.NodeRefQueryCallback; import org.alfresco.repo.lock.JobLockService; @@ -50,6 +51,7 @@ import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.security.permissions.PermissionServicePolicies; import org.alfresco.repo.security.permissions.PermissionServicePolicies.OnInheritPermissionsDisabled; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; +import org.alfresco.repo.transaction.RetryingTransactionHelper; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.repo.transaction.TransactionListenerAdapter; import org.alfresco.service.cmr.repository.NodeRef; @@ -64,6 +66,8 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.ConcurrencyFailureException; /** * Finds nodes with ASPECT_PENDING_FIX_ACL aspect and sets fixed ACLs for them @@ -91,6 +95,7 @@ public class FixedAclUpdater extends TransactionListenerAdapter implements Appli private int maxItemBatchSize = 100; private int numThreads = 4; + private boolean forceSharedACL = false; private ClassPolicyDelegate onInheritPermissionsDisabledDelegate; private PolicyComponent policyComponent; @@ -132,6 +137,11 @@ public class FixedAclUpdater extends TransactionListenerAdapter implements Appli this.maxItemBatchSize = maxItemBatchSize; } + public void setForceSharedACL(boolean forceSharedACL) + { + this.forceSharedACL = forceSharedACL; + } + public void setLockTimeToLive(long lockTimeToLive) { this.lockTimeToLive = lockTimeToLive; @@ -253,7 +263,7 @@ public class FixedAclUpdater extends TransactionListenerAdapter implements Appli { } - public void process(final NodeRef nodeRef) throws Throwable + public void process(final NodeRef nodeRef) { RunAsWork findAndUpdateAclRunAsWork = new RunAsWork() { @@ -265,34 +275,44 @@ public class FixedAclUpdater extends TransactionListenerAdapter implements Appli log.debug(String.format("Processing node %s", nodeRef)); } - final Long nodeId = nodeDAO.getNodePair(nodeRef).getFirst(); - - // MNT-22009 - If node was deleted and in archive store, remove the aspect and properties and do not - // process - if (nodeRef.getStoreRef().equals(StoreRef.STORE_REF_ARCHIVE_SPACESSTORE)) + try { + final Long nodeId = nodeDAO.getNodePair(nodeRef).getFirst(); + + // MNT-22009 - If node was deleted and in archive store, remove the aspect and properties and do + // not + // process + if (nodeRef.getStoreRef().equals(StoreRef.STORE_REF_ARCHIVE_SPACESSTORE)) + { + accessControlListDAO.removePendingAclAspect(nodeId); + return null; + } + + // retrieve acl properties from node + Long inheritFrom = (Long) nodeDAO.getNodeProperty(nodeId, ContentModel.PROP_INHERIT_FROM_ACL); + Long sharedAclToReplace = (Long) nodeDAO.getNodeProperty(nodeId, ContentModel.PROP_SHARED_ACL_TO_REPLACE); + + // set inheritance using retrieved prop + accessControlListDAO.setInheritanceForChildren(nodeRef, inheritFrom, sharedAclToReplace, true, + forceSharedACL); + + // Remove aspect accessControlListDAO.removePendingAclAspect(nodeId); - return null; + + if (!policyIgnoreUtil.ignorePolicy(nodeRef)) + { + boolean transformedToAsyncOperation = toBoolean((Boolean) AlfrescoTransactionSupport + .getResource(FixedAclUpdater.FIXED_ACL_ASYNC_REQUIRED_KEY)); + + OnInheritPermissionsDisabled onInheritPermissionsDisabledPolicy = onInheritPermissionsDisabledDelegate + .get(ContentModel.TYPE_BASE); + onInheritPermissionsDisabledPolicy.onInheritPermissionsDisabled(nodeRef, transformedToAsyncOperation); + } } - - // retrieve acl properties from node - Long inheritFrom = (Long) nodeDAO.getNodeProperty(nodeId, ContentModel.PROP_INHERIT_FROM_ACL); - Long sharedAclToReplace = (Long) nodeDAO.getNodeProperty(nodeId, ContentModel.PROP_SHARED_ACL_TO_REPLACE); - - // set inheritance using retrieved prop - accessControlListDAO.setInheritanceForChildren(nodeRef, inheritFrom, sharedAclToReplace, true); - - // Remove aspect - accessControlListDAO.removePendingAclAspect(nodeId); - - if (!policyIgnoreUtil.ignorePolicy(nodeRef)) + catch (Exception e) { - boolean transformedToAsyncOperation = toBoolean( - (Boolean) AlfrescoTransactionSupport.getResource(FixedAclUpdater.FIXED_ACL_ASYNC_REQUIRED_KEY)); - - OnInheritPermissionsDisabled onInheritPermissionsDisabledPolicy = onInheritPermissionsDisabledDelegate - .get(ContentModel.TYPE_BASE); - onInheritPermissionsDisabledPolicy.onInheritPermissionsDisabled(nodeRef, transformedToAsyncOperation); + log.error("Job could not process pending ACL node " + nodeRef + ": " + e); + e.printStackTrace(); } if (log.isDebugEnabled()) @@ -308,6 +328,7 @@ public class FixedAclUpdater extends TransactionListenerAdapter implements Appli AuthenticationUtil.runAs(findAndUpdateAclRunAsWork, AuthenticationUtil.getSystemUserName()); } }; + private class GetNodesWithAspectCallback implements NodeRefQueryCallback { diff --git a/repository/src/main/resources/alfresco/public-services-security-context.xml b/repository/src/main/resources/alfresco/public-services-security-context.xml index a0f20a0ff6..0129319643 100644 --- a/repository/src/main/resources/alfresco/public-services-security-context.xml +++ b/repository/src/main/resources/alfresco/public-services-security-context.xml @@ -117,6 +117,7 @@ + diff --git a/repository/src/main/resources/alfresco/repository.properties b/repository/src/main/resources/alfresco/repository.properties index ca8b038954..661ea44a49 100644 --- a/repository/src/main/resources/alfresco/repository.properties +++ b/repository/src/main/resources/alfresco/repository.properties @@ -1082,6 +1082,8 @@ system.fixedACLsUpdater.lockTTL=10000 system.fixedACLsUpdater.maxItemBatchSize=100 # fixedACLsUpdater - the number of threads to use system.fixedACLsUpdater.numThreads=4 +# fixedACLsUpdater - Force shared ACL to propagate through children even if there is an unexpected ACL +system.fixedACLsUpdater.forceSharedACL=false # fixedACLsUpdater cron expression - fire at midnight every day system.fixedACLsUpdater.cronExpression=0 0 0 * * ? diff --git a/repository/src/test/java/org/alfresco/repo/domain/permissions/FixedAclUpdaterTest.java b/repository/src/test/java/org/alfresco/repo/domain/permissions/FixedAclUpdaterTest.java index 5c62103894..cebd5b8070 100644 --- a/repository/src/test/java/org/alfresco/repo/domain/permissions/FixedAclUpdaterTest.java +++ b/repository/src/test/java/org/alfresco/repo/domain/permissions/FixedAclUpdaterTest.java @@ -90,8 +90,8 @@ public class FixedAclUpdaterTest extends TestCase private CheckOutCheckInService checkOutCheckInService; private ContentService contentService; private AuthorityService authorityService; - private static final long MAX_TRANSACTION_TIME_DEFAULT = 50; - private static final int[] filesPerLevelMoreFolders = { 5, 3, 1, 50 }; + private static final long MAX_TRANSACTION_TIME_DEFAULT = 10; + private static final int[] filesPerLevelMoreFolders = { 5, 1, 1, 1, 1, 1, 1 }; private static final int[] filesPerLevelMoreFiles = { 5, 100 }; private long maxTransactionTime; private static HashMap> errors; @@ -306,7 +306,7 @@ public class FixedAclUpdaterTest extends TestCase public void testSyncCopyNoTimeOut() throws FileExistsException, FileNotFoundException { NodeRef originalRef = createFolderHierarchyInRootForFolderTests("originFolder"); - NodeRef targetRef = createFolderHierarchyInRootForFolderTests("targetFolder"); + NodeRef targetRefBase = createFolderHierarchyInRootForFolderTests("targetFolder"); // Get ACLS for later comparison ACLComparator aclComparatorOrigin = new ACLComparator(originalRef); @@ -316,6 +316,19 @@ public class FixedAclUpdaterTest extends TestCase maxTransactionTime = 86400000; setFixedAclMaxTransactionTime(permissionsDaoComponent, homeFolderNodeRef, maxTransactionTime); + // Set permissions on target folder + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + permissionService.setInheritParentPermissions(targetRefBase, true, false); + permissionService.setPermission(targetRefBase, TEST_GROUP_NAME_FULL, PermissionService.CONSUMER, true); + return null; + }, false, true); + + // Trigger the job so the target folder structure has a different base ACL + triggerFixedACLJob(); + assertEquals("Not all nodes were processed", 0, getNodesCountWithPendingFixedAclAspect()); + + NodeRef targetRef = nodeDAO.getNodePair(getChild(nodeDAO.getNodePair(targetRefBase).getFirst())).getSecond(); + // Set Shared permissions on origin permissionService.setInheritParentPermissions(originalRef, true, false); permissionService.setPermission(originalRef, TEST_GROUP_NAME_FULL, PermissionService.COORDINATOR, true); @@ -343,7 +356,7 @@ public class FixedAclUpdaterTest extends TestCase finally { deleteNodes(originalRef); - deleteNodes(targetRef); + deleteNodes(targetRefBase); } } @@ -354,14 +367,26 @@ public class FixedAclUpdaterTest extends TestCase public void testAsyncWithNodeCopy() { NodeRef folderRef = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeCopyOriginFolder"); - NodeRef targetRef = createFile(fileFolderService, homeFolderNodeRef, "testAsyncWithNodeCopyTargetFolder", - ContentModel.TYPE_FOLDER); - - // Get ACLS for later comparison - ACLComparator aclComparatorTarget = new ACLComparator(targetRef); + NodeRef targetRefBase = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeCopyTargetFolder"); try { + // Set permissions on target folder + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + permissionService.setInheritParentPermissions(targetRefBase, true, false); + permissionService.setPermission(targetRefBase, TEST_GROUP_NAME_FULL, PermissionService.CONSUMER, true); + return null; + }, false, true); + + // Trigger the job so the target folder structure has a different base ACL + triggerFixedACLJob(); + assertEquals("Not all nodes were processed", 0, getNodesCountWithPendingFixedAclAspect()); + + NodeRef targetRef = nodeDAO.getNodePair(getChild(nodeDAO.getNodePair(targetRefBase).getFirst())).getSecond(); + + // Get ACLS for later comparison + ACLComparator aclComparatorTarget = new ACLComparator(targetRef); + // Set permissions on target folder txnHelper.doInTransaction((RetryingTransactionCallback) () -> { permissionService.setInheritParentPermissions(targetRef, false, false); @@ -410,7 +435,7 @@ public class FixedAclUpdaterTest extends TestCase finally { deleteNodes(folderRef); - deleteNodes(targetRef); + deleteNodes(targetRefBase); } } @@ -421,13 +446,26 @@ public class FixedAclUpdaterTest extends TestCase public void testAsyncWithNodeCopyToPendingFolder() { NodeRef folderRef = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeCopyOriginFolder"); - NodeRef targetRef = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeCopyTargetFolder"); - - // Get ACLS for later comparison - ACLComparator aclComparatorTarget = new ACLComparator(targetRef); + NodeRef targetRefBase = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeCopyTargetFolder"); try { + // Set permissions on target folder + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + permissionService.setInheritParentPermissions(targetRefBase, true, false); + permissionService.setPermission(targetRefBase, TEST_GROUP_NAME_FULL, PermissionService.CONSUMER, true); + return null; + }, false, true); + + // Trigger the job so the target folder structure has a different base ACL + triggerFixedACLJob(); + assertEquals("Not all nodes were processed", 0, getNodesCountWithPendingFixedAclAspect()); + + NodeRef targetRef = nodeDAO.getNodePair(getChild(nodeDAO.getNodePair(targetRefBase).getFirst())).getSecond(); + + // Get ACLS for later comparison + ACLComparator aclComparatorTarget = new ACLComparator(targetRef); + // Set permissions on target folder txnHelper.doInTransaction((RetryingTransactionCallback) () -> { permissionService.setInheritParentPermissions(targetRef, false, false); @@ -487,7 +525,7 @@ public class FixedAclUpdaterTest extends TestCase finally { deleteNodes(folderRef); - deleteNodes(targetRef); + deleteNodes(targetRefBase); } } @@ -499,13 +537,26 @@ public class FixedAclUpdaterTest extends TestCase public void testAsyncWithNodeCopyParentToChildPendingFolder() { NodeRef folderRef = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeCopyOriginFolder"); - NodeRef targetRef = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeCopyTargetFolder"); - - // Get ACLS for later comparison - ACLComparator aclComparatorTarget = new ACLComparator(targetRef); + NodeRef targetRefBase = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeCopyTargetFolder"); try { + // Set permissions on target folder + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + permissionService.setInheritParentPermissions(targetRefBase, true, false); + permissionService.setPermission(targetRefBase, TEST_GROUP_NAME_FULL, PermissionService.CONSUMER, true); + return null; + }, false, true); + + // Trigger the job so the target folder structure has a different base ACL + triggerFixedACLJob(); + assertEquals("Not all nodes were processed", 0, getNodesCountWithPendingFixedAclAspect()); + + NodeRef targetRef = nodeDAO.getNodePair(getChild(nodeDAO.getNodePair(targetRefBase).getFirst())).getSecond(); + + // Get ACLS for later comparison + ACLComparator aclComparatorTarget = new ACLComparator(targetRef); + // Set permissions on target folder txnHelper.doInTransaction((RetryingTransactionCallback) () -> { permissionService.setInheritParentPermissions(targetRef, false, false); @@ -585,7 +636,151 @@ public class FixedAclUpdaterTest extends TestCase finally { deleteNodes(folderRef); - deleteNodes(targetRef); + deleteNodes(targetRefBase); + } + } + + /* + * Move child of node that has the aspect to a child folder of a folder that also has the aspect applied before job + * runs + */ + @Test + public void testAsyncWithNodeMoveChildToChildPendingFolder() + { + NodeRef folderRef = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeMoveChildToChildPendingFolderOrigin"); + NodeRef targetRefBase = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeMoveChildToChildPendingFolderTarget"); + + try + { + // Set permissions on target folder + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + permissionService.setInheritParentPermissions(targetRefBase, true, false); + permissionService.setPermission(targetRefBase, TEST_GROUP_NAME_FULL, PermissionService.CONSUMER, true); + return null; + }, false, true); + + // Trigger the job so the target folder structure has a different base ACL + triggerFixedACLJob(); + assertEquals("Not all nodes were processed", 0, getNodesCountWithPendingFixedAclAspect()); + + NodeRef targetRef = nodeDAO.getNodePair(getChild(nodeDAO.getNodePair(targetRefBase).getFirst())).getSecond(); + + // Set permissions on a child to get a new shared ACL with pending acl nodes + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + permissionService.setInheritParentPermissions(targetRef, true, false); + permissionService.setPermission(targetRef, TEST_GROUP_NAME_FULL, PermissionService.COORDINATOR, true); + return null; + }, false, true); + + // Get target Folder with a pending ACL + NodeRef targetFolderWithPendingAcl = getFirstNodeWithAclPending(ContentModel.TYPE_FOLDER, targetRef); + assertNotNull("No children folders were found with pendingFixACl aspect", targetFolderWithPendingAcl); + NodeRef targetFolderWithPendingAclChild = nodeDAO + .getNodePair(getChild(nodeDAO.getNodePair(targetFolderWithPendingAcl).getFirst())).getSecond(); + + // Get ACLS for later comparison + ACLComparator aclComparatorTarget = new ACLComparator(targetFolderWithPendingAcl); + aclComparatorTarget.setOriginalPermission(TEST_GROUP_NAME_FULL, PermissionService.COORDINATOR); + + // Set permissions on origin folder + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + permissionService.setInheritParentPermissions(folderRef, true, false); + permissionService.setPermission(folderRef, TEST_GROUP_NAME_FULL, DEFAULT_PERMISSION, true); + return null; + }, false, true); + + // Find a pending ACL folder + NodeRef originFolderWithPendingAcl = getFirstNodeWithAclPending(ContentModel.TYPE_FOLDER, folderRef); + assertNotNull("No children folders were found with pendingFixACl aspect", originFolderWithPendingAcl); + NodeRef originFolderWithPendingAclChild = nodeDAO + .getNodePair(getChild(nodeDAO.getNodePair(originFolderWithPendingAcl).getFirst())).getSecond(); + + // Get ACLS for later comparison + ACLComparator aclComparatorMovedNode = new ACLComparator(originFolderWithPendingAclChild); + aclComparatorMovedNode.setOriginalPermission(TEST_GROUP_NAME_FULL, DEFAULT_PERMISSION); + + // Move one pending folder into the other + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + fileFolderService.move(originFolderWithPendingAclChild, targetFolderWithPendingAclChild, "movedFolder"); + return null; + }, false, true); + + // Trigger job + triggerFixedACLJob(); + + assertEquals("Not all nodes were processed", 0, getNodesCountWithPendingFixedAclAspect()); + assertTrue("Moved node did not inherit permissions from target", + aclComparatorMovedNode.hasPermission(TEST_GROUP_NAME_FULL, PermissionService.COORDINATOR)); + assertTrue("Child of Pending Moved node did not inherit permissions from target", + aclComparatorMovedNode.firstChildHasPermission(TEST_GROUP_NAME_FULL, PermissionService.COORDINATOR)); + assertFalse("Moved node kept original permissions", aclComparatorMovedNode.parentHasOriginalPermission()); + assertFalse("Child of Moved node kept original permissions", + aclComparatorMovedNode.firstChildHasOriginalPermission()); + } + finally + { + deleteNodes(folderRef); + deleteNodes(targetRefBase); + } + } + + /* + * Create a conflicting ACL on a node and then try to run the job normally, without forcing the ACL to get the + * expected error and then run it again with the forcedShareACL property as true so it can override the problematic + * ACL + */ + @Test + public void testAsyncWithErrorsForceSharedACL() + { + NodeRef folderRef = createFolderHierarchyInRootForFolderTests("testAsyncWithErrorsForceSharedACL"); + + try + { + // Set permissions on origin folder + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + permissionService.setInheritParentPermissions(folderRef, true, false); + permissionService.setPermission(folderRef, TEST_GROUP_NAME_FULL, PermissionService.COORDINATOR, true); + return null; + }, false, true); + + // Find a pending ACL folder + NodeRef originFolderWithPendingAcl = getFirstNodeWithAclPending(ContentModel.TYPE_FOLDER, folderRef); + assertNotNull("No children folders were found with pendingFixACl aspect", originFolderWithPendingAcl); + NodeRef originFolderWithPendingAclChild = nodeDAO + .getNodePair(getChild(nodeDAO.getNodePair(originFolderWithPendingAcl).getFirst())).getSecond(); + + // Create a new ACL elsewhere and put the shared ACL (from a child) on the pending node child to simulate + // conflict + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + NodeRef tempNode = createFile(fileFolderService, folderRef, "testAsyncWithErrorsForceSharedACLTemp", + ContentModel.TYPE_FOLDER); + permissionService.setInheritParentPermissions(tempNode, false, false); + permissionService.setPermission(tempNode, TEST_GROUP_NAME_FULL, PermissionService.CONSUMER, true); + NodeRef tempNodeChild = createFile(fileFolderService, tempNode, "testAsyncWithErrorsForceSharedACLTempChild", + ContentModel.TYPE_FOLDER); + setACL(permissionsDaoComponent, originFolderWithPendingAclChild, + nodeDAO.getNodeAclId(nodeDAO.getNodePair(tempNodeChild).getFirst())); + return null; + }, false, true); + + ACLComparator aclComparator = new ACLComparator(originFolderWithPendingAclChild); + + // Trigger job without forcing the shared ACL, only 1 error is expected + triggerFixedACLJob(false); + assertEquals("Unexpected number of errors", 1, getNodesCountWithPendingFixedAclAspect()); + + // Trigger job forcing the shared ACL + triggerFixedACLJob(true); + + assertEquals("Not all nodes were processed", 0, getNodesCountWithPendingFixedAclAspect()); + assertTrue("Child of node with conflict does not have correct permissions", + aclComparator.firstChildHasPermission(TEST_GROUP_NAME_FULL, PermissionService.COORDINATOR)); + assertTrue("Node with conflict does not have correct permissions", + aclComparator.hasPermission(TEST_GROUP_NAME_FULL, PermissionService.COORDINATOR)); + } + finally + { + deleteNodes(folderRef); } } @@ -596,14 +791,26 @@ public class FixedAclUpdaterTest extends TestCase public void testAsyncWithNodeMove() { NodeRef folderRef = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeMoveOriginFolder"); - NodeRef targetRef = createFile(fileFolderService, homeFolderNodeRef, "testAsyncWithNodeMoveTargetFolder", - ContentModel.TYPE_FOLDER); - - // Get ACLS for later comparison - ACLComparator aclComparatorTarget = new ACLComparator(targetRef); + NodeRef targetRefBase = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeMoveTargetFolder"); try { + // Set permissions on target folder + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + permissionService.setInheritParentPermissions(targetRefBase, true, false); + permissionService.setPermission(targetRefBase, TEST_GROUP_NAME_FULL, PermissionService.CONSUMER, true); + return null; + }, false, true); + + // Trigger the job so the target folder structure has a different base ACL + triggerFixedACLJob(); + assertEquals("Not all nodes were processed", 0, getNodesCountWithPendingFixedAclAspect()); + + NodeRef targetRef = nodeDAO.getNodePair(getChild(nodeDAO.getNodePair(targetRefBase).getFirst())).getSecond(); + + // Get ACLS for later comparison + ACLComparator aclComparatorTarget = new ACLComparator(targetRef); + // Set permissions on target folder txnHelper.doInTransaction((RetryingTransactionCallback) () -> { permissionService.setInheritParentPermissions(targetRef, false, false); @@ -649,7 +856,7 @@ public class FixedAclUpdaterTest extends TestCase finally { deleteNodes(folderRef); - deleteNodes(targetRef); + deleteNodes(targetRefBase); } } @@ -660,13 +867,27 @@ public class FixedAclUpdaterTest extends TestCase public void testAsyncWithNodeMoveToPendingFolder() { NodeRef folderRef = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeMoveOriginFolder"); - NodeRef targetRef = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeMoveTargetFolder"); - - // Get ACLS for later comparison - ACLComparator aclComparatorTarget = new ACLComparator(targetRef); + NodeRef targetRefBase = createFolderHierarchyInRootForFolderTests("testAsyncWithNodeMoveTargetFolder"); try { + + // Set permissions on target folder + txnHelper.doInTransaction((RetryingTransactionCallback) () -> { + permissionService.setInheritParentPermissions(targetRefBase, true, false); + permissionService.setPermission(targetRefBase, TEST_GROUP_NAME_FULL, PermissionService.CONSUMER, true); + return null; + }, false, true); + + // Trigger the job so the target folder structure has a different base ACL + triggerFixedACLJob(); + assertEquals("Not all nodes were processed", 0, getNodesCountWithPendingFixedAclAspect()); + + NodeRef targetRef = nodeDAO.getNodePair(getChild(nodeDAO.getNodePair(targetRefBase).getFirst())).getSecond(); + + // Get ACLS for later comparison + ACLComparator aclComparatorTarget = new ACLComparator(targetRef); + // Set permissions on target folder txnHelper.doInTransaction((RetryingTransactionCallback) () -> { permissionService.setInheritParentPermissions(targetRef, false, false); @@ -723,7 +944,7 @@ public class FixedAclUpdaterTest extends TestCase finally { deleteNodes(folderRef); - deleteNodes(targetRef); + deleteNodes(targetRefBase); } } @@ -1250,6 +1471,19 @@ public class FixedAclUpdaterTest extends TestCase } } + private static void setACL(PermissionsDaoComponent permissionsDaoComponent, NodeRef nodeRef, long aclId) + { + if (permissionsDaoComponent instanceof ADMPermissionsDaoComponentImpl) + { + AccessControlListDAO acldao = ((ADMPermissionsDaoComponentImpl) permissionsDaoComponent).getACLDAO(nodeRef); + if (acldao instanceof ADMAccessControlListDAO) + { + ADMAccessControlListDAO admAcLDao = (ADMAccessControlListDAO) acldao; + admAcLDao.setAccessControlList(nodeRef, aclId); + } + } + } + private NodeRef createFolderHierarchyInRoot(String folderName, int[] filesPerLevel) { return txnHelper.doInTransaction((RetryingTransactionCallback) () -> { @@ -1318,6 +1552,11 @@ public class FixedAclUpdaterTest extends TestCase } private void triggerFixedACLJob() + { + triggerFixedACLJob(false); + } + + private void triggerFixedACLJob(boolean forceSharedACL) { // run the fixedAclUpdater until there is nothing more to fix (running the updater may create more to fix up) or // the count doesn't change for 3 cycles, meaning we have a problem. @@ -1325,6 +1564,7 @@ public class FixedAclUpdaterTest extends TestCase int count = 0; int previousCount = 0; int rounds = 0; + fixedAclUpdater.setForceSharedACL(forceSharedACL); do { previousCount = count; @@ -1356,8 +1596,13 @@ public class FixedAclUpdaterTest extends TestCase isDescendent = true; } } + if (isDescendent && nodeDAO.getNodeType(nodeDAO.getNodePair(nodeRef).getFirst()).equals(nodeType)) { + // If folder, the tests will need a child and a grandchild to verify permissions + if (nodeType.equals(ContentModel.TYPE_FOLDER) && !hasGrandChilden(nodeRef)) { + continue; + } return nodeRef; } } @@ -1377,6 +1622,10 @@ public class FixedAclUpdaterTest extends TestCase NodeRef nodeRef = nodesWithAclPendingAspect.get(i); if (nodeDAO.getNodeType(nodeDAO.getNodePair(nodeRef).getFirst()).equals(nodeType)) { + // If folder, the tests will need a child and a grandchild to verify permissions + if (nodeType.equals(ContentModel.TYPE_FOLDER) && !hasGrandChilden(nodeRef)) { + continue; + } return nodeRef; } } @@ -1385,6 +1634,18 @@ public class FixedAclUpdaterTest extends TestCase } + private boolean hasGrandChilden(NodeRef nodeRef) + { + Long nodeId = nodeDAO.getNodePair(nodeRef).getFirst(); + Long childId = getChild(nodeId); + Long grandChild = null; + if (childId != null) + { + grandChild = getChild(childId); + } + return (grandChild != null); + } + private void deleteNodes(NodeRef folder) { txnHelper.doInTransaction((RetryingTransactionCallback) () -> {