diff --git a/app/(dashboard)/invoices/page.tsx b/app/(dashboard)/invoices/page.tsx index a4964e02..a420c876 100644 --- a/app/(dashboard)/invoices/page.tsx +++ b/app/(dashboard)/invoices/page.tsx @@ -124,7 +124,12 @@ export default function InvoicesPage() { unpaid: invoices.filter(isOutstandingReceivable).length, unpaidAmount: invoices .filter(isOutstandingReceivable) - .reduce((sum, i) => sum + Number(i.total_sek || i.total), 0), + .reduce((sum, i) => { + if (i.currency === 'SEK') { + return sum + getDisplayTotal({ total: Number(i.total), currency: 'SEK' }, { ore_rounding: oreRounding }).displayed + } + return sum + Number(i.total_sek || i.total) + }, 0), overdue: invoices.filter((i) => i.status === 'overdue' && !i.credited_invoice_id).length, } diff --git a/app/api/sandbox/seed/route.ts b/app/api/sandbox/seed/route.ts index 65e1f951..9473454c 100644 --- a/app/api/sandbox/seed/route.ts +++ b/app/api/sandbox/seed/route.ts @@ -1,25 +1,45 @@ +import crypto from 'crypto' import { createClient } from '@/lib/supabase/server' import { NextResponse } from 'next/server' import { getActiveCompanyId } from '@/lib/company/context' import { createLogger } from '@/lib/logger' -const log = createLogger('sandbox:seed') - /** * POST /api/sandbox/seed * Seeds demo data for an anonymous sandbox user. * Only callable by anonymous users (is_anonymous === true). + * + * Defense-in-depth: also requires SANDBOX_ENABLED=true. The anonymous-user + * check is the primary control; the env guard exists so that if anonymous + * sign-in is ever turned on accidentally in a production environment, this + * destructive seed endpoint stays inert until an operator explicitly opts in. */ export async function POST() { + // Per-request logger so seed-failure entries are correlatable in the SIEM. + // Cannot reuse withRouteContext here — it requires an active company, but + // the sandbox seed runs *before* a company exists for the user. + const requestId = `req_${crypto.randomUUID()}` + const log = createLogger('sandbox:seed', { requestId }) + + if (process.env.SANDBOX_ENABLED !== 'true') { + return NextResponse.json( + { error: 'Sandbox is not enabled in this environment', requestId }, + { status: 403 }, + ) + } + const supabase = await createClient() const { data: { user } } = await supabase.auth.getUser() if (!user) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + return NextResponse.json({ error: 'Unauthorized', requestId }, { status: 401 }) } if (!user.is_anonymous) { - return NextResponse.json({ error: 'Sandbox is only available for anonymous users' }, { status: 403 }) + return NextResponse.json( + { error: 'Sandbox is only available for anonymous users', requestId }, + { status: 403 }, + ) } // Anonymous users start with no company. Create one before seeding. @@ -39,7 +59,7 @@ export async function POST() { if (companyError || !newCompanyId) { log.error('failed to create sandbox company', { error: companyError, userId: user.id }) return NextResponse.json( - { error: 'Failed to create sandbox company' }, + { error: 'Failed to create sandbox company', requestId }, { status: 500 } ) } @@ -552,7 +572,7 @@ export async function POST() { } catch (err) { log.error('failed to seed sandbox data', { error: err, userId: user.id, companyId }) return NextResponse.json( - { error: 'Failed to seed sandbox data' }, + { error: 'Failed to seed sandbox data', requestId }, { status: 500 } ) } diff --git a/extensions/general/skatteverket/__tests__/api-client.test.ts b/extensions/general/skatteverket/__tests__/api-client.test.ts index 10ab1820..53a507ec 100644 --- a/extensions/general/skatteverket/__tests__/api-client.test.ts +++ b/extensions/general/skatteverket/__tests__/api-client.test.ts @@ -55,7 +55,7 @@ describe('skvRequest — error mapping', () => { } }) - it('maps 401 with body text → SESSION_EXPIRED and includes body', async () => { + it('maps 401 with body text → SESSION_EXPIRED with a clean Swedish message (no body leak)', async () => { mockFetchStatus(401, 'token expired') try { await skvRequest(fakeSupabase, 'user-1', 'GET', '/x') @@ -63,7 +63,10 @@ describe('skvRequest — error mapping', () => { } catch (e) { expect(e).toBeInstanceOf(SkatteverketAuthError) expect((e as SkatteverketAuthError).code).toBe('SESSION_EXPIRED') - expect((e as SkatteverketAuthError).message).toContain('token expired') + expect((e as SkatteverketAuthError).message).toMatch(/Sessionen har gått ut/) + // Audit V16.1: the raw response body must NOT be concatenated into the + // user-facing message — that information stays in server-side logs. + expect((e as SkatteverketAuthError).message).not.toContain('token expired') } }) diff --git a/extensions/general/skatteverket/lib/api-client.ts b/extensions/general/skatteverket/lib/api-client.ts index 26a12b16..dc41ace6 100644 --- a/extensions/general/skatteverket/lib/api-client.ts +++ b/extensions/general/skatteverket/lib/api-client.ts @@ -1,9 +1,27 @@ import crypto from 'crypto' import type { SupabaseClient } from '@supabase/supabase-js' +import { createLogger } from '@/lib/logger' import { refreshAccessToken } from './oauth' import { getTokens, storeTokens, deleteTokens } from './token-store' import type { SkatteverketTokens } from '../types' +const log = createLogger('skatteverket-api-client') + +// Cap diagnostic-body logging at 200 chars and redact any Bearer token +// patterns. The audit (V16.1 / A.8.15) flagged that raw 401/403 bodies were +// being concatenated into user-facing error messages and written to logs +// without redaction. Diagnostic data still belongs in server-side logs, but +// not in unbounded form and not in anything that reaches the user. +const MAX_LOG_BODY_LEN = 200 +const BEARER_PATTERN = /\bBearer\s+[A-Za-z0-9\-._~+/=]+/gi + +function safeBodyForLog(body: string): string { + const redacted = body.replace(BEARER_PATTERN, 'Bearer [REDACTED]') + return redacted.length > MAX_LOG_BODY_LEN + ? redacted.slice(0, MAX_LOG_BODY_LEN) + '…' + : redacted +} + /** * Skatteverket API client. * @@ -235,15 +253,17 @@ export async function skvRequest( skvHeaders[k] = v } }) - console.error('[skatteverket] 401 from API', { url, body: text, headers: skvHeaders }) + // Diagnostic detail (headers, body) belongs in server-side logs only — + // not in user-facing error messages. The structured logger redacts + // sensitive keys and we further cap body length and strip Bearer tokens + // so the diagnostic is bounded. + log.error('401 from Skatteverket API', { + url, + statusCode: 401, + body: safeBodyForLog(text), + headers: skvHeaders, + }) - // (A) Surface SKV's WWW-Authenticate verbatim — when the body is empty - // this header is usually the only diagnostic SKV gives us. Carry both - // header and body into every thrown message below. - const headerSuffix = Object.keys(skvHeaders).length > 0 - ? ` Headers: ${JSON.stringify(skvHeaders)}` - : '' - const bodySuffix = text ? ` Svar: ${text}` : '' const lower = text.toLowerCase() // OAuth's standard insufficient_scope marker. SKV sometimes emits this @@ -258,8 +278,7 @@ export async function skvRequest( throw new SkatteverketAuthError( 'Anslutningen mot Skatteverket saknar nödvändig behörighet för denna ' + 'tjänst. Koppla bort och anslut igen via Inställningar → Skatteverket ' + - 'för att förnya tokenen med rätt scope.' + - headerSuffix + bodySuffix, + 'för att förnya tokenen med rätt scope.', 'MISSING_SCOPE' ) } @@ -277,13 +296,12 @@ export async function skvRequest( try { await deleteTokens(supabase, userId) } catch (cleanupErr) { - console.error('[skatteverket] failed to clear revoked token row', cleanupErr) + log.error('failed to clear revoked token row', cleanupErr as Error, { userId }) } throw new SkatteverketAuthError( 'Skatteverket har återkallat anslutningen. Detta händer t.ex. om ' + 'BankID-sessionen avslutats eller om en ny anslutning gjorts från ' + - 'en annan enhet. Anslut igen med BankID för att fortsätta.' + - headerSuffix + bodySuffix, + 'en annan enhet. Anslut igen med BankID för att fortsätta.', 'TOKEN_REVOKED' ) } @@ -303,9 +321,7 @@ export async function skvRequest( throw new SkatteverketAuthError( 'Skatteverkets API-gateway nekade anropet. Kontrollera att din ' + 'APIGW-klient (SKATTEVERKET_APIGW_CLIENT_ID) har prenumeration på ' + - 'denna tjänst i Utvecklarportalen.' + - headerSuffix + - ` Svar från Skatteverket: ${text || '(tomt svar)'}`, + 'denna tjänst i Utvecklarportalen.', 'ACCESS_DENIED' ) } @@ -337,19 +353,26 @@ export async function skvRequest( `inte prenumeration på tjänsten "${apiHint}" i Utvecklarportalen, ` + 'eller den lagrade tokenen saknar rätt scope. Kontrollera ' + 'prenumerationen, koppla annars bort och anslut igen via ' + - 'Inställningar → Skatteverket.' + headerSuffix, + 'Inställningar → Skatteverket.', 'ACCESS_DENIED' ) } throw new SkatteverketAuthError( - `Sessionen har gått ut. Logga in med BankID igen.${headerSuffix}${bodySuffix}`, + 'Sessionen har gått ut. Logga in med BankID igen.', 'SESSION_EXPIRED' ) } if (response.status === 403) { - const text = await response.text() + const text = await response.text().catch(() => '') + // Same diagnostic-vs-user-message split as the 401 path: log the body + // server-side, surface only the actionable Swedish guidance. + log.error('403 from Skatteverket API', { + url, + statusCode: 403, + body: safeBodyForLog(text), + }) // Missing scope on the access token — fires when an existing connection // pre-dates an extension that needed a new scope (the AGI/`agd` rollout // is the canonical example). The user has to disconnect + reconnect to @@ -375,7 +398,7 @@ export async function skvRequest( ) } throw new SkatteverketAuthError( - `Åtkomst nekad av Skatteverket (403): ${text}`, + 'Åtkomst nekad av Skatteverket (403). Kontakta support om problemet kvarstår.', 'ACCESS_DENIED' ) } diff --git a/lib/invoices/__tests__/invoice-number-authorization.pg.test.ts b/lib/invoices/__tests__/invoice-number-authorization.pg.test.ts new file mode 100644 index 00000000..4ce6911a --- /dev/null +++ b/lib/invoices/__tests__/invoice-number-authorization.pg.test.ts @@ -0,0 +1,134 @@ +import { randomUUID } from 'node:crypto' +import { describe, expect, it } from 'vitest' +import { getPool, withUserContext } from '@/tests/pg/setup' +import { seedCompany } from '@/tests/pg/fixtures' + +// Authorization tests for the invoice-number RPCs added in +// 20260510140000_harden_invoice_number_rpcs.sql. The functions are +// SECURITY DEFINER, so they bypass caller RLS. The migration added an inline +// auth.uid() membership check as defense-in-depth; these tests prove it. + +async function ensureCompanySettings(params: { + userId: string + companyId: string + invoicePrefix?: string + nextInvoiceNumber?: number +}): Promise { + await getPool().query( + `INSERT INTO public.company_settings + (user_id, company_id, invoice_prefix, next_invoice_number) + VALUES ($1, $2, $3, $4) + ON CONFLICT (company_id) DO UPDATE + SET invoice_prefix = EXCLUDED.invoice_prefix, + next_invoice_number = EXCLUDED.next_invoice_number`, + [params.userId, params.companyId, params.invoicePrefix ?? 'F', params.nextInvoiceNumber ?? 1], + ) +} + +async function insertDraftInvoice(params: { + userId: string + companyId: string +}): Promise { + const customerId = randomUUID() + await getPool().query( + `INSERT INTO public.customers (id, user_id, company_id, name) + VALUES ($1, $2, $3, 'Test Customer')`, + [customerId, params.userId, params.companyId], + ) + + const invoiceId = randomUUID() + await getPool().query( + `INSERT INTO public.invoices + (id, user_id, company_id, customer_id, invoice_number, document_type, + invoice_date, due_date, currency, subtotal, vat_amount, total, + vat_treatment, vat_rate, moms_ruta, status) + VALUES ($1, $2, $3, $4, NULL, 'invoice', + '2026-04-27', '2026-05-27', 'SEK', 1000, 250, 1250, + 'standard_25', 25, '10', 'draft')`, + [invoiceId, params.userId, params.companyId, customerId], + ) + return invoiceId +} + +describe('invoice-number RPCs — authorization (defense in depth)', () => { + it('peek_next_invoice_number raises when caller is not a member of the target company', async () => { + const intruder = await seedCompany() + const target = await seedCompany() + await ensureCompanySettings({ userId: target.userId, companyId: target.companyId }) + + await expect( + withUserContext(intruder.userId, async (client) => { + await client.query('SELECT public.peek_next_invoice_number($1, $2)', [ + target.companyId, + 'invoice', + ]) + }), + ).rejects.toThrow(/unauthorized/) + }) + + it('peek_next_invoice_number succeeds for a member of the target company', async () => { + const { userId, companyId } = await seedCompany() + await ensureCompanySettings({ userId, companyId, invoicePrefix: 'F', nextInvoiceNumber: 7 }) + + const preview = await withUserContext(userId, async (client) => { + const { rows } = await client.query<{ peek_next_invoice_number: string }>( + 'SELECT public.peek_next_invoice_number($1, $2)', + [companyId, 'invoice'], + ) + return rows[0]!.peek_next_invoice_number + }) + + expect(preview).toBe('F007') + }) + + it('generate_invoice_number raises when caller is not a member of the target company', async () => { + const intruder = await seedCompany() + const target = await seedCompany() + await ensureCompanySettings({ userId: target.userId, companyId: target.companyId }) + const invoiceId = await insertDraftInvoice({ + userId: target.userId, + companyId: target.companyId, + }) + + await expect( + withUserContext(intruder.userId, async (client) => { + await client.query('SELECT public.generate_invoice_number($1, $2, $3)', [ + target.companyId, + invoiceId, + 'invoice', + ]) + }), + ).rejects.toThrow(/unauthorized/) + }) + + it('generate_invoice_number succeeds for a member of the target company', async () => { + const { userId, companyId } = await seedCompany() + await ensureCompanySettings({ userId, companyId, invoicePrefix: 'G', nextInvoiceNumber: 42 }) + const invoiceId = await insertDraftInvoice({ userId, companyId }) + + const assigned = await withUserContext(userId, async (client) => { + const { rows } = await client.query<{ generate_invoice_number: string }>( + 'SELECT public.generate_invoice_number($1, $2, $3)', + [companyId, invoiceId, 'invoice'], + ) + return rows[0]!.generate_invoice_number + }) + + expect(assigned).toBe('G042') + }) + + it('superuser (no JWT context) bypasses the membership check', async () => { + // Service role / cron / pg-real seed paths run without a JWT. The + // membership check is intentionally skipped when auth.uid() IS NULL — + // calling the RPC directly on the pool (no withUserContext) must still + // succeed. + const { userId, companyId } = await seedCompany() + await ensureCompanySettings({ userId, companyId, invoicePrefix: 'F', nextInvoiceNumber: 1 }) + + const { rows } = await getPool().query<{ peek_next_invoice_number: string }>( + 'SELECT public.peek_next_invoice_number($1, $2)', + [companyId, 'invoice'], + ) + expect(rows[0]!.peek_next_invoice_number).toBe('F001') + }) +}) diff --git a/supabase/migrations/20260510140000_harden_invoice_number_rpcs.sql b/supabase/migrations/20260510140000_harden_invoice_number_rpcs.sql new file mode 100644 index 00000000..32dc03d5 --- /dev/null +++ b/supabase/migrations/20260510140000_harden_invoice_number_rpcs.sql @@ -0,0 +1,128 @@ +-- Harden invoice-number RPCs with inline membership check + empty search_path. +-- +-- Compliance hardening triggered by external audit (OWASP V8.2.1, SOC 2 CC6.1, +-- ISO 27001 A.8.28). +-- +-- generate_invoice_number and peek_next_invoice_number are SECURITY DEFINER, +-- which means they execute with elevated privileges and bypass the caller's +-- RLS. They are reached from the /api/invoices/next-number route, which +-- already validates company membership at the application layer +-- (lib/company/context.ts → withRouteContext). This migration adds two layers +-- of defense-in-depth at the DB layer: +-- +-- 1. Inline membership check using auth.uid() against public.company_members. +-- Skipped when auth.uid() is NULL (service role / cron / pg-real +-- superuser tests that bypass JWT context). This mirrors how Supabase's +-- service role bypasses RLS by design. +-- +-- 2. SET search_path = '' (was 'public'). All table/function references +-- below are now schema-qualified so they cannot be hijacked by an +-- object created in another schema later on the search path. +-- pg_catalog is searched implicitly so built-in functions (LPAD, +-- GREATEST, COALESCE, length, now) still resolve without qualification. +-- +-- Behaviour for the LPAD / proforma / GREATEST(3, length) logic is preserved +-- verbatim from 20260510130000_invoice_number_no_truncate.sql. + +CREATE OR REPLACE FUNCTION public.generate_invoice_number( + p_company_id uuid, + p_invoice_id uuid, + p_document_type text DEFAULT 'invoice' +) +RETURNS text +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $function$ +DECLARE + v_existing text; + v_prefix text; + v_number integer; + v_final text; +BEGIN + -- Defense-in-depth: refuse to operate on companies the caller is not a + -- member of. NULL auth.uid() (service role / cron / superuser) is allowed + -- through; those code paths are trusted and need to operate across tenants. + IF auth.uid() IS NOT NULL AND NOT EXISTS ( + SELECT 1 FROM public.company_members + WHERE user_id = auth.uid() AND company_id = p_company_id + ) THEN + RAISE EXCEPTION 'unauthorized: caller is not a member of company %', p_company_id + USING ERRCODE = '42501'; + END IF; + + SELECT invoice_number INTO v_existing + FROM public.invoices + WHERE id = p_invoice_id AND company_id = p_company_id + FOR UPDATE; + + IF NOT FOUND THEN + RAISE EXCEPTION 'Invoice % not found in company %', p_invoice_id, p_company_id; + END IF; + + IF v_existing IS NOT NULL THEN + RETURN v_existing; + END IF; + + UPDATE public.company_settings + SET next_invoice_number = next_invoice_number + 1, + updated_at = now() + WHERE company_id = p_company_id + RETURNING invoice_prefix, next_invoice_number - 1 + INTO v_prefix, v_number; + + IF v_number IS NULL THEN + RAISE EXCEPTION 'Company settings not found for company %', p_company_id; + END IF; + + v_final := CASE + WHEN p_document_type = 'proforma' THEN 'PF-' + ELSE COALESCE(v_prefix, '') + END || LPAD(v_number::text, GREATEST(3, length(v_number::text)), '0'); + + UPDATE public.invoices + SET invoice_number = v_final + WHERE id = p_invoice_id AND company_id = p_company_id; + + RETURN v_final; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.peek_next_invoice_number( + p_company_id uuid, + p_document_type text DEFAULT 'invoice' +) +RETURNS text +LANGUAGE plpgsql +STABLE +SECURITY DEFINER +SET search_path = '' +AS $function$ +DECLARE + v_prefix text; + v_number integer; +BEGIN + IF auth.uid() IS NOT NULL AND NOT EXISTS ( + SELECT 1 FROM public.company_members + WHERE user_id = auth.uid() AND company_id = p_company_id + ) THEN + RAISE EXCEPTION 'unauthorized: caller is not a member of company %', p_company_id + USING ERRCODE = '42501'; + END IF; + + SELECT invoice_prefix, next_invoice_number INTO v_prefix, v_number + FROM public.company_settings + WHERE company_id = p_company_id; + + IF v_number IS NULL THEN + RETURN NULL; + END IF; + + RETURN CASE + WHEN p_document_type = 'proforma' THEN 'PF-' + ELSE COALESCE(v_prefix, '') + END || LPAD(v_number::text, GREATEST(3, length(v_number::text)), '0'); +END; +$function$; + +NOTIFY pgrst, 'reload schema';