diff --git a/lib/taskana-core/src/main/java/org/taskana/WorkbasketService.java b/lib/taskana-core/src/main/java/org/taskana/WorkbasketService.java index 914d157be..d5f8f536a 100644 --- a/lib/taskana-core/src/main/java/org/taskana/WorkbasketService.java +++ b/lib/taskana-core/src/main/java/org/taskana/WorkbasketService.java @@ -92,7 +92,7 @@ public interface WorkbasketService { * if the workbasket do not exist * @throws NotAuthorizedException */ - public void checkPermission(String workbasketId, WorkbasketAuthorization authorization) + public void checkAuthorization(String workbasketId, WorkbasketAuthorization authorization) throws NotAuthorizedException; /** @@ -119,6 +119,6 @@ public interface WorkbasketService { * as String like in this enum: {@link WorkbasketAuthorization} * @return all filtered workbaskets */ - List getWorkbaskets(List permission); + List getWorkbaskets(List permission); } diff --git a/lib/taskana-core/src/main/java/org/taskana/impl/TaskServiceImpl.java b/lib/taskana-core/src/main/java/org/taskana/impl/TaskServiceImpl.java index cb40122b0..146ff9ddc 100644 --- a/lib/taskana-core/src/main/java/org/taskana/impl/TaskServiceImpl.java +++ b/lib/taskana-core/src/main/java/org/taskana/impl/TaskServiceImpl.java @@ -67,7 +67,7 @@ public class TaskServiceImpl implements TaskService { @Override public Task create(Task task) throws NotAuthorizedException { - taskanaEngine.getWorkbasketService().checkPermission(task.getWorkbasketId(), WorkbasketAuthorization.APPEND); + taskanaEngine.getWorkbasketService().checkAuthorization(task.getWorkbasketId(), WorkbasketAuthorization.APPEND); Timestamp now = new Timestamp(System.currentTimeMillis()); task.setId(UUID.randomUUID().toString()); @@ -93,7 +93,7 @@ public class TaskServiceImpl implements TaskService { @Override public List getTasksForWorkbasket(String workbasketId) throws NotAuthorizedException { - taskanaEngine.getWorkbasketService().checkPermission(workbasketId, WorkbasketAuthorization.OPEN); + taskanaEngine.getWorkbasketService().checkAuthorization(workbasketId, WorkbasketAuthorization.OPEN); return taskMapper.findByWorkBasketId(workbasketId); } @@ -108,7 +108,7 @@ public class TaskServiceImpl implements TaskService { throws NotAuthorizedException { for (String workbasket : workbasketIds) { - taskanaEngine.getWorkbasketService().checkPermission(workbasket, WorkbasketAuthorization.OPEN); + taskanaEngine.getWorkbasketService().checkAuthorization(workbasket, WorkbasketAuthorization.OPEN); } return taskMapper.findByWorkbasketIdsAndStates(workbasketIds, states); @@ -140,8 +140,8 @@ public class TaskServiceImpl implements TaskService { // transfer requires TRANSFER in source and APPEND on destination // workbasket - taskanaEngine.getWorkbasketService().checkPermission(destinationWorkbasketId, WorkbasketAuthorization.APPEND); - taskanaEngine.getWorkbasketService().checkPermission(task.getWorkbasketId(), WorkbasketAuthorization.TRANSFER); + taskanaEngine.getWorkbasketService().checkAuthorization(destinationWorkbasketId, WorkbasketAuthorization.APPEND); + taskanaEngine.getWorkbasketService().checkAuthorization(task.getWorkbasketId(), WorkbasketAuthorization.TRANSFER); // if security is disabled, the implicit existance check on the // destination workbasket has been skipped and needs to be performed diff --git a/lib/taskana-core/src/main/java/org/taskana/impl/WorkbasketServiceImpl.java b/lib/taskana-core/src/main/java/org/taskana/impl/WorkbasketServiceImpl.java index eb3968441..a23d6e06b 100644 --- a/lib/taskana-core/src/main/java/org/taskana/impl/WorkbasketServiceImpl.java +++ b/lib/taskana-core/src/main/java/org/taskana/impl/WorkbasketServiceImpl.java @@ -49,15 +49,13 @@ public class WorkbasketServiceImpl implements WorkbasketService { } @Override - public List getWorkbaskets(List permissions) { - List workbaskets = workbasketMapper.findByPermission(permissions, CurrentUserContext.getUserid()); - return workbaskets; + public List getWorkbaskets(List permissions) { + return workbasketMapper.findByPermission(permissions, CurrentUserContext.getUserid()); } @Override public List getWorkbaskets() { - List workbaskets = workbasketMapper.findAll(); - return workbaskets; + return workbasketMapper.findAll(); } @Override @@ -129,7 +127,7 @@ public class WorkbasketServiceImpl implements WorkbasketService { } @Override - public void checkPermission(String workbasketId, WorkbasketAuthorization workbasketAuthorization) + public void checkAuthorization(String workbasketId, WorkbasketAuthorization workbasketAuthorization) throws NotAuthorizedException { // Skip permission check is security is not enabled diff --git a/lib/taskana-core/src/main/java/org/taskana/model/mappings/WorkbasketMapper.java b/lib/taskana-core/src/main/java/org/taskana/model/mappings/WorkbasketMapper.java index 05651ca56..0943a969f 100644 --- a/lib/taskana-core/src/main/java/org/taskana/model/mappings/WorkbasketMapper.java +++ b/lib/taskana-core/src/main/java/org/taskana/model/mappings/WorkbasketMapper.java @@ -13,6 +13,7 @@ import org.apache.ibatis.annotations.Select; import org.apache.ibatis.annotations.Update; import org.apache.ibatis.mapping.FetchType; import org.taskana.model.Workbasket; +import org.taskana.model.WorkbasketAuthorization; public interface WorkbasketMapper { @@ -55,12 +56,12 @@ public interface WorkbasketMapper { @Select("") @Results(value = { @Result(property = "id", column = "ID"), @@ -71,7 +72,7 @@ public interface WorkbasketMapper { @Result(property = "description", column = "DESCRIPTION"), @Result(property = "owner", column = "OWNER"), @Result(property = "distributionTargets", column = "ID", javaType = List.class, many = @Many(fetchType = FetchType.DEFAULT, select="findByDistributionTargets")) }) - public List findByPermission(@Param("permissions") List permissions, @Param("userId") String userId); + public List findByPermission(@Param("authorizations") List authorizations, @Param("userId") String userId); @Insert("INSERT INTO WORKBASKET (ID, TENANT_ID, CREATED, MODIFIED, NAME, DESCRIPTION, OWNER) VALUES (#{workbasket.id}, #{workbasket.tenantId}, #{workbasket.created}, #{workbasket.modified}, #{workbasket.name}, #{workbasket.description}, #{workbasket.owner})") @Options(keyProperty = "id", keyColumn="ID") diff --git a/lib/taskana-core/src/test/java/org/taskana/impl/TaskServiceImplTest.java b/lib/taskana-core/src/test/java/org/taskana/impl/TaskServiceImplTest.java index 3f5e67929..b8caf1515 100644 --- a/lib/taskana-core/src/test/java/org/taskana/impl/TaskServiceImplTest.java +++ b/lib/taskana-core/src/test/java/org/taskana/impl/TaskServiceImplTest.java @@ -41,7 +41,7 @@ public class TaskServiceImplTest { @Test public void testCreateSimpleTask() throws NotAuthorizedException { registerBasicMocks(false); - Mockito.doNothing().when(workbasketServiceImpl).checkPermission(any(), any()); + Mockito.doNothing().when(workbasketServiceImpl).checkAuthorization(any(), any()); Mockito.doNothing().when(taskMapper).insert(any()); Task task = new Task(); @@ -124,7 +124,7 @@ public class TaskServiceImplTest { @Test(expected = WorkbasketNotFoundException.class) public void testTransferFailsIfDestinationWorkbasketDoesNotExist_withSecurityDisabled() throws TaskNotFoundException, WorkbasketNotFoundException, NotAuthorizedException { registerBasicMocks(false); - Mockito.doThrow(WorkbasketNotFoundException.class).when(workbasketServiceImpl).checkPermission(eq("invalidWorkbasketId"), any()); + Mockito.doThrow(WorkbasketNotFoundException.class).when(workbasketServiceImpl).checkAuthorization(eq("invalidWorkbasketId"), any()); Task task = createUnitTestTask("1", "Unit Test Task 1", "1"); @@ -135,7 +135,7 @@ public class TaskServiceImplTest { @Test(expected = WorkbasketNotFoundException.class) public void testTransferFailsIfDestinationWorkbasketDoesNotExist_withSecurityEnabled() throws TaskNotFoundException, WorkbasketNotFoundException, NotAuthorizedException { registerBasicMocks(true); - Mockito.doThrow(WorkbasketNotFoundException.class).when(workbasketServiceImpl).checkPermission(eq("invalidWorkbasketId"), any()); + Mockito.doThrow(WorkbasketNotFoundException.class).when(workbasketServiceImpl).checkAuthorization(eq("invalidWorkbasketId"), any()); Task task = createUnitTestTask("1", "Unit Test Task 1", "1"); diff --git a/lib/taskana-core/src/test/java/org/taskana/impl/WorkbasketServiceImplTest.java b/lib/taskana-core/src/test/java/org/taskana/impl/WorkbasketServiceImplTest.java index e1ba07514..6e2ca0549 100644 --- a/lib/taskana-core/src/test/java/org/taskana/impl/WorkbasketServiceImplTest.java +++ b/lib/taskana-core/src/test/java/org/taskana/impl/WorkbasketServiceImplTest.java @@ -1,7 +1,7 @@ package org.taskana.impl; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -15,10 +15,13 @@ import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import org.taskana.TaskanaEngine; +import org.taskana.configuration.TaskanaEngineConfiguration; import org.taskana.exceptions.NotAuthorizedException; import org.taskana.exceptions.WorkbasketNotFoundException; import org.taskana.model.Workbasket; import org.taskana.model.WorkbasketAccessItem; +import org.taskana.model.WorkbasketAuthorization; import org.taskana.model.mappings.DistributionTargetMapper; import org.taskana.model.mappings.WorkbasketAccessMapper; import org.taskana.model.mappings.WorkbasketMapper; @@ -35,85 +38,137 @@ public class WorkbasketServiceImplTest { DistributionTargetMapper distributionTargetMapper; @Mock WorkbasketAccessMapper workbasketAccessMapper; + @Mock + TaskanaEngine taskanaEngine; + @Mock + TaskanaEngineConfiguration taskanaEngineConfiguration; @Test - public void testInsertWorkbasket() throws NotAuthorizedException { + public void should_ReturnWorkbasket_when_WorkbasketIdExists() throws WorkbasketNotFoundException { + when(workbasketMapper.findById(any())).thenReturn(new Workbasket()); + + Workbasket workbasket = workbasketServiceImpl.getWorkbasket("fail"); + + verify(workbasketMapper).findById(any()); + Assert.assertNotNull(workbasket); + } + + @Test(expected = WorkbasketNotFoundException.class) + public void should_ThrowWorkbasketNotFoundException_when_WorkbasketIdDoesNotExist() + throws WorkbasketNotFoundException { + workbasketServiceImpl.getWorkbasket("fail"); + } + + @Test + public void should_ReturnListOfWorkbaskets_when_PermissionAndUserExists() { + when(workbasketMapper.findByPermission(any(), any())).thenReturn(new ArrayList()); + + List authorizations = new ArrayList<>(); + authorizations.add(WorkbasketAuthorization.OPEN); + authorizations.add(WorkbasketAuthorization.APPEND); + List workbaskets = workbasketServiceImpl.getWorkbaskets(authorizations); + + verify(workbasketMapper).findByPermission(any(), any()); + Assert.assertNotNull(workbaskets); + } + + @Test + public void should_ReturnAllWorkbaskets_when_AllWorkbaskets() { + when(workbasketMapper.findAll()).thenReturn(new ArrayList()); + + List workbaskets = workbasketServiceImpl.getWorkbaskets(); + + verify(workbasketMapper).findAll(); + Assert.assertNotNull(workbaskets); + } + + @Test + public void should_InitializeAndStoreWorkbasket_when_WorkbasketIsCreated() throws NotAuthorizedException { doNothing().when(workbasketMapper).insert(any()); + Workbasket workbasket = new Workbasket(); workbasket.setId("1"); workbasketServiceImpl.createWorkbasket(workbasket); Assert.assertEquals("1", workbasket.getId()); + Assert.assertEquals(workbasket.getModified(), workbasket.getCreated()); + + verify(workbasketMapper).insert(any()); } @Test - public void testSelectAllWorkbaskets() throws NotAuthorizedException { + public void should_InitializeAndStoreWorkbasket_when_WorkbasketWithDistributionTargetsIsCreated() throws NotAuthorizedException { + doNothing().when(workbasketMapper).insert(any()); + doNothing().when(distributionTargetMapper).insert(any(), any()); + + Workbasket workbasket = new Workbasket(); + workbasket.setId("1"); + Workbasket workbasket1 = new Workbasket(); + workbasket1.setId("2"); + Workbasket workbasket2 = new Workbasket(); + workbasket2.setId("3"); + workbasket.setDistributionTargets(new ArrayList() { + { + add(workbasket1); + add(workbasket2); + } + }); + + workbasketServiceImpl.createWorkbasket(workbasket); + + Assert.assertEquals("1", workbasket.getId()); + Assert.assertEquals(workbasket.getModified(), workbasket.getCreated()); + + verify(workbasketMapper, times(3)).insert(any()); + verify(distributionTargetMapper, times(2)).insert(any(), any()); + } + + @Test + public void should_ReturnUpdatedWorkbasket_when_ExistingWorkbasketDescriptionIsChanged() throws NotAuthorizedException { doNothing().when(workbasketMapper).insert(any()); - Workbasket workbasket0 = new Workbasket(); - workbasket0.setId("0"); - workbasketServiceImpl.createWorkbasket(workbasket0); - Workbasket workbasket1 = new Workbasket(); - workbasket1.setId("1"); - workbasketServiceImpl.createWorkbasket(workbasket1); - Workbasket workbasket2 = new Workbasket(); - workbasket2.setId("2"); - workbasketServiceImpl.createWorkbasket(workbasket2); + Workbasket workbasket = new Workbasket(); + workbasket.setId("0"); + workbasket.setDescription("TestDescription"); + workbasket.setName("Cool New WorkintheBasket"); + workbasket.setOwner("Arthur Dent"); + workbasketServiceImpl.createWorkbasket(workbasket); - verify(workbasketMapper, atLeast(3)).insert(any()); + doNothing().when(workbasketMapper).update(any()); + workbasket.setDescription("42"); + workbasketServiceImpl.updateWorkbasket(workbasket); + + verify(workbasketMapper).update(any()); } @Test - public void testSelectWorkbasket() throws WorkbasketNotFoundException, NotAuthorizedException { + public void should_ReturnUpdatedWorkbasket_when_ExistingWorkbasketDistributionTargetIsChanged() + throws NotAuthorizedException { doNothing().when(workbasketMapper).insert(any()); - Workbasket workbasket0 = new Workbasket(); - workbasket0.setId("0"); - workbasketServiceImpl.createWorkbasket(workbasket0); + Workbasket workbasket = new Workbasket(); + workbasket.setId("0"); Workbasket workbasket1 = new Workbasket(); workbasket1.setId("1"); - workbasketServiceImpl.createWorkbasket(workbasket1); - Workbasket workbasket2 = new Workbasket(); - workbasket2.setId("2"); - workbasketServiceImpl.createWorkbasket(workbasket2); + workbasket.setDistributionTargets(new ArrayList() { + { + add(workbasket1); + } + }); + workbasketServiceImpl.createWorkbasket(workbasket); - verify(workbasketMapper, atLeast(3)).insert(any()); + doNothing().when(workbasketMapper).update(any()); + when(workbasketMapper.findById(any())).thenReturn(workbasket); + workbasket.getDistributionTargets().get(0).setDescription("Test123"); + Workbasket result = workbasketServiceImpl.updateWorkbasket(workbasket); - when(workbasketMapper.findById(any())).thenReturn(workbasket2); - - Workbasket foundWorkbasket = workbasketServiceImpl.getWorkbasket("2"); - Assert.assertEquals("2", foundWorkbasket.getId()); - } - - @Test(expected = WorkbasketNotFoundException.class) - public void testGetWorkbasketFail() throws WorkbasketNotFoundException { - workbasketServiceImpl.getWorkbasket("fail"); + verify(workbasketMapper).update(any()); + Assert.assertEquals("Test123", result.getDistributionTargets().get(0).getDescription()); } @Test - public void testSelectWorkbasketWithDistribution() throws WorkbasketNotFoundException, NotAuthorizedException { - doNothing().when(workbasketMapper).insert(any()); - - Workbasket workbasket0 = new Workbasket(); - workbasket0.setId("0"); - Workbasket workbasket1 = new Workbasket(); - workbasket1.setId("1"); - Workbasket workbasket2 = new Workbasket(); - workbasket2.setId("2"); - workbasket2.setDistributionTargets(new ArrayList<>()); - workbasket2.getDistributionTargets().add(workbasket0); - workbasket2.getDistributionTargets().add(workbasket1); - workbasketServiceImpl.createWorkbasket(workbasket2); - - when(workbasketMapper.findById(any())).thenReturn(workbasket2); - - Workbasket foundWorkbasket = workbasketServiceImpl.getWorkbasket("2"); - Assert.assertEquals("2", foundWorkbasket.getId()); - Assert.assertEquals(2, foundWorkbasket.getDistributionTargets().size()); - } - - @Test - public void testUpdateWorkbasket() throws Exception { + public void should_UpdateModifiedTimestamp_when_ExistingWorkbasketDistributionTargetIsChanged() + throws Exception { doNothing().when(workbasketMapper).insert(any()); Workbasket workbasket0 = new Workbasket(); @@ -137,21 +192,25 @@ public class WorkbasketServiceImplTest { when(workbasketMapper.findById("2")).thenReturn(workbasket2); Workbasket foundBasket = workbasketServiceImpl.getWorkbasket(workbasket2.getId()); - + when(workbasketMapper.findById("1")).thenReturn(workbasket1); when(workbasketMapper.findById("3")).thenReturn(workbasket1); - + List distributionTargets = foundBasket.getDistributionTargets(); Assert.assertEquals(1, distributionTargets.size()); Assert.assertEquals("3", distributionTargets.get(0).getId()); - - Assert.assertNotEquals(workbasketServiceImpl.getWorkbasket("2").getCreated(), workbasketServiceImpl.getWorkbasket("2").getModified()); - Assert.assertEquals(workbasketServiceImpl.getWorkbasket("1").getCreated(), workbasketServiceImpl.getWorkbasket("1").getModified()); - Assert.assertEquals(workbasketServiceImpl.getWorkbasket("3").getCreated(), workbasketServiceImpl.getWorkbasket("3").getModified()); + + Assert.assertNotEquals(workbasketServiceImpl.getWorkbasket("2").getCreated(), + workbasketServiceImpl.getWorkbasket("2").getModified()); + Assert.assertEquals(workbasketServiceImpl.getWorkbasket("1").getCreated(), + workbasketServiceImpl.getWorkbasket("1").getModified()); + Assert.assertEquals(workbasketServiceImpl.getWorkbasket("3").getCreated(), + workbasketServiceImpl.getWorkbasket("3").getModified()); } @Test - public void testInsertWorkbasketAccessUser() throws NotAuthorizedException { + public void should_ReturnWorkbasketAuthorization_when_NewWorkbasketAccessItemIsCreated() + throws NotAuthorizedException { doNothing().when(workbasketAccessMapper).insert(any()); WorkbasketAccessItem accessItem = new WorkbasketAccessItem(); @@ -165,7 +224,8 @@ public class WorkbasketServiceImplTest { } @Test - public void testUpdateWorkbasketAccessUser() throws NotAuthorizedException { + public void should_ReturnWorkbasketAuthorization_when_WorkbasketAccessItemIsUpdated() + throws NotAuthorizedException { doNothing().when(workbasketAccessMapper).insert(any()); WorkbasketAccessItem accessItem = new WorkbasketAccessItem(); @@ -184,4 +244,37 @@ public class WorkbasketServiceImplTest { Assert.assertEquals("Zaphod Beeblebrox", accessItem.getUserId()); } + @Test(expected = NotAuthorizedException.class) + public void should_ThrowNotAuthorizedException_when_OperationIsNotAuthorized() throws NotAuthorizedException { + when(taskanaEngine.getConfiguration()).thenReturn(taskanaEngineConfiguration); + when(taskanaEngine.getConfiguration().isSecurityEnabled()).thenReturn(true); + + workbasketServiceImpl.checkAuthorization("1", WorkbasketAuthorization.READ); + } + + @Test + public void should_Pass_when_OperationIsAuthorized() throws NotAuthorizedException { + when(taskanaEngine.getConfiguration()).thenReturn(taskanaEngineConfiguration); + when(taskanaEngine.getConfiguration().isSecurityEnabled()).thenReturn(true); + + when(workbasketAccessMapper.findByWorkbasketAndUserAndAuthorization(any(), any(), any())) + .thenReturn(new ArrayList() { + { + add(new WorkbasketAccessItem()); + } + }); + + workbasketServiceImpl.checkAuthorization("1", WorkbasketAuthorization.READ); + + verify(workbasketAccessMapper, times(1)).findByWorkbasketAndUserAndAuthorization(any(), any(), any()); + } + + @Test + public void should_Pass_when_SecurityIsDisabled() throws NotAuthorizedException { + when(taskanaEngine.getConfiguration()).thenReturn(taskanaEngineConfiguration); + when(taskanaEngine.getConfiguration().isSecurityEnabled()).thenReturn(false); + + workbasketServiceImpl.checkAuthorization("1", WorkbasketAuthorization.READ); + } + } diff --git a/rest/src/main/java/org/taskana/rest/WorkbasketController.java b/rest/src/main/java/org/taskana/rest/WorkbasketController.java index 87ed910cf..40a1d3489 100644 --- a/rest/src/main/java/org/taskana/rest/WorkbasketController.java +++ b/rest/src/main/java/org/taskana/rest/WorkbasketController.java @@ -21,6 +21,7 @@ import org.taskana.exceptions.NotAuthorizedException; import org.taskana.exceptions.WorkbasketNotFoundException; import org.taskana.model.Workbasket; import org.taskana.model.WorkbasketAccessItem; +import org.taskana.model.WorkbasketAuthorization; @RestController @RequestMapping(path = "/v1/workbaskets", produces = { MediaType.APPLICATION_JSON_VALUE }) @@ -32,15 +33,33 @@ public class WorkbasketController { @GetMapping public List getWorkbaskets(@RequestParam MultiValueMap params) { if (params.containsKey("requiredPermission")) { - List permissions = new ArrayList<>(); + List authorizations = new ArrayList<>(); params.get("requiredPermission").stream().forEach(item -> { - permissions.addAll(Arrays.asList(item.split(","))); + for (String authorization : Arrays.asList(item.split(","))) { + switch (authorization) { + case "READ": + authorizations.add(WorkbasketAuthorization.READ); + break; + case "OPEN": + authorizations.add(WorkbasketAuthorization.OPEN); + break; + case "APPEND": + authorizations.add(WorkbasketAuthorization.APPEND); + break; + case "TRANSFER": + authorizations.add(WorkbasketAuthorization.TRANSFER); + break; + case "DISTRIBUTE": + authorizations.add(WorkbasketAuthorization.DISTRIBUTE); + break; + } + } }); - return workbasketService.getWorkbaskets(permissions); + return workbasketService.getWorkbaskets(authorizations); } else { return workbasketService.getWorkbaskets(); } - + } @RequestMapping(value = "/{workbasketid}")