From 2d220394615c1be82d10c2c17da0901c96665068 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:36:18 +0200 Subject: [PATCH] fix(bank): renewal reuses IBAN-matched ledgers and keeps deselected accounts deselected (#1805) * fix(bank): renewal reuses IBAN-matched ledgers and keeps deselected accounts deselected Two Enable Banking renewal defects reported from a SEB connection: 1. Dead 19xx accounts per renewal. The callback pre-seeded the resolver's exclude set with every ledger the connection already mirrored. SEB mints new account uids on re-auth, so no uid matched, the IBAN hit on the old row was rejected by its own ledger being excluded, and a fresh 195x slot was allocated (and created in the chart) on every renewal. Only ledgers still claimed by a uid present in the new session are excluded now; the stale row is promoted via the IBAN match as intended. Stale ledgers stay safe from the allocator, which already skips every cash_accounts ledger. 2. Deselected accounts came back pre-checked. accounts_data was rebuilt with enabled:true unconditionally, so a private card set to "Synkas ej" was re-enabled and the mirror flipped cash_accounts.enabled back. The prior flag is now carried over by IBAN, then uid; only genuinely new accounts default to enabled. Co-Authored-By: Claude Fable 5 * fix(bank): prefer exact uid over IBAN when carrying the sync-enabled flag Skeptic refutation: one session can list the same IBAN twice (one resource per balance type). IBAN-first lookup made the first prior entry win for both, so a deselected duplicate could re-enable, or the live account could come back deselected and silently stop syncing. Exact uid identity now wins; IBAN is the fallback for ASPSPs that mint new uids on re-auth. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- .../callback/__tests__/route.test.ts | 183 ++++++++++++++++++ .../enable-banking/callback/route.ts | 38 +++- 2 files changed, 216 insertions(+), 5 deletions(-) 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 23934028..b3fbc653 100644 --- a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts +++ b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts @@ -342,6 +342,189 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(mirrored.external_uid).toBe('acc-new') }) + it('does not let a stale-uid mirrored row block the IBAN reuse of its own ledger on renewal', async () => { + // In-place renewal ("Förnya") of a connection whose ASPSP mints new uids + // on every re-auth. The connection already mirrors 1930/1940 under the OLD + // uids. Those ledgers must NOT be pre-seeded into the resolver's exclude + // set: the IBAN match on the stale row is the mapping to promote, and + // excluding it made every renewal allocate a fresh 19xx sub-account. + let callIndex = 0 + mockFrom.mockImplementation((table: string) => { + callIndex++ + if (callIndex === 1) { + return mockChain({ + data: { + id: 'conn-1', user_id: 'user-1', company_id: 'company-1', bank_name: 'SEB', status: 'expired', + accounts_data: [ + { uid: 'acc-old-1', iban: 'SE1234', name: 'Företagskonto', currency: 'SEK', enabled: true }, + { uid: 'acc-old-2', iban: 'SE5678', name: 'Sparkonto', currency: 'SEK', enabled: true }, + ], + }, + error: null, + }) + } + if (table === 'cash_accounts') { + return mockChain({ + data: [ + { external_uid: 'acc-old-1', ledger_account: '1930' }, + { external_uid: 'acc-old-2', ledger_account: '1940' }, + ], + error: null, + }) + } + const chain: Record = {} + chain.update = vi.fn(() => chain) + chain.eq = vi.fn().mockReturnValue(chain) + chain.select = vi.fn().mockReturnValue(chain) + chain.in = vi.fn().mockReturnValue(chain) + chain.single = vi.fn().mockResolvedValue({ + data: { id: 'conn-1', bank_name: 'SEB', company_id: 'company-1', user_id: 'user-1', status: 'expired' }, + error: null, + }) + chain.then = (resolve: (v: unknown) => void) => resolve({ data: null, error: null }) + return chain + }) + + // Resolver stand-in: answer the IBAN hit only when the caller did NOT + // exclude that ledger (mirrors resolvePsd2LedgerAccount's guard), else + // fall back to the allocator. + const ibanLedgers: Record = { + SE1234: { ledger: '1930', rowId: 'row-1' }, + SE5678: { ledger: '1940', rowId: 'row-2' }, + } + const seenExcludes: string[][] = [] + mockAllocate.mockImplementation( + async (_s: unknown, _c: unknown, _u: unknown, input: { iban?: string; currency: string; exclude?: ReadonlySet }) => { + const exclude = input.exclude ?? new Set() + seenExcludes.push([...exclude].sort()) + const hit = input.iban ? ibanLedgers[input.iban] : undefined + if (hit && !exclude.has(hit.ledger)) { + return { ledgerAccount: hit.ledger, reuseCashAccountId: hit.rowId, source: 'iban' } + } + for (let n = 1931; n <= 1959; n++) { + const candidate = String(n) + if (!exclude.has(candidate) && !['1932', '1933', '1934'].includes(candidate)) return candidate + } + return null + }, + ) + + mockCreateSession.mockResolvedValue({ + session_id: 'sess-3', + accounts: [ + { uid: 'acc-new-1', account_id: { iban: 'SE1234' }, name: 'Företagskonto', currency: 'SEK' }, + { uid: 'acc-new-2', account_id: { iban: 'SE5678' }, name: 'Sparkonto', currency: 'SEK' }, + ], + access: { valid_until: '2024-12-31T00:00:00Z' }, + aspsp: { name: 'SEB', country: 'SE' }, + }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + expect(response.status).toBe(200) + await response.text() + + // The first resolver call saw no pre-seeded excludes (no new-session uid + // matched a mirrored row), the second saw only the ledger just claimed. + expect(seenExcludes).toEqual([[], ['1930']]) + // Both accounts land back on their own ledgers and promote their rows. + const mirrored = mockUpsertFromPsd2.mock.calls.map( + (c) => c[2] as { ledger_account: string; reuse_cash_account_id: string | null; external_uid: string }, + ) + expect(mirrored.map((m) => [m.external_uid, m.ledger_account, m.reuse_cash_account_id])).toEqual([ + ['acc-new-1', '1930', 'row-1'], + ['acc-new-2', '1940', 'row-2'], + ]) + }) + + it('keeps a previously deselected account deselected on renewal, by IBAN or uid', async () => { + // "SEB Credit" was set to "Synkas ej" (enabled:false) in the old + // connection. On renewal it comes back under a NEW uid (same IBAN) and a + // no-IBAN card comes back under the SAME uid. Both must stay deselected; + // only the genuinely new account defaults to enabled. + const capturedUpdates: Record[] = [] + let callIndex = 0 + mockFrom.mockImplementation((table: string) => { + callIndex++ + if (callIndex === 1) { + return mockChain({ + data: { + id: 'conn-1', user_id: 'user-1', company_id: 'company-1', bank_name: 'SEB', status: 'expired', + accounts_data: [ + { uid: 'acc-old-1', iban: 'SE1234', name: 'Företagskonto', currency: 'SEK', enabled: true }, + { uid: 'card-old', iban: 'SE9999', name: 'SEB Credit', currency: 'SEK', enabled: false }, + { uid: 'card-noiban', name: 'Privatkort', currency: 'SEK', enabled: false }, + // Same IBAN listed twice (one resource per balance type): the + // user unticked the duplicate and kept the main one. Exact uid + // identity must win over the IBAN fallback in both directions. + { uid: 'dup-resource', iban: 'SE7777', name: 'Lönekonto (saldo)', currency: 'SEK', enabled: false }, + { uid: 'main-resource', iban: 'SE7777', name: 'Lönekonto', currency: 'SEK', enabled: true }, + ], + }, + error: null, + }) + } + if (table === 'cash_accounts') return mockChain({ data: [], error: null }) + const chain: Record = {} + chain.update = vi.fn((payload: Record) => { + capturedUpdates.push(payload) + return chain + }) + chain.eq = vi.fn().mockReturnValue(chain) + chain.select = vi.fn().mockReturnValue(chain) + chain.in = vi.fn().mockReturnValue(chain) + chain.single = vi.fn().mockResolvedValue({ + data: { id: 'conn-1', bank_name: 'SEB', company_id: 'company-1', user_id: 'user-1', status: 'expired' }, + error: null, + }) + chain.then = (resolve: (v: unknown) => void) => resolve({ data: null, error: null }) + return chain + }) + + mockCreateSession.mockResolvedValue({ + session_id: 'sess-4', + accounts: [ + { uid: 'acc-new-1', account_id: { iban: 'SE1234' }, name: 'Företagskonto', currency: 'SEK' }, + { uid: 'card-new', account_id: { iban: 'SE 9999' }, name: 'SEB Credit', currency: 'SEK' }, + { uid: 'card-noiban', name: 'Privatkort', currency: 'SEK' }, + { uid: 'acc-brand-new', account_id: { iban: 'SE4444' }, name: 'Nytt konto', currency: 'SEK' }, + { uid: 'dup-resource', account_id: { iban: 'SE7777' }, name: 'Lönekonto (saldo)', currency: 'SEK' }, + { uid: 'main-resource', account_id: { iban: 'SE7777' }, name: 'Lönekonto', currency: 'SEK' }, + ], + access: { valid_until: '2024-12-31T00:00:00Z' }, + aspsp: { name: 'SEB', country: 'SE' }, + }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + expect(response.status).toBe(200) + await response.text() + + const accountsData = capturedUpdates[0].accounts_data as Array<{ uid: string; enabled: boolean }> + expect(Object.fromEntries(accountsData.map((a) => [a.uid, a.enabled]))).toEqual({ + 'acc-new-1': true, + 'card-new': false, + 'card-noiban': false, + 'acc-brand-new': true, + 'dup-resource': false, + 'main-resource': true, + }) + // The mirror carries the same flag, so cash_accounts.enabled is not + // flipped back to true by the renewal. + const mirroredEnabled = Object.fromEntries( + mockUpsertFromPsd2.mock.calls.map((c) => { + const input = c[2] as { external_uid: string; enabled: boolean } + return [input.external_uid, input.enabled] + }), + ) + expect(mirroredEnabled).toEqual({ + 'acc-new-1': true, + 'card-new': false, + 'card-noiban': false, + 'acc-brand-new': true, + 'dup-resource': false, + 'main-resource': true, + }) + }) + it('runs the same-bank supersede pass with the new session, accounts, and connection identity', async () => { // Fresh connect while an EXPIRED sibling to the same bank exists: the // supersede pass (unit-tested separately) must be handed everything it diff --git a/app/api/extensions/enable-banking/callback/route.ts b/app/api/extensions/enable-banking/callback/route.ts index 953e3b20..0cde4c84 100644 --- a/app/api/extensions/enable-banking/callback/route.ts +++ b/app/api/extensions/enable-banking/callback/route.ts @@ -329,12 +329,23 @@ async function finalizeConnection( // carried sibling scope onto an account whose own scope is NOT explicit. const priorScopeByIban = new Map() const priorScopeByUid = new Map() + // The user's earlier sync choice per account ("Synkas ej" = enabled:false) + // must survive a renewal: a deselected private card that comes back + // pre-checked lands its transactions in the company's books the moment the + // user saves the picker with defaults. Matched by uid first (exact resource + // identity; one session can list the same IBAN twice, e.g. one resource per + // balance type), then by IBAN for ASPSPs that mint new uids on re-auth. + const priorEnabledByIban = new Map() + const priorEnabledByUid = new Map() for (const prior of priorAccounts) { const priorIban = normalizeIban(prior.iban) const priorScope = prior.dedup_scope || priorIban || prior.uid const priorEntry = { scope: priorScope, explicit: Boolean(prior.dedup_scope) } if (priorIban && !priorScopeByIban.has(priorIban)) priorScopeByIban.set(priorIban, priorEntry) if (!priorScopeByUid.has(prior.uid)) priorScopeByUid.set(prior.uid, priorEntry) + const priorEnabled = prior.enabled !== false + if (priorIban && !priorEnabledByIban.has(priorIban)) priorEnabledByIban.set(priorIban, priorEnabled) + if (!priorEnabledByUid.has(prior.uid)) priorEnabledByUid.set(prior.uid, priorEnabled) } const accountsMetadata: StoredAccount[] = accounts.map((account: AccountInfo) => { @@ -344,10 +355,14 @@ async function finalizeConnection( iban: account.account_id?.iban, name: account.name || account.product, currency: account.currency, - // Default to enabled. The user is presented with a picker - // immediately after this callback to uncheck unwanted accounts - // before any transactions are fetched. - enabled: true, + // Carry the user's earlier choice for an account we have seen before; + // only genuinely new accounts default to enabled. The picker shown + // right after this callback pre-checks from this flag, and no + // transactions are fetched before the user saves it. + enabled: + priorEnabledByUid.get(account.uid) ?? + (normalizedIban ? priorEnabledByIban.get(normalizedIban) : undefined) ?? + true, // Pin the external_id account scope at first ingest so it survives // re-authorizations. Byte-identical to the derivation lib/sync.ts // applied before this field existed (normalized IBAN, else uid). @@ -479,7 +494,20 @@ async function finalizeConnection( (r) => [r.external_uid, r.ledger_account], ), ) - const assignedLedgers = new Set(existingLedgerByUid.values()) + // Only ledgers still claimed by a uid the bank returned in THIS session + // block the resolver. A row whose uid the ASPSP retired on re-auth (SEB + // mints new uids on every renewal) is exactly the row the IBAN match must + // promote; seeding its ledger into the exclude set made the resolver reject + // its own IBAN hit and allocate a fresh 19xx slot per renewal, so the chart + // grew a dead sub-account each time. Stale ledgers are still safe from the + // allocator: findFreeLedgerAccount skips every ledger a cash_accounts row + // holds, whatever its uid. + const sessionUids = new Set(accountsMetadata.map((a) => a.uid)) + const assignedLedgers = new Set( + [...existingLedgerByUid.entries()] + .filter(([uid]) => sessionUids.has(uid)) + .map(([, ledger]) => ledger), + ) let accountsDataDirty = carriedScopeDirty for (const account of accountsMetadata) {