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 509de7e811..d8500e475f 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,28 +978,33 @@ public class UserController { } } - // 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). + // Lists enabled users for the signing picker; 'org' scope = instance-wide, else caller's team. @GetMapping("/users") public ResponseEntity> listUsers(Principal principal) { if (principal == null) { return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build(); } + Optional callerOpt = userService.findByUsernameIgnoreCase(principal.getName()); + + // Anonymous (SaaS) accounts must never enumerate users, in any scope or team. + if (callerOpt.map(UserController::isAnonymousUser).orElse(false)) { + return ResponseEntity.status(HttpStatus.FORBIDDEN).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. + Team callerTeam = callerOpt.map(User::getTeam).orElse(null); + if (callerTeam == null || isSystemTeam(callerTeam)) { + // No team or a shared system team: return only the caller, not the team's members. 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()); + // Scopes via the single User.team FK; revisit if multi-team membership is added. + source = userRepository.findAllByTeamId(callerTeam.getId()); } } else { source = userRepository.findAll(); @@ -1011,6 +1016,18 @@ public class UserController { return ResponseEntity.ok(users); } + // SaaS anonymous accounts, which must not enumerate users. + private static boolean isAnonymousUser(User user) { + return AuthenticationType.ANONYMOUS.name().equalsIgnoreCase(user.getAuthenticationType()); + } + + // System teams (Default/Internal) are not enumerable through the signing picker. + private static boolean isSystemTeam(Team team) { + String name = team.getName(); + return TeamService.DEFAULT_TEAM_NAME.equalsIgnoreCase(name) + || TeamService.INTERNAL_TEAM_NAME.equalsIgnoreCase(name); + } + private UserSummaryDTO toUserSummaryDTO(User user) { return new UserSummaryDTO( user.getId(), 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 61e4bf17ce..7bcc0da9f4 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 @@ -2,7 +2,6 @@ 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; @@ -28,6 +27,7 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders; import stirling.software.common.model.ApplicationProperties; import stirling.software.proprietary.model.Team; import stirling.software.proprietary.security.database.repository.UserRepository; +import stirling.software.proprietary.security.model.AuthenticationType; import stirling.software.proprietary.security.model.User; import stirling.software.proprietary.security.model.api.user.UsernameAndPass; import stirling.software.proprietary.security.repository.TeamRepository; @@ -195,8 +195,22 @@ class UserControllerTest { .andExpect(jsonPath("$[0].username").value("a@alpha.com")) .andExpect(jsonPath("$[1].username").value("b@alpha.com")); + // Caller is resolved (for the anonymous-gate) but org scope still uses findAll, not team. + verify(userRepository, never()).findAllByTeamId(any()); + } + + @Test + void listUsersForbiddenForAnonymousCaller() throws Exception { + // Anonymous SaaS accounts must never enumerate users, regardless of scope. + User anon = user(1L, "anon_abc", true, team(1L, TeamService.DEFAULT_TEAM_NAME)); + anon.setAuthenticationType(AuthenticationType.ANONYMOUS); + when(userService.findByUsernameIgnoreCase("anon_abc")).thenReturn(Optional.of(anon)); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("anon_abc"))) + .andExpect(status().isForbidden()); + + verify(userRepository, never()).findAll(); verify(userRepository, never()).findAllByTeamId(any()); - verify(userService, never()).findByUsernameIgnoreCase(anyString()); } @Test @@ -262,6 +276,39 @@ class UserControllerTest { verify(userRepository, never()).findAll(); } + @Test + void listUsersTeamScopeOnDefaultTeamReturnsSelfOnly() throws Exception { + // A caller on a shared system team must not enumerate its members. + applicationProperties.getStorage().getSigning().setUserListScope("team"); + Team defaultTeam = team(1L, TeamService.DEFAULT_TEAM_NAME); + User caller = user(1L, "new@saas.com", true, defaultTeam); + when(userService.findByUsernameIgnoreCase("new@saas.com")).thenReturn(Optional.of(caller)); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("new@saas.com"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)) + .andExpect(jsonPath("$[0].username").value("new@saas.com")); + + verify(userRepository, never()).findAllByTeamId(any()); + verify(userRepository, never()).findAll(); + } + + @Test + void listUsersTeamScopeOnInternalTeamReturnsSelfOnly() throws Exception { + applicationProperties.getStorage().getSigning().setUserListScope("team"); + Team internalTeam = team(2L, TeamService.INTERNAL_TEAM_NAME); + User caller = user(1L, "svc@saas.com", true, internalTeam); + when(userService.findByUsernameIgnoreCase("svc@saas.com")).thenReturn(Optional.of(caller)); + + mockMvc.perform(get("/api/v1/user/users").principal(auth("svc@saas.com"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)) + .andExpect(jsonPath("$[0].username").value("svc@saas.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. diff --git a/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java b/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java index f1b2681e69..aa678f7cbf 100644 --- a/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java +++ b/app/saas/src/main/java/stirling/software/saas/security/SupabaseAuthenticationFilter.java @@ -262,7 +262,10 @@ public class SupabaseAuthenticationFilter extends OncePerRequestFilter { user.setUsername(supabaseUser.getEmail()); } try { - return userService.saveUser(user); + User saved = userService.saveUser(user); + // Give the account its own team rather than the shared Default team. + saved.setTeam(saasTeamService.ensurePersonalTeam(saved)); + return saved; } catch (DataIntegrityViolationException e) { log.warn( "Email collision upgrading anonymous user {} to {}: {}", @@ -344,7 +347,8 @@ public class SupabaseAuthenticationFilter extends OncePerRequestFilter { newUser.setEnabled(true); newUser.setFirstLogin(true); newUser.setRoleName(roleId); - newUser.setTeam(teamService.getOrCreateDefaultTeam()); + // No shared Default team; a per-user personal team is assigned after save (team_id + // nullable). newUser.setAuthenticationType(authenticationType); newUser.setSupabaseId(supabaseId); newUser.addAuthority(new Authority(roleId, newUser)); @@ -379,8 +383,7 @@ public class SupabaseAuthenticationFilter extends OncePerRequestFilter { // Only the DB-race winner runs first-time init; the losers skip it. if (weCreatedThisUser) { try { - saasTeamService.createPersonalTeam(savedUser); - savedUser = userService.findBySupabaseId(supabaseId).orElse(savedUser); + savedUser.setTeam(saasTeamService.ensurePersonalTeam(savedUser)); } catch (Exception e) { log.warn( "Failed to create personal team for new user {} ({}): {}", diff --git a/app/saas/src/main/java/stirling/software/saas/service/SaasTeamService.java b/app/saas/src/main/java/stirling/software/saas/service/SaasTeamService.java index 7c77c6a0e9..e54aa00df2 100644 --- a/app/saas/src/main/java/stirling/software/saas/service/SaasTeamService.java +++ b/app/saas/src/main/java/stirling/software/saas/service/SaasTeamService.java @@ -50,6 +50,16 @@ public class SaasTeamService { public static final String DEFAULT_TEAM_NAME = "Default"; public static final String INTERNAL_TEAM_NAME = "Internal"; + /** Returns the user's personal team, creating one if they have none. Idempotent. */ + @Transactional + public Team ensurePersonalTeam(User user) { + Team existing = user.getTeam(); + if (existing != null && saasTeamExtensionService.isPersonal(existing)) { + return existing; + } + return createPersonalTeam(user); + } + /** * Create personal team for new user during signup or migrate existing user from Default team * diff --git a/app/saas/src/main/java/stirling/software/saas/service/SaasUserAccountService.java b/app/saas/src/main/java/stirling/software/saas/service/SaasUserAccountService.java index c4b0ad769d..51ad9cab17 100644 --- a/app/saas/src/main/java/stirling/software/saas/service/SaasUserAccountService.java +++ b/app/saas/src/main/java/stirling/software/saas/service/SaasUserAccountService.java @@ -31,6 +31,7 @@ public class SaasUserAccountService { private final SupabaseUserService supabaseUserService; private final SaasUserExtensionService saasUserExtensionService; private final SaasTeamExtensionService saasTeamExtensionService; + private final SaasTeamService saasTeamService; /** * Resolve a local {@link User} from a Supabase UUID string. Throws if the ID format is invalid @@ -173,6 +174,8 @@ public class SaasUserAccountService { user.setUsername(email); } user = userService.saveUser(user); + // Give the upgraded user their own team rather than the shared Default team. + user.setTeam(saasTeamService.ensurePersonalTeam(user)); log.info( "Upgraded anonymous user {} to {} ({})", user.getId(), diff --git a/app/saas/src/test/java/stirling/software/saas/security/SupabaseAuthenticationFilterTest.java b/app/saas/src/test/java/stirling/software/saas/security/SupabaseAuthenticationFilterTest.java index 0069b9742e..e906655edc 100644 --- a/app/saas/src/test/java/stirling/software/saas/security/SupabaseAuthenticationFilterTest.java +++ b/app/saas/src/test/java/stirling/software/saas/security/SupabaseAuthenticationFilterTest.java @@ -30,7 +30,6 @@ import org.springframework.security.oauth2.jwt.Jwt; import org.springframework.security.oauth2.jwt.JwtDecoder; import org.springframework.security.oauth2.jwt.JwtException; -import stirling.software.proprietary.model.Team; import stirling.software.proprietary.security.model.ApiKeyAuthenticationToken; import stirling.software.proprietary.security.model.AuthenticationType; import stirling.software.proprietary.security.model.User; @@ -155,7 +154,6 @@ class SupabaseAuthenticationFilterTest { when(supabaseUserService.getUser(supabaseId)) .thenReturn(supabaseUserMatching(supabaseId, "bob@example.com", false)); when(userService.findBySupabaseId(supabaseId)).thenReturn(Optional.empty()); - when(teamService.getOrCreateDefaultTeam()).thenReturn(new Team()); when(userService.saveUser(any())).thenAnswer(inv -> inv.getArgument(0)); request.setRequestURI("/api/v1/something"); @@ -166,6 +164,9 @@ class SupabaseAuthenticationFilterTest { verify(userService, times(1)).saveUser(any(User.class)); verify(supabaseUserService).createSupabaseUser(supabaseId, "bob@example.com", false); + // New users get their own personal team, never the shared Default team. + verify(saasTeamService).ensurePersonalTeam(any(User.class)); + verify(teamService, never()).getOrCreateDefaultTeam(); assertThat(SecurityContextHolder.getContext().getAuthentication()) .isInstanceOf(EnhancedJwtAuthenticationToken.class); } @@ -179,7 +180,6 @@ class SupabaseAuthenticationFilterTest { when(supabaseUserService.getUser(supabaseId)) .thenReturn(supabaseUserMatching(supabaseId, "carol@example.com", false)); when(userService.findBySupabaseId(supabaseId)).thenReturn(Optional.empty()); - when(teamService.getOrCreateDefaultTeam()).thenReturn(new Team()); when(userService.saveUser(any(User.class))) .thenAnswer( inv -> { @@ -208,7 +208,6 @@ class SupabaseAuthenticationFilterTest { when(supabaseUserService.getUser(supabaseId)) .thenReturn(supabaseUserMatching(supabaseId, "dave@example.com", false)); when(userService.findBySupabaseId(supabaseId)).thenReturn(Optional.empty()); - when(teamService.getOrCreateDefaultTeam()).thenReturn(new Team()); when(userService.saveUser(any(User.class))) .thenAnswer( inv -> { @@ -237,7 +236,6 @@ class SupabaseAuthenticationFilterTest { when(supabaseUserService.getUser(supabaseId)) .thenReturn(supabaseUserMatching(supabaseId, "eve@example.com", false)); when(userService.findBySupabaseId(supabaseId)).thenReturn(Optional.empty()); - when(teamService.getOrCreateDefaultTeam()).thenReturn(new Team()); when(userService.saveUser(any(User.class))) .thenAnswer( inv -> { diff --git a/app/saas/src/test/java/stirling/software/saas/service/SaasUserAccountServiceTest.java b/app/saas/src/test/java/stirling/software/saas/service/SaasUserAccountServiceTest.java index 03efa06ab6..5bf0a33e4c 100644 --- a/app/saas/src/test/java/stirling/software/saas/service/SaasUserAccountServiceTest.java +++ b/app/saas/src/test/java/stirling/software/saas/service/SaasUserAccountServiceTest.java @@ -51,6 +51,7 @@ class SaasUserAccountServiceTest { @Mock private SupabaseUserService supabaseUserService; @Mock private SaasUserExtensionService saasUserExtensionService; @Mock private SaasTeamExtensionService saasTeamExtensionService; + @Mock private SaasTeamService saasTeamService; @InjectMocks private SaasUserAccountService service; @@ -355,6 +356,8 @@ class SaasUserAccountServiceTest { assertThat(u.getEmail()).isEqualTo("alice@example.com"); assertThat(u.getUsername()).isEqualTo("alice@example.com"); verify(userService).saveUser(u); + // Upgrading from anon gives the user their own team. + verify(saasTeamService).ensurePersonalTeam(u); } @Test @@ -455,6 +458,7 @@ class SaasUserAccountServiceTest { assertThat(u.getUsername()).isEqualTo("existing@example.com"); assertThat(u.getAuthenticationType()).isEqualTo("web"); verify(userService, never()).saveUser(any()); + verify(saasTeamService, never()).ensurePersonalTeam(any()); } @Test