From 119676e143c24ea1a900cf3ab1fe10cf7756de6f Mon Sep 17 00:00:00 2001 From: Maverick Studer Date: Thu, 13 Jun 2024 15:55:57 +0200 Subject: [PATCH] RED-8491: Hide all KNECON_* roles for any possible access in all endpoints && RED-9346: User without roles not displayed in GET endpoint --- .../controller/external/UserController.java | 38 +++++---- ...ContollerV2.java => UserControllerV2.java} | 2 +- .../permissions/ApplicationRoles.java | 21 ++++- .../tenantusermanagement/tests/UserTest.java | 81 +++++++++++++++++-- 4 files changed, 116 insertions(+), 26 deletions(-) rename src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/v2/{UserContollerV2.java => UserControllerV2.java} (96%) diff --git a/src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/UserController.java b/src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/UserController.java index 1d13327..3001da7 100644 --- a/src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/UserController.java +++ b/src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/UserController.java @@ -7,6 +7,7 @@ import static com.knecon.fforesight.tenantusermanagement.permissions.UserManagem import java.util.List; import java.util.Set; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; @@ -38,6 +39,20 @@ import lombok.extern.slf4j.Slf4j; @RequiredArgsConstructor public class UserController implements UserResource, PublicResource { + public static final Predicate KNECON_ROLE_FILTER = user -> { + Set filteredRoles = user.getRoles() + .stream() + .filter(ApplicationRoles::isNoKneconRole) + .collect(Collectors.toSet()); + + if (!user.getRoles().isEmpty() && filteredRoles.isEmpty()) { + return false; + } + + user.setRoles(filteredRoles); + return true; + }; + private final UserService userService; private final TenantUserManagementProperties tenantUserManagementProperties; @@ -49,14 +64,15 @@ public class UserController implements UserResource, PublicResource { if (bypassCache) { userService.evictUserCache(); } + var mappedRoles = tenantUserManagementProperties.getKcRoleMapping().getAllRoles(); - var allRoles = tenantUserManagementProperties.getKcRoleMapping().getAllRoles(); return userService.getAllUsers() .stream() .filter(user -> user.getRoles() .stream() - .anyMatch(allRoles::contains)) - .collect(Collectors.toList()); + .anyMatch(mappedRoles::contains)) + .filter(KNECON_ROLE_FILTER) + .toList(); } @@ -70,19 +86,7 @@ public class UserController implements UserResource, PublicResource { return userService.getAllUsers() .stream() - .filter(user -> { - Set filteredRoles = user.getRoles() - .stream() - .filter(ApplicationRoles::isNoKneconRole) - .collect(Collectors.toSet()); - - if (filteredRoles.isEmpty()) { - return false; - } - - user.setRoles(filteredRoles); - return true; - }) + .filter(KNECON_ROLE_FILTER) .toList(); } @@ -142,7 +146,7 @@ public class UserController implements UserResource, PublicResource { .filter(ApplicationRoles::isNoKneconRole) .collect(Collectors.toSet()); - if (filteredRoles.isEmpty()) { + if (!user.getRoles().isEmpty() && filteredRoles.isEmpty()) { throw new ResponseStatusException(HttpStatus.NOT_FOUND, "User not found"); } diff --git a/src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/v2/UserContollerV2.java b/src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/v2/UserControllerV2.java similarity index 96% rename from src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/v2/UserContollerV2.java rename to src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/v2/UserControllerV2.java index 557fb8a..6e774a9 100644 --- a/src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/v2/UserContollerV2.java +++ b/src/main/java/com/knecon/fforesight/tenantusermanagement/controller/external/v2/UserControllerV2.java @@ -19,7 +19,7 @@ import lombok.extern.slf4j.Slf4j; @RestController @RequiredArgsConstructor @Tag(name = "6. Users endpoints", description = "Operations related to users.") -public class UserContollerV2 implements UserResourceV2, PublicResourceV2 { +public class UserControllerV2 implements UserResourceV2, PublicResourceV2 { private final UserController userController; diff --git a/src/main/java/com/knecon/fforesight/tenantusermanagement/permissions/ApplicationRoles.java b/src/main/java/com/knecon/fforesight/tenantusermanagement/permissions/ApplicationRoles.java index e33faac..488e48f 100644 --- a/src/main/java/com/knecon/fforesight/tenantusermanagement/permissions/ApplicationRoles.java +++ b/src/main/java/com/knecon/fforesight/tenantusermanagement/permissions/ApplicationRoles.java @@ -6,17 +6,36 @@ public final class ApplicationRoles { public static final String KNECON_ADMIN_ROLE = "KNECON_ADMIN"; public static final String KNECON_SUPPORT_ROLE = "KNECON_SUPPORT"; + public static final String RED_USER_ROLE = "RED_USER"; + public static final String RED_MANAGER_ROLE = "RED_MANAGER"; + public static final String RED_ADMIN_ROLE = "RED_ADMIN"; + public static final String RED_USER_ADMIN_ROLE = "RED_USER_ADMIN"; + + public static final Set KNECON_ROLES = Set.of(KNECON_ADMIN_ROLE, KNECON_SUPPORT_ROLE); + public static final Set RED_ROLES = Set.of(RED_USER_ROLE, RED_MANAGER_ROLE, RED_ADMIN_ROLE, RED_USER_ADMIN_ROLE); - private static final Set KNECON_ROLES = Set.of(KNECON_ADMIN_ROLE, KNECON_SUPPORT_ROLE); public static boolean isNoKneconRole(String role) { return !KNECON_ROLES.contains(role); } + public static boolean isKneconRole(String role) { return KNECON_ROLES.contains(role); } + + public static boolean isNoREDRole(String role) { + + return !RED_ROLES.contains(role); + } + + + public static boolean isREDRole(String role) { + + return RED_ROLES.contains(role); + } + } diff --git a/src/test/java/com/knecon/fforesight/tenantusermanagement/tests/UserTest.java b/src/test/java/com/knecon/fforesight/tenantusermanagement/tests/UserTest.java index 7030575..70c2419 100644 --- a/src/test/java/com/knecon/fforesight/tenantusermanagement/tests/UserTest.java +++ b/src/test/java/com/knecon/fforesight/tenantusermanagement/tests/UserTest.java @@ -9,6 +9,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -377,8 +378,6 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest { .filter(ApplicationRoles::isKneconRole) .collect(Collectors.toSet()); - var sizeBefore = userClient.getAllUsers(true).size(); - // create several users with different roles for testing var createUserRequest = new CreateUserRequest(); createUserRequest.setEmail("allroles@knecon.com"); @@ -405,8 +404,36 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest { User onlyKneconUser = userClient.createUser(createUserRequest3); var allUsers = userClient.getAllUsers(true); - // one role is completely hidden - assertEquals(allUsers.size(), sizeBefore + 2); + + // should not contain the knecon user but the others + assertTrue(allUsers.stream().anyMatch(u -> u.getUserId().equals(user.getUserId()))); + assertTrue(allUsers.stream().anyMatch(u -> u.getUserId().equals(noKneconUser.getUserId()))); + assertFalse(allUsers.stream().anyMatch(u -> u.getUserId().equals(onlyKneconUser.getUserId()))); + var foundUser = allUsers.stream() + .filter(u -> u.getUserId().equals(user.getUserId())) + .findFirst(); + assertTrue(foundUser.isPresent()); + assertFalse(foundUser.get().getRoles().stream().anyMatch(ApplicationRoles::isKneconRole)); + + // should not be found + var e = assertThrows(FeignException.class, () -> userClient.getUserById(onlyKneconUser.getUserId())); + assertEquals(404, e.status()); + + // should not show any knecon roles + User userById = userClient.getUserById(user.getUserId()); + assertTrue(userById.getRoles().stream().noneMatch(ApplicationRoles::isKneconRole)); + + List applicationSpecificUsers = userClient.getApplicationSpecificUsers(true); + + // should not contain the knecon user but the others + assertTrue(applicationSpecificUsers.stream().anyMatch(u -> u.getUserId().equals(user.getUserId()))); + assertTrue(applicationSpecificUsers.stream().anyMatch(u -> u.getUserId().equals(noKneconUser.getUserId()))); + assertFalse(applicationSpecificUsers.stream().anyMatch(u -> u.getUserId().equals(onlyKneconUser.getUserId()))); + foundUser = applicationSpecificUsers.stream() + .filter(u -> u.getUserId().equals(user.getUserId())) + .findFirst(); + assertTrue(foundUser.isPresent()); + assertFalse(foundUser.get().getRoles().stream().anyMatch(ApplicationRoles::isKneconRole)); // no knecon roles are visible assertEquals(allUsers.stream().filter(u -> u.getRoles().stream().anyMatch(ApplicationRoles::isKneconRole)).count(), 0); @@ -415,7 +442,7 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest { userClient.getUserById(noKneconUser.getUserId()); // but not this one as he only has knecon roles - var e = assertThrows(FeignException.class, () -> userClient.getUserById(onlyKneconUser.getUserId())); + e = assertThrows(FeignException.class, () -> userClient.getUserById(onlyKneconUser.getUserId())); assertEquals(404, e.status()); // switch token to the user without knecon roles @@ -492,12 +519,52 @@ public class UserTest extends AbstractTenantUserManagementIntegrationTest { // and again using the bulk call userClient.deleteUsers(List.of(user4.getUserId())); - // user should still be present despite deletes as it as a knecon user - assertEquals(allUsers.size(), sizeBefore + 2); + allUsers = userClient.getAllUsers(true); + assertTrue(allUsers.stream().anyMatch(u -> u.getUserId().equals(user4.getUserId()))); // and should not have changed var user5 = userClient.getUserById(user4.getUserId()); assertEquals(user4, user5); } + + + @Test + public void testShowUserWithoutRoles() { + + // set context and user + TenantContext.setTenantId(AbstractTenantUserManagementIntegrationTest.TEST_TENANT_ID); + tokenService.setUser("admin@knecon.com", "secret"); + + // different role sets and subsets + var allRoles = tenantUserManagementProperties.getKcRoleMapping().getAllRoles(); + + // create several users with different roles for testing + var createUserRequest = new CreateUserRequest(); + createUserRequest.setEmail("someotheruser@knecon.com"); + createUserRequest.setFirstName("Some Other"); + createUserRequest.setLastName("User"); + createUserRequest.setUsername("SomeOtherUser"); + createUserRequest.setRoles(allRoles); + User user = userClient.createUser(createUserRequest); + + var createUserRequest2 = new CreateUserRequest(); + createUserRequest2.setEmail("noroles@notknecon.com"); + createUserRequest2.setFirstName("No"); + createUserRequest2.setLastName("Roles"); + createUserRequest2.setUsername("NoRolesAtAll"); + createUserRequest2.setRoles(new HashSet<>()); + User noRolesUser = userClient.createUser(createUserRequest2); + + User userById = userClient.getUserById(noRolesUser.getUserId()); + + List allUsers = userClient.getAllUsers(true); + assertTrue(allUsers.stream().anyMatch(u -> u.getUserId().equals(noRolesUser.getUserId()))); + + List applicationSpecificUsers = userClient.getApplicationSpecificUsers(true); + assertFalse(applicationSpecificUsers.stream().anyMatch(u -> u.getUserId().equals(noRolesUser.getUserId()))); + + + } + }