diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index 31d3fc4e6..b955a2fe3 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -1005,6 +1005,10 @@ public class ApplicationProperties { @Data public static class Signing { private boolean enabled = false; + + // Signing user-picker scope: 'org' (default) = whole instance, anything else = + // caller's team only (fail-closed). The saas profile pins 'team'. + private String userListScope = "org"; } } diff --git a/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesLogicTest.java b/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesLogicTest.java index f075f1518..409b24430 100644 --- a/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesLogicTest.java +++ b/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesLogicTest.java @@ -31,6 +31,22 @@ class ApplicationPropertiesLogicTest { assertTrue(sys.isAnalyticsEnabled()); } + @Test + void storageSigning_userListScope_defaultsToOrg_andIsSettable() { + // Self-host backward-compat: scope must default to "org" (saas profile pins "team"). + ApplicationProperties.Storage.Signing signing = new ApplicationProperties.Storage.Signing(); + + assertFalse(signing.isEnabled()); + assertEquals("org", signing.getUserListScope()); + + signing.setUserListScope("team"); + assertEquals("team", signing.getUserListScope()); + + // Reachable from the full tree as storage.signing.userListScope. + assertEquals( + "org", new ApplicationProperties().getStorage().getSigning().getUserListScope()); + } + @Test void tempFileManagement_defaults_and_overrides() { Function normalize = s -> Paths.get(s).normalize().toString(); diff --git a/app/core/src/main/resources/settings.yml.template b/app/core/src/main/resources/settings.yml.template index fdf8f0fef..9bce7d1c5 100644 --- a/app/core/src/main/resources/settings.yml.template +++ b/app/core/src/main/resources/settings.yml.template @@ -290,6 +290,7 @@ storage: linkExpirationDays: 3 # Number of days before share links expire signing: enabled: false # set to 'true' to enable group signing workflow (requires storage.enabled) [ALPHA] + userListScope: org # Signing user-picker scope: 'org' (default) = whole instance, else caller's team only. autoPipeline: outputFolder: "" # Output folder for processed pipeline files (leave empty for default) fileReadiness: diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java index 90abd5f06..688277f09 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java @@ -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> 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 source; + if (teamScoped) { + Optional 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 users = - userRepository.findAll().stream() + source.stream() .filter(User::isEnabled) .map(this::toUserSummaryDTO) .collect(java.util.stream.Collectors.toList()); diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/UserControllerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/UserControllerTest.java index f5ea59f10..61e4bf17c 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/UserControllerTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/UserControllerTest.java @@ -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, "a@alpha.com", true, alpha), + user(2L, "b@alpha.com", true, alpha))); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("a@alpha.com"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(2)) + .andExpect(jsonPath("$[0].username").value("a@alpha.com")) + .andExpect(jsonPath("$[1].username").value("b@alpha.com")); + + 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, "enabled@alpha.com", true, alpha), + user(2L, "disabled@alpha.com", false, alpha))); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("enabled@alpha.com"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)) + .andExpect(jsonPath("$[0].username").value("enabled@alpha.com")); + } + + @Test + void listUsersTeamScopeReturnsOnlyCallerTeam() throws Exception { + applicationProperties.getStorage().getSigning().setUserListScope("team"); + Team alpha = team(7L, "alpha"); + User caller = user(1L, "caller@alpha.com", true, alpha); + when(userService.findByUsernameIgnoreCase("caller@alpha.com")) + .thenReturn(Optional.of(caller)); + when(userRepository.findAllByTeamId(7L)) + .thenReturn(List.of(caller, user(2L, "mate@alpha.com", true, alpha))); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("caller@alpha.com"))) + .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("ghost@alpha.com")).thenReturn(Optional.empty()); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("ghost@alpha.com"))) + .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, "solo@nowhere.com", true, null); + when(userService.findByUsernameIgnoreCase("solo@nowhere.com")) + .thenReturn(Optional.of(caller)); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("solo@nowhere.com"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)) + .andExpect(jsonPath("$[0].username").value("solo@nowhere.com")); + + 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("caller@alpha.com")) + .thenReturn(Optional.of(user(1L, "caller@alpha.com", true, alpha))); + when(userRepository.findAllByTeamId(3L)) + .thenReturn(List.of(user(1L, "caller@alpha.com", true, alpha))); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("caller@alpha.com"))) + .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("caller@alpha.com")) + .thenReturn(Optional.of(user(1L, "caller@alpha.com", true, alpha))); + when(userRepository.findAllByTeamId(4L)).thenReturn(List.of()); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("caller@alpha.com"))) + .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("caller@alpha.com")) + .thenReturn(Optional.of(user(1L, "caller@alpha.com", true, alpha))); + when(userRepository.findAllByTeamId(9L)).thenReturn(List.of()); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("caller@alpha.com"))) + .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, "a@alpha.com", true, null))); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("a@alpha.com"))) + .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()); + } } diff --git a/app/saas/src/main/resources/application-saas.properties b/app/saas/src/main/resources/application-saas.properties index 71e65de65..63ba01695 100644 --- a/app/saas/src/main/resources/application-saas.properties +++ b/app/saas/src/main/resources/application-saas.properties @@ -64,3 +64,8 @@ supabase.url=https://${app.supabase.project-ref}.supabase.co spring.security.oauth2.resourceserver.jwt.jwk-set-uri=https://${app.supabase.project-ref}.supabase.co/auth/v1/.well-known/jwks.json spring.security.oauth2.resourceserver.jwt.audiences=${app.supabase.expected-aud} + +# ---------- Multi-tenant scoping ---------- +# Restrict the signing user picker to the caller's team; SaaS must be 'team' +# or unrelated tenants leak emails to each other. +storage.signing.userListScope=team