diff --git a/app/(auth)/auth/callback/route.ts b/app/(auth)/auth/callback/route.ts index 224c9ac8..124cd004 100644 --- a/app/(auth)/auth/callback/route.ts +++ b/app/(auth)/auth/callback/route.ts @@ -113,12 +113,18 @@ export async function GET(request: NextRequest) { source: 'direct', }) - // Set active company - await serviceClient.from('user_preferences').upsert({ + // Set active company. Non-fatal on failure — middleware falls + // back to the membership created above — but log so silent + // persistence failures (#701) are observable. + const { error: prefError } = await serviceClient.from('user_preferences').upsert({ user_id: user.id, active_company_id: invite.company_id, }, { onConflict: 'user_id' }) + if (prefError) { + console.error('[auth/callback] failed to set active company', prefError) + } + // Mark invite as accepted await serviceClient .from('company_invitations') diff --git a/app/api/team/accept/route.ts b/app/api/team/accept/route.ts index 5c4a137a..9c020e71 100644 --- a/app/api/team/accept/route.ts +++ b/app/api/team/accept/route.ts @@ -112,14 +112,20 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: 'Kunde inte lägga till medlem.' }, { status: 500 }) } - // Set active company - await serviceClient + // Set active company. Non-fatal on failure — the membership insert already + // succeeded and middleware falls back to it — but log so silent + // persistence failures (#701) are observable. + const { error: prefError } = await serviceClient .from('user_preferences') .upsert({ user_id: user.id, active_company_id: companyInvite.company_id, }, { onConflict: 'user_id' }) + if (prefError) { + console.error('[team/accept] failed to set active company', prefError) + } + // Mark invite as accepted await serviceClient .from('company_invitations') diff --git a/components/dashboard/CompanySwitcher.tsx b/components/dashboard/CompanySwitcher.tsx index 4419f105..f7afb267 100644 --- a/components/dashboard/CompanySwitcher.tsx +++ b/components/dashboard/CompanySwitcher.tsx @@ -7,11 +7,13 @@ import { useTranslations } from 'next-intl' import { cn } from '@/lib/utils' import { useCompany } from '@/contexts/CompanyContext' import { switchCompany } from '@/lib/company/actions' +import { useToast } from '@/components/ui/use-toast' import { Check, ChevronsUpDown, Plus, Loader2 } from 'lucide-react' export default function CompanySwitcher() { const { company, companies, isSandbox } = useCompany() const t = useTranslations('company_switcher') + const { toast } = useToast() const [open, setOpen] = useState(false) const [isPending, setIsPending] = useState(false) const triggerRef = useRef(null) @@ -82,6 +84,10 @@ export default function CompanySwitcher() { const result = await switchCompany(companyId) if (result.error) { setIsPending(false) + toast({ + title: t(result.error === 'not_member' ? 'error_no_access' : 'error_switch_failed'), + variant: 'destructive', + }) return } // Notify every other open tab of the same user so they hard-reload diff --git a/components/onboarding/BankIdCompanyPicker.tsx b/components/onboarding/BankIdCompanyPicker.tsx index 6efcc483..7794e249 100644 --- a/components/onboarding/BankIdCompanyPicker.tsx +++ b/components/onboarding/BankIdCompanyPicker.tsx @@ -89,7 +89,10 @@ export default function BankIdCompanyPicker({ setSetup({ kind: 'opening', companyId }) const result = await switchCompany(companyId) if (result.error) { - toast({ title: result.error, variant: 'destructive' }) + toast({ + title: t(result.error === 'not_member' ? 'error_no_access' : 'error_switch_failed'), + variant: 'destructive', + }) setSetup({ kind: 'idle' }) return } diff --git a/lib/company/__tests__/actions.test.ts b/lib/company/__tests__/actions.test.ts index 450f9a74..68c51cf9 100644 --- a/lib/company/__tests__/actions.test.ts +++ b/lib/company/__tests__/actions.test.ts @@ -8,14 +8,19 @@ vi.mock('@/lib/supabase/server', () => ({ createClient: vi.fn(), })) -vi.mock('@/lib/company/context', () => ({ +// Keep the real CompanyContextError so instanceof checks in switchCompany +// see the same class the tests throw. +vi.mock('@/lib/company/context', async (importOriginal) => ({ + ...(await importOriginal()), setActiveCompany: vi.fn().mockResolvedValue(undefined), })) import { createClient } from '@/lib/supabase/server' -import { createCompanyFromOnboarding } from '../actions' +import { setActiveCompany, CompanyContextError } from '@/lib/company/context' +import { createCompanyFromOnboarding, switchCompany } from '../actions' const mockCreateClient = vi.mocked(createClient) +const mockSetActiveCompany = vi.mocked(setActiveCompany) type CapturedCall = { table: string; method: string; args: unknown[] } @@ -80,6 +85,62 @@ beforeEach(() => { vi.clearAllMocks() }) +describe('switchCompany', () => { + it('returns {} when the switch persists', async () => { + const { supabase } = buildSupabase({ user: { id: 'user-1' } }) + mockCreateClient.mockResolvedValue(supabase as never) + + const result = await switchCompany('company-2') + + expect(result).toEqual({}) + expect(mockSetActiveCompany).toHaveBeenCalledWith(supabase, 'user-1', 'company-2') + }) + + it('returns Unauthorized when there is no user', async () => { + const { supabase } = buildSupabase({ user: null }) + mockCreateClient.mockResolvedValue(supabase as never) + + const result = await switchCompany('company-2') + + expect(result).toEqual({ error: 'Unauthorized' }) + expect(mockSetActiveCompany).not.toHaveBeenCalled() + }) + + it('maps a membership failure to the not_member code', async () => { + const { supabase } = buildSupabase({ user: { id: 'user-1' } }) + mockCreateClient.mockResolvedValue(supabase as never) + mockSetActiveCompany.mockRejectedValueOnce( + new CompanyContextError('User is not a member of this company', 'not_member'), + ) + + const result = await switchCompany('company-2') + + expect(result).toEqual({ error: 'not_member' }) + }) + + it('maps a failed user_preferences write to persist_failed, not a permissions error (#701)', async () => { + const { supabase } = buildSupabase({ user: { id: 'user-1' } }) + mockCreateClient.mockResolvedValue(supabase as never) + mockSetActiveCompany.mockRejectedValueOnce( + new CompanyContextError('Failed to persist active company: timeout', 'persist_failed'), + ) + + const result = await switchCompany('company-2') + + expect(result).toEqual({ error: 'persist_failed' }) + }) + + it('maps unexpected errors to persist_failed rather than claiming missing access', async () => { + const { supabase } = buildSupabase({ user: { id: 'user-1' } }) + mockCreateClient.mockResolvedValue(supabase as never) + mockSetActiveCompany.mockRejectedValueOnce(new Error('cookies unavailable')) + + const result = await switchCompany('company-2') + + expect(result).toEqual({ error: 'persist_failed' }) + }) +}) + describe('createCompanyFromOnboarding — org_number validation', () => { it('rejects malformed org_numbers at the guard boundary', async () => { const { supabase } = buildSupabase({ diff --git a/lib/company/__tests__/context.test.ts b/lib/company/__tests__/context.test.ts new file mode 100644 index 00000000..4fe2b497 --- /dev/null +++ b/lib/company/__tests__/context.test.ts @@ -0,0 +1,111 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +const { mockCookieSet } = vi.hoisted(() => ({ mockCookieSet: vi.fn() })) + +vi.mock('next/headers', () => ({ + cookies: vi.fn(async () => ({ set: mockCookieSet })), +})) + +import { setActiveCompany, CompanyContextError } from '../context' + +type CapturedCall = { table: string; method: string; args: unknown[] } + +/** + * Chainable Supabase mock (same approach as actions.test.ts): a chain method + * terminates with `results[table][method]` when seeded, otherwise keeps + * chaining. setActiveCompany ends both its queries on `.single()`, on + * different tables, so seeding `single` per table drives each branch. + */ +function buildSupabase(results: Record>) { + const calls: CapturedCall[] = [] + + function makeChain(table: string) { + const chain: Record = {} + const methods = ['select', 'eq', 'is', 'order', 'limit', 'maybeSingle', 'single', 'insert', 'upsert', 'delete', 'update'] + for (const m of methods) { + chain[m] = (...args: unknown[]) => { + calls.push({ table, method: m, args }) + const terminal = results[table]?.[m] + if (terminal) { + return Promise.resolve({ data: terminal.data ?? null, error: terminal.error ?? null }) + } + return chain + } + } + chain.then = (resolve: (v: unknown) => void) => resolve({ data: null, error: null }) + return chain + } + + const supabase = { + from: vi.fn().mockImplementation((table: string) => makeChain(table)), + } + + return { supabase, calls } +} + +beforeEach(() => { + vi.clearAllMocks() +}) + +describe('setActiveCompany', () => { + it('throws not_member and never writes when the user lacks membership', async () => { + const { supabase, calls } = buildSupabase({ + company_members: { single: { data: null, error: { message: 'no rows' } } }, + }) + + const err = await setActiveCompany(supabase as never, 'user-1', 'company-2').catch((e) => e) + + expect(err).toBeInstanceOf(CompanyContextError) + expect(err.code).toBe('not_member') + expect(calls.find((c) => c.table === 'user_preferences')).toBeUndefined() + expect(mockCookieSet).not.toHaveBeenCalled() + }) + + it('throws persist_failed and does NOT set the cookie when the upsert errors (#701)', async () => { + const { supabase } = buildSupabase({ + company_members: { single: { data: { company_id: 'company-2' } } }, + user_preferences: { single: { data: null, error: { message: 'permission denied' } } }, + }) + + const err = await setActiveCompany(supabase as never, 'user-1', 'company-2').catch((e) => e) + + expect(err).toBeInstanceOf(CompanyContextError) + expect(err.code).toBe('persist_failed') + expect(err.message).toContain('permission denied') + // The exact regression from #701: cookie must not diverge from the DB. + expect(mockCookieSet).not.toHaveBeenCalled() + }) + + it('throws persist_failed when the read-back does not return the new company', async () => { + // An RLS-filtered UPDATE affects zero rows without an error; the + // read-back is what catches it. Simulate a stale/foreign row coming back. + const { supabase } = buildSupabase({ + company_members: { single: { data: { company_id: 'company-2' } } }, + user_preferences: { single: { data: { active_company_id: 'company-1' } } }, + }) + + const err = await setActiveCompany(supabase as never, 'user-1', 'company-2').catch((e) => e) + + expect(err).toBeInstanceOf(CompanyContextError) + expect(err.code).toBe('persist_failed') + expect(mockCookieSet).not.toHaveBeenCalled() + }) + + it('sets the cookie only after the write is verified', async () => { + const { supabase, calls } = buildSupabase({ + company_members: { single: { data: { company_id: 'company-2' } } }, + user_preferences: { single: { data: { active_company_id: 'company-2' } } }, + }) + + await expect(setActiveCompany(supabase as never, 'user-1', 'company-2')).resolves.toBeUndefined() + + const upsert = calls.find((c) => c.table === 'user_preferences' && c.method === 'upsert') + expect(upsert?.args[0]).toEqual({ user_id: 'user-1', active_company_id: 'company-2' }) + expect(mockCookieSet).toHaveBeenCalledTimes(1) + expect(mockCookieSet).toHaveBeenCalledWith( + 'gnubok-company-id', + 'company-2', + expect.objectContaining({ httpOnly: true, path: '/' }), + ) + }) +}) diff --git a/lib/company/actions.ts b/lib/company/actions.ts index 5a0c0415..3573c1e1 100644 --- a/lib/company/actions.ts +++ b/lib/company/actions.ts @@ -1,11 +1,17 @@ 'use server' import { createClient } from '@/lib/supabase/server' -import { setActiveCompany } from '@/lib/company/context' +import { setActiveCompany, CompanyContextError } from '@/lib/company/context' import { revalidatePath } from 'next/cache' import { normalizeOrgNumber } from '@/lib/company-lookup/normalize-org-number' import type { CompanyLookupResult } from '@/lib/company-lookup/types' +/** + * Switch the active company. Returns an error *code* (translated by the + * caller, same pattern as `org_number_invalid` below): 'not_member' when the + * user lacks membership, 'persist_failed' when the user_preferences write + * failed or could not be verified (#701). + */ export async function switchCompany(companyId: string): Promise<{ error?: string }> { const supabase = await createClient() const { data: { user } } = await supabase.auth.getUser() @@ -21,8 +27,14 @@ export async function switchCompany(companyId: string): Promise<{ error?: string // every React/router/fetch cache wholesale. revalidatePath would be a // no-op and would just race with the hard reload. return {} - } catch { - return { error: 'Du har inte tillgång till detta företag.' } + } catch (err) { + console.error('[switchCompany] failed', err) + if (err instanceof CompanyContextError && err.code === 'not_member') { + return { error: 'not_member' } + } + // persist_failed and anything unexpected: a retryable failure, not a + // permissions problem — don't tell the user they lack access. + return { error: 'persist_failed' } } } diff --git a/lib/company/context.ts b/lib/company/context.ts index 0f1aa13d..4e2b6c37 100644 --- a/lib/company/context.ts +++ b/lib/company/context.ts @@ -3,6 +3,21 @@ import { cookies } from 'next/headers' const COMPANY_COOKIE = 'gnubok-company-id' +/** + * Thrown by setActiveCompany so callers can tell a permissions problem + * ('not_member') apart from a failed/unverified database write + * ('persist_failed') and surface the right message to the user. + */ +export class CompanyContextError extends Error { + constructor( + message: string, + readonly code: 'not_member' | 'persist_failed' + ) { + super(message) + this.name = 'CompanyContextError' + } +} + /** * Get the active company ID for the authenticated user. * @@ -105,18 +120,39 @@ export async function setActiveCompany( .single() if (!membership) { - throw new Error('User is not a member of this company') + throw new CompanyContextError('User is not a member of this company', 'not_member') } - // Update user_preferences — this is the authoritative value RLS reads - await supabase + // Update user_preferences — this is the authoritative value RLS reads. + // The write MUST be verified: an UPDATE filtered out by RLS affects zero + // rows without raising an error, which previously made failed switches + // look successful while middleware kept resolving the old company (#701). + // `.select().single()` reads the row back, so both an explicit error and + // a silent zero-row write surface as a thrown CompanyContextError. + const { data: persisted, error: upsertError } = await supabase .from('user_preferences') .upsert( { user_id: userId, active_company_id: companyId }, { onConflict: 'user_id' } ) + .select('active_company_id') + .single() - // Refresh the cookie as a compat hint + if (upsertError) { + throw new CompanyContextError( + `Failed to persist active company: ${upsertError.message}`, + 'persist_failed' + ) + } + if (persisted?.active_company_id !== companyId) { + throw new CompanyContextError( + 'Active company write did not persist', + 'persist_failed' + ) + } + + // Refresh the cookie as a compat hint — only after the DB write is + // confirmed, so the cookie can never diverge from user_preferences. const cookieStore = await cookies() cookieStore.set(COMPANY_COOKIE, companyId, { path: '/', diff --git a/lib/supabase/middleware.ts b/lib/supabase/middleware.ts index 10a5597e..bccdce7c 100644 --- a/lib/supabase/middleware.ts +++ b/lib/supabase/middleware.ts @@ -281,12 +281,19 @@ async function resolveCompanyForMiddleware( // Write the fallback back to user_preferences so future RLS lookups // see the same active company without needing this fallback scan. - await supabase + // Non-fatal on failure: resolution for this request already succeeded, + // the write-back is an optimization — but log it so silent persistence + // failures (#701) are observable. + const { error: writeBackError } = await supabase .from('user_preferences') .upsert( { user_id: userId, active_company_id: firstCompany.company_id }, { onConflict: 'user_id' } ) + if (writeBackError) { + console.error('[middleware] active company write-back failed', writeBackError) + } + return { companyId: firstCompany.company_id, locale } } diff --git a/messages/en.json b/messages/en.json index 57c000eb..9451406e 100644 --- a/messages/en.json +++ b/messages/en.json @@ -918,7 +918,9 @@ "toast_create_failed_title": "Could not create company", "toast_create_failed_description": "Try again or add it manually.", "toast_welcome_title": "Welcome!", - "toast_company_ready": "Your company is ready." + "toast_company_ready": "Your company is ready.", + "error_no_access": "You don't have access to this company.", + "error_switch_failed": "The company switch could not be saved. Please try again." }, "companies_new": { "step1_title": "New company", @@ -3421,7 +3423,9 @@ "company_switcher": { "company_label": "Company", "default_company_name": "My business", - "add_company": "Add company" + "add_company": "Add company", + "error_no_access": "You don't have access to this company.", + "error_switch_failed": "The company switch could not be saved. Please try again." }, "transactions": { "page_title": "Transactions", diff --git a/messages/sv.json b/messages/sv.json index 7b3a06bb..ee0a985e 100644 --- a/messages/sv.json +++ b/messages/sv.json @@ -918,7 +918,9 @@ "toast_create_failed_title": "Kunde inte skapa företag", "toast_create_failed_description": "Försök igen eller lägg till manuellt.", "toast_welcome_title": "Välkommen!", - "toast_company_ready": "Ditt företag är nu redo." + "toast_company_ready": "Ditt företag är nu redo.", + "error_no_access": "Du har inte tillgång till detta företag.", + "error_switch_failed": "Företagsbytet kunde inte sparas. Försök igen." }, "companies_new": { "step1_title": "Nytt företag", @@ -3421,7 +3423,9 @@ "company_switcher": { "company_label": "Företag", "default_company_name": "Min verksamhet", - "add_company": "Lägg till företag" + "add_company": "Lägg till företag", + "error_no_access": "Du har inte tillgång till detta företag.", + "error_switch_failed": "Företagsbytet kunde inte sparas. Försök igen." }, "transactions": { "page_title": "Transaktioner",