From 88e24ed4a0624c74c225972d5da7929fe696537b Mon Sep 17 00:00:00 2001 From: Marcel Lengl <52546181+LenglBoy@users.noreply.github.com> Date: Tue, 27 Feb 2018 15:05:25 +0100 Subject: [PATCH] TSK-89: Throw WorkbasketNotFoundException on checkAuthorization --- .../src/main/java/pro/taskana/TaskService.java | 4 ++-- .../java/pro/taskana/WorkbasketService.java | 12 +++++++++--- .../java/pro/taskana/impl/TaskQueryImpl.java | 18 ++++++++++++++---- .../taskana/impl/WorkbasketServiceImpl.java | 12 +++++++++--- .../acceptance/task/TransferTaskAccTest.java | 6 +++--- .../pro/taskana/impl/TaskServiceImplTest.java | 2 +- .../impl/WorkbasketServiceImplTest.java | 4 ++-- .../resources/sql/workbasket-access-list.sql | 2 +- 8 files changed, 41 insertions(+), 19 deletions(-) diff --git a/lib/taskana-core/src/main/java/pro/taskana/TaskService.java b/lib/taskana-core/src/main/java/pro/taskana/TaskService.java index 34d2f6597..aaa8fa137 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/TaskService.java +++ b/lib/taskana-core/src/main/java/pro/taskana/TaskService.java @@ -163,7 +163,7 @@ public interface TaskService { * * @param taskId * The id of the {@link Task} to be transferred - * @param workbasketId + * @param destinationWorkbasketId * The Id of the target work basket * @return the transferred task * @throws TaskNotFoundException @@ -175,7 +175,7 @@ public interface TaskService { * @throws InvalidWorkbasketException * Thrown if either the source or the target workbasket has a missing required property */ - Task transfer(String taskId, String workbasketId) + Task transfer(String taskId, String destinationWorkbasketId) throws TaskNotFoundException, WorkbasketNotFoundException, NotAuthorizedException, InvalidWorkbasketException; /** diff --git a/lib/taskana-core/src/main/java/pro/taskana/WorkbasketService.java b/lib/taskana-core/src/main/java/pro/taskana/WorkbasketService.java index 35c409221..d5824c8db 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/WorkbasketService.java +++ b/lib/taskana-core/src/main/java/pro/taskana/WorkbasketService.java @@ -132,9 +132,11 @@ public interface WorkbasketService { * the needed Authorization * @throws NotAuthorizedException * if the current user has not the requested authorization for the specified workbasket + * @throws WorkbasketNotFoundException + * if the workbasket can´t be found foor the given ID. */ void checkAuthorization(String workbasketId, WorkbasketAuthorization authorization) - throws NotAuthorizedException; + throws NotAuthorizedException, WorkbasketNotFoundException; /** * This method checks the authorization with the saved one for the actual User. @@ -147,9 +149,11 @@ public interface WorkbasketService { * the needed Authorization * @throws NotAuthorizedException * if the current user has not the requested authorization for the specified workbasket + * @throws WorkbasketNotFoundException + * if no workbasket can be found for the given key+domain values. */ void checkAuthorization(String workbasketKey, String domain, WorkbasketAuthorization authorization) - throws NotAuthorizedException; + throws NotAuthorizedException, WorkbasketNotFoundException; /** * Get all authorizations for a Workbasket. @@ -291,9 +295,11 @@ public interface WorkbasketService { * The id of the target workbasket * @throws NotAuthorizedException * If the current user doesn't have READ permission for the source workbasket + * @throws WorkbasketNotFoundException + * if the source workbasket can´t be found by ID. */ void removeDistributionTarget(String sourceWorkbasketId, String targetWorkbasketId) - throws NotAuthorizedException; + throws NotAuthorizedException, WorkbasketNotFoundException; /** * Deletes the workbasket by the given ID of it. diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskQueryImpl.java index 68be52ae1..1b0826162 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskQueryImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskQueryImpl.java @@ -18,6 +18,7 @@ import pro.taskana.TimeInterval; import pro.taskana.exceptions.NotAuthorizedException; import pro.taskana.exceptions.NotAuthorizedToQueryWorkbasketException; import pro.taskana.exceptions.TaskanaRuntimeException; +import pro.taskana.exceptions.WorkbasketNotFoundException; import pro.taskana.impl.util.LoggerUtils; /** @@ -760,14 +761,23 @@ public class TaskQueryImpl implements TaskQuery { try { if (this.workbasketIdIn != null && this.workbasketIdIn.length > 0) { for (String workbasketId : workbasketIdIn) { - taskanaEngine.getWorkbasketService().checkAuthorization(workbasketId, - WorkbasketAuthorization.OPEN); + try { + taskanaEngine.getWorkbasketService().checkAuthorization(workbasketId, + WorkbasketAuthorization.OPEN); + } catch (WorkbasketNotFoundException e) { + LOGGER.warn("The workbasket with the ID '" + workbasketId + "' does not exist.", e); + } } } if (workbasketKeyDomainIn != null && workbasketKeyDomainIn.length > 0) { for (KeyDomain keyDomain : workbasketKeyDomainIn) { - taskanaEngine.getWorkbasketService().checkAuthorization(keyDomain.getKey(), - keyDomain.getDomain(), WorkbasketAuthorization.OPEN); + try { + taskanaEngine.getWorkbasketService().checkAuthorization(keyDomain.getKey(), + keyDomain.getDomain(), WorkbasketAuthorization.OPEN); + } catch (WorkbasketNotFoundException e) { + LOGGER.warn("The workbasket with the KEY '" + keyDomain.getKey() + "' and DOMAIN '" + + keyDomain.getDomain() + "'does not exist.", e); + } } } } catch (NotAuthorizedException e) { diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java index faaae6dd4..bb09afe34 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java @@ -273,14 +273,20 @@ public class WorkbasketServiceImpl implements WorkbasketService { @Override public void checkAuthorization(String workbasketId, - WorkbasketAuthorization workbasketAuthorization) throws NotAuthorizedException { + WorkbasketAuthorization workbasketAuthorization) throws NotAuthorizedException, WorkbasketNotFoundException { + if (workbasketMapper.findById(workbasketId) == null) { + throw new WorkbasketNotFoundException(workbasketId); + } checkAuthorization(null, null, workbasketId, workbasketAuthorization); } @Override public void checkAuthorization(String workbasketKey, String domain, WorkbasketAuthorization workbasketAuthorization) - throws NotAuthorizedException { + throws NotAuthorizedException, WorkbasketNotFoundException { + if (workbasketMapper.findByKeyAndDomain(workbasketKey, domain) == null) { + throw new WorkbasketNotFoundException(workbasketKey + " - " + domain); + } checkAuthorization(workbasketKey, domain, null, workbasketAuthorization); } @@ -598,7 +604,7 @@ public class WorkbasketServiceImpl implements WorkbasketService { @Override public void removeDistributionTarget(String sourceWorkbasketId, String targetWorkbasketId) - throws NotAuthorizedException { + throws NotAuthorizedException, WorkbasketNotFoundException { LOGGER.debug("entry to removeDistributionTarget(sourceWorkbasketId = {}, targetWorkbasketId = {})", sourceWorkbasketId, targetWorkbasketId); taskanaEngine.checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); diff --git a/lib/taskana-core/src/test/java/acceptance/task/TransferTaskAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/TransferTaskAccTest.java index 4bcf78a4c..f7e806550 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/TransferTaskAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/TransferTaskAccTest.java @@ -118,11 +118,11 @@ public class TransferTaskAccTest extends AbstractAccTest { TaskService taskService = taskanaEngine.getTaskService(); Task task = taskService.getTask("TKI:000000000000000000000000000000000001"); - taskService.transfer(task.getId(), "TEAMLEAD_2"); + taskService.transfer(task.getId(), "WBI:100000000000000000000000000000000005"); } @WithAccessId( - userName = "USER_1_1", + userName = "user_1_1", groupNames = {"group_1"}) @Test(expected = NotAuthorizedException.class) public void testThrowsExceptionIfTransferWithNoAppendAuthorization() @@ -131,7 +131,7 @@ public class TransferTaskAccTest extends AbstractAccTest { TaskService taskService = taskanaEngine.getTaskService(); Task task = taskService.getTask("TKI:000000000000000000000000000000000002"); - taskService.transfer(task.getId(), "USER_1_1"); + taskService.transfer(task.getId(), "WBI:100000000000000000000000000000000008"); } @WithAccessId( diff --git a/lib/taskana-core/src/test/java/pro/taskana/impl/TaskServiceImplTest.java b/lib/taskana-core/src/test/java/pro/taskana/impl/TaskServiceImplTest.java index 9f9ae0028..67b4d4f28 100644 --- a/lib/taskana-core/src/test/java/pro/taskana/impl/TaskServiceImplTest.java +++ b/lib/taskana-core/src/test/java/pro/taskana/impl/TaskServiceImplTest.java @@ -104,7 +104,7 @@ public class TaskServiceImplTest { private SqlSession sqlSessionMock; @Before - public void setup() { + public void setup() throws WorkbasketNotFoundException { MockitoAnnotations.initMocks(this); doReturn(workbasketServiceMock).when(taskanaEngineMock).getWorkbasketService(); doReturn(classificationServiceImplMock).when(taskanaEngineMock).getClassificationService(); diff --git a/lib/taskana-core/src/test/java/pro/taskana/impl/WorkbasketServiceImplTest.java b/lib/taskana-core/src/test/java/pro/taskana/impl/WorkbasketServiceImplTest.java index 36be5583e..3b7d955da 100644 --- a/lib/taskana-core/src/test/java/pro/taskana/impl/WorkbasketServiceImplTest.java +++ b/lib/taskana-core/src/test/java/pro/taskana/impl/WorkbasketServiceImplTest.java @@ -333,7 +333,7 @@ public class WorkbasketServiceImplTest { verify(taskanaEngineImplMock, times(2)).getConfiguration(); verify(taskanaEngineConfigurationMock, times(2)).isSecurityEnabled(); verify(workbasketMapperMock, times(1)).insert(expectedWb); - verify(workbasketMapperMock, times(2)).findById(expectedWb.getId()); + verify(workbasketMapperMock, times(4)).findById(expectedWb.getId()); verify(workbasketMapperMock, times(1)).update(any()); verify(taskanaEngineImplMock, times(5)).returnConnection(); verify(taskanaEngineImplMock, times(2)).checkRoleMembership(any()); @@ -369,7 +369,7 @@ public class WorkbasketServiceImplTest { verify(cutSpy, times(distTargetAmount + 1)).getWorkbasket(any()); verify(distributionTargetMapperMock, times(1)).deleteAllDistributionTargetsBySourceId(any()); verify(distributionTargetMapperMock, times(distTargetAmount)).insert(any(), any()); - verify(workbasketMapperMock, times(3)).findById(any()); + verify(workbasketMapperMock, times(4)).findById(any()); verify(workbasketMapperMock, times(1)).update(any()); verify(taskanaEngineImplMock, times(5)).returnConnection(); verify(taskanaEngineImplMock, times(4)).checkRoleMembership(any()); diff --git a/lib/taskana-core/src/test/resources/sql/workbasket-access-list.sql b/lib/taskana-core/src/test/resources/sql/workbasket-access-list.sql index 5a7b9be96..adbc22ab1 100644 --- a/lib/taskana-core/src/test/resources/sql/workbasket-access-list.sql +++ b/lib/taskana-core/src/test/resources/sql/workbasket-access-list.sql @@ -19,7 +19,7 @@ INSERT INTO WORKBASKET_ACCESS_LIST VALUES ('WAI:10000000000000000000000000000000 -- cross team tranfers INSERT INTO WORKBASKET_ACCESS_LIST VALUES ('WAI:100000000000000000000000000000000015', 'WBI:100000000000000000000000000000000006', 'group_2', true, false, true, false, false, false, false, false, false, false, false, false, false, false, false, false, false); INSERT INTO WORKBASKET_ACCESS_LIST VALUES ('WAI:100000000000000000000000000000000016', 'WBI:100000000000000000000000000000000007', 'group_2', true, false, true, false, false, false, false, false, false, false, false, false, false, false, false, false, false); -INSERT INTO WORKBASKET_ACCESS_LIST VALUES ('WAI:100000000000000000000000000000000017', 'WBI:100000000000000000000000000000000008', 'group_1', true, false, true, false, false, false, false, false, false, false, false, false, false, false, false, false, false); +INSERT INTO WORKBASKET_ACCESS_LIST VALUES ('WAI:100000000000000000000000000000000017', 'WBI:100000000000000000000000000000000008', 'group_1', true, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false); INSERT INTO WORKBASKET_ACCESS_LIST VALUES ('WAI:100000000000000000000000000000000018', 'WBI:100000000000000000000000000000000009', 'group_1', true, false, true, false, false, false, false, false, false, false, false, false, false, false, false, false, false); -- Team GPK access INSERT INTO WORKBASKET_ACCESS_LIST VALUES ('WAI:100000000000000000000000000000000019', 'WBI:100000000000000000000000000000000002', 'group_1', true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true);