From 5e3e89ccb2bbf3b662109bae07059d4557bd8b8c Mon Sep 17 00:00:00 2001 From: James Brunton Date: Tue, 21 Jul 2026 14:57:45 +0100 Subject: [PATCH] Fix existing teams logic (#7070) # Description of Changes Paired with https://github.com/Stirling-Tools/Stirling-PDF-SaaS/pull/320 Fixes the following bugs we found when testing the SaaS release: - Existing users couldn't join teams - this was because they were the last leader of their team, so it'd be left orphaned). Users now have a 'home team', which can have no members if they join another team, but they can then go back to it later. - Existing leaders didn't have unlimited seats - `saas_teams_extensions` had no row for them, so the app fell back to `max_seats=1`. The migration script fixes it. - Members without Processor access could still access the Processor - It was just checking "Are you the leader of **any** team", instead of the user's active team. --- .../access/service/ResourceAccessService.java | 19 +- .../api/ProprietaryUIDataController.java | 30 ++- .../ResourceAccessPortalBulkParityTest.java | 5 +- .../service/ResourceAccessServiceTest.java | 27 +- .../saas/model/SaasUserExtensions.java | 5 + .../SupabaseAuthenticationFilter.java | 6 +- .../saas/service/SaasTeamService.java | 252 ++++++++---------- .../service/SaasUserExtensionService.java | 15 ++ .../SupabaseAuthenticationFilterMoreTest.java | 2 + .../saas/service/SaasTeamServiceTest.java | 149 ++++++----- .../service/SaasUserExtensionServiceTest.java | 38 +++ .../auth/PortalAuthBoundary.test.tsx | 77 ++++-- .../portal-saas/auth/PortalAuthBoundary.tsx | 44 ++- .../proprietary/auth/supabase/UseSession.tsx | 85 +++--- 14 files changed, 466 insertions(+), 288 deletions(-) diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/access/service/ResourceAccessService.java b/app/proprietary/src/main/java/stirling/software/proprietary/access/service/ResourceAccessService.java index b3c987d6f2..7642dded85 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/access/service/ResourceAccessService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/access/service/ResourceAccessService.java @@ -43,8 +43,13 @@ public class ResourceAccessService { return canUseResource(ResourceType.PORTAL, "", null, portalDefaultPolicy, user); } - /** Portal access for a roster (admin, grant, or default policy). */ - public Set usersWithPortalAccess(Collection users, Set teamLeaderUserIds) { + /** + * Portal access for a roster (admin, grant, or default policy). {@code activeTeamLeaderUserIds} + * must hold ids of users who lead their own active team — the set the ADMINS_AND_TEAM_LEADS + * default admits, matching {@link #canAccessPortal}. + */ + public Set usersWithPortalAccess( + Collection users, Set activeTeamLeaderUserIds) { Set grantedPrincipals = new HashSet<>(); for (ResourceGrant g : grantRepository.findByResourceTypeAndResourceId(ResourceType.PORTAL, "")) { @@ -52,7 +57,7 @@ public class ResourceAccessService { grantedPrincipals.add(new PrincipalRef(g.getPrincipalType(), g.getPrincipalId())); } } - Set leaderIds = teamLeaderUserIds == null ? Set.of() : teamLeaderUserIds; + Set leaderIds = activeTeamLeaderUserIds == null ? Set.of() : activeTeamLeaderUserIds; Set allowed = new HashSet<>(); for (User user : users) { if (user != null @@ -214,11 +219,13 @@ public class ResourceAccessService { }; } - // Portal (no owner) admits any team lead; a team-owned resource admits only that team's - // leads; a user-owned resource admits no extra leads. + // Portal (no owner) admits the leader of the user's active team; a team-owned resource + // admits only that team's leads; a user-owned resource admits no extra leads. private boolean matchesTeamLeadDefault(PrincipalRef owner, User user) { if (owner == null) { - return teamLeadLookup.isAnyTeamLeader(user); + return user.getTeam() != null + && user.getTeam().getId() != null + && teamLeadLookup.isLeaderOfTeam(user, user.getTeam().getId()); } return owner.type() == PrincipalType.TEAM && owner.id() != null diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java b/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java index 2c0b995b8b..02fa1e0b25 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ProprietaryUIDataController.java @@ -345,10 +345,27 @@ public class ProprietaryUIDataController { int licenseMaxUsers = licenseSettingsService.getSettings().getLicenseMaxUsers(); boolean premiumEnabled = applicationProperties.getPremium().isEnabled(); - // Resolve portal access for the whole roster. - Set leaderUserIds = leaderUserIds(); + // Resolve portal access for the whole roster. The teamLead display flag counts a + // LEADER membership on any team (mirrors /me), but the portal default policy only + // admits leaders of their own active team, so the bulk check gets the narrower set. + List leaderMemberships = + teamMembershipRepository.findByRoleFetchingUserAndTeam(TeamRole.LEADER); + Set leaderUserIds = + leaderMemberships.stream() + .map(row -> row.getUser().getId()) + .collect(Collectors.toSet()); + Set activeTeamLeaderUserIds = + leaderMemberships.stream() + .filter( + row -> + row.getUser().getTeam() != null + && row.getTeam() + .getId() + .equals(row.getUser().getTeam().getId())) + .map(row -> row.getUser().getId()) + .collect(Collectors.toSet()); Set portalAccessUserIds = - resourceAccessService.usersWithPortalAccess(sortedUsers, leaderUserIds); + resourceAccessService.usersWithPortalAccess(sortedUsers, activeTeamLeaderUserIds); List userSummaries = sortedUsers.stream() .map(user -> convertUserToSummary(user, leaderUserIds, portalAccessUserIds)) @@ -536,13 +553,6 @@ public class ProprietaryUIDataController { return ResponseEntity.ok(data); } - /** User ids holding a LEADER membership on any team. */ - private Set leaderUserIds() { - return teamMembershipRepository.findByRoleFetchingUserAndTeam(TeamRole.LEADER).stream() - .map(row -> row.getUser().getId()) - .collect(Collectors.toSet()); - } - /** Whether the user holds the internal-API authority (never shown in the roster). */ private boolean isInternalApiUser(User user) { for (Authority authority : user.getAuthorities()) { diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessPortalBulkParityTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessPortalBulkParityTest.java index 858de91726..ba73fd2149 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessPortalBulkParityTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessPortalBulkParityTest.java @@ -62,8 +62,9 @@ class ResourceAccessPortalBulkParityTest { grant(PrincipalType.USER, 3L, AccessPermission.USE), grant(PrincipalType.TEAM, 20L, AccessPermission.USE))); - // Only #2 leads a team; leaderUserIds is what the controller passes to the bulk method. - lenient().when(teamLeadLookup.isAnyTeamLeader(leader)).thenReturn(true); + // Only #2 leads their active team; leaderUserIds is what the controller passes to the + // bulk method (the active-team-leader set). + lenient().when(teamLeadLookup.isLeaderOfTeam(leader, 10L)).thenReturn(true); leaderUserIds = Set.of(2L); } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessServiceTest.java index c63e6b8e47..90019c8cbb 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessServiceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessServiceTest.java @@ -239,10 +239,10 @@ class ResourceAccessServiceTest { } @Test - void teamLeadDefaultAllowsLeaderButNotRegularUser() { + void teamLeadDefaultAllowsActiveTeamLeaderButNotRegularUser() { stubGrants(); - User leader = user(5); - when(teamLeadLookup.isAnyTeamLeader(leader)).thenReturn(true); + User leader = userInTeam(5, 7L); + when(teamLeadLookup.isLeaderOfTeam(leader, 7L)).thenReturn(true); assertThat( service.canUseResource( TYPE, RID, null, DefaultAccessPolicy.ADMINS_AND_TEAM_LEADS, leader)) @@ -291,6 +291,27 @@ class ResourceAccessServiceTest { assertThat(service.canAccessPortal(user(5))).isFalse(); } + @Test + void portalAllowedToLeaderOfActiveTeam() { + when(grantRepository.findByResourceTypeAndResourceId(ResourceType.PORTAL, "")) + .thenReturn(List.of()); + User leader = userInTeam(10, 21L); + when(teamLeadLookup.isLeaderOfTeam(leader, 21L)).thenReturn(true); + assertThat(service.canAccessPortal(leader)).isTrue(); + } + + @Test + void portalDeniedToMemberWhoseActiveTeamTheyDoNotLead() { + // Durable home teams: a user still leads their dormant home team, but their ACTIVE + // team is one they only belong to -> no portal access (this is the bug-3 guard that + // active-team leadership preserves once home teams stop being deleted on join). + when(grantRepository.findByResourceTypeAndResourceId(ResourceType.PORTAL, "")) + .thenReturn(List.of()); + User member = userInTeam(11, 22L); + // isLeaderOfTeam(member, 22L) left unstubbed -> false: member of active team. + assertThat(service.canAccessPortal(member)).isFalse(); + } + // ---- helpers ---- private void stubGrants(ResourceGrant... grants) { diff --git a/app/saas/src/main/java/stirling/software/saas/model/SaasUserExtensions.java b/app/saas/src/main/java/stirling/software/saas/model/SaasUserExtensions.java index d483f848de..036fdd27be 100644 --- a/app/saas/src/main/java/stirling/software/saas/model/SaasUserExtensions.java +++ b/app/saas/src/main/java/stirling/software/saas/model/SaasUserExtensions.java @@ -57,6 +57,11 @@ public class SaasUserExtensions implements Serializable { @Column(name = "api_key_first_used_at") private LocalDateTime apiKeyFirstUsedAt; + // Durable "home team" the user returns to when leaving a joined team; distinct from the + // active users.team_id. Plain id (not a @ManyToOne) to avoid an eager Team load. Nullable. + @Column(name = "home_team_id") + private Long homeTeamId; + @CreationTimestamp @Column(name = "created_at", updatable = false) private LocalDateTime createdAt; 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 aa678f7cbf..34bcccfeb7 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 @@ -380,8 +380,10 @@ public class SupabaseAuthenticationFilter extends OncePerRequestFilter { dup)); } - // Only the DB-race winner runs first-time init; the losers skip it. - if (weCreatedThisUser) { + // Only the DB-race winner runs first-time init; the losers skip it. Guests (anonymous + // sessions) get NO team: the editor is free and needs none, and automation requires a + // real account. + if (weCreatedThisUser && !isAnonymous(jwt)) { try { savedUser.setTeam(saasTeamService.ensurePersonalTeam(savedUser)); } catch (Exception e) { 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 94996ca01c..bda0602261 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 @@ -46,19 +46,9 @@ public class SaasTeamService { private final UserRoleService userRoleService; private final SaasTeamExtensionService saasTeamExtensionService; private final SaasTeamExtensionsRepository saasTeamExtensionsRepository; + private final SaasUserExtensionService saasUserExtensionService; private final LinkedInstanceRepository linkedInstanceRepository; private final stirling.software.proprietary.security.service.UserService userService; - private final stirling.software.proprietary.access.repository.ResourceGrantRepository - resourceGrantRepository; - private final stirling.software.proprietary.integration.repository.IntegrationConfigRepository - integrationConfigRepository; - - // Team-owned integration configs + team grants FK the teams row; purge before deleting a team. - private void purgeTeamOwnedResources(Long teamId) { - integrationConfigRepository.deleteByOwnerTeam_Id(teamId); - resourceGrantRepository.deleteByPrincipalTypeAndPrincipalId( - stirling.software.proprietary.access.model.PrincipalType.TEAM, teamId); - } public static final String DEFAULT_TEAM_NAME = "Default"; public static final String INTERNAL_TEAM_NAME = "Internal"; @@ -114,6 +104,9 @@ public class SaasTeamService { user.setTeam(savedTeam); userRepository.save(user); + // A freshly-created personal team is the user's durable home team. + saasUserExtensionService.setHomeTeamId(user, savedTeam.getId()); + // Clean up old Default/Internal team membership if (oldTeam != null && (DEFAULT_TEAM_NAME.equals(oldTeam.getName()) @@ -129,6 +122,54 @@ public class SaasTeamService { return savedTeam; } + /** + * The user's durable home team id (the team they fall back to when leaving a joined team). Uses + * the stored pointer; if unset (e.g. an existing user before the backfill), derives it from a + * solo team they lead and persists it. Returns null when the user has no such team (e.g. they + * joined under the old delete-on-join flow); callers mint a fresh home in that case. + */ + private Long resolveHomeTeamId(User user) { + Long homeId = saasUserExtensionService.getHomeTeamId(user); + if (homeId != null) { + return homeId; + } + for (TeamMembership m : membershipRepository.findByUserId(user.getId())) { + if (m.isLeader() && membershipRepository.countByTeamId(m.getTeam().getId()) == 1) { + saasUserExtensionService.setHomeTeamId(user, m.getTeam().getId()); + return m.getTeam().getId(); + } + } + return null; + } + + /** + * Move the user back to their durable home team. Reuses the existing home membership when + * present (the durable model keeps it across joins); mints a fresh personal home team only when + * the user has none. + */ + private void returnUserToHome(User user) { + Long homeId = resolveHomeTeamId(user); + Team home = homeId == null ? null : teamRepository.findById(homeId).orElse(null); + if (home == null) { + createPersonalTeam(user); + return; + } + if (membershipRepository.findByTeamIdAndUserId(home.getId(), user.getId()).isEmpty()) { + TeamMembership membership = new TeamMembership(); + membership.setTeam(home); + membership.setUser(user); + membership.setRole(TeamRole.LEADER); + membership.setInvitedAt(LocalDateTime.now()); + membership.setAcceptedAt(LocalDateTime.now()); + membershipRepository.save(membership); + // Ignore the result: a full home returns 0, the correct end state (1 member = 1 seat) - + // throwing here would wrongly block the return. + saasTeamExtensionsRepository.incrementSeatsUsed(home.getId()); + } + userRepository.updateUserTeamId(user.getId(), home.getId()); + user.setTeam(home); + } + /** * Invite user to team (sends email via Supabase Edge Function) * @@ -314,29 +355,29 @@ public class SaasTeamService { throw new IllegalStateException("Team has no available seats"); } - // Validate: accepting won't orphan a team the user leads or that has a paid plan. - // Accepting moves the user off their current team; leaveTeam already blocks the - // last leader of a team from walking away, so accept must enforce the same rule. - assertCanLeaveCurrentTeamsToJoinAnother(acceptingUser); + // Establish/park the durable home team: joining never deletes it. + Long homeTeamId = resolveHomeTeamId(acceptingUser); - // User can only be in one team . leave existing teams before joining new one - List existingMemberships = - membershipRepository.findByUserId(acceptingUser.getId()); - List teamsToDelete = new java.util.ArrayList<>(); + // Guard: block only if joining would strand a paid/linked team the user is the last + // leader of. Home teams are parked (kept), so a join never orphans them. + assertCanLeaveCurrentTeamsToJoinAnother(acceptingUser, homeTeamId, team.getId()); - for (TeamMembership existingMembership : existingMemberships) { + // Leave any non-home team the user currently belongs to; keep the home team + membership. + for (TeamMembership existingMembership : + membershipRepository.findByUserId(acceptingUser.getId())) { Team oldTeam = existingMembership.getTeam(); - - membershipRepository.delete(existingMembership); - - saasTeamExtensionsRepository.decrementSeatsUsed(oldTeam.getId()); - - // Mark personal team for deletion if it's now empty (user was the only member) - if (saasTeamExtensionService.isPersonal(oldTeam) - && membershipRepository.countByTeamId(oldTeam.getId()) == 0) { - teamsToDelete.add(oldTeam); + if ((homeTeamId != null && homeTeamId.equals(oldTeam.getId())) + || oldTeam.getId().equals(team.getId())) { + continue; // keep the durable home; skip the team being joined } - + // Keep any team the user leads that still has other members: leaving it would orphan + // them (members, zero leaders). Solo/empty led teams and plain memberships still leave. + if (existingMembership.isLeader() + && membershipRepository.countByTeamId(oldTeam.getId()) > 1) { + continue; + } + membershipRepository.delete(existingMembership); + saasTeamExtensionsRepository.decrementSeatsUsed(oldTeam.getId()); log.info( "User {} left team {} to join team {}", acceptingUser.getUsername(), @@ -347,43 +388,27 @@ public class SaasTeamService { // Native query: avoids Hibernate touching the read-only supabase_auth_id column. userRepository.updateUserTeamId(acceptingUser.getId(), team.getId()); acceptingUser.setTeam(team); - log.info( - "User {} team reference updated to team {}", - acceptingUser.getUsername(), - team.getName()); - // Now safe to delete empty personal teams - for (Team teamToDelete : teamsToDelete) { - log.info( - "Deleting empty personal team {} after user {} joined another team", - teamToDelete.getId(), - acceptingUser.getUsername()); - purgeTeamOwnedResources(teamToDelete.getId()); - teamRepository.delete(teamToDelete); + // Add the MEMBER membership on the joined team (unless already present). + if (membershipRepository + .findByTeamIdAndUserId(team.getId(), acceptingUser.getId()) + .isEmpty()) { + TeamMembership membership = new TeamMembership(); + membership.setTeam(team); + membership.setUser(acceptingUser); + membership.setRole(TeamRole.MEMBER); + membership.setInvitedBy(inviter); + membership.setInvitedAt(invitation.getCreatedAt()); + membership.setAcceptedAt(LocalDateTime.now()); + membershipRepository.save(membership); + + // incrementSeatsUsed enforces the cap atomically; rowsUpdated==0 means at capacity. + int rowsUpdated = saasTeamExtensionsRepository.incrementSeatsUsed(team.getId()); + if (rowsUpdated == 0) { + throw new IllegalStateException("Team has no available seats"); + } } - // Create team membership - TeamMembership membership = new TeamMembership(); - membership.setTeam(team); - membership.setUser(acceptingUser); - membership.setRole(TeamRole.MEMBER); - membership.setInvitedBy(inviter); - membership.setInvitedAt(invitation.getCreatedAt()); - membership.setAcceptedAt(LocalDateTime.now()); - membershipRepository.save(membership); - - log.info( - "User {} added to team {} with role MEMBER", - acceptingUser.getUsername(), - team.getName()); - - // incrementSeatsUsed enforces the seat cap atomically; rowsUpdated==0 means at capacity. - int rowsUpdated = saasTeamExtensionsRepository.incrementSeatsUsed(team.getId()); - if (rowsUpdated == 0) { - throw new IllegalStateException("Team has no available seats"); - } - log.info("Team {} seats_used incremented", team.getName()); - // Don't set inviteeUser; acceptance is recorded via status + TeamMembership row. invitation.setStatus(InvitationStatus.ACCEPTED); invitationRepository.save(invitation); @@ -434,89 +459,61 @@ public class SaasTeamService { // Atomically decrement team seats_used (prevents race condition) saasTeamExtensionsRepository.decrementSeatsUsed(teamId); - // Fetch team for downstream checks - Team team = teamRepository.findById(teamId).orElseThrow(); + // Return the removed user to their durable home team (mints one only if they have none). + returnUserToHome(userToRemove); - // Create new personal team for removed user - createPersonalTeam(userToRemove); - - // Downgrade user to FREE tier after leaving team - // They either had a trial (which was cancelled) or had an existing subscription - // Either way, they should be FREE after leaving + // Downgrade to FREE after leaving the team. downgradeUserToFree(userToRemove); - // Delete non-personal team if it's now empty - if (!saasTeamExtensionService.isPersonal(team) - && membershipRepository.countByTeamId(teamId) == 0) { - log.info("Deleting empty non-personal team {} after last member removed", teamId); - purgeTeamOwnedResources(team.getId()); - teamRepository.delete(team); - } - log.info( - "User {} removed user {} from team {} and created new personal team", + "User {} removed user {} from team {}; returned them to their home team", remover.getId(), memberUserId, teamId); } /** - * Guard against silently orphaning a team when a user accepts an invite to another one. + * Guard against orphaning a still-billing team when a user joins another. * - *

{@link #acceptInvitation} moves a user to the inviting team by first leaving their current - * team(s). Personal teams are disposable (they get deleted on accept), but a non-personal team - * must not be left memberless while still billing. {@link #leaveTeam} already refuses to let - * the last leader walk away; accept took a shortcut around that check, which let a paid team's - * leader join another team and orphan their old team together with its live subscription. + *

In the durable-home model a join parks the user's home team (keeps the team, its + * membership and its wallet) rather than deleting it, so a plain team is never orphaned. The + * only real hazard is a team the user is the last leader of that still carries live + * billing: an active paid/PAYG subscription, or a non-revoked linked self-hosted instance + * ("Mode A"). Those block the join until the plan is cancelled / leadership transferred / + * instances revoked. An unpaid, unlinked team (personal or shared) no longer blocks. * - *

So: for each non-personal team the user leads as its last leader, block the - * accept. The message points them at the right remedy — cancel the plan if the team is paid, - * otherwise transfer leadership first. - * - *

Linked self-hosted instances (combined-billing "Mode A") bind to a team via {@code - * linked_instance.team_id}, so they too orphan a team that is left memberless — a personal team - * that accept deletes, or a non-personal team left by its last leader. They're checked in that - * same orphaning branch (not for a non-leader leaving a team that lives on); the remedy is to - * revoke them. + *

The home team and the team being joined are excluded: neither is left by the join (home is + * parked, the joined team is kept), so their live billing cannot be stranded. * * @param user the user attempting to accept an invitation - * @throws IllegalStateException if accepting would orphan a team the user leads or its - * instances + * @param homeTeamId the user's durable home team, parked by the join (may be null) + * @param joinedTeamId the team being joined + * @throws IllegalStateException if joining would strand a paid/linked team the user last-leads */ - private void assertCanLeaveCurrentTeamsToJoinAnother(User user) { + private void assertCanLeaveCurrentTeamsToJoinAnother( + User user, Long homeTeamId, Long joinedTeamId) { for (TeamMembership membership : membershipRepository.findByUserId(user.getId())) { Team team = membership.getTeam(); - boolean personal = saasTeamExtensionService.isPersonal(team); - if (!personal && !membership.isLeader()) { - // A non-leader leaving a shared team never orphans it. + // Home is parked and the joined team is kept, so neither can be orphaned. + if (team.getId().equals(joinedTeamId) || team.getId().equals(homeTeamId)) { continue; } - if (!personal - && membershipRepository.countByTeamIdAndRole(team.getId(), TeamRole.LEADER) + // Only a sole leader can strand a team; a member or co-leader leaving never does. + if (!membership.isLeader() + || membershipRepository.countByTeamIdAndRole(team.getId(), TeamRole.LEADER) > 1) { - // Another leader remains, so the team keeps an owner. continue; } - // Leaving here orphans the team: a personal team is deleted on accept; a non-personal - // team is being left by its last leader. Either way its linked self-hosted instances - // lose their billing team, so block until they're revoked. if (linkedInstanceRepository.countByTeamIdAndRevokedAtIsNull(team.getId()) > 0) { throw new IllegalStateException( "Revoke linked self-hosted instances on this team before joining another" + " team."); } - if (personal) { - // Personal teams are disposable (deleted on accept) and never billed/shared. - continue; - } if (hasActivePaidSubscription(team)) { throw new IllegalStateException( "Your team has an active plan and you are its last leader. Cancel the plan" + " or transfer leadership before joining another team."); } - throw new IllegalStateException( - "You are the last leader of your team. Transfer leadership before joining" - + " another team."); } } @@ -549,26 +546,13 @@ public class SaasTeamService { // Atomically decrement team seats_used (prevents race condition) saasTeamExtensionsRepository.decrementSeatsUsed(teamId); - // Fetch team for downstream checks - Team team = teamRepository.findById(teamId).orElseThrow(); + // Return the user to their durable home team (mints one only if they have none). + returnUserToHome(user); - // Create new personal team for user who left - createPersonalTeam(user); - - // Check if user should be downgraded after leaving team - // If user has an active subscription (including trial), they keep PRO access - // Otherwise, downgrade to FREE tier + // Downgrade to FREE unless they still hold their own active subscription. downgradeUserToFree(user); - // Delete non-personal team if it's now empty - if (!saasTeamExtensionService.isPersonal(team) - && membershipRepository.countByTeamId(teamId) == 0) { - log.info("Deleting empty non-personal team {} after last member left", teamId); - purgeTeamOwnedResources(team.getId()); - teamRepository.delete(team); - } - - log.info("User {} left team {} and created new personal team", user.getId(), teamId); + log.info("User {} left team {} and returned to their home team", user.getId(), teamId); } /** @@ -780,8 +764,8 @@ public class SaasTeamService { // Atomically decrement seats_used count (prevents race condition) saasTeamExtensionsRepository.decrementSeatsUsed(teamId); - // Create new personal team for removed user - createPersonalTeam(userToRemove); + // Return the evicted user to their durable home team. + returnUserToHome(userToRemove); // Downgrade user to FREE tier downgradeUserToFree(userToRemove); diff --git a/app/saas/src/main/java/stirling/software/saas/service/SaasUserExtensionService.java b/app/saas/src/main/java/stirling/software/saas/service/SaasUserExtensionService.java index ffd63b56e0..c62e686665 100644 --- a/app/saas/src/main/java/stirling/software/saas/service/SaasUserExtensionService.java +++ b/app/saas/src/main/java/stirling/software/saas/service/SaasUserExtensionService.java @@ -52,6 +52,21 @@ public class SaasUserExtensionService { .orElse(null); } + /** The user's durable home team id (the team they fall back to), or null if unset. */ + public Long getHomeTeamId(User user) { + return repository + .findByUserId(user.getId()) + .map(SaasUserExtensions::getHomeTeamId) + .orElse(null); + } + + @Transactional + public void setHomeTeamId(User user, Long homeTeamId) { + SaasUserExtensions ext = getOrCreate(user); + ext.setHomeTeamId(homeTeamId); + repository.save(ext); + } + /** Idempotent first-use marker. Records the first time this user's API key fired a request. */ @Transactional public void trackApiKeyFirstUse(User user) { diff --git a/app/saas/src/test/java/stirling/software/saas/security/SupabaseAuthenticationFilterMoreTest.java b/app/saas/src/test/java/stirling/software/saas/security/SupabaseAuthenticationFilterMoreTest.java index 448bafae6b..d312b74c75 100644 --- a/app/saas/src/test/java/stirling/software/saas/security/SupabaseAuthenticationFilterMoreTest.java +++ b/app/saas/src/test/java/stirling/software/saas/security/SupabaseAuthenticationFilterMoreTest.java @@ -276,6 +276,8 @@ class SupabaseAuthenticationFilterMoreTest { filter.doFilter(request, response, chain); verify(userService, times(1)).saveUser(any(User.class)); + // Guests get NO team; a home team + grant is provisioned only on signup/upgrade. + verify(saasTeamService, never()).ensurePersonalTeam(any()); // Anonymous mirror row created with null email and anon flag true. verify(supabaseUserService).createSupabaseUser(supabaseId, null, true); assertThat(SecurityContextHolder.getContext().getAuthentication()) diff --git a/app/saas/src/test/java/stirling/software/saas/service/SaasTeamServiceTest.java b/app/saas/src/test/java/stirling/software/saas/service/SaasTeamServiceTest.java index 5f1083cb3f..fc1fbcb0ab 100644 --- a/app/saas/src/test/java/stirling/software/saas/service/SaasTeamServiceTest.java +++ b/app/saas/src/test/java/stirling/software/saas/service/SaasTeamServiceTest.java @@ -65,17 +65,10 @@ class SaasTeamServiceTest { @Mock private UserRoleService userRoleService; @Mock private SaasTeamExtensionService saasTeamExtensionService; @Mock private SaasTeamExtensionsRepository saasTeamExtensionsRepository; + @Mock private SaasUserExtensionService saasUserExtensionService; @Mock private LinkedInstanceRepository linkedInstanceRepository; @Mock private stirling.software.proprietary.security.service.UserService userService; - @Mock - private stirling.software.proprietary.access.repository.ResourceGrantRepository - resourceGrantRepository; - - @Mock - private stirling.software.proprietary.integration.repository.IntegrationConfigRepository - integrationConfigRepository; - @InjectMocks private SaasTeamService service; private static final UUID SUPABASE_ID = UUID.fromString("11111111-2222-3333-4444-555555555555"); @@ -694,7 +687,6 @@ class SaasTeamServiceTest { when(saasTeamExtensionService.hasAvailableSeats(newTeam)).thenReturn(true); when(membershipRepository.findByUserId(5L)) .thenReturn(List.of(membership(ownTeam, u, TeamRole.LEADER))); - when(saasTeamExtensionService.isPersonal(ownTeam)).thenReturn(false); when(membershipRepository.countByTeamIdAndRole(200L, TeamRole.LEADER)).thenReturn(1L); when(billingSubscriptionRepository.existsActiveSubscriptionForTeam(200L)) .thenReturn(true); @@ -705,9 +697,10 @@ class SaasTeamServiceTest { } @Test - @DisplayName( - "blocks accept when the user is the last leader of an unpaid non-personal team") - void lastLeaderOfUnpaidTeam_blocksAccept() { + @DisplayName("allows accept for the last leader of an unpaid non-personal team") + void lastLeaderOfUnpaidTeam_allowsAccept() { + // The old over-broad "transfer leadership" block is gone: an unpaid, unlinked team is + // never orphaned in the durable model, so the join proceeds. User u = user(5L, "b@x.com", "bob"); Team newTeam = team(100L, "Acme"); Team ownTeam = team(200L, "Bob Co"); @@ -719,44 +712,40 @@ class SaasTeamServiceTest { when(saasTeamExtensionService.hasAvailableSeats(newTeam)).thenReturn(true); when(membershipRepository.findByUserId(5L)) .thenReturn(List.of(membership(ownTeam, u, TeamRole.LEADER))); - when(saasTeamExtensionService.isPersonal(ownTeam)).thenReturn(false); when(membershipRepository.countByTeamIdAndRole(200L, TeamRole.LEADER)).thenReturn(1L); when(billingSubscriptionRepository.existsActiveSubscriptionForTeam(200L)) .thenReturn(false); + when(saasTeamExtensionsRepository.incrementSeatsUsed(100L)).thenReturn(1); - assertThatThrownBy(() -> service.acceptInvitation("tok-123", u)) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Transfer leadership"); + service.acceptInvitation("tok-123", u); + + assertThat(inv.getStatus()).isEqualTo(InvitationStatus.ACCEPTED); + verify(userRepository).updateUserTeamId(5L, 100L); } @Test - @DisplayName( - "happy path: leaves personal team, deletes it, joins new team, increments seats") - void success_migratesFromPersonalTeam() { + @DisplayName("parks the home team (keeps it) and joins the new team, incrementing seats") + void success_parksHomeTeamAndJoins() { User u = user(5L, "b@x.com", "bob"); Team newTeam = team(100L, "Acme"); - Team personal = team(200L, "My Team"); + Team home = team(200L, "My Team"); User inviter = user(1L, "a@x.com", "alice"); TeamInvitation inv = pendingInvitation(newTeam, inviter, "b@x.com"); - TeamMembership personalMembership = membership(personal, u, TeamRole.LEADER); + TeamMembership homeMembership = membership(home, u, TeamRole.LEADER); when(userRepository.findById(5L)).thenReturn(Optional.of(u)); when(invitationRepository.findByInvitationToken("tok-123")) .thenReturn(Optional.of(inv)); when(saasTeamExtensionService.hasAvailableSeats(newTeam)).thenReturn(true); - // assertCanLeave... iterates memberships; personal team is skipped. - when(membershipRepository.findByUserId(5L)) - .thenReturn(List.of(personalMembership)) - .thenReturn(List.of(personalMembership)); - when(saasTeamExtensionService.isPersonal(personal)).thenReturn(true); - when(membershipRepository.countByTeamId(200L)).thenReturn(0L); + when(saasUserExtensionService.getHomeTeamId(u)).thenReturn(200L); + when(membershipRepository.findByUserId(5L)).thenReturn(List.of(homeMembership)); when(saasTeamExtensionsRepository.incrementSeatsUsed(100L)).thenReturn(1); service.acceptInvitation("tok-123", u); - verify(membershipRepository).delete(personalMembership); - verify(saasTeamExtensionsRepository).decrementSeatsUsed(200L); - verify(teamRepository).delete(personal); + // Home team + its membership are kept (parked), never deleted. + verify(membershipRepository, never()).delete(homeMembership); + verify(teamRepository, never()).delete(home); verify(userRepository).updateUserTeamId(5L, 100L); assertThat(inv.getStatus()).isEqualTo(InvitationStatus.ACCEPTED); ArgumentCaptor mcap = ArgumentCaptor.forClass(TeamMembership.class); @@ -803,7 +792,6 @@ class SaasTeamServiceTest { when(membershipRepository.findByUserId(5L)) .thenReturn(List.of(oldMembership)) .thenReturn(List.of(oldMembership)); - when(saasTeamExtensionService.isPersonal(oldTeam)).thenReturn(false); when(saasTeamExtensionsRepository.incrementSeatsUsed(100L)).thenReturn(1); service.acceptInvitation("tok-123", u); @@ -811,6 +799,34 @@ class SaasTeamServiceTest { verify(teamRepository, never()).delete(oldTeam); verify(userRepository).updateUserTeamId(5L, 100L); } + + @Test + @DisplayName("keeps a led team with other members (parks it) instead of orphaning it") + void soleLeaderOfSharedTeam_teamKeptNotOrphaned() { + User u = user(5L, "b@x.com", "bob"); + Team newTeam = team(100L, "Acme"); + Team shared = team(300L, "Bob's Org"); // u solely leads it; it has other members + TeamMembership sharedMembership = membership(shared, u, TeamRole.LEADER); + TeamInvitation inv = + pendingInvitation(newTeam, user(1L, "a@x.com", "alice"), "b@x.com"); + + when(userRepository.findById(5L)).thenReturn(Optional.of(u)); + when(invitationRepository.findByInvitationToken("tok-123")) + .thenReturn(Optional.of(inv)); + when(saasTeamExtensionService.hasAvailableSeats(newTeam)).thenReturn(true); + when(membershipRepository.findByUserId(5L)).thenReturn(List.of(sharedMembership)); + // Shared team: sole leader, but >1 member - leaving would orphan the other member. + when(membershipRepository.countByTeamId(300L)).thenReturn(2L); + when(membershipRepository.countByTeamIdAndRole(300L, TeamRole.LEADER)).thenReturn(1L); + when(saasTeamExtensionsRepository.incrementSeatsUsed(100L)).thenReturn(1); + + service.acceptInvitation("tok-123", u); + + // The led team is parked: its membership is kept, not deleted. + verify(membershipRepository, never()).delete(sharedMembership); + verify(userRepository).updateUserTeamId(5L, 100L); + assertThat(inv.getStatus()).isEqualTo(InvitationStatus.ACCEPTED); + } } // ============================================================================================= @@ -987,8 +1003,8 @@ class SaasTeamServiceTest { } @Test - @DisplayName("removes the member, decrements seats, makes a personal team, downgrades") - void success_removesMemberAndDeletesEmptyTeam() { + @DisplayName("removes the member, decrements seats, returns them home; no team deletion") + void success_removesMemberReturnsHome() { User remover = user(1L, "a@x.com", "alice"); User target = user(2L, "b@x.com", "bob"); Team t = team(teamId, "Acme"); @@ -1001,19 +1017,16 @@ class SaasTeamServiceTest { .thenReturn(List.of(leaderM)); when(membershipRepository.findByTeamIdAndUserId(teamId, 2L)) .thenReturn(Optional.of(targetM)); - when(teamRepository.findById(teamId)).thenReturn(Optional.of(t)); - // createPersonalTeam + downgradeUserToFree both refetch the removed user by id. + // No home stubbed for the removed user -> returnUserToHome mints a fresh personal home. // target has no PRO authority, so downgrade hits the early return. stubCreatePersonalTeam(target, 500L); - // team becomes empty + non-personal -> deleted - when(saasTeamExtensionService.isPersonal(t)).thenReturn(false); - when(membershipRepository.countByTeamId(teamId)).thenReturn(0L); service.removeTeamMember(teamId, 2L, remover); verify(membershipRepository).delete(targetM); verify(saasTeamExtensionsRepository).decrementSeatsUsed(teamId); - verify(teamRepository).delete(t); + // Teams are durable now - the emptied team is not deleted. + verify(teamRepository, never()).delete(any()); } @Test @@ -1031,10 +1044,7 @@ class SaasTeamServiceTest { .thenReturn(List.of(leaderM)); when(membershipRepository.findByTeamIdAndUserId(teamId, 2L)) .thenReturn(Optional.of(targetM)); - when(teamRepository.findById(teamId)).thenReturn(Optional.of(t)); stubCreatePersonalTeam(target, 500L); - when(saasTeamExtensionService.isPersonal(t)).thenReturn(false); - when(membershipRepository.countByTeamId(teamId)).thenReturn(2L); service.removeTeamMember(teamId, 2L, remover); @@ -1078,28 +1088,27 @@ class SaasTeamServiceTest { } @Test - @DisplayName("member leaves: deletes membership, decrements, makes personal team") + @DisplayName("member leaves: deletes membership, decrements, returns them home") void memberLeaves_success() { User u = user(1L, "a@x.com", "alice"); Team t = team(teamId, "Acme"); TeamMembership memberM = membership(t, u, TeamRole.MEMBER); when(membershipRepository.findByTeamIdAndUserId(teamId, 1L)) .thenReturn(Optional.of(memberM)); - when(teamRepository.findById(teamId)).thenReturn(Optional.of(t)); + // No home stubbed -> returnUserToHome mints a fresh personal home team. stubCreatePersonalTeam(u, 500L); - when(saasTeamExtensionService.isPersonal(t)).thenReturn(true); service.leaveTeam(teamId, u); verify(membershipRepository).delete(memberM); verify(saasTeamExtensionsRepository).decrementSeatsUsed(teamId); - // Personal team is never deleted on leave. + // Teams are durable now; none is deleted on leave. verify(teamRepository, never()).delete(any()); } @Test - @DisplayName("leader leaves when another leader remains: deletes empty non-personal team") - void leaderLeavesWithCoLeader_deletesEmptyTeam() { + @DisplayName("co-leader leaves and returns home; the team is durable (not deleted)") + void leaderLeavesWithCoLeader_returnsHome() { User u = user(1L, "a@x.com", "alice"); Team t = team(teamId, "Acme"); TeamMembership leaderM = membership(t, u, TeamRole.LEADER); @@ -1108,14 +1117,12 @@ class SaasTeamServiceTest { .thenReturn(Optional.of(leaderM)); when(membershipRepository.findByTeamIdAndRole(teamId, TeamRole.LEADER)) .thenReturn(List.of(leaderM, coLeaderM)); - when(teamRepository.findById(teamId)).thenReturn(Optional.of(t)); stubCreatePersonalTeam(u, 500L); - when(saasTeamExtensionService.isPersonal(t)).thenReturn(false); - when(membershipRepository.countByTeamId(teamId)).thenReturn(0L); service.leaveTeam(teamId, u); - verify(teamRepository).delete(t); + verify(membershipRepository).delete(leaderM); + verify(teamRepository, never()).delete(any()); } @Test @@ -1126,9 +1133,7 @@ class SaasTeamServiceTest { TeamMembership memberM = membership(t, u, TeamRole.MEMBER); when(membershipRepository.findByTeamIdAndUserId(teamId, 1L)) .thenReturn(Optional.of(memberM)); - when(teamRepository.findById(teamId)).thenReturn(Optional.of(t)); stubTeamSave(500L); - when(saasTeamExtensionService.isPersonal(t)).thenReturn(true); // Both createPersonalTeam and downgradeUserToFree refetch by id; return the PRO user // with an active sub -> keep PRO. User proRefetch = proUser(1L, "a@x.com", "alice"); @@ -1150,9 +1155,7 @@ class SaasTeamServiceTest { TeamMembership memberM = membership(t, u, TeamRole.MEMBER); when(membershipRepository.findByTeamIdAndUserId(teamId, 1L)) .thenReturn(Optional.of(memberM)); - when(teamRepository.findById(teamId)).thenReturn(Optional.of(t)); stubTeamSave(500L); - when(saasTeamExtensionService.isPersonal(t)).thenReturn(true); User proRefetch = proUser(1L, "a@x.com", "alice"); proRefetch.setSupabaseId(SUPABASE_ID); when(userRepository.findById(1L)).thenReturn(Optional.of(proRefetch)); @@ -1172,9 +1175,7 @@ class SaasTeamServiceTest { TeamMembership memberM = membership(t, u, TeamRole.MEMBER); when(membershipRepository.findByTeamIdAndUserId(teamId, 1L)) .thenReturn(Optional.of(memberM)); - when(teamRepository.findById(teamId)).thenReturn(Optional.of(t)); stubTeamSave(500L); - when(saasTeamExtensionService.isPersonal(t)).thenReturn(true); // PRO user without supabaseId skips the subscription check and downgrades. User proRefetch = proUser(1L, "a@x.com", "alice"); when(userRepository.findById(1L)).thenReturn(Optional.of(proRefetch)); @@ -1192,9 +1193,7 @@ class SaasTeamServiceTest { TeamMembership memberM = membership(t, u, TeamRole.MEMBER); when(membershipRepository.findByTeamIdAndUserId(teamId, 1L)) .thenReturn(Optional.of(memberM)); - when(teamRepository.findById(teamId)).thenReturn(Optional.of(t)); stubTeamSave(500L); - when(saasTeamExtensionService.isPersonal(t)).thenReturn(true); User proRefetch = proUser(1L, "a@x.com", "alice"); proRefetch.setSupabaseId(SUPABASE_ID); when(userRepository.findById(1L)).thenReturn(Optional.of(proRefetch)); @@ -1472,6 +1471,36 @@ class SaasTeamServiceTest { assertThat(invitation.getStatus()).isEqualTo(InvitationStatus.ACCEPTED); } + @Test + @DisplayName("does not block when the only linked instance is on the parked home team") + void passesGuardWhenLinkedInstanceIsOnHomeTeam() { + User joiner = user(USER_ID, EMAIL, EMAIL); + Team homeTeam = team(OLD_TEAM_ID, "home-team"); + Team newTeam = team(NEW_TEAM_ID, "new-team"); + TeamInvitation invitation = pendingInvitation(newTeam, joiner); + TeamMembership homeMembership = membership(homeTeam, joiner, TeamRole.LEADER); + + when(userRepository.findById(USER_ID)).thenReturn(Optional.of(joiner)); + when(invitationRepository.findByInvitationToken(TOKEN)) + .thenReturn(Optional.of(invitation)); + when(saasTeamExtensionService.hasAvailableSeats(newTeam)).thenReturn(true); + // The linked team IS the durable home, so the join parks it rather than orphaning it. + when(saasUserExtensionService.getHomeTeamId(joiner)).thenReturn(OLD_TEAM_ID); + when(membershipRepository.findByUserId(USER_ID)).thenReturn(List.of(homeMembership)); + // Home still carries a non-revoked linked instance - the old guard wrongly blocked + // here. + when(linkedInstanceRepository.countByTeamIdAndRevokedAtIsNull(OLD_TEAM_ID)) + .thenReturn(1L); + when(saasTeamExtensionsRepository.incrementSeatsUsed(NEW_TEAM_ID)).thenReturn(1); + + service.acceptInvitation(TOKEN, joiner); + + // Home parked (never deleted), user re-pointed to the new team, invite accepted. + verify(membershipRepository, never()).delete(homeMembership); + verify(userRepository).updateUserTeamId(USER_ID, NEW_TEAM_ID); + assertThat(invitation.getStatus()).isEqualTo(InvitationStatus.ACCEPTED); + } + private TeamInvitation pendingInvitation(Team team, User invitee) { TeamInvitation inv = new TeamInvitation(); inv.setTeam(team); diff --git a/app/saas/src/test/java/stirling/software/saas/service/SaasUserExtensionServiceTest.java b/app/saas/src/test/java/stirling/software/saas/service/SaasUserExtensionServiceTest.java index 4bf9b06baa..4ef7569bce 100644 --- a/app/saas/src/test/java/stirling/software/saas/service/SaasUserExtensionServiceTest.java +++ b/app/saas/src/test/java/stirling/software/saas/service/SaasUserExtensionServiceTest.java @@ -214,4 +214,42 @@ class SaasUserExtensionServiceTest { assertThat(captor.getValue().getApiKeyFirstUsedAt()).isNotNull(); } } + + @Nested + @DisplayName("home team") + class HomeTeam { + + @Test + @DisplayName("getHomeTeamId returns the stored id when a row exists") + void existing_returnsId() { + SaasUserExtensions ext = new SaasUserExtensions(user); + ext.setHomeTeamId(7L); + when(repository.findByUserId(USER_ID)).thenReturn(Optional.of(ext)); + + assertThat(service.getHomeTeamId(user)).isEqualTo(7L); + } + + @Test + @DisplayName("getHomeTeamId returns null when no row exists") + void missing_returnsNull() { + when(repository.findByUserId(USER_ID)).thenReturn(Optional.empty()); + + assertThat(service.getHomeTeamId(user)).isNull(); + verify(repository, never()).save(any()); + } + + @Test + @DisplayName("setHomeTeamId writes the id on the existing row and saves") + void set_updatesExistingRow() { + SaasUserExtensions ext = new SaasUserExtensions(user); + when(repository.findByUserId(USER_ID)).thenReturn(Optional.of(ext)); + when(repository.save(any(SaasUserExtensions.class))) + .thenAnswer(inv -> inv.getArgument(0)); + + service.setHomeTeamId(user, 9L); + + assertThat(ext.getHomeTeamId()).isEqualTo(9L); + verify(repository).save(ext); + } + } } diff --git a/frontend/editor/src/portal-saas/auth/PortalAuthBoundary.test.tsx b/frontend/editor/src/portal-saas/auth/PortalAuthBoundary.test.tsx index 367a3c59f7..474089f33c 100644 --- a/frontend/editor/src/portal-saas/auth/PortalAuthBoundary.test.tsx +++ b/frontend/editor/src/portal-saas/auth/PortalAuthBoundary.test.tsx @@ -3,15 +3,21 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { render, screen } from "@testing-library/react"; import { allowConsole } from "@app/tests/failOnConsole"; -// Controllable auth state for the mocked provider. +// Controllable auth state for the mocked provider. `portalAccess` is the collapsed +// context value (raw user.portalAccess ?? isAdminRole(role)); `user.portalAccess` +// is the raw tri-state (undefined until /api/v1/auth/me resolves). const authState: { session: unknown; loading: boolean; isAnonymous: boolean; + portalAccess: boolean; + user: { portalAccess?: boolean } | null; } = { session: null, loading: false, isAnonymous: false, + portalAccess: false, + user: null, }; vi.mock("@app/auth", () => ({ @@ -24,57 +30,76 @@ vi.mock("@portal/auth/saasSupabase", () => ({ ensureSaasSupabase: vi.fn() })); import { PortalAuthBoundary } from "@portal/auth/PortalAuthBoundary"; +function renderBoundary() { + render( + +

PORTAL
+ , + ); +} + describe("PortalAuthBoundary — SaaS", () => { beforeEach(() => { authState.session = null; authState.loading = false; authState.isAnonymous = false; + authState.portalAccess = false; + authState.user = null; }); - it("renders the portal when a real (non-guest) Supabase session is present", () => { + it("renders the portal for a real session WITH portal access", () => { authState.session = { user: { id: "u1" }, access_token: "tok" }; - render( - -
PORTAL
- , - ); + authState.portalAccess = true; + authState.user = { portalAccess: true }; + renderBoundary(); expect(screen.getByTestId("portal")).toBeInTheDocument(); }); + it("renders the portal for an admin (collapsed access true before /me resolves)", () => { + authState.session = { user: { id: "admin" }, access_token: "tok" }; + authState.portalAccess = true; // isAdminRole fallback + authState.user = {}; // raw portalAccess still undefined + renderBoundary(); + expect(screen.getByTestId("portal")).toBeInTheDocument(); + }); + + it("gates a real session WITHOUT portal access (member) and bounces to the editor", () => { + authState.session = { user: { id: "member" }, access_token: "tok" }; + authState.portalAccess = false; + authState.user = { portalAccess: false }; + allowConsole.error(/not implemented|navigation/i); + renderBoundary(); + expect(screen.queryByTestId("portal")).not.toBeInTheDocument(); + }); + + it("waits (no portal, no redirect) while portal access is still resolving", () => { + authState.session = { user: { id: "u1" }, access_token: "tok" }; + authState.portalAccess = false; + authState.user = {}; // /me not back yet -> raw portalAccess undefined + // Deliberately do NOT allow a navigation error: if the gate wrongly bounced + // this still-resolving user, jsdom's navigation warning would fail the test. + renderBoundary(); + expect(screen.queryByTestId("portal")).not.toBeInTheDocument(); + }); + it("gates (does not render the portal) for an anonymous guest session", () => { authState.session = { user: { id: "guest" }, access_token: "tok" }; authState.isAnonymous = true; - // The gate bounces a guest to the editor; jsdom doesn't implement - // navigation, so absorb that incidental warning. allowConsole.error(/not implemented|navigation/i); - render( - -
PORTAL
- , - ); + renderBoundary(); expect(screen.queryByTestId("portal")).not.toBeInTheDocument(); }); it("gates (does not render the portal) when there is no session", () => { authState.session = null; - // The gate bounces to /login; jsdom doesn't implement navigation, so absorb - // that incidental warning rather than fail the console guard. allowConsole.error(/not implemented|navigation/i); - render( - -
PORTAL
- , - ); + renderBoundary(); expect(screen.queryByTestId("portal")).not.toBeInTheDocument(); }); it("gates while the session is still resolving", () => { authState.loading = true; - render( - -
PORTAL
- , - ); + renderBoundary(); expect(screen.queryByTestId("portal")).not.toBeInTheDocument(); }); }); diff --git a/frontend/editor/src/portal-saas/auth/PortalAuthBoundary.tsx b/frontend/editor/src/portal-saas/auth/PortalAuthBoundary.tsx index 8ab2c58dcd..8824883baa 100644 --- a/frontend/editor/src/portal-saas/auth/PortalAuthBoundary.tsx +++ b/frontend/editor/src/portal-saas/auth/PortalAuthBoundary.tsx @@ -22,22 +22,40 @@ function FullScreen({ children }: { children: ReactNode }) { } /** - * SaaS gate: viewing your own usage is not admin-gated, so any real (signed-in, - * non-guest) account may enter - deliberately laxer than the self-hosted - * RequirePortalAccess admin gate. But an anonymous guest session has no account - * to view or manage, so it is not eligible: bounce it to the editor (where a - * guest can sign up), mirroring the self-hosted forbidden path. No session at - * all -> the editor's Supabase login, which returns here signed in. + * SaaS portal gate: enter only with backend-granted portal/processor access + * (`portalAccess`, from /api/v1/auth/me), mirroring self-hosted RequirePortalAccess. + * The old "any signed-in account may enter" behaviour let team members without + * access into the Processor. + * + * portalAccess resolves *after* the session does (/me runs once `loading` is + * already false), so treat "real session, access not yet known" (raw + * user.portalAccess still undefined, and not admin-by-role) as still-loading + * rather than bouncing a legitimate user mid-load. Once settled: no session -> + * login; a guest or a real account without access -> the free editor. */ function SaasPortalGate({ children }: { children: ReactNode }) { - const { session, loading, isAnonymous } = useAuth(); - const blocked = !loading && (!session || isAnonymous); + const { session, loading, isAnonymous, portalAccess, user } = useAuth(); + + const accessPending = + !!session && + !isAnonymous && + !portalAccess && + user?.portalAccess === undefined; + const settling = loading || accessPending; + + const redirectTo = settling + ? null + : !session + ? withBasePath("/login") + : isAnonymous || !portalAccess + ? EDITOR_URL + : null; + useEffect(() => { - if (!blocked) return; - // Guest (has a session but anonymous) -> editor; no session -> login. - window.location.href = session ? EDITOR_URL : withBasePath("/login"); - }, [blocked, session]); - if (loading || blocked) { + if (redirectTo) window.location.href = redirectTo; + }, [redirectTo]); + + if (settling || redirectTo) { return ( diff --git a/frontend/editor/src/proprietary/auth/supabase/UseSession.tsx b/frontend/editor/src/proprietary/auth/supabase/UseSession.tsx index 9d2b3f2682..6bcc6b501e 100644 --- a/frontend/editor/src/proprietary/auth/supabase/UseSession.tsx +++ b/frontend/editor/src/proprietary/auth/supabase/UseSession.tsx @@ -141,56 +141,77 @@ export function SupabaseAuthProvider({ }; }, []); - // Enrich with backend truth: grant-based portal access and team leadership - // come from /api/v1/auth/me, not Supabase claims. Best-effort; the - // isAdminRole fallback below covers failures and anonymous sessions. + // Enrich with backend truth: grant-based portal access and team leadership come from + // /api/v1/auth/me, not Supabase claims. Portal/Processor access is active-team-dependent on the + // backend, and a team switch made elsewhere doesn't change our session object - so besides the + // initial load we re-validate whenever the tab regains focus, else a user whose access dropped + // would keep seeing the Processor until a full reload. Best-effort; the isAdminRole fallback + // below covers failures and anonymous sessions. useEffect(() => { const token = session?.access_token; const sessionUser = session?.user; - if ( - !token || - !sessionUser || - sessionUser.is_anonymous || - sessionUser.portalAccess !== undefined - ) { + if (!token || !sessionUser || sessionUser.is_anonymous) { return; } let cancelled = false; - void fetch("/api/v1/auth/me", { - headers: { - Authorization: `Bearer ${token}`, - Accept: "application/json", - }, - }) - .then((res) => (res.ok ? res.json() : null)) - .then( - ( - data: { - user?: { portalAccess?: boolean; teamLead?: boolean }; - } | null, - ) => { - if (cancelled || !data?.user) return; + const loadAccess = () => { + void fetch("/api/v1/auth/me", { + headers: { + Authorization: `Bearer ${token}`, + Accept: "application/json", + }, + }) + .then((res) => (res.ok ? res.json() : null)) + .then( + ( + data: { + user?: { portalAccess?: boolean; teamLead?: boolean }; + } | null, + ) => { + if (cancelled || !data?.user) return; + setSession((prev) => + prev + ? { + ...prev, + user: { + ...prev.user, + portalAccess: data.user?.portalAccess, + teamLead: data.user?.teamLead, + }, + } + : prev, + ); + }, + ) + .catch(() => { + // Backend unreachable or /me unsupported: resolve portalAccess to the role-based + // fallback so gates awaiting it don't hang on a spinner. This deliberately ignores + // grant-based access (a non-admin grant-holder is denied while /me is down) - grants + // can't be known without /me, so we fail safe; a later focus refetch recovers it. + if (cancelled) return; setSession((prev) => - prev + prev && prev.user.portalAccess === undefined ? { ...prev, user: { ...prev.user, - portalAccess: data.user?.portalAccess, - teamLead: data.user?.teamLead, + portalAccess: isAdminRole(prev.user.role), }, } : prev, ); - }, - ) - .catch(() => { - // Backend unreachable or /me unsupported: keep the claim fallback. - }); + }); + }; + loadAccess(); + const onVisible = () => { + if (document.visibilityState === "visible") loadAccess(); + }; + document.addEventListener("visibilitychange", onVisible); return () => { cancelled = true; + document.removeEventListener("visibilitychange", onVisible); }; - }, [session?.access_token, session?.user?.id, session?.user?.portalAccess]); + }, [session?.access_token, session?.user?.id]); const user = session?.user ?? null; const value: AuthContextValue = {