PRODSEC-4422 - Scripts not in Data Dictionary can be executed by action (#596)

* Added validation to the ScriptActionExecuter class to enforce the existing constraints on parameter script-ref (Repo has the constraint to only allow scripts in Data Dictionary / Scripts and AGS has the constraint to only allow scripts in Data Dictionary / Records Management / Records Management Scripts") by validating if the given scriptRef is in the allowed valued of the constraint set on that param
* Added a new unit test for AGS to make sure that rmscript action still works as expected when the script is in the correct folder and fails when not
* Added new case in ActionServiceImpl2Test#testExecuteScript to assert that the transaction fails when we execute the action with an invalid script
* Moved test testActionResult from ActionServiceImplTest to class ActionServiceImpl2Test - Before it ran with a script not in Data Dictionary so with the added validation it started to fail. I moved the unit test do avoid duplicating the code to create the script in the correct location.
This commit is contained in:
evasques
2021-07-15 17:45:22 +01:00
committed by GitHub
parent af0fd146f2
commit ac4a1643e1
5 changed files with 294 additions and 57 deletions

View File

@@ -34,7 +34,10 @@ import org.alfresco.repo.action.ParameterDefinitionImpl;
import org.alfresco.repo.admin.SysAdminParams;
import org.alfresco.repo.jscript.ScriptAction;
import org.alfresco.service.ServiceRegistry;
import org.alfresco.service.cmr.action.Action;
import org.alfresco.service.cmr.action.Action;
import org.alfresco.service.cmr.action.ActionDefinition;
import org.alfresco.service.cmr.action.ActionService;
import org.alfresco.service.cmr.action.ParameterConstraint;
import org.alfresco.service.cmr.action.ParameterDefinition;
import org.alfresco.service.cmr.dictionary.DataTypeDefinition;
import org.alfresco.service.cmr.repository.NodeRef;
@@ -126,6 +129,10 @@ public class ScriptActionExecuter extends ActionExecuterAbstractBase
if (nodeService.exists(actionedUponNodeRef))
{
NodeRef scriptRef = (NodeRef)action.getParameterValue(PARAM_SCRIPTREF);
if(!isValidScriptRef(action))
{
throw new IllegalStateException("Invalid script ref path: " + scriptRef);
}
NodeRef spaceRef = this.serviceRegistry.getRuleService().getOwningNodeRef(action);
if (spaceRef == null)
{
@@ -222,4 +229,19 @@ public class ScriptActionExecuter extends ActionExecuterAbstractBase
return companyHomeRef;
}
private boolean isValidScriptRef(Action action)
{
NodeRef scriptRef = (NodeRef) action.getParameterValue(PARAM_SCRIPTREF);
ActionService actionService = this.serviceRegistry.getActionService();
ActionDefinition actDef = actionService.getActionDefinition(action.getActionDefinitionName());
ParameterDefinition parameterDef = actDef.getParameterDefintion(PARAM_SCRIPTREF);
String paramConstraintName = parameterDef.getParameterConstraintName();
if (paramConstraintName != null)
{
ParameterConstraint paramConstraint = actionService.getParameterConstraint(paramConstraintName);
return paramConstraint.isValidValue(scriptRef.toString());
}
return true;
}
}

View File

@@ -26,8 +26,8 @@
package org.alfresco.repo.action;
import static java.lang.Thread.sleep;
import static junit.framework.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
@@ -39,6 +39,7 @@ import java.util.List;
import java.util.Map;
import org.alfresco.model.ContentModel;
import org.alfresco.repo.action.executer.ActionExecuter;
import org.alfresco.repo.action.executer.ContentMetadataExtracter;
import org.alfresco.repo.action.executer.CounterIncrementActionExecuter;
import org.alfresco.repo.action.executer.ScriptActionExecuter;
@@ -259,7 +260,7 @@ public class ActionServiceImpl2Test
public void testExecuteScript() throws Exception
{
final NodeRef scriptToBeExecuted = addTempScript("changeFileNameTest.js",
"document.properties.name = \"Changed\" + \"_\" + document.properties.name;\ndocument.save();");
"document.properties.name = \"Changed_\" + document.properties.name;\ndocument.save();");
assertNotNull("Failed to add the test script.", scriptToBeExecuted);
// add a test file to the Site in order to change its name
@@ -310,6 +311,73 @@ public class ActionServiceImpl2Test
return null;
}
});
//Execute script not in Data Dictionary > Scripts
AuthenticationUtil.setFullyAuthenticatedUser(testSiteAndMemberInfo.siteManager);
NodeRef companyHomeRef = wellKnownNodes.getCompanyHome();
NodeRef sharedFolderRef = nodeService.getChildByName(companyHomeRef, ContentModel.ASSOC_CONTAINS,
"Shared");
final NodeRef invalidScriptRef = addTempScript("changeFileNameTest.js",
"document.properties.name = \"Invalid_Change.pdf\";\ndocument.save();",sharedFolderRef);
assertNotNull("Failed to add the test script.", scriptToBeExecuted);
transactionHelper.doInTransaction(new RetryingTransactionCallback<Void>()
{
public Void execute() throws Throwable
{
// Create the action
Action action = actionService.createAction(ScriptActionExecuter.NAME);
action.setParameterValue(ScriptActionExecuter.PARAM_SCRIPTREF, invalidScriptRef);
try
{
// Execute the action
actionService.executeAction(action, testNode);
}
catch (Throwable th)
{
// do nothing
}
assertFalse("Scripts outside of Data Dictionary Scripts folder should not be executed",
("Invalid_Change.pdf".equals(nodeService.getProperty(testNode, ContentModel.PROP_NAME))));
return null;
}
});
}
@Test
public void testActionResult() throws Exception
{
AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
transactionHelper.doInTransaction(new RetryingTransactionCallback<Void>()
{
public Void execute() throws Throwable
{
try
{
// Create the script node reference
NodeRef script = addTempScript("test-action-result-script.js", "\"VALUE\";");
// Create the action
Action action = actionService.createAction(ScriptActionExecuter.NAME);
action.setParameterValue(ScriptActionExecuter.PARAM_SCRIPTREF, script);
// Execute the action
actionService.executeAction(action, testNode);
// Get the result
String result = (String) action.getParameterValue(ActionExecuter.PARAM_RESULT);
assertNotNull(result);
assertEquals("VALUE", result);
}
finally
{
AuthenticationUtil.clearCurrentSecurityContext();
}
return null;
}
});
}
@Test
@@ -369,6 +437,32 @@ public class ActionServiceImpl2Test
});
}
private NodeRef addTempScript(final String scriptFileName, final String javaScript, final NodeRef parentRef)
{
AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
return transactionHelper.doInTransaction(new RetryingTransactionCallback<NodeRef>()
{
public NodeRef execute() throws Throwable
{
// Create the script node reference
NodeRef script = nodeService.createNode(parentRef, ContentModel.ASSOC_CONTAINS,
QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, scriptFileName),
ContentModel.TYPE_CONTENT).getChildRef();
nodeService.setProperty(script, ContentModel.PROP_NAME, scriptFileName);
ContentWriter contentWriter = contentService.getWriter(script, ContentModel.PROP_CONTENT, true);
contentWriter.setMimetype(MimetypeMap.MIMETYPE_JAVASCRIPT);
contentWriter.setEncoding("UTF-8");
contentWriter.putContent(javaScript);
tempNodes.addNodeRef(script);
return script;
}
});
}
private NodeRef addTempScript(final String scriptFileName, final String javaScript)
{
AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
@@ -386,20 +480,7 @@ public class ActionServiceImpl2Test
NodeRef scriptsRef = nodeService.getChildByName(dataDictionaryRef, ContentModel.ASSOC_CONTAINS,
"Scripts");
// Create the script node reference
NodeRef script = nodeService.createNode(scriptsRef, ContentModel.ASSOC_CONTAINS,
QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, scriptFileName),
ContentModel.TYPE_CONTENT).getChildRef();
nodeService.setProperty(script, ContentModel.PROP_NAME, scriptFileName);
ContentWriter contentWriter = contentService.getWriter(script, ContentModel.PROP_CONTENT, true);
contentWriter.setMimetype(MimetypeMap.MIMETYPE_JAVASCRIPT);
contentWriter.setEncoding("UTF-8");
contentWriter.putContent(javaScript);
tempNodes.addNodeRef(script);
return script;
return addTempScript(scriptFileName, javaScript, scriptsRef);
}
});
}

View File

@@ -805,46 +805,6 @@ public class ActionServiceImplTest extends BaseAlfrescoSpringTest
assertEquals(action4, savedAction2.getAction(2));
}
/**
* Test the action result parameter
*/
@Test
public void testActionResult()
{
// We need to run this test as Administrator. The ScriptAction has to run as a full user (instead of as System)
// so that we can setup the Person object in the ScriptNode
AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
try
{
// Create the script node reference
NodeRef script = this.nodeService.createNode(
this.folder,
ContentModel.ASSOC_CONTAINS,
QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "testScript.js"),
ContentModel.TYPE_CONTENT).getChildRef();
this.nodeService.setProperty(script, ContentModel.PROP_NAME, "testScript.js");
ContentWriter contentWriter = this.contentService.getWriter(script, ContentModel.PROP_CONTENT, true);
contentWriter.setMimetype("text/plain");
contentWriter.setEncoding("UTF-8");
contentWriter.putContent("\"VALUE\";");
// Create the action
Action action1 = this.actionService.createAction(ScriptActionExecuter.NAME);
action1.setParameterValue(ScriptActionExecuter.PARAM_SCRIPTREF, script);
// Execute the action
this.actionService.executeAction(action1, this.nodeRef);
// Get the result
String result = (String)action1.getParameterValue(ActionExecuter.PARAM_RESULT);
assertNotNull(result);
assertEquals("VALUE", result);
}
finally
{
AuthenticationUtil.clearCurrentSecurityContext();
}
}
/** ===================================================================================
* Test asynchronous actions