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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Mattsson
co-authored by
Claude Fable 5
Claude Fable 5
parent
e53b478833
commit
2d22039461
@@ -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<string, unknown> = {}
|
||||
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<string, { ledger: string; rowId: string }> = {
|
||||
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<string> }) => {
|
||||
const exclude = input.exclude ?? new Set<string>()
|
||||
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<string, unknown>[] = []
|
||||
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<string, unknown> = {}
|
||||
chain.update = vi.fn((payload: Record<string, unknown>) => {
|
||||
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
|
||||
|
||||
@@ -329,12 +329,23 @@ async function finalizeConnection(
|
||||
// carried sibling scope onto an account whose own scope is NOT explicit.
|
||||
const priorScopeByIban = new Map<string, { scope: string; explicit: boolean }>()
|
||||
const priorScopeByUid = new Map<string, { scope: string; explicit: boolean }>()
|
||||
// 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<string, boolean>()
|
||||
const priorEnabledByUid = new Map<string, boolean>()
|
||||
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<string>(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<string>(
|
||||
[...existingLedgerByUid.entries()]
|
||||
.filter(([uid]) => sessionUids.has(uid))
|
||||
.map(([, ledger]) => ledger),
|
||||
)
|
||||
let accountsDataDirty = carriedScopeDirty
|
||||
|
||||
for (const account of accountsMetadata) {
|
||||
|
||||
Reference in New Issue
Block a user