RM-6928 adding check to allow specified properties on frozen nodes to be updated

rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml

@@ -1,8 +1,11 @@
 <?xml version='1.0' encoding='UTF-8'?>
 <beans xmlns="http://www.springframework.org/schema/beans"
        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns:util="http://www.springframework.org/schema/util"
        xsi:schemaLocation="http://www.springframework.org/schema/beans
-        http://www.springframework.org/schema/beans/spring-beans.xsd">
+        http://www.springframework.org/schema/beans/spring-beans.xsd
+      http://www.springframework.org/schema/util
+      http://www.springframework.org/schema/util/spring-util.xsd">
 
 
    <!-- Ensure backwards compatibility with 4.2 and 4.2.1 -->
@@ -170,6 +173,18 @@
 
    <bean id="rma.freeze" class="org.alfresco.module.org_alfresco_module_rm.model.rma.aspect.FrozenAspect" parent="rm.baseBehaviour">
       <property name="freezeService" ref="FreezeService" />
+      <property name="propertyModificationAllowedCheck" ref="updateFrozenPropertyCheck"/>
+   </bean>
+
+   <util:constant id="propThumbnailModification" static-field="org.alfresco.model.ContentModel.PROP_LAST_THUMBNAIL_MODIFICATION_DATA" />
+
+   <bean name="updateFrozenPropertyCheck"
+         class="org.alfresco.module.org_alfresco_module_rm.util.PropertyModificationAllowedCheck">
+      <property name="whiteList">
+         <list>
+            <ref bean="propThumbnailModification" />
+         </list>
+      </property>
    </bean>
 
    <bean id="rma.vitalRecordDefinition" class="org.alfresco.module.org_alfresco_module_rm.model.rma.aspect.VitalRecordDefinitionAspect" parent="rm.baseBehaviour">

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<Void>) () -> {
             // 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"));
                 }

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 <http://www.gnu.org/licenses/>.
+ * #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<QName> whiteList;
+
+    /**
+     * Setter for list of qnames
+     * @param whiteList List<QName>
+     */
+    public void setWhiteList(List<QName> 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<QName, Serializable> before, Map<QName, Serializable> after)
+    {
+        boolean proceed = true;
+        HashSet<QName> 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;
+    }
+
+}

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);
     }
 }

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 <http://www.gnu.org/licenses/>.
+ * #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<QName, Serializable> before, after;
+
+    private QName qName, qName2;
+
+    private List<QName> 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));
+    }
+}