Merge pull request #1497 from Alfresco/feature/ACS-3651_NodeValidation

ACS-3651 Validate node pameters.

packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java

@@ -522,6 +522,49 @@ public class CreateRulesTests extends RestTest
         restClient.assertLastError().containsSummary("Missing action's mandatory parameter: destination-folder");
     }
 
+    /**
+     * Check we get error when attempting to create a rule that copies files to a non-existent folder.
+     */
+    @Test (groups = { TestGroup.REST_API, TestGroup.RULES })
+    public void createRuleThatUsesNonExistentNode()
+    {
+        RestRuleModel ruleModel = rulesUtils.createRuleModelWithDefaultValues();
+        RestActionBodyExecTemplateModel invalidAction = new RestActionBodyExecTemplateModel();
+        String actionDefinitionId = "copy";
+        invalidAction.setActionDefinitionId(actionDefinitionId);
+        invalidAction.setParams(Map.of("destination-folder", "non-existent-node"));
+        ruleModel.setActions(List.of(invalidAction));
+
+        restClient.authenticateUser(user).withPrivateAPI().usingNode(ruleFolder).usingDefaultRuleSet()
+                  .createSingleRule(ruleModel);
+
+        restClient.assertStatusCodeIs(NOT_FOUND);
+        restClient.assertLastError().containsSummary("The entity with id: non-existent-node was not found");
+    }
+
+    /**
+     * Check we get error when attempting to create a rule that references a folder that the user does not have read permission for.
+     */
+    @Test (groups = { TestGroup.REST_API, TestGroup.RULES })
+    public void createRuleThatUsesNodeWithoutReadPermission()
+    {
+        SiteModel privateSite = dataSite.usingAdmin().createPrivateRandomSite();
+        FolderModel privateFolder = dataContent.usingAdmin().usingSite(privateSite).createFolder();
+
+        RestRuleModel ruleModel = rulesUtils.createRuleModelWithDefaultValues();
+        RestActionBodyExecTemplateModel invalidAction = new RestActionBodyExecTemplateModel();
+        String actionDefinitionId = "copy";
+        invalidAction.setActionDefinitionId(actionDefinitionId);
+        invalidAction.setParams(Map.of("destination-folder", privateFolder.getNodeRef()));
+        ruleModel.setActions(List.of(invalidAction));
+
+        restClient.authenticateUser(user).withPrivateAPI().usingNode(ruleFolder).usingDefaultRuleSet()
+                  .createSingleRule(ruleModel);
+
+        restClient.assertStatusCodeIs(NOT_FOUND);
+        restClient.assertLastError().containsSummary("The entity with id: " + privateFolder.getNodeRef() + " was not found");
+    }
+
     /**
      * Check we can create a rule with multiple conditions
      */

packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/RulesTestsUtils.java

@@ -93,6 +93,7 @@ public class RulesTestsUtils implements InitializingBean
     private FolderModel copyDestinationFolder;
 
     private FolderModel checkOutDestinationFolder;
+
     /**
      * Initialise the util class.
      */

remote-api/src/main/java/org/alfresco/rest/api/impl/rules/ActionParameterConverter.java

@@ -26,12 +26,17 @@
 
 package org.alfresco.rest.api.impl.rules;
 
+import static org.alfresco.rest.framework.core.exceptions.NotFoundException.DEFAULT_MESSAGE_ID;
+import static org.alfresco.service.cmr.security.AccessStatus.ALLOWED;
+
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.alfresco.rest.api.Nodes;
+import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException;
 import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException;
 import org.alfresco.rest.framework.core.exceptions.NotFoundException;
 import org.alfresco.service.Experimental;
@@ -42,8 +47,8 @@ import org.alfresco.service.cmr.dictionary.DataTypeDefinition;
 import org.alfresco.service.cmr.dictionary.DictionaryException;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.repository.NodeRef;
-import org.alfresco.service.cmr.repository.StoreRef;
 import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
+import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.json.JSONArray;
@@ -56,13 +61,17 @@ public class ActionParameterConverter
     private final DictionaryService dictionaryService;
     private final ActionService actionService;
     private final NamespaceService namespaceService;
+    private final PermissionService permissionService;
+    private final Nodes nodes;
 
-    public ActionParameterConverter(DictionaryService dictionaryService, ActionService actionService,
+    public ActionParameterConverter(DictionaryService dictionaryService, ActionService actionService, NamespaceService namespaceService,
-                                    NamespaceService namespaceService)
+                                    PermissionService permissionService, Nodes nodes)
     {
         this.dictionaryService = dictionaryService;
         this.actionService = actionService;
         this.namespaceService = namespaceService;
+        this.permissionService = permissionService;
+        this.nodes = nodes;
     }
 
     public Map<String, Serializable> getConvertedParams(Map<String, Serializable> params, String name)
@@ -74,10 +83,12 @@ public class ActionParameterConverter
             definition = actionService.getActionDefinition(name);
             if (definition == null)
             {
-                throw new NotFoundException(NotFoundException.DEFAULT_MESSAGE_ID, new String[]{name});
+                throw new NotFoundException(DEFAULT_MESSAGE_ID, new String[]{name});
             }
-        } catch (NoSuchBeanDefinitionException e) {
+        }
-            throw new NotFoundException(NotFoundException.DEFAULT_MESSAGE_ID, new String[]{name});
+        catch (NoSuchBeanDefinitionException e)
+        {
+            throw new NotFoundException(DEFAULT_MESSAGE_ID, new String[]{name});
         }
 
         for (Map.Entry<String, Serializable> param : params.entrySet())
@@ -91,7 +102,8 @@ public class ActionParameterConverter
             {
                 final QName typeQName = paramDef.getType();
                 parameters.put(param.getKey(), convertValue(typeQName, param.getValue()));
-            } else
+            }
+            else
             {
                 parameters.put(param.getKey(), param.getValue().toString());
             }
@@ -105,7 +117,8 @@ public class ActionParameterConverter
         {
             return ((QName) param).toPrefixString(namespaceService);
         }
-        else if (param instanceof NodeRef) {
+        else if (param instanceof NodeRef)
+        {
             return ((NodeRef) param).getId();
         }
         else
@@ -121,7 +134,7 @@ public class ActionParameterConverter
         final DataTypeDefinition typeDef = dictionaryService.getDataType(typeQName);
         if (typeDef == null)
         {
-            throw new NotFoundException(NotFoundException.DEFAULT_MESSAGE_ID, new String[]{typeQName.toPrefixString()});
+            throw new NotFoundException(DEFAULT_MESSAGE_ID, new String[]{typeQName.toPrefixString()});
         }
 
         if (propertyValue instanceof JSONArray)
@@ -130,7 +143,8 @@ public class ActionParameterConverter
             try
             {
                 Class.forName(javaClassName);
-            } catch (ClassNotFoundException e)
+            }
+            catch (ClassNotFoundException e)
             {
                 throw new DictionaryException("Java class " + javaClassName + " of property type " + typeDef.getName() + " is invalid", e);
             }
@@ -151,7 +165,12 @@ public class ActionParameterConverter
             }
             else if (typeQName.isMatch(DataTypeDefinition.NODE_REF))
             {
-                value = new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, propertyValue.toString());
+                NodeRef nodeRef = nodes.validateOrLookupNode(propertyValue.toString(), null);
+                if (permissionService.hasReadPermission(nodeRef) != ALLOWED)
+                {
+                    throw new EntityNotFoundException(propertyValue.toString());
+                }
+                value = nodeRef;
             }
             else
             {

remote-api/src/main/java/org/alfresco/rest/framework/core/exceptions/EntityNotFoundException.java

@@ -34,7 +34,7 @@ public class EntityNotFoundException extends NotFoundException
 {
     private static final long serialVersionUID = -1198595000441207734L;
     public static String DEFAULT_MESSAGE_ID = "framework.exception.EntityNotFound";
-    
+
     /**
      * The entity id param will be shown in the default error message.
      * @param entityId String
@@ -44,6 +44,17 @@ public class EntityNotFoundException extends NotFoundException
         super(DEFAULT_MESSAGE_ID, new String[] {entityId});
     }
 
+    /**
+     * The entity id param will be shown in the default error message.
+     *
+     * @param msgId The message template.
+     * @param parameters The message template parameters.
+     */
+    public EntityNotFoundException(String msgId, String[] parameters)
+    {
+        super(msgId, parameters);
+    }
+
     public EntityNotFoundException(String msgId, Throwable cause)
     {
         super(msgId, cause);

remote-api/src/main/resources/alfresco/public-rest-context.xml

@@ -910,6 +910,8 @@
         <constructor-arg name="actionService" ref="ActionService"/>
         <constructor-arg name="dictionaryService" ref="DictionaryService"/>
         <constructor-arg name="namespaceService" ref="NamespaceService"/>
+        <constructor-arg name="permissionService" ref="PermissionService" />
+        <constructor-arg name="nodes" ref="Nodes"/>
     </bean>
 
     <bean id="actionPermissionValidator" class="org.alfresco.rest.api.impl.rules.ActionPermissionValidator">

remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionParameterConverterTest.java

@@ -26,8 +26,12 @@
 
 package org.alfresco.rest.api.impl.rules;
 
+import static org.alfresco.repo.action.executer.CopyActionExecuter.PARAM_DESTINATION_FOLDER;
 import static org.alfresco.service.cmr.repository.StoreRef.STORE_REF_WORKSPACE_SPACESSTORE;
+import static org.alfresco.service.cmr.security.AccessStatus.ALLOWED;
+import static org.alfresco.service.cmr.security.AccessStatus.DENIED;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
@@ -50,6 +54,8 @@ import org.alfresco.repo.action.executer.RemoveFeaturesActionExecuter;
 import org.alfresco.repo.action.executer.ScriptActionExecuter;
 import org.alfresco.repo.action.executer.SetPropertyValueActionExecuter;
 import org.alfresco.repo.action.executer.SimpleWorkflowActionExecuter;
+import org.alfresco.rest.api.Nodes;
+import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException;
 import org.alfresco.rest.framework.core.exceptions.NotFoundException;
 import org.alfresco.service.Experimental;
 import org.alfresco.service.cmr.action.ActionDefinition;
@@ -58,8 +64,10 @@ import org.alfresco.service.cmr.action.ParameterDefinition;
 import org.alfresco.service.cmr.dictionary.DataTypeDefinition;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
@@ -81,8 +89,10 @@ public class ActionParameterConverterTest
     private static final String IDENTIFIER = "identifier";
     private static final String IDENTIFIER_ASPECT = NamespaceService.CONTENT_MODEL_PREFIX + QName.NAMESPACE_PREFIX + IDENTIFIER;
 
-    private static final String DUMMY_FOLDER_NODE_ID = "dummy-folder-node";
+    private static final String DUMMY_FOLDER_NODE_ID = "dummy://folder/node";
-    private static final String DUMMY_SCRIPT_NODE_ID = "dummy-script-ref";
+    private static final NodeRef DUMMY_FOLDER_NODE = new NodeRef(DUMMY_FOLDER_NODE_ID);
+    private static final String DUMMY_SCRIPT_NODE_ID = "dummy://script/ref";
+    private static final NodeRef DUMMY_SCRIPT_NODE = new NodeRef(DUMMY_SCRIPT_NODE_ID);
 
     @Mock
     private DictionaryService dictionaryService;
@@ -90,6 +100,10 @@ public class ActionParameterConverterTest
     private ActionService actionService;
     @Mock
     private NamespaceService namespaceService;
+    @Mock
+    private PermissionService permissionService;
+    @Mock
+    private Nodes nodes;
 
     @Mock
     private ActionDefinition actionDefinition;
@@ -109,6 +123,15 @@ public class ActionParameterConverterTest
     @InjectMocks
     private ActionParameterConverter objectUnderTest;
 
+    @Before
+    public void setUp()
+    {
+        given(nodes.validateOrLookupNode(DUMMY_FOLDER_NODE_ID, null)).willReturn(DUMMY_FOLDER_NODE);
+        given(nodes.validateOrLookupNode(DUMMY_SCRIPT_NODE_ID, null)).willReturn(DUMMY_SCRIPT_NODE);
+        given(permissionService.hasReadPermission(DUMMY_FOLDER_NODE)).willReturn(ALLOWED);
+        given(permissionService.hasReadPermission(DUMMY_SCRIPT_NODE)).willReturn(ALLOWED);
+    }
+
     @Test
     public void testAddAspectConversion()
     {
@@ -146,7 +169,7 @@ public class ActionParameterConverterTest
     public void testCopyConversion()
     {
         final String name = CopyActionExecuter.NAME;
-        final String destinationFolderKey = CopyActionExecuter.PARAM_DESTINATION_FOLDER;
+        final String destinationFolderKey = PARAM_DESTINATION_FOLDER;
         final String deepCopyKey = CopyActionExecuter.PARAM_DEEP_COPY;
         final Map<String, Serializable> params = Map.of(destinationFolderKey, DUMMY_FOLDER_NODE_ID, deepCopyKey, true);
 
@@ -177,8 +200,7 @@ public class ActionParameterConverterTest
 
         final Serializable convertedCopyParam = convertedParams.get(destinationFolderKey);
         assertTrue(convertedCopyParam instanceof NodeRef);
-        assertEquals(STORE_REF_WORKSPACE_SPACESSTORE, ((NodeRef) convertedCopyParam).getStoreRef());
+        assertEquals(DUMMY_FOLDER_NODE, convertedCopyParam);
-        assertEquals(DUMMY_FOLDER_NODE_ID, ((NodeRef) convertedCopyParam).getId());
         final Serializable convertedDeepCopyParam = convertedParams.get(deepCopyKey);
         assertThat(convertedDeepCopyParam instanceof Boolean).isTrue();
         assertTrue(((Boolean) convertedDeepCopyParam));
@@ -211,8 +233,7 @@ public class ActionParameterConverterTest
 
         final Serializable convertedCopyParam = convertedParams.get(executeScriptKey);
         assertTrue(convertedCopyParam instanceof NodeRef);
-        assertEquals(STORE_REF_WORKSPACE_SPACESSTORE, ((NodeRef) convertedCopyParam).getStoreRef());
+        assertEquals(DUMMY_SCRIPT_NODE, convertedCopyParam);
-        assertEquals(DUMMY_SCRIPT_NODE_ID, ((NodeRef) convertedCopyParam).getId());
     }
 
     @Test
@@ -242,8 +263,7 @@ public class ActionParameterConverterTest
 
         final Serializable convertedCopyParam = convertedParams.get(destinationFolderKey);
         assertTrue(convertedCopyParam instanceof NodeRef);
-        assertEquals(STORE_REF_WORKSPACE_SPACESSTORE, ((NodeRef) convertedCopyParam).getStoreRef());
+        assertEquals(DUMMY_FOLDER_NODE, convertedCopyParam);
-        assertEquals(DUMMY_FOLDER_NODE_ID, ((NodeRef) convertedCopyParam).getId());
     }
 
     @Test
@@ -330,8 +350,7 @@ public class ActionParameterConverterTest
 
         final Serializable convertedDestinationParam = convertedParams.get(destinationFolderKey);
         assertTrue(convertedDestinationParam instanceof NodeRef);
-        assertEquals(STORE_REF_WORKSPACE_SPACESSTORE, ((NodeRef) convertedDestinationParam).getStoreRef());
+        assertEquals(DUMMY_FOLDER_NODE, convertedDestinationParam);
-        assertEquals(DUMMY_FOLDER_NODE_ID, ((NodeRef) convertedDestinationParam).getId());
         final Serializable convertedAssocNameParam = convertedParams.get(assocNameKey);
         assertTrue(convertedAssocNameParam instanceof QName);
         assertEquals(CHECKOUT, ((QName) convertedAssocNameParam).getLocalName());
@@ -385,8 +404,7 @@ public class ActionParameterConverterTest
         assertEquals(NamespaceService.DICTIONARY_MODEL_1_0_URI, ((QName) convertedCatValueParam).getNamespaceURI());
         final Serializable convertedDestinationParam = convertedParams.get(categoryValueKey);
         assertTrue(convertedDestinationParam instanceof NodeRef);
-        assertEquals(STORE_REF_WORKSPACE_SPACESSTORE, ((NodeRef) convertedDestinationParam).getStoreRef());
+        assertEquals(DUMMY_FOLDER_NODE, convertedDestinationParam);
-        assertEquals(DUMMY_FOLDER_NODE_ID, ((NodeRef) convertedDestinationParam).getId());
     }
 
     @Test
@@ -484,12 +502,10 @@ public class ActionParameterConverterTest
         assertEquals(reject, convertedRejectStepParam);
         final Serializable convertedApproveFolderParam = convertedParams.get(approveFolderKey);
         assertTrue(convertedApproveFolderParam instanceof NodeRef);
-        assertEquals(STORE_REF_WORKSPACE_SPACESSTORE, ((NodeRef) convertedApproveFolderParam).getStoreRef());
+        assertEquals(DUMMY_FOLDER_NODE, convertedApproveFolderParam);
-        assertEquals(DUMMY_FOLDER_NODE_ID, ((NodeRef) convertedApproveFolderParam).getId());
         final Serializable convertedRejectFolderParam = convertedParams.get(rejectFolderKey);
         assertTrue(convertedRejectFolderParam instanceof NodeRef);
-        assertEquals(STORE_REF_WORKSPACE_SPACESSTORE, ((NodeRef) convertedRejectFolderParam).getStoreRef());
+        assertEquals(DUMMY_FOLDER_NODE, convertedRejectFolderParam);
-        assertEquals(DUMMY_FOLDER_NODE_ID, ((NodeRef) convertedRejectFolderParam).getId());
         final Serializable convertedApproveMoveParam = convertedParams.get(approveMoveKey);
         assertTrue(convertedApproveMoveParam instanceof Boolean);
         assertTrue((Boolean) convertedApproveMoveParam);
@@ -555,6 +571,44 @@ public class ActionParameterConverterTest
         assertEquals(propType, convertedPropTypeParam);
     }
 
+    @Test
+    public void testNonExistentNodeParam()
+    {
+        final String name = CopyActionExecuter.NAME;
+        final Map<String, Serializable> params = Map.of(PARAM_DESTINATION_FOLDER, "non://existent/node");
+
+        given(actionService.getActionDefinition(name)).willReturn(actionDefinition);
+        given(actionDefinition.getParameterDefintion(PARAM_DESTINATION_FOLDER)).willReturn(actionDefinitionParam1);
+        final QName nodeRef = DataTypeDefinition.NODE_REF;
+        given(actionDefinitionParam1.getType()).willReturn(nodeRef);
+
+        given(dictionaryService.getDataType(nodeRef)).willReturn(dataTypeDefinition1);
+
+        //when
+        assertThatExceptionOfType(EntityNotFoundException.class).isThrownBy(() -> objectUnderTest.getConvertedParams(params, name));
+    }
+
+    @Test
+    public void testNoReadPermissionForNodeParam()
+    {
+        final String name = CopyActionExecuter.NAME;
+        String permissionDeniedNodeId = "permission://denied/node";
+        final Map<String, Serializable> params = Map.of(PARAM_DESTINATION_FOLDER, permissionDeniedNodeId);
+        NodeRef permissionDeniedNode = new NodeRef(permissionDeniedNodeId);
+        given(nodes.validateOrLookupNode(permissionDeniedNodeId, null)).willReturn(permissionDeniedNode);
+        given(permissionService.hasReadPermission(permissionDeniedNode)).willReturn(DENIED);
+
+        given(actionService.getActionDefinition(name)).willReturn(actionDefinition);
+        given(actionDefinition.getParameterDefintion(PARAM_DESTINATION_FOLDER)).willReturn(actionDefinitionParam1);
+        final QName nodeRef = DataTypeDefinition.NODE_REF;
+        given(actionDefinitionParam1.getType()).willReturn(nodeRef);
+
+        given(dictionaryService.getDataType(nodeRef)).willReturn(dataTypeDefinition1);
+
+        //when
+        assertThatExceptionOfType(EntityNotFoundException.class).isThrownBy(() -> objectUnderTest.getConvertedParams(params, name));
+    }
+
     @Test
     public void testInvalidActionDefinitionConversion() {
         final String invalidName = "dummy-definition";