fix(bank): Swedish provider errors and a durable failure trail for bank connect attempts (#1841)
Issue #1716: a user stuck in Handelsbankens fullmakt step got a raw provider token back (server_error, invalid_state) and support had nothing to look at afterwards: the failed pending row is deleted by design, the callback only logged to console (short retention), and event_log recorded successes only. Diagnosis of the reported case: the failures were on the bank's side (the corporate fullmakt requirement); both of the reporter's companies connected successfully on 2026-08-12 with no code change on our side in between, and the connections have been active and syncing since. Changes: - lib/errors/get-error-message.ts: getBankConnectionErrorMessage() maps PSD2 callback outcomes (access_denied, server_error, temporarily_unavailable, session expiry, plus the internal invalid_state, missing_parameters and invalid_code_format tokens) to Swedish user messages, appending the raw provider description so the underlying error is still surfaced. - callback route: every bank_error redirect and the stored error_message now carry the mapped Swedish text; bank_error_code, bank_name and psu_type still flow so the settings page keeps its targeted guidance (Handelsbanken fullmakt steps included). - New audit events bank_connection.consent_denied and bank_connection.finalize_failed are emitted on the two failure paths and persisted to event_log, so support can answer which attempt failed, with which provider error, on whose side, even after the row is gone. Claude-Session: https://claude.ai/code/session_01SyDuePXxUFowaPBKpAv8SF Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Jakob Wennberg
Claude Fable 5
Jakob Wennberg
Claude Fable 5
parent
1307d4db2e
commit
64119d30bc
@@ -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<string, string>) {
|
||||
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()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})()
|
||||
|
||||
Reference in New Issue
Block a user