Report failed bank connections and stop resurrecting deleted accounts

Three defects made new connections silently vanish while removed
accounts returned:

- A single shared account the journal cannot represent (securities or
  card entries without IBAN, stable identification or currency) aborted
  the entire consent. Usable accounts are now linked and the rest
  counted and reported.
- A consent that linked nothing was stored, redirected as success and
  later reaped by session recovery. It now fails with the reason.
- Callback failures rendered a bare JSON error page and were never
  logged. They now log and redirect into the app with the reason shown.
- Deleting an account left its session binding, so the next connect or
  sync recovered the binding and re-added the account. Account deletion
  now releases bindings, consents and cursors before committing.
This commit is contained in:
Lars Nolden
2026-09-11 11:46:59 +02:00
parent 3df9bda989
commit e77969b8c5
9 changed files with 242 additions and 34 deletions
+6 -6
View File
@@ -116,7 +116,7 @@ func TestBankingRuntimeRotationPreservesConsentAndRejectsPendingCallback(t *test
t.Fatal(err)
}
pending := bankingAuthorization(t, a, key, "app-one", bankingCallback)
if err := a.Callback(ctx, "code", pending); err != nil {
if _, err := a.Callback(ctx, "code", pending); err != nil {
t.Fatal(err)
}
before, err := a.Snapshot(ctx)
@@ -138,7 +138,7 @@ func TestBankingRuntimeRotationPreservesConsentAndRejectsPendingCallback(t *test
if !reflect.DeepEqual(before.Sessions, after.Sessions) || !reflect.DeepEqual(before.Data, after.Data) || a.ops.AccountSync[accountID] == "" {
t.Fatal("same-application rotation discarded consent, cursor or canonical data")
}
if err := a.Callback(ctx, "code", pending); err == nil {
if _, err := a.Callback(ctx, "code", pending); err == nil {
t.Fatal("rotation accepted a stale pending callback")
}
bankingAuthorization(t, a, rotated, "app-one", callback)
@@ -168,7 +168,7 @@ func TestBankingAppSwitchAndDisableNeverReuseOldSessions(t *testing.T) {
t.Fatal(err)
}
pending := bankingAuthorization(t, a, key, "app-one", bankingCallback)
if err := a.Callback(ctx, "code", pending); err != nil {
if _, err := a.Callback(ctx, "code", pending); err != nil {
t.Fatal(err)
}
before, _ := a.Snapshot(ctx)
@@ -195,7 +195,7 @@ func TestBankingAppSwitchAndDisableNeverReuseOldSessions(t *testing.T) {
if !remove {
bankingAuthorization(t, a, key, "app-two", bankingCallback)
}
if err := a.Callback(ctx, "code", pending); err == nil {
if _, err := a.Callback(ctx, "code", pending); err == nil {
t.Fatal("old pending authorization accepted after app change")
}
if _, err := a.Balances(ctx, accountID); err == nil {
@@ -286,7 +286,7 @@ func TestBankingRejectedSettingsAndFailedWritePreserveActiveProvider(t *testing.
if _, err := a.RemoveBankingSettings(ctx); err == nil {
t.Fatal("failed removal reported success")
}
if err := a.Callback(ctx, "code", pending); err != nil {
if _, err := a.Callback(ctx, "code", pending); err != nil {
t.Fatal("failed credential write invalidated active authorization", err)
}
bankingAuthorization(t, a, key, "active-app", bankingCallback)
@@ -367,7 +367,7 @@ func TestBankingEnvironmentAppChangeInvalidatesBoundSessions(t *testing.T) {
a = reopenBankingApp(t, a)
ctx := context.Background()
pending := bankingAuthorization(t, a, key, "env-one", bankingCallback)
if err := a.Callback(ctx, "code", pending); err != nil {
if _, err := a.Callback(ctx, "code", pending); err != nil {
t.Fatal(err)
}
before, _ := a.Snapshot(ctx)
+99 -2
View File
@@ -2,6 +2,8 @@ package app
import (
"context"
"slices"
"strings"
"testing"
"time"
@@ -95,7 +97,7 @@ func TestAuthorizedHistorySurvivesReopenAndRespectsIncrementalCursor(t *testing.
if _, err := a.Authorize(ctx, "N26", "DE", 24); err != nil {
t.Fatal(err)
}
if err := a.Callback(ctx, "one_time_code", b.authState); err != nil {
if _, err := a.Callback(ctx, "one_time_code", b.authState); err != nil {
t.Fatal(err)
}
a = reopenBankingApp(t, a)
@@ -170,7 +172,7 @@ func TestRenewedConsentWakesSchedulerAndAutomaticallyImports(t *testing.T) {
done := make(chan struct{})
go func() { defer close(done); a.RunScheduler(ctx) }()
defer func() { cancel(); <-done }()
if err := a.Callback(context.Background(), "one_time_code", "state"); err != nil {
if _, err := a.Callback(context.Background(), "one_time_code", "state"); err != nil {
t.Fatal(err)
}
select {
@@ -216,3 +218,98 @@ func TestInterruptedRenewalDiscardsSupersededConsentDuringRecovery(t *testing.T)
t.Fatal("recovered replacement did not resume imports")
}
}
// A removed bank account must stay removed. Session recovery reconstructs
// bindings for interrupted connects, so a stale binding silently resurrects
// the account on the next connect or sync.
func TestDeletedBankAccountIsNotResurrectedByLaterConnectOrSync(t *testing.T) {
a, s := testApp(t)
ctx := context.Background()
kept := s.Data.Accounts[0]
kept.ExternalAccountID = "kept_uid"
removed := domain.Account{ID: "removed_acct", DisplayName: "Mwst", Institution: "N26", Currency: "EUR", ExternalAccountID: "removed_uid", Active: true}
s, err := a.Mutate(ctx, s.Revision, func(d *domain.Dataset) error {
d.Accounts = []domain.Account{kept, removed}
return nil
})
if err != nil {
t.Fatal(err)
}
session := banking.Session{ID: "n26_session", ValidUntil: time.Now().Add(24 * time.Hour).Format(time.RFC3339), Accounts: []domain.Account{kept, removed}}
b := &historyBank{bankScenario: bankScenario{session: session}}
a.bank = b
a.ops.Sessions = []banking.Session{session}
a.ops.Consents[session.ID] = Consent{Institution: "N26", Country: "DE", HistoryMonths: 12}
a.ops.AccountSync[removed.ID] = time.Now().UTC().Format(time.RFC3339)
if err := a.saveOps(); err != nil {
t.Fatal(err)
}
s, err = a.ManageRegistry(ctx, s.Revision, "account", "delete", removed.ID, "")
if err != nil {
t.Fatal(err)
}
if slices.ContainsFunc(s.Data.Accounts, func(v domain.Account) bool { return v.ID == removed.ID }) {
t.Fatal("account deletion did not remove the account")
}
if _, tracked := a.ops.AccountSync[removed.ID]; tracked {
t.Fatal("deleted account kept its sync cursor")
}
// A later connect for a different bank triggers binding recovery.
other := domain.Account{ID: "ing_acct", DisplayName: "ING Giro", Institution: "ING", Currency: "EUR", ExternalAccountID: "ing_uid", Active: true}
b.session = banking.Session{ID: "ing_session", ValidUntil: time.Now().Add(24 * time.Hour).Format(time.RFC3339), Accounts: []domain.Account{other}}
if _, err := a.Authorize(ctx, "ING", "DE", 12); err != nil {
t.Fatal(err)
}
if _, err := a.Callback(ctx, "one_time_code", b.authState); err != nil {
t.Fatal(err)
}
a = reopenBankingApp(t, a)
a.bank = b
after, err := a.Sync(ctx)
if err != nil {
t.Fatal(err)
}
names := map[string]bool{}
for _, account := range after.Data.Accounts {
names[account.ID] = true
}
if names[removed.ID] {
t.Fatalf("deleted bank account reappeared: %+v", after.Data.Accounts)
}
if !names[kept.ID] || !names[other.ID] {
t.Fatalf("connecting another bank lost existing accounts: %+v", after.Data.Accounts)
}
for _, connection := range after.Connections {
if connection.AccountID == removed.ID {
t.Fatal("deleted account still reported as connected")
}
}
}
// A consent that shares no accounts cannot sync and its session is reaped by
// recovery, so the user must be told instead of being silently redirected.
func TestConnectWithoutSharedAccountsFailsVisibly(t *testing.T) {
a, _ := testApp(t)
ctx := context.Background()
b := &historyBank{bankScenario: bankScenario{session: banking.Session{ID: "empty_session", ValidUntil: time.Now().Add(24 * time.Hour).Format(time.RFC3339)}}}
a.bank = b
if _, err := a.Authorize(ctx, "Kontist", "DE", 12); err != nil {
t.Fatal(err)
}
_, err := a.Callback(ctx, "one_time_code", b.authState)
if err == nil || !strings.Contains(err.Error(), "no accounts") {
t.Fatalf("empty consent reported success: %v", err)
}
after, e := a.Snapshot(ctx)
if e != nil {
t.Fatal(e)
}
if len(after.Sessions) != 0 {
t.Fatalf("unusable consent was stored: %+v", after.Sessions)
}
for _, connection := range after.Connections {
if connection.Status != "local" {
t.Fatalf("unusable consent produced a bank connection: %+v", connection)
}
}
}
+25 -14
View File
@@ -249,29 +249,40 @@ func connectAccounts(d *domain.Dataset, session *banking.Session, reconnect bool
}
}
}
func (a *App) Callback(ctx context.Context, code, state string) error {
// Callback completes a bank authorization and returns how many accounts the
// bank shared that could not be linked to a journal account.
func (a *App) Callback(ctx context.Context, code, state string) (int, error) {
a.mu.Lock()
defer a.mu.Unlock()
auth, ok := a.authStates[state]
delete(a.authStates, state)
if !ok || time.Now().After(auth.Expires) {
return errors.New("authorization state expired or invalid; reconnect again")
return 0, errors.New("authorization state expired or invalid; reconnect again")
}
if a.bank == nil || code == "" {
return errors.New("authorization did not provide a code")
return 0, errors.New("authorization did not provide a code")
}
session, err := a.bank.Exchange(ctx, code)
if err != nil {
return err
return 0, err
}
// A consent without linkable accounts can never sync and its stored
// session would be reaped silently. Fail visibly instead.
if len(session.Accounts) == 0 {
if session.Unlinkable > 0 {
return 0, fmt.Errorf("the bank shared %d account(s), but none could be linked: they lack an IBAN or stable identification, or use an unsupported currency", session.Unlinkable)
}
return 0, errors.New("the bank authorized the connection but shared no accounts, so nothing was linked; accounts of another type (for example business) may need a separate consent")
}
a.ops.Sessions = append(a.ops.Sessions, session)
a.ops.Consents[session.ID] = Consent{Institution: auth.Institution, Country: auth.Country, HistoryMonths: auth.HistoryMonths}
if err = a.saveOps(); err != nil {
return err
return 0, err
}
s, err := a.snapshot(ctx)
if err != nil {
return err
return 0, err
}
connectAccounts(&s.Data, &session, true)
// Remove superseded account bindings, not unrelated bank consents.
@@ -295,16 +306,16 @@ func (a *App) Callback(ctx context.Context, code, state string) error {
// Save once-only provider details before the canonical commit. Sync can recover
// the account bindings if a crash or external edit interrupts that commit.
if err = a.saveOps(); err != nil {
return err
return 0, err
}
_, err = a.commit(ctx, s.Revision, s.Data)
if err == nil {
select {
case a.syncRequested <- struct{}{}:
default:
}
if _, err = a.commit(ctx, s.Revision, s.Data); err != nil {
return 0, err
}
return err
select {
case a.syncRequested <- struct{}{}:
default:
}
return session.Unlinkable, nil
}
func (a *App) Balances(ctx context.Context, id string) ([]banking.Balance, error) {
a.mu.Lock()
+44
View File
@@ -1,14 +1,58 @@
package app
import (
"context"
"errors"
"fmt"
"slices"
"strings"
"finance-duck/internal/banking"
"finance-duck/internal/domain"
)
// ManageRegistry applies registry management and, for account deletions, also
// releases the account's bank bindings first: a deleted account must never be
// resurrected by the session recovery that reconstructs interrupted connects.
func (a *App) ManageRegistry(ctx context.Context, rev, entity, action, id, target string) (State, error) {
a.mu.Lock()
defer a.mu.Unlock()
s, err := a.snapshot(ctx)
if err != nil {
return State{}, err
}
if rev != s.Revision {
return State{}, errors.New("revision conflict: reload before editing")
}
if err = Manage(&s.Data, entity, action, id, target); err != nil {
return State{}, err
}
// Prune bindings before the canonical commit: an interruption then leaves
// an unbound local account rather than a resurrected bank connection.
if entity == "account" {
changed := false
sessions := make([]banking.Session, 0, len(a.ops.Sessions))
for _, session := range a.ops.Sessions {
accounts := slices.DeleteFunc(slices.Clone(session.Accounts), func(account domain.Account) bool { return account.ID == id })
changed = changed || len(accounts) != len(session.Accounts)
session.Accounts = accounts
if len(accounts) == 0 {
delete(a.ops.Consents, session.ID)
continue
}
sessions = append(sessions, session)
}
if _, tracked := a.ops.AccountSync[id]; tracked || changed {
a.ops.Sessions = sessions
delete(a.ops.AccountSync, id)
if err = a.saveOps(); err != nil {
return State{}, err
}
}
}
return a.commit(ctx, rev, s.Data)
}
func SaveAccount(d *domain.Dataset, v domain.Account) error {
v.DisplayName = strings.TrimSpace(v.DisplayName)
if v.ID == "" {
+2 -2
View File
@@ -102,7 +102,7 @@ func TestReconnectReplacesOldConsentWithoutDuplicatingLocalAccount(t *testing.T)
renewed.DisplayName = "Bank-generated name"
a.bank = &bankScenario{session: banking.Session{ID: "new_session", ValidUntil: time.Now().Add(24 * time.Hour).Format(time.RFC3339), Accounts: []domain.Account{renewed}}}
a.authStates["one_time_state"] = authorization{Expires: time.Now().Add(time.Minute), Institution: "N26", Country: "DE", HistoryMonths: 24}
if err = a.Callback(context.Background(), "bank_code", "one_time_state"); err != nil {
if _, err = a.Callback(context.Background(), "bank_code", "one_time_state"); err != nil {
t.Fatal(err)
}
after, err := a.Snapshot(context.Background())
@@ -118,7 +118,7 @@ func TestReconnectReplacesOldConsentWithoutDuplicatingLocalAccount(t *testing.T)
if a.ops.AccountSync[account.ID] != cursor || after.Connections[0].HistoryMonths != 24 {
t.Fatal("reconnect reset the account cursor or lost the history choice")
}
if err = a.Callback(context.Background(), "bank_code", "one_time_state"); err == nil {
if _, err = a.Callback(context.Background(), "bank_code", "one_time_state"); err == nil {
t.Fatal("authorization state replay was accepted")
}
}
+9 -1
View File
@@ -37,6 +37,10 @@ type Session struct {
ID string `json:"session_id"`
ValidUntil string `json:"valid_until"`
Accounts []domain.Account `json:"accounts"`
// Unlinkable counts accounts the bank shared that cannot be represented
// as a journal account (no IBAN or stable identification, or an
// unsupported currency). They are excluded, never silently merged.
Unlinkable int `json:"unlinkable,omitempty"`
}
// SessionStatus contains only the current consent expiry and external account
@@ -585,10 +589,14 @@ func (p *EnableBanking) Exchange(ctx context.Context, code string) (Session, err
return Session{}, fmt.Errorf("Enable Banking returned invalid session expiry")
}
result := Session{ID: response.ID, ValidUntil: response.Access.ValidUntil, Accounts: []domain.Account{}}
// One account the journal cannot represent (securities or card entries
// commonly lack an IBAN, stable identification or a currency) must not
// discard the whole consent: link the usable accounts and count the rest.
for _, a := range response.Accounts {
account, err := a.account(response.ASPSP.Name)
if err != nil {
return Session{}, err
result.Unlinkable++
continue
}
result.Accounts = append(result.Accounts, account)
}
+21
View File
@@ -283,6 +283,27 @@ func TestEnableBankingInstitutionsListsOnlyConnectableBanksWithSafeLogos(t *test
t.Fatal("accepted an invalid country code")
}
}
func TestEnableBankingLinksUsableAccountsAndCountsTheRest(t *testing.T) {
p, _ := testProvider(t, func(w http.ResponseWriter, r *http.Request) {
fmt.Fprint(w, `{"session_id":"session-1","access":{"valid_until":"2099-01-01T00:00:00Z"},"aspsp":{"name":"ING","country":"DE"},"accounts":[
{"uid":"giro","identification_hash":"hash-giro","account_id":{"iban":"DE02120300000000202051"},"details":"Girokonto","currency":"EUR"},
{"uid":"depot","identification_hash":"hash-depot","details":"Direkt-Depot"},
{"uid":"card","account_id":{},"details":"Credit card","currency":"EUR"},
{"uid":"extra","identification_hash":"hash-extra","details":"Extra-Konto","currency":"EUR"}
]}`)
})
session, err := p.Exchange(context.Background(), "code")
if err != nil {
t.Fatalf("one unusable shared account discarded the whole consent: %v", err)
}
var names []string
for _, account := range session.Accounts {
names = append(names, account.DisplayName)
}
if !reflect.DeepEqual(names, []string{"Girokonto", "Extra-Konto"}) || session.Unlinkable != 2 {
t.Fatalf("wrong linked accounts or unlinkable count: %v %d", names, session.Unlinkable)
}
}
func TestEnableBankingSurfacesOnlyDocumentedErrorCodes(t *testing.T) {
body := ""
p, _ := testProvider(t, func(w http.ResponseWriter, r *http.Request) {
+13 -4
View File
@@ -6,10 +6,12 @@ import (
"errors"
"io"
"io/fs"
"log"
"mime"
"net"
"net/http"
"net/url"
"strconv"
"strings"
"time"
@@ -304,7 +306,7 @@ func (s *Server) manage(w http.ResponseWriter, r *http.Request) {
if !decode(w, r, &b) {
return
}
v, e := s.app.Mutate(r.Context(), b.Revision, func(d *domain.Dataset) error { return app.Manage(d, b.Entity, b.Action, b.ID, b.TargetID) })
v, e := s.app.ManageRegistry(r.Context(), b.Revision, b.Entity, b.Action, b.ID, b.TargetID)
respond(w, v, e)
}
func (s *Server) importCSV(w http.ResponseWriter, r *http.Request) {
@@ -410,12 +412,19 @@ func (s *Server) authorize(w http.ResponseWriter, r *http.Request) {
respond(w, map[string]string{"url": v}, e)
}
func (s *Server) callback(w http.ResponseWriter, r *http.Request) {
e := s.app.Callback(r.Context(), r.URL.Query().Get("code"), r.URL.Query().Get("state"))
unlinkable, e := s.app.Callback(r.Context(), r.URL.Query().Get("code"), r.URL.Query().Get("state"))
if e != nil {
respond(w, nil, e)
// Callback errors are locally generated and sanitized. Landing on the
// app with the reason visible beats a bare JSON error page.
log.Printf("bank connection callback failed: %v", e)
http.Redirect(w, r, "/?connect_error="+url.QueryEscape(e.Error()), http.StatusSeeOther)
return
}
http.Redirect(w, r, "/?connected=1", http.StatusSeeOther)
target := "/?connected=1"
if unlinkable > 0 {
target += "&unlinkable=" + strconv.Itoa(unlinkable)
}
http.Redirect(w, r, target, http.StatusSeeOther)
}
func (s *Server) preview(w http.ResponseWriter, r *http.Request) {
var b app.PreviewRequest