From d4ab03667c5e49ab46175874111e913a2974aeec Mon Sep 17 00:00:00 2001 From: BerndBreier <33351391+BerndBreier@users.noreply.github.com> Date: Wed, 7 Mar 2018 15:40:43 +0100 Subject: [PATCH] TSK-348 Consider admin rights in workbasket query and hide tasks from workbaskets without read permission --- .../java/pro/taskana/TaskanaRestTest.java | 3 +- .../main/java/pro/taskana/TaskService.java | 33 +++-- .../java/pro/taskana/WorkbasketQuery.java | 9 ++ .../taskana/impl/ClassificationQueryImpl.java | 35 +++--- .../main/java/pro/taskana/impl/TaskImpl.java | 4 + .../java/pro/taskana/impl/TaskQueryImpl.java | 52 +++++--- .../pro/taskana/impl/TaskServiceImpl.java | 50 ++++++-- .../pro/taskana/impl/WorkbasketQueryImpl.java | 106 +++++++++++----- .../pro/taskana/mappings/QueryMapper.java | 5 + .../java/pro/taskana/mappings/TaskMapper.java | 10 +- .../security/ClassificationQueryAccTest.java | 71 +++++++++++ .../acceptance/security/TaskQueryAccTest.java | 80 ++++++++++++ .../security/WorkbasketQueryAccTest.java | 114 ++++++++++++++++++ .../acceptance/task/DeleteTaskAccTest.java | 5 +- .../task/UpdateTaskAttachmentsAccTest.java | 71 ++++++++--- .../workbasket/QueryWorkbasketAccTest.java | 33 +++++ .../pro/taskana/impl/TaskServiceImplTest.java | 59 ++++++--- .../java/pro/taskana/rest/TaskController.java | 12 ++ 18 files changed, 629 insertions(+), 123 deletions(-) create mode 100644 lib/taskana-core/src/test/java/acceptance/security/ClassificationQueryAccTest.java create mode 100644 lib/taskana-core/src/test/java/acceptance/security/TaskQueryAccTest.java create mode 100644 lib/taskana-core/src/test/java/acceptance/security/WorkbasketQueryAccTest.java diff --git a/lib/taskana-cdi/src/test/java/pro/taskana/TaskanaRestTest.java b/lib/taskana-cdi/src/test/java/pro/taskana/TaskanaRestTest.java index 51f7f4e37..b06e30f0b 100644 --- a/lib/taskana-cdi/src/test/java/pro/taskana/TaskanaRestTest.java +++ b/lib/taskana-cdi/src/test/java/pro/taskana/TaskanaRestTest.java @@ -73,7 +73,8 @@ public class TaskanaRestTest { @DELETE @Path("{id}") public void completeTask(@PathParam("id") String id) - throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, ClassificationNotFoundException { + throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, ClassificationNotFoundException, + NotAuthorizedException { logger.info(id); taskanaEjb.getTaskService().completeTask(id, true); } 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 aaa8fa137..c7880ffe1 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/TaskService.java +++ b/lib/taskana-core/src/main/java/pro/taskana/TaskService.java @@ -33,9 +33,11 @@ public interface TaskService { * if the state of the task with taskId is not READY * @throws InvalidOwnerException * if the task with taskId is claimed by some else + * @throws NotAuthorizedException + * if the current user has no read permission for the workbasket the task is in */ Task claim(String taskId) - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException; + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException; /** * Claim an existing task for the current user. Enable forced claim. @@ -51,9 +53,11 @@ public interface TaskService { * if the state of the task with taskId is not READY * @throws InvalidOwnerException * if the task with taskId is claimed by someone else + * @throws NotAuthorizedException + * if the current user has no read permission for the workbasket the task is in */ Task claim(String taskId, boolean forceClaim) - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException; + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException; /** * Unclaim a existing Task which was claimed and owned by you before. @@ -67,8 +71,11 @@ public interface TaskService { * when the task is already completed. * @throws InvalidOwnerException * when the task is claimed by another user. + * @throws NotAuthorizedException + * if the current user has no read permission for the workbasket the task is in */ - Task cancelClaim(String taskId) throws TaskNotFoundException, InvalidStateException, InvalidOwnerException; + Task cancelClaim(String taskId) + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException; /** * Unclaim a existing Task which was claimed and owned by you before. Also there can be enabled a force flag for @@ -86,9 +93,11 @@ public interface TaskService { * when the task is already completed. * @throws InvalidOwnerException * when forceCancel is false and the task is claimed by another user. + * @throws NotAuthorizedException + * if the current user has no read permission for the workbasket the task is in */ Task cancelClaim(String taskId, boolean forceCancel) - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException; + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException; /** * Complete a claimed Task as owner/admin and update State and Timestamps. @@ -102,9 +111,11 @@ public interface TaskService { * if the given Task can´t be found in DB. * @throws InvalidOwnerException * if current user is not the task-owner or administrator. + * @throws NotAuthorizedException + * if the current user has no read permission for the workbasket the task is in */ Task completeTask(String taskId) - throws TaskNotFoundException, InvalidOwnerException, InvalidStateException; + throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, NotAuthorizedException; /** * Complete a claimed Task and update State and Timestamps. @@ -120,9 +131,11 @@ public interface TaskService { * if the given Task can´t be found in DB. * @throws InvalidOwnerException * if current user is not the task-owner or administrator. + * @throws NotAuthorizedException + * if the current user has no read permission for the workbasket the task is in */ Task completeTask(String taskId, boolean isForced) - throws TaskNotFoundException, InvalidOwnerException, InvalidStateException; + throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, NotAuthorizedException; /** * Persists a not persisted Task which does not exist already. @@ -155,8 +168,10 @@ public interface TaskService { * @return the Task * @throws TaskNotFoundException * thrown of the {@link Task} with taskId is not found + * @throws NotAuthorizedException + * if the current user has no READ permission for the workbasket the task is in. */ - Task getTask(String taskId) throws TaskNotFoundException; + Task getTask(String taskId) throws TaskNotFoundException, NotAuthorizedException; /** * Transfer a task to another work basket. The transfer sets the transferred flag and resets the read flag. @@ -210,8 +225,10 @@ public interface TaskService { * @return the updated Task * @throws TaskNotFoundException * Thrown if the {@link Task} with taskId was not found + * @throws NotAuthorizedException + * if the current user has no read permission for the workbasket the task is in */ - Task setTaskRead(String taskId, boolean isRead) throws TaskNotFoundException; + Task setTaskRead(String taskId, boolean isRead) throws TaskNotFoundException, NotAuthorizedException; /** * This method provides a query builder for quering the database. diff --git a/lib/taskana-core/src/main/java/pro/taskana/WorkbasketQuery.java b/lib/taskana-core/src/main/java/pro/taskana/WorkbasketQuery.java index ab5274bc0..aebba959b 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/WorkbasketQuery.java +++ b/lib/taskana-core/src/main/java/pro/taskana/WorkbasketQuery.java @@ -8,6 +8,15 @@ import pro.taskana.exceptions.NotAuthorizedException; */ public interface WorkbasketQuery extends BaseQuery { + /** + * Add your ids to your query. The ids are compared to the ids of workbaskets with the IN operator. + * + * @param id + * the id as Strings + * @return the query + */ + WorkbasketQuery idIn(String... id); + /** * Add your keys to your query. The keys are compared case-insensitively to the keys of workbaskets with the IN * operator. diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationQueryImpl.java index ee9fd3779..911bb8c87 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationQueryImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationQueryImpl.java @@ -432,6 +432,20 @@ public class ClassificationQueryImpl implements ClassificationQuery { } } + @Override + public long count() { + LOGGER.debug("entry to count(), this = {}", this); + Long rowCount = null; + try { + taskanaEngine.openConnection(); + rowCount = taskanaEngine.getSqlSession().selectOne(LINK_TO_COUNTER, this); + return (rowCount == null) ? 0L : rowCount; + } finally { + taskanaEngine.returnConnection(); + LOGGER.debug("exit from count(). Returning result {} ", rowCount); + } + } + private ClassificationQuery addOrderCriteria(String columnName, SortDirection sortDirection) { String orderByDirection = " ASC"; if (sortDirection != null && SortDirection.DESCENDING.equals(sortDirection)) { @@ -573,24 +587,12 @@ public class ClassificationQueryImpl implements ClassificationQuery { return columnName; } - @Override - public long count() { - LOGGER.debug("entry to count(), this = {}", this); - Long rowCount = null; - try { - taskanaEngine.openConnection(); - rowCount = taskanaEngine.getSqlSession().selectOne(LINK_TO_COUNTER, this); - return (rowCount == null) ? 0L : rowCount; - } finally { - taskanaEngine.returnConnection(); - LOGGER.debug("exit from count(). Returning result {} ", rowCount); - } - } - @Override public String toString() { StringBuilder builder = new StringBuilder(); - builder.append("ClassificationQueryImpl [key="); + builder.append("ClassificationQueryImpl [columnName="); + builder.append(columnName); + builder.append(", key="); builder.append(Arrays.toString(key)); builder.append(", parentId="); builder.append(Arrays.toString(parentId)); @@ -654,7 +656,10 @@ public class ClassificationQueryImpl implements ClassificationQuery { builder.append(Arrays.toString(custom8In)); builder.append(", custom8Like="); builder.append(Arrays.toString(custom8Like)); + builder.append(", orderBy="); + builder.append(orderBy); builder.append("]"); return builder.toString(); } + } diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskImpl.java index 1bbf37f40..06067d295 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskImpl.java @@ -496,6 +496,10 @@ public class TaskImpl implements Task { return (WorkbasketSummaryImpl) workbasketSummary; } + public void setWorkbasketSummaryImpl(WorkbasketSummaryImpl workbasketSummary) { + this.workbasketSummary = workbasketSummary; + } + public void setClassificationSummaryImpl(ClassificationSummaryImpl classificationSummary) { this.classificationSummary = classificationSummary; } 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 b999aa40c..01499b49e 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 @@ -32,6 +32,8 @@ public class TaskQueryImpl implements TaskQuery { private static final String LINK_TO_MAPPER = "pro.taskana.mappings.QueryMapper.queryTaskSummaries"; private static final String LINK_TO_COUNTER = "pro.taskana.mappings.QueryMapper.countQueryTasks"; private static final String LINK_TO_VALUEMAPPER = "pro.taskana.mappings.QueryMapper.queryTaskColumnValues"; + private static final String TIME_INTERVAL = "TimeInterval "; + private static final String IS_INVALID = " is invalid."; private static final Logger LOGGER = LoggerFactory.getLogger(TaskQueryImpl.class); private TaskanaEngineImpl taskanaEngine; private TaskServiceImpl taskService; @@ -139,7 +141,7 @@ public class TaskQueryImpl implements TaskQuery { this.createdIn = intervals; for (TimeInterval ti : intervals) { if (!ti.isValid()) { - throw new IllegalArgumentException("TimeInterval " + ti + " is invalid."); + throw new IllegalArgumentException(TIME_INTERVAL + ti + IS_INVALID); } } return this; @@ -150,7 +152,7 @@ public class TaskQueryImpl implements TaskQuery { this.claimedIn = intervals; for (TimeInterval ti : intervals) { if (!ti.isValid()) { - throw new IllegalArgumentException("TimeInterval " + ti + " is invalid."); + throw new IllegalArgumentException(TIME_INTERVAL + ti + IS_INVALID); } } return this; @@ -161,7 +163,7 @@ public class TaskQueryImpl implements TaskQuery { this.completedIn = intervals; for (TimeInterval ti : intervals) { if (!ti.isValid()) { - throw new IllegalArgumentException("TimeInterval " + ti + " is invalid."); + throw new IllegalArgumentException(TIME_INTERVAL + ti + IS_INVALID); } } return this; @@ -172,7 +174,7 @@ public class TaskQueryImpl implements TaskQuery { this.modifiedIn = intervals; for (TimeInterval ti : intervals) { if (!ti.isValid()) { - throw new IllegalArgumentException("TimeInterval " + ti + " is invalid."); + throw new IllegalArgumentException(TIME_INTERVAL + ti + IS_INVALID); } } return this; @@ -183,7 +185,7 @@ public class TaskQueryImpl implements TaskQuery { this.plannedIn = intervals; for (TimeInterval ti : intervals) { if (!ti.isValid()) { - throw new IllegalArgumentException("TimeInterval " + ti + " is invalid."); + throw new IllegalArgumentException(TIME_INTERVAL + ti + IS_INVALID); } } return this; @@ -194,7 +196,7 @@ public class TaskQueryImpl implements TaskQuery { this.dueIn = intervals; for (TimeInterval ti : intervals) { if (!ti.isValid()) { - throw new IllegalArgumentException("TimeInterval " + ti + " is invalid."); + throw new IllegalArgumentException(TIME_INTERVAL + ti + IS_INVALID); } } return this; @@ -673,6 +675,10 @@ public class TaskQueryImpl implements TaskQuery { checkOpenPermissionForSpecifiedWorkbaskets(); List tasks = new ArrayList<>(); tasks = taskanaEngine.getSqlSession().selectList(LINK_TO_MAPPER, this); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("mapper returned {} resulting Objects: {} ", tasks.size(), + LoggerUtils.listToString(tasks)); + } result = taskService.augmentTaskSummariesByContainedSummaries(tasks); return result; } catch (NotAuthorizedException e) { @@ -787,23 +793,12 @@ public class TaskQueryImpl implements TaskQuery { try { if (this.workbasketIdIn != null && this.workbasketIdIn.length > 0) { for (String workbasketId : workbasketIdIn) { - try { - taskanaEngine.getWorkbasketService().checkAuthorization(workbasketId, - WorkbasketPermission.OPEN); - } catch (WorkbasketNotFoundException e) { - LOGGER.warn("The workbasket with the ID '" + workbasketId + "' does not exist.", e); - } + checkOpenPermissionById(workbasketId); } } if (workbasketKeyDomainIn != null && workbasketKeyDomainIn.length > 0) { for (KeyDomain keyDomain : workbasketKeyDomainIn) { - try { - taskanaEngine.getWorkbasketService().checkAuthorization(keyDomain.getKey(), - keyDomain.getDomain(), WorkbasketPermission.OPEN); - } catch (WorkbasketNotFoundException e) { - LOGGER.warn("The workbasket with the KEY '" + keyDomain.getKey() + "' and DOMAIN '" - + keyDomain.getDomain() + "'does not exist.", e); - } + checkOpenPermissionByKeyDomain(keyDomain); } } } catch (NotAuthorizedException e) { @@ -811,6 +806,25 @@ public class TaskQueryImpl implements TaskQuery { } } + private void checkOpenPermissionById(String workbasketId) throws NotAuthorizedException { + try { + taskanaEngine.getWorkbasketService().checkAuthorization(workbasketId, + WorkbasketPermission.OPEN); + } catch (WorkbasketNotFoundException e) { + LOGGER.warn("The workbasket with the ID '" + workbasketId + "' does not exist.", e); + } + } + + private void checkOpenPermissionByKeyDomain(KeyDomain keyDomain) throws NotAuthorizedException { + try { + taskanaEngine.getWorkbasketService().checkAuthorization(keyDomain.getKey(), + keyDomain.getDomain(), WorkbasketPermission.OPEN); + } catch (WorkbasketNotFoundException e) { + LOGGER.warn("The workbasket with the KEY '" + keyDomain.getKey() + "' and DOMAIN '" + + keyDomain.getDomain() + "'does not exist.", e); + } + } + public TaskanaEngineImpl getTaskanaEngine() { return taskanaEngine; } diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java index 64680d32c..ca02fac7a 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java @@ -76,13 +76,13 @@ public class TaskServiceImpl implements TaskService { @Override public Task claim(String taskId) - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException { return claim(taskId, false); } @Override public Task claim(String taskId, boolean forceClaim) - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException { String userId = CurrentUserContext.getUserid(); LOGGER.debug("entry to claim(id = {}, forceClaim = {}, userId = {})", taskId, forceClaim, userId); TaskImpl task = null; @@ -118,13 +118,14 @@ public class TaskServiceImpl implements TaskService { } @Override - public Task cancelClaim(String taskId) throws TaskNotFoundException, InvalidStateException, InvalidOwnerException { + public Task cancelClaim(String taskId) + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException { return this.cancelClaim(taskId, false); } @Override public Task cancelClaim(String taskId, boolean forceUnclaim) - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException { String userId = CurrentUserContext.getUserid(); LOGGER.debug("entry to cancelClaim(taskId = {}) with userId = {}, forceFlag = {}", taskId, userId, forceUnclaim); @@ -163,13 +164,13 @@ public class TaskServiceImpl implements TaskService { @Override public Task completeTask(String taskId) - throws TaskNotFoundException, InvalidOwnerException, InvalidStateException { + throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, NotAuthorizedException { return completeTask(taskId, false); } @Override public Task completeTask(String taskId, boolean isForced) - throws TaskNotFoundException, InvalidOwnerException, InvalidStateException { + throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, NotAuthorizedException { LOGGER.debug("entry to completeTask(id = {}, isForced {})", taskId, isForced); TaskImpl task = null; try { @@ -325,7 +326,7 @@ public class TaskServiceImpl implements TaskService { } @Override - public Task getTask(String id) throws TaskNotFoundException { + public Task getTask(String id) throws TaskNotFoundException, NotAuthorizedException { LOGGER.debug("entry to getTaskById(id = {})", id); TaskImpl resultTask = null; try { @@ -333,6 +334,22 @@ public class TaskServiceImpl implements TaskService { resultTask = taskMapper.findById(id); if (resultTask != null) { + WorkbasketQueryImpl query = (WorkbasketQueryImpl) workbasketService.createWorkbasketQuery(); + query.setUsedToAugmentTasks(true); + String workbasketId = resultTask.getWorkbasketSummary().getId(); + List workbaskets = query + .idIn(workbasketId) + .list(); + if (workbaskets.isEmpty()) { + String currentUser = CurrentUserContext.getUserid(); + LOGGER.error("The current user {} has no read permission for workbasket {}.", currentUser, + workbasketId); + throw new NotAuthorizedException( + "The current user " + currentUser + " has no read permission for workbasket " + workbasketId); + } else { + resultTask.setWorkbasketSummary(workbaskets.get(0)); + } + List attachmentImpls = attachmentMapper.findAttachmentsByTaskId(resultTask.getId()); if (attachmentImpls == null) { attachmentImpls = new ArrayList<>(); @@ -503,7 +520,9 @@ public class TaskServiceImpl implements TaskService { // check source WB (read)+transfer Set workbasketKeys = new HashSet<>(); taskSummaries.stream().forEach(t -> workbasketKeys.add(t.getWorkbasketKey())); - List sourceWorkbaskets = workbasketService.createWorkbasketQuery() + WorkbasketQueryImpl query = (WorkbasketQueryImpl) workbasketService.createWorkbasketQuery(); + query.setUsedToAugmentTasks(true); + List sourceWorkbaskets = query .callerHasPermission(WorkbasketPermission.TRANSFER) .keyIn(workbasketKeys.toArray(new String[0])) .list(); @@ -547,7 +566,7 @@ public class TaskServiceImpl implements TaskService { @Override public Task setTaskRead(String taskId, boolean isRead) - throws TaskNotFoundException { + throws TaskNotFoundException, NotAuthorizedException { LOGGER.debug("entry to setTaskRead(taskId = {}, isRead = {})", taskId, isRead); TaskImpl task = null; try { @@ -793,11 +812,15 @@ public class TaskServiceImpl implements TaskService { String[] workbasketKeyArray = workbasketKeySet.toArray(new String[0]); // perform workbasket query LOGGER.debug("addWorkbasketSummariesToTaskSummaries() about to query workbaskets"); - List workbaskets = this.workbasketService.createWorkbasketQuery() + WorkbasketQueryImpl query = (WorkbasketQueryImpl) workbasketService.createWorkbasketQuery(); + query.setUsedToAugmentTasks(true); + List workbaskets = query .keyIn(workbasketKeyArray) .list(); // assign query results to appropriate tasks. - for (TaskSummaryImpl task : taskSummaries) { + Iterator taskIterator = taskSummaries.iterator(); + while (taskIterator.hasNext()) { + TaskSummaryImpl task = taskIterator.next(); String workbasketKey = task.getWorkbasketSummaryImpl().getKey(); // find the appropriate workbasket from the query result @@ -806,8 +829,9 @@ public class TaskServiceImpl implements TaskService { .findFirst() .orElse(null); if (aWorkbasket == null) { - LOGGER.error("Could not find a Workbasket for task {}.", task.getTaskId()); - throw new SystemException("Could not find a Workbasket for task " + task.getTaskId()); + LOGGER.warn("Could not find a Workbasket for task {}.", task.getTaskId()); + taskIterator.remove(); + continue; } // set the classification on the task object task.setWorkbasketSummary(aWorkbasket); diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketQueryImpl.java index 4b0e89c46..11638fb78 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketQueryImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketQueryImpl.java @@ -36,6 +36,7 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { private static final Logger LOGGER = LoggerFactory.getLogger(WorkbasketQueryImpl.class); private String columnName; private String[] accessId; + private String[] idIn; private WorkbasketPermission permission; private String[] nameIn; private String[] nameLike; @@ -68,12 +69,21 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { private String[] orgLevel4Like; private TaskanaEngineImpl taskanaEngine; private List orderBy; + private boolean joinWithAccessList; + private boolean checkReadPermission; + private boolean usedToAugmentTasks; WorkbasketQueryImpl(TaskanaEngine taskanaEngine) { this.taskanaEngine = (TaskanaEngineImpl) taskanaEngine; this.orderBy = new ArrayList<>(); } + @Override + public WorkbasketQuery idIn(String... id) { + this.idIn = id; + return this; + } + @Override public WorkbasketQuery keyIn(String... key) { this.keyIn = toUpperCopy(key); @@ -360,7 +370,7 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { List workbaskets = null; try { taskanaEngine.openConnection(); - addAccessIdsOfCallerToQuery(); + handleCallerRolesAndAccessIds(); workbaskets = taskanaEngine.getSqlSession().selectList(LINK_TO_MAPPER, this); return workbaskets; } finally { @@ -380,7 +390,7 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { try { taskanaEngine.openConnection(); this.columnName = columnName; - addAccessIdsOfCallerToQuery(); + handleCallerRolesAndAccessIds(); this.orderBy.clear(); this.addOrderCriteria(columnName, sortDirection); result = taskanaEngine.getSqlSession().selectList(LINK_TO_VALUEMAPPER, this); @@ -402,17 +412,15 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { try { taskanaEngine.openConnection(); RowBounds rowBounds = new RowBounds(offset, limit); - addAccessIdsOfCallerToQuery(); + handleCallerRolesAndAccessIds(); workbaskets = taskanaEngine.getSqlSession().selectList(LINK_TO_MAPPER, this, rowBounds); return workbaskets; - } catch (Exception e) { - if (e instanceof PersistenceException) { - if (e.getMessage().contains("ERRORCODE=-4470")) { - TaskanaRuntimeException ex = new TaskanaRuntimeException( - "The offset beginning was set over the amount of result-rows.", e.getCause()); - ex.setStackTrace(e.getStackTrace()); - throw ex; - } + } catch (PersistenceException e) { + if (e.getMessage().contains("ERRORCODE=-4470")) { + TaskanaRuntimeException ex = new TaskanaRuntimeException( + "The offset beginning was set over the amount of result-rows.", e.getCause()); + ex.setStackTrace(e.getStackTrace()); + throw ex; } throw e; } finally { @@ -431,7 +439,7 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { WorkbasketSummary workbasket = null; try { taskanaEngine.openConnection(); - addAccessIdsOfCallerToQuery(); + handleCallerRolesAndAccessIds(); workbasket = taskanaEngine.getSqlSession().selectOne(LINK_TO_MAPPER, this); return workbasket; } finally { @@ -440,6 +448,21 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { } } + @Override + public long count() { + LOGGER.debug("entry to count(), this = {}", this); + Long rowCount = null; + try { + taskanaEngine.openConnection(); + handleCallerRolesAndAccessIds(); + rowCount = taskanaEngine.getSqlSession().selectOne(LINK_TO_COUNTER, this); + return (rowCount == null) ? 0L : rowCount; + } finally { + taskanaEngine.returnConnection(); + LOGGER.debug("exit from count(). Returning result {} ", rowCount); + } + } + public String[] getAccessId() { return accessId; } @@ -564,6 +587,10 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { return ownerLike; } + public String[] getIdIn() { + return idIn; + } + public List getOrderBy() { return orderBy; } @@ -572,26 +599,27 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { return columnName; } - @Override - public long count() { - LOGGER.debug("entry to count(), this = {}", this); - Long rowCount = null; - try { - taskanaEngine.openConnection(); - addAccessIdsOfCallerToQuery(); - rowCount = taskanaEngine.getSqlSession().selectOne(LINK_TO_COUNTER, this); - return (rowCount == null) ? 0L : rowCount; - } finally { - taskanaEngine.returnConnection(); - LOGGER.debug("exit from count(). Returning result {} ", rowCount); - } + public boolean isJoinWithAccessList() { + return joinWithAccessList; + } + + public boolean isCheckReadPermission() { + return checkReadPermission; + } + + void setUsedToAugmentTasks(boolean usedToAugmentTasks) { + this.usedToAugmentTasks = usedToAugmentTasks; } @Override public String toString() { StringBuilder builder = new StringBuilder(); - builder.append("WorkbasketQueryImpl [accessId="); + builder.append("WorkbasketQueryImpl [columnName="); + builder.append(columnName); + builder.append(", accessId="); builder.append(Arrays.toString(accessId)); + builder.append(", idIn="); + builder.append(Arrays.toString(idIn)); builder.append(", permission="); builder.append(permission); builder.append(", nameIn="); @@ -654,11 +682,34 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { builder.append(Arrays.toString(orgLevel4Like)); builder.append(", orderBy="); builder.append(orderBy); + builder.append(", joinWithAccessList="); + builder.append(joinWithAccessList); + builder.append(", checkReadPermission="); + builder.append(checkReadPermission); + builder.append(", usedToAugmentTasks="); + builder.append(usedToAugmentTasks); builder.append("]"); return builder.toString(); } - private void addAccessIdsOfCallerToQuery() { + private void handleCallerRolesAndAccessIds() { + // if user is admin or businessadmin, don't check read permission on workbasket. + // in addition, if user is admin or businessadmin and no accessIds were specified, don't join with access list + // if this query is used to augment task, a business admin should be treated like a normal user + // (joinWithAccessList,checkReadPermission) can assume the following combinations: + // (t,t) -> query performed by user + // (f,f) -> admin queries w/o access ids specified + // (t,f) -> admin queries with access ids specified + // (f,t) -> cannot happen, cannot be matched to meaningful query + joinWithAccessList = true; + checkReadPermission = true; + if (taskanaEngine.isUserInRole(TaskanaRole.ADMIN) + || (taskanaEngine.isUserInRole(TaskanaRole.BUSINESS_ADMIN) && !usedToAugmentTasks)) { + checkReadPermission = false; + if (accessId == null) { + joinWithAccessList = false; + } + } // might already be set by accessIdsHavePermission if (this.accessId == null) { String[] accessIds = new String[0]; @@ -670,6 +721,7 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { this.accessId = accessIds; lowercaseAccessIds(this.accessId); } + } static void lowercaseAccessIds(String[] accessIdArray) { diff --git a/lib/taskana-core/src/main/java/pro/taskana/mappings/QueryMapper.java b/lib/taskana-core/src/main/java/pro/taskana/mappings/QueryMapper.java index 8ca0e144b..f58cf06bd 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/mappings/QueryMapper.java +++ b/lib/taskana-core/src/main/java/pro/taskana/mappings/QueryMapper.java @@ -214,6 +214,7 @@ public interface QueryMapper { + "" + "AND w.OWNER IN(#{item}) " + "AND (UPPER(w.OWNER) LIKE #{item}) " + + "AND w.ID IN(#{item}) " + "AND UPPER(w.KEY) IN(#{item}) " + "AND (UPPER(w.KEY) LIKE #{item}) " + "AND UPPER(w.NAME) IN(#{item}) " @@ -449,6 +450,7 @@ public interface QueryMapper { + "" + "AND w.OWNER IN(#{item}) " + "AND (UPPER(w.OWNER) LIKE #{item}) " + + "AND w.ID IN(#{item}) " + "AND UPPER(w.KEY) IN(#{item}) " + "AND (UPPER(w.KEY) LIKE #{item}) " + "AND UPPER(w.NAME) IN(#{item}) " @@ -479,6 +481,7 @@ public interface QueryMapper { + " " + " " + "AND (a.MAX_READ = 1 " + + " " + "AND " + " " + "( " @@ -501,6 +504,7 @@ public interface QueryMapper { + "a.MAX_CUSTOM_11" + "a.MAX_CUSTOM_12 = 1 " + ")" + + "" + "" + "") Long countQueryWorkbaskets(WorkbasketQueryImpl workbasketQuery); @@ -646,6 +650,7 @@ public interface QueryMapper { + "" + "AND w.OWNER IN(#{item}) " + "AND (UPPER(w.OWNER) LIKE #{item}) " + + "AND w.ID IN(#{item}) " + "AND UPPER(w.KEY) IN(#{item}) " + "AND (UPPER(w.KEY) LIKE #{item}) " + "AND UPPER(w.NAME) IN(#{item}) " diff --git a/lib/taskana-core/src/main/java/pro/taskana/mappings/TaskMapper.java b/lib/taskana-core/src/main/java/pro/taskana/mappings/TaskMapper.java index 4190ca327..2a05b7d0f 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/mappings/TaskMapper.java +++ b/lib/taskana-core/src/main/java/pro/taskana/mappings/TaskMapper.java @@ -5,7 +5,6 @@ import java.util.Map; import org.apache.ibatis.annotations.Delete; import org.apache.ibatis.annotations.Insert; -import org.apache.ibatis.annotations.One; import org.apache.ibatis.annotations.Options; import org.apache.ibatis.annotations.Param; import org.apache.ibatis.annotations.Result; @@ -18,7 +17,6 @@ import pro.taskana.TaskState; import pro.taskana.impl.MinimalTaskSummary; import pro.taskana.impl.TaskImpl; import pro.taskana.impl.TaskSummaryImpl; -import pro.taskana.impl.WorkbasketSummaryImpl; import pro.taskana.impl.persistence.MapTypeHandler; /** @@ -26,10 +24,6 @@ import pro.taskana.impl.persistence.MapTypeHandler; */ public interface TaskMapper { - String OBJECTREFERENCEMAPPER_FINDBYID = "pro.taskana.mappings.ObjectReferenceMapper.findById"; - String WORKBASKET_FINDSUMMARYBYID = "pro.taskana.mappings.WorkbasketMapper.findSummaryById"; - String CLASSIFICATION_FINDBYID = "pro.taskana.mappings.ClassificationMapper.findById"; - @Select("SELECT ID, CREATED, CLAIMED, COMPLETED, MODIFIED, PLANNED, DUE, NAME, CREATOR, DESCRIPTION, NOTE, PRIORITY, STATE, CLASSIFICATION_CATEGORY, CLASSIFICATION_KEY, CLASSIFICATION_ID, WORKBASKET_ID, WORKBASKET_KEY, DOMAIN, BUSINESS_PROCESS_ID, PARENT_BUSINESS_PROCESS_ID, OWNER, POR_COMPANY, POR_SYSTEM, POR_INSTANCE, POR_TYPE, POR_VALUE, IS_READ, IS_TRANSFERRED, CUSTOM_ATTRIBUTES, CUSTOM_1, CUSTOM_2, CUSTOM_3, CUSTOM_4, CUSTOM_5, CUSTOM_6, CUSTOM_7, CUSTOM_8, CUSTOM_9, CUSTOM_10 " + "FROM TASK " + "WHERE ID = #{id}") @@ -47,8 +41,8 @@ public interface TaskMapper { @Result(property = "note", column = "NOTE"), @Result(property = "priority", column = "PRIORITY"), @Result(property = "state", column = "STATE"), - @Result(property = "workbasketSummary", column = "WORKBASKET_ID", javaType = WorkbasketSummaryImpl.class, - one = @One(select = WORKBASKET_FINDSUMMARYBYID)), + @Result(property = "workbasketSummaryImpl.id", column = "WORKBASKET_ID"), + @Result(property = "workbasketSummaryImpl.key", column = "WORKBASKET_KEY"), @Result(property = "classificationSummaryImpl.category", column = "CLASSIFICATION_CATEGORY"), @Result(property = "classificationSummaryImpl.id", column = "CLASSIFICATION_ID"), @Result(property = "classificationSummaryImpl.key", column = "CLASSIFICATION_KEY"), diff --git a/lib/taskana-core/src/test/java/acceptance/security/ClassificationQueryAccTest.java b/lib/taskana-core/src/test/java/acceptance/security/ClassificationQueryAccTest.java new file mode 100644 index 000000000..aeac5642c --- /dev/null +++ b/lib/taskana-core/src/test/java/acceptance/security/ClassificationQueryAccTest.java @@ -0,0 +1,71 @@ +package acceptance.security; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import java.sql.SQLException; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import acceptance.AbstractAccTest; +import pro.taskana.ClassificationService; +import pro.taskana.ClassificationSummary; +import pro.taskana.exceptions.ClassificationNotFoundException; +import pro.taskana.exceptions.InvalidArgumentException; +import pro.taskana.exceptions.NotAuthorizedException; +import pro.taskana.security.JAASRunner; +import pro.taskana.security.WithAccessId; + +/** + * Acceptance test for classification queries and authorization. + * + * @author bbr + */ +@RunWith(JAASRunner.class) +public class ClassificationQueryAccTest extends AbstractAccTest { + + public ClassificationQueryAccTest() { + super(); + } + + @Test + public void testFindClassificationsByDomainUnauthenticated() + throws SQLException, ClassificationNotFoundException, NotAuthorizedException, InvalidArgumentException { + ClassificationService classificationService = taskanaEngine.getClassificationService(); + List classificationSummaryList = classificationService.createClassificationQuery() + .domainIn("DOMAIN_A") + .list(); + + assertNotNull(classificationSummaryList); + assertEquals(16, classificationSummaryList.size()); + } + + @WithAccessId(userName = "businessadmin") + @Test + public void testFindClassificationsByDomainBusinessAdmin() + throws SQLException, ClassificationNotFoundException, NotAuthorizedException, InvalidArgumentException { + ClassificationService classificationService = taskanaEngine.getClassificationService(); + List classificationSummaryList = classificationService.createClassificationQuery() + .domainIn("DOMAIN_A") + .list(); + + assertNotNull(classificationSummaryList); + assertEquals(16, classificationSummaryList.size()); + } + + @WithAccessId(userName = "admin") + @Test + public void testFindClassificationsByDomainAdmin() + throws SQLException, ClassificationNotFoundException, NotAuthorizedException, InvalidArgumentException { + ClassificationService classificationService = taskanaEngine.getClassificationService(); + List classificationSummaryList = classificationService.createClassificationQuery() + .domainIn("DOMAIN_A") + .list(); + + assertNotNull(classificationSummaryList); + assertEquals(16, classificationSummaryList.size()); + } + +} diff --git a/lib/taskana-core/src/test/java/acceptance/security/TaskQueryAccTest.java b/lib/taskana-core/src/test/java/acceptance/security/TaskQueryAccTest.java new file mode 100644 index 000000000..5f91d8f0b --- /dev/null +++ b/lib/taskana-core/src/test/java/acceptance/security/TaskQueryAccTest.java @@ -0,0 +1,80 @@ +package acceptance.security; + +import static org.hamcrest.core.IsEqual.equalTo; +import static org.junit.Assert.assertThat; + +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import acceptance.AbstractAccTest; +import pro.taskana.TaskService; +import pro.taskana.TaskSummary; +import pro.taskana.security.JAASRunner; +import pro.taskana.security.WithAccessId; + +/** + * Acceptance test for task queries and authorization. + */ +@RunWith(JAASRunner.class) +public class TaskQueryAccTest extends AbstractAccTest { + + public TaskQueryAccTest() { + super(); + } + + public void testTaskQueryUnauthenticated() { + TaskService taskService = taskanaEngine.getTaskService(); + + List results = taskService.createTaskQuery() + .ownerLike("%a%", "%u%") + .list(); + + assertThat(results.size(), equalTo(0)); + + } + + @WithAccessId( + userName = "user_1_1") // , groupNames = {"businessadmin"}) + @Test + public void testTaskQueryUser_1_1() { + TaskService taskService = taskanaEngine.getTaskService(); + + List results = taskService.createTaskQuery() + .ownerLike("%a%", "%u%") + .list(); + + assertThat(results.size(), equalTo(3)); + + } + + @WithAccessId( + userName = "user_1_1", groupNames = {"businessadmin"}) + @Test + public void testTaskQueryUser_1_1BusinessAdm() { + TaskService taskService = taskanaEngine.getTaskService(); + + List results = taskService.createTaskQuery() + .ownerLike("%a%", "%u%") + .list(); + + assertThat(results.size(), equalTo(3)); + + } + + @WithAccessId( + userName = "user_1_1", groupNames = {"admin"}) + @Test + public void testTaskQueryUser_1_1Admin() { + TaskService taskService = taskanaEngine.getTaskService(); + + List results = taskService.createTaskQuery() + .ownerLike("%a%", "%u%") + .list(); + + assertThat(results.size(), equalTo(25)); + + } + +} diff --git a/lib/taskana-core/src/test/java/acceptance/security/WorkbasketQueryAccTest.java b/lib/taskana-core/src/test/java/acceptance/security/WorkbasketQueryAccTest.java new file mode 100644 index 000000000..e88119966 --- /dev/null +++ b/lib/taskana-core/src/test/java/acceptance/security/WorkbasketQueryAccTest.java @@ -0,0 +1,114 @@ +package acceptance.security; + +import java.sql.SQLException; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; + +import acceptance.AbstractAccTest; +import pro.taskana.WorkbasketPermission; +import pro.taskana.WorkbasketService; +import pro.taskana.WorkbasketSummary; +import pro.taskana.exceptions.InvalidArgumentException; +import pro.taskana.exceptions.InvalidRequestException; +import pro.taskana.exceptions.NotAuthorizedException; +import pro.taskana.security.JAASRunner; +import pro.taskana.security.WithAccessId; + +/** + * Acceptance test for workbasket queries and authorization. + */ +@RunWith(JAASRunner.class) +public class WorkbasketQueryAccTest extends AbstractAccTest { + + public WorkbasketQueryAccTest() { + super(); + } + + @Test + public void testQueryWorkbasketByUnauthenticated() + throws SQLException, NotAuthorizedException, InvalidRequestException, InvalidArgumentException { + WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + List results = workbasketService.createWorkbasketQuery() + .nameLike("%") + .list(); + Assert.assertEquals(0L, results.size()); + + try { + results = workbasketService.createWorkbasketQuery() + .nameLike("%") + .accessIdsHavePermission(WorkbasketPermission.TRANSFER, "teamlead_1", "group_1", "group_2") + .list(); + Assert.fail("NotAuthrorizedException was expected"); + } catch (NotAuthorizedException ex) { + + } + + } + + @WithAccessId( + userName = "unknown") + @Test + public void testQueryWorkbasketByUnknownUser() + throws SQLException, NotAuthorizedException, InvalidRequestException, InvalidArgumentException { + WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + List results = workbasketService.createWorkbasketQuery() + .nameLike("%") + .list(); + Assert.assertEquals(0L, results.size()); + + try { + results = workbasketService.createWorkbasketQuery() + .nameLike("%") + .accessIdsHavePermission(WorkbasketPermission.TRANSFER, "teamlead_1", "group_1", "group_2") + .list(); + Assert.fail("NotAuthrorizedException was expected"); + } catch (NotAuthorizedException ex) { + + } + + } + + @WithAccessId( + userName = "unknown", + groupNames = "businessadmin") + @Test + public void testQueryWorkbasketByBusinessAdmin() + throws SQLException, NotAuthorizedException, InvalidRequestException, InvalidArgumentException { + WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + List results = workbasketService.createWorkbasketQuery() + .nameLike("%") + .list(); + Assert.assertEquals(24L, results.size()); + + results = workbasketService.createWorkbasketQuery() + .nameLike("%") + .accessIdsHavePermission(WorkbasketPermission.TRANSFER, "teamlead_1", "group_1", "group_2") + .list(); + + Assert.assertEquals(13L, results.size()); + + } + + @WithAccessId( + userName = "unknown", + groupNames = "admin") + @Test + public void testQueryWorkbasketByAdmin() + throws SQLException, NotAuthorizedException, InvalidRequestException, InvalidArgumentException { + WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + List results = workbasketService.createWorkbasketQuery() + .nameLike("%") + .list(); + Assert.assertEquals(24L, results.size()); + + results = workbasketService.createWorkbasketQuery() + .nameLike("%") + .accessIdsHavePermission(WorkbasketPermission.TRANSFER, "teamlead_1", "group_1", "group_2") + .list(); + + Assert.assertEquals(13L, results.size()); + } +} diff --git a/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskAccTest.java index 590415722..df7a2179b 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/DeleteTaskAccTest.java @@ -93,7 +93,7 @@ public class DeleteTaskAccTest extends AbstractAccTest { userName = "user_1_2", groupNames = {"group_1"}) @Test(expected = TaskNotFoundException.class) - public void testBulkDeleteTask() throws TaskNotFoundException, InvalidArgumentException { + public void testBulkDeleteTask() throws TaskNotFoundException, InvalidArgumentException, NotAuthorizedException { TaskService taskService = taskanaEngine.getTaskService(); ArrayList taskIdList = new ArrayList<>(); @@ -110,7 +110,8 @@ public class DeleteTaskAccTest extends AbstractAccTest { userName = "user_1_2", groupNames = {"group_1"}) @Test(expected = TaskNotFoundException.class) - public void testBulkDeleteTasksWithException() throws TaskNotFoundException, InvalidArgumentException { + public void testBulkDeleteTasksWithException() + throws TaskNotFoundException, InvalidArgumentException, NotAuthorizedException { TaskService taskService = taskanaEngine.getTaskService(); ArrayList taskIdList = new ArrayList<>(); diff --git a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAttachmentsAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAttachmentsAccTest.java index 7260a0919..a2b3a565b 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAttachmentsAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskAttachmentsAccTest.java @@ -13,7 +13,6 @@ import java.time.Duration; import java.util.ArrayList; import java.util.List; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -52,8 +51,10 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { super(); } - @Before - public void setUpMethod() + // this method needs to run with access ids, otherwise getTask throws NotAuthorizedException + // since only @Test and not @Before methods are run by JAASRunner, we call this method explicitely at + // the begin of each testcase.... + private void setUpMethod() throws TaskNotFoundException, ClassificationNotFoundException, NotAuthorizedException, SQLException, WorkbasketNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, AttachmentPersistenceException { @@ -69,11 +70,15 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { assertThat(task, not(equalTo(null))); } + @WithAccessId( + userName = "user_1_1", + groupNames = {"group_1"}) @Test public void testAddNewAttachment() throws TaskNotFoundException, ClassificationNotFoundException, NotAuthorizedException, WorkbasketNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, - AttachmentPersistenceException { + AttachmentPersistenceException, SQLException { + setUpMethod(); int attachmentCount = task.getAttachments().size(); assertTrue(task.getPriority() == 1); assertTrue(task.getDue().equals(task.getPlanned().plus(Duration.ofDays(1)))); @@ -88,11 +93,15 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { assertTrue(task.getDue().equals(task.getPlanned().plus(Duration.ofDays(1)))); } + @WithAccessId( + userName = "user_1_1", + groupNames = {"group_1"}) @Test(expected = AttachmentPersistenceException.class) public void testAddNewAttachmentTwiceWithoutTaskanaMethodWillThrowAttachmentPersistenceException() throws TaskNotFoundException, WorkbasketNotFoundException, ClassificationNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, NotAuthorizedException, - AttachmentPersistenceException { + AttachmentPersistenceException, SQLException { + setUpMethod(); int attachmentCount = 0; task.getAttachments().clear(); task = taskService.updateTask(task); @@ -107,11 +116,15 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { task = taskService.updateTask(task); } + @WithAccessId( + userName = "user_1_1", + groupNames = {"group_1"}) @Test public void testAddExistingAttachmentAgainWillUpdateWhenNotEqual() throws TaskNotFoundException, ClassificationNotFoundException, NotAuthorizedException, WorkbasketNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, - AttachmentPersistenceException { + AttachmentPersistenceException, SQLException { + setUpMethod(); // Add attachment before task = taskService.getTask(task.getId()); int attachmentCount = task.getAttachments().size(); @@ -138,11 +151,15 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { } + @WithAccessId( + userName = "user_1_1", + groupNames = {"group_1"}) @Test public void testAddExistingAttachmentAgainWillDoNothingWhenEqual() throws TaskNotFoundException, ClassificationNotFoundException, NotAuthorizedException, WorkbasketNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, - AttachmentPersistenceException { + AttachmentPersistenceException, SQLException { + setUpMethod(); // Add Attachment before int attachmentCount = task.getAttachments().size(); ((AttachmentImpl) attachment).setId("TAI:0001"); @@ -161,11 +178,15 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { assertThat(task.getAttachments().size(), equalTo(attachmentCount)); } + @WithAccessId( + userName = "user_1_1", + groupNames = {"group_1"}) @Test public void testAddAttachmentAsNullValueWillBeIgnored() throws TaskNotFoundException, WorkbasketNotFoundException, ClassificationNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, NotAuthorizedException, - AttachmentPersistenceException { + AttachmentPersistenceException, SQLException { + setUpMethod(); // Try to add a single NULL-Element int attachmentCount = task.getAttachments().size(); task.addAttachment(null); @@ -194,11 +215,15 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { assertTrue(task.getDue().equals(task.getPlanned().plus(Duration.ofDays(1)))); } + @WithAccessId( + userName = "user_1_1", + groupNames = {"group_1"}) @Test public void testRemoveAttachment() throws TaskNotFoundException, WorkbasketNotFoundException, ClassificationNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, NotAuthorizedException, - AttachmentPersistenceException { + AttachmentPersistenceException, SQLException { + setUpMethod(); task.addAttachment(attachment); task = taskService.updateTask(task); assertTrue(task.getPriority() == 99); @@ -214,11 +239,15 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { assertTrue(task.getDue().equals(task.getPlanned().plus(Duration.ofDays(1)))); } + @WithAccessId( + userName = "user_1_1", + groupNames = {"group_1"}) @Test public void testRemoveAttachmentWithNullAndNotAddedId() throws TaskNotFoundException, WorkbasketNotFoundException, ClassificationNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, NotAuthorizedException, - AttachmentPersistenceException { + AttachmentPersistenceException, SQLException { + setUpMethod(); task.addAttachment(attachment); task = taskService.updateTask(task); int attachmentCount = task.getAttachments().size(); @@ -236,11 +265,15 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { assertThat(task.getAttachments().size(), equalTo(attachmentCount)); // persisted, still same } + @WithAccessId( + userName = "user_1_1", + groupNames = {"group_1"}) @Test public void testUpdateAttachment() throws TaskNotFoundException, WorkbasketNotFoundException, ClassificationNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, NotAuthorizedException, - AttachmentPersistenceException { + AttachmentPersistenceException, SQLException { + setUpMethod(); ((TaskImpl) task).setAttachments(new ArrayList<>()); task = taskService.updateTask(task); assertTrue(task.getPriority() == 1); @@ -268,11 +301,15 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { } + @WithAccessId( + userName = "user_1_1", + groupNames = {"group_1"}) @Test public void modifyExistingAttachment() throws TaskNotFoundException, ClassificationNotFoundException, NotAuthorizedException, WorkbasketNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, - AttachmentPersistenceException { + AttachmentPersistenceException, SQLException { + setUpMethod(); // setup test assertThat(task.getAttachments().size(), equalTo(0)); task.addAttachment(attachment); @@ -343,11 +380,15 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { assertTrue(faxFound && rohrpostFound); } + @WithAccessId( + userName = "user_1_1", + groupNames = {"group_1"}) @Test public void replaceExistingAttachments() throws TaskNotFoundException, ClassificationNotFoundException, NotAuthorizedException, WorkbasketNotFoundException, InvalidArgumentException, ConcurrencyException, InvalidWorkbasketException, - AttachmentPersistenceException { + AttachmentPersistenceException, SQLException { + setUpMethod(); // setup test assertThat(task.getAttachments().size(), equalTo(0)); task.addAttachment(attachment); @@ -392,8 +433,10 @@ public class UpdateTaskAttachmentsAccTest extends AbstractAccTest { @Test public void testPrioDurationOfTaskFromAttachmentsAtUpdate() throws SQLException, NotAuthorizedException, InvalidArgumentException, ClassificationNotFoundException, - WorkbasketNotFoundException, TaskAlreadyExistException, InvalidWorkbasketException, TaskNotFoundException { + WorkbasketNotFoundException, TaskAlreadyExistException, InvalidWorkbasketException, TaskNotFoundException, + ConcurrencyException, AttachmentPersistenceException { + setUpMethod(); TaskService taskService = taskanaEngine.getTaskService(); Task newTask = taskService.newTask("USER_1_1", "DOMAIN_A"); newTask.setClassificationKey("L12010"); // prio 8, SL P7D diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/QueryWorkbasketAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/QueryWorkbasketAccTest.java index 940112386..a5c4e076e 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/QueryWorkbasketAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/QueryWorkbasketAccTest.java @@ -12,6 +12,7 @@ import org.junit.runner.RunWith; import acceptance.AbstractAccTest; import pro.taskana.BaseQuery.SortDirection; +import pro.taskana.WorkbasketPermission; import pro.taskana.WorkbasketService; import pro.taskana.WorkbasketSummary; import pro.taskana.WorkbasketType; @@ -358,4 +359,36 @@ public class QueryWorkbasketAccTest extends AbstractAccTest { Assert.assertEquals(8L, results.size()); } + @WithAccessId( + userName = "unknown", + groupNames = "admin") + @Test + public void testQueryWorkbasketByAdmin() + throws SQLException, NotAuthorizedException, InvalidRequestException, InvalidArgumentException { + WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + List results = workbasketService.createWorkbasketQuery() + .nameLike("%") + .orderByName(desc) + .list(); + Assert.assertEquals(24L, results.size()); + // check sort order is correct + WorkbasketSummary previousSummary = null; + for (WorkbasketSummary wbSummary : results) { + if (previousSummary != null) { + Assert.assertTrue(wbSummary.getName().compareToIgnoreCase( + previousSummary.getName()) <= 0); + } + previousSummary = wbSummary; + } + + results = workbasketService.createWorkbasketQuery() + .nameLike("%") + .accessIdsHavePermission(WorkbasketPermission.TRANSFER, "teamlead_1", "group_1", "group_2") + .orderByName(desc) + .list(); + + Assert.assertEquals(13L, results.size()); + + } + } 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 5b77755b0..b0dd423fd 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 @@ -43,6 +43,7 @@ import pro.taskana.TaskState; import pro.taskana.Workbasket; import pro.taskana.WorkbasketPermission; import pro.taskana.WorkbasketService; +import pro.taskana.WorkbasketSummary; import pro.taskana.configuration.TaskanaEngineConfiguration; import pro.taskana.exceptions.AttachmentPersistenceException; import pro.taskana.exceptions.ClassificationAlreadyExistException; @@ -452,7 +453,7 @@ public class TaskServiceImplTest { @Test public void testClaimDefaultFlag() - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); TaskImpl expectedTask = createUnitTestTask("1", "Unit Test Task 1", "1", null); doReturn(expectedTask).when(cutSpy).claim(expectedTask.getId(), false); @@ -556,7 +557,7 @@ public class TaskServiceImplTest { @Test(expected = InvalidStateException.class) public void testCancelClaimForcedWithInvalidState() throws TaskNotFoundException, - InvalidStateException, InvalidOwnerException { + InvalidStateException, InvalidOwnerException, NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); TaskImpl expectedTask = createUnitTestTask("1", "Unit Test Task 1", "1", null); expectedTask.setState(TaskState.COMPLETED); @@ -578,7 +579,7 @@ public class TaskServiceImplTest { @Test(expected = InvalidOwnerException.class) public void testCancelClaimNotForcedWithInvalidOwner() throws TaskNotFoundException, - InvalidStateException, InvalidOwnerException { + InvalidStateException, InvalidOwnerException, NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); TaskImpl expectedTask = createUnitTestTask("1", "Unit Test Task 1", "1", null); expectedTask.setOwner("Thomas"); @@ -602,7 +603,7 @@ public class TaskServiceImplTest { @Test public void testCancelClaimDefaultFlag() - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); TaskImpl expectedTask = createUnitTestTask("1", "Unit Test Task 1", "1", null); doReturn(expectedTask).when(cutSpy).cancelClaim(expectedTask.getId(), false); @@ -616,7 +617,7 @@ public class TaskServiceImplTest { @Test public void testCancelClaimSuccesfullForced() throws TaskNotFoundException, - InvalidStateException, InvalidOwnerException { + InvalidStateException, InvalidOwnerException, NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); String owner = "John Does"; TaskImpl expectedTask = createUnitTestTask("1", "Unit Test Task 1", "1", null); @@ -646,7 +647,7 @@ public class TaskServiceImplTest { @Test public void testCancelClaimInvalidState() - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); String owner = "John Does"; TaskImpl expectedTask = createUnitTestTask("1", "Unit Test Task 1", "1", null); @@ -677,7 +678,7 @@ public class TaskServiceImplTest { @Test public void testCompleteTaskDefault() throws TaskNotFoundException, InvalidOwnerException, InvalidStateException, InterruptedException, - ClassificationNotFoundException { + ClassificationNotFoundException, NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); final long sleepTime = 100L; final boolean isForced = false; @@ -699,6 +700,13 @@ public class TaskServiceImplTest { doReturn(classificationList).when( classificationQueryImplMock) .list(); + doReturn(workbasketQueryImplMock).when(workbasketServiceMock).createWorkbasketQuery(); + doReturn(workbasketQueryImplMock).when(workbasketQueryImplMock).idIn(any()); + List wbList = new ArrayList<>(); + WorkbasketSummaryImpl wb = new WorkbasketSummaryImpl(); + wb.setDomain("dummy-domain"); + wbList.add(wb); + doReturn(wbList).when(workbasketQueryImplMock).list(); Task actualTask = cut.completeTask(task.getId()); @@ -711,6 +719,10 @@ public class TaskServiceImplTest { verify(classificationQueryImplMock, times(1)).list(); verify(taskMapperMock, times(1)).update(any()); verify(taskanaEngineMock, times(2)).returnConnection(); + verify(workbasketServiceMock, times(1)).createWorkbasketQuery(); + verify(workbasketQueryImplMock, times(1)).idIn(any()); + verify(workbasketQueryImplMock, times(1)).list(); + verifyNoMoreInteractions(attachmentMapperMock, taskanaEngineConfigurationMock, taskanaEngineMock, taskMapperMock, objectReferenceMapperMock, workbasketServiceMock, sqlSessionMock, classificationQueryImplMock); @@ -723,7 +735,7 @@ public class TaskServiceImplTest { @Test public void testCompleteTaskNotForcedWorking() throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, InterruptedException, - ClassificationNotFoundException { + ClassificationNotFoundException, NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); final long sleepTime = 100L; final boolean isForced = false; @@ -755,7 +767,8 @@ public class TaskServiceImplTest { @Test(expected = InvalidStateException.class) public void testCompleteTaskNotForcedNotClaimedBefore() - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, ClassificationNotFoundException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, ClassificationNotFoundException, + NotAuthorizedException { final boolean isForced = false; TaskServiceImpl cutSpy = Mockito.spy(cut); Classification dummyClassification = createDummyClassification(); @@ -780,7 +793,8 @@ public class TaskServiceImplTest { @Test(expected = InvalidOwnerException.class) public void testCompleteTaskNotForcedInvalidOwnerException() - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, ClassificationNotFoundException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, ClassificationNotFoundException, + NotAuthorizedException { final boolean isForced = false; TaskServiceImpl cutSpy = Mockito.spy(cut); Classification dummyClassification = createDummyClassification(); @@ -806,7 +820,8 @@ public class TaskServiceImplTest { @Test(expected = TaskNotFoundException.class) public void testCompleteTaskTaskNotFound() - throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, ClassificationNotFoundException { + throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, ClassificationNotFoundException, + NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); final boolean isForced = false; String taskId = "1"; @@ -828,7 +843,7 @@ public class TaskServiceImplTest { @Test public void testCompleteForcedAndAlreadyClaimed() throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, InterruptedException, - ClassificationNotFoundException { + ClassificationNotFoundException, NotAuthorizedException { final boolean isForced = true; final long sleepTime = 100L; TaskServiceImpl cutSpy = Mockito.spy(cut); @@ -860,7 +875,7 @@ public class TaskServiceImplTest { @Test public void testCompleteForcedNotClaimed() throws TaskNotFoundException, InvalidStateException, InvalidOwnerException, InterruptedException, - ClassificationNotFoundException { + ClassificationNotFoundException, NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); final boolean isForced = true; final long sleepTime = 100L; @@ -1079,7 +1094,8 @@ public class TaskServiceImplTest { @Test public void testSetTaskReadWIthExistingTask() - throws TaskNotFoundException, ClassificationAlreadyExistException, ClassificationNotFoundException { + throws TaskNotFoundException, ClassificationAlreadyExistException, ClassificationNotFoundException, + NotAuthorizedException { TaskServiceImpl cutSpy = Mockito.spy(cut); Classification dummyClassification = createDummyClassification(); TaskImpl task = createUnitTestTask("1", "Unit Test Task 1", "1", dummyClassification); @@ -1122,7 +1138,8 @@ public class TaskServiceImplTest { @Test public void testGetTaskByIdWithExistingTask() - throws TaskNotFoundException, ClassificationAlreadyExistException, ClassificationNotFoundException { + throws TaskNotFoundException, ClassificationAlreadyExistException, ClassificationNotFoundException, + NotAuthorizedException { Classification dummyClassification = createDummyClassification(); Task expectedTask = createUnitTestTask("1", "DUMMY-TASK", "1", dummyClassification); doReturn(expectedTask).when(taskMapperMock).findById(expectedTask.getId()); @@ -1131,7 +1148,13 @@ public class TaskServiceImplTest { doReturn(classificationQueryImplMock).when(classificationServiceImplMock).createClassificationQuery(); doReturn(classificationQueryImplMock).when(classificationQueryImplMock).domainIn(any()); doReturn(classificationQueryImplMock).when(classificationQueryImplMock).keyIn(any()); - doReturn(new ArrayList<>()).when(classificationQueryImplMock).list(); + doReturn(workbasketQueryImplMock).when(workbasketServiceMock).createWorkbasketQuery(); + doReturn(workbasketQueryImplMock).when(workbasketQueryImplMock).idIn(any()); + List wbList = new ArrayList<>(); + WorkbasketSummaryImpl wb = new WorkbasketSummaryImpl(); + wb.setDomain("dummy-domain"); + wbList.add(wb); + doReturn(wbList).when(workbasketQueryImplMock).list(); List classificationList = Arrays .asList((ClassificationSummaryImpl) dummyClassification.asSummary()); @@ -1147,6 +1170,10 @@ public class TaskServiceImplTest { verify(classificationQueryImplMock, times(1)).domainIn(any()); verify(classificationQueryImplMock, times(1)).keyIn(any()); verify(classificationQueryImplMock, times(1)).list(); + verify(workbasketServiceMock, times(1)).createWorkbasketQuery(); + verify(workbasketQueryImplMock, times(1)).idIn(any()); + verify(workbasketQueryImplMock, times(1)).list(); + verify(taskanaEngineMock, times(1)).returnConnection(); verifyNoMoreInteractions(attachmentMapperMock, taskanaEngineConfigurationMock, taskanaEngineMock, taskMapperMock, objectReferenceMapperMock, workbasketServiceMock, sqlSessionMock, diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskController.java index 4bdeb0644..25d8fae87 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/TaskController.java @@ -77,6 +77,10 @@ public class TaskController { LOGGER.error("The searched Task couldn´t be found or does not exist.", e); TransactionInterceptor.currentTransactionStatus().setRollbackOnly(); return ResponseEntity.status(HttpStatus.NOT_FOUND).build(); + } catch (NotAuthorizedException e) { + LOGGER.error("The current user is not authorized to retrieve the task.", e); + TransactionInterceptor.currentTransactionStatus().setRollbackOnly(); + return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build(); } } @@ -115,6 +119,10 @@ public class TaskController { LOGGER.error("The given Task could not be claimed. Reason: {}", e); TransactionInterceptor.currentTransactionStatus().setRollbackOnly(); return ResponseEntity.status(HttpStatus.CONFLICT).build(); + } catch (NotAuthorizedException e) { + LOGGER.error("The current user is not authorized to claim the task.", e); + TransactionInterceptor.currentTransactionStatus().setRollbackOnly(); + return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build(); } } @@ -131,6 +139,10 @@ public class TaskController { } catch (InvalidStateException | InvalidOwnerException e) { TransactionInterceptor.currentTransactionStatus().setRollbackOnly(); return ResponseEntity.status(HttpStatus.PRECONDITION_FAILED).build(); + } catch (NotAuthorizedException e) { + LOGGER.error("The current user is not authorized to complete the task.", e); + TransactionInterceptor.currentTransactionStatus().setRollbackOnly(); + return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build(); } }