Skip to content

feat(bank-sync): add Enable Banking as a bank-sync provider - #454

Merged
otherworld-dev merged 47 commits into
otherworld-dev:devfrom
saesch83:feature/enable-banking-provider
Nov 7, 2026
Merged

otherworld-dev merged 47 commits into
otherworld-dev:devfrom
saesch83:feature/enable-banking-provider

Conversation

@saesch83

Copy link
Copy Markdown
Contributor

Adds Enable Banking as a second bank-sync provider next to GoCardless. Follows up on #271: GoCardless no longer takes new sign-ups, while Enable Banking's free restricted production mode lets a private person connect their own accounts at 2,500+ European banks without a business contract or AISP licence.

What it does

  • Keys once per user. Each user stores their Enable Banking application ID and private key (PEM) once. They are kept as one encrypted user setting (EncryptionService), never returned by the settings API, excluded from backups/export, and kept when settings are reset.
  • Connect wizard. Choose "Enable Banking", pick country and bank, confirm at the bank, return via a callback route. The callback uses Post-Redirect-Get, so the one-time code/state don't stay in the URL. Re-authorize keeps existing account mappings and revokes the old session.
  • Sync. Fetches details, balances and transactions per mapped account.
    • Sync windows are per account. The first fetch reaches back 90 days; later fetches start at the last sync minus 7 days.
    • Unmapped or disabled accounts get no transaction fetch, which saves the bank's daily request quota.
    • Transactions without a bank ID (e.g. Trade Republic) get a stable hash ID with numbering, so identical rows are not dropped.
  • Exact balances. On a mapping's first sync, Budget sets the opening balance so its balance equals the bank's. After that, the UI shows the difference and offers a one-click reconcile.
    • The comparison uses the date the bank reports the balance for. For example, comdirect reports the previous day's close.
    • Only providers that implement ReliableBalanceProviderInterface auto-reconcile.
  • Clear errors for the cases we hit with real banks: an inactive restricted application, a consent that shared no accounts, or an authorization still in progress.

Changes to shared code

  • New small interfaces: InstitutionListingProviderInterface, ReauthorizableProviderInterface, ReliableBalanceProviderInterface. BankSyncController / BankSyncService use these instead of GoCardless-specific checks.
  • GoCardless: reports no balance when the bank sends none, instead of 0. Otherwise behaviour is unchanged.
  • Migrations:
    • Version001000127Date20261027 adds budget_bam.balance_reconciled_at.
    • Version001000128Date20261027 adds budget_bam.last_balance_date.
  • README.md: Enable Banking setup section, covering production needs an https redirect URL, linking accounts in the Enable Banking control panel first, and Trade Republic's limited data.

Privacy / security

  • Outbound traffic goes to api.enablebanking.com only. Requests are signed with a short-lived RS256 JWT built with ext-openssl. There are no new dependencies.
  • Keys, JWTs, session IDs, auth codes, state and amounts never reach logs or exception messages.
    • Key parameters are marked #[\SensitiveParameter], and the key value object hides itself from var_dump/print_r.
    • Transport errors are re-wrapped without the URL.
    • The callback logs nothing, because Nextcloud appends the request URL to every log entry.
  • Bank logo URLs are not passed to the browser, so opening the bank list contacts no third-party host.

Testing

  • Automated: 7,689 PHPUnit tests pass (1 skipped). cs:check, Psalm and npm run lint are clean, and there are 15 new vitest tests for the wizard.
  • End-to-end:
    • Ran on Nextcloud 31 + MariaDB with all egress forced through a logging proxy.
    • Enable Banking sandbox: connect, callback, re-authorize and repeated sync all work without duplicates.
    • Real banks, restricted production: comdirect (two accounts) and Trade Republic. The Budget balance matched the bank balance to the cent.

Notes for review

  • Version bump needed. appinfo/info.xml is still 3.2.0. Instances that already run 3.2.0 only apply migrations 127/128 after a version bump, so I left the version to you.
  • Trade Republic data is limited. Over PSD2 it sends only amount, currency and booking date per transaction, with no description or counterparty. Its account label is a product code such as CHECKING_ACCOUNT (…1234). In our test it returned at most 100 transactions per request.
  • Commits. The branch has 44 Conventional Commits, so it reads commit by commit. Happy to squash if you prefer.

🤖 Generated with Claude Code

saesch83 and others added 30 commits October 27, 2026 14:42
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Normalizer maps Enable Banking JSON (signed amounts via string ops,
balance type matched to includePending, masked IBAN names) and the
provider covers auth start, session exchange, paginated sync with a
90-day/7-day-overlap window, reauth, revoke and the bank list. Accounts
are keyed by identification_hash so mappings survive new session UIDs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Transport errors from the HTTP client carry the request URI, which holds
session IDs, account UIDs and continuation keys. The client now replaces
them with a fixed message (no chained previous exception), and the
provider logs only the exception class.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… any provider

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bank redirects to /bank-sync/enablebanking/callback with code and
state. The state is matched in constant time only among the logged-in
user's pending Enable Banking connections, expires after 30 minutes and
is cleared on use. Completing stores the new session, revokes the
previous one, keeps mappings by identification_hash and records the
consent expiry on each mapping.

The settings API stores, describes and deletes the user's application
keys without ever returning the private key; deleting is refused while
an Enable Banking connection exists.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds the Enable Banking provider to the bank-sync wizard: one-time key
entry (or reuse of server-stored keys), bank list without secrets,
connect/re-authorize without a client-supplied redirect URL, and
enablebanking-specific step 3 wording.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d offer reconcile

A bank sync only fetches recent history, so an account filled by it
started at the sum of those rows instead of the bank's balance.

On the first sync into an account that holds nothing but the rows just
imported, the opening balance is now set so that the balance equals the
bank's. This applies to every provider, and skips liabilities and
accounts kept in another currency than the bank reports.

After that nothing is corrected silently: a difference usually means a
missing or doubled row. The account mappings list shows "Bank / Budget /
Difference" for a mapping whose balances differ, with a Reconcile action
(POST .../mappings/{mappingId}/reconcile) that moves the opening balance
by the difference. It is audited as bank_balance_reconciled, without
amounts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An account that stayed empty passed the "holds only the rows just
imported" check on every sync (0 === 0, later 1 === 1), so its opening
balance silently followed the bank's and hid a row the bank never
delivered.

The mapping now records when its balance was matched to the bank's
(budget_bam.balance_reconciled_at, migration 127). The first sync sets
it, and so does Reconcile; while it is set, nothing is matched
automatically. Mapping the bank account to another Budget account, or
unmapping it, clears it, so the newly mapped account gets its own first
sync.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tion traces

With zend.exception_ignore_args off, logged exceptions kept every frame's
arguments, so a 401 from Enable Banking put the PEM into nextcloud.log.
Key material now travels as EnableBankingKeys (private properties,
redacted debug info) so PHP 8.1 is covered too, and parameters carrying
keys, credentials JSON, session paths, codes or states are marked
#[\SensitiveParameter]. The key store no longer passes on exceptions
from encrypting the key, and saveSettings reads the PEM from the request
instead of taking it as an argument.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ettings API

The encrypted key blob lived in budget_settings, so it was exported in
the data backup, replaced or wiped by a restore while the connections
stayed, and returned to the browser by GET /api/settings. It is now a
server setting (never exported, kept through a restore), hidden from
the settings index and show endpoints, and cannot be deleted there,
which would have skipped the check for connections still using it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…apart

Rows without transaction_id or entry_reference fall back to a hash of
date, amount, direction and description, so two genuine identical
bookings on one day got the same ID and the sync dropped the second.
Repeated fallback hashes are now numbered in feed order (h:<hash>,
h:<hash>:2, ...), which the same feed reproduces on the next sync.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CLBD is usually the previous day's closing balance, while Budget
compares against its balance as of today, so the first-sync match set
the opening balance off by today's bookings. The booked order is now
ITBD, CLBD, XPCD; the order with pending rows is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GoCardless returned '0' for an account without balances, which the
balance comparison took for a real zero. It now returns null, as the
provider interface documents.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… providers

GoCardless and SimpleFIN can report an available balance that includes
a credit line, or none, so matching a new account's opening balance to
it after an upgrade could set it wrong without anyone noticing. Only
providers implementing the new ReliableBalanceProviderInterface marker
(Enable Banking) are auto-matched; the others still show the difference
with the Reconcile action.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…read

When every account of the session failed with a generic error,
fetchAccounts returned an empty list and the connection stayed active
without an error. It now throws "The bank could not be reached. Please
try again later.", which the sync stores as the connection's error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A failed POST /sessions now clears the used-up state and records "Authorization
was cancelled or failed at the bank.", like a bank-side error does. Syncing a
pending connection that never got a session asks the user to finish connecting
instead of flipping it to expired. During a re-authorization the old session
still syncs, but the connection stays pending, so the bank's redirect matches it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ization

A pending Enable Banking connection with an error (cancelled or failed at
the bank) showed only Sync, which cannot help it. Its card now shows the
Re-authorize button, as an expired one does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t off

Reaching MAX_TRANSACTION_PAGES still counted as a complete fetch and
moved lastSyncDate forward, so the rows after the last page were never
asked for again. The account now counts as incomplete, and a warning
with a fixed text is logged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reloading the callback page re-sent the spent code and showed a
failure, and code and state stayed in the address bar and history.
The callback now processes the bank's redirect once and redirects to
itself with only ?result=success|failed, which renders the page.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One mapping whose difference could not be computed failed the whole
mappings request. That mapping now gets balanceDifference null and a
warning with a fixed text is logged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…resets

The PEM stayed in the hidden textarea (and the file input kept the file)
after Cancel. Resetting the wizard, which opening it and now Cancel do,
clears the key, the application ID and the file input.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use the wizard's labels, make the admin switch a separate step before
the setup, say that the automatic first-sync balance match applies to
Enable Banking only, and say plainly who loads bank logos.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… a stuck Enable Banking connection

Nextcloud core appends the request URL to every log entry, so a warning
written while handling the bank's redirect leaked code and state. The
callback path now logs nothing: the controller no longer logs failures
(they are kept on the connection), and revoking the previous session after
a re-authorization uses a new non-logging EnableBankingProvider::revokeSession.

The server now decides whether a connection can be re-authorized and
returns it as canReauthorize with each connection: expired connections of
a re-authorizable provider, and Enable Banking connections in pending_auth
with an error or without a fresh state. The card follows that flag instead
of guessing client-side. sync() keeps such a connection pending only while
its state is present and younger than the TTL.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…untouched

While a re-authorization is under way (fresh state), a manual sync could
expire the connection and "I've Completed Authorization" could promote it
to active with the old session; either way the bank's redirect no longer
matched. Both now refuse with "Please finish authorizing at your bank
first." before any provider call or status change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…estamp

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
saesch83 and others added 15 commits October 27, 2026 15:06
The callback caught only \Exception, so an \Error reached Nextcloud's
logging handler together with the request URL carrying code and state.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The UI shows Enable Banking banks by name only, so listInstitutions
returns logo => null for every bank.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…type

The docblock already said entries may carry extra keys; the sealed shape
made psalm reject Enable Banking's maximumConsentValidity.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…it is actually imported

The UI syncs once right after connecting, before any account is mapped.
That sync advanced the connection-wide lastSyncDate, so the first real
import fetched only 7 days instead of 90.

BankSyncService now passes syncAccounts (external ID => needsFullHistory,
from enabled mappings with a budget account; full history while the
balance was never reconciled). Enable Banking fetches transactions only
for those accounts and keeps a lastSyncDates entry per account, advanced
only after a complete read. The legacy scalar lastSyncDate is ignored.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
balanceReconciledAt stays null when auto-reconcile skips an account
(other rows, liability, other currency, ...), so such accounts asked for
90 days on every sync. A mapping now needs full history only while it is
unreconciled and its budget account holds no row imported by this
provider (TransactionMapper::hasImportIdPrefix, one LIMIT 1 query).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ccounts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
comdirect reports CLBD (the previous day's close) and XPCD, each with a
reference_date. Budget compared any bank balance with its own balance as
of today, so a row booked today showed as a difference, and the first
sync moved the opening balance by it.

The Enable Banking normalizer now returns the chosen balance's
reference_date, the provider passes it on as the optional balanceDate,
and the mapping stores it (last_balance_date, migration 128). The
difference, the manual reconcile and the first-sync auto-reconcile use
Budget's balance as of that day; without a date (GoCardless, SimpleFIN)
or with one after today, as of today.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rst-sync window

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e one day ahead

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@saesch83

saesch83 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Hi, I saw you rejected that kind of pull request already ones. But this time it is implemented in a good shape. And the problem with GoCardless is, that it is only for business use and not for private use. While EnableBanking is allowed to use for your own bank account privately. Also I implemented the appid and private key handling savely and easily.

Regarding your plan to involve an API hoster on your own with a proper license in place. I doupt, that you will be able to do so, as this kind of service is quite expensive.

I would be very happy to have enable banking in your version. Otherwise I need to maintain it in parallel. I have also some friends who would use it then in future.

@otherworld-dev

Copy link
Copy Markdown
Owner

I've been through this and tested it on a throwaway instance with a mocked Enable Banking API - connect, callback, sync, resync and reconcile all worked, with nothing secret in the logs or the backup zip. Unit, integration, Psalm, cs and lint all pass, and it merges cleanly into dev.

A few things before I merge:

  • If Enable Banking rejects the app ID or key (any 401/403 apart from the "not active" one), the user only gets "Failed to connect bank" or "Failed to fetch institutions". Can you map that to a message saying the application ID or key was rejected?
  • Can you validate what comes back from the API before it's stored - currency as 3 letters, amounts as numbers, and the auth url as https? The mapping list renders currency and balance into HTML, so I'd rather not trust them.
  • On every sync, unmapped accounts still get /details and /balances. With the daily access limits, could those be skipped unless it's a refresh?
  • User docs live on budget.otherworld.dev, so I'll move the README setup section over there. Could you cut the README back to a one-line mention?

One bigger thing. I'm planning a hosted bank sync option later that will use the same redirect-with-state flow, and at the moment the callback route, the state lookup and the pending_auth handling are all tied to 'enablebanking'. If you're up for it, a small interface for "provider that completes auth from a callback" (your completeAuthorization already fits) plus a provider-neutral route like /bank-sync/callback would let the next provider reuse it. The route matters most, because users register it in the Enable Banking control panel and it's awkward to change after a release. If you'd rather not, no problem - I'll do that bit on dev before the release.

I'll do the .pot, the js bundle rebuild and the CHANGELOG after merging, and I'll squash it on merge.

@saesch83

saesch83 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for your review and feedback. Will adjust and add that the next days.

saesch83 and others added 2 commits November 6, 2026 07:42
…ng-provider

# Conflicts:
#	budget/lib/Controller/BankSyncController.php
#	budget/lib/Service/BankSync/BankSyncService.php
#	budget/src/modules/bank-sync/BankSyncModule.js
- Provider-neutral callback: the bank's redirect goes to
  /bank-sync/callback (BankSyncCallbackController). Providers that
  complete their authorization from it implement
  CallbackAuthorizationProviderInterface (completeAuthorization,
  isAuthorized, revokeReplacedAuthorization); the state lookup,
  pending_auth handling and Re-authorize no longer check for
  'enablebanking'.
- A 401/403 from Enable Banking other than "application not active"
  now says the application ID or private key was rejected, when
  listing banks, connecting and re-authorizing.
- Data from the API is validated before it is stored: transactions
  without a plain decimal amount or a real date are dropped, a balance
  without a usable amount is null, and the currency of accounts from a
  completed authorization goes through cleanCurrency() like the rest.
- A sync no longer reads accounts that aren't imported (/details and
  /balances count against the bank's daily limit); refreshing the
  account list still reads them.
- README: the setup section moves to the docs site, the feature list
  keeps the mention.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@saesch83

saesch83 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review! All of it is in:

  • Rejected keys: any 401/403 other than "application not active" now says "Enable Banking rejected the application ID or private key. Check that both belong to the same application in the Enable Banking control panel, then enter them again with Replace keys." This applies when listing banks, connecting and re-authorizing.
  • Validation: I merged dev, so the Enable Banking paths use your applyProviderAccountDetails(), safeAuthorizationUrl(), noopener and handleErrorWithoutTrace() from fix(bank-sync): treat provider data as untrusted and keep secrets out of logs #474. Accounts from a completed authorization also go through cleanCurrency(). In the normalizer, a transaction without a plain decimal amount or a real Y-m-d date is dropped, and a balance without a usable amount is null.
  • Daily limits: a sync no longer reads accounts that aren't imported, so it sends no /details and no /balances for them. Refresh still reads every account. With nothing mapped, a sync makes no request and doesn't mark the connection expired.
  • README: the setup section is gone and the feature list keeps the mention. The callback path for your docs is now /apps/budget/bank-sync/callback.
  • Provider-neutral callback: there's a new CallbackAuthorizationProviderInterface (completeAuthorization, isAuthorized, revokeReplacedAuthorization). BankSyncCallbackController serves /bank-sync/callback. The state lookup, the pending_auth handling and Re-authorize go through the interface instead of checking for 'enablebanking'. Pending credentials keep state and stateCreatedAt, as documented on the interface. A unit test runs a second "hosted" provider through the same flow.

I merged dev rather than rebasing, since you squash anyway. PHPUnit (7,795), the integration tests, Psalm, cs and lint pass. I also tested it end to end on my test instance against a real bank: connect, callback on the new URL, sync, re-authorize and a deliberately wrong key.

@otherworld-dev

Copy link
Copy Markdown
Owner

Merged. The one thing left from my review was the factory reset, it deleted the keys before the revoke ran so the Enable Banking session was never ended. I have fixed that on dev rather than hold this up any longer. It will be in the next release, and the setup docs will go up on budget.otherworld.dev with it. Thanks for sticking with this one.

otherworld-dev added a commit that referenced this pull request Nov 7, 2026
The two tests check that #[\SensitiveParameter] keeps the session ID and key
out of exception traces, which PHP 8.1 ignores, so they failed the 8.1 CI job
after #454 was merged. On 8.1 these errors are only logged without a trace.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants