diff --git a/DECISIONS.md b/DECISIONS.md index dc250855..55b9cb00 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1056,3 +1056,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-18] Defect record (compliance-swarm PI1.3, no risk_register.csv exists in repo so this log is the equivalent): Swish QR on invoice PDFs encoded the pre-deduction total on ROT/RUT invoices with the amount locked (editmask 0), risking customer overpayment by the full skattereduktion; impact window = since Swish QR + ROT/RUT coexisted; remediation PR #1685 = QR now encodes getAmountToPay().toPay on every send/download surface (dashboard send/preview-of-send/pdf, v1 send/pdf, MCP pending-operations commit, recurring, issue-and-book; the editor preview is tracked separately in #1686), v1 pdf/send projections gained the amount-path columns (deduction_total, ore_rounding), pinned by lib/api/v1/__tests__/invoice-columns.test.ts. [2026-08-18] Generic CSV mapping (#1671): description guess now excludes clock-time columns (Time/Tid/Klockslag by label, HH:MM by values) and knows Lunar's Title/Titel label; Lunar detect() sniffs comma/semicolon/tab and matches header CELLS exactly (date, title|text, amount, balance) instead of substrings, aligned with what parse() resolves on. NOT changed: the 2026-08-13 generic_csv exemption from the parsed-0-rows auto-detect fallback stays; lifting it would route an explicit "Annan CSV" pick into a dedicated parser and remove the manual escape hatch. Not verified against the customer's actual file (Gmail thread not readable in-session): the semicolon/tab widening is the plausible detection miss, a Swedish-localized Lunar header is not confirmed to exist and was not special-cased in the Lunar detector (the generic path now maps it correctly anyway). [2026-08-18] Editor PDF preview (#1686) recomputes ROT/RUT server-side from the posted lines with the same helpers as build-invoice-write.ts (computeDeduction / computeInvoiceDeductionTotal, base inkl. moms at the rendered rate, invoice-doc only) and resolves the masked personnummer the same way (typed value, else an individual customer's kundkort personnummer): the client is not trusted with the deduction math, and the preview must state the same avdrag row, info box and "Att betala" as the invoice that gets created. The editor now also posts deduction_personnummer / deduction_housing_designation to the preview route, only when a line claims a deduction (same privacy rule as buildInvoiceWritePayload). Swish QR amount in the preview follows buildSwishQrDataUrl, fixed separately in #1685. +[2026-08-18] Skatteverket read data is visible to every company member, no new role gate (#1673): token rows are per (user, company) but the fetched skattekonto/declaration data belongs to the company, and viewers already read `skattekonto_transactions` and the local snapshot with no role check; membership (dispatcher-resolved ctx.companyId + company-scoped SELECT policy on `skatteverket_tokens`) is the gate. Reads resolve the caller's own token first, then the most recently issued active token of any member (all rows ordered, never `.maybeSingle()`, which errored once two members had connected). Writes (moms utkast/las/submit, AGI submit/spara/las, connect/disconnect, /status) stay on the caller's own token: BankID signing is personal. diff --git a/app/api/extensions/skatteverket/agi/kvittenser/cron/__tests__/route.test.ts b/app/api/extensions/skatteverket/agi/kvittenser/cron/__tests__/route.test.ts index bd892fb3..d92f0cd7 100644 --- a/app/api/extensions/skatteverket/agi/kvittenser/cron/__tests__/route.test.ts +++ b/app/api/extensions/skatteverket/agi/kvittenser/cron/__tests__/route.test.ts @@ -147,7 +147,7 @@ function makeSupabaseStub(tables: Record { { ...PENDING_DECLARATION, id: 'decl-3', company_id: 'comp-3' }, ], }, - skatteverket_tokens: { data: { user_id: 'user-1', status: 'active' } }, + skatteverket_tokens: { data: [{ user_id: 'user-1', status: 'active' }] }, company_settings: { data: { org_number: '556123-4567', entity_type: 'aktiebolag' } }, }), ) diff --git a/app/api/extensions/skatteverket/agi/kvittenser/cron/route.ts b/app/api/extensions/skatteverket/agi/kvittenser/cron/route.ts index ce903ce6..08837123 100644 --- a/app/api/extensions/skatteverket/agi/kvittenser/cron/route.ts +++ b/app/api/extensions/skatteverket/agi/kvittenser/cron/route.ts @@ -5,7 +5,7 @@ import { createLogger } from '@/lib/logger' import { verifyCronSecret } from '@/lib/auth/cron' import { SkatteverketAuthError } from '@/extensions/general/skatteverket/lib/api-client' import { markNeedsReconsent, RECONSENT_ERROR_CODES } from '@/extensions/general/skatteverket/lib/token-store' -import { currentSkvEnvironment } from '@/extensions/general/skatteverket/lib/resolve-auth' +import { currentSkvEnvironment, findCompanyTokenUser } from '@/extensions/general/skatteverket/lib/resolve-auth' import { markGrantRevoked } from '@/extensions/general/skatteverket/lib/connection-store' import { reconcileAgiDeclaration } from '@/extensions/general/skatteverket/lib/agi-kvittens-reconcile' import { formatRedovisningsperiod } from '@/lib/skatteverket/format' @@ -166,13 +166,12 @@ export async function GET(request: Request) { ) { // Persist the health flag so both crons stop retrying this // connection and the UI can prompt for re-consent proactively. - const { data: tokenRow } = await supabase - .from('skatteverket_tokens') - .select('user_id') - .eq('company_id', companyId) - .maybeSingle() - if (tokenRow?.user_id) { - await markNeedsReconsent(supabase, tokenRow.user_id as string, companyId, err.code) + // Same pick as the reconciler's resolveReadAuth (a company can hold + // one row per connected member), so the flag lands on the row that + // just failed. + const tokenRow = await findCompanyTokenUser(supabase, companyId) + if (tokenRow) { + await markNeedsReconsent(supabase, tokenRow.userId, companyId, err.code) } results.push({ declarationId, period, status: 'expired_token', error: err.code }) continue diff --git a/app/api/extensions/skatteverket/vat/kvittenser/cron/__tests__/route.test.ts b/app/api/extensions/skatteverket/vat/kvittenser/cron/__tests__/route.test.ts index 267988c9..66dff671 100644 --- a/app/api/extensions/skatteverket/vat/kvittenser/cron/__tests__/route.test.ts +++ b/app/api/extensions/skatteverket/vat/kvittenser/cron/__tests__/route.test.ts @@ -121,7 +121,7 @@ function stubHappyTables(state: Record = LOCKED_STATE) { extension_data: { data: [{ company_id: 'comp-1', key: 'submission_202606', value: JSON.stringify(state) }], }, - skatteverket_tokens: { data: { user_id: 'user-1', status: 'active' } }, + skatteverket_tokens: { data: [{ user_id: 'user-1', status: 'active' }] }, }) } @@ -312,7 +312,7 @@ describe('VAT kvittenser cron', () => { data: [{ company_id: 'comp-1', key: 'submission_202606', value: JSON.stringify(LOCKED_STATE) }], updateError: { message: 'connection reset', code: '08006' }, }, - skatteverket_tokens: { data: { user_id: 'user-1', status: 'active' } }, + skatteverket_tokens: { data: [{ user_id: 'user-1', status: 'active' }] }, }), ) mockSkvRequest.mockResolvedValueOnce({ @@ -345,7 +345,7 @@ describe('VAT kvittenser cron', () => { { company_id: 'comp-2', key: 'submission_202606', value: JSON.stringify(LOCKED_STATE) }, ], }, - skatteverket_tokens: { data: { user_id: 'user-1', status: 'active' } }, + skatteverket_tokens: { data: [{ user_id: 'user-1', status: 'active' }] }, }) } diff --git a/app/api/extensions/skatteverket/vat/kvittenser/cron/route.ts b/app/api/extensions/skatteverket/vat/kvittenser/cron/route.ts index db8eea15..ce01302e 100644 --- a/app/api/extensions/skatteverket/vat/kvittenser/cron/route.ts +++ b/app/api/extensions/skatteverket/vat/kvittenser/cron/route.ts @@ -5,7 +5,7 @@ import { verifyCronSecret } from '@/lib/auth/cron' import { skvRequestWithAuth, SkatteverketAuthError } from '@/extensions/general/skatteverket/lib/api-client' import { markNeedsReconsent, RECONSENT_ERROR_CODES } from '@/extensions/general/skatteverket/lib/token-store' import { sendKvittensNotification } from '@/extensions/general/skatteverket/lib/kvittens-notification' -import { resolveReadAuth, currentSkvEnvironment } from '@/extensions/general/skatteverket/lib/resolve-auth' +import { resolveReadAuth, currentSkvEnvironment, findCompanyTokenUser } from '@/extensions/general/skatteverket/lib/resolve-auth' import { markGrantRevoked } from '@/extensions/general/skatteverket/lib/connection-store' import { completeTaxDeadline } from '@/lib/deadlines/complete-tax-deadline' import { hasCapability } from '@/lib/entitlements/has-capability' @@ -260,13 +260,11 @@ export async function GET(request: Request) { // connection. Best-effort: a failure here must not abort the // remaining companies' rows. try { - const { data: tokenRow } = await supabase - .from('skatteverket_tokens') - .select('user_id') - .eq('company_id', companyId) - .maybeSingle() - if (tokenRow?.user_id) { - await markNeedsReconsent(supabase, tokenRow.user_id as string, companyId, err.code) + // Same pick as resolveReadAuth above (a company can hold one row + // per connected member), so the flag lands on the row that failed. + const tokenRow = await findCompanyTokenUser(supabase, companyId) + if (tokenRow) { + await markNeedsReconsent(supabase, tokenRow.userId, companyId, err.code) } } catch (reconsentErr) { console.warn('[vat-kvittenser-cron] Failed to persist reconsent flag', { diff --git a/extensions/general/skatteverket/__tests__/connection-expired-notification.test.ts b/extensions/general/skatteverket/__tests__/connection-expired-notification.test.ts index 6648de3d..bb21cee4 100644 --- a/extensions/general/skatteverket/__tests__/connection-expired-notification.test.ts +++ b/extensions/general/skatteverket/__tests__/connection-expired-notification.test.ts @@ -181,6 +181,18 @@ describe('sendConnectionExpiredNotification', () => { expect(mockSendEmail).not.toHaveBeenCalled() }) + it('keys the episode on the (user, company) token row, not the user alone', async () => { + const { supabase, ops } = makeSupabase() + + await sendConnectionExpiredNotification(supabase, baseInput) + + // Rows are per (user, company): a multi-company operator holds several, + // and a user-only maybeSingle() errored out and silently skipped the + // email (#1673 follow-through). + const tokenRead = ops.find((o) => o.table === 'skatteverket_tokens') + expect(tokenRead?.filters).toMatchObject({ user_id: 'user-1', company_id: 'company-1' }) + }) + it('skips when no token row exists (already disconnected)', async () => { const { supabase } = makeSupabase({ token: null }) diff --git a/extensions/general/skatteverket/__tests__/resolve-auth.test.ts b/extensions/general/skatteverket/__tests__/resolve-auth.test.ts index fd3d00b5..1a234b6d 100644 --- a/extensions/general/skatteverket/__tests__/resolve-auth.test.ts +++ b/extensions/general/skatteverket/__tests__/resolve-auth.test.ts @@ -2,6 +2,10 @@ * resolveReadAuth preference matrix: system credentials when the flag is on * and the grant is verified; the company's user token otherwise; explicit * no_token / needs_reconsent outcomes for the crons' quiet buckets. + * + * Token rows are per (user, company): the caller's own row wins when it + * exists, any other member's active row serves otherwise (#1673), and two + * connected members must never degrade to "nobody connected". */ import { describe, it, expect, vi, beforeEach } from 'vitest' @@ -22,16 +26,37 @@ vi.mock('../lib/system-auth/config', async (importOriginal) => { } }) -import { resolveReadAuth, hasVerifiedGrant } from '../lib/resolve-auth' +import { resolveReadAuth, hasVerifiedGrant, findCompanyTokenUser } from '../lib/resolve-auth' import type { SupabaseClient } from '@supabase/supabase-js' -function makeSupabase(tokenRow: { user_id: string; status: string } | null) { +type TokenRow = { user_id: string; status: string; created_at?: string } + +/** + * Chain stub for the company token lookup: `.select().eq().order()` resolves + * to the given rows, already in created_at DESC order (the DB does the + * ordering; the resolver only picks). Pass a single row for the one-member + * case, an array for several members, null for no rows. + */ +function makeSupabase( + tokenRows: TokenRow | TokenRow[] | null, + opts: { error?: { message: string } } = {}, +) { + const rows = tokenRows === null ? [] : Array.isArray(tokenRows) ? tokenRows : [tokenRows] const chain: Record = {} for (const m of ['select', 'eq']) { chain[m] = vi.fn(() => chain) } - chain.maybeSingle = vi.fn().mockResolvedValue({ data: tokenRow, error: null }) - return { from: vi.fn(() => chain) } as unknown as SupabaseClient + chain.order = vi.fn().mockResolvedValue( + opts.error ? { data: null, error: opts.error } : { data: rows, error: null }, + ) + // Regression guard for #1673: the company lookup must never go through + // maybeSingle(), which errors as soon as two members have connected. + chain.maybeSingle = vi.fn(() => { + throw new Error('maybeSingle() must not be used for the company token lookup') + }) + return { from: vi.fn(() => chain), _chain: chain } as unknown as SupabaseClient & { + _chain: Record> + } } const GRANTED_CONNECTION = { @@ -100,13 +125,26 @@ describe('resolveReadAuth', () => { expect(result).toMatchObject({ ok: true, source: 'user' }) }) - it('explicit userId short-circuits the company lookup', async () => { + it('explicit userId with no company row at all -> no_token (no longer short-circuits to the caller)', async () => { const supabase = makeSupabase(null) const result = await resolveReadAuth(supabase, 'company-1', { requires: 'moms_ombud', userId: 'user-9', }) - expect(result).toMatchObject({ ok: true, source: 'user', tokenUserId: 'user-9' }) + expect(result).toEqual({ ok: false, reason: 'no_token' }) + }) + + it('orders the company lookup by created_at desc and filters by company', async () => { + const supabase = makeSupabase({ user_id: 'user-1', status: 'active' }) + await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud' }) + expect(supabase._chain.eq).toHaveBeenCalledWith('company_id', 'company-1') + expect(supabase._chain.order).toHaveBeenCalledWith('created_at', { ascending: false }) + }) + + it('lookup error -> no_token (never throws into the caller)', async () => { + const supabase = makeSupabase(null, { error: { message: 'boom' } }) + const result = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud' }) + expect(result).toEqual({ ok: false, reason: 'no_token' }) }) it('no token row -> no_token', async () => { @@ -122,6 +160,115 @@ describe('resolveReadAuth', () => { }) }) +describe('resolveReadAuth: two members of the same company (#1673)', () => { + const A = 'user-a' + const B = 'user-b' + const NEWER = '2026-08-18T10:00:00Z' + const OLDER = '2026-08-18T09:00:00Z' + + it('one connects, both read: the member who never connected resolves the connected token', async () => { + const supabase = makeSupabase({ user_id: A, status: 'active', created_at: NEWER }) + + const asA = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: A }) + expect(asA).toMatchObject({ ok: true, source: 'user', tokenUserId: A }) + + const asB = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: B }) + expect(asB).toMatchObject({ ok: true, source: 'user', tokenUserId: A }) + // The auth handed to skvRequest carries the token OWNER: the refresh + // writes back to A's row, never to B's (missing) one. + if (asB.ok) expect(asB.auth).toEqual({ mode: 'user', supabase, userId: A, companyId: 'company-1' }) + }) + + it('both connect, both read: each member gets their own token, nobody degrades to no_token', async () => { + const supabase = makeSupabase([ + { user_id: B, status: 'active', created_at: NEWER }, + { user_id: A, status: 'active', created_at: OLDER }, + ]) + + const asA = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: A }) + expect(asA).toMatchObject({ ok: true, tokenUserId: A }) + + const asB = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: B }) + expect(asB).toMatchObject({ ok: true, tokenUserId: B }) + }) + + it('both connect, background read (no caller): the most recently issued row wins', async () => { + const supabase = makeSupabase([ + { user_id: B, status: 'active', created_at: NEWER }, + { user_id: A, status: 'active', created_at: OLDER }, + ]) + const result = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud' }) + expect(result).toMatchObject({ ok: true, tokenUserId: B }) + }) + + it('a dead own row does not shadow another member\'s live token', async () => { + const supabase = makeSupabase([ + { user_id: B, status: 'needs_reconsent', created_at: NEWER }, + { user_id: A, status: 'active', created_at: OLDER }, + ]) + const asB = await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: B }) + expect(asB).toMatchObject({ ok: true, tokenUserId: A }) + }) + + it('every row flagged needs_reconsent -> needs_reconsent (own row reported when present)', async () => { + const supabase = makeSupabase([ + { user_id: A, status: 'needs_reconsent', created_at: NEWER }, + { user_id: B, status: 'needs_reconsent', created_at: OLDER }, + ]) + expect(await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: B })).toEqual({ + ok: false, + reason: 'needs_reconsent', + }) + expect(await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud' })).toEqual({ + ok: false, + reason: 'needs_reconsent', + }) + }) + + it('system mode keeps the caller as notification recipient, falls back to the token owner', async () => { + mockMode.mockReturnValue('on') + mockConfigured.mockReturnValue(true) + mockGetConnection.mockResolvedValue(GRANTED_CONNECTION) + const supabase = makeSupabase({ user_id: A, status: 'active', created_at: NEWER }) + + expect( + await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud', userId: B }), + ).toMatchObject({ ok: true, source: 'system', tokenUserId: B }) + expect(await resolveReadAuth(supabase, 'company-1', { requires: 'lasombud' })).toMatchObject({ + ok: true, + source: 'system', + tokenUserId: A, + }) + }) +}) + +describe('findCompanyTokenUser', () => { + it('returns null for a company with no rows', async () => { + expect(await findCompanyTokenUser(makeSupabase(null), 'company-1')).toBeNull() + }) + + it('prefers the given user, then active status, then recency', async () => { + const supabase = makeSupabase([ + { user_id: 'user-c', status: 'needs_reconsent', created_at: '3' }, + { user_id: 'user-b', status: 'active', created_at: '2' }, + { user_id: 'user-a', status: 'active', created_at: '1' }, + ]) + expect(await findCompanyTokenUser(supabase, 'company-1')).toEqual({ + userId: 'user-b', + needsReconsent: false, + }) + expect(await findCompanyTokenUser(supabase, 'company-1', { preferUserId: 'user-a' })).toEqual({ + userId: 'user-a', + needsReconsent: false, + }) + // Preferring a user whose row is dead still yields the live row. + expect(await findCompanyTokenUser(supabase, 'company-1', { preferUserId: 'user-c' })).toEqual({ + userId: 'user-b', + needsReconsent: false, + }) + }) +}) + describe('hasVerifiedGrant', () => { it('requires both the grant and a verified/partial aggregate status', async () => { mockGetConnection.mockResolvedValue({ diff --git a/extensions/general/skatteverket/__tests__/skattekonto-company-read.test.ts b/extensions/general/skatteverket/__tests__/skattekonto-company-read.test.ts new file mode 100644 index 00000000..9eebdb94 --- /dev/null +++ b/extensions/general/skatteverket/__tests__/skattekonto-company-read.test.ts @@ -0,0 +1,163 @@ +/** + * Skattekonto read routes resolve the COMPANY's connection, not the caller's + * (#1673): a member who never pressed "Anslut" sees the saldo snapshot and can + * trigger a sync on the token another member connected. The auth handed to + * the sync carries the token owner's userId, so the refresh writes back to + * the owner's row. Two connected members both keep working, and a company + * with no row at all still answers 401 NOT_CONNECTED (the page's empty state). + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +vi.mock('@/lib/entitlements/has-capability', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, requireCapability: vi.fn().mockResolvedValue(null) } +}) + +const mockSyncSkattekonto = vi.fn() +vi.mock('../lib/skattekonto-sync', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, syncSkattekonto: (...args: unknown[]) => mockSyncSkattekonto(...args) } +}) + +import { skatteverketExtension } from '../index' +import type { ExtensionContext } from '@/lib/extensions/types' + +const A = 'user-a' +const B = 'user-b' +const COMPANY = 'company-1' +type TokenRow = { user_id: string; status: string; created_at: string } + +function makeContext(userId: string, tokenRows: TokenRow[]): ExtensionContext { + const chain: Record = {} + for (const m of ['select', 'eq']) chain[m] = vi.fn(() => chain) + chain.order = vi.fn().mockResolvedValue({ data: tokenRows, error: null }) + chain.maybeSingle = vi.fn(() => { + throw new Error('maybeSingle() must not be used for the company token lookup') + }) + const supabase = { from: vi.fn(() => chain) } + return { + userId, + companyId: COMPANY, + extensionId: 'skatteverket', + requestId: 'req_test', + supabase, + emit: vi.fn().mockResolvedValue(undefined), + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), child: vi.fn() }, + settings: { + get: vi.fn(async (key: string) => { + if (key === 'skattekonto_balance_snapshot') { + return { saldo: { saldoSkatteverket: 1234.5, saldoKronofogden: 0 }, fetchedAt: 1_700_000_000_000 } + } + if (key === 'skattekonto_last_synced_at') return '2026-08-18T06:00:00.000Z' + return null + }), + set: vi.fn().mockResolvedValue(undefined), + clear: vi.fn().mockResolvedValue(undefined), + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any +} + +function route(method: string, path: string) { + const found = skatteverketExtension.apiRoutes?.find(r => r.method === method && r.path === path) + if (!found) throw new Error(`${method} ${path} must be registered`) + return found +} + +function req(method: string, path: string) { + return new Request(`https://test.local/api/extensions/ext/skatteverket${path}`, { method }) +} + +const A_ROW: TokenRow = { user_id: A, status: 'active', created_at: '2026-08-18T09:00:00Z' } +const B_ROW: TokenRow = { user_id: B, status: 'active', created_at: '2026-08-18T10:00:00Z' } + +beforeEach(() => { + vi.clearAllMocks() + mockSyncSkattekonto.mockResolvedValue({ + booked: 1, + upcoming: 0, + saldoSkatteverket: 1234.5, + saldoKronofogden: 0, + syncedAt: '2026-08-18T12:00:00.000Z', + }) +}) + +describe('GET /skattekonto/saldo: company-scoped connected check', () => { + it('one member connects, both members read the snapshot', async () => { + const handler = route('GET', '/skattekonto/saldo').handler + + const asA = await handler(req('GET', '/skattekonto/saldo'), makeContext(A, [A_ROW])) + expect(asA.status).toBe(200) + + const asB = await handler(req('GET', '/skattekonto/saldo'), makeContext(B, [A_ROW])) + expect(asB.status).toBe(200) + const body = (await asB.json()) as { data: { saldoSkatteverket: number } | null } + expect(body.data?.saldoSkatteverket).toBe(1234.5) + }) + + it('both members connect, both read (no maybeSingle failure on two rows)', async () => { + const handler = route('GET', '/skattekonto/saldo').handler + const asA = await handler(req('GET', '/skattekonto/saldo'), makeContext(A, [B_ROW, A_ROW])) + const asB = await handler(req('GET', '/skattekonto/saldo'), makeContext(B, [B_ROW, A_ROW])) + expect(asA.status).toBe(200) + expect(asB.status).toBe(200) + }) + + it('no member has connected -> 401 NOT_CONNECTED (page empty state)', async () => { + const handler = route('GET', '/skattekonto/saldo').handler + const res = await handler(req('GET', '/skattekonto/saldo'), makeContext(B, [])) + expect(res.status).toBe(401) + expect(await res.json()).toMatchObject({ code: 'NOT_CONNECTED' }) + }) + + it('a row flagged needs_reconsent still counts as connected (stale snapshot stays visible)', async () => { + const handler = route('GET', '/skattekonto/saldo').handler + const res = await handler( + req('GET', '/skattekonto/saldo'), + makeContext(B, [{ ...A_ROW, status: 'needs_reconsent' }]), + ) + expect(res.status).toBe(200) + }) +}) + +describe('POST /skattekonto/sync: syncs on the company token, refresh goes to the owner', () => { + it('member B triggers a sync on member A\'s token: auth carries A as the token owner', async () => { + const ctx = makeContext(B, [A_ROW]) + const res = await route('POST', '/skattekonto/sync').handler(req('POST', '/skattekonto/sync'), ctx) + expect(res.status).toBe(200) + expect(mockSyncSkattekonto).toHaveBeenCalledTimes(1) + const [passedCtx, auth] = mockSyncSkattekonto.mock.calls[0] + expect(passedCtx).toBe(ctx) + expect(auth).toEqual({ mode: 'user', supabase: ctx.supabase, userId: A, companyId: COMPANY }) + }) + + it('both connected: each member syncs on their own token', async () => { + const asA = makeContext(A, [B_ROW, A_ROW]) + await route('POST', '/skattekonto/sync').handler(req('POST', '/skattekonto/sync'), asA) + expect(mockSyncSkattekonto.mock.calls[0][1]).toMatchObject({ mode: 'user', userId: A }) + + const asB = makeContext(B, [B_ROW, A_ROW]) + await route('POST', '/skattekonto/sync').handler(req('POST', '/skattekonto/sync'), asB) + expect(mockSyncSkattekonto.mock.calls[1][1]).toMatchObject({ mode: 'user', userId: B }) + }) + + it('nobody connected -> 401 NOT_CONNECTED without touching SKV', async () => { + const res = await route('POST', '/skattekonto/sync').handler( + req('POST', '/skattekonto/sync'), + makeContext(B, []), + ) + expect(res.status).toBe(401) + expect(await res.json()).toMatchObject({ code: 'NOT_CONNECTED' }) + expect(mockSyncSkattekonto).not.toHaveBeenCalled() + }) + + it('only dead rows -> 401 SESSION_EXPIRED reconnect prompt, no refresh attempt', async () => { + const res = await route('POST', '/skattekonto/sync').handler( + req('POST', '/skattekonto/sync'), + makeContext(B, [{ ...A_ROW, status: 'needs_reconsent' }]), + ) + expect(res.status).toBe(401) + expect(await res.json()).toMatchObject({ code: 'SESSION_EXPIRED' }) + expect(mockSyncSkattekonto).not.toHaveBeenCalled() + }) +}) diff --git a/extensions/general/skatteverket/__tests__/skattekonto-sync-expired-owner.test.ts b/extensions/general/skatteverket/__tests__/skattekonto-sync-expired-owner.test.ts new file mode 100644 index 00000000..fff3e3b6 --- /dev/null +++ b/extensions/general/skatteverket/__tests__/skattekonto-sync-expired-owner.test.ts @@ -0,0 +1,87 @@ +/** + * When a sync run on another member's token dies with a terminal auth error, + * the connection-expired event must name the token OWNER, not whoever + * pressed "Synkronisera nu": only the owner can redo the BankID consent, and + * the notification handler keys its dedup on the owner's token row (#1673). + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { createQueuedMockSupabase } from '@/tests/helpers' +import { eventBus } from '@/lib/events/bus' + +const { supabase, enqueue, reset } = createQueuedMockSupabase() + +const getSaldoMock = vi.fn() +const getTransaktionerMock = vi.fn() +vi.mock('../lib/skattekonto-client', () => ({ + getSaldo: (...args: unknown[]) => getSaldoMock(...args), + getTransaktioner: (...args: unknown[]) => getTransaktionerMock(...args), +})) + +vi.mock('../lib/agi-tax-settlement', () => ({ + settleAgiTaxPayments: vi.fn().mockResolvedValue(undefined), +})) + +import { syncSkattekonto } from '../lib/skattekonto-sync' +import { SkatteverketAuthError } from '../lib/api-client' +import type { ExtensionContext } from '@/lib/extensions/types' +import type { SupabaseClient } from '@supabase/supabase-js' + +function makeCtx(userId: string): ExtensionContext { + return { + supabase, + companyId: 'company-1', + userId, + settings: { + get: vi.fn().mockResolvedValue(null), + set: vi.fn().mockResolvedValue(undefined), + }, + emit: vi.fn().mockResolvedValue(undefined), + } as unknown as ExtensionContext +} + +describe('syncSkattekonto: connection.expired names the token owner', () => { + beforeEach(() => { + vi.clearAllMocks() + reset() + eventBus.clear() + enqueue({ data: { org_number: '556677-8899', entity_type: 'aktiebolag' } }) // company_settings + const dead = new SkatteverketAuthError('Sessionen har gått ut. Logga in med BankID igen.', 'SESSION_EXPIRED') + getSaldoMock.mockRejectedValue(dead) + getTransaktionerMock.mockRejectedValue(dead) + }) + + it('member B syncs on member A\'s token: the event carries A', async () => { + const emitted: Array<{ type: string; payload: Record }> = [] + const emitSpy = vi.spyOn(eventBus, 'emit').mockImplementation(async (event) => { + emitted.push(event as { type: string; payload: Record }) + }) + + await expect( + syncSkattekonto(makeCtx('user-b'), { + mode: 'user', + supabase: supabase as unknown as SupabaseClient, + userId: 'user-a', + companyId: 'company-1', + }), + ).rejects.toBeInstanceOf(SkatteverketAuthError) + + expect(emitted).toHaveLength(1) + expect(emitted[0]).toMatchObject({ + type: 'skattekonto.connection.expired', + payload: { reason: 'SESSION_EXPIRED', userId: 'user-a', companyId: 'company-1' }, + }) + emitSpy.mockRestore() + }) + + it('default (own token) keeps the ctx user as recipient', async () => { + const emitted: Array<{ type: string; payload: Record }> = [] + const emitSpy = vi.spyOn(eventBus, 'emit').mockImplementation(async (event) => { + emitted.push(event as { type: string; payload: Record }) + }) + + await expect(syncSkattekonto(makeCtx('user-a'))).rejects.toBeInstanceOf(SkatteverketAuthError) + + expect(emitted[0]?.payload).toMatchObject({ userId: 'user-a' }) + emitSpy.mockRestore() + }) +}) diff --git a/extensions/general/skatteverket/index.ts b/extensions/general/skatteverket/index.ts index 833ae931..224b5abc 100644 --- a/extensions/general/skatteverket/index.ts +++ b/extensions/general/skatteverket/index.ts @@ -171,6 +171,41 @@ async function requireSkvCapability(ctx: ExtensionContext): Promise(SKATTEKONTO_BALANCE_SNAPSHOT_KEY) const lastSyncedAt = await ctx.settings.get(SKATTEKONTO_LAST_SYNCED_AT_KEY) @@ -2165,7 +2194,14 @@ export const skatteverketExtension: Extension = { const blocked = await requireSkvCapability(ctx) if (blocked) return blocked try { - const result = await syncSkattekonto(ctx) + // Any member may trigger a sync on the company's connection: the + // resolved auth carries the token OWNER's userId, so the refresh + // writes back to their row, never to the caller's (#1673). A row + // flagged needs_reconsent cannot heal on its own, so answer the + // reconnect prompt directly instead of burning a refresh call. + const resolved = await resolveCompanyReadAuth(ctx) + if (!resolved.ok) return readAuthFailureResponse(resolved.reason) + const result = await syncSkattekonto(ctx, resolved.auth) return NextResponse.json({ data: result }) } catch (err) { return handleSkvError(err) diff --git a/extensions/general/skatteverket/lib/connection-expired-notification.ts b/extensions/general/skatteverket/lib/connection-expired-notification.ts index ebdd9cd7..c4f49191 100644 --- a/extensions/general/skatteverket/lib/connection-expired-notification.ts +++ b/extensions/general/skatteverket/lib/connection-expired-notification.ts @@ -66,11 +66,14 @@ export async function sendConnectionExpiredNotification( if (!email.isConfigured()) return { sent: false, reason: 'email_not_configured' } // Episode key: the token row's created_at. No token row means nothing to - // reconnect (already disconnected): skip. + // reconnect (already disconnected): skip. Rows are per (user, company): + // without the company filter a multi-company operator's second row made + // maybeSingle() error out and the email was never sent. const { data: token } = await supabase .from('skatteverket_tokens') .select('created_at') .eq('user_id', input.userId) + .eq('company_id', input.companyId) .maybeSingle() const tokenCreatedAt = (token as { created_at?: string | null } | null)?.created_at if (!tokenCreatedAt) return { sent: false, reason: 'no_token' } diff --git a/extensions/general/skatteverket/lib/resolve-auth.ts b/extensions/general/skatteverket/lib/resolve-auth.ts index 818f50b3..7c6ef7ff 100644 --- a/extensions/general/skatteverket/lib/resolve-auth.ts +++ b/extensions/general/skatteverket/lib/resolve-auth.ts @@ -1,4 +1,5 @@ import type { SupabaseClient } from '@supabase/supabase-js' +import { createLogger } from '@/lib/logger' import { getSkatteverketEnvironment, type SkvAuth } from './api-client' import { getSystemAuthMode, isSystemAuthConfigured } from './system-auth/config' import { @@ -7,20 +8,35 @@ import { type SkvEnvironment, } from './connection-store' +const log = createLogger('skatteverket-resolve-auth') + /** - * Auth resolution for background READ paths (skattekonto sync, kvittens - * polling, moms inlamnat/beslutat checks). + * Auth resolution for READ paths (skattekonto sync, kvittens polling, moms + * inlamnat/beslutat checks, the skattekonto page's connected check). * * Preference order: * 1. System credentials, when SKATTEVERKET_SYSTEM_AUTH_MODE=on, the system * flow is configured, and the company's connection row shows the * required behorighet as granted for the current environment. - * 2. The company's user token (the pre-hybrid behavior), looked up by - * company_id like the kvittens cron always did. + * 2. A user token connected to the company. Token rows are per + * (user_id, company_id), so a company can hold one row per member who + * ran the BankID consent. The caller's own row is preferred when it + * exists; otherwise the most recently issued active row for the company + * is used. That is what lets member B read data member A connected + * (issue #1673): the fetched skattekonto/declaration data belongs to + * the company, not to the person who happened to press "Anslut". + * + * Who may read: any member of the company. Membership is enforced upstream + * (the extension dispatcher resolves ctx.companyId from the caller's own + * memberships, and the token table's SELECT policy is company-scoped), so no + * extra role check is added here. Reads never move the row: refresh writes + * go back to the token owner's row (auth.userId is the owner, not the + * caller). * * Write paths (moms utkast/las, AGI submit/spara/granskningsunderlag) stay - * hard-wired to user mode: the personal flow needs no ombud grant and the - * BankID signing step is personal by nature. + * hard-wired to user mode with the CALLER's own token: the personal flow + * needs no ombud grant and the BankID signing step is personal by nature. + * Connect/disconnect likewise only touch the caller's own row. * * Retiring user-token reads later (the full ombud switch) is a policy change * inside this function only. @@ -63,21 +79,18 @@ export async function resolveReadAuth( ok: true, auth: { mode: 'system' }, source: 'system', - tokenUserId: opts.userId ?? (await lookupTokenUser(supabase, companyId))?.userId ?? null, + tokenUserId: + opts.userId ?? (await findCompanyTokenUser(supabase, companyId))?.userId ?? null, } } } - if (opts.userId) { - return { - ok: true, - auth: { mode: 'user', supabase, userId: opts.userId, companyId }, - source: 'user', - tokenUserId: opts.userId, - } - } - - const token = await lookupTokenUser(supabase, companyId) + // The caller's own token wins when it exists; any other member's active + // token serves otherwise. Passing userId no longer short-circuits to "the + // caller's row or nothing": a member who never connected used to resolve + // to their own missing row and see NOT_CONNECTED for a company that is + // connected (#1673). + const token = await findCompanyTokenUser(supabase, companyId, { preferUserId: opts.userId }) if (!token) return { ok: false, reason: 'no_token' } if (token.needsReconsent) return { ok: false, reason: 'needs_reconsent' } @@ -89,21 +102,57 @@ export async function resolveReadAuth( } } -async function lookupTokenUser( +export interface CompanyTokenUser { + userId: string + needsReconsent: boolean +} + +/** + * Pick the token row that should serve a read for this company. + * + * Deterministic order: + * 1. active rows before needs_reconsent rows (a dead row must not shadow a + * live one connected by another member) + * 2. within the same status, the preferred user's own row first + * 3. then the most recently issued row (storeTokens is DELETE + INSERT, so + * created_at is the last consent or refresh) + * + * Reads all of the company's rows (one per connected member; a handful at + * most) instead of `.maybeSingle()`, which errors on the second row and used + * to turn "two members connected" into "nobody connected" for everyone. + * + * Returns null when the company has no token row at all. The token table's + * SELECT policy is company-scoped, so a user-session client only ever sees + * rows for companies the caller belongs to. + */ +export async function findCompanyTokenUser( supabase: SupabaseClient, - companyId: string -): Promise<{ userId: string; needsReconsent: boolean } | null> { - // The token table is user-scoped (one BankID identity per user) but - // carries company_id: match on company so a multi-company operator's - // token is only used for the company that owns the work. - const { data } = await supabase + companyId: string, + opts: { preferUserId?: string } = {} +): Promise { + const { data, error } = await supabase .from('skatteverket_tokens') - .select('user_id, status') + .select('user_id, status, created_at') .eq('company_id', companyId) - .maybeSingle() - if (!data?.user_id) return null + .order('created_at', { ascending: false }) + + if (error) { + log.warn('failed to look up company token rows', { companyId, error: error.message }) + return null + } + + const rows = ((data ?? []) as Array<{ user_id: string | null; status: string | null }>).filter( + (row): row is { user_id: string; status: string | null } => typeof row.user_id === 'string' + ) + if (rows.length === 0) return null + + const active = rows.filter(row => row.status !== 'needs_reconsent') + const pool = active.length > 0 ? active : rows + const own = opts.preferUserId ? pool.find(row => row.user_id === opts.preferUserId) : undefined + const pick = own ?? pool[0] + return { - userId: data.user_id as string, - needsReconsent: data.status === 'needs_reconsent', + userId: pick.user_id, + needsReconsent: pick.status === 'needs_reconsent', } } diff --git a/extensions/general/skatteverket/lib/skattekonto-sync.ts b/extensions/general/skatteverket/lib/skattekonto-sync.ts index c08ee302..f290980a 100644 --- a/extensions/general/skatteverket/lib/skattekonto-sync.ts +++ b/extensions/general/skatteverket/lib/skattekonto-sync.ts @@ -120,9 +120,10 @@ function upcomingToRow(companyId: string, tx: SkatteverketUpcomingTransaction): */ export async function syncSkattekonto( ctx: ExtensionContext, - // Defaults to the ctx user's personal token: the interactive manual-sync - // route keeps its exact pre-hybrid behavior. The cron passes system auth - // for companies with a verified lasombud grant. + // Defaults to the ctx user's personal token (the post-connect refresh, where + // the ctx user just consented). The manual-sync route and the cron pass + // resolved auth: system credentials for companies with a verified lasombud + // grant, otherwise the token of whichever member connected the company. auth: SkvAuth = { mode: 'user', supabase: ctx.supabase, userId: ctx.userId, companyId: ctx.companyId }, ): Promise { const omfragad = await resolveOmfragad(ctx.supabase, ctx.companyId) @@ -141,11 +142,14 @@ export async function syncSkattekonto( err.code === 'SESSION_EXPIRED' || err.code === 'TOKEN_CORRUPTED' ) { + // Notify the token OWNER, not whoever triggered the sync: only the + // owner can redo the BankID consent, and the notification handler + // keys its dedup on the owner's token row. await eventBus.emit({ type: 'skattekonto.connection.expired', payload: { reason: err.code, - userId: ctx.userId, + userId: auth.mode === 'user' ? auth.userId : ctx.userId, companyId: ctx.companyId, }, })