From 87ff9e6fc8db0acfaac0e5fe1b3e32b9e99c0670 Mon Sep 17 00:00:00 2001 From: Ross Gale Date: Tue, 24 Sep 2019 16:45:51 +0100 Subject: [PATCH] RM-6928 adding check to allow specified properties on frozen nodes to be updated --- .../rm-model-context.xml | 17 +- .../model/rma/aspect/FrozenAspect.java | 17 +- .../PropertyModificationAllowedCheck.java | 106 ++++++++ .../rma/aspect/FrozenAspectUnitTest.java | 5 + ...pertyModificationAllowedCheckUnitTest.java | 235 ++++++++++++++++++ 5 files changed, 378 insertions(+), 2 deletions(-) create mode 100644 rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java create mode 100644 rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java 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 6c0783de97..932d24c525 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 @@ -1,8 +1,11 @@ + http://www.springframework.org/schema/beans/spring-beans.xsd + http://www.springframework.org/schema/util + http://www.springframework.org/schema/util/spring-util.xsd"> @@ -170,6 +173,18 @@ + + + + + + + + + + + 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 b40ccea490..d5d9067a2a 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,6 +38,7 @@ 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.module.org_alfresco_module_rm.util.PropertyModificationAllowedCheck; import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.annotation.Behaviour; @@ -76,6 +77,11 @@ public class FrozenAspect extends BaseBehaviourBean /** freeze service */ protected FreezeService freezeService; + /** + * Utility class for property modification + */ + private PropertyModificationAllowedCheck propertyModificationAllowedCheck; + /** * @param freezeService freeze service */ @@ -84,6 +90,14 @@ public class FrozenAspect extends BaseBehaviourBean this.freezeService = freezeService; } + /** + * Setter for property modification check utility + * @param propertyModificationAllowedCheck Utility class for property modification + */ + public void setPropertyModificationAllowedCheck(PropertyModificationAllowedCheck propertyModificationAllowedCheck) + { + this.propertyModificationAllowedCheck = propertyModificationAllowedCheck; + } /** * Disable the on update properties for frozen aspect behaviour @@ -268,7 +282,8 @@ 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("frozen").contains(nodeRef) ) + !transactionalResourceHelper.getSet("frozen").contains(nodeRef) && + !propertyModificationAllowedCheck.check(before, after)) { throw new PermissionDeniedException(I18NUtil.getMessage("rm.hold.update-frozen-node")); } 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 new file mode 100644 index 0000000000..8e98601b61 --- /dev/null +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java @@ -0,0 +1,106 @@ +/* + * #%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.util; + +import java.io.Serializable; +import java.util.HashSet; +import java.util.List; +import java.util.Map; + +import org.alfresco.service.namespace.QName; + +/** + * Utility check for modification of a properties based off presence in a whitelist. + * + * @author Ross Gale + * @since 3.2 + */ +public class PropertyModificationAllowedCheck +{ + /** + * List of qnames that can be modified + */ + private List whiteList; + + /** + * Setter for list of qnames + * @param whiteList List + */ + public void setWhiteList(List whiteList) + { + this.whiteList = 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 + */ + public boolean check(Map before, Map after) + { + boolean proceed = true; + HashSet unionKeys = new HashSet<>(before.keySet()); + unionKeys.addAll(after.keySet()); + for (QName key : unionKeys) + { + //Check if property has been added or removed + if (!before.containsKey(key) || !after.containsKey(key)) + { + //Property modified check to see if allowed + proceed = whiteList.contains(key); + if (!proceed) + { + break; + } + } + //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)) + { + //Property modified check to see if allowed + proceed = whiteList.contains(key); + if (!proceed) + { + break; + } + } + //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)))) + { + //Property modified check to see if allowed + proceed = whiteList.contains(key); + if (!proceed) + { + break; + } + } + } + return proceed; + } + +} 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 78a0b7b38d..79f0f68025 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 @@ -42,6 +42,7 @@ import java.util.Set; import org.alfresco.model.ContentModel; 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.PropertyModificationAllowedCheck; import org.alfresco.module.org_alfresco_module_rm.util.TransactionalResourceHelper; import org.alfresco.rest.framework.core.exceptions.PermissionDeniedException; import org.alfresco.service.cmr.repository.ChildAssociationRef; @@ -92,6 +93,9 @@ public class FrozenAspectUnitTest @Mock private Set mockSet; + @Mock + private PropertyModificationAllowedCheck mockPropertyModificationAllowedCheck; + @InjectMocks private FrozenAspect frozenAspect; @@ -245,6 +249,7 @@ public class FrozenAspectUnitTest when(mockFreezeService.isFrozen(content)).thenReturn(true); when(mockResourceHelper.getSet(content)).thenReturn(mockSet); when(mockSet.contains("frozen")).thenReturn(false); + when(mockPropertyModificationAllowedCheck.check(null, null)).thenReturn(false); frozenAspect.onUpdateProperties(content, null, null); } } 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 new file mode 100644 index 0000000000..1b192a62c0 --- /dev/null +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java @@ -0,0 +1,235 @@ +/* + * #%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.util; + + +import static junit.framework.TestCase.assertFalse; +import static junit.framework.TestCase.assertTrue; + +import java.io.Serializable; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.alfresco.service.namespace.QName; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +/** + * Unit test class for PropertyModificationAllowedCheck + * + * @author Ross Gale + * @since 3.2 + */ +public class PropertyModificationAllowedCheckUnitTest +{ + + private PropertyModificationAllowedCheck propertyModificationAllowedCheck; + + private Map before, after; + + private QName qName, qName2; + + private List list; + + @Mock + private Serializable serializable, serializable2; + + @Before + public void setUp() + { + MockitoAnnotations.initMocks(this); + propertyModificationAllowedCheck = new PropertyModificationAllowedCheck(); + before = new HashMap(); + after = new HashMap(); + qName = QName.createQName("foo","bar"); + qName2 = QName.createQName("bar", "foo"); + before.put(qName, serializable); + after.put(qName, serializable2); + list = new ArrayList(); + } + + /** + * Test modification check passes when property is in whitelist + */ + @Test + public void testCheckMethodReturnsTrueWhenPropertyInList() + { + list.add(qName); + propertyModificationAllowedCheck.setWhiteList(list); + assertTrue(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check fails when property is not in whitelist + */ + @Test + public void testCheckMethodReturnsFalseIfAnyNonAllowedPropertyInListIsChanged() + { + list.add(qName); + before.put(qName2, serializable2); + after.put(qName2, serializable); + propertyModificationAllowedCheck.setWhiteList(list); + assertFalse(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check fails when first property is not in whitelist + */ + @Test + public void testCheckMethodReturnsFalseIfFirstPropertyInListIsChangedWithoutWhitelist() + { + list.add(qName2); + before.put(qName2, serializable2); + after.put(qName2, serializable); + propertyModificationAllowedCheck.setWhiteList(list); + assertFalse(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check passes when all properties are in whitelist + */ + @Test + public void testCheckMethodReturnsTrueIfAllEditedPropertiesInWhitelist() + { + list.add(qName); + list.add(qName2); + before.put(qName2, serializable2); + after.put(qName2, serializable); + propertyModificationAllowedCheck.setWhiteList(list); + assertTrue(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check fails when property added + */ + @Test + public void testCheckMethodReturnsFalseIfPropertyNotInBeforeList() + { + list.add(qName); + after.put(qName2, serializable); + propertyModificationAllowedCheck.setWhiteList(list); + assertFalse(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check passes when allowed property added + */ + @Test + public void testCheckMethodReturnsTrueIfAllowedPropertyNotInBeforeList() + { + list.add(qName2); + after.put(qName2, serializable); + propertyModificationAllowedCheck.setWhiteList(list); + assertFalse(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check fails when property removed + */ + @Test + public void testCheckMethodReturnsFalseIfPropertyNotInAfterList() + { + list.add(qName); + before.put(qName2, serializable); + propertyModificationAllowedCheck.setWhiteList(list); + assertFalse(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check passes when allowed property removed + */ + @Test + public void testCheckMethodReturnsTrueIfAllowedPropertyNotInAfterList() + { + list.add(qName); + list.add(qName2); + before.put(qName2, serializable); + propertyModificationAllowedCheck.setWhiteList(list); + assertTrue(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check for empty property in before map without whitelist + */ + @Test + public void testNullValueInBeforeList() + { + before.put(qName, null); + propertyModificationAllowedCheck.setWhiteList(list); + assertFalse(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check for empty property in after map without whitelist + */ + @Test + public void testNullValueInAfterList() + { + after.put(qName, null); + propertyModificationAllowedCheck.setWhiteList(list); + assertFalse(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check for empty property in before map with whitelist + */ + @Test + public void testNullValueInBeforeListWithAllowedProperty() + { + list.add(qName); + before.put(qName, null); + propertyModificationAllowedCheck.setWhiteList(list); + assertTrue(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check for empty property in after list with whitelist + */ + @Test + public void testNullValueInAfterListWithAllowedProperty() + { + list.add(qName); + after.put(qName, null); + propertyModificationAllowedCheck.setWhiteList(list); + assertTrue(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test modification check for empty property in both maps + */ + @Test + public void testNullValueInBoth() + { + before.put(qName, null); + after.put(qName, null); + assertTrue(propertyModificationAllowedCheck.check(before, after)); + } +}