From 90a64a9b881110483b99d2b0f446a3ed1a601520 Mon Sep 17 00:00:00 2001 From: Norman Schmidt <60552466+norman-schmidt@users.noreply.github.com> Date: Thu, 22 Sep 2022 12:04:00 +0200 Subject: [PATCH] TSK-1957: extend getUser() method with domain information - extended WorkbasketQuery methods 'accessIdsHavePermission' & 'callerHasPermission' to accept multiple permissions - added domain attribute to User API - extended UserService to aggregate domain information Co-authored-by: Mustapha Zorgati <15628173+mustaphazorgati@users.noreply.github.com> --- .../acceptance/user/UserServiceAccTest.java | 635 +++++++++++------- .../task/internal/TaskTransferrer.java | 2 +- .../exceptions/UserAlreadyExistException.java | 5 +- .../pro/taskana/user/api/models/User.java | 14 + .../pro/taskana/user/internal/UserMapper.java | 5 +- .../user/internal/UserServiceImpl.java | 70 +- .../user/internal/models/UserImpl.java | 21 +- .../workbasket/api/WorkbasketQuery.java | 44 +- .../internal/WorkbasketQueryImpl.java | 31 +- .../internal/WorkbasketQueryMapper.java | 22 +- .../query/QueryWorkbasketAccTest.java | 4 +- .../QueryWorkbasketByPermissionAccTest.java | 185 +++-- .../query/WorkbasketQueryAccTest.java | 20 +- .../rest/WorkbasketQueryFilterParameter.java | 6 +- .../rest/WorkbasketControllerIntTest.java | 8 +- 15 files changed, 701 insertions(+), 371 deletions(-) diff --git a/lib/taskana-core-test/src/test/java/acceptance/user/UserServiceAccTest.java b/lib/taskana-core-test/src/test/java/acceptance/user/UserServiceAccTest.java index 88c5dfd79..538188f2e 100644 --- a/lib/taskana-core-test/src/test/java/acceptance/user/UserServiceAccTest.java +++ b/lib/taskana-core-test/src/test/java/acceptance/user/UserServiceAccTest.java @@ -2,320 +2,483 @@ package acceptance.user; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.catchThrowableOfType; +import static pro.taskana.common.internal.util.CheckedSupplier.wrap; +import static pro.taskana.testapi.DefaultTestEntities.defaultTestWorkbasket; +import static pro.taskana.testapi.DefaultTestEntities.randomTestUser; +import static pro.taskana.testapi.builder.UserBuilder.newUser; import java.util.Arrays; import java.util.Iterator; +import java.util.List; import java.util.stream.Stream; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DynamicTest; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestFactory; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestInstance.Lifecycle; import org.junit.jupiter.api.function.ThrowingConsumer; +import pro.taskana.TaskanaEngineConfiguration; +import pro.taskana.common.api.TaskanaEngine; import pro.taskana.common.api.TaskanaRole; import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.MismatchedRoleException; +import pro.taskana.testapi.TaskanaEngineConfigurationModifier; import pro.taskana.testapi.TaskanaInject; import pro.taskana.testapi.TaskanaIntegrationTest; +import pro.taskana.testapi.builder.WorkbasketAccessItemBuilder; import pro.taskana.testapi.security.WithAccessId; import pro.taskana.user.api.UserService; import pro.taskana.user.api.exceptions.UserAlreadyExistException; import pro.taskana.user.api.exceptions.UserNotFoundException; import pro.taskana.user.api.models.User; +import pro.taskana.workbasket.api.WorkbasketPermission; +import pro.taskana.workbasket.api.WorkbasketService; +import pro.taskana.workbasket.api.models.Workbasket; +import pro.taskana.workbasket.api.models.WorkbasketAccessItem; /** Acceptance test which tests the functionality of the UserService. */ @TaskanaIntegrationTest class UserServiceAccTest { + @TaskanaInject WorkbasketService workbasketService; @TaskanaInject UserService userService; + @TaskanaInject TaskanaEngine taskanaEngine; - protected User createExampleUser(String id) { - User user = userService.newUser(); - user.setId(id); - user.setFirstName("Hans"); - user.setLastName("Georg"); - user.setFullName("Georg, Hans"); - user.setLongName("Georg, Hans - (user-10-20)"); - user.setEmail("hans.georg@web.com"); - user.setPhone("1234"); - user.setMobilePhone("01574275632"); - user.setOrgLevel4("level4"); - user.setOrgLevel3("level3"); - user.setOrgLevel2("level2"); - user.setOrgLevel1("level1"); - user.setData("ab"); + private WorkbasketAccessItem createAccessItem( + User user, Workbasket workbasket, WorkbasketPermission... permissions) throws Exception { + WorkbasketAccessItemBuilder builder = + WorkbasketAccessItemBuilder.newWorkbasketAccessItem() + .accessId(user.getId()) + .workbasketId(workbasket.getId()); + for (WorkbasketPermission permission : permissions) { + builder.permission(permission); + } - return user; + return builder.buildAndStore(workbasketService, "businessadmin"); } - @WithAccessId(user = "admin") - @BeforeAll - void setup() throws Exception { - User testuser1 = userService.newUser(); - testuser1.setId("testuser1"); - testuser1.setFirstName("Max"); - testuser1.setLastName("Mustermann"); - testuser1.setFullName("Max, Mustermann"); - testuser1.setLongName("Max, Mustermann - (testuser1)"); - testuser1.setEmail("max.mustermann@web.com"); - testuser1.setPhone("040-2951854"); - testuser1.setMobilePhone("015637683197"); - testuser1.setOrgLevel4("Novatec"); - testuser1.setOrgLevel3("BPM"); - testuser1.setOrgLevel2("Human Workflow"); - testuser1.setOrgLevel1("TASKANA"); - testuser1.setData(""); - userService.createUser(testuser1); + @Nested + @TestInstance(Lifecycle.PER_CLASS) + class GetUser { - User testuser2 = userService.newUser(); - testuser2.setId("testuser2"); - testuser2.setFirstName("Elena"); - testuser2.setLastName("Eifrig"); - testuser2.setFullName("Elena, Eifrig"); - testuser2.setLongName("Elena, Eifrig - (testuser2)"); - testuser2.setEmail("elena.eifrig@web.com"); - testuser2.setPhone("040-2951854"); - testuser2.setMobilePhone("015637683197"); - testuser2.setOrgLevel4("Novatec"); - testuser2.setOrgLevel3("BPM"); - testuser2.setOrgLevel2("Human Workflow"); - testuser2.setOrgLevel1("TASKANA"); - testuser2.setData(""); - userService.createUser(testuser2); + @WithAccessId(user = "user-1-1") + @Test + void should_ThrowUserNotFoundException_When_TryingToGetUserWithNonExistingId() { + ThrowingCallable callable = () -> userService.getUser("NOT_EXISTING"); + + assertThatThrownBy(callable) + .isInstanceOf(UserNotFoundException.class) + .extracting(UserNotFoundException.class::cast) + .extracting(UserNotFoundException::getUserId) + .isEqualTo("NOT_EXISTING"); + } + + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnUserWithAllFields_When_TryingToGetUserWithIdExisting() throws Exception { + final User userToGet = + newUser() + .id("max-mustermann") + .firstName("Max") + .lastName("Mustermann") + .fullName("Max Mustermann") + .longName("Mustermann, Max") + .email("max@mustermann.de") + .phone("123456798") + .mobilePhone("987654321") + .orgLevel1("org1") + .orgLevel2("org2") + .orgLevel3("org3") + .orgLevel4("org4") + .data("this is some extra data about max") + .buildAndStore(userService, "businessadmin"); + + User userInDatabase = userService.getUser(userToGet.getId()); + + assertThat(userInDatabase).hasNoNullFieldsOrProperties().isEqualTo(userToGet); + } } - @WithAccessId(user = "user-1-2") - @Test - void should_ReturnUserWithAllFields_When_IdExisting() throws Exception { - User userInDatabase = userService.getUser("testuser1"); + @Nested + @TestInstance(Lifecycle.PER_CLASS) + class CreateUser { - User userToCompare = userService.newUser(); - userToCompare.setId("testuser1"); - userToCompare.setFirstName("Max"); - userToCompare.setLastName("Mustermann"); - userToCompare.setFullName("Max, Mustermann"); - userToCompare.setLongName("Max, Mustermann - (testuser1)"); - userToCompare.setEmail("max.mustermann@web.com"); - userToCompare.setPhone("040-2951854"); - userToCompare.setMobilePhone("015637683197"); - userToCompare.setOrgLevel4("Novatec"); - userToCompare.setOrgLevel3("BPM"); - userToCompare.setOrgLevel2("Human Workflow"); - userToCompare.setOrgLevel1("TASKANA"); - userToCompare.setData(""); + @WithAccessId(user = "businessadmin") + @Test + void should_InsertUserInDatabase_When_CreatingUser() throws Exception { + User userToCreate = userService.newUser(); + userToCreate.setId("anton"); + userToCreate.setFirstName("Anton"); + userToCreate.setLastName("Miller"); + userService.createUser(userToCreate); - assertThat(userInDatabase).hasNoNullFieldsOrProperties().isEqualTo(userToCompare); + User userInDatabase = userService.getUser(userToCreate.getId()); + assertThat(userToCreate).isNotSameAs(userInDatabase).isEqualTo(userInDatabase); + } + + @WithAccessId(user = "businessadmin") + @Test + void should_SetTheLongAndFullNameAccordingToRules_When_CreatingUserWithThoseFieldsEmpty() + throws Exception { + User userToCreate = userService.newUser(); + userToCreate.setId("martina"); + userToCreate.setFirstName("Martina"); + userToCreate.setLastName("Schmidt"); + + User createdUser = userService.createUser(userToCreate); + + assertThat(createdUser.getLongName()).isEqualTo("Schmidt, Martina - (martina)"); + assertThat(createdUser.getFullName()).isEqualTo("Schmidt, Martina"); + } + + @WithAccessId(user = "businessadmin") + @Test + void should_ThrowInvalidArgumentException_When_TryingToCreateUserWithFirstNameNull() { + User userToCreate = userService.newUser(); + userToCreate.setId("user-1"); + userToCreate.setFirstName(null); + userToCreate.setLastName("Schmidt"); + + ThrowingCallable callable = () -> userService.createUser(userToCreate); + + assertThatThrownBy(callable) + .isInstanceOf(InvalidArgumentException.class) + .hasMessage("First and last name of User must be set or empty."); + } + + @WithAccessId(user = "businessadmin") + @Test + void should_ThrowInvalidArgumentException_When_TryingToCreateUserWithLastNameNull() { + User userToCreate = userService.newUser(); + userToCreate.setId("user-2"); + userToCreate.setFirstName("User 1"); + userToCreate.setLastName(null); + + ThrowingCallable callable = () -> userService.createUser(userToCreate); + + assertThatThrownBy(callable) + .isInstanceOf(InvalidArgumentException.class) + .hasMessage("First and last name of User must be set or empty."); + } + + @WithAccessId(user = "businessadmin") + @TestFactory + Stream should_ThrowInvalidArgumentException_When_TryingToCreateUserWithNotSetId() { + Iterator iterator = Arrays.asList("", null).iterator(); + + ThrowingConsumer test = + userId -> { + User userToCreate = userService.newUser(); + userToCreate.setFirstName("firstName"); + userToCreate.setLastName("lastName"); + userToCreate.setId(userId); + + ThrowingCallable callable = () -> userService.createUser(userToCreate); + + assertThatThrownBy(callable) + .isInstanceOf(InvalidArgumentException.class) + .hasMessage("UserId must not be empty when creating User."); + }; + + return DynamicTest.stream(iterator, c -> "for " + c, test); + } + + @WithAccessId(user = "businessadmin") + @Test + void should_ThrowUserAlreadyExistException_When_TryingToCreateUserWithExistingId() + throws Exception { + User existingUser = randomTestUser().buildAndStore(userService); + + ThrowingCallable callable = + () -> { + User userToCreate = userService.newUser(); + userToCreate.setId(existingUser.getId()); + userToCreate.setFirstName("firstName"); + userToCreate.setLastName("lastName"); + + userService.createUser(userToCreate); + }; + + assertThatThrownBy(callable) + .isInstanceOf(UserAlreadyExistException.class) + .extracting(UserAlreadyExistException.class::cast) + .extracting(UserAlreadyExistException::getUserId) + .isEqualTo(existingUser.getId()); + } + + @WithAccessId(user = "user-1-1") + @Test + void should_ThrowNotAuthorizedException_When_TryingToCreateUserWithoutAdminRole() { + User userToCreate = userService.newUser(); + userToCreate.setId("user-3"); + userToCreate.setFirstName("firstName"); + userToCreate.setLastName("lastName"); + ThrowingCallable callable = () -> userService.createUser(userToCreate); + + MismatchedRoleException ex = catchThrowableOfType(callable, MismatchedRoleException.class); + assertThat(ex.getCurrentUserId()).isEqualTo("user-1-1"); + assertThat(ex.getRoles()) + .isEqualTo(new TaskanaRole[] {TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN}); + } } - @WithAccessId(user = "user-1-2") - @Test - void should_ThrowUserNotFoundException_When_TryingToGetUserWithNonExistingId() { - ThrowingCallable callable = () -> userService.getUser("NOT_EXISTING"); + @Nested + @TestInstance(Lifecycle.PER_CLASS) + class UpdateUser { - assertThatThrownBy(callable) - .isInstanceOf(UserNotFoundException.class) - .hasFieldOrPropertyWithValue("userId", "NOT_EXISTING") - .hasMessage("User with id 'NOT_EXISTING' was not found."); + @WithAccessId(user = "businessadmin") + @Test + void should_UpdateUserInDatabase_When_IdExisting() throws Exception { + User userToUpdate = randomTestUser().buildAndStore(userService); + + userToUpdate.setFirstName("Anton"); + userService.updateUser(userToUpdate); + + User userInDatabase = userService.getUser(userToUpdate.getId()); + assertThat(userInDatabase).isNotSameAs(userToUpdate).isEqualTo(userToUpdate); + } + + @WithAccessId(user = "user-1-1") + @Test + void should_ThrowNotAuthorizedException_When_TryingToUpdateUserWithNoAdminRole() + throws Exception { + User userToUpdate = randomTestUser().buildAndStore(userService, "businessadmin"); + + ThrowingCallable callable = + () -> { + userToUpdate.setLastName("updated last name"); + userService.updateUser(userToUpdate); + }; + + MismatchedRoleException ex = catchThrowableOfType(callable, MismatchedRoleException.class); + assertThat(ex.getCurrentUserId()).isEqualTo("user-1-1"); + assertThat(ex.getRoles()) + .isEqualTo(new TaskanaRole[] {TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN}); + } + + @WithAccessId(user = "businessadmin") + @Test + void should_ThrowUserNotFoundException_When_TryingToUpdateUserWithNonExistingId() { + User userToUpdate = userService.newUser(); + userToUpdate.setId("user-4"); + userToUpdate.setFirstName("firstName"); + userToUpdate.setLastName("lastName"); + + ThrowingCallable callable = () -> userService.updateUser(userToUpdate); + + assertThatThrownBy(callable) + .isInstanceOf(UserNotFoundException.class) + .extracting(UserNotFoundException.class::cast) + .extracting(UserNotFoundException::getUserId) + .isEqualTo(userToUpdate.getId()); + } } - @WithAccessId(user = "admin") - @Test - void should_InsertUserInDatabase_When_CreatingUser() throws Exception { - User userToCreate = createExampleUser("user-10-20"); + @Nested + @TestInstance(Lifecycle.PER_CLASS) + class DeleteUser { - userService.createUser(userToCreate); + @WithAccessId(user = "businessadmin") + @Test + void should_DeleteUserFromDatabase_When_IdExisting() throws Exception { + User userToDelete = randomTestUser().buildAndStore(userService); - User userInDatabse = userService.getUser(userToCreate.getId()); + userService.deleteUser(userToDelete.getId()); - assertThat(userToCreate) - .hasNoNullFieldsOrProperties() - .isNotSameAs(userInDatabse) - .isEqualTo(userInDatabse); + // Validate that user is indeed deleted + ThrowingCallable callable = () -> userService.getUser(userToDelete.getId()); + assertThatThrownBy(callable) + .isInstanceOf(UserNotFoundException.class) + .extracting(UserNotFoundException.class::cast) + .extracting(UserNotFoundException::getUserId) + .isEqualTo(userToDelete.getId()); + } + + @WithAccessId(user = "businessadmin") + @Test + void should_ThrowUserNotFoundException_When_TryingToDeleteUserWithNonExistingId() { + ThrowingCallable callable = () -> userService.deleteUser("NOT_EXISTING"); + + assertThatThrownBy(callable) + .isInstanceOf(UserNotFoundException.class) + .extracting(UserNotFoundException.class::cast) + .extracting(UserNotFoundException::getUserId) + .isEqualTo("NOT_EXISTING"); + } + + @WithAccessId(user = "user-1-1") + @Test + void should_ThrowNotAuthorizedException_When_TryingToDeleteUserWithNoAdminRole() + throws Exception { + User userToDelete = randomTestUser().buildAndStore(userService, "businessadmin"); + + ThrowingCallable callable = () -> userService.deleteUser(userToDelete.getId()); + + MismatchedRoleException ex = catchThrowableOfType(callable, MismatchedRoleException.class); + assertThat(ex.getCurrentUserId()).isEqualTo("user-1-1"); + assertThat(ex.getRoles()) + .isEqualTo(new TaskanaRole[] {TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN}); + } } - @WithAccessId(user = "admin") - @Test - void should_SetTheLongAndFullNameAccordingToRules_When_CreatingUserWithThoseFieldsEmpty() - throws Exception { - User userToCreate = createExampleUser("user-10-21"); - userToCreate.setLongName(null); - userToCreate.setFullName(null); + @Nested + @TestInstance(Lifecycle.PER_CLASS) + class DynamicDomainComputation { - String fullName = userToCreate.getLastName() + ", " + userToCreate.getFirstName(); - String longName = - userToCreate.getLastName() - + ", " - + userToCreate.getFirstName() - + " - (" - + userToCreate.getId() - + ")"; + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnEmptyDomains_When_UserHasInsufficientMinimalPermissionsToAssignDomains() + throws Exception { + User user = randomTestUser().buildAndStore(userService, "businessadmin"); + Workbasket workbasket = + defaultTestWorkbasket().buildAndStore(workbasketService, "businessadmin"); + createAccessItem(user, workbasket, WorkbasketPermission.READ); - User createdUser = userService.createUser(userToCreate); + User userInDatabase = userService.getUser(user.getId()); - assertThat(createdUser.getLongName()).isEqualTo(longName); - assertThat(createdUser.getFullName()).isEqualTo(fullName); - } + assertThat(userInDatabase.getDomains()).isEmpty(); + } - @WithAccessId(user = "admin") - @Test - void should_ThrowInvalidArgumentException_When_TryingToCreateUserWithFirstOrLastNameNull() - throws Exception { - User userToCreate = createExampleUser("user-10-20"); - userToCreate.setFirstName(null); + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnEmptyDomains_When_UserHasNoPermissions() throws Exception { + User user = randomTestUser().buildAndStore(userService, "businessadmin"); + defaultTestWorkbasket().buildAndStore(workbasketService, "businessadmin"); - ThrowingCallable callable = () -> userService.createUser(userToCreate); + User userInDatabase = userService.getUser(user.getId()); - assertThatThrownBy(callable) - .isInstanceOf(InvalidArgumentException.class) - .hasMessage("First and last name of User must be set or empty."); - } + assertThat(userInDatabase.getDomains()).isEmpty(); + } - @WithAccessId(user = "admin") - @Test - void should_ThrowInvalidArgumentException_When_TryingToCreateUserWithLastNameNull() { - User userToCreate = createExampleUser("user-10-20"); - userToCreate.setLastName(null); + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnOneDomain_When_UserHasSufficientMinimalPermissionsToAssignDomains() + throws Exception { + User user = randomTestUser().buildAndStore(userService, "businessadmin"); + Workbasket workbasket = + defaultTestWorkbasket().buildAndStore(workbasketService, "businessadmin"); + createAccessItem(user, workbasket, WorkbasketPermission.OPEN, WorkbasketPermission.READ); - ThrowingCallable callable = () -> userService.createUser(userToCreate); + User userInDatabase = userService.getUser(user.getId()); - assertThatThrownBy(callable) - .isInstanceOf(InvalidArgumentException.class) - .hasMessage("First and last name of User must be set or empty."); - } + assertThat(userInDatabase.getDomains()).containsExactly(workbasket.getDomain()); + } - @WithAccessId(user = "admin") - @TestFactory - Stream should_ThrowInvalidArgumentException_When_TryingToCreateUserWithNotSetId() - throws Exception { - Iterator iterator = Arrays.asList("", null).iterator(); + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnEmptyDomains_When_UserHasSufficientPermissionsWhichThenGetRevoked() + throws Exception { + User user = randomTestUser().buildAndStore(userService, "businessadmin"); + Workbasket workbasket = + defaultTestWorkbasket().buildAndStore(workbasketService, "businessadmin"); + WorkbasketAccessItem wai = + createAccessItem(user, workbasket, WorkbasketPermission.OPEN, WorkbasketPermission.READ); - ThrowingConsumer test = - userId -> { - User userToCreate = createExampleUser("user-10-20"); - userToCreate.setId(userId); + User userInDatabase = userService.getUser(user.getId()); - ThrowingCallable callable = () -> userService.createUser(userToCreate); + assertThat(userInDatabase.getDomains()).containsExactly(workbasket.getDomain()); - assertThatThrownBy(callable) - .isInstanceOf(InvalidArgumentException.class) - .hasMessage("UserId must not be empty when creating User."); - }; + // then permission gets revoked - return DynamicTest.stream(iterator, c -> "for " + c, test); - } + wai.setPermission(WorkbasketPermission.OPEN, false); + taskanaEngine.runAsAdmin(wrap(() -> workbasketService.updateWorkbasketAccessItem(wai))); - @WithAccessId(user = "admin") - @Test - void should_ThrowUserAlreadyExistException_When_TryingToCreateUserWithExistingId() { - User userToCreate = createExampleUser("testuser1"); // existing userId + userInDatabase = userService.getUser(user.getId()); - ThrowingCallable callable = () -> userService.createUser(userToCreate); + assertThat(userInDatabase.getDomains()).isEmpty(); + } - assertThatThrownBy(callable) - .isInstanceOf(UserAlreadyExistException.class) - .hasFieldOrPropertyWithValue("userId", "testuser1") - .hasMessage("User with id 'testuser1' already exists."); - } + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnMultipleDomains_When_UserHasSufficientMinimalPermissionsForMultipleDomains() + throws Exception { + User user = randomTestUser().buildAndStore(userService, "businessadmin"); + Workbasket workbasket1 = + defaultTestWorkbasket().buildAndStore(workbasketService, "businessadmin"); + Workbasket workbasket2 = + defaultTestWorkbasket() + .domain("DOMAIN_B") + .buildAndStore(workbasketService, "businessadmin"); + createAccessItem(user, workbasket1, WorkbasketPermission.OPEN, WorkbasketPermission.READ); + createAccessItem(user, workbasket2, WorkbasketPermission.OPEN, WorkbasketPermission.READ); - @WithAccessId(user = "user-1-2") - @Test - void should_ThrowNotAuthorizedException_When_TryingToCreateUserWithoutAdminRole() { - User userToCreate = createExampleUser("user-10-22"); + User userInDatabase = userService.getUser(user.getId()); - ThrowingCallable callable = () -> userService.createUser(userToCreate); + assertThat(userInDatabase.getDomains()) + .containsExactlyInAnyOrder(workbasket1.getDomain(), workbasket2.getDomain()); + } - assertThatThrownBy(callable) - .isInstanceOf(MismatchedRoleException.class) - .hasFieldOrPropertyWithValue("currentUserId", "user-1-2") - .hasFieldOrPropertyWithValue( - "roles", new TaskanaRole[] {TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN}) - .hasMessage( - "Not authorized. The current user 'user-1-2' is not member of role(s) " - + "'[BUSINESS_ADMIN, ADMIN]'."); - } + @Nested + @TestInstance(Lifecycle.PER_CLASS) + class DifferentMinimalPermissionsToAssignDomains implements TaskanaEngineConfigurationModifier { - @WithAccessId(user = "admin") - @Test - void should_UpdateUserInDatabase_When_IdExisting() throws Exception { - User userToUpdate = createExampleUser("testuser1"); // existing userId + @TaskanaInject UserService userService; + @TaskanaInject WorkbasketService workbasketService; - userService.updateUser(userToUpdate); + @Override + public void modify(TaskanaEngineConfiguration taskanaEngineConfiguration) { + taskanaEngineConfiguration.setMinimalPermissionsToAssignDomains( + List.of(WorkbasketPermission.APPEND)); + } - User userInDatabase = userService.getUser("testuser1"); + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnEmptyDomains_When_UserHasInsufficientMinimalPermissionsToAssignDomains() + throws Exception { + User user = randomTestUser().buildAndStore(userService, "businessadmin"); + Workbasket workbasket = + defaultTestWorkbasket().buildAndStore(workbasketService, "businessadmin"); + createAccessItem(user, workbasket, WorkbasketPermission.OPEN, WorkbasketPermission.READ); - assertThat(userToUpdate).isNotSameAs(userInDatabase).isEqualTo(userInDatabase); - } + User userInDatabase = userService.getUser(user.getId()); - @WithAccessId(user = "admin") - @Test - void should_ThrowUserNotFoundException_When_TryingToUpdateUserWithNonExistingId() { - User userToUpdate = createExampleUser("NOT_EXISTING"); + assertThat(userInDatabase.getDomains()).isEmpty(); + } - ThrowingCallable callable = () -> userService.updateUser(userToUpdate); + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnOneDomain_When_UserHasSufficientMinimalPermissionsToAssignDomains() + throws Exception { + User user = randomTestUser().buildAndStore(userService, "businessadmin"); + Workbasket workbasket = + defaultTestWorkbasket().buildAndStore(workbasketService, "businessadmin"); + createAccessItem(user, workbasket, WorkbasketPermission.APPEND); - assertThatThrownBy(callable) - .isInstanceOf(UserNotFoundException.class) - .hasFieldOrPropertyWithValue("userId", "NOT_EXISTING") - .hasMessage("User with id 'NOT_EXISTING' was not found."); - } + User userInDatabase = userService.getUser(user.getId()); - @WithAccessId(user = "user-1-2") - @Test - void should_ThrowNotAuthorizedException_When_TryingToUpdateUserWithNoAdminRole() { - User userToUpdate = createExampleUser("testuser1"); // existing userId + assertThat(userInDatabase.getDomains()).isEmpty(); + } + } - ThrowingCallable callable = () -> userService.updateUser(userToUpdate); + @Nested + @TestInstance(Lifecycle.PER_CLASS) + class PropertyMinimalPermissionsToAssignDomainsIsNotSet + implements TaskanaEngineConfigurationModifier { - assertThatThrownBy(callable) - .isInstanceOf(MismatchedRoleException.class) - .hasFieldOrPropertyWithValue("currentUserId", "user-1-2") - .hasFieldOrPropertyWithValue( - "roles", new TaskanaRole[] {TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN}) - .hasMessage( - "Not authorized. The current user 'user-1-2' is not member of role(s) " - + "'[BUSINESS_ADMIN, ADMIN]'."); - } + @TaskanaInject UserService userService; + @TaskanaInject WorkbasketService workbasketService; - @WithAccessId(user = "admin") - @Test - void should_DeleteUserFromDatabase_When_IdExisting() throws Exception { - String id = "testuser2"; - userService.getUser(id); // User existing + @Override + public void modify(TaskanaEngineConfiguration taskanaEngineConfiguration) { + taskanaEngineConfiguration.setMinimalPermissionsToAssignDomains(null); + } - userService.deleteUser(id); + @WithAccessId(user = "user-1-1") + @Test + void should_ReturnEmptyDomains_When_PropertyIsNotSet() throws Exception { + User user = randomTestUser().buildAndStore(userService, "businessadmin"); + Workbasket workbasket = + defaultTestWorkbasket().buildAndStore(workbasketService, "businessadmin"); + createAccessItem(user, workbasket, WorkbasketPermission.OPEN, WorkbasketPermission.READ); - ThrowingCallable callable = () -> userService.getUser(id); // User deleted + User userInDatabase = userService.getUser(user.getId()); - assertThatThrownBy(callable) - .isInstanceOf(UserNotFoundException.class) - .hasFieldOrPropertyWithValue("userId", "testuser2") - .hasMessage("User with id 'testuser2' was not found."); - } - - @WithAccessId(user = "admin") - @Test - void should_ThrowUserNotFoundException_When_TryingToDeleteUserWithNonExistingId() { - ThrowingCallable callable = () -> userService.deleteUser("NOT_EXISTING"); - - assertThatThrownBy(callable) - .isInstanceOf(UserNotFoundException.class) - .hasFieldOrPropertyWithValue("userId", "NOT_EXISTING") - .hasMessage("User with id 'NOT_EXISTING' was not found."); - } - - @WithAccessId(user = "user-1-2") - @Test - void should_ThrowNotAuthorizedException_When_TryingToDeleteUserWithNoAdminRole() { - ThrowingCallable callable = () -> userService.deleteUser("testuser1"); - - assertThatThrownBy(callable) - .isInstanceOf(MismatchedRoleException.class) - .hasFieldOrPropertyWithValue("currentUserId", "user-1-2") - .hasFieldOrPropertyWithValue( - "roles", new TaskanaRole[] {TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN}) - .hasMessage( - "Not authorized. The current user 'user-1-2' is not member of role(s) " - + "'[BUSINESS_ADMIN, ADMIN]'."); + assertThat(userInDatabase.getDomains()).isEmpty(); + } + } } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskTransferrer.java b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskTransferrer.java index 8249518e1..94621b525 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskTransferrer.java +++ b/lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskTransferrer.java @@ -238,7 +238,7 @@ final class TaskTransferrer { .toArray(String[]::new); List sourceWorkbaskets = - query.callerHasPermission(WorkbasketPermission.TRANSFER).idIn(workbasketIds).list(); + query.callerHasPermissions(WorkbasketPermission.TRANSFER).idIn(workbasketIds).list(); return sourceWorkbaskets.stream().map(WorkbasketSummary::getId).collect(Collectors.toSet()); } diff --git a/lib/taskana-core/src/main/java/pro/taskana/user/api/exceptions/UserAlreadyExistException.java b/lib/taskana-core/src/main/java/pro/taskana/user/api/exceptions/UserAlreadyExistException.java index 4d5b794ba..5e57fd6c4 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/user/api/exceptions/UserAlreadyExistException.java +++ b/lib/taskana-core/src/main/java/pro/taskana/user/api/exceptions/UserAlreadyExistException.java @@ -13,10 +13,11 @@ public class UserAlreadyExistException extends TaskanaException { public static final String ERROR_KEY = "USER_ALREADY_EXISTS"; private final String userId; - public UserAlreadyExistException(String userId) { + public UserAlreadyExistException(String userId, Exception cause) { super( String.format("User with id '%s' already exists.", userId), - ErrorCode.of(ERROR_KEY, MapCreator.of("userId", userId))); + ErrorCode.of(ERROR_KEY, MapCreator.of("userId", userId)), + cause); this.userId = userId; } diff --git a/lib/taskana-core/src/main/java/pro/taskana/user/api/models/User.java b/lib/taskana-core/src/main/java/pro/taskana/user/api/models/User.java index c80e532c1..685a8fdff 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/user/api/models/User.java +++ b/lib/taskana-core/src/main/java/pro/taskana/user/api/models/User.java @@ -1,5 +1,9 @@ package pro.taskana.user.api.models; +import java.util.Set; + +import pro.taskana.TaskanaEngineConfiguration; + /** The User holds some relevant information about the TASKANA users. */ public interface User { @@ -185,6 +189,16 @@ public interface User { */ void setData(String data); + /** + * Returns the domains of the User. + * + *

The domains are derived from the workbasket permissions and the according Taskana property + * {@linkplain TaskanaEngineConfiguration#getMinimalPermissionsToAssignDomains()}. + * + * @return domains + */ + Set getDomains(); + /** * Duplicates this User. * diff --git a/lib/taskana-core/src/main/java/pro/taskana/user/internal/UserMapper.java b/lib/taskana-core/src/main/java/pro/taskana/user/internal/UserMapper.java index db56b5d8a..be523aafe 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/user/internal/UserMapper.java +++ b/lib/taskana-core/src/main/java/pro/taskana/user/internal/UserMapper.java @@ -7,6 +7,7 @@ import org.apache.ibatis.annotations.Result; import org.apache.ibatis.annotations.SelectProvider; import org.apache.ibatis.annotations.UpdateProvider; +import pro.taskana.user.api.models.User; import pro.taskana.user.internal.models.UserImpl; public interface UserMapper { @@ -27,10 +28,10 @@ public interface UserMapper { UserImpl findById(@Param("id") String id); @InsertProvider(type = UserMapperSqlProvider.class, method = "insert") - void insert(UserImpl user); + void insert(User user); @UpdateProvider(type = UserMapperSqlProvider.class, method = "update") - void update(UserImpl user); + void update(User user); @DeleteProvider(type = UserMapperSqlProvider.class, method = "delete") void delete(String id); diff --git a/lib/taskana-core/src/main/java/pro/taskana/user/internal/UserServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/user/internal/UserServiceImpl.java index 98ed5fcc6..b5e41e58a 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/user/internal/UserServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/user/internal/UserServiceImpl.java @@ -1,9 +1,16 @@ package pro.taskana.user.internal; +import static pro.taskana.common.internal.util.CheckedSupplier.wrap; + +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import org.apache.ibatis.exceptions.PersistenceException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import pro.taskana.common.api.BaseQuery.SortDirection; import pro.taskana.common.api.TaskanaRole; import pro.taskana.common.api.exceptions.InvalidArgumentException; import pro.taskana.common.api.exceptions.NotAuthorizedException; @@ -13,16 +20,23 @@ import pro.taskana.user.api.exceptions.UserAlreadyExistException; import pro.taskana.user.api.exceptions.UserNotFoundException; import pro.taskana.user.api.models.User; import pro.taskana.user.internal.models.UserImpl; +import pro.taskana.workbasket.api.WorkbasketPermission; +import pro.taskana.workbasket.api.WorkbasketQueryColumnName; +import pro.taskana.workbasket.api.WorkbasketService; public class UserServiceImpl implements UserService { private static final Logger LOGGER = LoggerFactory.getLogger(UserServiceImpl.class); - - private final InternalTaskanaEngine taskanaEngine; + private final InternalTaskanaEngine internalTaskanaEngine; private final UserMapper userMapper; + private final WorkbasketService workbasketService; + private final List minimalWorkbasketPermissions; - public UserServiceImpl(InternalTaskanaEngine taskanaEngine, UserMapper userMapper) { - this.taskanaEngine = taskanaEngine; + public UserServiceImpl(InternalTaskanaEngine internalTaskanaEngine, UserMapper userMapper) { + this.internalTaskanaEngine = internalTaskanaEngine; this.userMapper = userMapper; + this.workbasketService = internalTaskanaEngine.getEngine().getWorkbasketService(); + minimalWorkbasketPermissions = + internalTaskanaEngine.getEngine().getConfiguration().getMinimalPermissionsToAssignDomains(); } @Override @@ -32,18 +46,22 @@ public class UserServiceImpl implements UserService { @Override public User getUser(String id) throws UserNotFoundException { - User user = taskanaEngine.executeInDatabaseConnection(() -> userMapper.findById(id)); + UserImpl user = + internalTaskanaEngine.executeInDatabaseConnection(() -> userMapper.findById(id)); if (user == null) { throw new UserNotFoundException(id); } + user.setDomains(determineDomains(user.getId())); return user; } @Override public User createUser(User userToCreate) throws InvalidArgumentException, NotAuthorizedException, UserAlreadyExistException { - taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); + internalTaskanaEngine + .getEngine() + .checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); validateAndPopulateFields(userToCreate); insertIntoDatabase(userToCreate); @@ -56,10 +74,12 @@ public class UserServiceImpl implements UserService { @Override public User updateUser(User userToUpdate) throws UserNotFoundException, NotAuthorizedException { - taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); + internalTaskanaEngine + .getEngine() + .checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); getUser(userToUpdate.getId()); - taskanaEngine.executeInDatabaseConnection(() -> userMapper.update((UserImpl) userToUpdate)); + internalTaskanaEngine.executeInDatabaseConnection(() -> userMapper.update(userToUpdate)); if (LOGGER.isDebugEnabled()) { LOGGER.debug("Method updateUser() updated User '{}'.", userToUpdate); } @@ -69,23 +89,45 @@ public class UserServiceImpl implements UserService { @Override public void deleteUser(String id) throws UserNotFoundException, NotAuthorizedException { - taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); + internalTaskanaEngine + .getEngine() + .checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); getUser(id); - taskanaEngine.executeInDatabaseConnection(() -> userMapper.delete(id)); + internalTaskanaEngine.executeInDatabaseConnection(() -> userMapper.delete(id)); if (LOGGER.isDebugEnabled()) { LOGGER.debug("Method deleteUser() deleted User with id '{}'.", id); } } + private Set determineDomains(String... accessIds) { + if (minimalWorkbasketPermissions != null && !minimalWorkbasketPermissions.isEmpty()) { + // since WorkbasketService#accessIdsHavePermissions requires some role permissions we have to + // execute this query as an admin. Since we're only determining the domains of a given user + // (and any user can request information on any other user) this query is "harmless". + return new HashSet<>( + internalTaskanaEngine + .getEngine() + .runAsAdmin( + wrap( + () -> + workbasketService + .createWorkbasketQuery() + .accessIdsHavePermissions(minimalWorkbasketPermissions, accessIds) + .listValues( + WorkbasketQueryColumnName.DOMAIN, SortDirection.ASCENDING)))); + } + return Collections.emptySet(); + } + private void insertIntoDatabase(User userToCreate) throws UserAlreadyExistException { try { - taskanaEngine.openConnection(); - userMapper.insert((UserImpl) userToCreate); + internalTaskanaEngine.openConnection(); + userMapper.insert(userToCreate); } catch (PersistenceException e) { - throw new UserAlreadyExistException(userToCreate.getId()); + throw new UserAlreadyExistException(userToCreate.getId(), e); } finally { - taskanaEngine.returnConnection(); + internalTaskanaEngine.returnConnection(); } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/user/internal/models/UserImpl.java b/lib/taskana-core/src/main/java/pro/taskana/user/internal/models/UserImpl.java index 34482628a..9bfa19d4e 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/user/internal/models/UserImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/user/internal/models/UserImpl.java @@ -1,6 +1,8 @@ package pro.taskana.user.internal.models; +import java.util.HashSet; import java.util.Objects; +import java.util.Set; import pro.taskana.user.api.models.User; @@ -18,6 +20,7 @@ public class UserImpl implements User { private String orgLevel2; private String orgLevel1; private String data; + private Set domains = new HashSet<>(); public UserImpl() {} @@ -35,6 +38,7 @@ public class UserImpl implements User { this.orgLevel2 = copyFrom.orgLevel2; this.orgLevel1 = copyFrom.orgLevel1; this.data = copyFrom.data; + this.domains = copyFrom.domains; } @Override @@ -167,6 +171,15 @@ public class UserImpl implements User { this.data = data; } + @Override + public Set getDomains() { + return domains; + } + + public void setDomains(Set domains) { + this.domains = domains; + } + @Override public UserImpl copy() { return new UserImpl(this); @@ -187,7 +200,8 @@ public class UserImpl implements User { orgLevel3, orgLevel2, orgLevel1, - data); + data, + domains); } @Override @@ -214,7 +228,8 @@ public class UserImpl implements User { && Objects.equals(orgLevel3, other.orgLevel3) && Objects.equals(orgLevel2, other.orgLevel2) && Objects.equals(orgLevel1, other.orgLevel1) - && Objects.equals(data, other.data); + && Objects.equals(data, other.data) + && Objects.equals(domains, other.domains); } @Override @@ -245,6 +260,8 @@ public class UserImpl implements User { + orgLevel1 + ", data=" + data + + ", domains=" + + domains + "]"; } } diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/WorkbasketQuery.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/WorkbasketQuery.java index 0f5486d44..874de1874 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/WorkbasketQuery.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/api/WorkbasketQuery.java @@ -1,5 +1,7 @@ package pro.taskana.workbasket.api; +import java.util.List; + import pro.taskana.common.api.BaseQuery; import pro.taskana.common.api.TimeInterval; import pro.taskana.common.api.exceptions.InvalidArgumentException; @@ -143,16 +145,37 @@ public interface WorkbasketQuery extends BaseQuery - * The AccessIds and the given permission will throw a Exception if they would be NULL. + * The AccessIds and the given permission will throw an Exception if they would be NULL. * * @param permission which should be used for results. - * @param accessIds Users which sould be checked for given permissions on workbaskets. + * @param accessIds Users which should be checked for given permissions on workbaskets. * @return the current query object. * @throws InvalidArgumentException if permission OR the accessIds are NULL. * @throws NotAuthorizedException if the current user is not member of role BUSINESS_ADMIN or * ADMIN + * @deprecated Use {@linkplain #accessIdsHavePermissions(List, String...)} instead */ - WorkbasketQuery accessIdsHavePermission(WorkbasketPermission permission, String... accessIds) + @Deprecated(forRemoval = true) + default WorkbasketQuery accessIdsHavePermission( + WorkbasketPermission permission, String... accessIds) + throws InvalidArgumentException, NotAuthorizedException { + return accessIdsHavePermissions(List.of(permission), accessIds); + } + + /** + * Setting up the permissions which should be granted on the result workbaskets and the users + * which should be checked. READ permission will always be checked by default.
+ * The AccessIds and the given permission will throw an Exception if they would be NULL. + * + * @param permissions which should be used for results. + * @param accessIds Users which should be checked for given permissions on workbaskets. + * @return the current query object. + * @throws InvalidArgumentException if permissions OR the accessIds are NULL or empty. + * @throws NotAuthorizedException if the current user is not member of role BUSINESS_ADMIN or + * ADMIN + */ + WorkbasketQuery accessIdsHavePermissions( + List permissions, String... accessIds) throws InvalidArgumentException, NotAuthorizedException; /** @@ -161,8 +184,21 @@ public interface WorkbasketQuery extends BaseQuery orderBy; + private final List orderColumns; private WorkbasketQueryColumnName columnName; private String[] accessId; private String[] idIn; - private WorkbasketPermission permission; + private WorkbasketPermission[] permissions; private String[] nameIn; private String[] nameLike; private String[] keyIn; @@ -64,10 +67,6 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { private String[] orgLevel4In; private String[] orgLevel4Like; private boolean markedForDeletion; - - private InternalTaskanaEngine taskanaEngine; - private List orderBy; - private List orderColumns; private boolean joinWithAccessList; private boolean checkReadPermission; private boolean usedToAugmentTasks; @@ -165,22 +164,22 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { } @Override - public WorkbasketQuery accessIdsHavePermission( - WorkbasketPermission permission, String... accessIds) + public WorkbasketQuery accessIdsHavePermissions( + List permissions, String... accessIds) throws InvalidArgumentException, NotAuthorizedException { taskanaEngine .getEngine() .checkRoleMembership(TaskanaRole.ADMIN, TaskanaRole.BUSINESS_ADMIN, TaskanaRole.TASK_ADMIN); // Checking pre-conditions - if (permission == null) { - throw new InvalidArgumentException("Permission can't be null."); + if (permissions == null || permissions.isEmpty()) { + throw new InvalidArgumentException("Permissions can't be null or empty."); } if (accessIds == null || accessIds.length == 0) { throw new InvalidArgumentException("accessIds can't be NULL or empty."); } // set up permissions and ids - this.permission = permission; + this.permissions = permissions.toArray(WorkbasketPermission[]::new); this.accessId = accessIds; lowercaseAccessIds(this.accessId); @@ -188,8 +187,8 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { } @Override - public WorkbasketQuery callerHasPermission(WorkbasketPermission permission) { - this.permission = permission; + public WorkbasketQuery callerHasPermissions(WorkbasketPermission... permissions) { + this.permissions = permissions; return this; } @@ -428,8 +427,8 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { return accessId; } - public WorkbasketPermission getPermission() { - return permission; + public WorkbasketPermission[] getPermissions() { + return permissions; } public String[] getNameIn() { @@ -625,7 +624,7 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { } else if (taskanaEngine.getEngine().isUserInRole(TaskanaRole.BUSINESS_ADMIN) && !usedToAugmentTasks) { checkReadPermission = false; - if (accessId == null && permission == null) { + if (accessId == null && (permissions == null || permissions.length == 0)) { joinWithAccessList = false; } } @@ -659,7 +658,7 @@ public class WorkbasketQueryImpl implements WorkbasketQuery { + ", idIn=" + Arrays.toString(idIn) + ", permission=" - + permission + + Arrays.toString(permissions) + ", nameIn=" + Arrays.toString(nameIn) + ", nameLike=" diff --git a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketQueryMapper.java b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketQueryMapper.java index e9addd1a7..ae109df97 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketQueryMapper.java +++ b/lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketQueryMapper.java @@ -69,10 +69,11 @@ public interface WorkbasketQueryMapper { + " " + "AND (a.MAX_READ = 1 " + " " - + "AND " + + "AND " + " " + "( " + " " + + "" + "a.MAX_READ " + "a.MAX_OPEN " + "a.MAX_APPEND" @@ -90,6 +91,7 @@ public interface WorkbasketQueryMapper { + "a.MAX_CUSTOM_10" + "a.MAX_CUSTOM_11" + "a.MAX_CUSTOM_12 = 1 " + + "" + ")" + "" + "" @@ -211,10 +213,11 @@ public interface WorkbasketQueryMapper { + " " + "AND (a.MAX_READ = 1 " + " " - + "AND " + + "AND " + " " + "( " + " " + + "" + "a.MAX_READ " + "a.MAX_OPEN " + "a.MAX_APPEND" @@ -232,6 +235,7 @@ public interface WorkbasketQueryMapper { + "a.MAX_CUSTOM_10" + "a.MAX_CUSTOM_11" + "a.MAX_CUSTOM_12 = 1 " + + "" + ")" + "" + "" @@ -306,10 +310,11 @@ public interface WorkbasketQueryMapper { + " " + "AND (a.MAX_READ = 1 " + " " - + "AND " + + "AND " + " " + "( " + " " + + "" + "a.MAX_READ " + "a.MAX_OPEN " + "a.MAX_APPEND" @@ -326,15 +331,8 @@ public interface WorkbasketQueryMapper { + "a.MAX_CUSTOM_9" + "a.MAX_CUSTOM_10" + "a.MAX_CUSTOM_11" - + "a.MAX_CUSTOM_12" - + "" - + "" - + "= TRUE " - + "" - + "" - + "= 1 " - + "" - + "" + + "a.MAX_CUSTOM_12 = 1 " + + "" + ")" + "" + "" diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/query/QueryWorkbasketAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/query/QueryWorkbasketAccTest.java index 9d894badf..0ea080a52 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/query/QueryWorkbasketAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/query/QueryWorkbasketAccTest.java @@ -317,8 +317,8 @@ class QueryWorkbasketAccTest extends AbstractAccTest { WORKBASKET_SERVICE .createWorkbasketQuery() .nameLike("%") - .accessIdsHavePermission( - WorkbasketPermission.TRANSFER, "teamlead-1", GROUP_1_DN, GROUP_2_DN) + .accessIdsHavePermissions( + List.of(WorkbasketPermission.TRANSFER), "teamlead-1", GROUP_1_DN, GROUP_2_DN) .orderByName(DESCENDING) .list(); diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/query/QueryWorkbasketByPermissionAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/query/QueryWorkbasketByPermissionAccTest.java index 197e35d4a..8f2799ff7 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/query/QueryWorkbasketByPermissionAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/query/QueryWorkbasketByPermissionAccTest.java @@ -7,6 +7,7 @@ import acceptance.AbstractAccTest; import java.util.List; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; import pro.taskana.common.api.BaseQuery.SortDirection; @@ -21,90 +22,122 @@ import pro.taskana.workbasket.api.models.WorkbasketSummary; @ExtendWith(JaasExtension.class) class QueryWorkbasketByPermissionAccTest extends AbstractAccTest { - private static SortDirection asc = SortDirection.ASCENDING; - private static SortDirection desc = SortDirection.DESCENDING; + private static final WorkbasketService WORKBASKET_SERVICE = taskanaEngine.getWorkbasketService(); - QueryWorkbasketByPermissionAccTest() { - super(); - } + // region accessIdsHavePermission @WithAccessId(user = "businessadmin") @Test - void testQueryAllTransferTargetsForUser() throws Exception { - WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + void should_GetAllTransferTargetsForUser_When_QueryingForSinglePermission() throws Exception { List results = - workbasketService + WORKBASKET_SERVICE .createWorkbasketQuery() - .accessIdsHavePermission(WorkbasketPermission.APPEND, "user-1-1") + .accessIdsHavePermissions(List.of(WorkbasketPermission.APPEND), "user-1-1") .list(); + assertThat(results).hasSize(1); assertThat(results.get(0).getKey()).isEqualTo("USER-1-1"); } + @WithAccessId(user = "businessadmin") + @Test + void should_GetAllTransferTargetsForUser_When_QueryingForMultiplePermissions() throws Exception { + List results = + WORKBASKET_SERVICE + .createWorkbasketQuery() + .accessIdsHavePermissions( + List.of(WorkbasketPermission.APPEND, WorkbasketPermission.DISTRIBUTE), "teamlead-1") + .list(); + + assertThat(results) + .extracting(WorkbasketSummary::getKey) + .hasSize(2) + .containsExactlyInAnyOrder("GPK_KSC", "TEAMLEAD-1"); + } + @WithAccessId(user = "unknownuser") @Test - void testQueryAllTransferTargetsForUserNotAuthorized() { - WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + void should_ThrowNotAuthorizedException_When_QueryingWithUnknownUser() { ThrowingCallable call = - () -> { - workbasketService - .createWorkbasketQuery() - .accessIdsHavePermission(WorkbasketPermission.APPEND, "user-1-1") - .list(); - }; + () -> + WORKBASKET_SERVICE + .createWorkbasketQuery() + .accessIdsHavePermissions(List.of(WorkbasketPermission.APPEND), "user-1-1") + .list(); + assertThatThrownBy(call).isInstanceOf(NotAuthorizedException.class); } @WithAccessId(user = "businessadmin") @Test - void testQueryAllTransferTargetsForUserAndGroup() throws Exception { - WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + void should_GetAllTransferTargetsForUserAndGroup_When_QueryingForSinglePermission() + throws Exception { List results = - workbasketService + WORKBASKET_SERVICE .createWorkbasketQuery() - .accessIdsHavePermission(WorkbasketPermission.APPEND, "user-1-1", GROUP_1_DN) + .accessIdsHavePermissions(List.of(WorkbasketPermission.APPEND), "user-1-1", GROUP_1_DN) .list(); + assertThat(results).hasSize(6); } @WithAccessId(user = "businessadmin") @Test - void testQueryAllTransferTargetsForUserAndGroupSortedByNameAscending() throws Exception { - WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + void should_GetAllTransferTargetsForUserAndGroup_When_QueryingForMultiplePermissions() + throws Exception { List results = - workbasketService + WORKBASKET_SERVICE .createWorkbasketQuery() - .accessIdsHavePermission(WorkbasketPermission.APPEND, "user-1-1", GROUP_1_DN) - .orderByName(asc) + .accessIdsHavePermissions( + List.of(WorkbasketPermission.APPEND, WorkbasketPermission.OPEN), + "user-1-1", + GROUP_1_DN) .list(); + + assertThat(results).hasSize(4); + } + + @WithAccessId(user = "businessadmin") + @Test + void should_GetAllTransferTargetsForUserAndGroup_When_QueryingForSortedByNameAscending() + throws Exception { + List results = + WORKBASKET_SERVICE + .createWorkbasketQuery() + .accessIdsHavePermissions(List.of(WorkbasketPermission.APPEND), "user-1-1", GROUP_1_DN) + .orderByName(SortDirection.ASCENDING) + .list(); + assertThat(results).hasSize(6); assertThat(results.get(0).getKey()).isEqualTo("GPK_KSC_1"); } @WithAccessId(user = "businessadmin") @Test - void testQueryAllTransferTargetsForUserAndGroupSortedByNameDescending() throws Exception { - WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + void should_GetAllTransferTargetsForUserAndGroup_When_QueryingForSortedByNameDescending() + throws Exception { List results = - workbasketService + WORKBASKET_SERVICE .createWorkbasketQuery() - .accessIdsHavePermission(WorkbasketPermission.APPEND, "user-1-1", GROUP_1_DN) - .orderByName(desc) - .orderByKey(asc) + .accessIdsHavePermissions(List.of(WorkbasketPermission.APPEND), "user-1-1", GROUP_1_DN) + .orderByName(SortDirection.DESCENDING) + .orderByKey(SortDirection.ASCENDING) .list(); + assertThat(results).hasSize(6); assertThat(results.get(0).getKey()).isEqualTo("USER-2-2"); } @WithAccessId(user = "businessadmin") @Test - void testQueryAllTransferSourcesForUserAndGroup() throws Exception { - WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + void should_GetAllTransferSourcesForUserAndGroup_When_QueryingForSinglePermission() + throws Exception { List results = - workbasketService + WORKBASKET_SERVICE .createWorkbasketQuery() - .accessIdsHavePermission(WorkbasketPermission.DISTRIBUTE, "user-1-1", GROUP_1_DN) + .accessIdsHavePermissions( + List.of(WorkbasketPermission.DISTRIBUTE), "user-1-1", GROUP_1_DN) .list(); assertThat(results) @@ -112,51 +145,83 @@ class QueryWorkbasketByPermissionAccTest extends AbstractAccTest { .containsExactlyInAnyOrder("GPK_KSC_1", "USER-1-1"); } - @WithAccessId(user = "user-1-1", groups = GROUP_1_DN) - @Test - void testQueryAllTransferTargetsForUserAndGroupFromSubject() { - WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); - List results = - workbasketService - .createWorkbasketQuery() - .callerHasPermission(WorkbasketPermission.APPEND) - .list(); - assertThat(results).hasSize(6); - } + // endregion + + // region callerHasPermission @WithAccessId(user = "user-1-1") @Test - void testQueryAllAvailableWorkbasketForOpeningForUserAndGroupFromSubject() { - WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + void should_GetAllTransferTargetsForSubjectUser_When_QueryingForSinglePermission() { List results = - workbasketService + WORKBASKET_SERVICE .createWorkbasketQuery() - .callerHasPermission(WorkbasketPermission.READ) + .callerHasPermissions(WorkbasketPermission.READ) .list(); + assertThat(results).hasSize(1); } @WithAccessId(user = "teamlead-1") @Test - void testConsiderBusinessAdminPermissionsWhileQueryingWorkbaskets() { - WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + void should_GetAllTransferTargetsForSubjectUser_When_QueryingForMultiplePermission() { List results = - workbasketService + WORKBASKET_SERVICE .createWorkbasketQuery() - .callerHasPermission(WorkbasketPermission.OPEN) + .callerHasPermissions(WorkbasketPermission.READ, WorkbasketPermission.OPEN) .list(); + assertThat(results).hasSize(3); } - @WithAccessId(user = "admin") + @WithAccessId(user = "user-1-1", groups = GROUP_1_DN) @Test - void testSkipAuthorizationCheckForAdminWhileQueryingWorkbaskets() { - WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + void should_GetAllTransferTargetsForSubjectUserAndGroup_When_QueryingForSinglePermission() { List results = - workbasketService + WORKBASKET_SERVICE .createWorkbasketQuery() - .callerHasPermission(WorkbasketPermission.OPEN) + .callerHasPermissions(WorkbasketPermission.APPEND) .list(); + + assertThat(results).hasSize(6); + } + + @WithAccessId(user = "user-1-1", groups = GROUP_1_DN) + @Test + void should_GetAllTransferTargetsForSubjectUserAndGroup_When_QueryingForMultiplePermissions() { + List results = + WORKBASKET_SERVICE + .createWorkbasketQuery() + .callerHasPermissions(WorkbasketPermission.APPEND, WorkbasketPermission.OPEN) + .list(); + + assertThat(results).hasSize(4); + } + + @WithAccessId(user = "businessadmin") + @Test + void should_ConsiderBusinessAdminPermissions_When_QueryingWorkbaskets() { + List filteredWorkbaskets = + WORKBASKET_SERVICE + .createWorkbasketQuery() + .callerHasPermissions(WorkbasketPermission.OPEN) + .list(); + + assertThat(filteredWorkbaskets).isEmpty(); + } + + @WithAccessId(user = "admin") + @WithAccessId(user = "taskadmin") + @TestTemplate + void should_SkipAuthorizationCheckForAdmin_When_QueryingWorkbaskets() { + List results = + WORKBASKET_SERVICE + .createWorkbasketQuery() + .callerHasPermissions(WorkbasketPermission.OPEN) + .list(); + assertThat(results).hasSize(26); } + + // endregion + } diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/query/WorkbasketQueryAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/query/WorkbasketQueryAccTest.java index 11c995f40..52cbc9d7a 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/query/WorkbasketQueryAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/query/WorkbasketQueryAccTest.java @@ -20,10 +20,6 @@ import pro.taskana.workbasket.api.models.WorkbasketSummary; @ExtendWith(JaasExtension.class) class WorkbasketQueryAccTest extends AbstractAccTest { - WorkbasketQueryAccTest() { - super(); - } - @Test void testQueryWorkbasketByUnauthenticated() { WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); @@ -35,8 +31,8 @@ class WorkbasketQueryAccTest extends AbstractAccTest { workbasketService .createWorkbasketQuery() .nameLike("%") - .accessIdsHavePermission( - WorkbasketPermission.TRANSFER, "teamlead-1", GROUP_1_DN, GROUP_2_DN) + .accessIdsHavePermissions( + List.of(WorkbasketPermission.TRANSFER), "teamlead-1", GROUP_1_DN, GROUP_2_DN) .list(); }; assertThatThrownBy(call).isInstanceOf(NotAuthorizedException.class); @@ -54,8 +50,8 @@ class WorkbasketQueryAccTest extends AbstractAccTest { workbasketService .createWorkbasketQuery() .nameLike("%") - .accessIdsHavePermission( - WorkbasketPermission.TRANSFER, "teamlead-1", GROUP_1_DN, GROUP_2_DN) + .accessIdsHavePermissions( + List.of(WorkbasketPermission.TRANSFER), "teamlead-1", GROUP_1_DN, GROUP_2_DN) .list(); }; assertThatThrownBy(call).isInstanceOf(NotAuthorizedException.class); @@ -73,8 +69,8 @@ class WorkbasketQueryAccTest extends AbstractAccTest { workbasketService .createWorkbasketQuery() .nameLike("%") - .accessIdsHavePermission( - WorkbasketPermission.TRANSFER, "teamlead-1", GROUP_1_DN, GROUP_2_DN) + .accessIdsHavePermissions( + List.of(WorkbasketPermission.TRANSFER), "teamlead-1", GROUP_1_DN, GROUP_2_DN) .list(); assertThat(results).hasSize(13); @@ -92,8 +88,8 @@ class WorkbasketQueryAccTest extends AbstractAccTest { workbasketService .createWorkbasketQuery() .nameLike("%") - .accessIdsHavePermission( - WorkbasketPermission.TRANSFER, "teamlead-1", GROUP_1_DN, GROUP_2_DN) + .accessIdsHavePermissions( + List.of(WorkbasketPermission.TRANSFER), "teamlead-1", GROUP_1_DN, GROUP_2_DN) .list(); assertThat(results).hasSize(13); diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketQueryFilterParameter.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketQueryFilterParameter.java index d4beba3c5..7194efa6b 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketQueryFilterParameter.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketQueryFilterParameter.java @@ -65,7 +65,7 @@ public class WorkbasketQueryFilterParameter implements QueryParameter response = TEMPLATE.exchange( - URLDecoder.decode(url, "UTF-8"), + URLDecoder.decode(url, StandardCharsets.UTF_8), HttpMethod.GET, auth, ParameterizedTypeReference.forType(WorkbasketRepresentationModel.class)); assertThat(response.getBody()).isNotNull(); - assertThat(response.getBody().getLink(IanaLinkRelations.SELF)).isNotNull(); - assertThat(response.getBody().getLink(IanaLinkRelations.SELF)) - .isEqualTo(Optional.of(Link.of(url))); + assertThat(response.getBody().getLink(IanaLinkRelations.SELF)).contains(Link.of(url)); assertThat(response.getHeaders().getContentType()).isEqualTo(MediaTypes.HAL_JSON); }