Skip to content

feat(banking): add provider contracts and Enable Banking foundation - #536

Merged
theperu merged 15 commits into
RIP-Comm:mainfrom
marianialessandro:feature/enable-banking-foundation
Sep 12, 2026
Merged

theperu merged 15 commits into
RIP-Comm:mainfrom
marianialessandro:feature/enable-banking-foundation

Conversation

@marianialessandro

@marianialessandro marianialessandro commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Adds a banking foundation with provider-independent contracts and an Enable Banking implementation. Application services can depend on institution discovery, remote consent operations or account data separately, and replace each capability through Riverpod.

Structure

  • lib/services/banking/ defines BankInstitutionDirectory, BankConsentService, BankAccountDataSource and their shared domain models. BankingProvider groups the three capabilities.
  • lib/services/banking/enable_banking/ contains the REST client, wire DTOs, RS256 JWT authentication, BYOC setup and secure credential storage, plus separate adapters and mappers for each capability.
  • lib/providers/banking_provider.dart composes the implementation and exposes independently replaceable dependencies.

The adapters support institution discovery, authorization and remote session create/read/revoke, account details, balances and transaction pages. Mapping preserves exact decimal amounts, separate transaction identifiers, account identity hashes, balance metadata, calendar-date filters and opaque pagination cursors. Invalid responses and foreign-provider references fail explicitly; implementation errors are translated at the boundary.

The foundation also registers the callback scheme, verifies candidate credentials through GET /application before atomically replacing the encrypted configuration, derives the environment and supported countries from the server, and checks Android release network access. Credential storage keys and authentication behavior are preserved by the module move.

Scope

Callback acceptance and state validation belong to the application consent lifecycle in the next PR. Persistence, reconnect orchestration, transaction synchronization and user-facing screens remain in subsequent stacked PRs. This foundation exposes no banking UI entry point. The following branches will need to adopt the new module paths and contracts.

Validation

Verified locally with Flutter 3.38.8 / Dart 3.10.7 and JDK 21:

  • Dependency resolution, build_runner and formatting: passed (216 Dart files, no formatting changes).
  • Flutter analyze: no issues.
  • Serial test suite: 134 passed, including capability substitution, HTTP adapter mapping, malformed responses, exact amounts, identity isolation and calendar-date regressions.
  • Android default release App Bundle: built successfully (55.2 MB); Internet permission verified in the merged release manifest.

Adapter tests use synthetic HTTP responses; this change does not claim a new live-bank or UI end-to-end verification.

Remote CI on 5660478: upstream PR checks passed. The fork push workflow passed dependency resolution, code generation, formatting and tests, then failed at Dump keystore because the Android signing secrets are missing. Its Android build remains blocked; the local release build passed as listed above.

DTOs for ASPSPs, authorization, session, account, balance and transactions, with tests over sample JSON fixtures.
Adds EnableBankingConfig (environment/redirect/base URL) and a
FlutterSecureStorage-backed store for the user's BYOC app_id and
private key PEM, plus a draft riverpod provider exposing them.
Signs Enable Banking API JWTs on-device from the user's BYOC app_id
and private key PEM, caching the token in memory and refreshing it
shortly before expiry.
Thin http client for the Enable Banking endpoints (aspsps, auth,
sessions, balances, transactions), mapping responses onto the DTOs
and raising EnableBankingException on HTTP errors.

@mikev-cw mikev-cw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this key foundation! Generally I think it's very good!

In addition to the comment regarding the CI workflow and dates, my main concerns are around architecture and design.

Since this PR introduces the first banking integration layer, I think we should be careful not to make the app architecture too EnableBanking-centric. What if, in the future we replace EB, or add other providers in parallel?
What do you think on isolating Enable Banking under a provider-specific module, and expose a more generic banking abstraction to the rest of the app?
For example, the app-facing layer could use generic concepts such as BankingProvider, BankingConnection, BankInstitution, BankAccount, BankTransaction, and BankingCredentials, while the Enable Banking DTOs/client stay internal to enable_banking.

I see that lib/providers/banking_provider.dart already creates a central access point for the banking services, which is a good start. My concern is that this app-facing provider layer still exposes Enable Banking-specific names and types, so the rest of the app may naturally start depending on EB concepts directly.

I'm just reasoning out loud, I'm interested in getting your opinion @marianialessandro, @lucaantonelli and @theperu. Because yes, this might be better, but I'm confident it would make development longer and more complex, which might not be what we want.
On the other hand, we're building the foundations right now: a lot of these decisions can only be made at this stage, if we don't want to rewrite half the app later.

Comment thread .github/workflows/ci-cd.yml
Comment thread lib/services/banking/enable_banking/enable_banking_api.dart
@marianialessandro

Copy link
Copy Markdown
Contributor Author

I agree with your concern. The Riverpod layer currently exposes Enable Banking specific types, so the rest of the app could easily become coupled to them.
It's a reasonable approach to introduce a small app-facing banking interface with provider-independent models, and keep the EB client, DTOs and their mapping inside an enable_banking module.
I would keep credentials and setup provider-specific, since EB's app ID/private key configuration may not fit another provider. I'd also scope the shared interface to the operations we actually need, without building full multi-provider support upfront.

This would require adapting the consent and sync work already in progress, but I think establishing that boundary before wiring up the UI is worthwhile.
Does that sound like the level of abstraction you had in mind?

This afternoon I'll get to work immediately to begin implementing this aspect because I think it could be an added value if in the future we want to add alternatives to Enable Banking rather than replace it.

@marianialessandro marianialessandro changed the title feat(banking): add secure Enable Banking API foundation feat(banking): add provider contracts and Enable Banking foundation Sep 10, 2026
@marianialessandro

Copy link
Copy Markdown
Contributor Author

@mikev-cw I’ve updated the foundation to separate provider-independent banking contracts from the Enable Banking implementation. Institution discovery, remote consent operations, and account data now have dedicated interfaces, adapters, and mappers, making them independently replaceable and testable

@mikev-cw

Copy link
Copy Markdown
Collaborator

Ok, it moves in a SOLID-friendly direction, i like it!
The only thing to keep in mind for the future is the multi-provider scenario: supporting multiple active providers simultaneously might require an additional registry/composite layer.
However, that feels like overkill at this stage and should be straightforward to implement later if needed.

So let's skip it for now.
Nothing else to consider on my end. Excellent contribution!

@theperu theperu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! ✅

I agree with what was pointed out by Mike in his review and I think we can now proceed with the merge!

@theperu
theperu merged commit 841d620 into RIP-Comm:main Sep 12, 2026
1 check passed
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.

3 participants