fix(company): validate user_preferences write when switching company (#708)
Fixes #701. setActiveCompany upserted active_company_id without checking the result, then set the gnubok-company-id cookie unconditionally. A failed write — including an RLS-filtered UPDATE, which affects zero rows without raising an error — looked like a successful switch: switchCompany returned {}, the UI hard-reloaded, and middleware (which reads user_preferences, not the cookie) resolved the old company. - setActiveCompany now verifies the upsert with .select().single() and throws a typed CompanyContextError ('not_member' | 'persist_failed'); the cookie is only set after the write is confirmed, so it can no longer diverge from the database. - switchCompany logs the failure and returns distinct error codes instead of reporting every failure as a permissions problem. - CompanySwitcher now shows a destructive toast on failure (it previously failed with no feedback); BankIdCompanyPicker translates the codes. Messages added to sv/en under company_switcher and select_company. - The remaining fire-and-forget user_preferences writers (middleware fallback write-back, team invite accept, auth callback invite accept) now check and log errors; non-fatal by design since each has a working fallback path. - New tests cover every failure mode, including cookie-not-set on a failed write and the silent zero-row write caught by the read-back. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
205610d200
commit
4253afc343
@@ -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')
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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<HTMLButtonElement>(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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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<typeof import('@/lib/company/context')>()),
|
||||
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({
|
||||
|
||||
@@ -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<string, Record<string, { data?: unknown; error?: unknown }>>) {
|
||||
const calls: CapturedCall[] = []
|
||||
|
||||
function makeChain(table: string) {
|
||||
const chain: Record<string, unknown> = {}
|
||||
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: '/' }),
|
||||
)
|
||||
})
|
||||
})
|
||||
+15
-3
@@ -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' }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+40
-4
@@ -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: '/',
|
||||
|
||||
@@ -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 }
|
||||
}
|
||||
|
||||
+6
-2
@@ -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",
|
||||
|
||||
+6
-2
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user