Merge branch 'RED-8491-9346' into 'main'

RED-8491: Hide all KNECON_* roles for any possible access in all endpoints && RED-9346: User without roles not displayed in GET endpoint

See merge request fforesight/tenant-user-management-service!110

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<User> KNECON_ROLE_FILTER = user -> {
+        Set<String> 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))
+                        .anyMatch(mappedRoles::contains))
-                .collect(Collectors.toList());
+                .filter(KNECON_ROLE_FILTER)
+                .toList();
     }
 
 
@@ -70,19 +86,7 @@ public class UserController implements UserResource, PublicResource {
 
         return userService.getAllUsers()
                 .stream()
-                .filter(user -> {
+                .filter(KNECON_ROLE_FILTER)
-                    Set<String> filteredRoles = user.getRoles()
-                            .stream()
-                            .filter(ApplicationRoles::isNoKneconRole)
-                            .collect(Collectors.toSet());
-
-                    if (filteredRoles.isEmpty()) {
-                        return false;
-                    }
-
-                    user.setRoles(filteredRoles);
-                    return true;
-                })
                 .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");
         }
 

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;
 

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<String> KNECON_ROLES = Set.of(KNECON_ADMIN_ROLE, KNECON_SUPPORT_ROLE);
+    public static final Set<String> RED_ROLES = Set.of(RED_USER_ROLE, RED_MANAGER_ROLE, RED_ADMIN_ROLE, RED_USER_ADMIN_ROLE);
 
-    private static final Set<String> 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);
+    }
+
 }

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<User> 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
+        allUsers = userClient.getAllUsers(true);
-        assertEquals(allUsers.size(), sizeBefore + 2);
+        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<User> allUsers = userClient.getAllUsers(true);
+        assertTrue(allUsers.stream().anyMatch(u -> u.getUserId().equals(noRolesUser.getUserId())));
+
+        List<User> applicationSpecificUsers = userClient.getApplicationSpecificUsers(true);
+        assertFalse(applicationSpecificUsers.stream().anyMatch(u -> u.getUserId().equals(noRolesUser.getUserId())));
+
+
+    }
+
 }