diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-action-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-action-context.xml index 88765a490b..649022988c 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-action-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-action-context.xml @@ -1025,7 +1025,6 @@ - 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 932d24c525..9119c2f01c 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 @@ -177,6 +177,11 @@ + + + + http://www.alfresco.org/model/system/1.0 + @@ -185,6 +190,7 @@ + diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/CreateHoldAuditEvent.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/CreateHoldAuditEvent.java index 6e585cecd3..922ad61459 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/CreateHoldAuditEvent.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/CreateHoldAuditEvent.java @@ -30,10 +30,11 @@ package org.alfresco.module.org_alfresco_module_rm.audit.event; import java.io.Serializable; import java.util.Map; +import org.alfresco.repo.node.NodeServicePolicies; +import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.annotation.Behaviour; import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.repo.policy.annotation.BehaviourKind; -import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; @@ -41,14 +42,18 @@ import org.alfresco.service.namespace.QName; /** * Create hold audit event. + * This listens to the NodeServicePolicies.OnCreateNodePolicy in order to cover the create hold action from Share + * since that does not call the createHold from HoldService * * @author Sara Aspery * @since 3.3 */ @BehaviourBean -public class CreateHoldAuditEvent extends AuditEvent +public class CreateHoldAuditEvent extends AuditEvent implements NodeServicePolicies.OnCreateNodePolicy { - /** Node Service */ + /** + * Node Service + */ private NodeService nodeService; /** @@ -62,16 +67,16 @@ public class CreateHoldAuditEvent extends AuditEvent } /** - * @param childAssociationRef child association reference + * @see org.alfresco.repo.node.NodeServicePolicies.OnCreateNodePolicy#onCreateNode(org.alfresco.service.cmr.repository.ChildAssociationRef) */ + @Override @Behaviour ( kind = BehaviourKind.CLASS, type = "rma:hold", - policy = "alf:onCreateNode", - notificationFrequency= NotificationFrequency.TRANSACTION_COMMIT + notificationFrequency = NotificationFrequency.TRANSACTION_COMMIT ) - public void onCreateHold(ChildAssociationRef childAssociationRef) + public void onCreateNode(ChildAssociationRef childAssociationRef) { NodeRef holdNodeRef = childAssociationRef.getChildRef(); diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/DeleteHoldAuditEvent.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/DeleteHoldAuditEvent.java index 2789df192b..a2ecc1fbc2 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/DeleteHoldAuditEvent.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/DeleteHoldAuditEvent.java @@ -27,10 +27,12 @@ package org.alfresco.module.org_alfresco_module_rm.audit.event; +import static org.alfresco.repo.policy.Behaviour.NotificationFrequency.EVERY_EVENT; + import java.io.Serializable; import java.util.Map; -import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies; +import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.policy.annotation.Behaviour; import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.repo.policy.annotation.BehaviourKind; @@ -38,18 +40,19 @@ import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.namespace.QName; -import static org.alfresco.repo.policy.Behaviour.NotificationFrequency.EVERY_EVENT; - /** * Delete hold audit event. + * This listens to the NodeServicePolicies.BeforeDeleteNodePolicy in order to cover the delete hold using nodes service * * @author Sara Aspery * @since 3.3 */ @BehaviourBean -public class DeleteHoldAuditEvent extends AuditEvent implements HoldServicePolicies.BeforeDeleteHoldPolicy +public class DeleteHoldAuditEvent extends AuditEvent implements NodeServicePolicies.BeforeDeleteNodePolicy { - /** Node Service */ + /** + * Node Service + */ private NodeService nodeService; /** @@ -63,16 +66,15 @@ public class DeleteHoldAuditEvent extends AuditEvent implements HoldServicePolic } /** - * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeDeleteHoldPolicy#beforeDeleteHold(org.alfresco.service.cmr.repository.NodeRef) + * @see org.alfresco.repo.node.NodeServicePolicies.BeforeDeleteNodePolicy#beforeDeleteNode(org.alfresco.service.cmr.repository.NodeRef) */ @Override - @Behaviour - ( - kind = BehaviourKind.CLASS, - type = "rma:hold", - notificationFrequency = EVERY_EVENT - ) - public void beforeDeleteHold(NodeRef holdNodeRef) + @Behaviour ( + kind = BehaviourKind.CLASS, + type = "rma:hold", + notificationFrequency = EVERY_EVENT + ) + public void beforeDeleteNode(NodeRef holdNodeRef) { Map auditProperties = HoldUtils.makePropertiesMap(holdNodeRef, nodeService); recordsManagementAuditService.auditEvent(holdNodeRef, getName(), auditProperties, null, true, false); 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 0b89575c22..32400091e0 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 @@ -651,7 +651,8 @@ public class HoldServiceImpl extends ServiceBaseImpl // fire before add to hold policy invokeBeforeAddToHold(hold, nodeRef); // run as system to ensure we have all the appropriate permissions to perform the manipulations we require - authenticationUtil.runAsSystem((RunAsWork) () -> { + authenticationUtil.runAsSystem((RunAsWork) () -> + { // gather freeze properties final Map props = new HashMap<>(2); props.put(PROP_FROZEN_AT, new Date()); @@ -671,11 +672,11 @@ public class HoldServiceImpl extends ServiceBaseImpl records.forEach(record -> addFrozenAspect(record, props)); } - // fire on add to hold policy - invokeOnAddToHold(hold, nodeRef); - return null; }); + + // fire on add to hold policy + invokeOnAddToHold(hold, nodeRef); } } } @@ -791,6 +792,7 @@ public class HoldServiceImpl extends ServiceBaseImpl if (!holds.isEmpty()) { + List removedHolds = new ArrayList<>(); for (final NodeRef hold : holds) { if (!isHold(hold)) @@ -807,34 +809,38 @@ public class HoldServiceImpl extends ServiceBaseImpl if (getHeld(hold).contains(nodeRef)) { + // fire before remove from hold policy + invokeBeforeRemoveFromHold(hold, nodeRef); // 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) () -> { - // fire before remove from hold policy - invokeBeforeRemoveFromHold(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 + // audit that the node has been removed from the hold // TODO add details of the hold that the node was removed from recordsManagementAuditService.auditEvent(nodeRef, AUDIT_REMOVE_FROM_HOLD); - // fire on remove from hold policy - invokeOnRemoveFromHold(hold, nodeRef); - return null; - }); + removedHolds.add(hold); } } // run as system as we can't be sure if have remove aspect rights on node - authenticationUtil.runAsSystem((RunAsWork) () -> { + authenticationUtil.runAsSystem((RunAsWork) () -> + { removeFreezeAspect(nodeRef, 0); return null; }); + for (NodeRef removedHold : removedHolds) + { + // fire on remove from hold policy + invokeOnRemoveFromHold(removedHold, nodeRef); + } } } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java index 8e98601b61..88544fbbd5 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java @@ -26,10 +26,13 @@ */ package org.alfresco.module.org_alfresco_module_rm.util; +import static java.util.Collections.unmodifiableList; + import java.io.Serializable; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.alfresco.service.namespace.QName; @@ -46,61 +49,98 @@ public class PropertyModificationAllowedCheck */ private List whiteList; + /** + * List of model URI's for which the properties can be updated + */ + private List editableURIs; + + /** + * Getter for list of model URI's + * + * @return return the list of model URI's + */ + private List getEditableURIs() + { + return unmodifiableList(editableURIs); + } + + /** + * Setter for list of model URI's + * + * @param editableURIs List + */ + public void setEditableURIs(List editableURIs) + { + this.editableURIs = unmodifiableList(editableURIs); + } + /** * Setter for list of qnames + * * @param whiteList List */ public void setWhiteList(List whiteList) { - this.whiteList = whiteList; + this.whiteList = unmodifiableList(whiteList); + } /** * Compares the node properties with the requested update to make sure all potential updates are permitted + * * @param before current node properties - * @param after updated properties for the node - * @return true - if all modified property keys are in the whitelist + * @param after updated properties for the node + * @return true - if all modified property keys are in the whitelist or + * in the list of model URI's for which the properties can be modified */ public boolean check(Map before, Map after) { boolean proceed = true; - HashSet unionKeys = new HashSet<>(before.keySet()); - unionKeys.addAll(after.keySet()); - for (QName key : unionKeys) + // Initially check for changes to existing keys and values. + for (final Map.Entry entry : before.entrySet()) { - //Check if property has been added or removed - if (!before.containsKey(key) || !after.containsKey(key)) + final QName key = entry.getKey(); + final Serializable beforeValue = entry.getValue(); + //check if property has been updated + final boolean modified = after.containsKey(key) && after.get(key) != null + && !after.get(key).equals(beforeValue); + + //check if the property has been emptied or removed + final boolean propertyRemovedEmptied = (after.get(key) == null && beforeValue != null) + || !after.containsKey(key); + if (modified || propertyRemovedEmptied) { - //Property modified check to see if allowed - proceed = whiteList.contains(key); - if (!proceed) - { - break; - } + proceed = allowPropertyUpdate(key); } - //Check if property emptied or empty property filled - if ((before.get(key) == null && after.get(key) != null) || - (after.get(key) == null && before.get(key) != null)) + if (!proceed) { - //Property modified check to see if allowed - proceed = whiteList.contains(key); - if (!proceed) - { - break; - } + return proceed; } - //If properties aren't missing or empty check equality - if (before.get(key) != null && after.get(key) != null && !(after.get(key).equals(before.get(key)))) + } + + // Check for new values. Record individual values and group as a single map. + final Set newKeys = new HashSet<>(after.keySet()); + newKeys.removeAll(before.keySet()); + for (final QName key : newKeys) + { + proceed = allowPropertyUpdate(key); + if (!proceed) { - //Property modified check to see if allowed - proceed = whiteList.contains(key); - if (!proceed) - { - break; - } + break; } } return proceed; } + /** + * Determines whether the property should be allowed to be updated or not. + * + * @param key property + * @return true if property update is allowed + */ + private boolean allowPropertyUpdate(QName key) + { + return whiteList.contains(key) || getEditableURIs().contains(key.getNamespaceURI()); + } + } diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/CreateHoldAuditEventUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/CreateHoldAuditEventUnitTest.java index a82a77c619..9c4038d6df 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/CreateHoldAuditEventUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/CreateHoldAuditEventUnitTest.java @@ -87,7 +87,7 @@ public class CreateHoldAuditEventUnitTest extends BaseUnitTest @Test public void testCreateHoldCausesAuditEvent() { - createHoldAuditEvent.onCreateHold(childAssociationRef); + createHoldAuditEvent.onCreateNode(childAssociationRef); verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(holdNodeRef), any(String.class), isNull(Map.class), any(Map.class)); } } diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/DeleteHoldAuditEventUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/DeleteHoldAuditEventUnitTest.java index 5f787af6f5..73e0ccb215 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/DeleteHoldAuditEventUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/DeleteHoldAuditEventUnitTest.java @@ -82,7 +82,7 @@ public class DeleteHoldAuditEventUnitTest extends BaseUnitTest @Test public void testDeleteHoldCausesAuditEvent() { - deleteHoldAuditEvent.beforeDeleteHold(holdNodeRef); + deleteHoldAuditEvent.beforeDeleteNode(holdNodeRef); verify(mockedRecordsManagementAuditService, times(1)) .auditEvent(eq(holdNodeRef), any(String.class), any(Map.class), isNull(Map.class), Matchers.eq(true), Matchers.eq(false)); } diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java index 1b192a62c0..a152a7c533 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java @@ -58,6 +58,7 @@ public class PropertyModificationAllowedCheckUnitTest private QName qName, qName2; private List list; + private List editableURIs; @Mock private Serializable serializable, serializable2; @@ -67,13 +68,16 @@ public class PropertyModificationAllowedCheckUnitTest { MockitoAnnotations.initMocks(this); propertyModificationAllowedCheck = new PropertyModificationAllowedCheck(); - before = new HashMap(); + before = new HashMap(); after = new HashMap(); - qName = QName.createQName("foo","bar"); + qName = QName.createQName("foo", "bar"); qName2 = QName.createQName("bar", "foo"); before.put(qName, serializable); after.put(qName, serializable2); list = new ArrayList(); + editableURIs = new ArrayList<>(); + propertyModificationAllowedCheck.setWhiteList(list); + propertyModificationAllowedCheck.setEditableURIs(editableURIs); } /** @@ -232,4 +236,26 @@ public class PropertyModificationAllowedCheckUnitTest after.put(qName, null); assertTrue(propertyModificationAllowedCheck.check(before, after)); } + + /** + * Test update of a property from the model URI for which properties can be updated + */ + @Test + public void testUpdatePropertyFromAllowedModelURI() + { + editableURIs.add("foo"); + propertyModificationAllowedCheck.setEditableURIs(editableURIs); + assertTrue(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test update of a property that is not in the model URI for which properties can be updated + */ + @Test + public void testUpdatePropertyFromNotAllowedModelURI() + { + editableURIs.add("bar"); + propertyModificationAllowedCheck.setEditableURIs(editableURIs); + assertFalse(propertyModificationAllowedCheck.check(before, after)); + } }