Fixed ALF-4921: Incorrect behaviour of pooled tasks for sub-groups

Also fixed unreported error that allowed any member of the assigned pooled group to edit a task after it was claimed.

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@22987 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/workflow/WorkflowModel.java

@@ -67,6 +67,8 @@ public interface WorkflowModel
     static final QName PROP_WORKFLOW_DUE_DATE = QName.createQName(NamespaceService.BPM_MODEL_1_0_URI, "workflowDueDate");
     static final QName ASSOC_ASSIGNEE = QName.createQName(NamespaceService.BPM_MODEL_1_0_URI, "assignee");
     static final QName ASSOC_ASSIGNEES = QName.createQName(NamespaceService.BPM_MODEL_1_0_URI, "assignees");
+    static final QName ASSOC_GROUP_ASSIGNEE = QName.createQName(NamespaceService.BPM_MODEL_1_0_URI, "groupAssignee");
+    static final QName ASSOC_GROUP_ASSIGNEES = QName.createQName(NamespaceService.BPM_MODEL_1_0_URI, "groupAssignees");
 
     // workflow package
     static final QName ASPECT_WORKFLOW_PACKAGE = QName.createQName(NamespaceService.BPM_MODEL_1_0_URI, "workflowPackage");

source/java/org/alfresco/repo/workflow/WorkflowServiceImpl.java

@@ -665,10 +665,19 @@ public class WorkflowServiceImpl implements WorkflowService
             return true;
         }
         
-        // if the current user is not the owner or initiator check whether they are
+        if (task.getProperties().get(ContentModel.PROP_OWNER) == null)
+        {
+            // if the user is not the owner or initiator check whether they are
             // a member of the pooled actors for the task (if it has any)
             return isUserInPooledActors(task, username);
         }
+        else
+        {
+            // if the task has an owner and the user is not the owner
+            // or the initiator do not allow editing
+            return false;
+        }
+    }
     
     /*
      * @see org.alfresco.service.cmr.workflow.WorkflowService#isTaskReassignable(org.alfresco.service.cmr.workflow.WorkflowTask, java.lang.String)
@@ -980,7 +989,7 @@ public class WorkflowServiceImpl implements WorkflowService
                 String name = (String)props.get(ContentModel.PROP_AUTHORITY_NAME);
                 
                 // retrieve the users of the group
-                Set<String> users = this.authorityService.getContainedAuthorities(AuthorityType.USER, name, true);
+                Set<String> users = this.authorityService.getContainedAuthorities(AuthorityType.USER, name, false);
                 
                 // see if the user is one of the users in the group
                 if (users != null && !users.isEmpty() && users.contains(username))

source/java/org/alfresco/repo/workflow/WorkflowServiceImplTest.java

@@ -19,17 +19,26 @@
 package org.alfresco.repo.workflow;
 
 import java.io.Serializable;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.security.authentication.AuthenticationComponent;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.person.TestPersonManager;
 import org.alfresco.service.ServiceRegistry;
 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.repository.StoreRef;
+import org.alfresco.service.cmr.search.ResultSet;
+import org.alfresco.service.cmr.search.SearchService;
+import org.alfresco.service.cmr.security.AuthorityService;
+import org.alfresco.service.cmr.security.AuthorityType;
+import org.alfresco.service.cmr.security.MutableAuthenticationService;
+import org.alfresco.service.cmr.security.PersonService;
 import org.alfresco.service.cmr.workflow.WorkflowDefinition;
 import org.alfresco.service.cmr.workflow.WorkflowInstance;
 import org.alfresco.service.cmr.workflow.WorkflowPath;
@@ -41,6 +50,7 @@ import org.alfresco.service.cmr.workflow.WorkflowTaskState;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.util.BaseSpringTest;
+import org.alfresco.util.GUID;
 
 
 /**
@@ -50,18 +60,133 @@ import org.alfresco.util.BaseSpringTest;
  */
 public class WorkflowServiceImplTest extends BaseSpringTest
 {
-    WorkflowService workflowService;
+    private final static String USER1 = "WFUser1" + GUID.generate();
-    NodeService nodeService;
+    private final static String USER2 = "WFUser2" + GUID.generate();
+    private final static String USER3 = "WFUser3" + GUID.generate();
+    private final static String GROUP = "WFGroup" + GUID.generate();
+    private final static String SUB_GROUP = "WFSubGroup" + GUID.generate();
+    
+    private WorkflowService workflowService;
+    private NodeService nodeService;
+    private SearchService searchService;
+    private AuthenticationComponent authenticationComponent;
+    private MutableAuthenticationService authenticationService;
+    private AuthorityService authorityService;
+    private PersonService personService;
+    private TestPersonManager personManager;
 
     //@Override
+    @SuppressWarnings("deprecation")
     protected void onSetUpInTransaction() throws Exception
     {
         workflowService = (WorkflowService)applicationContext.getBean(ServiceRegistry.WORKFLOW_SERVICE.getLocalName());
         nodeService = (NodeService)applicationContext.getBean(ServiceRegistry.NODE_SERVICE.getLocalName());
+        searchService = (SearchService)applicationContext.getBean(ServiceRegistry.SEARCH_SERVICE.getLocalName());
+        authenticationService = (MutableAuthenticationService) applicationContext.getBean(ServiceRegistry.AUTHENTICATION_SERVICE.getLocalName());
+        authorityService = (AuthorityService) applicationContext.getBean(ServiceRegistry.AUTHORITY_SERVICE.getLocalName());
+        personService = (PersonService) applicationContext.getBean(ServiceRegistry.PERSON_SERVICE.getLocalName());
         
         // authenticate
-        AuthenticationComponent auth = (AuthenticationComponent) applicationContext.getBean("authenticationComponent");
+        authenticationComponent = (AuthenticationComponent) applicationContext.getBean("authenticationComponent");
-        auth.setSystemUserAsCurrentUser();
+        authenticationComponent.setSystemUserAsCurrentUser();
+        
+        // create test users
+        personManager = new TestPersonManager(authenticationService, personService, nodeService);
+        personManager.createPerson(USER1);
+        personManager.createPerson(USER2);
+        personManager.createPerson(USER3);
+
+        // create test groups
+        createGroup(null, GROUP);
+        createGroup(GROUP, SUB_GROUP);
+        
+        // add users to groups
+        addUserToGroup(GROUP, USER1);
+        addUserToGroup(SUB_GROUP, USER2);
+    }
+    
+    @SuppressWarnings("deprecation")
+    @Override
+    protected void onTearDownInTransaction() throws Exception
+    {
+        super.onTearDownInTransaction();
+        
+        authenticationComponent.setSystemUserAsCurrentUser();
+        deleteGroup(SUB_GROUP);
+        deleteGroup(GROUP);
+        personManager.clearPeople();
+    }
+
+    protected void createGroup(String parentGroupShortName, String groupShortName)
+    {
+        if (parentGroupShortName != null)
+        {
+            String parentGroupFullName = authorityService.getName(AuthorityType.GROUP, parentGroupShortName);
+            if (authorityService.authorityExists(parentGroupFullName))
+            {
+                authorityService.createAuthority(AuthorityType.GROUP, groupShortName, groupShortName, null);
+                
+                String groupFullName = authorityService.getName(AuthorityType.GROUP, groupShortName);
+                authorityService.addAuthority(parentGroupFullName, groupFullName);
+            }
+        }
+        else
+        {
+            authorityService.createAuthority(AuthorityType.GROUP, groupShortName, groupShortName, null);
+        }
+    }
+    
+    protected void addUserToGroup(String groupName, String userName)
+    {
+        // get the full name for the group
+        String fullGroupName = this.authorityService.getName(AuthorityType.GROUP, groupName);
+
+        // create group if it does not exist
+        if (this.authorityService.authorityExists(fullGroupName) == false)
+        {
+            this.authorityService.createAuthority(AuthorityType.GROUP, fullGroupName);
+        }
+
+        // add the user to the group
+        this.authorityService.addAuthority(fullGroupName, userName);
+    }
+    
+    protected void deleteGroup(String groupShortName)
+    {
+        String groupFullName = authorityService.getName(AuthorityType.GROUP, groupShortName);
+        if (authorityService.authorityExists(groupFullName) == true)
+        {
+            authorityService.deleteAuthority(groupFullName);
+        }
+    }
+    
+    protected NodeRef findGroup(String shortGroupName)
+    {
+        NodeRef group = null;
+        
+        String query = "+TYPE:\"cm:authorityContainer\" AND @cm\\:authorityName:*" + shortGroupName;
+        
+        ResultSet results = null;
+        try
+        {
+            results = this.searchService.query(
+                    new StoreRef(StoreRef.PROTOCOL_WORKSPACE, "SpacesStore"), 
+                    SearchService.LANGUAGE_LUCENE, query);
+            
+            if (results.length() > 0)
+            {
+                group = results.getNodeRefs().get(0);
+            }
+        }
+        finally
+        {
+            if (results != null)
+            {
+                results.close();
+            }
+        }
+        
+        return group;
     }
 
     public void testGetWorkflowDefinitions()
@@ -197,4 +322,164 @@ public class WorkflowServiceImplTest extends BaseSpringTest
             assertTrue(workflowTaskDefs.size() > 0);
         }
     }
+    
+    public void testTaskCapabilities()
+    {
+        // start Adhoc workflow as USER1 and assign to USER2
+        personManager.setUser(USER1);
+        
+        // Get the workflow definition.
+        WorkflowDefinition workflowDef = this.workflowService.getDefinitionByName("jbpm$wf:adhoc");
+        assertNotNull(workflowDef);
+        
+        // Create workflow parameters
+        Map<QName, Serializable> params = new HashMap<QName, Serializable>();
+        Serializable wfPackage = workflowService.createPackage(null);
+        params.put(WorkflowModel.ASSOC_PACKAGE, wfPackage);
+        Date dueDate = new Date();
+        params.put(WorkflowModel.PROP_WORKFLOW_DUE_DATE, dueDate);
+        params.put(WorkflowModel.PROP_WORKFLOW_PRIORITY, 1);
+        NodeRef assignee = personService.getPerson(USER2);
+        params.put(WorkflowModel.ASSOC_ASSIGNEE, assignee);
+        
+        // Start a workflow instance
+        WorkflowPath path = workflowService.startWorkflow(workflowDef.getId(), params);
+        assertNotNull(path);
+        assertTrue(path.isActive());
+        String workflowInstanceId = path.getInstance().getId();
+        
+        // End start task to progress workflow
+        List<WorkflowTask> tasks = workflowService.getTasksForWorkflowPath(path.getId());
+        assertEquals(1, tasks.size());
+        WorkflowTask startTask = tasks.get(0);
+        String startTaskId = startTask.getId();
+        workflowService.endTask(startTaskId, null);
+        
+        // Fetch start task and check capabilities
+        startTask = workflowService.getTaskById(startTaskId);
+        assertNotNull(startTask);
+        assertEquals(startTask.getState(), WorkflowTaskState.COMPLETED);
+        
+        // check nothing can be done to the task as its completed
+        assertFalse(workflowService.isTaskClaimable(startTask, USER1));
+        assertFalse(workflowService.isTaskEditable(startTask, USER1));
+        assertFalse(workflowService.isTaskReassignable(startTask, USER1));
+        assertFalse(workflowService.isTaskReleasable(startTask, USER1));
+        
+        // Fetch the current task in the workflow
+        List<WorkflowPath> paths = workflowService.getWorkflowPaths(workflowInstanceId);
+        assertNotNull(paths);
+        assertEquals(1, paths.size());
+        tasks = workflowService.getTasksForWorkflowPath(path.getId());
+        assertEquals(1, tasks.size());
+        WorkflowTask currentTask = tasks.get(0);
+        assertEquals(currentTask.getState(), WorkflowTaskState.IN_PROGRESS);
+        assertEquals(currentTask.getProperties().get(ContentModel.PROP_OWNER), USER2);
+        
+        // check the task is not claimable or releasable by any user as it is not a pooled task
+        assertFalse(workflowService.isTaskClaimable(currentTask, USER1));
+        assertFalse(workflowService.isTaskClaimable(currentTask, USER2));
+        assertFalse(workflowService.isTaskClaimable(currentTask, USER3));
+        assertFalse(workflowService.isTaskReleasable(currentTask, USER1));
+        assertFalse(workflowService.isTaskReleasable(currentTask, USER2));
+        assertFalse(workflowService.isTaskReleasable(currentTask, USER3));
+        
+        // user1 (initiator) and user2 (owner) should be able to edit and reassign task
+        assertTrue(workflowService.isTaskEditable(currentTask, USER1));
+        assertTrue(workflowService.isTaskEditable(currentTask, USER2));
+        assertTrue(workflowService.isTaskReassignable(currentTask, USER1));
+        assertTrue(workflowService.isTaskReassignable(currentTask, USER2));
+        
+        // user3 should not be able to edit or reassign task
+        assertFalse(workflowService.isTaskEditable(currentTask, USER3));
+        assertFalse(workflowService.isTaskReassignable(currentTask, USER3));
+        
+        // cancel the workflow
+        workflowService.cancelWorkflow(workflowInstanceId);
+        assertNull(workflowService.getWorkflowById(workflowInstanceId));
+    }
+    
+    public void testPooledTaskCapabilities()
+    {
+        // make admin current user
+        AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
+        
+        // start pooled review and approve workflow
+        WorkflowDefinition workflowDef = this.workflowService.getDefinitionByName("jbpm$wf:reviewpooled");
+        assertNotNull(workflowDef);
+        
+        // Create workflow parameters
+        Map<QName, Serializable> params = new HashMap<QName, Serializable>();
+        Serializable wfPackage = workflowService.createPackage(null);
+        params.put(WorkflowModel.ASSOC_PACKAGE, wfPackage);
+        Date dueDate = new Date();
+        params.put(WorkflowModel.PROP_WORKFLOW_DUE_DATE, dueDate);
+        params.put(WorkflowModel.PROP_WORKFLOW_PRIORITY, 1);
+        NodeRef group = findGroup(GROUP);
+        assertNotNull(group);
+        params.put(WorkflowModel.ASSOC_GROUP_ASSIGNEE, group);
+        
+        // Start a workflow instance
+        WorkflowPath path = workflowService.startWorkflow(workflowDef.getId(), params);
+        assertNotNull(path);
+        assertTrue(path.isActive());
+        String workflowInstanceId = path.getInstance().getId();
+        
+        // End start task to progress workflow
+        List<WorkflowTask> tasks = workflowService.getTasksForWorkflowPath(path.getId());
+        assertEquals(1, tasks.size());
+        WorkflowTask startTask = tasks.get(0);
+        String startTaskId = startTask.getId();
+        workflowService.endTask(startTaskId, null);
+        
+        // Fetch the current task in the workflow
+        List<WorkflowPath> paths = workflowService.getWorkflowPaths(workflowInstanceId);
+        assertNotNull(paths);
+        assertEquals(1, paths.size());
+        tasks = workflowService.getTasksForWorkflowPath(path.getId());
+        assertEquals(1, tasks.size());
+        WorkflowTask currentTask = tasks.get(0);
+        assertEquals(currentTask.getState(), WorkflowTaskState.IN_PROGRESS);
+        assertNull(currentTask.getProperties().get(ContentModel.PROP_OWNER));
+        
+        // ensure the task is not reassignable by any user
+        assertFalse(workflowService.isTaskReassignable(currentTask, USER1));
+        assertFalse(workflowService.isTaskReassignable(currentTask, USER2));
+        assertFalse(workflowService.isTaskReassignable(currentTask, USER3));
+        
+        // ensure the task is not releasable by any user
+        assertFalse(workflowService.isTaskReleasable(currentTask, USER1));
+        assertFalse(workflowService.isTaskReleasable(currentTask, USER2));
+        assertFalse(workflowService.isTaskReleasable(currentTask, USER3));
+        
+        // ensure the task is claimable by the members of the group and sub group
+        assertTrue(workflowService.isTaskClaimable(currentTask, USER1));
+        assertTrue(workflowService.isTaskClaimable(currentTask, USER2));
+        
+        // ensure the task is not claimable by members outside of the group
+        assertFalse(workflowService.isTaskClaimable(currentTask, USER3));
+        
+        // ensure the task can be edited 
+        assertTrue(workflowService.isTaskEditable(currentTask, USER1));
+        assertTrue(workflowService.isTaskEditable(currentTask, USER2));
+        assertFalse(workflowService.isTaskEditable(currentTask, USER3));
+        
+        // claim the task for USER1
+        Map<QName, Serializable> properties = new HashMap<QName, Serializable>(8);
+        properties.put(ContentModel.PROP_OWNER, USER1);
+        workflowService.updateTask(currentTask.getId(), properties, null, null);
+        currentTask = workflowService.getTaskById(currentTask.getId());
+        
+        // check flags are correct now USER1 is the owner
+        assertFalse(workflowService.isTaskClaimable(currentTask, USER1));
+        assertTrue(workflowService.isTaskReleasable(currentTask, USER1));
+        assertTrue(workflowService.isTaskEditable(currentTask, USER1));
+        assertFalse(workflowService.isTaskReassignable(currentTask, USER1));
+        assertFalse(workflowService.isTaskClaimable(currentTask, USER2));
+        assertFalse(workflowService.isTaskEditable(currentTask, USER2));
+        
+        // cancel the workflow
+        workflowService.cancelWorkflow(workflowInstanceId);
+        assertNull(workflowService.getWorkflowById(workflowInstanceId));
+    }
 }