From 43925bc2d3188a498091bce1ea6ca3e75cec1d31 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Sun, 14 Jun 2026 23:40:26 +0200 Subject: [PATCH] =?UTF-8?q?fix(import):=20SIE=20bulk-delete=20on=20service?= =?UTF-8?q?=20client=20+=20provider/reporting/b=E2=80=A6=20(#724)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(import): SIE bulk-delete on service client + provider/reporting/banking fixes Rebuilt branch onto main as a single commit. - import: run SIE bulk-delete RPCs on the service client to escape the 8s statement_timeout; undo_sie_import now takes an explicit actor (p_user_id) so its owner/admin gate works when auth.uid() is NULL on the service client (migration 20260624120000) + pg-real regression test - providers: distinguish missing Fortnox license from expired connection; provider_consent_tokens PK regression test - reports: include unmapped BAS expense groups in the income statement - enable-banking: reconnect closed/expired bank sessions in place - bookkeeping: surface linked invoices as underlag on the verifikat view - scripts: track BL cleanup/diagnostic tooling; data files (*.csv) are git-ignored and consentId is now a required arg with no silent default Co-Authored-By: Claude Opus 4.8 (1M context) * fix(import): add Cache-Control header to journal entry references response --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .gitignore | 7 +- app/(dashboard)/bookkeeping/[id]/page.tsx | 82 ++- .../journal-entries/[id]/references/route.ts | 30 + .../callback/__tests__/route.test.ts | 2 +- .../enable-banking/callback/route.ts | 25 +- .../enable-banking/sync/cron/route.ts | 17 +- app/api/import/sie/[id]/undo/route.ts | 4 +- .../general/ArcimMigrationWorkspace.tsx | 118 +++- components/transactions/BankSyncNowButton.tsx | 116 +++- extensions/general/arcim-migration/index.ts | 109 +++- .../__tests__/session-expired.test.ts | 262 ++++++++ .../components/BankConnectionStatus.tsx | 11 +- .../components/BankingSettingsPanel.tsx | 51 +- extensions/general/enable-banking/index.ts | 239 ++++++-- .../general/enable-banking/lib/api-client.ts | 58 ++ .../journal-entry-references.test.ts | 94 +++ .../bookkeeping/journal-entry-references.ts | 143 +++++ lib/errors/structured-errors.ts | 7 + .../undo-sie-import-actor.pg.test.ts | 156 +++++ lib/import/sie-import.ts | 44 +- lib/pending-operations/commit.ts | 5 +- .../__tests__/resolve-consent.pg.test.ts | 63 ++ .../__tests__/resolve-consent.test.ts | 67 +- lib/providers/resolve-consent.ts | 59 +- lib/providers/with-provider-call.ts | 26 + .../__tests__/income-statement.test.ts | 68 +++ lib/reports/income-statement.ts | 67 +- messages/en.json | 8 + messages/sv.json | 8 + scripts/bl-balance-broken.ts | 252 ++++++++ scripts/bl-delete-broken.ts | 145 +++++ scripts/debug-bl-list-consents.ts | 30 + scripts/debug-bl-unbalanced.ts | 102 ++++ scripts/remap-krister-bas96-to-bas2025.sql | 289 --------- scripts/remap-krister-bas96-to-bas2025.ts | 575 ------------------ scripts/verify-krister-remap.sql | 103 ---- ...4120000_undo_sie_import_explicit_actor.sql | 166 +++++ 37 files changed, 2456 insertions(+), 1152 deletions(-) create mode 100644 app/api/bookkeeping/journal-entries/[id]/references/route.ts create mode 100644 extensions/general/enable-banking/__tests__/session-expired.test.ts create mode 100644 lib/core/bookkeeping/__tests__/journal-entry-references.test.ts create mode 100644 lib/core/bookkeeping/journal-entry-references.ts create mode 100644 lib/import/__tests__/undo-sie-import-actor.pg.test.ts create mode 100644 lib/providers/__tests__/resolve-consent.pg.test.ts create mode 100644 scripts/bl-balance-broken.ts create mode 100644 scripts/bl-delete-broken.ts create mode 100644 scripts/debug-bl-list-consents.ts create mode 100644 scripts/debug-bl-unbalanced.ts delete mode 100644 scripts/remap-krister-bas96-to-bas2025.sql delete mode 100644 scripts/remap-krister-bas96-to-bas2025.ts delete mode 100644 scripts/verify-krister-remap.sql create mode 100644 supabase/migrations/20260624120000_undo_sie_import_explicit_actor.sql diff --git a/.gitignore b/.gitignore index cf2cf427..b6e739ad 100644 --- a/.gitignore +++ b/.gitignore @@ -87,7 +87,10 @@ supabase/.temp/ # out of the box without running the generator. supabase/.branches/ -/scripts - # Local-only SIE test fixtures — may contain real/scrubbed company data, never commit tests/fixtures/sie/ + +# Diagnostic/cleanup tooling under /scripts is tracked, but the DATA those +# scripts read or emit (ledger dumps, reconciliation exports) is real customer +# räkenskapsinformation — never commit it. Keep the .ts/.sql tooling, ignore the data. +scripts/*.csv diff --git a/app/(dashboard)/bookkeeping/[id]/page.tsx b/app/(dashboard)/bookkeeping/[id]/page.tsx index d83a9c43..c973d018 100644 --- a/app/(dashboard)/bookkeeping/[id]/page.tsx +++ b/app/(dashboard)/bookkeeping/[id]/page.tsx @@ -8,7 +8,7 @@ import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card' import { Button } from '@/components/ui/button' import { AccountNumber } from '@/components/ui/account-number' import { Textarea } from '@/components/ui/textarea' -import { Loader2, ArrowLeft, Paperclip, AlertTriangle, Lock, MessageSquare, Pencil, Check, X, Copy, ChevronDown, CalendarClock } from 'lucide-react' +import { Loader2, ArrowLeft, Paperclip, AlertTriangle, Lock, MessageSquare, Pencil, Check, X, Copy, ChevronDown, CalendarClock, FileText, Link2 } from 'lucide-react' import { DropdownMenu, DropdownMenuTrigger, @@ -27,6 +27,7 @@ import { ConfirmationDialog } from '@/components/ui/confirmation-dialog' import { useToast } from '@/components/ui/use-toast' import { getErrorMessage } from '@/lib/errors/get-error-message' import type { JournalEntry, JournalEntryLine } from '@/types' +import type { UnderlagReference } from '@/lib/core/bookkeeping/journal-entry-references' export default function JournalEntryDetailPage({ params }: { params: Promise<{ id: string }> }) { const { id } = use(params) @@ -46,6 +47,7 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i const [isCommitting, setIsCommitting] = useState(false) const [isLastInSeries, setIsLastInSeries] = useState(false) const [attachmentCount, setAttachmentCount] = useState(0) + const [references, setReferences] = useState([]) const [editingNotes, setEditingNotes] = useState(false) const [notesValue, setNotesValue] = useState('') const [savingNotes, setSavingNotes] = useState(false) @@ -54,16 +56,27 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i setIsLoading(true) setError(null) try { - const res = await fetch(`/api/bookkeeping/journal-entries/${id}/chain`) - if (!res.ok) { - const { error: msg } = await res.json() + const [chainRes, refsRes] = await Promise.all([ + fetch(`/api/bookkeeping/journal-entries/${id}/chain`), + fetch(`/api/bookkeeping/journal-entries/${id}/references`), + ]) + if (!chainRes.ok) { + const { error: msg } = await chainRes.json() setError(msg || t('error_load_failed')) return } - const { data } = await res.json() + const { data } = await chainRes.json() setEntry(data.entry) setChain(data.chain) setIsLastInSeries(data.is_last_in_series ?? false) + // Underlag references (linked invoices) — best-effort; the verifikat still + // renders if this fails, it just falls back to documents-only. + if (refsRes.ok) { + const { data: refData } = await refsRes.json() + setReferences(refData?.references ?? []) + } else { + setReferences([]) + } } catch { setError(t('error_load_failed')) } finally { @@ -405,19 +418,27 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i {t('attachments_title')} -
- {attachmentCount > 0 ? ( - <> - - {t('attachments_count', { count: attachmentCount })} - - ) : ( - <> - - {t('no_attachments')} - - )} -
+ {attachmentCount === 0 && references.length === 0 ? ( +
+ + {t('no_attachments')} +
+ ) : ( +
+ {attachmentCount > 0 && ( +
+ + {t('attachments_count', { count: attachmentCount })} +
+ )} + {references.length > 0 && ( +
+ + {t('references_count', { count: references.length })} +
+ )} +
+ )}
@@ -569,6 +590,31 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i {t('attachments_title')} + {references.length > 0 && ( +
+
+

{t('references_title')}

+

{t('references_subtitle')}

+
+
    + {references.map((ref) => ( +
  • + + + + {ref.type === 'invoice' + ? t('reference_invoice', { number: ref.number }) + : t('reference_supplier_invoice', { number: ref.number })} + + +
  • + ))} +
+
+ )} }>( + 'journal_entry.references', + async (_request, { supabase, companyId }, { params }) => { + const { id } = await params + const references = await getJournalEntryUnderlagReferences(supabase, companyId, id) + return NextResponse.json( + { data: { references } }, + { headers: { 'Cache-Control': 'private, no-store' } }, + ) + }, +) diff --git a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts index ae48e6db..503b7728 100644 --- a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts +++ b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts @@ -34,7 +34,7 @@ function makeRequest(params: Record) { function mockChain(result: { data?: unknown; error?: unknown }) { const chain: Record = {} - for (const m of ['select', 'eq', 'single', 'update', 'order', 'limit']) { + for (const m of ['select', 'eq', 'in', 'single', 'update', 'order', 'limit']) { chain[m] = vi.fn().mockReturnValue(chain) } chain.single = vi.fn().mockResolvedValue({ data: result.data ?? null, error: result.error ?? null }) diff --git a/app/api/extensions/enable-banking/callback/route.ts b/app/api/extensions/enable-banking/callback/route.ts index 795f253b..7bf30439 100644 --- a/app/api/extensions/enable-banking/callback/route.ts +++ b/app/api/extensions/enable-banking/callback/route.ts @@ -49,12 +49,14 @@ export async function GET(request: Request) { try { const supabase = await createServiceClient() - // Fetch connection details for logging before updating + // Fetch connection details for logging before updating. Match by + // oauth_state across pending/expired/error so an in-place reconnect + // (which stays 'expired' during the round-trip) is also handled. const { data: pendingConn } = await supabase .from('bank_connections') .select('id, user_id, bank_name') .eq('oauth_state', state) - .eq('status', 'pending') + .in('status', ['pending', 'expired', 'error']) .single() if (pendingConn) { @@ -66,9 +68,16 @@ export async function GET(request: Request) { error_description: errorDescription, }) + // If the bank reports a session-expiry during authorization itself, + // mark the row 'expired' (not generic 'error') so the settings panel + // surfaces the reconnect button rather than a dead-end error state. + const isSessionExpiry = /session.?expired|expired.?session|closed.?session|session.?closed|invalid.?session|session.?not.?found/i.test( + `${error} ${errorDescription ?? ''}` + ) + await supabase .from('bank_connections') - .update({ status: 'error', error_message: errorMessage, oauth_state: null }) + .update({ status: isSessionExpiry ? 'expired' : 'error', error_message: errorMessage, oauth_state: null }) .eq('id', pendingConn.id) // Include bank name and error code in redirect so the UI can offer PSU type retry @@ -102,12 +111,16 @@ export async function GET(request: Request) { const supabase = await createServiceClient() try { - // Look up pending connection by oauth_state (CSRF-safe) + // Look up the connection awaiting this callback by oauth_state (CSRF-safe). + // oauth_state is a single-use random token cleared after use, so it uniquely + // identifies the row regardless of status. Accept 'expired'/'error' too: an + // in-place reconnect keeps the row in 'expired' during the round-trip (so + // the nightly stale-'pending' cleanup can't delete an established row). const { data: pendingConnection, error: findError } = await supabase .from('bank_connections') .select('id, user_id, company_id') .eq('oauth_state', state) - .eq('status', 'pending') + .in('status', ['pending', 'expired', 'error']) .single() if (findError || !pendingConnection) { @@ -278,7 +291,7 @@ export async function GET(request: Request) { .from('bank_connections') .update({ status: 'error', error_message: error instanceof Error ? error.message : 'Connection failed', oauth_state: null }) .eq('oauth_state', state) - .eq('status', 'pending') + .in('status', ['pending', 'expired', 'error']) } catch (cleanupError) { console.error('[enable-banking] Callback cleanup failed', { cleanupError: cleanupError instanceof Error ? cleanupError.message : String(cleanupError), diff --git a/app/api/extensions/enable-banking/sync/cron/route.ts b/app/api/extensions/enable-banking/sync/cron/route.ts index 5b6d188f..3fb72e10 100644 --- a/app/api/extensions/enable-banking/sync/cron/route.ts +++ b/app/api/extensions/enable-banking/sync/cron/route.ts @@ -2,7 +2,7 @@ import { createClient, type SupabaseClient } from '@supabase/supabase-js' import { NextResponse } from 'next/server' import { syncAccountTransactions } from '@/extensions/general/enable-banking/lib/sync' import { runReconciliation } from '@/lib/reconciliation/bank-reconciliation' -import { isConsentExpiringSoon, getDaysUntilExpiry } from '@/extensions/general/enable-banking/lib/api-client' +import { isConsentExpiringSoon, getDaysUntilExpiry, SessionExpiredError } from '@/extensions/general/enable-banking/lib/api-client' import { getEmailService } from '@/lib/email/service' import { generateConsentExpiryEmailHtml, @@ -266,10 +266,19 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { lastSyncedAt: connection.last_synced_at, }) - // Persist error status on sync failure + // A dead PSD2 session (closed/expired/invalid consent) is a re-auth + // condition, not a transient failure — flip it to 'expired' (same state + // the consent-elapsed branch uses) so the UI offers a reconnect instead + // of a retry. Other errors stay 'error'. + const isSessionDead = error instanceof SessionExpiredError + const failureStatus = isSessionDead ? 'expired' : 'error' + const failureMessage = isSessionDead + ? 'Bankanslutningen har löpt ut. Förnya anslutningen för att fortsätta synka.' + : message + await supabase .from('bank_connections') - .update({ status: 'error', error_message: message }) + .update({ status: failureStatus, error_message: failureMessage }) .eq('id', connection.id) results.push({ @@ -279,7 +288,7 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { imported: 0, duplicates: 0, errors: 1, - status: 'error', + status: failureStatus, }) } } diff --git a/app/api/import/sie/[id]/undo/route.ts b/app/api/import/sie/[id]/undo/route.ts index 8e1b868d..5b82dc11 100644 --- a/app/api/import/sie/[id]/undo/route.ts +++ b/app/api/import/sie/[id]/undo/route.ts @@ -16,10 +16,10 @@ export const DELETE = withRouteContext( 'sie_import.undo', async (_request, ctx, { params }: { params: Promise<{ id: string }> }) => { const { id } = await params - const { supabase, companyId, log, requestId } = ctx + const { supabase, companyId, user, log, requestId } = ctx const opLog = log.child({ sieImportId: id }) - const result = await undoSIEImport(supabase, companyId!, id) + const result = await undoSIEImport(supabase, companyId!, id, user.id) if (!result.success) { return errorResponseFromCode('SIE_UNDO_FAILED', opLog, { diff --git a/components/extensions/general/ArcimMigrationWorkspace.tsx b/components/extensions/general/ArcimMigrationWorkspace.tsx index 15dfa9ca..b234c9e6 100644 --- a/components/extensions/general/ArcimMigrationWorkspace.tsx +++ b/components/extensions/general/ArcimMigrationWorkspace.tsx @@ -68,6 +68,16 @@ function apiErrorMessage(data: unknown, fallback: string): string { return fallback } +/** Pull the structured error `code` from an envelope, if present. */ +function apiErrorCode(data: unknown): string | null { + const err = (data as { error?: unknown } | null)?.error + if (err && typeof err === 'object') { + const code = (err as { code?: unknown }).code + if (typeof code === 'string' && code) return code + } + return null +} + interface SkipReasons { duplicate?: number inactive?: number @@ -632,12 +642,18 @@ function PreviewStep({ preview, isLoading, error, + authExpired, + licenseMissing, + onReconnect, onContinue, onBack, }: { preview: PreviewData | null isLoading: boolean error: string | null + authExpired: boolean + licenseMissing: boolean + onReconnect: () => void onContinue: () => void onBack: () => void }) { @@ -663,13 +679,26 @@ function PreviewStep({ <>
-

{error}

+
+

{error}

+ {authExpired && ( + + )} +
- + {/* License-missing keeps the SIE fallback visible: re-auth loops + until the customer re-orders the Fortnox Integration license, + so a manual SIE import is the reliable escape hatch. */} + {(!authExpired || licenseMissing) && ( + + )} )} @@ -1654,6 +1683,14 @@ export default function ArcimMigrationWorkspace(_props: WorkspaceComponentProps) // Preview state const [preview, setPreview] = useState(null) + // Set when a preview/sync fails because the provider connection expired + // (dead refresh token → PROVIDER_AUTH_EXPIRED). Drives the "Återanslut" + // affordance so the user can re-authorize in place instead of disconnecting. + const [authExpired, setAuthExpired] = useState(false) + // Set when the failure is specifically a missing/inactive Fortnox integration + // license (PROVIDER_LICENSE_MISSING). Re-auth alone can't fix it, so the SIE + // fallback stays available alongside the "Återanslut" CTA. + const [licenseMissing, setLicenseMissing] = useState(false) // SIE data state (held between mapping and execution steps) const [sieData, setSieData] = useState(null) @@ -1707,17 +1744,30 @@ export default function ArcimMigrationWorkspace(_props: WorkspaceComponentProps) setStep('preview') setIsLoading(true) setError(null) + setAuthExpired(false) + setLicenseMissing(false) + setConsentId(cId) try { const res = await fetch(`/api/extensions/ext/arcim-migration/preview?consentId=${cId}`) if (!res.ok) { const data = await res.json().catch(() => ({})) + // A dead connection (expired/revoked refresh token) is recoverable in + // place — flag it so the UI offers "Återanslut" instead of a dead end. + // A missing Fortnox integration license shows the same CTA but keeps the + // SIE fallback, because re-auth loops until the license is re-ordered. + const code = apiErrorCode(data) + if (code === 'PROVIDER_AUTH_EXPIRED' || code === 'PROVIDER_LICENSE_MISSING') { + setAuthExpired(true) + } + if (code === 'PROVIDER_LICENSE_MISSING') { + setLicenseMissing(true) + } throw new Error(apiErrorMessage(data, `HTTP ${res.status}`)) } const data = await res.json() setPreview(data) - setConsentId(cId) // If SIE is not available, disable SIE import by default if (!data.sieAvailable) { @@ -1780,6 +1830,55 @@ export default function ArcimMigrationWorkspace(_props: WorkspaceComponentProps) await loadPreview(existingConsentId) }, [loadPreview]) + // Re-authorize a dead connection in place. Re-runs provider auth against the + // SAME consent so fresh tokens overwrite the expired pair — no disconnect. + // OAuth providers open the login popup (the existing postMessage listener + // reloads the preview on success); token providers drop to the credential + // form. Triggered from the "Återanslut" CTA after a sync hits + // PROVIDER_AUTH_EXPIRED. + const handleReconnect = useCallback(async (provider: ArcimProvider, existingConsentId: string) => { + setError(null) + setAuthExpired(false) + setLicenseMissing(false) + setIsLoading(true) + setSelectedProvider(provider) + + try { + const res = await fetch('/api/extensions/ext/arcim-migration/connect', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ provider, reconnect: true }), + }) + if (!res.ok) { + const data = await res.json().catch(() => ({})) + throw new Error(apiErrorMessage(data, `HTTP ${res.status}`)) + } + + const data = await res.json() + setConsentId(data.consentId ?? existingConsentId) + setAuthType(data.authType) + + if (data.authType === 'oauth' && data.authUrl) { + // Open immediately — this runs inside the button's click handler, so + // the popup is a trusted user gesture and won't be blocked. + const w = 600 + const h = 700 + const left = window.screenX + (window.outerWidth - w) / 2 + const top = window.screenY + (window.outerHeight - h) / 2 + window.open(data.authUrl, 'arcim-oauth', `width=${w},height=${h},left=${left},top=${top}`) + setAuthUrl(data.authUrl) + } else if (data.authType === 'token') { + // Re-enter credentials for token-based providers + setStep('connect') + } + } catch (err) { + setError(err instanceof Error ? err.message : 'Kunde inte återansluta') + setAuthExpired(true) + } finally { + setIsLoading(false) + } + }, []) + // Disconnect an existing consent const handleDisconnect = useCallback(async (consentIdToDelete: string) => { try { @@ -2163,6 +2262,11 @@ export default function ArcimMigrationWorkspace(_props: WorkspaceComponentProps) preview={preview} isLoading={isLoading} error={error} + authExpired={authExpired} + licenseMissing={licenseMissing} + onReconnect={() => { + if (selectedProvider && consentId) handleReconnect(selectedProvider, consentId) + }} onContinue={handlePreviewContinue} onBack={() => setStep('provider')} /> diff --git a/components/transactions/BankSyncNowButton.tsx b/components/transactions/BankSyncNowButton.tsx index 242494b6..412a5c37 100644 --- a/components/transactions/BankSyncNowButton.tsx +++ b/components/transactions/BankSyncNowButton.tsx @@ -6,6 +6,7 @@ import { useTranslations } from 'next-intl' import { Loader2, RefreshCw } from 'lucide-react' import { Button } from '@/components/ui/button' import { useToast } from '@/components/ui/use-toast' +import { ToastAction } from '@/components/ui/toast' import { DropdownMenu, DropdownMenuContent, @@ -15,24 +16,30 @@ import { import { createClient } from '@/lib/supabase/client' import { useCompany } from '@/contexts/CompanyContext' -interface ActiveConnection { +interface BankConn { id: string bank_name: string + status: string + provider: string } /** * On-demand "Sync now" button beside BankSyncStatusChip. Reuses the - * per-connection sync endpoint that BankingSettingsPanel already calls; - * if the user has multiple active connections, a dropdown lets them - * pick which one to sync. + * per-connection sync endpoint that BankingSettingsPanel already calls. + * + * Also handles dead PSD2 sessions: a connection whose consent has closed/expired + * shows a "Förnya anslutning" action that re-authorizes in place (no disconnect + * needed), and a sync that fails with a session-expiry surfaces the same + * reconnect action right in the error toast. If the user has multiple + * connections, a dropdown lets them pick which one to sync/reconnect. */ export default function BankSyncNowButton() { const t = useTranslations('transactions') const { toast } = useToast() const router = useRouter() const { company } = useCompany() - const [connections, setConnections] = useState(null) - const [syncingId, setSyncingId] = useState(null) + const [connections, setConnections] = useState(null) + const [busyId, setBusyId] = useState(null) useEffect(() => { if (!company?.id) return @@ -40,11 +47,13 @@ export default function BankSyncNowButton() { const supabase = createClient() supabase .from('bank_connections') - .select('id, bank_name') + .select('id, bank_name, status, provider') + // Include expired/error so the reconnect entry point survives a reload — + // not just active connections that can sync. + .in('status', ['active', 'expired', 'error']) .eq('company_id', company.id) - .eq('status', 'active') .then(({ data }) => { - if (!cancelled) setConnections(data ?? []) + if (!cancelled) setConnections((data as BankConn[]) ?? []) }) return () => { cancelled = true @@ -53,16 +62,63 @@ export default function BankSyncNowButton() { if (!connections || connections.length === 0) return null - async function syncConnection(connectionId: string) { - setSyncingId(connectionId) + // Re-authorize an existing connection in place — posts the connection_id so + // the server reuses the same row, then hands off to the bank's consent screen. + async function reconnect(conn: BankConn) { + setBusyId(conn.id) + try { + const country = conn.provider?.split('-').pop()?.toUpperCase() || 'SE' + const res = await fetch('/api/extensions/ext/enable-banking/connect', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + connection_id: conn.id, + aspsp_name: conn.bank_name, + aspsp_country: country, + }), + }) + const data = await res.json() + if (!res.ok) throw new Error(data.error || 'Reconnect failed') + window.location.href = data.authorization_url + } catch (error) { + toast({ + title: t('bank_reconnect'), + description: error instanceof Error ? error.message : 'Reconnect failed', + variant: 'destructive', + }) + setBusyId(null) + } + } + + async function syncConnection(conn: BankConn) { + setBusyId(conn.id) try { const res = await fetch('/api/extensions/ext/enable-banking/sync', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ connection_id: connectionId }), + body: JSON.stringify({ connection_id: conn.id }), }) const data = await res.json() if (!res.ok) { + // A dead PSD2 session can't be fixed by retrying — surface a one-click + // reconnect in the toast instead of a dead-end error. + if (data?.reauth_required) { + toast({ + title: t('bank_sync_session_expired'), + description: t('bank_sync_session_expired_desc'), + variant: 'destructive', + action: ( + reconnect(conn)}> + {t('bank_reconnect')} + + ), + }) + // Reflect the now-expired status so the button flips to reconnect. + setConnections((prev) => + (prev ?? []).map((c) => (c.id === conn.id ? { ...c, status: 'expired' } : c)) + ) + return + } throw new Error(data.error || 'Sync failed') } toast({ @@ -79,28 +135,36 @@ export default function BankSyncNowButton() { variant: 'destructive', }) } finally { - setSyncingId(null) + setBusyId((prev) => (prev === conn.id ? null : prev)) } } - const isSyncing = syncingId !== null - const label = isSyncing ? t('bank_sync_button_syncing') : t('bank_sync_button_now') + // Active connections sync; expired/error connections reconnect. + function runFor(conn: BankConn) { + if (conn.status === 'active') return syncConnection(conn) + return reconnect(conn) + } + + const isBusy = busyId !== null + const syncLabel = isBusy ? t('bank_sync_button_syncing') : t('bank_sync_button_now') if (connections.length === 1) { + const conn = connections[0] + const needsReconnect = conn.status !== 'active' return ( ) } @@ -112,24 +176,26 @@ export default function BankSyncNowButton() { variant="outline" size="sm" className="h-7 gap-1.5 px-2.5 text-xs" - disabled={isSyncing} + disabled={isBusy} > - {isSyncing ? ( + {isBusy ? ( ) : ( )} - {label} + {syncLabel} {connections.map((conn) => ( syncConnection(conn.id)} + disabled={isBusy} + onSelect={() => runFor(conn)} > - {conn.bank_name} + {conn.status === 'active' + ? conn.bank_name + : `${conn.bank_name} · ${t('bank_reconnect')}`} ))} diff --git a/extensions/general/arcim-migration/index.ts b/extensions/general/arcim-migration/index.ts index 3da180fa..4071cbda 100644 --- a/extensions/general/arcim-migration/index.ts +++ b/extensions/general/arcim-migration/index.ts @@ -55,6 +55,40 @@ function translateOAuthError(error: string, description: string | null): string return description ? `${error}: ${description}` : error } +/** + * Build a provider OAuth authorization URL bound to an EXISTING consent id. + * Used by both first-time connect and reconnect (token revival): the callback + * runs exchangeAuthToken(consentId, …) which upserts the fresh tokens keyed by + * consent_id, so re-running OAuth against the same consent overwrites a dead + * refresh-token pair in place — no disconnect/recreate needed. + */ +async function buildArcimOAuthUrl(consentId: string, provider: ArcimProvider): Promise { + const otc = await generateOtc(consentId) + + // Prefer a provider-specific redirect override (e.g. VISMA_REDIRECT_URI) when + // set — lets dev environments route through a single registered URI rather + // than registering every ngrok URL on the OAuth client. Falls back to + // NEXT_PUBLIC_APP_URL + the canonical callback path. + const appUrl = process.env.NEXT_PUBLIC_APP_URL || '' + const providerRedirectEnv = + provider === 'visma' + ? process.env.VISMA_REDIRECT_URI + : provider === 'fortnox' + ? process.env.FORTNOX_REDIRECT_URI + : undefined + const callbackUrl = + providerRedirectEnv && providerRedirectEnv.trim().length > 0 + ? providerRedirectEnv + : `${appUrl}/api/extensions/ext/arcim-migration/callback` + + // Encode consentId + provider in state so the callback rebinds to this consent + const statePayload = JSON.stringify({ otc: otc.code, consentId, provider }) + const stateEncoded = Buffer.from(statePayload).toString('base64url') + + const { url } = await getAuthUrl(provider, stateEncoded, callbackUrl) + return url +} + /** * Provider Migration extension * @@ -158,10 +192,11 @@ export const arcimMigrationExtension: Extension = { const companyId = ctx?.companyId ?? user.id - const { provider, companyName, orgNumber } = await request.json() as { + const { provider, companyName, orgNumber, reconnect } = await request.json() as { provider: ArcimProvider companyName?: string orgNumber?: string + reconnect?: boolean } if (!provider) { @@ -180,8 +215,43 @@ export const arcimMigrationExtension: Extension = { try { const { createServiceClient: createSvc } = await import('@/lib/supabase/server') - // Reuse existing accepted consent if one exists for this provider const existingConsents = await listConsents(companyId) + + // Reconnect: an existing connection's stored tokens are dead (refresh + // failed → PROVIDER_AUTH_EXPIRED). Re-run auth against the SAME consent + // so fresh tokens overwrite the dead pair in place — no disconnect, no + // duplicate consent, import history preserved. Bypasses the + // alreadyConnected short-circuit below (which would otherwise skip the + // auth that's the whole point here). + if (reconnect) { + const stale = existingConsents.find( + c => c.provider === provider && (c.status === 0 || c.status === 1), + ) + if (stale) { + if (ctx?.settings) { + await ctx.settings.set('consent_id', stale.id) + await ctx.settings.set('provider', provider) + } + if (providerInfo.authType === 'oauth') { + const authUrl = await buildArcimOAuthUrl(stale.id, provider) + return NextResponse.json({ + consentId: stale.id, + authType: 'oauth', + authUrl, + reconnect: true, + }) + } + // Token-based providers re-authorize by re-entering credentials + return NextResponse.json({ + consentId: stale.id, + authType: 'token', + reconnect: true, + }) + } + // No existing consent to revive — fall through to a normal connect. + } + + // Reuse existing accepted consent if one exists for this provider const accepted = existingConsents.find(c => c.provider === provider && c.status === 1) if (accepted) { @@ -205,7 +275,11 @@ export const arcimMigrationExtension: Extension = { for (const p of pending) { const { data: tokens } = await svc .from('provider_consent_tokens') - .select('id') + // consent_id is the PK — there is no `id` column. Selecting `id` + // errors silently (only `data` is read), so `tokens` was always + // null and the reuse branch below never fired, deleting valid + // status-0 consents as "abandoned". + .select('consent_id') .eq('consent_id', p.id) .limit(1) if (tokens && tokens.length > 0) { @@ -242,37 +316,12 @@ export const arcimMigrationExtension: Extension = { } if (providerInfo.authType === 'oauth') { - // Generate OTC for OAuth flow - const otc = await generateOtc(consent.id) - - // Build the OAuth callback URL. Prefer a provider-specific override - // (e.g. VISMA_REDIRECT_URI) when set — this lets dev environments - // route through a single registered URI (production) rather than - // requiring every ngrok URL to be registered on the OAuth client. - // Falls back to NEXT_PUBLIC_APP_URL + the canonical callback path. - const appUrl = process.env.NEXT_PUBLIC_APP_URL || '' - const providerRedirectEnv = - provider === 'visma' - ? process.env.VISMA_REDIRECT_URI - : provider === 'fortnox' - ? process.env.FORTNOX_REDIRECT_URI - : undefined - const callbackUrl = - providerRedirectEnv && providerRedirectEnv.trim().length > 0 - ? providerRedirectEnv - : `${appUrl}/api/extensions/ext/arcim-migration/callback` - - // Encode consentId + provider in state - const statePayload = JSON.stringify({ otc: otc.code, consentId: consent.id, provider }) - const stateEncoded = Buffer.from(statePayload).toString('base64url') - - const { url } = await getAuthUrl(provider, stateEncoded, callbackUrl) + const authUrl = await buildArcimOAuthUrl(consent.id, provider) return NextResponse.json({ consentId: consent.id, authType: 'oauth', - authUrl: url, - otcCode: otc.code, + authUrl, }) } else { // Token-based providers: consent is ready for direct use diff --git a/extensions/general/enable-banking/__tests__/session-expired.test.ts b/extensions/general/enable-banking/__tests__/session-expired.test.ts new file mode 100644 index 00000000..0ad1430b --- /dev/null +++ b/extensions/general/enable-banking/__tests__/session-expired.test.ts @@ -0,0 +1,262 @@ +import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest' +import type { ExtensionContext } from '@/lib/extensions/types' +import type { StoredAccount } from '../types' + +// Mock the JWT signer so api-client can build a request header without real +// ENABLE_BANKING credentials (part 2 stubs fetch directly). +vi.mock('../lib/jwt', () => ({ + getAuthorizationHeader: () => 'Bearer test-token', +})) + +// Mock the sync orchestrator so the /sync handler test can force a dead-session +// failure without hitting the network. +vi.mock('../lib/sync', () => ({ + syncAccountTransactions: vi.fn(), +})) + +import { + isSessionExpiredResponse, + SessionExpiredError, + getAllTransactionsWithRaw, +} from '../lib/api-client' +import { enableBankingExtension } from '../index' +import { syncAccountTransactions } from '../lib/sync' + +const CLOSED_SESSION_BODY = JSON.stringify({ + code: 401, + message: 'Session is closed', + error: 'CLOSED_SESSION', + detail: null, +}) + +describe('isSessionExpiredResponse', () => { + it('matches the CLOSED_SESSION 401 from the screenshot', () => { + expect(isSessionExpiredResponse(401, CLOSED_SESSION_BODY)).toBe(true) + }) + + it('matches the lowercase session_expired variant', () => { + expect(isSessionExpiredResponse(401, '{"error":"session_expired"}')).toBe(true) + }) + + it.each([ + '{"error":"EXPIRED_SESSION"}', + '{"error":"INVALID_SESSION"}', + '{"error":"SESSION_NOT_FOUND"}', + '{"error":"WRONG_SESSION_STATUS"}', + '{"message":"Session is closed"}', + ])('matches session-dead body %s', (body) => { + expect(isSessionExpiredResponse(401, body)).toBe(true) + // 403 is also a valid session-rejection status from some ASPSPs. + expect(isSessionExpiredResponse(403, body)).toBe(true) + }) + + it('does NOT match a bare 401 Unauthorized (app-credential problem, not a dead session)', () => { + expect(isSessionExpiredResponse(401, '{"error":"Unauthorized"}')).toBe(false) + }) + + it('does NOT match a non-401/403 status even with a session code in the body', () => { + expect(isSessionExpiredResponse(500, CLOSED_SESSION_BODY)).toBe(false) + expect(isSessionExpiredResponse(400, '{"error":"ASPSP_ERROR"}')).toBe(false) + }) +}) + +describe('getAllTransactionsWithRaw — dead session', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('throws SessionExpiredError on a CLOSED_SESSION 401', async () => { + const fetchMock = vi.fn(async () => ({ + ok: false, + status: 401, + statusText: 'Unauthorized', + text: async () => CLOSED_SESSION_BODY, + })) + vi.stubGlobal('fetch', fetchMock) + + await expect( + getAllTransactionsWithRaw('acc-1', '2026-01-01', '2026-06-01') + ).rejects.toBeInstanceOf(SessionExpiredError) + }) +}) + +const syncRoute = enableBankingExtension.apiRoutes?.find( + r => r.method === 'POST' && r.path === '/sync' +) + +if (!syncRoute) { + throw new Error('POST /sync route not registered on enable-banking extension') +} + +function makeContext(connection: Record, updateSpy: Mock, insertSpy?: Mock): ExtensionContext { + // One universal chainable per from() call. Each table only ever terminates on + // single() (bank_connections lookup) OR maybeSingle() (sie_imports / + // company_members), so a single shared resolver is unambiguous. update()/ + // insert() record their payloads and return the chain so trailing + // .eq()/.select() resolve. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const chain: any = {} + chain.select = vi.fn(() => chain) + chain.eq = vi.fn(() => chain) + chain.gte = vi.fn(() => chain) + chain.limit = vi.fn(() => chain) + chain.order = vi.fn(() => chain) + chain.insert = vi.fn((payload: unknown) => { + insertSpy?.(payload) + return chain + }) + chain.single = vi.fn().mockResolvedValue({ data: connection, error: null }) + chain.maybeSingle = vi.fn().mockResolvedValue({ data: null, error: null }) + chain.update = vi.fn((payload: unknown) => { + updateSpy(payload) + return chain + }) + + const supabase = { + auth: { + getUser: vi.fn().mockResolvedValue({ data: { user: { id: 'user-1' } }, error: null }), + }, + from: vi.fn(() => chain), + } + + return { + userId: 'user-1', + companyId: 'company-1', + extensionId: 'enable-banking', + requestId: 'req_test', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + supabase: supabase as any, + emit: vi.fn().mockResolvedValue(undefined), + settings: { get: vi.fn(), set: vi.fn(), getAll: vi.fn() } as never, + storage: {} as never, + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as never, + services: {} as never, + } +} + +function makeRequest(): Request { + return new Request('http://localhost/api/extensions/ext/enable-banking/sync', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ connection_id: 'conn-1' }), + }) +} + +describe('POST /sync (enable-banking) — dead session reconnect', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('flips the connection to expired and returns a reauth-required 409', async () => { + ;(syncAccountTransactions as unknown as Mock).mockRejectedValue( + new SessionExpiredError(401, CLOSED_SESSION_BODY) + ) + + const updateSpy = vi.fn() + const ctx = makeContext( + { + id: 'conn-1', + company_id: 'company-1', + status: 'active', + bank_name: 'Nordea', + accounts_data: [{ uid: 'acc-1', currency: 'SEK', enabled: true }] as StoredAccount[], + }, + updateSpy + ) + + const res = await syncRoute.handler(makeRequest(), ctx) + + expect(res.status).toBe(409) + const body = await res.json() + expect(body.reauth_required).toBe(true) + expect(body.code).toBe('SESSION_EXPIRED') + expect(body.connection_id).toBe('conn-1') + + // The connection must be marked 'expired' so the UI surfaces the reconnect + // affordance instead of looping on the dead session. + expect(updateSpy).toHaveBeenCalledWith( + expect.objectContaining({ status: 'expired' }) + ) + }) +}) + +const connectRoute = enableBankingExtension.apiRoutes?.find( + r => r.method === 'POST' && r.path === '/connect' +) + +if (!connectRoute) { + throw new Error('POST /connect route not registered on enable-banking extension') +} + +describe('POST /connect (enable-banking) — reconnect in place', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('reuses the existing row (UPDATE, no INSERT) and keeps it out of the stale-pending sweep', async () => { + // startAuthorization() POSTs to /auth — stub it (jwt is already mocked). + vi.stubGlobal( + 'fetch', + vi.fn(async () => ({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ url: 'https://bank.example/auth', authorization_id: 'auth-123' }), + text: async () => '', + })) + ) + + const updateSpy = vi.fn() + const insertSpy = vi.fn() + const ctx = makeContext( + { + id: 'conn-1', + company_id: 'company-1', + bank_name: 'Nordea', + provider: 'nordea-se', + session_id: null, // null → skip the best-effort revoke call + status: 'expired', + }, + updateSpy, + insertSpy + ) + + const req = new Request('http://localhost/api/extensions/ext/enable-banking/connect', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ connection_id: 'conn-1', aspsp_name: 'Nordea', aspsp_country: 'SE' }), + }) + + const res = await connectRoute.handler(req, ctx) + + expect(res.status).toBe(200) + const body = await res.json() + expect(body.connection_id).toBe('conn-1') + expect(body.authorization_url).toBe('https://bank.example/auth') + + // In-place: a fresh authorization on the SAME row, never a new INSERT. + expect(insertSpy).not.toHaveBeenCalled() + + // The CSRF state is staged on the row FIRST — before startAuthorization, so + // before authorization_id even exists — guaranteeing the callback can always + // find the row by oauth_state and the bank session can never be orphaned. + // It stays 'expired' (not 'pending') so the cron's stale-pending cleanup + // can't delete an established connection mid-reconnect. + const firstUpdate = updateSpy.mock.calls[0][0] + expect(firstUpdate).toMatchObject({ + oauth_state: expect.any(String), + status: 'expired', + session_id: null, + error_message: null, + }) + expect(firstUpdate).not.toHaveProperty('authorization_id') + + // The bank's authorization_id is recorded in a follow-up write (audit only; + // the callback never reads it, so a failure here can't break the reconnect). + expect(updateSpy.mock.calls[1][0]).toEqual({ authorization_id: 'auth-123' }) + }) +}) diff --git a/extensions/general/enable-banking/components/BankConnectionStatus.tsx b/extensions/general/enable-banking/components/BankConnectionStatus.tsx index 4ba1599f..7fd999c1 100644 --- a/extensions/general/enable-banking/components/BankConnectionStatus.tsx +++ b/extensions/general/enable-banking/components/BankConnectionStatus.tsx @@ -23,7 +23,7 @@ interface BankConnectionStatusProps { connection: BankConnection onSync: (connectionId: string) => void onDisconnect: (connectionId: string) => void - onReconnect?: (bank: { name: string; country: string }) => void + onReconnect?: (connection: BankConnection) => void onManageAccounts?: (connectionId: string) => void isSyncing?: boolean } @@ -132,21 +132,18 @@ export function BankConnectionStatus({
- {isConnectionExpired && onReconnect && ( + {(isConnectionExpired || isConnectionError) && onReconnect && ( )} {isConnectionError && (