diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskCommentQuery.java b/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskCommentQuery.java index 04ae63f4b..e21ca2651 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskCommentQuery.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/api/TaskCommentQuery.java @@ -51,33 +51,6 @@ public interface TaskCommentQuery extends BaseQuery list() { + checkTaskPermission(); setupAccessIds(); return taskanaEngine.executeInDatabaseConnection( () -> taskanaEngine.getSqlSession().selectList(LINK_TO_MAPPER, this)); @@ -179,6 +166,7 @@ public class TaskCommentQueryImpl implements TaskCommentQuery { @Override public List list(int offset, int limit) { + checkTaskPermission(); setupAccessIds(); RowBounds rowBounds = new RowBounds(offset, limit); return taskanaEngine.executeInDatabaseConnection( @@ -188,10 +176,11 @@ public class TaskCommentQueryImpl implements TaskCommentQuery { @Override public List listValues( TaskCommentQueryColumnName columnName, SortDirection sortDirection) { + checkTaskPermission(); setupAccessIds(); queryColumnName = columnName; // TO-DO: order? - if (columnName == TaskCommentQueryColumnName.CREATOR_LONG_NAME) { + if (columnName == TaskCommentQueryColumnName.CREATOR_FULL_NAME) { joinWithUserInfo = true; } @@ -201,6 +190,7 @@ public class TaskCommentQueryImpl implements TaskCommentQuery { @Override public TaskComment single() { + checkTaskPermission(); setupAccessIds(); return taskanaEngine.executeInDatabaseConnection( () -> taskanaEngine.getSqlSession().selectOne(LINK_TO_MAPPER, this)); @@ -208,6 +198,7 @@ public class TaskCommentQueryImpl implements TaskCommentQuery { @Override public long count() { + checkTaskPermission(); setupAccessIds(); Long rowCount = taskanaEngine.executeInDatabaseConnection( @@ -239,18 +230,6 @@ public class TaskCommentQueryImpl implements TaskCommentQuery { return taskIdIn; } - public String[] getTaskIdNotIn() { - return taskIdNotIn; - } - - public String[] getTaskIdLike() { - return taskIdLike; - } - - public String[] getTaskIdNotLike() { - return taskIdNotLike; - } - public String[] getCreatorIn() { return creatorIn; } @@ -313,6 +292,32 @@ public class TaskCommentQueryImpl implements TaskCommentQuery { return addOrderCriteria("MODIFIED", sortDirection); } + private void checkTaskPermission() { + + if (taskIdIn != null) { + if (taskanaEngine.getEngine().isUserInRole(TaskanaRole.ADMIN, TaskanaRole.TASK_ADMIN)) { + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Skipping permissions check since user is in role ADMIN or TASK_ADMIN."); + } + return; + } + + Arrays.stream(taskIdIn) + .forEach( + taskId -> { + try { + taskService.getTask(taskId); + } catch (NotAuthorizedException e) { + throw new NotAuthorizedToQueryWorkbasketException( + e.getMessage(), e.getErrorCode(), e); + } catch (TaskNotFoundException e) { + LOGGER.warn( + String.format("The Task with the ID ' %s ' does not exist.", taskId), e); + } + }); + } + } + private TaskCommentQuery addOrderCriteria(String columnName, SortDirection sortDirection) { String orderByDirection = " " + (sortDirection == null ? SortDirection.ASCENDING : sortDirection); @@ -362,12 +367,6 @@ public class TaskCommentQueryImpl implements TaskCommentQuery { + Arrays.toString(idNotLike) + ", taskIdIn=" + Arrays.toString(taskIdIn) - + ", taskIdNotIn=" - + Arrays.toString(taskIdNotIn) - + ", taskIdLike=" - + Arrays.toString(taskIdLike) - + ", taskIdNotLike=" - + Arrays.toString(taskIdNotLike) + ", creatorIn=" + Arrays.toString(creatorIn) + ", creatorNotIn=" diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentQueryMapper.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentQueryMapper.java index a54315b9b..dce070254 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentQueryMapper.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentQueryMapper.java @@ -15,7 +15,7 @@ public interface TaskCommentQueryMapper { @Result(property = "taskId", column = "TASK_ID") @Result(property = "textField", column = "TEXT_FIELD") @Result(property = "creator", column = "CREATOR") - @Result(property = "creatorLongName", column = "FULL_NAME") + @Result(property = "creatorFullName", column = "FULL_NAME") @Result(property = "created", column = "CREATED") @Result(property = "modified", column = "MODIFIED") List queryTaskComments( diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentQuerySqlProvider.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentQuerySqlProvider.java index 8ce2e8cd5..06f0f27f0 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentQuerySqlProvider.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentQuerySqlProvider.java @@ -89,9 +89,6 @@ public class TaskCommentQuerySqlProvider { whereLike("idLike", "tc.ID", sb); whereNotLike("idNotLike", "tc.ID", sb); whereIn("taskIdIn", "tc.TASK_ID", sb); - whereNotIn("taskIdNotIn", "tc.TASK_ID", sb); - whereLike("taskIdLike", "tc.TASK_ID", sb); - whereNotLike("taskIdNotLike", "tc.TASK_ID", sb); whereLike("textFieldLike", "tc.TEXT_FIELD", sb); whereNotLike("textFieldNotLike", "tc.TEXT_FIELD", sb); whereIn("creatorIn", "tc.CREATOR", sb); diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java index 828adf681..24ac7114e 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskCommentServiceImpl.java @@ -184,15 +184,15 @@ class TaskCommentServiceImpl { throw new TaskCommentNotFoundException(taskCommentId); } + taskService.getTask(result.getTaskId()); + if (taskanaEngine.getEngine().getConfiguration().getAddAdditionalUserInfo()) { User creator = userMapper.findById(result.getCreator()); if (creator != null) { - result.setCreatorLongName(creator.getFullName()); + result.setCreatorFullName(creator.getFullName()); } } - taskService.getTask(result.getTaskId()); - return result; } finally { diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/models/TaskCommentImpl.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/models/TaskCommentImpl.java index 637b1dfe2..e3465dbca 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/models/TaskCommentImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/models/TaskCommentImpl.java @@ -12,7 +12,7 @@ public class TaskCommentImpl implements TaskComment { private String taskId; private String textField; private String creator; - private String creatorLongName; + private String creatorFullName; private Instant created; private Instant modified; @@ -54,12 +54,12 @@ public class TaskCommentImpl implements TaskComment { } @Override - public String getCreatorLongName() { - return creatorLongName; + public String getCreatorFullName() { + return creatorFullName; } - public void setCreatorLongName(String creatorLongName) { - this.creatorLongName = creatorLongName; + public void setCreatorFullName(String creatorFullName) { + this.creatorFullName = creatorFullName; } public String getTextField() { @@ -99,7 +99,7 @@ public class TaskCommentImpl implements TaskComment { @Override public int hashCode() { - return Objects.hash(id, taskId, textField, creator, creatorLongName, created, modified); + return Objects.hash(id, taskId, textField, creator, creatorFullName, created, modified); } @Override @@ -120,7 +120,7 @@ public class TaskCommentImpl implements TaskComment { && Objects.equals(taskId, other.getTaskId()) && Objects.equals(textField, other.getTextField()) && Objects.equals(creator, other.getCreator()) - && Objects.equals(creatorLongName, other.getCreatorLongName()) + && Objects.equals(creatorFullName, other.getCreatorFullName()) && Objects.equals(created, other.getCreated()) && Objects.equals(modified, other.getModified()); } @@ -135,8 +135,8 @@ public class TaskCommentImpl implements TaskComment { + textField + ", creator=" + creator - + ", creatorLongName=" - + creatorLongName + + ", creatorFullName=" + + creatorFullName + ", created=" + created + ", modified=" diff --git a/lib/taskana-core/src/test/java/acceptance/builder/TaskCommentBuilderTest.java b/lib/taskana-core/src/test/java/acceptance/builder/TaskCommentBuilderTest.java index 497dc6fce..03577ca53 100644 --- a/lib/taskana-core/src/test/java/acceptance/builder/TaskCommentBuilderTest.java +++ b/lib/taskana-core/src/test/java/acceptance/builder/TaskCommentBuilderTest.java @@ -107,7 +107,7 @@ class TaskCommentBuilderTest { expectedTaskComment.setCreator("user-1-1"); assertThat(taskComment) - .hasNoNullFieldsOrPropertiesExcept("creatorLongName") + .hasNoNullFieldsOrPropertiesExcept("creatorFullName") .usingRecursiveComparison() .ignoringFields("id") .isEqualTo(expectedTaskComment); diff --git a/lib/taskana-core/src/test/java/acceptance/jobs/helper/TaskUpdatePriorityBatchStatementAccTest.java b/lib/taskana-core/src/test/java/acceptance/jobs/helper/TaskUpdatePriorityBatchStatementAccTest.java index 98c3c544e..0c28c960c 100644 --- a/lib/taskana-core/src/test/java/acceptance/jobs/helper/TaskUpdatePriorityBatchStatementAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/jobs/helper/TaskUpdatePriorityBatchStatementAccTest.java @@ -41,7 +41,9 @@ class TaskUpdatePriorityBatchStatementAccTest extends AbstractAccTest { new TaskUpdatePriorityBatchStatement(connection); batchStatement.addPriorityUpdate(taskId, priorityUpdate); batchStatement.executeBatch(); - connection.commit(); + if (!connection.getAutoCommit()) { + connection.commit(); + } }); // then diff --git a/lib/taskana-core/src/test/java/acceptance/task/GetTaskCommentAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/GetTaskCommentAccTest.java index 73709b631..58f1cb635 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/GetTaskCommentAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/GetTaskCommentAccTest.java @@ -97,9 +97,9 @@ class GetTaskCommentAccTest extends AbstractAccTest { TaskComment taskComment = taskService.getTaskComment("TCI:000000000000000000000000000000000000"); - String creatorLongName = + String creatorFullName = taskanaEngine.getUserService().getUser(taskComment.getCreator()).getFullName(); - assertThat(taskComment).extracting(TaskComment::getCreatorLongName).isEqualTo(creatorLongName); + assertThat(taskComment).extracting(TaskComment::getCreatorFullName).isEqualTo(creatorFullName); } @WithAccessId(user = "admin") @@ -112,7 +112,7 @@ class GetTaskCommentAccTest extends AbstractAccTest { TaskComment taskComment = taskService.getTaskComment("TCI:000000000000000000000000000000000000"); - assertThat(taskComment).extracting(TaskComment::getCreatorLongName).isNull(); + assertThat(taskComment).extracting(TaskComment::getCreatorFullName).isNull(); } @WithAccessId(user = "admin") @@ -128,11 +128,11 @@ class GetTaskCommentAccTest extends AbstractAccTest { taskComments.forEach( wrap( taskComment -> { - String creatorLongName = + String creatorFullName = taskanaEngine.getUserService().getUser(taskComment.getCreator()).getFullName(); assertThat(taskComment) - .extracting(TaskComment::getCreatorLongName) - .isEqualTo(creatorLongName); + .extracting(TaskComment::getCreatorFullName) + .isEqualTo(creatorFullName); })); } @@ -148,6 +148,6 @@ class GetTaskCommentAccTest extends AbstractAccTest { taskComments.forEach( taskComment -> - assertThat(taskComment).extracting(TaskComment::getCreatorLongName).isNull()); + assertThat(taskComment).extracting(TaskComment::getCreatorFullName).isNull()); } } diff --git a/lib/taskana-core/src/test/java/acceptance/task/QueryTaskCommentAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/QueryTaskCommentAccTest.java index a9d1c3a3f..88b292d16 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/QueryTaskCommentAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/QueryTaskCommentAccTest.java @@ -7,6 +7,7 @@ import acceptance.AbstractAccTest; import java.time.Instant; import java.util.List; import org.apache.ibatis.exceptions.TooManyResultsException; +import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -16,6 +17,7 @@ import pro.taskana.common.test.security.WithAccessId; import pro.taskana.task.api.TaskCommentQuery; import pro.taskana.task.api.TaskCommentQueryColumnName; import pro.taskana.task.api.models.TaskComment; +import pro.taskana.workbasket.api.exceptions.NotAuthorizedToQueryWorkbasketException; /** Test for TaskComment queries. */ @ExtendWith(JaasExtension.class) @@ -45,6 +47,20 @@ class QueryTaskCommentAccTest extends AbstractAccTest { assertThat(comments).hasSize(3); } + @WithAccessId(user = "user-1-1") + @Test + void should_ThrowException_When_NotAuthorizedToReadTaskComments() { + + ThrowingCallable call = + () -> + taskService + .createTaskCommentQuery() + .taskIdIn("TKI:000000000000000000000000000000000020") + .list(); + + assertThatThrownBy(call).isInstanceOf(NotAuthorizedToQueryWorkbasketException.class); + } + @WithAccessId(user = "admin") @Test void should_FilterTaskComments_For_CreatorIn() { @@ -70,30 +86,6 @@ class QueryTaskCommentAccTest extends AbstractAccTest { assertThat(comments).isEmpty(); } - @WithAccessId(user = "admin") - @Test - void should_FilterTaskComments_For_IdNotIn() { - List comments = - taskService - .createTaskCommentQuery() - .idNotIn( - "TCI:000000000000000000000000000000000000", - "TCI:000000000000000000000000000000000002") - .list(); - assertThat(comments).hasSize(11); - } - - @WithAccessId(user = "admin") - @Test - void should_FilterTaskComments_For_TaskIdNotIn() { - List comments = - taskService - .createTaskCommentQuery() - .taskIdNotIn("TKI:000000000000000000000000000000000000") - .list(); - assertThat(comments).hasSize(10); - } - @WithAccessId(user = "admin") @Test void should_FilterTaskComments_For_CreatorNotIn() { @@ -127,13 +119,6 @@ class QueryTaskCommentAccTest extends AbstractAccTest { assertThat(comments).hasSize(4); } - @WithAccessId(user = "admin") - @Test - void should_FilterTaskComments_For_TaskIdLike() { - List comments = taskService.createTaskCommentQuery().taskIdLike("%00026%").list(); - assertThat(comments).hasSize(2); - } - @WithAccessId(user = "admin") @Test void should_FilterTaskComments_For_TextFieldLike() { @@ -156,14 +141,6 @@ class QueryTaskCommentAccTest extends AbstractAccTest { assertThat(comments).hasSize(9); } - @WithAccessId(user = "admin") - @Test - void should_FilterTaskComments_For_TaskIdNotLike() { - List comments = - taskService.createTaskCommentQuery().taskIdNotLike("%00026%").list(); - assertThat(comments).hasSize(11); - } - @WithAccessId(user = "admin") @Test void should_FilterTaskComments_For_TextFieldNotLike() { @@ -234,7 +211,7 @@ class QueryTaskCommentAccTest extends AbstractAccTest { List listedValues = taskService .createTaskCommentQuery() - .listValues(TaskCommentQueryColumnName.CREATOR_LONG_NAME, null); + .listValues(TaskCommentQueryColumnName.CREATOR_FULL_NAME, null); assertThat(listedValues).hasSize(3); } @@ -277,7 +254,7 @@ class QueryTaskCommentAccTest extends AbstractAccTest { @WithAccessId(user = "admin") @Test - void should_ReturnSingleHistoryEvent_When_UsingSingleMethod() { + void should_ReturnSingleTaskComment_When_UsingSingleMethod() { TaskComment single = taskService .createTaskCommentQuery() @@ -309,7 +286,7 @@ class QueryTaskCommentAccTest extends AbstractAccTest { String creatorFullName = taskanaEngine.getUserService().getUser(taskComments.get(0).getCreator()).getFullName(); assertThat(taskComments.get(0)) - .extracting(TaskComment::getCreatorLongName) + .extracting(TaskComment::getCreatorFullName) .isEqualTo(creatorFullName); } @@ -324,7 +301,7 @@ class QueryTaskCommentAccTest extends AbstractAccTest { .list(); assertThat(taskComments).hasSize(1); - assertThat(taskComments.get(0)).extracting(TaskComment::getCreatorLongName).isNull(); + assertThat(taskComments.get(0)).extracting(TaskComment::getCreatorFullName).isNull(); } @WithAccessId(user = "admin") @@ -338,6 +315,6 @@ class QueryTaskCommentAccTest extends AbstractAccTest { .list(); assertThat(taskComments).hasSize(1); - assertThat(taskComments.get(0)).extracting(TaskComment::getCreatorLongName).isNull(); + assertThat(taskComments.get(0)).extracting(TaskComment::getCreatorFullName).isNull(); } } diff --git a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskCommentAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskCommentAccTest.java index a6ba5f793..9380738aa 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskCommentAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/UpdateTaskCommentAccTest.java @@ -51,7 +51,7 @@ class UpdateTaskCommentAccTest extends AbstractAccTest { List taskComments = taskService.getTaskComments("TKI:000000000000000000000000000000000000"); assertThat(taskComments).hasSize(3); - assertThat(taskComments.get(0).getTextField()).isEqualTo("some other text in textfield"); + assertThat(taskComments.get(0).getTextField()).isEqualTo("some text in textfield"); TaskComment taskComment = taskService.getTaskComment("TCI:000000000000000000000000000000000001"); @@ -64,7 +64,7 @@ class UpdateTaskCommentAccTest extends AbstractAccTest { List taskCommentsAfterUpdateAttempt = taskService.getTaskComments("TKI:000000000000000000000000000000000000"); assertThat(taskCommentsAfterUpdateAttempt.get(0).getTextField()) - .isEqualTo("some other text in textfield"); + .isEqualTo("some text in textfield"); } @WithAccessId(user = "user-1-1") diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskCommentQueryFilterParameter.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskCommentQueryFilterParameter.java index b6ef36a63..aeddb65bc 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskCommentQueryFilterParameter.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/task/rest/TaskCommentQueryFilterParameter.java @@ -38,26 +38,6 @@ public class TaskCommentQueryFilterParameter implements QueryParameter auth = new HttpEntity<>(RestHelper.generateHeadersForUser("user-1-1")); - ResponseEntity taskComments = - TEMPLATE.exchange(url, HttpMethod.GET, auth, TASK_COMMENT_PAGE_MODEL_TYPE); + ThrowingCallable httpCall = + () -> { + TEMPLATE.exchange(url, HttpMethod.GET, auth, TASK_COMMENT_PAGE_MODEL_TYPE); + }; - assertThat(taskComments.getBody().getContent()).isEmpty(); + assertThatThrownBy(httpCall) + .extracting(HttpStatusCodeException.class::cast) + .extracting(HttpStatusCodeException::getStatusCode) + .isEqualTo(HttpStatus.FORBIDDEN); } @Test diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/assembler/TaskCommentRepresentationModelAssemblerTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/assembler/TaskCommentRepresentationModelAssemblerTest.java index 3a73489dd..7555815e7 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/assembler/TaskCommentRepresentationModelAssemblerTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/task/rest/assembler/TaskCommentRepresentationModelAssemblerTest.java @@ -34,7 +34,7 @@ class TaskCommentRepresentationModelAssemblerTest { taskComment.setId("taskCommentId"); taskComment.setCreator("user-1-1"); - taskComment.setCreatorLongName("longName"); + taskComment.setCreatorFullName("fullName"); taskComment.setTextField("this is a task comment"); taskComment.setCreated(Instant.parse("2010-01-01T12:00:00Z")); taskComment.setModified(Instant.parse("2011-11-11T11:00:00Z")); @@ -52,7 +52,7 @@ class TaskCommentRepresentationModelAssemblerTest { repModel.setTaskId("TKI:000000000000000000000000000000000000"); repModel.setTaskCommentId("TCI:000000000000000000000000000000000000"); repModel.setCreator("user-1-1"); - repModel.setCreatorLongName("longName"); + repModel.setCreatorFullName("fullName"); repModel.setCreated(Instant.parse("2010-01-01T12:00:00Z")); repModel.setModified(Instant.parse("2011-11-11T11:00:00Z")); repModel.setTextField("textField"); @@ -68,7 +68,7 @@ class TaskCommentRepresentationModelAssemblerTest { (TaskCommentImpl) taskService.newTaskComment("TKI:000000000000000000000000000000000000"); taskComment.setId("taskCommentId"); taskComment.setCreator("user-1-1"); - taskComment.setCreatorLongName("longName"); + taskComment.setCreatorFullName("fullName"); taskComment.setTextField("this is a task comment"); taskComment.setCreated(Instant.parse("2010-01-01T12:00:00Z")); taskComment.setModified(Instant.parse("2011-11-11T11:00:00Z")); @@ -89,7 +89,7 @@ class TaskCommentRepresentationModelAssemblerTest { assertThat(taskComment.getTaskId()).isEqualTo(repModel.getTaskId()); assertThat(taskComment.getTextField()).isEqualTo(repModel.getTextField()); assertThat(taskComment.getCreator()).isEqualTo(repModel.getCreator()); - assertThat(taskComment.getCreatorLongName()).isEqualTo(repModel.getCreatorLongName()); + assertThat(taskComment.getCreatorFullName()).isEqualTo(repModel.getCreatorFullName()); assertThat(taskComment.getCreated()).isEqualTo(repModel.getCreated()); assertThat(taskComment.getModified()).isEqualTo(repModel.getModified()); }