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 aec23cf7..f38d73ad 100644 --- a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts +++ b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts @@ -65,6 +65,7 @@ vi.mock('@/lib/cash-accounts/service', () => ({ vi.stubEnv('NEXT_PUBLIC_APP_URL', 'http://localhost:3000') import { GET } from '../route' +import { eventBus } from '@/lib/events/bus' function makeRequest(params: Record) { const url = new URL('http://localhost:3000/api/extensions/enable-banking/callback') @@ -122,7 +123,9 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(response.status).toBe(307) const location = response.headers.get('location') || '' expect(location).toContain('/settings/banking?') - expect(location).toContain('bank_error=invalid_state') + // The raw 'invalid_state' token used to be shown verbatim: the banner now + // carries the Swedish explanation instead (issue #1716). + expect(decodeURIComponent(location)).toContain('Starta bankkopplingen på nytt') }) it('writes pending_selection and streams a finalizing page that redirects to the picker', async () => { @@ -1085,7 +1088,9 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(response.status).toBe(307) const location = response.headers.get('location') || '' expect(location).toContain('/settings/banking?') - expect(location).toContain('bank_error=User%20cancelled') + // The user-facing message is Swedish; a cancel is an expected outcome, so + // the raw provider text is not echoed back. + expect(decodeURIComponent(location)).toContain('Anslutningen avbröts hos banken') // No state → no DB cleanup attempted expect(mockFrom).not.toHaveBeenCalled() }) @@ -1104,7 +1109,7 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(response.status).toBe(307) const location = response.headers.get('location') || '' expect(location).toContain('/settings/banking?') - expect(location).toContain('bank_error=Denied%20data%20sharing%20consent') + expect(decodeURIComponent(location)).toContain('Anslutningen avbröts hos banken') // Should clean up the pending row expect(mockFrom).toHaveBeenCalledWith('bank_connections') }) @@ -1138,7 +1143,9 @@ describe('GET /api/extensions/enable-banking/callback', () => { const location = response.headers.get('location') || '' // URLSearchParams encodes spaces as '+', unlike the encodeURIComponent // fallback used when no matching row exists. - expect(location).toContain('bank_error=User+cancelled') + expect(decodeURIComponent(location.replace(/\+/g, ' '))).toContain( + 'Anslutningen avbröts hos banken' + ) expect(deleteCalls).toHaveLength(1) expect(updateCalls).toHaveLength(0) }) @@ -1172,6 +1179,10 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(deleteCalls).toHaveLength(0) expect(updateCalls).toHaveLength(1) expect(updateCalls[0].status).toBe('expired') + // The stored error_message is user-facing on the connection card: Swedish + // explanation with the raw provider description surfaced in parentheses. + expect(updateCalls[0].error_message).toContain('inloggningssession') + expect(updateCalls[0].error_message).toContain('Session expired at ASPSP') }) it('forwards bank_error_code and psu_type when the denied state matches a pending connection', async () => { @@ -1190,8 +1201,9 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(response.status).toBe(307) const location = response.headers.get('location') || '' expect(location).toContain('/settings/banking?') - // error_description is null for server_error, so the code doubles as message - expect(location).toContain('bank_error=server_error') + // A bare server_error used to surface as the literal token; the banner + // now gets the Swedish explanation (issue #1716). + expect(decodeURIComponent(location.replace(/\+/g, ' '))).toContain('fel på bankens sida') expect(location).toContain('bank_name=Handelsbanken') // The code is forwarded for every error, not just access_denied, together // with the connection's psu_type — the settings page keys the Handelsbanken @@ -1206,7 +1218,7 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(response.status).toBe(307) const location = response.headers.get('location') || '' expect(location).toContain('/settings/banking?') - expect(location).toContain('bank_error=missing_parameters') + expect(decodeURIComponent(location)).toContain('ofullständigt svar') }) it('redirects with error when code fails format validation', async () => { @@ -1215,6 +1227,85 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(response.status).toBe(307) const location = response.headers.get('location') || '' expect(location).toContain('/settings/banking?') - expect(location).toContain('bank_error=invalid_code_format') + expect(decodeURIComponent(location)).toContain('ogiltigt svar') + }) + + it('emits a durable consent_denied audit event when the bank denies with a matching row', async () => { + const emitSpy = vi.spyOn(eventBus, 'emit').mockResolvedValue(undefined) + try { + mockFrom.mockImplementation(() => + mockChain({ + data: { + id: 'conn-1', + user_id: 'user-1', + company_id: 'company-1', + bank_name: 'Handelsbanken', + psu_type: 'business', + status: 'pending', + }, + error: null, + }) + ) + + const response = await GET(makeRequest({ + error: 'server_error', + error_description: 'ASPSP authorization failed', + state: 'pending-state', + })) + + expect(response.status).toBe(307) + expect(emitSpy).toHaveBeenCalledWith({ + type: 'bank_connection.consent_denied', + payload: { + connectionId: 'conn-1', + bankName: 'Handelsbanken', + psuType: 'business', + errorCode: 'server_error', + errorDescription: 'ASPSP authorization failed', + priorStatus: 'pending', + userId: 'user-1', + companyId: 'company-1', + }, + }) + } finally { + emitSpy.mockRestore() + } + }) + + it('emits a durable finalize_failed audit event when the session exchange fails', async () => { + const emitSpy = vi.spyOn(eventBus, 'emit').mockResolvedValue(undefined) + try { + mockFrom.mockImplementation(() => + mockChain({ + data: { + id: 'conn-1', + user_id: 'user-1', + company_id: 'company-1', + bank_name: 'TestBank', + status: 'pending', + }, + error: null, + }) + ) + mockCreateSession.mockRejectedValue(new Error('upstream timeout')) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + expect(response.status).toBe(200) + await response.text() + + expect(emitSpy).toHaveBeenCalledWith({ + type: 'bank_connection.finalize_failed', + payload: { + connectionId: 'conn-1', + bankName: 'TestBank', + reason: 'upstream timeout', + priorStatus: 'pending', + userId: 'user-1', + companyId: 'company-1', + }, + }) + } finally { + emitSpy.mockRestore() + } }) }) diff --git a/app/api/extensions/enable-banking/callback/route.ts b/app/api/extensions/enable-banking/callback/route.ts index 66e330f2..d3dea077 100644 --- a/app/api/extensions/enable-banking/callback/route.ts +++ b/app/api/extensions/enable-banking/callback/route.ts @@ -14,6 +14,7 @@ import { } from '@/lib/cash-accounts/service' import { fanOutSessionRenewal } from '@/extensions/general/enable-banking/lib/session-sharing' import { supersedeSiblingConnections } from '@/extensions/general/enable-banking/lib/supersede' +import { getBankConnectionErrorMessage } from '@/lib/errors/get-error-message' import { renderFinalizeShell, renderFinalizeRedirect } from './finalize-page' // This route emits bank_connection.consent_granted / .cash_account_mirror_failed @@ -82,7 +83,11 @@ export async function GET(request: Request) { const baseUrl = process.env.NEXT_PUBLIC_APP_URL || 'http://localhost:3000' if (error) { - const errorMessage = errorDescription || error + // Swedish user-facing message carrying the underlying provider error; the + // raw code/description stays in the log lines and the audit event below. + // Previously the raw provider text was passed through verbatim, which + // gave a stuck user nothing to act on (issue #1716). + const userMessage = getBankConnectionErrorMessage(error, errorDescription) // access_denied is the user cancelling at the bank — an expected outcome, // not a runtime error. Only bank-side failures stay at error level. const isUserCancel = @@ -104,7 +109,7 @@ export async function GET(request: Request) { // (which stays 'expired' during the round-trip) is also handled. const { data: pendingConn } = await supabase .from('bank_connections') - .select('id, user_id, bank_name, psu_type, status') + .select('id, user_id, company_id, bank_name, psu_type, status') .eq('oauth_state', state) .in('status', ['pending', 'expired', 'error']) .single() @@ -142,16 +147,41 @@ export async function GET(request: Request) { await supabase .from('bank_connections') - .update({ status: isSessionExpiry ? 'expired' : 'error', error_message: errorMessage, oauth_state: null }) + .update({ status: isSessionExpiry ? 'expired' : 'error', error_message: userMessage, oauth_state: null }) .eq('id', pendingConn.id) } + // Durable audit trail for the failed attempt (issue #1716): the + // fresh-connect row was just deleted and console logs expire, so + // event_log is the only place support can later see which attempt + // failed with which provider error. + try { + await eventBus.emit({ + type: 'bank_connection.consent_denied', + payload: { + connectionId: pendingConn.id, + bankName: pendingConn.bank_name ?? null, + psuType: pendingConn.psu_type ?? null, + errorCode: error, + errorDescription: errorDescription ?? null, + priorStatus: pendingConn.status, + userId: pendingConn.user_id, + companyId: pendingConn.company_id, + }, + }) + } catch (emitError) { + log.error(AUDIT_EMIT_FAILED, emitError as Error, { + eventType: 'bank_connection.consent_denied', + connectionId: pendingConn.id, + }) + } + // Include bank name, error code, and psu_type in the redirect so the // UI can render targeted guidance (e.g. PSU-type retry on // access_denied, or the Handelsbanken corporate fullmakt steps on // server_error for a business connect). const params = new URLSearchParams({ - bank_error: errorMessage, + bank_error: userMessage, ...(pendingConn.bank_name ? { bank_name: pendingConn.bank_name } : {}), bank_error_code: error, ...(pendingConn.psu_type ? { psu_type: pendingConn.psu_type } : {}), @@ -164,18 +194,22 @@ export async function GET(request: Request) { } return NextResponse.redirect( - `${baseUrl}/settings/banking?bank_error=${encodeURIComponent(errorMessage)}` + `${baseUrl}/settings/banking?bank_error=${encodeURIComponent(userMessage)}` ) } if (!code || !state) { - return NextResponse.redirect(`${baseUrl}/settings/banking?bank_error=missing_parameters`) + return NextResponse.redirect( + `${baseUrl}/settings/banking?bank_error=${encodeURIComponent(getBankConnectionErrorMessage('missing_parameters'))}` + ) } // Validate authorization code format const codePattern = /^[a-zA-Z0-9._~+\/-]{8,2048}$/ if (!codePattern.test(code)) { - return NextResponse.redirect(`${baseUrl}/settings/banking?bank_error=invalid_code_format`) + return NextResponse.redirect( + `${baseUrl}/settings/banking?bank_error=${encodeURIComponent(getBankConnectionErrorMessage('invalid_code_format'))}` + ) } const supabase = await createServiceClient() @@ -201,7 +235,7 @@ export async function GET(request: Request) { hasCode: !!code, }) return NextResponse.redirect( - `${baseUrl}/settings/banking?bank_error=${encodeURIComponent('invalid_state')}` + `${baseUrl}/settings/banking?bank_error=${encodeURIComponent(getBankConnectionErrorMessage('invalid_state'))}` ) } @@ -214,13 +248,36 @@ export async function GET(request: Request) { try { return await finalizeConnection(supabase, pendingConnection, code) } catch (finalizeError) { + const reason = + finalizeError instanceof Error ? finalizeError.message : String(finalizeError) console.error('[enable-banking] Callback error', { - message: finalizeError instanceof Error ? finalizeError.message : String(finalizeError), + message: reason, stack: finalizeError instanceof Error ? finalizeError.stack : undefined, name: finalizeError instanceof Error ? finalizeError.name : undefined, state, connectionId: pendingConnection.id, }) + // Durable audit trail (issue #1716): the fresh-connect row is deleted by + // the cleanup below and console logs expire, so event_log is the only + // place support can later see that this attempt failed and why. + try { + await eventBus.emit({ + type: 'bank_connection.finalize_failed', + payload: { + connectionId: pendingConnection.id, + bankName: pendingConnection.bank_name ?? null, + reason, + priorStatus: pendingConnection.status, + userId: pendingConnection.user_id, + companyId: pendingConnection.company_id, + }, + }) + } catch (emitError) { + log.error(AUDIT_EMIT_FAILED, emitError as Error, { + eventType: 'bank_connection.finalize_failed', + connectionId: pendingConnection.id, + }) + } return cleanupFailedFinalize(supabase, pendingConnection) } })() diff --git a/lib/errors/__tests__/get-bank-connection-error-message.test.ts b/lib/errors/__tests__/get-bank-connection-error-message.test.ts new file mode 100644 index 00000000..fb8462d6 --- /dev/null +++ b/lib/errors/__tests__/get-bank-connection-error-message.test.ts @@ -0,0 +1,66 @@ +import { describe, it, expect } from 'vitest' +import { getBankConnectionErrorMessage } from '../get-error-message' + +// The PSD2 callback mapper (issue #1716): raw provider tokens used to reach +// the user verbatim ("server_error", "invalid_state"), which left them with +// nothing to act on and support with nothing to answer. +describe('getBankConnectionErrorMessage', () => { + it('maps a user cancel (access_denied) without echoing the provider text', () => { + const msg = getBankConnectionErrorMessage('access_denied', 'User cancelled') + expect(msg).toContain('Anslutningen avbröts hos banken') + expect(msg).not.toContain('User cancelled') + }) + + it('treats "Cancelled by user" descriptions as a cancel regardless of code', () => { + const msg = getBankConnectionErrorMessage('server_error', 'Cancelled by user') + expect(msg).toContain('Anslutningen avbröts hos banken') + }) + + it('maps a bare server_error to the bank-side failure explanation', () => { + const msg = getBankConnectionErrorMessage('server_error') + expect(msg).toContain('fel på bankens sida') + // The Handelsbanken corporate case: point at mandates without naming a bank + // (the settings page adds the bank-specific steps from bank_error_code). + expect(msg).toContain('fullmakt') + }) + + it('surfaces the provider description in parentheses on unknown codes', () => { + const msg = getBankConnectionErrorMessage('aspsp_error', 'PSU lacks corporate mandate') + expect(msg).toContain('Banken avvisade anslutningen') + expect(msg).toContain('(PSU lacks corporate mandate)') + }) + + it('does not duplicate the code when the description equals the code', () => { + const msg = getBankConnectionErrorMessage('server_error', 'server_error') + expect(msg).toContain('fel på bankens sida') + expect(msg).not.toContain('(server_error)') + }) + + it('maps session-expiry descriptions to the expired-session message', () => { + const msg = getBankConnectionErrorMessage('server_error', 'Session expired at ASPSP') + expect(msg).toContain('inloggningssession') + expect(msg).toContain('(Session expired at ASPSP)') + }) + + it('maps the internal invalid_state token to a retry explanation', () => { + const msg = getBankConnectionErrorMessage('invalid_state') + expect(msg).toContain('Starta bankkopplingen på nytt') + expect(msg).not.toContain('invalid_state') + }) + + it('maps missing_parameters and invalid_code_format to Swedish', () => { + expect(getBankConnectionErrorMessage('missing_parameters')).toContain('ofullständigt svar') + expect(getBankConnectionErrorMessage('invalid_code_format')).toContain('ogiltigt svar') + }) + + it('maps temporarily_unavailable to the try-later message', () => { + const msg = getBankConnectionErrorMessage('temporarily_unavailable') + expect(msg).toContain('tillfälligt otillgänglig') + }) + + it('falls back to a Swedish rejection message for unknown codes without description', () => { + const msg = getBankConnectionErrorMessage('weird_code') + expect(msg).toContain('Banken avvisade anslutningen') + expect(msg).not.toContain('weird_code') + }) +}) diff --git a/lib/errors/get-error-message.ts b/lib/errors/get-error-message.ts index 8230a4d6..ae375a8b 100644 --- a/lib/errors/get-error-message.ts +++ b/lib/errors/get-error-message.ts @@ -574,6 +574,75 @@ export function getErrorMessage( return pick(GENERIC_FALLBACK, locale) } +// PSD2 bank-connection OAuth callback errors. The Enable Banking callback +// route redirects the browser back to /settings/banking with a user-facing +// message. The raw provider code/description used to be passed through +// verbatim ("server_error", "invalid_state"), which left a stuck user with +// nothing to act on and support with nothing to answer (issue #1716: the +// Handelsbanken corporate fullmakt failures). Known codes get a Swedish +// explanation; the raw provider description is appended in parentheses so +// the underlying error still reaches the user (and a screenshot to support). +const BANK_CONNECTION_ERROR_MAP: Record = { + server_error: + 'Banken kunde inte slutföra godkännandet på grund av ett fel på bankens sida. Försök igen om en stund. Gäller det företagskonton kan banken kräva en fullmakt innan kopplingen godkänns.', + temporarily_unavailable: + 'Bankens anslutningstjänst är tillfälligt otillgänglig. Försök igen om en stund.', + invalid_request: + 'Banken avvisade anslutningsförfrågan som ogiltig. Försök igen, och kontakta supporten om felet kvarstår.', + // Internal callback tokens (not from the bank) that were previously shown raw. + invalid_state: + 'Anslutningsförsöket kunde inte matchas mot ett pågående försök. Det kan hända om försöket tog för lång tid eller om ett nytt försök startades under tiden. Starta bankkopplingen på nytt.', + missing_parameters: + 'Banken skickade ett ofullständigt svar tillbaka. Starta bankkopplingen på nytt.', + invalid_code_format: + 'Banken skickade ett ogiltigt svar tillbaka. Starta bankkopplingen på nytt.', +} + +const BANK_CONNECTION_CANCELLED_MESSAGE = + 'Anslutningen avbröts hos banken innan den slutfördes. Ingen bankkoppling skapades. Försök igen och slutför alla steg hos banken.' + +const BANK_CONNECTION_SESSION_EXPIRED_MESSAGE = + 'Bankens inloggningssession hann gå ut innan anslutningen slutfördes. Starta bankkopplingen på nytt och slutför alla steg hos banken direkt.' + +const BANK_CONNECTION_FALLBACK_MESSAGE = + 'Banken avvisade anslutningen. Försök igen, och kontakta supporten om felet kvarstår.' + +// Same shape the callback route keys its expired-vs-error decision on. +const BANK_SESSION_EXPIRY_PATTERN = + /session.?expired|expired.?session|closed.?session|session.?closed|invalid.?session|session.?not.?found/i + +/** + * Map a PSD2 authorization callback outcome (OAuth error code plus optional + * provider description) to a Swedish user message. Always Swedish: the bank + * redirect carries no locale, and bank-connection surfaces follow the + * user-facing-errors-are-Swedish rule. + */ +export function getBankConnectionErrorMessage( + errorCode: string, + errorDescription?: string | null +): string { + const code = errorCode.trim() + const description = errorDescription?.trim() || null + const combined = `${code} ${description ?? ''}` + + // User cancelled at the bank: an expected outcome, keep it clean without + // echoing the provider text back. + if (code === 'access_denied' || /cancel/i.test(combined)) { + return BANK_CONNECTION_CANCELLED_MESSAGE + } + + let base: string + if (BANK_SESSION_EXPIRY_PATTERN.test(combined)) { + base = BANK_CONNECTION_SESSION_EXPIRED_MESSAGE + } else { + base = BANK_CONNECTION_ERROR_MAP[code] ?? BANK_CONNECTION_FALLBACK_MESSAGE + } + + // Surface the underlying provider error: without it the user (and support, + // via a screenshot) cannot tell one failure from another. + return description && description !== code ? `${base} (${description})` : base +} + /** * Helper that parses a Response body and returns a user-friendly error message. */ diff --git a/lib/events/handlers/event-log-handler.ts b/lib/events/handlers/event-log-handler.ts index 09f44e39..8943b1dc 100644 --- a/lib/events/handlers/event-log-handler.ts +++ b/lib/events/handlers/event-log-handler.ts @@ -60,6 +60,12 @@ const PERSISTED_EVENT_TYPES: CoreEventType[] = [ // Bank connection consent lifecycle: required audit trail per ASVS V16 // and GDPR Art.30 (records of processing) for PSD2 consent decisions. 'bank_connection.consent_granted', + // Denied/failed consent attempts: the pending bank_connections row is + // deleted on failure and console logs expire, so this durable trail is the + // only way support can answer "which attempt, which error, whose side" + // (issue #1716). + 'bank_connection.consent_denied', + 'bank_connection.finalize_failed', 'bank_connection.account_selection_changed', 'bank_connection.revoked', 'bank_connection.superseded', diff --git a/lib/events/types.ts b/lib/events/types.ts index 57db0607..5d5b3efc 100644 --- a/lib/events/types.ts +++ b/lib/events/types.ts @@ -90,6 +90,35 @@ export type CoreEvent = // superseded_by pointing at the replacement, and its transactions are // re-pointed. connectionId is the SUPERSEDED (old) row, mirroring .revoked. | { type: 'bank_connection.superseded'; payload: { connectionId: string; supersededById: string; bankName: string | null; userId: string; companyId: string } } + // Emitted when the bank/provider redirects the OAuth callback back with an + // error instead of an authorization code: denied consent, a bank-side + // failure (e.g. Handelsbanken's missing corporate fullmakt), or an expired + // signing session. Durable failure trail (issue #1716): the pending row is + // deleted right after and console logs expire, so without this event + // support cannot answer which attempt failed, with which error, on whose + // side. connectionId may reference a row deleted by the same request. + | { type: 'bank_connection.consent_denied'; payload: { + connectionId: string + bankName: string | null + psuType: string | null + errorCode: string + errorDescription: string | null + priorStatus: string + userId: string + companyId: string + } } + // Emitted when the code-for-session exchange or connection finalization + // throws after the bank redirected back successfully. Same audit doctrine + // as consent_denied: the fresh-connect row is deleted by cleanup and the + // console log expires, leaving support nothing to answer from. + | { type: 'bank_connection.finalize_failed'; payload: { + connectionId: string + bankName: string | null + reason: string + priorStatus: string + userId: string + companyId: string + } } // Emitted when the PSD2 callback fails to mirror a returned account into // cash_accounts. ASVS V16 / ISO 27001 A.8.15: security-relevant failures // must land in a structured audit log (event_log, 30-day TTL) rather than