diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java index c2a4123273..e0a529ad04 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java +++ b/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 */ diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/RulesTestsUtils.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/RulesTestsUtils.java index 8447946762..7399ef259f 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/RulesTestsUtils.java +++ b/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. */ diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/ActionParameterConverter.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/ActionParameterConverter.java index e503495580..1c3cf11eb2 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/ActionParameterConverter.java +++ b/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, - NamespaceService namespaceService) + public ActionParameterConverter(DictionaryService dictionaryService, ActionService actionService, NamespaceService namespaceService, + PermissionService permissionService, Nodes nodes) { this.dictionaryService = dictionaryService; this.actionService = actionService; this.namespaceService = namespaceService; + this.permissionService = permissionService; + this.nodes = nodes; } public Map getConvertedParams(Map 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 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 { diff --git a/remote-api/src/main/java/org/alfresco/rest/framework/core/exceptions/EntityNotFoundException.java b/remote-api/src/main/java/org/alfresco/rest/framework/core/exceptions/EntityNotFoundException.java index 8ba193cff8..e053e1da13 100644 --- a/remote-api/src/main/java/org/alfresco/rest/framework/core/exceptions/EntityNotFoundException.java +++ b/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); diff --git a/remote-api/src/main/resources/alfresco/public-rest-context.xml b/remote-api/src/main/resources/alfresco/public-rest-context.xml index 1178e6cc04..ab10dbe021 100644 --- a/remote-api/src/main/resources/alfresco/public-rest-context.xml +++ b/remote-api/src/main/resources/alfresco/public-rest-context.xml @@ -910,6 +910,8 @@ + + diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionParameterConverterTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionParameterConverterTest.java index 7e5077834c..5bd9e7254b 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionParameterConverterTest.java +++ b/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_SCRIPT_NODE_ID = "dummy-script-ref"; + private static final String DUMMY_FOLDER_NODE_ID = "dummy://folder/node"; + 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 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_ID, ((NodeRef) convertedCopyParam).getId()); + assertEquals(DUMMY_FOLDER_NODE, convertedCopyParam); 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_ID, ((NodeRef) convertedCopyParam).getId()); + assertEquals(DUMMY_SCRIPT_NODE, convertedCopyParam); } @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_ID, ((NodeRef) convertedCopyParam).getId()); + assertEquals(DUMMY_FOLDER_NODE, convertedCopyParam); } @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_ID, ((NodeRef) convertedDestinationParam).getId()); + assertEquals(DUMMY_FOLDER_NODE, convertedDestinationParam); 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_ID, ((NodeRef) convertedDestinationParam).getId()); + assertEquals(DUMMY_FOLDER_NODE, convertedDestinationParam); } @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_ID, ((NodeRef) convertedApproveFolderParam).getId()); + assertEquals(DUMMY_FOLDER_NODE, convertedApproveFolderParam); final Serializable convertedRejectFolderParam = convertedParams.get(rejectFolderKey); assertTrue(convertedRejectFolderParam instanceof NodeRef); - assertEquals(STORE_REF_WORKSPACE_SPACESSTORE, ((NodeRef) convertedRejectFolderParam).getStoreRef()); - assertEquals(DUMMY_FOLDER_NODE_ID, ((NodeRef) convertedRejectFolderParam).getId()); + assertEquals(DUMMY_FOLDER_NODE, convertedRejectFolderParam); 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 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 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";