diff --git a/DECISIONS.md b/DECISIONS.md index 8b65274f..a1d36050 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1427,6 +1427,7 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-09-01] multi_user skeptic fixes: Stripe cancel EXPIRES the multi_user stripe grant instead of deleting it (grace anchor; other grants still deleted per freeze-and-retain); app-side state checks go RPC-first via SECURITY DEFINER company_multi_user_state (capability_grants RLS hides team rows from non-team users, byrå clients would misread as frozen); byra-kind teams get a standing team-scoped multi_user grant via backfill + teams trigger (WL-10 assumption made real; partner billing is out-of-band); PGRST202 on resolution fails OPEN (pre-migration DB has no multi_user rows: gated fallback would freeze all non-owners); /api/v1 got the same dormancy gate as MCP. RLS-level enforcement and the mid-session API fallback write-back window stay v2 follow-ups (documented, same class as pre-existing stale-preference fallback). [2026-09-01] Declined CodeRabbit's UpgradeNote suggestion (PR #1758 follow-up) to append the self-host connector sentence to children instead of replacing them: every caller's children is hosted subscription copy ("... kräver ett abonnemang"), so appending would show subscription wording on a self-host, the exact thing the branch exists to avoid; the "CSV/SIE import stays free" text it cited is a code comment in BankSyncNowButton, not children. Replace-on-self-host stays; a dedicated selfHosted children prop can come when a caller actually needs per-panel reassurance there. [2026-09-01] getConnectorConfig() rebuilds baseUrl as origin + path (userinfo/query/fragment stripped, warn-logged without the raw value): /api/connector/status echoes baseUrl to the operator and the sync/proxy URLs get paths appended, so nothing secret-shaped pasted into GNUBOK_CONNECT_URL may survive; the stripped parts were never meaningful in a base URL. The status route is also Cache-Control: no-store (key prefix + wiring layout out of shared browser caches). +[2026-09-01] EB callback cross-company guard: an account the connection row itself already carried keeps its own enabled state even when a sibling company claims the IBAN (own-prior outranks claim): flipping a standing feed off on renewal would break legacy pre-session-sharing overlaps, while the F1 exposure only involves accounts NEW to the row. Claim-lookup failure fails closed to enabled:false but still mirrors (unknown claim state is not a proven foreign claim; skipping the mirror on a transient error would strand normal single-company connects). [2026-09-01] Provider-migration kreditfakturor import as ordinary invoice rows with reversed amounts and status 'credited', always unlinked. No provider DTO carries a reference to the credited invoice, so exact-match linking has no input to match on; guessing would corrupt the AR ledger. The wizard now says so rather than burying them in `skipped`. [2026-09-01] OAuth callback replay is fixed with no-store plus history replacement only, with no "Anslutningen ar redan klar" page. Keeps the anti-oracle property that consumed-vs-unknown state stays unobservable to an unauthenticated caller. [2026-09-01] PostgREST embeds between a table pair with more than one foreign key must name the relationship. Enforced by scripts/checks/ambiguous-embed.mjs in the ratchet guard, because neither test layer can see this class: a mocked Supabase client never resolves a relationship, and pg-real bypasses PostgREST entirely. Third appearance, second shipped instance. @@ -1436,3 +1437,5 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-09-01] Anon-callable SECURITY DEFINER writes: the guard shape `IF auth.uid() IS NOT NULL AND NOT EXISTS (membership)` is unsafe on its own. The anon JWT carries no `sub` claim, so auth.uid() is NULL for role anon too and the guard short-circuits into the trusted branch. It is defense in depth behind a REVOKE FROM PUBLIC, anon, never a substitute for one. Every new SECURITY DEFINER function ships with that REVOKE; tests/pg/definer-function-grants.pg.test.ts enforces it from a sweep rather than a hand list, because hand-listing is exactly how three guarded numbering RPCs were wrongly declared safe. [2026-09-01] public.create_invoice_with_items(jsonb,jsonb) is revoked, not dropped. It is prod-only, uncalled and already non-functional, so removing it is cleanup rather than security, and the revoke closes the hole in full. An irreversible schema deletion against production belongs in its own reviewable migration. [2026-09-01] SKV connector refresh classification fixed broker-side (skeptic refutation on PR #2103, found independently by two skeptics): the broker's /oauth/token catch-all had collapsed SKV's terminal dead-refresh-token dialects (404 id_not_found, 400 invalid_grant, "Refresh Token status is expired": the DOMINANT refresh outcome, per-flow tokens live 65 min) into the generic 502, so a connector instance could never classify ordinary session expiry: raw English 500s instead of the reconnect flow, staged filing ops consumed as non-recoverable, crons retrying raw forever. The broker now re-codes those dialects (refresh grant only, never the code exchange where invalid_grant means an expired one-shot code) as 401 CONNECTOR_SKV_REFRESH_DEAD, which the instance maps to SESSION_EXPIRED; the generic 502 remains raw so a transient SKV outage still never re-arms the reconnect banner (#1155). Same pass: the data proxy now forwards WWW-Authenticate + x-skv-*/x-amzn-*/x-api-* response headers (the instance's MISSING_SCOPE classification reads them; nothing secret rides in them), and the instance's gateway-refusal guidance is connector-aware (a self-host has no SKATTEVERKET_APIGW_CLIENT_ID or Utvecklarportalen access: point at /api/connector/status + support instead). +[2026-09-01] EB claim guard, skeptic round (PR #2116): active-company standing state (enabled cash_accounts + enabled accounts on its live-ish rows) outranks sibling claims COMPANY-wide, not row-wide: a bank-list renewal arrives on a fresh row and must not switch a working feed off. pending_selection rows neither claim nor remember deselections (unconfirmed callback output; also stops fail-closed writes from poisoning later connects). Guard-disabled accounts are never mirrored from the callback (mirroring enabled:false can promote the seeded primary 1930 manual row and disable it under a foreign identity) and the selection save skips allocation+mirror for disabled never-mirrored accounts, so the no-slot-burned invariant holds end to end. Deselection carry got a picker note; enabling an account clears the guard flags. Legacy both-companies-enabled overlaps stay untouched (Swedish review advisory: prod sweep is a follow-up, not this PR). +[2026-09-01] EB claim guard round 2 (skeptic re-verify): pending_selection rows are asymmetric, not excluded: their ENABLED accounts still claim (attach-created rows hold offered accounts with no cash rows until saved; excluding them reopened the attach-window double-booking), while their disabled flags stay out of deselection memory (unconfirmed callback output). Both fetchAllRows claim queries order('id'): unordered .range() pagination can silently skip rows at page boundaries, and a skipped row is a missed claim (fail-open). 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 44acd30a..97d5c936 100644 --- a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts +++ b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts @@ -9,13 +9,15 @@ vi.mock('@/extensions/general/enable-banking/lib/api-client', () => ({ })) // Use hoisted to safely create mock objects referenced in vi.mock factories -const { mockFrom, mockUpsertFromPsd2, mockAllocate, mockSupersede } = vi.hoisted(() => { - const mockFrom = vi.fn() - const mockUpsertFromPsd2 = vi.fn() - const mockAllocate = vi.fn() - const mockSupersede = vi.fn() - return { mockFrom, mockUpsertFromPsd2, mockAllocate, mockSupersede } -}) +const { mockFrom, mockUpsertFromPsd2, mockAllocate, mockSupersede, mockCrossCompanyContext } = + vi.hoisted(() => { + const mockFrom = vi.fn() + const mockUpsertFromPsd2 = vi.fn() + const mockAllocate = vi.fn() + const mockSupersede = vi.fn() + const mockCrossCompanyContext = vi.fn() + return { mockFrom, mockUpsertFromPsd2, mockAllocate, mockSupersede, mockCrossCompanyContext } + }) // The supersede pass has its own unit tests (extensions/general/enable-banking/ // __tests__/supersede.test.ts); here it is mocked so these tests assert the @@ -24,6 +26,19 @@ vi.mock('@/extensions/general/enable-banking/lib/supersede', () => ({ supersedeSiblingConnections: (...args: unknown[]) => mockSupersede(...args), })) +// The cross-company claim lookup has its own unit tests (extensions/general/ +// enable-banking/lib/__tests__/session-sharing.test.ts); mocked here so the +// per-test mockFrom scripts don't have to answer its queries too. Everything +// else in session-sharing stays real. +vi.mock('@/extensions/general/enable-banking/lib/session-sharing', async (importOriginal) => { + const actual = + await importOriginal() + return { + ...actual, + fetchCrossCompanyAccountContext: (...args: unknown[]) => mockCrossCompanyContext(...args), + } +}) + vi.mock('@/lib/supabase/server', () => ({ createServiceClient: vi.fn().mockResolvedValue({ from: mockFrom, @@ -91,6 +106,12 @@ describe('GET /api/extensions/enable-banking/callback', () => { vi.clearAllMocks() mockUpsertFromPsd2.mockResolvedValue(undefined) mockSupersede.mockResolvedValue({ supersededIds: [], dedupScopeByIban: new Map() }) + // No sibling company claims anything by default; individual tests override. + mockCrossCompanyContext.mockResolvedValue({ + claims: new Map(), + deselectedIbans: new Set(), + activeCompanyIbans: new Set(), + }) // Allocator stand-in mirroring the real behavior: currency default first, // then the next free 1931–1959 slot (skipping other currency defaults). mockAllocate.mockImplementation( @@ -271,6 +292,195 @@ describe('GET /api/extensions/enable-banking/callback', () => { expect(mirrorLedgers).toEqual(['1930', '1931']) }) + // The F1 scenario: at a one-session bank the PSU's single consent covers + // accounts another of the user's companies books (and, before this guard, + // stored them pre-enabled in the wrong company and mirrored them into its + // cash_accounts, one save away from cross-company bookkeeping). + function mockConnectionFlow(pendingRow: Record) { + const capturedUpdates: Record[] = [] + let callIndex = 0 + mockFrom.mockImplementation(() => { + callIndex++ + if (callIndex === 1) { + return mockChain({ data: pendingRow, 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.single = vi.fn().mockResolvedValue({ + data: { id: 'conn-1', bank_name: 'SEB', company_id: 'company-1', user_id: 'user-1' }, + error: null, + }) + chain.then = (resolve: (v: unknown) => void) => resolve({ data: null, error: null }) + return chain + }) + return capturedUpdates + } + + it('stores accounts claimed by a sibling company disabled + flagged and never mirrors them', async () => { + const capturedUpdates = mockConnectionFlow({ + id: 'conn-1', user_id: 'user-1', company_id: 'company-1', bank_name: 'SEB', status: 'pending', + }) + mockCrossCompanyContext.mockResolvedValue({ + claims: new Map([ + ['SE9999', { companyId: 'company-2', companyName: 'Other Energy AB' }], + ]), + deselectedIbans: new Set(), + activeCompanyIbans: new Set(), + }) + mockCreateSession.mockResolvedValue({ + session_id: 'sess-1', + accounts: [ + { uid: 'acc-own', account_id: { iban: 'SE1234' }, name: 'Företagskonto', currency: 'SEK' }, + { uid: 'acc-foreign', account_id: { iban: 'SE9999' }, name: 'Annat bolags konto', currency: 'SEK' }, + ], + access: { valid_until: '2027-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) + // Drain the stream: the finalize work completes behind the interim page. + await response.text() + const accountsData = capturedUpdates[0].accounts_data as Array<{ + uid: string + enabled: boolean + claimed_by_company_id?: string + claimed_by_company_name?: string + }> + const own = accountsData.find(a => a.uid === 'acc-own') + const foreign = accountsData.find(a => a.uid === 'acc-foreign') + expect(own?.enabled).toBe(true) + expect(own?.claimed_by_company_id).toBeUndefined() + expect(foreign?.enabled).toBe(false) + expect(foreign?.claimed_by_company_id).toBe('company-2') + expect(foreign?.claimed_by_company_name).toBe('Other Energy AB') + + // The claimed account gets NO cash_accounts row and NO 19xx slot in this + // company's chart: only the own account is mirrored. + expect(mockUpsertFromPsd2).toHaveBeenCalledTimes(1) + expect((mockUpsertFromPsd2.mock.calls[0][2] as { external_uid: string }).external_uid).toBe('acc-own') + expect(mockAllocate).toHaveBeenCalledTimes(1) + }) + + it('keeps an account this row already carried enabled even when a sibling claims its IBAN', async () => { + // A standing feed in the active company outranks a sibling's claim: a + // renewal must never switch a working account off. (Legacy double-claims + // from the pre-session-sharing era make this overlap real.) + const capturedUpdates = mockConnectionFlow({ + id: 'conn-1', user_id: 'user-1', company_id: 'company-1', bank_name: 'SEB', status: 'expired', + accounts_data: [ + { uid: 'acc-old', iban: 'SE1234', name: 'Företagskonto', currency: 'SEK', enabled: true }, + ], + }) + mockCrossCompanyContext.mockResolvedValue({ + claims: new Map([['SE1234', { companyId: 'company-2', companyName: 'Other AB' }]]), + deselectedIbans: new Set(), + activeCompanyIbans: new Set(), + }) + mockCreateSession.mockResolvedValue({ + session_id: 'sess-2', + accounts: [ + { uid: 'acc-new', account_id: { iban: 'SE1234' }, name: 'Företagskonto', currency: 'SEK' }, + ], + access: { valid_until: '2027-12-31T00:00:00Z' }, + aspsp: { name: 'SEB', country: 'SE' }, + }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + // Drain the stream: the finalize work completes behind the interim page. + await response.text() + + const accountsData = capturedUpdates[0].accounts_data as Array<{ + uid: string + enabled: boolean + claimed_by_company_id?: string + }> + expect(accountsData[0].enabled).toBe(true) + expect(accountsData[0].claimed_by_company_id).toBeUndefined() + expect(mockUpsertFromPsd2).toHaveBeenCalledTimes(1) + }) + + it('carries a deselection made on another connection row onto a fresh connect', async () => { + // C2: "Synkas ej" chosen under one company must not come back pre-checked + // when a new connection row (any company) sees the same IBAN. + const capturedUpdates = mockConnectionFlow({ + id: 'conn-1', user_id: 'user-1', company_id: 'company-1', bank_name: 'SEB', status: 'pending', + }) + mockCrossCompanyContext.mockResolvedValue({ + claims: new Map(), + deselectedIbans: new Set(['SE5555']), + activeCompanyIbans: new Set(), + }) + mockCreateSession.mockResolvedValue({ + session_id: 'sess-1', + accounts: [ + { uid: 'acc-card', account_id: { iban: 'SE5555' }, name: 'Privat kreditkort', currency: 'SEK' }, + ], + access: { valid_until: '2027-12-31T00:00:00Z' }, + aspsp: { name: 'SEB', country: 'SE' }, + }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + // Drain the stream: the finalize work completes behind the interim page. + await response.text() + + const accountsData = capturedUpdates[0].accounts_data as Array<{ + uid: string + enabled: boolean + claimed_by_company_id?: string + deselected_elsewhere?: boolean + }> + expect(accountsData[0].enabled).toBe(false) + // Not a claim (no company books it), but flagged so the picker can say + // WHY the box is unchecked instead of leaving a silent gap. + expect(accountsData[0].claimed_by_company_id).toBeUndefined() + expect(accountsData[0].deselected_elsewhere).toBe(true) + // Guard-disabled accounts are never mirrored from the callback: writing + // enabled:false for a new-to-row account can promote an existing manual + // holder (the seeded primary 1930) and flip it to disabled. + expect(mockUpsertFromPsd2).not.toHaveBeenCalled() + }) + + it('fails closed when the claim lookup errors: new accounts stored deselected, unflagged', async () => { + const capturedUpdates = mockConnectionFlow({ + id: 'conn-1', user_id: 'user-1', company_id: 'company-1', bank_name: 'SEB', status: 'pending', + }) + mockCrossCompanyContext.mockResolvedValue(null) + mockCreateSession.mockResolvedValue({ + session_id: 'sess-1', + accounts: [ + { uid: 'acc-1', account_id: { iban: 'SE1234' }, name: 'Företagskonto', currency: 'SEK' }, + ], + access: { valid_until: '2027-12-31T00:00:00Z' }, + aspsp: { name: 'SEB', country: 'SE' }, + }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + + // The connect itself still succeeds: fail-closed costs a checkbox, not + // the connection. + expect(response.status).toBe(200) + // Drain the stream: the finalize work completes behind the interim page. + await response.text() + const accountsData = capturedUpdates[0].accounts_data as Array<{ + uid: string + enabled: boolean + claimed_by_company_id?: string + }> + expect(accountsData[0].enabled).toBe(false) + expect(accountsData[0].claimed_by_company_id).toBeUndefined() + // Fail-closed accounts are not mirrored either: enabled:false for a + // new-to-row account can promote and disable an existing manual holder. + // The selection save mirrors whatever the user enables. + expect(mockUpsertFromPsd2).not.toHaveBeenCalled() + }) + it('preserves existing mirrored ledgers on reconnect instead of re-deriving them', async () => { let callIndex = 0 mockFrom.mockImplementation((table: string) => { diff --git a/app/api/extensions/enable-banking/callback/route.ts b/app/api/extensions/enable-banking/callback/route.ts index b9f167bf..0e69ff17 100644 --- a/app/api/extensions/enable-banking/callback/route.ts +++ b/app/api/extensions/enable-banking/callback/route.ts @@ -12,7 +12,10 @@ import { defaultLedgerForCurrency, normalizeIban, } from '@/lib/cash-accounts/service' -import { fanOutSessionRenewal } from '@/extensions/general/enable-banking/lib/session-sharing' +import { + fanOutSessionRenewal, + fetchCrossCompanyAccountContext, +} 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' @@ -520,6 +523,84 @@ async function finalizeConnection( } } + // Cross-company guard: at one-session banks (SEB) the PSU's single consent + // can cover accounts another of the user's companies already books. The + // deliberate reuse path (findReusableSessions) never offers a claimed IBAN, + // but this callback used to trust the session wholesale: a connect performed + // under company B stored company A's accounts pre-enabled and mirrored them + // into B's cash_accounts, one "Spara val" away from booking A's transactions + // in B's ledger. Accounts claimed elsewhere are stored disabled + flagged + // (the picker names the claiming company) and skipped by the mirror below. + // Accounts this row itself carried before keep their own state: the active + // company's standing choice outranks a sibling's claim, so a renewal can + // never switch a working feed off. + const crossCompany = await fetchCrossCompanyAccountContext( + supabase, + userId, + pendingConnection.company_id, + pendingConnection.id, + ) + // Every account the guard itself disabled, whatever the branch. These are + // excluded from the cash_accounts mirror below: mirroring enabled:false for + // a new-to-row account can PROMOTE an existing manual holder (the seeded + // primary 1930 included) and flip it to disabled with a foreign identity, + // and a claimed account's row would double-claim the IBAN besides. The + // selection save allocates + mirrors any of them the user turns on. + const guardDisabledUids = new Set() + let claimedCount = 0 + for (const account of accountsMetadata) { + const normalizedIban = normalizeIban(account.iban) + // Row-local memory only. The active company's standing state on OTHER + // rows (a bank-list renewal arrives on a fresh row while the old row is + // waiting to be superseded) is already folded into crossCompany: + // activeCompanyIbans outrank claims and deselections there, so such + // accounts fall through to the enabled default below. + const seenOnThisRow = + priorEnabledByUid.has(account.uid) || + (normalizedIban ? priorEnabledByIban.has(normalizedIban) : false) || + pairedPriorUidByNewUid.has(account.uid) + if (seenOnThisRow) continue + + if (crossCompany === null) { + // Fail closed: without the claim set a free account cannot be told from + // one another company books, and pre-checking a claimed account is the + // one outcome this guard must never produce. The user just re-ticks. + account.enabled = false + guardDisabledUids.add(account.uid) + continue + } + const claim = normalizedIban ? crossCompany.claims.get(normalizedIban) : undefined + if (claim) { + account.enabled = false + account.claimed_by_company_id = claim.companyId + if (claim.companyName) account.claimed_by_company_name = claim.companyName + guardDisabledUids.add(account.uid) + claimedCount += 1 + continue + } + if (normalizedIban && crossCompany.deselectedIbans.has(normalizedIban)) { + // The user already said "Synkas ej" to this account on another + // connection row: a fresh row must not resurrect it pre-checked. The + // flag makes the picker say so; an unexplained unchecked box reads as + // a glitch and a silent one hides a sync gap. + account.enabled = false + account.deselected_elsewhere = true + guardDisabledUids.add(account.uid) + } + } + if (crossCompany === null) { + log.error('cross-company claim lookup failed: storing new accounts deselected', { + connectionId: pendingConnection.id, + }) + } else if (claimedCount > 0) { + log.warn('session covers accounts claimed by sibling companies', { + connectionId: pendingConnection.id, + companyId: pendingConnection.company_id, + claimedCount, + accountCount: accountsMetadata.length, + }) + } + // Stay in 'pending_selection' until the user confirms which accounts to sync. // The cron and manual sync routes both skip this status, so no transactions // can be pulled before the user has had a chance to deselect accounts. @@ -657,6 +738,14 @@ async function finalizeConnection( let accountsDataDirty = carriedScopeDirty for (const account of accountsMetadata) { + // Nothing the guard disabled is mirrored here: a claimed account's row + // would be another company's data in this routing table (and an enabled + // one would double-claim the IBAN), and mirroring enabled:false for any + // new-to-row account can promote an existing manual holder — the seeded + // primary 1930 included — flipping it to disabled under a foreign + // identity. No 19xx slot is burned either. The selection save allocates + // and mirrors whichever of them the user deliberately turns on. + if (guardDisabledUids.has(account.uid)) continue let targetLedger = mirroredByUid.get(account.uid)?.ledger_account let reuseCashAccountId: string | null = null if (!targetLedger) { diff --git a/extensions/general/enable-banking/components/AccountPickerDialog.tsx b/extensions/general/enable-banking/components/AccountPickerDialog.tsx index 05f17ede..bc0fce55 100644 --- a/extensions/general/enable-banking/components/AccountPickerDialog.tsx +++ b/extensions/general/enable-banking/components/AccountPickerDialog.tsx @@ -899,6 +899,27 @@ export function AccountPickerDialog({ {account.iban.replace(/(.{4})/g, '$1 ').trim()}

)} + {/* The callback found this IBAN already booked by another + of the user's companies (one consent can cover several + companies' accounts at e.g. SEB). Unchecked by default; + naming the claimant is what stops a reflexive + select-all from booking it here too. */} + {account.claimed_by_company_id && ( +

+ Synkas redan i{' '} + + {account.claimed_by_company_name || 'ett annat bolag'} + +

+ )} + {/* Carried deselection: the user said "Synkas ej" to this + IBAN on another connection. An unexplained unchecked + box reads as a glitch; a silent one hides a sync gap. */} + {!account.claimed_by_company_id && account.deselected_elsewhere && ( +

+ Tidigare bortvald: markera för att synka i detta bolag +

+ )} {account.balance !== undefined && (

diff --git a/extensions/general/enable-banking/index.ts b/extensions/general/enable-banking/index.ts index d161d0bd..6e4cad6c 100644 --- a/extensions/general/enable-banking/index.ts +++ b/extensions/general/enable-banking/index.ts @@ -1134,7 +1134,7 @@ export const enableBankingExtension: Extension = { const mappingsByUid = new Map(mappings.map(m => [m.uid, m])) const updatedAccounts: StoredAccount[] = existing.map(a => { const mapping = mappingsByUid.get(a.uid) - return { + const next: StoredAccount = { ...a, enabled: enabledSet.has(a.uid), // Apply ledger_account from mapping when present. Explicit null clears it. @@ -1144,8 +1144,29 @@ export const enableBankingExtension: Extension = { ? { ledger_account: mapping.ledger_account ?? undefined } : {}), } + // Enabling an account is the deliberate takeover the callback's + // guard flags exist to force: once made, the flags are stale (the + // account syncs HERE now) and would keep rendering a false + // "synkas i annat bolag" note in every later picker. + if (next.enabled) { + delete next.claimed_by_company_id + delete next.claimed_by_company_name + delete next.deselected_elsewhere + } + return next }) + // Accounts the callback guard left disabled AND unmirrored (no ledger + // anywhere) stay that way through a save that does not enable them: + // allocating a 19xx slot and upserting a cash_accounts row for a + // still-disabled claimed account would recreate exactly the state the + // guard exists to prevent (another company's IBAN and name in this + // company's chart and routing table), one screen after the callback + // avoided it. Disabled accounts that already have a ledger or a + // mirrored row keep the existing behavior: their row's enabled flag + // must still flip off. + const neverMirroredDisabledUids = new Set() + // Resolve the effective mirror ledger for every account up front and // reject collisions with a 400 — the mirror pass below writes into // cash_accounts, whose UNIQUE (company_id, ledger_account) constraint @@ -1268,6 +1289,13 @@ export const enableBankingExtension: Extension = { // mirror pass surface any collision per-account, as before. for (const a of updatedAccounts) { if (effectiveLedgerByUid.has(a.uid)) continue + // See neverMirroredDisabledUids above: a disabled account that has + // never held a ledger or a mirrored row gets neither allocated nor + // mirrored by this save. + if (!enabledSet.has(a.uid) && !reuseRowByUid.has(a.uid)) { + neverMirroredDisabledUids.add(a.uid) + continue + } let allocated: string | null = null try { const resolved = await resolvePsd2LedgerAccount(supabase, companyId, user.id, { @@ -1290,8 +1318,10 @@ export const enableBankingExtension: Extension = { } // accounts_data mirrors the resolved assignment so the picker - // pre-fills reality on the next open. + // pre-fills reality on the next open. Skipped disabled accounts keep + // no assignment: their slot is only claimed if they are ever enabled. for (const a of updatedAccounts) { + if (neverMirroredDisabledUids.has(a.uid)) continue a.ledger_account = effectiveLedgerByUid.get(a.uid) } @@ -1326,6 +1356,9 @@ export const enableBankingExtension: Extension = { // without reading the JSONB column. { for (const a of updatedAccounts) { + // Never-mirrored disabled accounts (callback-guard leftovers the + // user did not enable) get no cash_accounts row: see above. + if (neverMirroredDisabledUids.has(a.uid)) continue const ledgerAccount = a.ledger_account ?? '1930' // Only reuse the IBAN-matched row when it already sits on the // ledger we are about to write. If the user deliberately remapped diff --git a/extensions/general/enable-banking/lib/__tests__/session-sharing.test.ts b/extensions/general/enable-banking/lib/__tests__/session-sharing.test.ts new file mode 100644 index 00000000..2d8dd084 --- /dev/null +++ b/extensions/general/enable-banking/lib/__tests__/session-sharing.test.ts @@ -0,0 +1,306 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +vi.mock('@/lib/cash-accounts/service', () => ({ + normalizeIban: (iban?: string | null) => { + if (!iban) return null + const normalized = iban.replace(/\s+/g, '').toUpperCase() + return normalized || null + }, +})) + +import type { SupabaseClient } from '@supabase/supabase-js' +import { + fetchCrossCompanyAccountContext, + unclaimedAccountsFor, +} from '../session-sharing' +import type { StoredAccount } from '../../types' + +interface TableResult { + data?: unknown + error?: { message: string } | null +} + +type MockChain = Record> & { + then: (resolve: (v: unknown) => void) => void +} + +/** + * Chainable mock resolving per table: every filter method returns the chain, + * awaiting it yields the preset result for that table. Chains are recorded so + * tests can assert which filters were applied (the mock does not filter). + */ +function makeSupabase(results: Record): { + supabase: SupabaseClient + chainsByTable: Map +} { + const chainsByTable = new Map() + const supabase = { + from: (table: string) => { + const result = results[table] ?? { data: [], error: null } + const chain = {} as MockChain + for (const m of ['select', 'eq', 'neq', 'in', 'not', 'is', 'order', 'limit', 'range']) { + chain[m] = vi.fn().mockReturnValue(chain) + } + chain.then = (resolve: (v: unknown) => void) => + resolve({ data: result.data ?? null, error: result.error ?? null }) + const bucket = chainsByTable.get(table) + if (bucket) bucket.push(chain) + else chainsByTable.set(table, [chain]) + return chain + }, + } as unknown as SupabaseClient + return { supabase, chainsByTable } +} + +describe('fetchCrossCompanyAccountContext', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('claims enabled accounts of sibling companies, remembers deselections, resolves names', async () => { + const { supabase } = makeSupabase({ + company_members: { + data: [{ company_id: 'company-1' }, { company_id: 'company-2' }], + }, + bank_connections: { + data: [ + { + id: 'conn-sibling', + company_id: 'company-2', + accounts_data: [ + { uid: 'a1', iban: 'SE11', currency: 'SEK', enabled: true }, + { uid: 'a2', iban: 'SE22', currency: 'SEK', enabled: false }, + ], + }, + { + id: 'conn-own-other-row', + company_id: 'company-1', + // The active company's own account never becomes a claim; its + // enabled accounts are its standing set and its deselections are + // remembered. + accounts_data: [ + { uid: 'a3', iban: 'SE33', currency: 'SEK', enabled: true }, + { uid: 'a4', iban: 'SE44', currency: 'SEK', enabled: false }, + ], + }, + ], + }, + cash_accounts: { data: [] }, + companies: { data: [{ id: 'company-2', name: 'Sibling AB' }] }, + }) + + const context = await fetchCrossCompanyAccountContext( + supabase, + 'user-1', + 'company-1', + 'conn-active', + ) + + expect(context).not.toBeNull() + expect(context!.claims.get('SE11')).toEqual({ + companyId: 'company-2', + companyName: 'Sibling AB', + }) + expect(context!.claims.has('SE33')).toBe(false) + expect(context!.activeCompanyIbans).toEqual(new Set(['SE33'])) + expect(context!.deselectedIbans).toEqual(new Set(['SE22', 'SE44'])) + }) + + it("lets the active company's own standing state outrank a sibling claim", async () => { + // The bank-list renewal case: the active company's old row (about to be + // superseded) and its cash_accounts row still book the IBAN. A sibling + // claim on the same IBAN must not win, or a renewal arriving on a fresh + // row would switch a working feed off. + const { supabase } = makeSupabase({ + company_members: { + data: [{ company_id: 'company-1' }, { company_id: 'company-2' }], + }, + bank_connections: { + data: [ + { + id: 'conn-sibling', + company_id: 'company-2', + accounts_data: [{ uid: 'a1', iban: 'SE11', currency: 'SEK', enabled: true }], + }, + ], + }, + cash_accounts: { + data: [{ company_id: 'company-1', iban: 'SE11' }], + }, + companies: { data: [] }, + }) + + const context = await fetchCrossCompanyAccountContext( + supabase, + 'user-1', + 'company-1', + 'conn-active', + ) + + expect(context!.claims.has('SE11')).toBe(false) + expect(context!.activeCompanyIbans.has('SE11')).toBe(true) + expect(context!.deselectedIbans.has('SE11')).toBe(false) + }) + + it('claims IBANs held by sibling companies via cash_accounts too', async () => { + const { supabase } = makeSupabase({ + company_members: { + data: [{ company_id: 'company-1' }, { company_id: 'company-2' }], + }, + bank_connections: { data: [] }, + cash_accounts: { data: [{ company_id: 'company-2', iban: 'SE55' }] }, + companies: { data: [] }, + }) + + const context = await fetchCrossCompanyAccountContext( + supabase, + 'user-1', + 'company-1', + 'conn-active', + ) + + expect(context!.claims.get('SE55')).toEqual({ companyId: 'company-2', companyName: null }) + }) + + it('lets a claim outrank a remembered deselection for the same IBAN', async () => { + const { supabase } = makeSupabase({ + company_members: { + data: [{ company_id: 'company-1' }, { company_id: 'company-2' }], + }, + bank_connections: { + data: [ + { + id: 'conn-a', + company_id: 'company-2', + accounts_data: [{ uid: 'a1', iban: 'SE11', currency: 'SEK', enabled: true }], + }, + { + id: 'conn-b', + company_id: 'company-1', + accounts_data: [{ uid: 'a2', iban: 'SE11', currency: 'SEK', enabled: false }], + }, + ], + }, + cash_accounts: { data: [] }, + companies: { data: [] }, + }) + + const context = await fetchCrossCompanyAccountContext( + supabase, + 'user-1', + 'company-1', + 'conn-active', + ) + + expect(context!.claims.has('SE11')).toBe(true) + expect(context!.deselectedIbans.has('SE11')).toBe(false) + }) + + it('treats pending_selection rows asymmetrically: enabled accounts claim, disabled flags are ignored', async () => { + // An attach-created row is pending_selection with deliberately-offered + // enabled accounts and NO cash rows until its picker is saved: those must + // claim, or a second company can take the same physical account inside + // that window. Its disabled flags are unconfirmed callback output + // (including the guard's own fail-closed writes) and must NOT feed the + // deselection memory. + const { supabase, chainsByTable } = makeSupabase({ + company_members: { + data: [{ company_id: 'company-1' }, { company_id: 'company-2' }], + }, + bank_connections: { + data: [ + { + id: 'conn-attached', + company_id: 'company-2', + status: 'pending_selection', + accounts_data: [ + { uid: 'a1', iban: 'SE11', currency: 'SEK', enabled: true }, + { uid: 'a2', iban: 'SE22', currency: 'SEK', enabled: false }, + ], + }, + ], + }, + cash_accounts: { data: [] }, + companies: { data: [] }, + }) + + const context = await fetchCrossCompanyAccountContext( + supabase, + 'user-1', + 'company-1', + 'conn-active', + ) + + expect(context!.claims.has('SE11')).toBe(true) + expect(context!.deselectedIbans.has('SE22')).toBe(false) + + const connectionChain = chainsByTable.get('bank_connections')![0] + expect(connectionChain.in).toHaveBeenCalledWith('status', [ + 'active', + 'pending_selection', + 'expired', + 'error', + ]) + // Paged reads must order on a unique column or rows can be silently + // skipped at page boundaries (a skipped row is a missed claim). + expect(connectionChain.order).toHaveBeenCalledWith('id') + expect(chainsByTable.get('cash_accounts')![0].order).toHaveBeenCalledWith('id') + }) + + it('reads cash_accounts for ALL member companies (active included)', async () => { + const { supabase, chainsByTable } = makeSupabase({ + company_members: { + data: [{ company_id: 'company-1' }, { company_id: 'company-2' }], + }, + bank_connections: { data: [] }, + cash_accounts: { data: [] }, + companies: { data: [] }, + }) + + await fetchCrossCompanyAccountContext(supabase, 'user-1', 'company-1', 'conn-active') + + const cashChain = chainsByTable.get('cash_accounts')![0] + expect(cashChain.in).toHaveBeenCalledWith('company_id', ['company-1', 'company-2']) + }) + + it('returns null when a lookup fails, so the caller can fail closed', async () => { + const { supabase } = makeSupabase({ + bank_connections: { data: null, error: { message: 'boom' } }, + }) + + const context = await fetchCrossCompanyAccountContext( + supabase, + 'user-1', + 'company-1', + 'conn-active', + ) + + expect(context).toBeNull() + }) +}) + +describe('unclaimedAccountsFor', () => { + it('strips stale claimed_by and deselected flags from offered accounts', () => { + const accounts: StoredAccount[] = [ + { + uid: 'a1', + iban: 'SE11', + currency: 'SEK', + enabled: false, + ledger_account: '1938', + claimed_by_company_id: 'company-9', + claimed_by_company_name: 'Stale AB', + deselected_elsewhere: true, + }, + ] + + const offered = unclaimedAccountsFor(accounts, new Set()) + + expect(offered).toHaveLength(1) + expect(offered[0].enabled).toBe(true) + expect(offered[0].ledger_account).toBeUndefined() + expect(offered[0].claimed_by_company_id).toBeUndefined() + expect(offered[0].claimed_by_company_name).toBeUndefined() + expect(offered[0].deselected_elsewhere).toBeUndefined() + }) +}) diff --git a/extensions/general/enable-banking/lib/session-sharing.ts b/extensions/general/enable-banking/lib/session-sharing.ts index 09c5cf89..146ba246 100644 --- a/extensions/general/enable-banking/lib/session-sharing.ts +++ b/extensions/general/enable-banking/lib/session-sharing.ts @@ -1,5 +1,6 @@ import type { SupabaseClient } from '@supabase/supabase-js' import { normalizeIban } from '@/lib/cash-accounts/service' +import { fetchAllRows } from '@/lib/supabase/fetch-all' import { createLogger } from '@/lib/logger' import type { StoredAccount } from '../types' @@ -72,8 +73,17 @@ export function unclaimedAccountsFor( seen.add(iban) // The source company's enable/disable choice is its own; company B starts // with everything on and unchecks in the picker. Drop the source's ledger - // mapping too: that number belongs to the other company's chart. - const { ledger_account: _ledger, ...rest } = account + // mapping too: that number belongs to the other company's chart. The + // claimed_by_* flags are likewise the source row's history: everything + // offered here is unclaimed by definition, so a stale flag must not + // render a "synkas i annat bolag" note on a genuinely free account. + const { + ledger_account: _ledger, + claimed_by_company_id: _claimedId, + claimed_by_company_name: _claimedName, + deselected_elsewhere: _deselected, + ...rest + } = account out.push({ ...rest, enabled: true }) } return out @@ -271,6 +281,208 @@ async function fetchCompanyNames( ) } +/** The company whose books an IBAN is already synced into. */ +export interface ForeignIbanClaim { + companyId: string + companyName: string | null +} + +export interface CrossCompanyAccountContext { + /** + * Normalized IBAN → the OTHER company of this user that already books it + * (enabled cash_accounts row, or an enabled account on a live-ish + * connection). First claimant wins; which sibling is named is cosmetic. + * Never contains an IBAN the ACTIVE company itself already books: the + * active company's standing state outranks a sibling's claim, whatever row + * the callback happens to be finalizing (a bank-list renewal arrives on a + * FRESH row with no priors, and must not switch a working feed off). + */ + claims: Map + /** + * Normalized IBANs the user has deselected ("Synkas ej") on other + * connection rows, in any company, so a fresh connection row does not + * resurrect an account the user already opted out of (the recurring + * came-back-pre-checked complaint). Only rows whose selection the user has + * actually SAVED contribute ('active'/'expired'/'error'): + * a 'pending_selection' row's flags are unconfirmed callback output, + * including this guard's own fail-closed writes, and treating them as user + * intent would make one abandoned picker (or one transient lookup error) + * poison every later connect. Cleared of anything the active company + * books or a sibling claims (both outrank a remembered deselection). + */ + deselectedIbans: Set + /** + * Normalized IBANs the ACTIVE company already books: enabled cash_accounts + * rows, plus enabled accounts on its own live-ish connection rows. Exposed + * for the callers/tests; claims and deselectedIbans are already cleaned + * against it. + */ + activeCompanyIbans: Set +} + +/** + * What the user's OTHER companies already do with each IBAN, for the OAuth + * callback's cross-company guard. At one-session banks (SEB) the PSU's single + * consent can cover accounts that belong in a sibling company's books, and the + * callback stores whatever the session returns: without this lookup those + * accounts land pre-enabled in the wrong company. + * + * Runs on the callback's SERVICE-ROLE client, so nothing is RLS-scoped: + * every query below filters on the user's own rows explicitly + * (bank_connections.user_id, cash_accounts.company_id via company_members). + * + * Returns null when any lookup failed. The caller must fail closed (treat + * every unrecognized account as not-safe-to-pre-check): an empty result is + * indistinguishable from "nothing is claimed", which is the one answer this + * guard must never invent. + */ +export async function fetchCrossCompanyAccountContext( + serviceSupabase: SupabaseClient, + userId: string, + activeCompanyId: string, + excludeConnectionId: string, +): Promise { + const claims = new Map() + const deselectedIbans = new Set() + const activeCompanyIbans = new Set() + + // Statuses whose accounts count: a revoked row's accounts are released + // territory; 'expired'/'error' still count (a sibling whose feed + // momentarily died at a one-session bank has NOT given its accounts up). + // 'pending_selection' rows count ASYMMETRICALLY: their enabled accounts + // still claim, because an attach-created row holds deliberately-offered + // accounts with no cash_accounts rows until its picker is saved, and + // ignoring that window lets a second company take the same physical + // account (the exact failure fetchIbanCarriers documents). Their DISABLED + // flags are unconfirmed callback output though — including this guard's + // own fail-closed writes — so they never feed the deselection memory: + // one abandoned picker or transient lookup error must not poison every + // later connect. + let connectionRows: Array<{ + id: string + company_id: string + status: string + accounts_data: StoredAccount[] | null + }> + try { + connectionRows = await fetchAllRows(range => + serviceSupabase + .from('bank_connections') + .select('id, company_id, status, accounts_data') + .eq('user_id', userId) + .neq('id', excludeConnectionId) + .in('status', ['active', 'pending_selection', 'expired', 'error']) + // Unique-column order: fetchAllRows pages with .range(), and an + // unordered paged read can silently SKIP rows at page boundaries — + // a skipped row here is a missed claim, which fails open. + .order('id') + .range(range.from, range.to), + ) + } catch (connectionError) { + log.error('cross-company claim lookup failed (bank_connections)', { + activeCompanyId, + error: connectionError instanceof Error ? connectionError.message : String(connectionError), + }) + return null + } + + for (const row of connectionRows) { + for (const account of row.accounts_data ?? []) { + const iban = normalizeIban(account.iban) + if (!iban) continue + if (account.enabled === false) { + if (row.status !== 'pending_selection') deselectedIbans.add(iban) + } else if (row.company_id === activeCompanyId) { + activeCompanyIbans.add(iban) + } else if (!claims.has(iban)) { + claims.set(iban, { companyId: row.company_id, companyName: null }) + } + } + } + + // cash_accounts catches standing state the connection rows no longer carry + // (older connects, CSV-era rows promoted onto a feed, a row a supersede is + // about to demote). ALL of the user's companies are read: sibling rows + // become claims, the active company's rows become its own standing set. + // Scoped via memberships, since the service client sees everything. + const { data: membershipRows, error: membershipError } = await serviceSupabase + .from('company_members') + .select('company_id') + .eq('user_id', userId) + + if (membershipError) { + log.error('cross-company claim lookup failed (company_members)', { + activeCompanyId, + error: membershipError.message, + }) + return null + } + + const memberCompanyIds = [ + ...new Set( + ((membershipRows ?? []) as Array<{ company_id: string }>).map(r => r.company_id), + ), + ] + + if (memberCompanyIds.length > 0) { + let cashRows: Array<{ company_id: string; iban: string | null }> + try { + // fetchAllRows, not a bare select: PostgREST silently caps at 1000 + // rows, and a silently truncated claim set fails OPEN for exactly the + // multi-company consultants this guard exists for. + cashRows = await fetchAllRows(range => + serviceSupabase + .from('cash_accounts') + .select('company_id, iban') + .in('company_id', memberCompanyIds) + .eq('enabled', true) + .not('iban', 'is', null) + // Unique-column order: see the bank_connections page above. + .order('id') + .range(range.from, range.to), + ) + } catch (cashError) { + log.error('cross-company claim lookup failed (cash_accounts)', { + activeCompanyId, + error: cashError instanceof Error ? cashError.message : String(cashError), + }) + return null + } + + for (const row of cashRows) { + const iban = normalizeIban(row.iban) + if (!iban) continue + if (row.company_id === activeCompanyId) { + activeCompanyIbans.add(iban) + } else if (!claims.has(iban)) { + claims.set(iban, { companyId: row.company_id, companyName: null }) + } + } + } + + // Precedence, strongest first: the active company's own standing state + // (a renewal must never switch a working feed off, whichever row it + // arrives on), then a sibling's claim, then a remembered deselection (the + // picker note "synkas i X" is strictly more information than a bare + // unchecked box). + for (const iban of activeCompanyIbans) { + claims.delete(iban) + deselectedIbans.delete(iban) + } + for (const iban of claims.keys()) deselectedIbans.delete(iban) + + if (claims.size > 0) { + const names = await fetchCompanyNames(serviceSupabase, [ + ...new Set([...claims.values()].map(c => c.companyId)), + ]) + for (const claim of claims.values()) { + claim.companyName = names.get(claim.companyId) ?? null + } + } + + return { claims, deselectedIbans, activeCompanyIbans } +} + /** * How many OTHER connections still depend on this session. * diff --git a/extensions/general/enable-banking/types.ts b/extensions/general/enable-banking/types.ts index 1d742a4d..7c7a4620 100644 --- a/extensions/general/enable-banking/types.ts +++ b/extensions/general/enable-banking/types.ts @@ -23,6 +23,20 @@ export interface StoredAccount { // the whole history. lib/sync.ts falls back to IBAN-then-uid when unset // (rows that predate this field) and stamps it on the next sync. dedup_scope?: string + // Set by the OAuth callback when the account's IBAN is already booked by + // ANOTHER of the user's companies. At one-session banks (SEB) the PSU's + // single consent can cover accounts that belong in a sibling company's + // books; such accounts are stored disabled and never mirrored into this + // company's cash_accounts, and the picker renders the claim so the user + // sees why the account is unchecked. Enabling one is a deliberate act. + claimed_by_company_id?: string + claimed_by_company_name?: string + // Set by the OAuth callback when the account arrived deselected because the + // user chose "Synkas ej" for the same IBAN on another connection row (any + // company). Rendered as a note in the picker so the unchecked box is never + // silent; cleared by the selection save when the user re-enables the + // account. + deselected_elsewhere?: boolean } // Re-export API types from the client