Scope signing user picker to team for multi-tenant SaaS (#6583)

2026-07-16 11:23:10 +02:00 · 2026-06-15 13:44:34 +01:00
parent 2a905c01c3
commit 9d7467cf90
6 changed files with 234 additions and 7 deletions
@@ -978,20 +978,35 @@ public class UserController {
        }
    }

-    /**
-     * List all enabled users for selection in signing workflows.
-     *
-     * @param principal The authenticated user
-     * @return List of user summaries
-     */
+    // Lists enabled users for the signing user picker, scoped by storage.signing.userListScope:
+    // 'org' (default) = whole instance, anything else = caller's team only (fail-closed).
    @GetMapping("/users")
    public ResponseEntity<List<UserSummaryDTO>> listUsers(Principal principal) {
        if (principal == null) {
            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
        }

+        // Fail-closed: only literal "org" opens the whole instance; anything else scopes to team.
+        String scope = applicationProperties.getStorage().getSigning().getUserListScope();
+        boolean teamScoped = !"org".equalsIgnoreCase(scope == null ? "" : scope.trim());
+
+        List<User> source;
+        if (teamScoped) {
+            Optional<User> callerOpt = userService.findByUsernameIgnoreCase(principal.getName());
+            if (callerOpt.isEmpty() || callerOpt.get().getTeam() == null) {
+                // No team: return only the caller rather than leak the org.
+                source = callerOpt.map(List::of).orElse(List.of());
+            } else {
+                // KNOWN LIMITATION: scopes the team via the single User.team FK - correct while
+                // acceptInvitation() collapses users to one team; revisit if multi-team enabled.
+                source = userRepository.findAllByTeamId(callerOpt.get().getTeam().getId());
+            }
+        } else {
+            source = userRepository.findAll();
+        }
+
        List<UserSummaryDTO> users =
-                userRepository.findAll().stream()
+                source.stream()
                        .filter(User::isEnabled)
                        .map(this::toUserSummaryDTO)
                        .collect(java.util.stream.Collectors.toList());
@@ -1,13 +1,17 @@
 package stirling.software.proprietary.security.controller.api;

+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

+import java.util.List;
 import java.util.Optional;

 import org.junit.jupiter.api.BeforeEach;
@@ -150,4 +154,186 @@ class UserControllerTest {

        verify(loginAttemptService).resetAttempts("lockeduser");
    }
+
+    // ---------------------------------------------------------------------
+    // GET /api/v1/user/users - storage.signing.userListScope scoping
+    // ---------------------------------------------------------------------
+
+    private static User user(long id, String username, boolean enabled, Team team) {
+        User u = new User();
+        u.setId(id);
+        u.setUsername(username);
+        u.setEnabled(enabled);
+        u.setTeam(team);
+        return u;
+    }
+
+    private static Team team(long id, String name) {
+        Team t = new Team();
+        t.setId(id);
+        t.setName(name);
+        return t;
+    }
+
+    private static Authentication auth(String username) {
+        return new UsernamePasswordAuthenticationToken(username, "pw");
+    }
+
+    @Test
+    void listUsersDefaultScopeIsOrgWide() throws Exception {
+        // Default "org" scope returns every enabled user via findAll(), no team lookup.
+        Team alpha = team(1L, "alpha");
+        when(userRepository.findAll())
+                .thenReturn(
+                        List.of(
+                                user(1L, "[email protected]", true, alpha),
+                                user(2L, "[email protected]", true, alpha)));
+
+        mockMvc.perform(get("/api/v1/user/users").principal(auth("[email protected]")))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.length()").value(2))
+                .andExpect(jsonPath("$[0].username").value("[email protected]"))
+                .andExpect(jsonPath("$[1].username").value("[email protected]"));
+
+        verify(userRepository, never()).findAllByTeamId(any());
+        verify(userService, never()).findByUsernameIgnoreCase(anyString());
+    }
+
+    @Test
+    void listUsersOrgScopeFiltersDisabledUsers() throws Exception {
+        Team alpha = team(1L, "alpha");
+        when(userRepository.findAll())
+                .thenReturn(
+                        List.of(
+                                user(1L, "[email protected]", true, alpha),
+                                user(2L, "[email protected]", false, alpha)));
+
+        mockMvc.perform(get("/api/v1/user/users").principal(auth("[email protected]")))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.length()").value(1))
+                .andExpect(jsonPath("$[0].username").value("[email protected]"));
+    }
+
+    @Test
+    void listUsersTeamScopeReturnsOnlyCallerTeam() throws Exception {
+        applicationProperties.getStorage().getSigning().setUserListScope("team");
+        Team alpha = team(7L, "alpha");
+        User caller = user(1L, "[email protected]", true, alpha);
+        when(userService.findByUsernameIgnoreCase("[email protected]"))
+                .thenReturn(Optional.of(caller));
+        when(userRepository.findAllByTeamId(7L))
+                .thenReturn(List.of(caller, user(2L, "[email protected]", true, alpha)));
+
+        mockMvc.perform(get("/api/v1/user/users").principal(auth("[email protected]")))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.length()").value(2))
+                .andExpect(jsonPath("$[0].teamName").value("alpha"));
+
+        verify(userRepository).findAllByTeamId(7L);
+        verify(userRepository, never()).findAll();
+    }
+
+    @Test
+    void listUsersTeamScopeWithMissingCallerReturnsEmpty() throws Exception {
+        applicationProperties.getStorage().getSigning().setUserListScope("team");
+        when(userService.findByUsernameIgnoreCase("[email protected]")).thenReturn(Optional.empty());
+
+        mockMvc.perform(get("/api/v1/user/users").principal(auth("[email protected]")))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.length()").value(0));
+
+        verify(userRepository, never()).findAllByTeamId(any());
+        verify(userRepository, never()).findAll();
+    }
+
+    @Test
+    void listUsersTeamScopeWithNullTeamReturnsSelfOnly() throws Exception {
+        applicationProperties.getStorage().getSigning().setUserListScope("team");
+        User caller = user(1L, "[email protected]", true, null);
+        when(userService.findByUsernameIgnoreCase("[email protected]"))
+                .thenReturn(Optional.of(caller));
+
+        mockMvc.perform(get("/api/v1/user/users").principal(auth("[email protected]")))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.length()").value(1))
+                .andExpect(jsonPath("$[0].username").value("[email protected]"));
+
+        verify(userRepository, never()).findAllByTeamId(any());
+        verify(userRepository, never()).findAll();
+    }
+
+    @Test
+    void listUsersFailsClosedOnUnrecognisedScope() throws Exception {
+        // Any non-"org" value must restrict to the caller's team, not leak the instance.
+        applicationProperties.getStorage().getSigning().setUserListScope("tewm");
+        Team alpha = team(3L, "alpha");
+        when(userService.findByUsernameIgnoreCase("[email protected]"))
+                .thenReturn(Optional.of(user(1L, "[email protected]", true, alpha)));
+        when(userRepository.findAllByTeamId(3L))
+                .thenReturn(List.of(user(1L, "[email protected]", true, alpha)));
+
+        mockMvc.perform(get("/api/v1/user/users").principal(auth("[email protected]")))
+                .andExpect(status().isOk());
+
+        verify(userRepository).findAllByTeamId(3L);
+        verify(userRepository, never()).findAll();
+    }
+
+    @Test
+    void listUsersFailsClosedOnBlankScope() throws Exception {
+        applicationProperties.getStorage().getSigning().setUserListScope("   ");
+        Team alpha = team(4L, "alpha");
+        when(userService.findByUsernameIgnoreCase("[email protected]"))
+                .thenReturn(Optional.of(user(1L, "[email protected]", true, alpha)));
+        when(userRepository.findAllByTeamId(4L)).thenReturn(List.of());
+
+        mockMvc.perform(get("/api/v1/user/users").principal(auth("[email protected]")))
+                .andExpect(status().isOk());
+
+        verify(userRepository).findAllByTeamId(4L);
+        verify(userRepository, never()).findAll();
+    }
+
+    @Test
+    void listUsersFailsClosedOnNullScope() throws Exception {
+        // A null value must also fail closed to the caller's team.
+        applicationProperties.getStorage().getSigning().setUserListScope(null);
+        Team alpha = team(9L, "alpha");
+        when(userService.findByUsernameIgnoreCase("[email protected]"))
+                .thenReturn(Optional.of(user(1L, "[email protected]", true, alpha)));
+        when(userRepository.findAllByTeamId(9L)).thenReturn(List.of());
+
+        mockMvc.perform(get("/api/v1/user/users").principal(auth("[email protected]")))
+                .andExpect(status().isOk());
+
+        verify(userRepository).findAllByTeamId(9L);
+        verify(userRepository, never()).findAll();
+    }
+
+    @Test
+    void listUsersOrgScopeIsCaseInsensitive() throws Exception {
+        applicationProperties.getStorage().getSigning().setUserListScope("ORG");
+        when(userRepository.findAll()).thenReturn(List.of(user(1L, "[email protected]", true, null)));
+
+        mockMvc.perform(get("/api/v1/user/users").principal(auth("[email protected]")))
+                .andExpect(status().isOk());
+
+        verify(userRepository).findAll();
+        verify(userRepository, never()).findAllByTeamId(any());
+    }
+
+    @Test
+    void listUsersRequiresAuthentication() throws Exception {
+        mockMvc.perform(get("/api/v1/user/users")).andExpect(status().isUnauthorized());
+
+        verify(userRepository, never()).findAll();
+        verify(userRepository, never()).findAllByTeamId(any());
+    }
+
+    @Test
+    void signingUserListScopeDefaultsToOrg() {
+        // Self-host backward-compat: default must stay "org" (saas profile flips it to "team").
+        assertEquals(
+                "org", new ApplicationProperties().getStorage().getSigning().getUserListScope());
+    }
 }