Added task update protection to the task form processor and changed the check in TaskInstancePut to use the new isTaskEditable method on the WorkflowService.

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@21918 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
2025-08-07 17:49:17 +00:00 · 2010-08-23 07:24:12 +00:00
parent 4cad8ab1fd
commit 96b61bae8c
5 changed files with 82 additions and 6 deletions
--- a/source/java/org/alfresco/repo/forms/FormServiceImplTest.java
+++ b/source/java/org/alfresco/repo/forms/FormServiceImplTest.java
@@ -35,6 +35,7 @@ import org.alfresco.repo.forms.PropertyFieldDefinition.FieldConstraint;
 import org.alfresco.repo.forms.processor.node.FormFieldConstants;
 import org.alfresco.repo.forms.processor.node.TypeFormProcessor;
 import org.alfresco.repo.jscript.ClasspathScriptLocation;
+import org.alfresco.repo.security.permissions.AccessDeniedException;
 import org.alfresco.repo.workflow.WorkflowModel;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.repository.ContentData;
@@ -114,9 +115,11 @@ public class FormServiceImplTest extends BaseAlfrescoSpringTest
    private static String LABEL_CONTAINS = "Contains";
    
    private static final String USER_ONE = "UserOne_FormServiceImplTest";
+    private static final String USER_TWO = "UserTwo_FormServiceImplTest";
    private static final String NODE_FORM_ITEM_KIND = "node";
    private static final String TYPE_FORM_ITEM_KIND = "type";
    private static final String WORKFLOW_FORM_ITEM_KIND = "workflow";
+    private static final String TASK_FORM_ITEM_KIND = "task";
    
    /**
     * Called during the transaction setup
@@ -135,8 +138,11 @@ public class FormServiceImplTest extends BaseAlfrescoSpringTest
        this.contentService = (ContentService)this.applicationContext.getBean("ContentService");
        this.workflowService = (WorkflowService)this.applicationContext.getBean("WorkflowService");
        
-        // Do the tests as userOne
+        // create users
        createUser(USER_ONE);
+        createUser(USER_TWO);
+        
+        // Do the tests as userOne
        authenticationComponent.setCurrentUser(USER_ONE);
        
        String guid = GUID.generate();
@@ -1304,7 +1310,7 @@ public class FormServiceImplTest extends BaseAlfrescoSpringTest
        return fields;
    }
    
-    public void testWorkflowForm() throws Exception
+    public void testWorkflowForms() throws Exception
    {
        // generate a form for a well known workflow-definition supplying
        // a legitimate set of fields for the workflow
@@ -1374,6 +1380,33 @@ public class FormServiceImplTest extends BaseAlfrescoSpringTest
        
        // check workflow instance details
        assertEquals("jbpm$wf:adhoc", workflow.definition.name);
+        
+        // update the first task in the users list
+        String taskId = tasks.get(0).getId();
+        String comment = "This is a comment";
+        data = new FormData();
+        data.addFieldData("prop_bpm_comment", comment);
+        this.formService.saveForm(new Item(TASK_FORM_ITEM_KIND, taskId), data);
+        
+        // check the comment was updated
+        WorkflowTask task = workflowService.getTaskById(taskId);
+        String taskComment = (String)task.getProperties().get(WorkflowModel.PROP_COMMENT);
+        assertEquals(comment, taskComment);
+        
+        // make sure unauthorized user can not update the task
+        authenticationComponent.setCurrentUser(USER_TWO);
+        
+        try
+        {
+            // try and update task
+            this.formService.saveForm(new Item(TASK_FORM_ITEM_KIND, taskId), data);
+            
+            fail("Task was updated by an unauthorized user");
+        }
+        catch (AccessDeniedException ade)
+        {
+            // expected
+        }
    }
    
    public void testNoForm() throws Exception
--- a/source/java/org/alfresco/repo/forms/processor/workflow/TaskFormPersister.java
+++ b/source/java/org/alfresco/repo/forms/processor/workflow/TaskFormPersister.java
@@ -24,10 +24,12 @@ import java.util.List;

 import org.alfresco.repo.forms.FormData.FieldData;
 import org.alfresco.repo.forms.processor.node.ContentModelItemData;
+import org.alfresco.repo.security.permissions.AccessDeniedException;
 import org.alfresco.repo.workflow.TaskUpdater;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
+import org.alfresco.service.cmr.security.AuthenticationService;
 import org.alfresco.service.cmr.workflow.WorkflowService;
 import org.alfresco.service.cmr.workflow.WorkflowTask;
 import org.alfresco.service.namespace.NamespaceService;
@@ -50,10 +52,18 @@ public class TaskFormPersister extends ContentModelFormPersister<WorkflowTask>
                DictionaryService dictionaryService,
                WorkflowService workflowService,
                NodeService nodeService,
+                AuthenticationService authenticationService,
                Log logger)
    {
        super(itemData, namespaceService, dictionaryService, logger);
        WorkflowTask item = itemData.getItem();
+        
+        // make sure the current user is able to edit the task
+        if (!workflowService.isTaskEditable(item, authenticationService.getCurrentUserName()))
+        {
+            throw new AccessDeniedException("Failed to update task with id '" + item.getId() + "'.");
+        }
+        
        this.updater = new TaskUpdater(item.id, workflowService, nodeService);
    }
    
--- a/source/java/org/alfresco/repo/forms/processor/workflow/TaskFormProcessor.java
+++ b/source/java/org/alfresco/repo/forms/processor/workflow/TaskFormProcessor.java
@@ -38,7 +38,10 @@ import org.alfresco.repo.forms.processor.node.ContentModelItemData;
 import org.alfresco.repo.workflow.WorkflowModel;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.dictionary.TypeDefinition;
+import org.alfresco.service.cmr.model.FileFolderService;
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.repository.NodeService;
+import org.alfresco.service.cmr.security.AuthenticationService;
 import org.alfresco.service.cmr.workflow.WorkflowService;
 import org.alfresco.service.cmr.workflow.WorkflowTask;
 import org.alfresco.service.cmr.workflow.WorkflowTransition;
@@ -58,6 +61,8 @@ public class TaskFormProcessor extends AbstractWorkflowFormProcessor<WorkflowTas
    /** Logger */
    private static final Log LOGGER = LogFactory.getLog(TaskFormProcessor.class);

+    protected AuthenticationService authenticationService;
+    
    // Constructor for Spring
    public TaskFormProcessor()
    {
@@ -66,14 +71,26 @@ public class TaskFormProcessor extends AbstractWorkflowFormProcessor<WorkflowTas

    // Constructor for tests.
    public TaskFormProcessor(WorkflowService workflowService, NamespaceService namespaceService,
-            DictionaryService dictionaryService, FieldProcessorRegistry fieldProcessorRegistry)
+            DictionaryService dictionaryService, AuthenticationService authenticationService,
+            FieldProcessorRegistry fieldProcessorRegistry)
    {
        this.workflowService = workflowService;
        this.namespaceService = namespaceService;
        this.dictionaryService = dictionaryService;
+        this.authenticationService = authenticationService;
        this.fieldProcessorRegistry = fieldProcessorRegistry;
    }

+    /**
+     * Sets the authentication service
+     * 
+     * @param authenticationService The AuthenticationService instance
+     */
+    public void setAuthenticationService(AuthenticationService authenticationService)
+    {
+        this.authenticationService = authenticationService;
+    }
+    
    /* (non-Javadoc)
     * @see org.alfresco.repo.forms.processor.workflow.AbstractWorkflowFormProcessor#getTypedItemForDecodedId(java.lang.String)
     */
@@ -216,6 +233,7 @@ public class TaskFormProcessor extends AbstractWorkflowFormProcessor<WorkflowTas
    protected ContentModelFormPersister<WorkflowTask> makeFormPersister(WorkflowTask item)
    {
        ContentModelItemData<WorkflowTask> itemData = makeItemData(item);
-        return new TaskFormPersister(itemData, namespaceService, dictionaryService, workflowService, nodeService, LOGGER);
+        return new TaskFormPersister(itemData, namespaceService, dictionaryService, 
+                    workflowService, nodeService, authenticationService, LOGGER);
    }
 }
--- a/source/java/org/alfresco/repo/forms/processor/workflow/TaskFormProcessorTest.java
+++ b/source/java/org/alfresco/repo/forms/processor/workflow/TaskFormProcessorTest.java
@@ -76,6 +76,7 @@ import org.alfresco.service.cmr.dictionary.TypeDefinition;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
+import org.alfresco.service.cmr.security.AuthenticationService;
 import org.alfresco.service.cmr.workflow.WorkflowDefinition;
 import org.alfresco.service.cmr.workflow.WorkflowException;
 import org.alfresco.service.cmr.workflow.WorkflowInstance;
@@ -119,6 +120,7 @@ public class TaskFormProcessorTest extends TestCase
    private TaskFormProcessor processor;
    private WorkflowTask task;
    private NamespaceService namespaceService;
+    private AuthenticationService authenticationService;
    private WorkflowTask newTask;
    private Map<QName, Serializable> actualProperties = null;
    private Map<QName, List<NodeRef>> actualAdded= null;
@@ -523,6 +525,7 @@ public class TaskFormProcessorTest extends TestCase
        nodeService = makeNodeService();
        DictionaryService dictionaryService = makeDictionaryService();
        namespaceService = makeNamespaceService();
+        authenticationService = makeAuthenticationService();
        MockFieldProcessorRegistry fieldProcessorRegistry = new MockFieldProcessorRegistry(namespaceService,
                    dictionaryService);
        DefaultFieldProcessor defaultProcessor = makeDefaultFieldProcessor(dictionaryService);
@@ -537,6 +540,7 @@ public class TaskFormProcessorTest extends TestCase
        processor1.setNodeService(nodeService);
        processor1.setNamespaceService(namespaceService);
        processor1.setDictionaryService(dictionaryService);
+        processor1.setAuthenticationService(authenticationService);
        processor1.setFieldProcessorRegistry(fieldProcessorRegistry);
        return processor1;
    }
@@ -670,6 +674,13 @@ public class TaskFormProcessorTest extends TestCase
        return mock;
    }

+    private AuthenticationService makeAuthenticationService()
+    {
+        AuthenticationService mock = mock(AuthenticationService.class);
+        when(mock.getCurrentUserName()).thenReturn("admin");
+        return mock;
+    }
+
    @SuppressWarnings("unchecked")
    private WorkflowService makeWorkflowService()
    {
@@ -707,6 +718,9 @@ public class TaskFormProcessorTest extends TestCase
        
        when(service.endTask(eq(TASK_ID), anyString()))
            .thenReturn(newTask);
+        
+        when(service.isTaskEditable((WorkflowTask)any(), anyString())).thenReturn(true);
+        
        return service;
    }