diff --git a/apps/api/src/auth/auth.controller.spec.ts b/apps/api/src/auth/auth.controller.spec.ts new file mode 100644 index 0000000000..2831954d20 --- /dev/null +++ b/apps/api/src/auth/auth.controller.spec.ts @@ -0,0 +1,105 @@ +const mockUserFindUnique = jest.fn(); +const mockMemberFindMany = jest.fn(); +const mockMemberCount = jest.fn(); +const mockInvitationFindFirst = jest.fn(); + +jest.mock('@db', () => ({ + db: { + user: { findUnique: (...a: unknown[]) => mockUserFindUnique(...a) }, + member: { + findMany: (...a: unknown[]) => mockMemberFindMany(...a), + count: (...a: unknown[]) => mockMemberCount(...a), + }, + invitation: { findFirst: (...a: unknown[]) => mockInvitationFindFirst(...a) }, + }, +})); + +// Both guards import ./auth.server, which validates SECRET_KEY and pulls in +// better-auth/redis at module load. Stub them so importing the controller is +// hermetic — the test calls the method directly and never runs the guards. +jest.mock('./hybrid-auth.guard', () => ({ HybridAuthGuard: class {} })); +jest.mock('./permission.guard', () => ({ + PermissionGuard: class {}, + PERMISSIONS_KEY: 'permissions', +})); + +import { AuthController } from './auth.controller'; +import type { AuthContext } from './types'; + +const sessionContext = (): AuthContext => ({ + organizationId: 'org_1', + authType: 'session', + isApiKey: false, + isPlatformAdmin: false, + userRoles: null, + userId: 'user_1', + userEmail: 'user@example.com', +}); + +describe('AuthController.getMe — hasInactiveMembership (CS-569)', () => { + const controller = new AuthController(); + + beforeEach(() => { + jest.clearAllMocks(); + mockUserFindUnique.mockResolvedValue({ + id: 'user_1', + email: 'user@example.com', + name: 'User', + image: null, + role: 'user', + }); + mockInvitationFindFirst.mockResolvedValue(null); + }); + + it('returns hasInactiveMembership: false for a genuinely new user (no memberships at all)', async () => { + mockMemberFindMany.mockResolvedValue([]); + mockMemberCount.mockResolvedValue(0); + + const res = await controller.getMe(sessionContext()); + + expect(res.organizations).toEqual([]); + expect(res.hasInactiveMembership).toBe(false); + }); + + it('returns hasInactiveMembership: true for an offboarded user (no active org, only deactivated memberships)', async () => { + mockMemberFindMany.mockResolvedValue([]); // no ACTIVE memberships + mockMemberCount.mockResolvedValue(1); // one deactivated/inactive membership + + const res = await controller.getMe(sessionContext()); + + expect(res.organizations).toEqual([]); + expect(res.hasInactiveMembership).toBe(true); + // The count that distinguishes "offboarded" from "new" must match + // memberships that are deactivated OR no longer active. + expect(mockMemberCount).toHaveBeenCalledWith({ + where: { + userId: 'user_1', + OR: [{ deactivated: true }, { isActive: false }], + }, + }); + }); + + it('returns hasInactiveMembership: false for an active member', async () => { + mockMemberFindMany.mockResolvedValue([ + { + id: 'member_1', + role: 'owner', + organizationId: 'org_1', + organization: { + id: 'org_1', + name: 'Org', + logo: null, + onboardingCompleted: true, + hasAccess: true, + createdAt: new Date(), + }, + }, + ]); + mockMemberCount.mockResolvedValue(0); + + const res = await controller.getMe(sessionContext()); + + expect(res.organizations).toHaveLength(1); + expect(res.hasInactiveMembership).toBe(false); + }); +}); diff --git a/apps/api/src/auth/auth.controller.ts b/apps/api/src/auth/auth.controller.ts index 77d4126c3d..5848b72778 100644 --- a/apps/api/src/auth/auth.controller.ts +++ b/apps/api/src/auth/auth.controller.ts @@ -35,44 +35,55 @@ export class AuthController { return { user: null, organizations: [], pendingInvitation: null }; } - const [user, memberships, pendingInvitation] = await Promise.all([ - db.user.findUnique({ - where: { id: userId }, - select: { - id: true, - email: true, - name: true, - image: true, - role: true, - }, - }), - db.member.findMany({ - where: { userId, isActive: true, deactivated: false }, - select: { - id: true, - role: true, - organizationId: true, - organization: { - select: { - id: true, - name: true, - logo: true, - onboardingCompleted: true, - hasAccess: true, - createdAt: true, + const [user, memberships, pendingInvitation, inactiveMembershipCount] = + await Promise.all([ + db.user.findUnique({ + where: { id: userId }, + select: { + id: true, + email: true, + name: true, + image: true, + role: true, + }, + }), + db.member.findMany({ + where: { userId, isActive: true, deactivated: false }, + select: { + id: true, + role: true, + organizationId: true, + organization: { + select: { + id: true, + name: true, + logo: true, + onboardingCompleted: true, + hasAccess: true, + createdAt: true, + }, }, }, - }, - orderBy: { createdAt: 'desc' }, - }), - db.invitation.findFirst({ - where: { - email: authContext.userEmail ?? '', - status: 'pending', - }, - select: { id: true }, - }), - ]); + orderBy: { createdAt: 'desc' }, + }), + db.invitation.findFirst({ + where: { + email: authContext.userEmail ?? '', + status: 'pending', + }, + select: { id: true }, + }), + // Count memberships that exist but are no longer active (deactivated + // or removed). Lets the app tell a genuinely new user (no memberships + // at all → onboarding) apart from an offboarded user whose access was + // revoked (→ "access removed" instead of a spurious new org). CS-569. + db.member.count({ + where: { + userId, + OR: [{ deactivated: true }, { isActive: false }], + }, + }), + ]); return { user, @@ -82,6 +93,7 @@ export class AuthController { memberId: m.id, })), pendingInvitation, + hasInactiveMembership: inactiveMembershipCount > 0, }; } diff --git a/apps/api/src/trust-portal/trust-portal.service.spec.ts b/apps/api/src/trust-portal/trust-portal.service.spec.ts new file mode 100644 index 0000000000..7149f7cbb2 --- /dev/null +++ b/apps/api/src/trust-portal/trust-portal.service.spec.ts @@ -0,0 +1,203 @@ +import { db } from '@db'; +import { TrustPortalService } from './trust-portal.service'; + +jest.mock('@db', () => ({ + db: { + vendor: { + findMany: jest.fn(), + update: jest.fn(), + }, + globalVendors: { + findUnique: jest.fn(), + }, + }, + Prisma: {}, + TrustFramework: { + iso_27001: 'iso_27001', + iso_42001: 'iso_42001', + gdpr: 'gdpr', + hipaa: 'hipaa', + soc2_type1: 'soc2_type1', + soc2_type2: 'soc2_type2', + pci_dss: 'pci_dss', + nen_7510: 'nen_7510', + iso_9001: 'iso_9001', + }, +})); + +jest.mock('../app/s3', () => ({ + APP_AWS_ORG_ASSETS_BUCKET: 'org-assets', + s3Client: { send: jest.fn() }, + getSignedUrl: jest.fn(), +})); + +const mockDb = db as unknown as { + vendor: { findMany: jest.Mock; update: jest.Mock }; + globalVendors: { findUnique: jest.Mock }; +}; + +describe('TrustPortalService getAllVendorsWithSync compliance badges', () => { + const service = new TrustPortalService(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + // Runs a set of verified certifications through the badge-sync path and + // returns the resulting badge types (empty when nothing maps). + const badgeTypesFor = async ( + certifications: Array<{ type: string; status: string }>, + ): Promise => { + const baseVendor = { + id: 'vnd_1', + name: 'Acme', + description: null, + website: 'acme.com', + showOnTrustPortal: true, + logoUrl: 'https://logo.example/acme.png', + complianceBadges: null, + trustPortalOrder: 0, + }; + + mockDb.vendor.findMany.mockResolvedValue([baseVendor]); + mockDb.globalVendors.findUnique.mockResolvedValue({ + riskAssessmentData: { certifications }, + }); + mockDb.vendor.update.mockImplementation( + async ({ data }: { data: Record }) => ({ + ...baseVendor, + ...data, + }), + ); + + const result = await service.getAllVendorsWithSync('org_1'); + const badges = + (result[0].complianceBadges as unknown as Array<{ type: string }>) ?? []; + return badges.map((badge) => badge.type); + }; + + // Regression: Scaleway's GlobalVendors record lists "ISO/IEC 27001:2022", + // "HDS" and "GDPR Compliance" as verified, but the Trust Centre only showed + // GDPR. The "IEC" infix and ":2022" suffix caused the ISO 27001 certification + // to be dropped during badge sync, understating the vendor's posture. + it('maps "ISO/IEC 27001:2022" to the iso27001 badge alongside gdpr', async () => { + const baseVendor = { + id: 'vnd_scaleway', + name: 'Scaleway', + description: null, + website: 'scaleway.com', + showOnTrustPortal: true, + logoUrl: 'https://logo.example/scaleway.png', + complianceBadges: null, + trustPortalOrder: 0, + }; + + mockDb.vendor.findMany.mockResolvedValue([baseVendor]); + mockDb.globalVendors.findUnique.mockResolvedValue({ + riskAssessmentData: { + certifications: [ + { type: 'ISO/IEC 27001:2022', status: 'verified' }, + { type: 'HDS', status: 'verified' }, + { type: 'GDPR Compliance', status: 'verified' }, + ], + }, + }); + mockDb.vendor.update.mockImplementation( + async ({ data }: { data: Record }) => ({ + ...baseVendor, + ...data, + }), + ); + + const result = await service.getAllVendorsWithSync('org_1'); + + const badgeTypes = ( + result[0].complianceBadges as unknown as Array<{ type: string }> + ).map((badge) => badge.type); + + expect(badgeTypes).toContain('iso27001'); + expect(badgeTypes).toContain('gdpr'); + }); + + it('maps "ISO 9001:2015" to the iso9001 badge', async () => { + const badgeTypes = await badgeTypesFor([ + { type: 'ISO 9001:2015', status: 'verified' }, + ]); + + expect(badgeTypes).toContain('iso9001'); + }); + + // The "IEC" infix must be tolerated for every joint ISO/IEC standard, not + // just 27001 — otherwise ISO/IEC 42001 certifications are silently dropped. + it('maps "ISO/IEC 42001:2023" to the iso42001 badge', async () => { + const badgeTypes = await badgeTypesFor([ + { type: 'ISO/IEC 42001:2023', status: 'verified' }, + ]); + + expect(badgeTypes).toContain('iso42001'); + }); + + // Regression: a bare substring match on the digits ("9001") misclassified + // unrelated identifiers that merely contain them (e.g. a catalog id "19001") + // as ISO 9001, surfacing a certification the vendor never held. The match now + // requires the "ISO" prefix, so the digits alone are not enough. + it('does not misclassify an unrelated id containing "9001" as iso9001', async () => { + const badgeTypes = await badgeTypesFor([ + { type: 'Catalog 19001', status: 'verified' }, + { type: 'GDPR Compliance', status: 'verified' }, + ]); + + expect(badgeTypes).not.toContain('iso9001'); + // Sanity check that the sync path still ran and produced real badges. + expect(badgeTypes).toContain('gdpr'); + }); + + // Regression: the ISO number must not match as a prefix of a longer number. + // "ISO 90010" normalizes to "iso90010" and would otherwise match iso9001; + // only an optional 4-digit year (e.g. ":2015") may follow the standard number. + it('does not read "ISO 90010" as the iso9001 badge', async () => { + const badgeTypes = await badgeTypesFor([ + { type: 'ISO 90010', status: 'verified' }, + { type: 'GDPR Compliance', status: 'verified' }, + ]); + + expect(badgeTypes).not.toContain('iso9001'); + expect(badgeTypes).toContain('gdpr'); + }); + + // Real-world boundary: ISO/IEC 27017 (cloud security) shares the "2701" + // prefix with 27001 but is a distinct standard and must not earn its badge. + it('does not read "ISO/IEC 27017:2015" as the iso27001 badge', async () => { + const badgeTypes = await badgeTypesFor([ + { type: 'ISO/IEC 27017:2015', status: 'verified' }, + ]); + + expect(badgeTypes).not.toContain('iso27001'); + }); + + // ISO/IEC 27018 (PII in the cloud) is another distinct 2701x standard. + it('does not read "ISO/IEC 27018:2019" as the iso27001 badge', async () => { + const badgeTypes = await badgeTypesFor([ + { type: 'ISO/IEC 27018:2019', status: 'verified' }, + ]); + + expect(badgeTypes).not.toContain('iso27001'); + }); + + // Guardrail: a realistic mix of certification names (all carrying the "ISO" + // prefix, as produced by the extraction schema) must still map to every badge + // — the precision tightening must not drop any legitimate certification. + it('maps a realistic mix of certifications without dropping any', async () => { + const badgeTypes = await badgeTypesFor([ + { type: 'SOC 2 Type II', status: 'verified' }, + { type: 'ISO/IEC 27001:2013', status: 'verified' }, + { type: 'ISO 9001', status: 'verified' }, + { type: 'ISO/IEC 42001:2023', status: 'verified' }, + { type: 'GDPR Compliance', status: 'verified' }, + ]); + + expect(badgeTypes).toEqual( + expect.arrayContaining(['soc2', 'iso27001', 'iso9001', 'iso42001', 'gdpr']), + ); + }); +}); diff --git a/apps/api/src/trust-portal/trust-portal.service.ts b/apps/api/src/trust-portal/trust-portal.service.ts index f58099f8a1..00d9a05fe4 100644 --- a/apps/api/src/trust-portal/trust-portal.service.ts +++ b/apps/api/src/trust-portal/trust-portal.service.ts @@ -1950,10 +1950,8 @@ export class TrustPortalService { if (normalized.includes('soc2') || normalized.includes('soc 2')) return 'soc2'; - if (normalized.includes('iso27001') || normalized.includes('iso 27001')) - return 'iso27001'; - if (normalized.includes('iso42001') || normalized.includes('iso 42001')) - return 'iso42001'; + if (this.matchesIsoStandard(normalized, '27001')) return 'iso27001'; + if (this.matchesIsoStandard(normalized, '42001')) return 'iso42001'; if (normalized.includes('gdpr')) return 'gdpr'; if (normalized.includes('hipaa')) return 'hipaa'; if ( @@ -1964,12 +1962,35 @@ export class TrustPortalService { return 'pci_dss'; if (normalized.includes('nen7510') || normalized.includes('nen 7510')) return 'nen7510'; - if (normalized.includes('iso9001') || normalized.includes('iso 9001')) - return 'iso9001'; + if (this.matchesIsoStandard(normalized, '9001')) return 'iso9001'; return null; } + /** + * Whether a normalized cert string (lowercased, alphanumerics only) names the + * given ISO standard number. + * + * - Requires an "iso" / "iso iec" prefix, so unrelated ids that merely contain + * the digits ("19001", "127001") are not misclassified. + * - The optional "iec" handles joint ISO/IEC standards whose "IEC" infix would + * otherwise break the match ("ISO/IEC 27001:2022" -> "isoiec270012022"). + * - Allows an optional trailing 4-digit year ("ISO 9001:2015" -> "iso90012015") + * but forbids any other trailing digit, so a longer number is not read as a + * shorter standard ("ISO 90010" is not "ISO 9001", "ISO 27017" is not 27001). + * + * `standardNumber` is always a hard-coded digit literal — never user input — + * so building the RegExp from it carries no injection risk. + */ + private matchesIsoStandard( + normalized: string, + standardNumber: string, + ): boolean { + return new RegExp(`iso(?:iec)?${standardNumber}(?:\\d{4})?(?!\\d)`).test( + normalized, + ); + } + private generateLogoUrl(website: string | null): string | null { if (!website) return null; try { diff --git a/apps/app/src/app/(app)/setup/actions/create-organization-minimal.ts b/apps/app/src/app/(app)/setup/actions/create-organization-minimal.ts index 665fd5eb53..8fd3ff9398 100644 --- a/apps/app/src/app/(app)/setup/actions/create-organization-minimal.ts +++ b/apps/app/src/app/(app)/setup/actions/create-organization-minimal.ts @@ -43,6 +43,36 @@ export const createOrganizationMinimal = authActionClientWithoutOrg }; } + // CS-569 backstop: never create an org for a user whose only memberships + // are deactivated (0 active, >=1 inactive) — that user was offboarded and + // the routing layer already sends them to /auth/access-removed. This + // guards against a stale/replayed form POST slipping past that redirect + // and spawning a spurious empty org. Genuinely new users (no memberships) + // and users adding an additional org (have active memberships) pass. + const [activeMembershipCount, inactiveMembershipCount] = await Promise.all([ + db.member.count({ + where: { + userId: session.user.id, + isActive: true, + deactivated: false, + }, + }), + db.member.count({ + where: { + userId: session.user.id, + OR: [{ deactivated: true }, { isActive: false }], + }, + }), + ]); + + if (activeMembershipCount === 0 && inactiveMembershipCount > 0) { + return { + success: false, + error: + 'Your access to this organization was removed. Contact your administrator to be re-invited.', + }; + } + // Internal team accounts (verified @trycomp.ai) have access provisioned up front. const userEmail = session.user.email; const isVerifiedTryCompEmail = diff --git a/apps/app/src/app/(app)/setup/route.test.ts b/apps/app/src/app/(app)/setup/route.test.ts new file mode 100644 index 0000000000..5e792b1057 --- /dev/null +++ b/apps/app/src/app/(app)/setup/route.test.ts @@ -0,0 +1,80 @@ +import { vi } from 'vitest'; + +vi.mock('@/utils/auth', async () => { + const { mockAuth } = await import('@/test-utils/mocks/auth'); + return { auth: mockAuth }; +}); + +const { mockServerApiGet, mockCreateSetupSession } = vi.hoisted(() => ({ + mockServerApiGet: vi.fn(), + mockCreateSetupSession: vi.fn(async () => ({ id: 'setup_1' })), +})); +vi.mock('@/lib/api-server', () => ({ serverApi: { get: mockServerApiGet } })); +vi.mock('./lib/setup-session', () => ({ createSetupSession: mockCreateSetupSession })); + +import { describe, it, expect, beforeEach } from 'vitest'; +import { NextRequest } from 'next/server'; +import { redirect } from 'next/navigation'; +import { GET } from './route'; +import { mockAuthApi, createMockSession, createMockUser } from '@/test-utils/mocks/auth'; + +const mockRedirect = vi.mocked(redirect); + +interface MeData { + organizations: unknown[]; + pendingInvitation: { id: string } | null; + hasInactiveMembership?: boolean; +} +const mockMe = (data: MeData) => mockServerApiGet.mockResolvedValue({ data, status: 200 }); +const call = (url: string) => GET(new NextRequest(url)); + +describe('/setup route — CS-569 offboard guard vs invite precedence', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockCreateSetupSession.mockResolvedValue({ id: 'setup_1' }); + mockRedirect.mockImplementation((url: string) => { + throw new Error(`REDIRECT:${url}`); + }); + mockAuthApi.getSession.mockResolvedValue({ + session: createMockSession(), + user: createMockUser(), + }); + }); + + it('redirects an offboarded user with no invite to access-removed', async () => { + mockMe({ organizations: [], pendingInvitation: null, hasInactiveMembership: true }); + + await expect(call('http://localhost/setup')).rejects.toThrow('REDIRECT:/auth/access-removed'); + expect(mockCreateSetupSession).not.toHaveBeenCalled(); + }); + + it('does NOT block an offboarded user arriving via ?inviteCode= (invite handled downstream)', async () => { + // Regression (cubic): the guard must not pre-empt the legacy invite entry. + mockMe({ organizations: [], pendingInvitation: null, hasInactiveMembership: true }); + + await expect(call('http://localhost/setup?inviteCode=inv_abc')).rejects.toThrow( + 'REDIRECT:/setup/setup_1?inviteCode=inv_abc', + ); + // Guard is skipped entirely when an invite code is present. + expect(mockServerApiGet).not.toHaveBeenCalled(); + expect(mockRedirect).not.toHaveBeenCalledWith('/auth/access-removed'); + }); + + it('routes an offboarded user with a pending invitation to the invite, not access-removed', async () => { + mockMe({ + organizations: [], + pendingInvitation: { id: 'inv_xyz' }, + hasInactiveMembership: true, + }); + + await expect(call('http://localhost/setup')).rejects.toThrow('REDIRECT:/invite/inv_xyz'); + expect(mockRedirect).not.toHaveBeenCalledWith('/auth/access-removed'); + }); + + it('lets a genuinely new user through to onboarding', async () => { + mockMe({ organizations: [], pendingInvitation: null, hasInactiveMembership: false }); + + await expect(call('http://localhost/setup')).rejects.toThrow('REDIRECT:/setup/setup_1'); + expect(mockCreateSetupSession).toHaveBeenCalled(); + }); +}); diff --git a/apps/app/src/app/(app)/setup/route.ts b/apps/app/src/app/(app)/setup/route.ts index fe0471cd90..c8033d73d7 100644 --- a/apps/app/src/app/(app)/setup/route.ts +++ b/apps/app/src/app/(app)/setup/route.ts @@ -1,3 +1,5 @@ +import { serverApi } from '@/lib/api-server'; +import { resolveNoActiveOrgRedirect } from '@/lib/no-active-org-redirect'; import { auth } from '@/utils/auth'; import { headers } from 'next/headers'; import { redirect } from 'next/navigation'; @@ -17,6 +19,31 @@ export async function GET(request: NextRequest) { redirect(`/sign-in${queryString}`); } + // Invite flows take precedence over the CS-569 offboard guard: a raw + // ?inviteCode= is turned into /invite/{code} downstream by /setup/[setupId], + // so let it flow through untouched rather than pre-empting it here. Matches + // the downstream truthy check (an empty ?inviteCode= is not an invite). + const inviteCode = request.nextUrl.searchParams.get('inviteCode'); + if (!inviteCode) { + // CS-569: guard the onboarding loop at the setup entry too (direct nav or + // the create-additional intent). Reuse the same decision the landing page + // uses so the two can't drift. A non-null target means the user is invited + // or offboarded; `null` means a new user (or one with active orgs) who + // should fall through to onboarding. + const meRes = await serverApi.get<{ + organizations: unknown[]; + pendingInvitation: { id: string } | null; + hasInactiveMembership?: boolean; + }>('/v1/auth/me'); + const hasActiveOrg = (meRes.data?.organizations?.length ?? 0) > 0; + if (!hasActiveOrg) { + const target = resolveNoActiveOrgRedirect(meRes.data); + if (target) { + redirect(target); + } + } + } + const setupSession = await createSetupSession(session.user.id); redirect(`/setup/${setupSession.id}${queryString}`); } diff --git a/apps/app/src/app/(public)/auth/access-removed/page.tsx b/apps/app/src/app/(public)/auth/access-removed/page.tsx new file mode 100644 index 0000000000..5f2bbacd2f --- /dev/null +++ b/apps/app/src/app/(public)/auth/access-removed/page.tsx @@ -0,0 +1,34 @@ +import { SignOut } from '@/components/sign-out'; + +/** + * Shown to a signed-in user who has no active organization membership but was + * previously a member (all memberships deactivated/removed) — i.e. they were + * offboarded. Without this, such a user was silently dropped into onboarding, + * which spawned a spurious empty org and locked them into a loop (CS-569). + * + * The most common cause is a domain change: the old account keeps signing in + * after being offboarded. The primary action is therefore "sign out" so they + * can sign back in with their current account. + */ +export default function AccessRemovedPage() { + return ( +
+
+
+

+ {'Your access was removed'} +

+

+ { + 'This account is no longer a member of any organization. If your company recently changed email domains or offboarded this account, sign out and sign back in with your current account. Otherwise, contact your administrator to be re-invited.' + } +

+
+ +
+ +
+
+
+ ); +} diff --git a/apps/app/src/app/page.test.tsx b/apps/app/src/app/page.test.tsx new file mode 100644 index 0000000000..a2dc827269 --- /dev/null +++ b/apps/app/src/app/page.test.tsx @@ -0,0 +1,108 @@ +import { vi } from 'vitest'; + +// Mock the auth module before importing the page so getSession is controllable. +vi.mock('@/utils/auth', async () => { + const { mockAuth } = await import('@/test-utils/mocks/auth'); + return { auth: mockAuth }; +}); + +// serverApi.get('/v1/auth/me') drives every routing decision on this page. +// vi.hoisted so the fn exists when the hoisted vi.mock factory runs. +const { mockServerApiGet } = vi.hoisted(() => ({ mockServerApiGet: vi.fn() })); +vi.mock('@/lib/api-server', () => ({ + serverApi: { get: mockServerApiGet }, +})); + +// The page imports `db` from '@db/server' (only used for custom-role +// resolution, which the zero-org redirect paths never reach). Stub it so the +// test doesn't pull the real Prisma client. +vi.mock('@db/server', () => ({ db: {} })); + +// '@/lib/permissions' pulls in @trycompai/auth (a built package) and is only +// used past the zero-org redirects. Stub it so the test stays hermetic. +vi.mock('@/lib/permissions', () => ({ + getDefaultRoute: vi.fn(() => '/'), + mergePermissions: vi.fn(), + resolveBuiltInPermissions: vi.fn(() => ({ permissions: {}, customRoleNames: [] })), +})); + +import { describe, it, expect, beforeEach } from 'vitest'; +import { redirect } from 'next/navigation'; +import Page from './page'; +import { mockAuthApi, createMockSession, createMockUser } from '@/test-utils/mocks/auth'; + +const mockRedirect = vi.mocked(redirect); + +interface MeData { + organizations: unknown[]; + pendingInvitation: { id: string } | null; + hasInactiveMembership?: boolean; +} + +const mockMe = (data: MeData) => { + mockServerApiGet.mockResolvedValue({ data, status: 200 }); +}; + +describe('RootPage onboarding-loop guard (CS-569)', () => { + beforeEach(() => { + vi.clearAllMocks(); + // The real Next.js redirect() throws to halt rendering. Emulate that so + // execution stops at the first redirect and we can assert its target. + mockRedirect.mockImplementation((url: string) => { + throw new Error(`REDIRECT:${url}`); + }); + mockAuthApi.getSession.mockResolvedValue({ + session: createMockSession(), + user: createMockUser(), + }); + }); + + it('sends an offboarded user (no active org, has a deactivated membership) to the access-removed page, NOT onboarding', async () => { + // This is the bug: without the guard, this user was redirected to /setup, + // which silently spawned a spurious empty org and locked them into a loop. + mockMe({ organizations: [], pendingInvitation: null, hasInactiveMembership: true }); + + await expect(Page({ searchParams: Promise.resolve({}) })).rejects.toThrow( + 'REDIRECT:/auth/access-removed', + ); + + expect(mockRedirect).toHaveBeenCalledWith('/auth/access-removed'); + expect(mockRedirect).not.toHaveBeenCalledWith('/setup'); + }); + + it('still sends a genuinely new user (no memberships at all) to onboarding', async () => { + mockMe({ organizations: [], pendingInvitation: null, hasInactiveMembership: false }); + + await expect(Page({ searchParams: Promise.resolve({}) })).rejects.toThrow('REDIRECT:/setup'); + + expect(mockRedirect).toHaveBeenCalledWith('/setup'); + expect(mockRedirect).not.toHaveBeenCalledWith('/auth/access-removed'); + }); + + it('lets an explicit ?inviteCode= win over the offboard guard (hands off to /setup for downstream invite handling)', async () => { + // Regression (cubic P2): an offboarded user landing on /?inviteCode=... must + // still be able to accept the invite, not be short-circuited to access-removed. + mockMe({ organizations: [], pendingInvitation: null, hasInactiveMembership: true }); + + await expect( + Page({ searchParams: Promise.resolve({ inviteCode: 'inv_abc' }) }), + ).rejects.toThrow('REDIRECT:/setup?inviteCode=inv_abc'); + + expect(mockRedirect).not.toHaveBeenCalledWith('/auth/access-removed'); + }); + + it('prefers a pending invitation over the access-removed page', async () => { + mockMe({ + organizations: [], + pendingInvitation: { id: 'inv_abc123' }, + hasInactiveMembership: true, + }); + + await expect(Page({ searchParams: Promise.resolve({}) })).rejects.toThrow( + 'REDIRECT:/invite/inv_abc123', + ); + + expect(mockRedirect).toHaveBeenCalledWith('/invite/inv_abc123'); + expect(mockRedirect).not.toHaveBeenCalledWith('/auth/access-removed'); + }); +}); diff --git a/apps/app/src/app/page.tsx b/apps/app/src/app/page.tsx index d2124d7f40..ad486bb17c 100644 --- a/apps/app/src/app/page.tsx +++ b/apps/app/src/app/page.tsx @@ -1,4 +1,5 @@ import { serverApi } from '@/lib/api-server'; +import { resolveNoActiveOrgRedirect } from '@/lib/no-active-org-redirect'; import { getDefaultRoute, mergePermissions, resolveBuiltInPermissions } from '@/lib/permissions'; import { auth } from '@/utils/auth'; import { db } from '@db/server'; @@ -15,6 +16,9 @@ interface OrgInfo { interface AuthMeResponse { organizations: OrgInfo[]; pendingInvitation: { id: string } | null; + // True when the user has memberships that are all deactivated/removed + // (i.e. they were offboarded), used to avoid the onboarding loop (CS-569). + hasInactiveMembership?: boolean; } export default async function RootPage({ @@ -53,13 +57,20 @@ export default async function RootPage({ const meRes = await serverApi.get('/v1/auth/me'); const memberships = meRes.data?.organizations ?? []; - const pendingInvitation = meRes.data?.pendingInvitation; if (memberships.length === 0) { - if (pendingInvitation) { - return redirect(await buildUrlWithParams(`/invite/${pendingInvitation.id}`)); + // An explicit ?inviteCode= (the user is accepting a specific invitation) + // always wins over the offboard guard: hand off to /setup, which preserves + // the code and passes it through to /invite downstream. Mirrors the /setup + // route precedence so an offboarded user can still accept a valid invite. + const inviteCode = (await searchParams)?.inviteCode; + if (inviteCode) { + return redirect(await buildUrlWithParams('/setup')); } - return redirect(await buildUrlWithParams('/setup')); + // Otherwise route via the shared decision (pending invite > offboarded > + // new user). `null` = genuinely new user → onboarding. + const target = resolveNoActiveOrgRedirect(meRes.data); + return redirect(await buildUrlWithParams(target ?? '/setup')); } // Always use the org the user last switched to (stored in session) diff --git a/apps/app/src/lib/no-active-org-redirect.test.ts b/apps/app/src/lib/no-active-org-redirect.test.ts new file mode 100644 index 0000000000..d61eb879e5 --- /dev/null +++ b/apps/app/src/lib/no-active-org-redirect.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect } from 'vitest'; +import { resolveNoActiveOrgRedirect } from './no-active-org-redirect'; + +describe('resolveNoActiveOrgRedirect (CS-569)', () => { + it('routes a pending invitation to the invite — highest precedence', () => { + expect( + resolveNoActiveOrgRedirect({ + pendingInvitation: { id: 'inv_1' }, + hasInactiveMembership: true, + }), + ).toBe('/invite/inv_1'); + }); + + it('routes an offboarded user (no invite) to access-removed', () => { + expect( + resolveNoActiveOrgRedirect({ pendingInvitation: null, hasInactiveMembership: true }), + ).toBe('/auth/access-removed'); + }); + + it('returns null for a genuinely new user so the caller onboards them', () => { + expect( + resolveNoActiveOrgRedirect({ pendingInvitation: null, hasInactiveMembership: false }), + ).toBeNull(); + }); + + it('is null-safe when the /v1/auth/me payload is missing (degrade to onboarding)', () => { + expect(resolveNoActiveOrgRedirect(undefined)).toBeNull(); + expect(resolveNoActiveOrgRedirect(null)).toBeNull(); + expect(resolveNoActiveOrgRedirect({})).toBeNull(); + }); +}); diff --git a/apps/app/src/lib/no-active-org-redirect.ts b/apps/app/src/lib/no-active-org-redirect.ts new file mode 100644 index 0000000000..edc972273d --- /dev/null +++ b/apps/app/src/lib/no-active-org-redirect.ts @@ -0,0 +1,34 @@ +/** + * Decides where to send a signed-in user who has **no active organization**. + * + * This centralizes the CS-569 offboarding-loop guard so the root landing page + * (`app/page.tsx`) and the `/setup` entry route can't drift apart — divergent + * per-path membership handling is exactly what caused CS-569. + * + * Precedence: + * 1. A pending invitation is always the way in → `/invite/{id}`. + * 2. Offboarded (memberships exist but are all deactivated) → `/auth/access-removed` + * (do NOT drop them into onboarding; that spawns a spurious org + loop). + * 3. Genuinely new user (no memberships at all) → `null` (caller decides the + * onboarding target). + * + * Null-safe: a missing `/v1/auth/me` payload is treated as "new user" (`null`), + * so a transient API failure degrades to onboarding rather than a dead-end. + */ +export function resolveNoActiveOrgRedirect( + me: + | { + pendingInvitation?: { id: string } | null; + hasInactiveMembership?: boolean; + } + | null + | undefined, +): string | null { + if (me?.pendingInvitation) { + return `/invite/${me.pendingInvitation.id}`; + } + if (me?.hasInactiveMembership) { + return '/auth/access-removed'; + } + return null; +}