diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties index fa48e03455..c91fb2c11a 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties @@ -2,4 +2,8 @@ rm.hold.not-hold=The node {0} is not a hold. rm.hold.add-to-hold-invalid-type={0} is neither a record nor a record folder nor active content. Only records, record \ folders or active content can be added to a hold. rm.hold.add-to-hold-archived-node=Archived nodes can't be added to hold. -rm.hold.add-to-hold-locked-node=Locked nodes can't be added to hold. \ No newline at end of file +rm.hold.add-to-hold-locked-node=Locked nodes can't be added to hold. +rm.hold.delete-frozen-node=Frozen nodes can not be deleted. +rm.hold.delete-node-frozen-children=Can not delete node, because it contains a frozen child node. +rm.hold.move-frozen-node=Frozen nodes can not be moved. +rm.hold.update-frozen-node=Frozen nodes can not be updated. \ No newline at end of file diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java index 1b8bbe0401..a4e7c868af 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java @@ -205,6 +205,8 @@ public class HoldServiceImpl extends ServiceBaseImpl List frozenNodes = getHeld(hold); for (NodeRef frozenNode : frozenNodes) { + //set in transaction cache in order not to trigger update policy when removing the child association + transactionalResourceHelper.getSet("frozen").add(frozenNode); removeFreezeAspect(frozenNode, 1); } @@ -231,6 +233,8 @@ public class HoldServiceImpl extends ServiceBaseImpl if (nodeService.hasAspect(nodeRef, ASPECT_FROZEN)) { // remove the freeze aspect from the node + //set in transaction cache in order not to trigger update policy when removing the aspect + transactionalResourceHelper.getSet("frozen").add(nodeRef); nodeService.removeAspect(nodeRef, ASPECT_FROZEN); } @@ -245,6 +249,8 @@ public class HoldServiceImpl extends ServiceBaseImpl if (recordsOtherHolds.size() == index) { // remove the freeze aspect from the node + //set in transaction cache in order not to trigger update policy when removing the aspect + transactionalResourceHelper.getSet("frozen").add(record); nodeService.removeAspect(record, ASPECT_FROZEN); } } @@ -646,7 +652,7 @@ public class HoldServiceImpl extends ServiceBaseImpl if (!nodeService.hasAspect(nodeRef, ASPECT_FROZEN)) { //set in transaction cache in order not to trigger update policy when adding the aspect - transactionalResourceHelper.getSet(nodeRef).add("frozen"); + transactionalResourceHelper.getSet("frozen").add(nodeRef); // add freeze aspect nodeService.addAspect(nodeRef, ASPECT_FROZEN, props); @@ -732,33 +738,26 @@ public class HoldServiceImpl extends ServiceBaseImpl { // run as system so we don't run into further permission issues // we already know we have to have the correct capability to get here - authenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() - { - // remove from hold - nodeService.removeChild(hold, nodeRef); + authenticationUtil.runAsSystem((RunAsWork) () -> { + // remove from hold + //set in transaction cache in order not to trigger update policy when removing the child association + transactionalResourceHelper.getSet("frozen").add(nodeRef); + nodeService.removeChild(hold, nodeRef); - // audit that the node has been remove from the hold - // TODO add details of the hold that the node was removed from - recordsManagementAuditService.auditEvent(nodeRef, AUDIT_REMOVE_FROM_HOLD); + // audit that the node has been remove from the hold + // TODO add details of the hold that the node was removed from + recordsManagementAuditService.auditEvent(nodeRef, AUDIT_REMOVE_FROM_HOLD); - return null; - } - }); + return null; + + }); } } // run as system as we can't be sure if have remove aspect rights on node - authenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() - { - removeFreezeAspect(nodeRef, 0); - return null; - } + authenticationUtil.runAsSystem((RunAsWork) () -> { + removeFreezeAspect(nodeRef, 0); + return null; }); } } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java index 42ce032b8d..28b61a2d3f 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java @@ -38,8 +38,6 @@ import java.util.Map; import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean; -import org.alfresco.repo.content.ContentServicePolicies; -import org.alfresco.repo.copy.CopyServicePolicies; import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.annotation.Behaviour; @@ -47,10 +45,11 @@ import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.repo.policy.annotation.BehaviourKind; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; -import org.alfresco.repo.security.permissions.AccessDeniedException; +import org.alfresco.rest.framework.core.exceptions.PermissionDeniedException; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.namespace.QName; +import org.springframework.extensions.surf.util.I18NUtil; /** * rma:frozen behaviour bean @@ -67,9 +66,7 @@ public class FrozenAspect extends BaseBehaviourBean NodeServicePolicies.OnAddAspectPolicy, NodeServicePolicies.OnRemoveAspectPolicy, NodeServicePolicies.OnUpdatePropertiesPolicy, - NodeServicePolicies.OnMoveNodePolicy, - ContentServicePolicies.OnContentUpdatePolicy, - CopyServicePolicies.BeforeCopyPolicy + NodeServicePolicies.BeforeMoveNodePolicy { /** freeze service */ protected FreezeService freezeService; @@ -99,7 +96,7 @@ public class FrozenAspect extends BaseBehaviourBean if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) { // never allow to delete a frozen node - throw new AccessDeniedException("Frozen nodes can not be deleted."); + throw new PermissionDeniedException(I18NUtil.getMessage("rm.hold.delete-frozen-node")); } // check children @@ -125,7 +122,7 @@ public class FrozenAspect extends BaseBehaviourBean if (freezeService.isFrozen(nodeRef)) { // never allow to delete a node with a frozen child - throw new AccessDeniedException("Can not delete node, because it contains a frozen child node."); + throw new PermissionDeniedException(I18NUtil.getMessage("rm.hold.delete-node-frozen-children")); } // check children @@ -209,16 +206,16 @@ public class FrozenAspect extends BaseBehaviourBean @Override @Behaviour ( - kind = BehaviourKind.ASSOCIATION, + kind = BehaviourKind.CLASS, notificationFrequency = NotificationFrequency.FIRST_EVENT ) - public void onMoveNode(final ChildAssociationRef oldChildAssocRef, final ChildAssociationRef newChildAssocRef) + public void beforeMoveNode(ChildAssociationRef oldChildAssocRef, NodeRef newParentRef) { AuthenticationUtil.runAsSystem((RunAsWork) () -> { - if (nodeService.exists(newChildAssocRef.getParentRef()) && - nodeService.exists(newChildAssocRef.getChildRef())) + if (nodeService.exists(oldChildAssocRef.getChildRef()) && + freezeService.isFrozen(oldChildAssocRef.getChildRef())) { - throw new AccessDeniedException("Frozen nodes can not be moved."); + throw new PermissionDeniedException(I18NUtil.getMessage("rm.hold.move-frozen-node")); } return null; }); @@ -233,59 +230,18 @@ public class FrozenAspect extends BaseBehaviourBean @Behaviour ( kind = BehaviourKind.CLASS, - notificationFrequency = NotificationFrequency.TRANSACTION_COMMIT + notificationFrequency = NotificationFrequency.FIRST_EVENT ) public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) - { AuthenticationUtil.runAsSystem((RunAsWork) () -> { // check to not throw exception when the aspect is being added if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef) && - !transactionalResourceHelper.getSet(nodeRef).contains("frozen") ) + !transactionalResourceHelper.getSet("frozen").contains(nodeRef) ) { - throw new AccessDeniedException("Frozen nodes can not be updated."); + throw new PermissionDeniedException(I18NUtil.getMessage("rm.hold.update-frozen-node")); } return null; }); } - - /** - * Behaviour associated with updating the content - *

- * Ensures that the content of a frozen node can not be updated - */ - @Override - @Behaviour - ( - kind = BehaviourKind.CLASS, - notificationFrequency = NotificationFrequency.TRANSACTION_COMMIT - ) - public void onContentUpdate(NodeRef nodeRef, boolean newContent) - { - AuthenticationUtil.runAsSystem((RunAsWork) () -> { - if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) - { - // never allow to update the content of a frozen node - throw new AccessDeniedException("Frozen nodes content can not be updated."); - } - return null; - }); - } - - @Override - @Behaviour - ( - kind = BehaviourKind.CLASS - ) - public void beforeCopy(QName classRef, NodeRef sourceNodeRef, NodeRef targetNodeRef) - { - AuthenticationUtil.runAsSystem((RunAsWork) () -> { - if (nodeService.exists(sourceNodeRef) && freezeService.isFrozen(sourceNodeRef)) - { - throw new AccessDeniedException("Frozen nodes can not be copied."); - } - - return null; - }); - } } diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/UpdateHeldActiveContentTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/UpdateHeldActiveContentTest.java new file mode 100644 index 0000000000..a50c65a3b8 --- /dev/null +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/UpdateHeldActiveContentTest.java @@ -0,0 +1,212 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2019 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * - + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * - + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.module.org_alfresco_module_rm.test.integration.hold; + +import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase; +import org.alfresco.repo.content.MimetypeMap; +import org.alfresco.repo.security.permissions.AccessDeniedException; +import org.alfresco.rest.framework.core.exceptions.PermissionDeniedException; +import org.alfresco.service.cmr.model.FileNotFoundException; +import org.alfresco.service.cmr.repository.ContentData; +import org.alfresco.service.cmr.repository.NodeRef; +import org.springframework.extensions.webscripts.GUID; + +/** + * Prevent Updating Held Active Content Integration Tests + * + * @author Claudia Agache + * @since 3.2 + */ +public class UpdateHeldActiveContentTest extends BaseRMTestCase +{ + @Override + protected boolean isCollaborationSiteTest() + { + return true; + } + + /** + * Given active content on hold + * When I try to delete the content + * Then I am not successful + */ + public void testDeleteHeldDocument() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + public void given() + { + // create a hold + NodeRef hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // add the active content to hold + holdService.addToHold(hold, dmDocument); + } + + public void when() + { + try + { + fileFolderService.delete(dmDocument); + fail("Expected PermissionDeniedException to be thrown"); + } + catch (PermissionDeniedException pde) + { + assertTrue(pde.getMessage().contains("Frozen nodes can not be deleted.")); + } + } + }); + } + + /** + * Given active content on hold + * When I try to copy the content + * Then I am not successful + */ + public void testCopyHeldDocument() + { + doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class) + { + public void given() + { + // create a hold + NodeRef hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // add the active content to hold + holdService.addToHold(hold, dmDocument); + } + + public void when() throws FileNotFoundException + { + fileFolderService.copy(dmDocument, dmFolder1, null); + } + }); + } + + /** + * Given active content on hold + * When I try to move the content + * Then I am not successful + */ + public void testMoveHeldDocument() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + public void given() + { + // create a hold + NodeRef hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // add the active content to hold + holdService.addToHold(hold, dmDocument); + } + + public void when() throws FileNotFoundException + { + try + { + fileFolderService.move(dmDocument, dmFolder1, null); + fail("Expected PermissionDeniedException to be thrown"); + } + catch (PermissionDeniedException pde) + { + assertTrue(pde.getMessage().contains("Frozen nodes can not be moved.")); + } + } + }); + } + + /** + * Given active content on hold + * When I try to edit the properties + * Or perform an action that edits the properties + * Then I am not successful + */ + public void testUpdateHeldDocumentProperties() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + public void given() + { + // create a hold + NodeRef hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // add the active content to hold + holdService.addToHold(hold, dmDocument); + } + + public void when() + { + try + { + nodeService.setProperty(dmDocument, ContentModel.PROP_DESCRIPTION, "description"); + fail("Expected PermissionDeniedException to be thrown"); + } + catch (PermissionDeniedException pde) + { + assertTrue(pde.getMessage().contains("Frozen nodes can not be updated.")); + } + } + }); + } + + /** + * Given active content on hold + * When I try to update the content + * Then I am not successful + */ + public void testUpdateHeldDocumentContent() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + public void given() + { + // create a hold + NodeRef hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // add the active content to hold + holdService.addToHold(hold, dmDocument); + } + + public void when() + { + try + { + ContentData content = (ContentData) nodeService.getProperty(dmDocument, PROP_CONTENT); + nodeService.setProperty(dmDocument, PROP_CONTENT, ContentData.setMimetype(content, + MimetypeMap.MIMETYPE_TEXT_PLAIN)); + fail("Expected PermissionDeniedException to be thrown"); + } + catch (PermissionDeniedException pde) + { + assertTrue(pde.getMessage().contains("Frozen nodes can not be updated.")); + } + } + }); + } +} diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java index 2a4010142a..90a69b6ca7 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java @@ -44,6 +44,7 @@ import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; import org.alfresco.module.org_alfresco_module_rm.util.NodeTypeUtility; import org.alfresco.module.org_alfresco_module_rm.util.TransactionalResourceHelper; import org.alfresco.repo.security.permissions.AccessDeniedException; +import org.alfresco.rest.framework.core.exceptions.PermissionDeniedException; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; @@ -166,7 +167,7 @@ public class FrozenAspectUnitTest /** * Test before delete throws an error if a node is frozen */ - @Test(expected = AccessDeniedException.class) + @Test(expected = PermissionDeniedException.class) public void testBeforeDeleteNodeThrowsExceptionIfNodeFrozen() { when(mockFreezeService.isFrozen(content)).thenReturn(true); @@ -187,7 +188,7 @@ public class FrozenAspectUnitTest /** * Test before delete throws an error for a node with frozen children */ - @Test (expected = AccessDeniedException.class) + @Test (expected = PermissionDeniedException.class) public void testBeforeDeleteThrowsExceptionForFrozenChild() { when(mockChildRef.getChildRef()).thenReturn(child); @@ -217,31 +218,33 @@ public class FrozenAspectUnitTest public void testOnAddAspectForContent() { when(mockNodeService.getType(record)).thenReturn(ContentModel.TYPE_CONTENT); + when(mockedNodeTypeUtility.instanceOf(mockNodeService.getType(record), ContentModel.TYPE_CONTENT)).thenReturn(true); when(mockNodeService.getPrimaryParent(record)).thenReturn(mockParentRef); when(mockParentRef.getParentRef()).thenReturn(parent); when(mockNodeService.hasAspect(parent, ASPECT_HELD_CHILDREN)).thenReturn(false); when(mockNodeService.getType(parent)).thenReturn(ContentModel.TYPE_FOLDER); + when(mockedNodeTypeUtility.instanceOf(mockNodeService.getType(parent), ContentModel.TYPE_FOLDER)).thenReturn(true); frozenAspect.onAddAspect(record, null); verify(mockNodeService, times(1)).addAspect(any(NodeRef.class), any(QName.class), anyMap()); } /** - * Test on move throws an error for a frozen node + * Test before move throws an error for a frozen node */ - @Test(expected = AccessDeniedException.class) - public void testOnMoveThrowsExceptionForFrozenNode() + @Test(expected = PermissionDeniedException.class) + public void testBeforeMoveThrowsExceptionForFrozenNode() { - when(mockNewRef.getParentRef()).thenReturn(parent); - when(mockNewRef.getChildRef()).thenReturn(child); - when(mockNodeService.exists(parent)).thenReturn(true); + when(mockOldRef.getParentRef()).thenReturn(parent); + when(mockOldRef.getChildRef()).thenReturn(child); when(mockNodeService.exists(child)).thenReturn(true); - frozenAspect.onMoveNode(mockOldRef, mockNewRef); + when(mockFreezeService.isFrozen(child)).thenReturn(true); + frozenAspect.beforeMoveNode(mockOldRef, null); } /** * Test update properties throws an error for frozen nodes */ - @Test(expected = AccessDeniedException.class) + @Test(expected = PermissionDeniedException.class) public void testUpdatePropertiesThrowsExceptionForFrozenNode() { when(mockFreezeService.isFrozen(content)).thenReturn(true); @@ -249,24 +252,4 @@ public class FrozenAspectUnitTest when(mockSet.contains("frozen")).thenReturn(false); frozenAspect.onUpdateProperties(content, null, null); } - - /** - * Test on content update throws an error for frozen nodes - */ - @Test(expected = AccessDeniedException.class) - public void testOnContentUpdateThrowsExceptionForFrozenNode() - { - when(mockFreezeService.isFrozen(content)).thenReturn(true); - frozenAspect.onContentUpdate(content, false); - } - - /** - * Test before copy throws an error for frozen node - */ - @Test(expected = AccessDeniedException.class) - public void testBeforeCopyThrowsExceptionForFrozenNode() - { - when(mockFreezeService.isFrozen(content)).thenReturn(true); - frozenAspect.beforeCopy(null, content, null); - } }