From 4e726d5d33c7cb7eb0c328b9227c22408044c8fd Mon Sep 17 00:00:00 2001 From: rlucanu Date: Thu, 8 Aug 2019 13:15:22 +0300 Subject: [PATCH 01/10] RM-6904 Prevent updating held active content --- .../model/rma/aspect/FrozenAspect.java | 111 +++++++++++++++++- 1 file changed, 108 insertions(+), 3 deletions(-) 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 c3dcc1f4fb..d44187f9a9 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 @@ -33,12 +33,17 @@ import static org.alfresco.repo.site.SiteModel.ASPECT_SITE_CONTAINER; import java.io.Serializable; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; +import org.alfresco.model.ApplicationModel; import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean; +import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; +import org.alfresco.repo.content.ContentServicePolicies; import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.annotation.Behaviour; @@ -47,9 +52,12 @@ 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.alfresco.util.PropertyMap; +import org.springframework.dao.PermissionDeniedDataAccessException; /** * rma:frozen behaviour bean @@ -64,7 +72,10 @@ import org.alfresco.service.namespace.QName; public class FrozenAspect extends BaseBehaviourBean implements NodeServicePolicies.BeforeDeleteNodePolicy, NodeServicePolicies.OnAddAspectPolicy, - NodeServicePolicies.OnRemoveAspectPolicy + NodeServicePolicies.OnRemoveAspectPolicy, + NodeServicePolicies.OnCreateChildAssociationPolicy, + NodeServicePolicies.OnUpdatePropertiesPolicy, + ContentServicePolicies.OnContentUpdatePolicy { /** file plan service */ protected FilePlanService filePlanService; @@ -106,8 +117,7 @@ public class FrozenAspect extends BaseBehaviourBean @Override public Void doWork() { - if (nodeService.exists(nodeRef) && - filePlanService.isFilePlanComponent(nodeRef)) + if (nodeService.exists(nodeRef)) { if (freezeService.isFrozen(nodeRef)) { @@ -221,4 +231,99 @@ public class FrozenAspect extends BaseBehaviourBean } + /** + * Behaviour associated with moving/copying a frozen node + *

+ * Prevent frozen items being moved or copied + */ + @Override + @Behaviour + ( + kind = BehaviourKind.ASSOCIATION, + notificationFrequency = NotificationFrequency.FIRST_EVENT + ) + public void onCreateChildAssociation(ChildAssociationRef childAssocRef, boolean isNewNode) + { + AuthenticationUtil.runAsSystem(new RunAsWork() + { + @Override + public Void doWork() + { + NodeRef childNodeRef = childAssocRef.getChildRef(); + if (freezeService.isFrozen(childNodeRef)) + { + // never allow to move or copy a frozen node + throw new AccessDeniedException("Frozen nodes can not be moved or copied."); + } + return null; + } + }); + } + + /** + * Behaviour associated with updating properties + *

+ * Prevents frozen items being updated + */ + @Override + @Behaviour + ( + kind = BehaviourKind.CLASS, + notificationFrequency = NotificationFrequency.TRANSACTION_COMMIT + ) + public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) + { + AuthenticationUtil.runAsSystem(new RunAsWork() + { + @Override + public Void doWork() + { + if (nodeService.exists(nodeRef)) + { + if (freezeService.isFrozen(nodeRef)) + { + // Determine the properties that have changed + Map changedProperties = PropertyMap.getChangedProperties(before, after); + // never allow to update a frozen node + if (changedProperties != null && !changedProperties.isEmpty()) + { + throw new AccessDeniedException("Frozen nodes can not be updated."); + } + } + } + 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(new RunAsWork() + { + @Override + public Void doWork() + { + if (nodeService.exists(nodeRef)) + { + if (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; + } + }); + } } From 80974603d0565cd920ba3ea3c96b674f3a50fb0b Mon Sep 17 00:00:00 2001 From: rlucanu Date: Tue, 13 Aug 2019 01:24:22 +0300 Subject: [PATCH 02/10] RM-6904 Tiding up --- .../model/rma/aspect/FrozenAspect.java | 77 ++++++++++++------- 1 file changed, 49 insertions(+), 28 deletions(-) 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 d44187f9a9..d9622c20af 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 @@ -44,6 +44,9 @@ import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.repo.content.ContentServicePolicies; +import org.alfresco.repo.copy.CopyBehaviourCallback; +import org.alfresco.repo.copy.CopyDetails; +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; @@ -73,9 +76,10 @@ public class FrozenAspect extends BaseBehaviourBean implements NodeServicePolicies.BeforeDeleteNodePolicy, NodeServicePolicies.OnAddAspectPolicy, NodeServicePolicies.OnRemoveAspectPolicy, - NodeServicePolicies.OnCreateChildAssociationPolicy, NodeServicePolicies.OnUpdatePropertiesPolicy, - ContentServicePolicies.OnContentUpdatePolicy + NodeServicePolicies.OnMoveNodePolicy, + ContentServicePolicies.OnContentUpdatePolicy, + CopyServicePolicies.BeforeCopyPolicy { /** file plan service */ protected FilePlanService filePlanService; @@ -117,17 +121,14 @@ public class FrozenAspect extends BaseBehaviourBean @Override public Void doWork() { - if (nodeService.exists(nodeRef)) + if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) { - if (freezeService.isFrozen(nodeRef)) - { - // never allowed to delete a frozen node - throw new AccessDeniedException("Frozen nodes can not be deleted."); - } - - // check children - checkChildren(nodeService.getChildAssocs(nodeRef)); + // never allow to delete a frozen node + throw new AccessDeniedException("Frozen nodes can not be deleted."); } + + // check children + checkChildren(nodeService.getChildAssocs(nodeRef)); return null; } }); @@ -208,22 +209,22 @@ public class FrozenAspect extends BaseBehaviourBean public Void doWork() { if (nodeService.exists(record) && - isRecord(record)) + isRecord(record)) { // get the owning record folder NodeRef recordFolder = nodeService.getPrimaryParent(record).getParentRef(); - + // check that the aspect has been added if (nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)) { // decrement current count - int currentCount = (Integer)nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT); + int currentCount = (Integer) nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT); if (currentCount > 0) { currentCount = currentCount - 1; nodeService.setProperty(recordFolder, PROP_HELD_CHILDREN_COUNT, currentCount); } - } + } } return null; } @@ -232,9 +233,9 @@ public class FrozenAspect extends BaseBehaviourBean } /** - * Behaviour associated with moving/copying a frozen node + * Behaviour associated with moving a frozen node *

- * Prevent frozen items being moved or copied + * Prevent frozen items being moved */ @Override @Behaviour @@ -242,22 +243,21 @@ public class FrozenAspect extends BaseBehaviourBean kind = BehaviourKind.ASSOCIATION, notificationFrequency = NotificationFrequency.FIRST_EVENT ) - public void onCreateChildAssociation(ChildAssociationRef childAssocRef, boolean isNewNode) + public void onMoveNode(final ChildAssociationRef oldChildAssocRef, final ChildAssociationRef newChildAssocRef) { - AuthenticationUtil.runAsSystem(new RunAsWork() + AuthenticationUtil.runAs(new RunAsWork() { @Override public Void doWork() { - NodeRef childNodeRef = childAssocRef.getChildRef(); - if (freezeService.isFrozen(childNodeRef)) + if (nodeService.exists(newChildAssocRef.getParentRef()) && + nodeService.exists(newChildAssocRef.getChildRef())) { - // never allow to move or copy a frozen node - throw new AccessDeniedException("Frozen nodes can not be moved or copied."); + throw new AccessDeniedException("Frozen nodes can not be moved."); } return null; } - }); + }, AuthenticationUtil.getSystemUserName()); } /** @@ -272,6 +272,7 @@ public class FrozenAspect extends BaseBehaviourBean notificationFrequency = NotificationFrequency.TRANSACTION_COMMIT ) public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) + { AuthenticationUtil.runAsSystem(new RunAsWork() { @@ -314,16 +315,36 @@ public class FrozenAspect extends BaseBehaviourBean @Override public Void doWork() { - if (nodeService.exists(nodeRef)) - { - if (freezeService.isFrozen(nodeRef)) + 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.runAs(new RunAsWork() + { + @Override + public Void doWork() + { + //nodeService.getPrimaryParent(sourceNodeRef).getParentRef() + if (nodeService.exists(sourceNodeRef) && freezeService.isFrozen(sourceNodeRef)) + { + throw new AccessDeniedException("Frozen nodes can not be copied."); + } + + return null; + } + }, AuthenticationUtil.getSystemUserName()); + } } From aed9a1abd1515b5d8532159851d4ef7d932191b1 Mon Sep 17 00:00:00 2001 From: rlucanu Date: Wed, 14 Aug 2019 12:12:54 +0300 Subject: [PATCH 03/10] RM-6904 Fixing tests --- .../rm-model-context.xml | 1 - .../hold/HoldServiceImpl.java | 2 + .../model/rma/aspect/FrozenAspect.java | 153 ++++++------------ 3 files changed, 49 insertions(+), 107 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml index a52089bd08..07adb5d9bf 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml @@ -168,7 +168,6 @@ - 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 93ca463d99..0fdb4fb3c9 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 @@ -636,6 +636,8 @@ 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"); // add freeze aspect nodeService.addAspect(nodeRef, ASPECT_FROZEN, props); 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 d9622c20af..00cdb290d2 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 @@ -33,19 +33,12 @@ import static org.alfresco.repo.site.SiteModel.ASPECT_SITE_CONTAINER; import java.io.Serializable; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; -import org.alfresco.model.ApplicationModel; -import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean; -import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.repo.content.ContentServicePolicies; -import org.alfresco.repo.copy.CopyBehaviourCallback; -import org.alfresco.repo.copy.CopyDetails; import org.alfresco.repo.copy.CopyServicePolicies; import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.policy.Behaviour.NotificationFrequency; @@ -55,12 +48,9 @@ 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.alfresco.util.PropertyMap; -import org.springframework.dao.PermissionDeniedDataAccessException; /** * rma:frozen behaviour bean @@ -81,20 +71,9 @@ public class FrozenAspect extends BaseBehaviourBean ContentServicePolicies.OnContentUpdatePolicy, CopyServicePolicies.BeforeCopyPolicy { - /** file plan service */ - protected FilePlanService filePlanService; - /** freeze service */ protected FreezeService freezeService; - /** - * @param filePlanService file plan service - */ - public void setFilePlanService(FilePlanService filePlanService) - { - this.filePlanService = filePlanService; - } - /** * @param freezeService freeze service */ @@ -116,21 +95,16 @@ public class FrozenAspect extends BaseBehaviourBean ) public void beforeDeleteNode(final NodeRef nodeRef) { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() + AuthenticationUtil.runAsSystem((RunAsWork) () -> { + if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) { - if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) - { - // never allow to delete a frozen node - throw new AccessDeniedException("Frozen nodes can not be deleted."); - } - - // check children - checkChildren(nodeService.getChildAssocs(nodeRef)); - return null; + // never allow to delete a frozen node + throw new AccessDeniedException("Frozen nodes can not be deleted."); } + + // check children + checkChildren(nodeService.getChildAssocs(nodeRef)); + return null; }); } @@ -150,7 +124,7 @@ public class FrozenAspect extends BaseBehaviourBean NodeRef nodeRef = assoc.getChildRef(); if (freezeService.isFrozen(nodeRef)) { - // never allowed to delete a node with a frozen child + // never allow to delete a node with a frozen child throw new AccessDeniedException("Can not delete node, because it contains a frozen child node."); } @@ -203,32 +177,27 @@ public class FrozenAspect extends BaseBehaviourBean ) public void onRemoveAspect(final NodeRef record, QName aspectTypeQName) { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() + AuthenticationUtil.runAsSystem((RunAsWork) () -> { + if (nodeService.exists(record) && + isRecord(record)) { - if (nodeService.exists(record) && - isRecord(record)) - { - // get the owning record folder - NodeRef recordFolder = nodeService.getPrimaryParent(record).getParentRef(); + // get the owning record folder + NodeRef recordFolder = nodeService.getPrimaryParent(record).getParentRef(); - // check that the aspect has been added - if (nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)) + // check that the aspect has been added + if (nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)) + { + // decrement current count + int currentCount = (Integer) nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT); + if (currentCount > 0) { - // decrement current count - int currentCount = (Integer) nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT); - if (currentCount > 0) - { - currentCount = currentCount - 1; - nodeService.setProperty(recordFolder, PROP_HELD_CHILDREN_COUNT, currentCount); - } + currentCount = currentCount - 1; + nodeService.setProperty(recordFolder, PROP_HELD_CHILDREN_COUNT, currentCount); } } - return null; } - }); + return null; + }); } @@ -245,18 +214,13 @@ public class FrozenAspect extends BaseBehaviourBean ) public void onMoveNode(final ChildAssociationRef oldChildAssocRef, final ChildAssociationRef newChildAssocRef) { - AuthenticationUtil.runAs(new RunAsWork() - { - @Override - public Void doWork() + AuthenticationUtil.runAs((RunAsWork) () -> { + if (nodeService.exists(newChildAssocRef.getParentRef()) && + nodeService.exists(newChildAssocRef.getChildRef())) { - if (nodeService.exists(newChildAssocRef.getParentRef()) && - nodeService.exists(newChildAssocRef.getChildRef())) - { - throw new AccessDeniedException("Frozen nodes can not be moved."); - } - return null; + throw new AccessDeniedException("Frozen nodes can not be moved."); } + return null; }, AuthenticationUtil.getSystemUserName()); } @@ -274,26 +238,14 @@ public class FrozenAspect extends BaseBehaviourBean public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() - { - if (nodeService.exists(nodeRef)) + 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") ) { - if (freezeService.isFrozen(nodeRef)) - { - // Determine the properties that have changed - Map changedProperties = PropertyMap.getChangedProperties(before, after); - // never allow to update a frozen node - if (changedProperties != null && !changedProperties.isEmpty()) - { - throw new AccessDeniedException("Frozen nodes can not be updated."); - } - } + throw new AccessDeniedException("Frozen nodes can not be updated."); } - return null; - } + return null; }); } @@ -310,18 +262,13 @@ public class FrozenAspect extends BaseBehaviourBean ) public void onContentUpdate(NodeRef nodeRef, boolean newContent) { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() - { - 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; - } + 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; }); } @@ -332,19 +279,13 @@ public class FrozenAspect extends BaseBehaviourBean ) public void beforeCopy(QName classRef, NodeRef sourceNodeRef, NodeRef targetNodeRef) { - AuthenticationUtil.runAs(new RunAsWork() - { - @Override - public Void doWork() + AuthenticationUtil.runAs((RunAsWork) () -> { + if (nodeService.exists(sourceNodeRef) && freezeService.isFrozen(sourceNodeRef)) { - //nodeService.getPrimaryParent(sourceNodeRef).getParentRef() - if (nodeService.exists(sourceNodeRef) && freezeService.isFrozen(sourceNodeRef)) - { - throw new AccessDeniedException("Frozen nodes can not be copied."); - } - - return null; + throw new AccessDeniedException("Frozen nodes can not be copied."); } + + return null; }, AuthenticationUtil.getSystemUserName()); } } From ef79db9665ef5ac6c8815b2a19f7745ce2d34bdf Mon Sep 17 00:00:00 2001 From: rlucanu Date: Wed, 14 Aug 2019 17:54:37 +0300 Subject: [PATCH 04/10] RM-6904 code review changes --- .../model/rma/aspect/FrozenAspect.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 00cdb290d2..7ddceb5808 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 @@ -214,14 +214,14 @@ public class FrozenAspect extends BaseBehaviourBean ) public void onMoveNode(final ChildAssociationRef oldChildAssocRef, final ChildAssociationRef newChildAssocRef) { - AuthenticationUtil.runAs((RunAsWork) () -> { + AuthenticationUtil.runAsSystem((RunAsWork) () -> { if (nodeService.exists(newChildAssocRef.getParentRef()) && nodeService.exists(newChildAssocRef.getChildRef())) { throw new AccessDeniedException("Frozen nodes can not be moved."); } return null; - }, AuthenticationUtil.getSystemUserName()); + }); } /** @@ -279,13 +279,13 @@ public class FrozenAspect extends BaseBehaviourBean ) public void beforeCopy(QName classRef, NodeRef sourceNodeRef, NodeRef targetNodeRef) { - AuthenticationUtil.runAs((RunAsWork) () -> { + AuthenticationUtil.runAsSystem((RunAsWork) () -> { if (nodeService.exists(sourceNodeRef) && freezeService.isFrozen(sourceNodeRef)) { throw new AccessDeniedException("Frozen nodes can not be copied."); } return null; - }, AuthenticationUtil.getSystemUserName()); + }); } } From 1c227eb336c46e7149d1aa7ae870f1aabb4b73c5 Mon Sep 17 00:00:00 2001 From: cagache Date: Fri, 16 Aug 2019 10:56:08 +0300 Subject: [PATCH 05/10] code review comments --- .../module/org_alfresco_module_rm/hold/HoldServiceImpl.java | 2 +- .../org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 0fdb4fb3c9..d64918e545 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 @@ -637,7 +637,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); 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 7ddceb5808..99678eace2 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 @@ -241,7 +241,7 @@ public class FrozenAspect extends BaseBehaviourBean 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."); } From 16b2e0203db5104a43a006a88f713e29c4755933 Mon Sep 17 00:00:00 2001 From: cagache Date: Fri, 16 Aug 2019 16:10:09 +0300 Subject: [PATCH 06/10] RM-6906 Add integration tests for prevent update of held content. Added fix to not trigger onUpdateProperties policy when removing the frozen aspect. --- .../hold/HoldServiceImpl.java | 25 +-- .../model/rma/aspect/FrozenAspect.java | 56 +---- .../hold/UpdateHeldActiveContentTest.java | 211 ++++++++++++++++++ 3 files changed, 229 insertions(+), 63 deletions(-) create mode 100644 rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/UpdateHeldActiveContentTest.java 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 d64918e545..e2bdd33280 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 @@ -228,6 +228,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(nodeRef).add("frozen"); nodeService.removeAspect(nodeRef, ASPECT_FROZEN); } @@ -242,6 +244,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(record).add("frozen"); nodeService.removeAspect(record, ASPECT_FROZEN); } } @@ -716,12 +720,12 @@ 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() + authenticationUtil.runAsSystem((RunAsWork) () -> { { + // remove from hold + //set in transaction cache in order not to trigger update policy when removing the child association + transactionalResourceHelper.getSet(nodeRef).add("frozen"); nodeService.removeChild(hold, nodeRef); // audit that the node has been remove from the hold @@ -730,19 +734,14 @@ public class HoldServiceImpl extends ServiceBaseImpl 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 99678eace2..c6a5a35853 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; @@ -67,9 +65,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; @@ -209,14 +205,14 @@ 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."); } @@ -233,7 +229,7 @@ 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) @@ -248,44 +244,4 @@ public class FrozenAspect extends BaseBehaviourBean 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..b981a84c5d --- /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,211 @@ +/* + * #%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.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 AccessDeniedException to be thrown"); + } + catch (AccessDeniedException ade) + { + assertTrue(ade.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 AccessDeniedException to be thrown"); + } + catch (AccessDeniedException ade) + { + assertTrue(ade.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 AccessDeniedException to be thrown"); + } + catch (AccessDeniedException ade) + { + assertTrue(ade.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 AccessDeniedException to be thrown"); + } + catch (AccessDeniedException ade) + { + assertTrue(ade.getMessage().contains("Frozen nodes can not be updated.")); + } + } + }); + } +} From 6e4991a658e662df9f34c5f2cf87d45d472c7870 Mon Sep 17 00:00:00 2001 From: cagache Date: Fri, 16 Aug 2019 17:28:53 +0300 Subject: [PATCH 07/10] small fix --- .../hold/HoldServiceImpl.java | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) 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 e2bdd33280..70a08a409c 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 @@ -229,7 +229,7 @@ public class HoldServiceImpl extends ServiceBaseImpl { // remove the freeze aspect from the node //set in transaction cache in order not to trigger update policy when removing the aspect - transactionalResourceHelper.getSet(nodeRef).add("frozen"); + transactionalResourceHelper.getSet("frozen").add(nodeRef); nodeService.removeAspect(nodeRef, ASPECT_FROZEN); } @@ -245,7 +245,7 @@ public class HoldServiceImpl extends ServiceBaseImpl { // remove the freeze aspect from the node //set in transaction cache in order not to trigger update policy when removing the aspect - transactionalResourceHelper.getSet(record).add("frozen"); + transactionalResourceHelper.getSet("frozen").add(record); nodeService.removeAspect(record, ASPECT_FROZEN); } } @@ -721,19 +721,17 @@ 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((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); - // remove from hold - //set in transaction cache in order not to trigger update policy when removing the child association - transactionalResourceHelper.getSet(nodeRef).add("frozen"); - 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; - } }); } } From cf9c40b11bc22eaca9288f8bb273d642907c578b Mon Sep 17 00:00:00 2001 From: cagache Date: Fri, 16 Aug 2019 19:08:54 +0300 Subject: [PATCH 08/10] Added fix to not trigger onUpdateProperties policy when deleting a hold. --- .../module/org_alfresco_module_rm/hold/HoldServiceImpl.java | 2 ++ 1 file changed, 2 insertions(+) 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 70a08a409c..c4da746aa2 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 @@ -202,6 +202,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); } From a080939d04a47ce66c0e0c51a9b1d005235844d1 Mon Sep 17 00:00:00 2001 From: cagache Date: Mon, 19 Aug 2019 15:02:40 +0300 Subject: [PATCH 09/10] Throw PermissionDeniedException instead of AccessDeniedException and internationalized the messages. --- .../messages/hold-service.properties | 6 ++- .../model/rma/aspect/FrozenAspect.java | 12 ++--- .../hold/UpdateHeldActiveContentTest.java | 53 ++++--------------- 3 files changed, 22 insertions(+), 49 deletions(-) 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 ae21a7ddb8..6e3cc58d4f 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 @@ -1,4 +1,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. \ No newline at end of file +rm.hold.add-to-hold-archived-node=Archived 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/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 c6a5a35853..18cbd5f544 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 @@ -45,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 @@ -95,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 @@ -121,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 @@ -214,7 +215,7 @@ public class FrozenAspect extends BaseBehaviourBean 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; }); @@ -232,14 +233,13 @@ public class FrozenAspect extends BaseBehaviourBean 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("frozen").contains(nodeRef) ) { - throw new AccessDeniedException("Frozen nodes can not be updated."); + throw new PermissionDeniedException(I18NUtil.getMessage("rm.hold.update-frozen-node")); } 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 index b981a84c5d..a5e8ac9cd0 100644 --- 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 @@ -30,6 +30,7 @@ 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; @@ -56,7 +57,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase */ public void testDeleteHeldDocument() { - doBehaviourDrivenTest(new BehaviourDrivenTest() + doBehaviourDrivenTest(new BehaviourDrivenTest(PermissionDeniedException.class) { public void given() { @@ -69,15 +70,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase public void when() { - try - { - fileFolderService.delete(dmDocument); - fail("Expected AccessDeniedException to be thrown"); - } - catch (AccessDeniedException ade) - { - assertTrue(ade.getMessage().contains("Frozen nodes can not be deleted.")); - } + fileFolderService.delete(dmDocument); } }); } @@ -114,7 +107,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase */ public void testMoveHeldDocument() { - doBehaviourDrivenTest(new BehaviourDrivenTest() + doBehaviourDrivenTest(new BehaviourDrivenTest(PermissionDeniedException.class) { public void given() { @@ -127,15 +120,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase public void when() throws FileNotFoundException { - try - { - fileFolderService.move(dmDocument, dmFolder1, null); - fail("Expected AccessDeniedException to be thrown"); - } - catch (AccessDeniedException ade) - { - assertTrue(ade.getMessage().contains("Frozen nodes can not be moved.")); - } + fileFolderService.move(dmDocument, dmFolder1, null); } }); } @@ -148,7 +133,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase */ public void testUpdateHeldDocumentProperties() { - doBehaviourDrivenTest(new BehaviourDrivenTest() + doBehaviourDrivenTest(new BehaviourDrivenTest(PermissionDeniedException.class) { public void given() { @@ -161,15 +146,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase public void when() { - try - { - nodeService.setProperty(dmDocument, ContentModel.PROP_DESCRIPTION, "description"); - fail("Expected AccessDeniedException to be thrown"); - } - catch (AccessDeniedException ade) - { - assertTrue(ade.getMessage().contains("Frozen nodes can not be updated.")); - } + nodeService.setProperty(dmDocument, ContentModel.PROP_DESCRIPTION, "description"); } }); } @@ -181,7 +158,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase */ public void testUpdateHeldDocumentContent() { - doBehaviourDrivenTest(new BehaviourDrivenTest() + doBehaviourDrivenTest(new BehaviourDrivenTest(PermissionDeniedException.class) { public void given() { @@ -194,17 +171,9 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase 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 AccessDeniedException to be thrown"); - } - catch (AccessDeniedException ade) - { - assertTrue(ade.getMessage().contains("Frozen nodes can not be updated.")); - } + ContentData content = (ContentData) nodeService.getProperty(dmDocument, PROP_CONTENT); + nodeService.setProperty(dmDocument, PROP_CONTENT, ContentData.setMimetype(content, + MimetypeMap.MIMETYPE_TEXT_PLAIN)); } }); } From ceb4768e5a2f67941ceabc1a19bf9e6ac1d05970 Mon Sep 17 00:00:00 2001 From: cagache Date: Mon, 19 Aug 2019 17:12:58 +0300 Subject: [PATCH 10/10] Check exception messages --- .../hold/UpdateHeldActiveContentTest.java | 52 +++++++++++++++---- 1 file changed, 42 insertions(+), 10 deletions(-) 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 index a5e8ac9cd0..a50c65a3b8 100644 --- 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 @@ -57,7 +57,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase */ public void testDeleteHeldDocument() { - doBehaviourDrivenTest(new BehaviourDrivenTest(PermissionDeniedException.class) + doBehaviourDrivenTest(new BehaviourDrivenTest() { public void given() { @@ -70,7 +70,15 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase public void when() { - fileFolderService.delete(dmDocument); + try + { + fileFolderService.delete(dmDocument); + fail("Expected PermissionDeniedException to be thrown"); + } + catch (PermissionDeniedException pde) + { + assertTrue(pde.getMessage().contains("Frozen nodes can not be deleted.")); + } } }); } @@ -107,7 +115,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase */ public void testMoveHeldDocument() { - doBehaviourDrivenTest(new BehaviourDrivenTest(PermissionDeniedException.class) + doBehaviourDrivenTest(new BehaviourDrivenTest() { public void given() { @@ -120,7 +128,15 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase public void when() throws FileNotFoundException { - fileFolderService.move(dmDocument, dmFolder1, null); + 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.")); + } } }); } @@ -133,7 +149,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase */ public void testUpdateHeldDocumentProperties() { - doBehaviourDrivenTest(new BehaviourDrivenTest(PermissionDeniedException.class) + doBehaviourDrivenTest(new BehaviourDrivenTest() { public void given() { @@ -146,7 +162,15 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase public void when() { - nodeService.setProperty(dmDocument, ContentModel.PROP_DESCRIPTION, "description"); + 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.")); + } } }); } @@ -158,7 +182,7 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase */ public void testUpdateHeldDocumentContent() { - doBehaviourDrivenTest(new BehaviourDrivenTest(PermissionDeniedException.class) + doBehaviourDrivenTest(new BehaviourDrivenTest() { public void given() { @@ -171,9 +195,17 @@ public class UpdateHeldActiveContentTest extends BaseRMTestCase public void when() { - ContentData content = (ContentData) nodeService.getProperty(dmDocument, PROP_CONTENT); - nodeService.setProperty(dmDocument, PROP_CONTENT, ContentData.setMimetype(content, - MimetypeMap.MIMETYPE_TEXT_PLAIN)); + 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.")); + } } }); }