From 7e774609a86945f3c6b68c617201e05b624bfd69 Mon Sep 17 00:00:00 2001 From: Gavin Cornwell Date: Fri, 8 Oct 2010 12:46:10 +0000 Subject: [PATCH] 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 --- .../alfresco/repo/workflow/WorkflowModel.java | 2 + .../repo/workflow/WorkflowServiceImpl.java | 17 +- .../workflow/WorkflowServiceImplTest.java | 295 +++++++++++++++++- 3 files changed, 305 insertions(+), 9 deletions(-) diff --git a/source/java/org/alfresco/repo/workflow/WorkflowModel.java b/source/java/org/alfresco/repo/workflow/WorkflowModel.java index e7895ccea2..7ef6099b83 100644 --- a/source/java/org/alfresco/repo/workflow/WorkflowModel.java +++ b/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"); diff --git a/source/java/org/alfresco/repo/workflow/WorkflowServiceImpl.java b/source/java/org/alfresco/repo/workflow/WorkflowServiceImpl.java index f0676b1dc9..1cdd594ec9 100644 --- a/source/java/org/alfresco/repo/workflow/WorkflowServiceImpl.java +++ b/source/java/org/alfresco/repo/workflow/WorkflowServiceImpl.java @@ -665,9 +665,18 @@ public class WorkflowServiceImpl implements WorkflowService return true; } - // if the current 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); + 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; + } } /* @@ -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 users = this.authorityService.getContainedAuthorities(AuthorityType.USER, name, true); + Set 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)) diff --git a/source/java/org/alfresco/repo/workflow/WorkflowServiceImplTest.java b/source/java/org/alfresco/repo/workflow/WorkflowServiceImplTest.java index e62fe4eeb9..0d37e7a580 100644 --- a/source/java/org/alfresco/repo/workflow/WorkflowServiceImplTest.java +++ b/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; - NodeService nodeService; + private final static String USER1 = "WFUser1" + GUID.generate(); + 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"); - auth.setSystemUserAsCurrentUser(); + authenticationComponent = (AuthenticationComponent) applicationContext.getBean("authenticationComponent"); + 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 params = new HashMap(); + 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 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 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 params = new HashMap(); + 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 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 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 properties = new HashMap(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)); + } }