From 9238c48bb507e104bdd40d8fabcd5efc16ace10a Mon Sep 17 00:00:00 2001 From: Herculino Trotta Date: Sun, 8 Nov 2026 16:08:37 -0300 Subject: [PATCH] feat(bank-sync): add SimpleFIN provider Users paste a one-time setup token. On save it's claimed and the access URL it returns is stored; credentials go in a header, never the URL, so errors don't leak them. - Providers can validate or transform their settings (clean_settings), run by the service form once every other field is valid, so a token isn't spent on a form that fails anyway. - Providers can declare a minimum sync interval. SimpleFIN Bridge disables tokens that go much past ~24 requests a day, so it's 6 hours. - Tokens and access URLs are user input pointing the server at a host: only SimpleFIN Bridge is allowed unless SIMPLEFIN_ALLOWED_HOSTS says otherwise. Checked again on every request, for restored settings. - Transactions are fetched per account in 90-day windows, Bridge's limit per request. Pending ones are skipped. --- app/apps/bank_sync/forms.py | 35 +++- app/apps/bank_sync/migrations/0001_initial.py | 2 +- app/apps/bank_sync/models.py | 1 + app/apps/bank_sync/providers/__init__.py | 2 + app/apps/bank_sync/providers/base.py | 10 + app/apps/bank_sync/providers/simplefin.py | 182 ++++++++++++++++++ app/apps/bank_sync/tests/test_bank_sync.py | 138 +++++++++++++ 7 files changed, 361 insertions(+), 9 deletions(-) create mode 100644 app/apps/bank_sync/providers/simplefin.py diff --git a/app/apps/bank_sync/forms.py b/app/apps/bank_sync/forms.py index 924f0234..52d0bc85 100644 --- a/app/apps/bank_sync/forms.py +++ b/app/apps/bank_sync/forms.py @@ -41,9 +41,11 @@ class BankSyncServiceForm(forms.ModelForm): service_type = self.data.get("service_type") or self.initial.get( "service_type" ) - provider = PROVIDER_MAPPING.get(service_type) + self.provider = PROVIDER_MAPPING.get(service_type) self.setting_names = [] - for key, field in (provider.settings_fields if provider else {}).items(): + for key, field in ( + self.provider.settings_fields if self.provider else {} + ).items(): name = SETTING_PREFIX + key self.fields[name] = copy.deepcopy(field) if key in self.instance.settings: @@ -79,12 +81,29 @@ class BankSyncServiceForm(forms.ModelForm): ), ) - def save(self, commit=True): - self.instance.settings = { - name.removeprefix(SETTING_PREFIX): self.cleaned_data[name] - for name in self.setting_names - } - return super().save(commit) + def clean_sync_interval(self): + interval = self.cleaned_data["sync_interval"] + if self.provider and interval < self.provider.min_sync_interval: + raise forms.ValidationError( + _("This service can't sync more often than every %(hours)s hours.") + % {"hours": self.provider.min_sync_interval} + ) + return interval + + def clean(self): + cleaned_data = super().clean() + # Only once the rest is valid: a provider may spend a one-time token here + if self.provider and not self.errors: + try: + self.instance.settings = self.provider.clean_settings( + { + name.removeprefix(SETTING_PREFIX): cleaned_data[name] + for name in self.setting_names + } + ) + except forms.ValidationError as e: + self.add_error(None, e) + return cleaned_data class BankSyncAccountForm(forms.ModelForm): diff --git a/app/apps/bank_sync/migrations/0001_initial.py b/app/apps/bank_sync/migrations/0001_initial.py index 5f49ee9e..4fa180fc 100644 --- a/app/apps/bank_sync/migrations/0001_initial.py +++ b/app/apps/bank_sync/migrations/0001_initial.py @@ -21,7 +21,7 @@ class Migration(migrations.Migration): fields=[ ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('name', models.CharField(max_length=255, verbose_name='Name')), - ('service_type', models.CharField(choices=[('lunchflow', 'Lunch Flow')], max_length=255, verbose_name='Service Type')), + ('service_type', models.CharField(choices=[('lunchflow', 'Lunch Flow'), ('simplefin', 'SimpleFIN')], max_length=255, verbose_name='Service Type')), ('is_active', models.BooleanField(default=True, verbose_name='Active')), ('settings', models.JSONField(blank=True, default=dict, verbose_name='Settings')), ('sync_interval', models.PositiveIntegerField(default=24, help_text='Most banks only update once a day.', validators=[django.core.validators.MinValueValidator(1), django.core.validators.MaxValueValidator(168)], verbose_name='Sync every X hours')), diff --git a/app/apps/bank_sync/models.py b/app/apps/bank_sync/models.py index 079e4832..765dc118 100644 --- a/app/apps/bank_sync/models.py +++ b/app/apps/bank_sync/models.py @@ -8,6 +8,7 @@ from apps.common.models import OwnedObject, OwnedObjectManager class BankSyncService(OwnedObject): class ServiceType(models.TextChoices): LUNCHFLOW = "lunchflow", "Lunch Flow" + SIMPLEFIN = "simplefin", "SimpleFIN" name = models.CharField(max_length=255, verbose_name=_("Name")) service_type = models.CharField( diff --git a/app/apps/bank_sync/providers/__init__.py b/app/apps/bank_sync/providers/__init__.py index 1edb212d..a097ee9c 100644 --- a/app/apps/bank_sync/providers/__init__.py +++ b/app/apps/bank_sync/providers/__init__.py @@ -1,6 +1,8 @@ from apps.bank_sync.providers.lunchflow import LunchFlowProvider +from apps.bank_sync.providers.simplefin import SimpleFINProvider # Keys must match BankSyncService.ServiceType PROVIDER_MAPPING = { "lunchflow": LunchFlowProvider, + "simplefin": SimpleFINProvider, } diff --git a/app/apps/bank_sync/providers/base.py b/app/apps/bank_sync/providers/base.py index da1921c9..810a338e 100644 --- a/app/apps/bank_sync/providers/base.py +++ b/app/apps/bank_sync/providers/base.py @@ -34,10 +34,20 @@ class BankSyncProvider(ABC): """ settings_fields: dict = {} + # In hours, for services that limit how often they can be called + min_sync_interval: int = 1 def __init__(self, settings: dict): self.settings = settings or {} + @classmethod + def clean_settings(cls, settings: dict) -> dict: + """Validate or transform the settings before they're saved. + + Runs once every other form field is valid. Raise ValidationError to reject. + """ + return settings + @abstractmethod def list_accounts(self) -> list[RemoteAccount]: """Every account the credentials can see.""" diff --git a/app/apps/bank_sync/providers/simplefin.py b/app/apps/bank_sync/providers/simplefin.py new file mode 100644 index 00000000..4d0b5612 --- /dev/null +++ b/app/apps/bank_sync/providers/simplefin.py @@ -0,0 +1,182 @@ +import base64 +import binascii +import os +from datetime import date, datetime, time, timedelta +from decimal import Decimal +from urllib.parse import unquote, urlsplit, urlunsplit + +import requests +from django import forms +from django.utils import timezone +from django.utils.translation import gettext as _ +from django.utils.translation import gettext_lazy + +from apps.bank_sync.providers.base import ( + BankSyncProvider, + RemoteAccount, + RemoteTransaction, +) + +DEFAULT_ALLOWED_HOSTS = "bridge.simplefin.org,beta-bridge.simplefin.org" +# SimpleFIN Bridge refuses longer ranges in a single request +MAX_RANGE_DAYS = 90 + + +def _check_host(url: str) -> None: + # Tokens are user input: without this they could point the server at any host + allowed = { + host.strip().lower() + for host in os.getenv("SIMPLEFIN_ALLOWED_HOSTS", DEFAULT_ALLOWED_HOSTS).split(",") + if host.strip() + } + parts = urlsplit(url) + if parts.scheme not in ("http", "https") or (parts.hostname or "") not in allowed: + raise ValueError( + _("SimpleFIN server %(host)s isn't allowed on this instance.") + % {"host": parts.hostname or url} + ) + + +def _timestamp(day: date) -> int: + # Local midnight, so windows and transaction dates agree on what a day is + return int( + datetime.combine(day, time(), tzinfo=timezone.get_current_timezone()).timestamp() + ) + + +class SimpleFINProvider(BankSyncProvider): + """https://www.simplefin.org/protocol.html""" + + # SimpleFIN Bridge disables tokens that go much past ~24 requests a day, and a + # sync costs one plus one per account + min_sync_interval = 6 + + settings_fields = { + "access_url": forms.CharField( + label=gettext_lazy("Setup Token"), + widget=forms.TextInput(attrs={"autocomplete": "off"}), + help_text=gettext_lazy( + "Paste a setup token from your SimpleFIN server. It can only be " + "used once: on save it's exchanged for an access URL, which is " + "what you'll see here afterwards." + ), + ), + } + + @classmethod + def clean_settings(cls, settings: dict) -> dict: + try: + return cls._claim(settings) + except ValueError as e: + raise forms.ValidationError(str(e)) + + @staticmethod + def _claim(settings: dict) -> dict: + value = settings["access_url"].strip() + # Already claimed: the form is being saved again + if not value.startswith(("http://", "https://")): + try: + claim_url = base64.b64decode( + value + "=" * (-len(value) % 4), validate=True + ).decode() + except (binascii.Error, UnicodeDecodeError): + raise ValueError(_("This isn't a valid SimpleFIN setup token.")) + _check_host(claim_url) + try: + response = requests.post(claim_url, timeout=30) + response.raise_for_status() + except requests.RequestException as e: + raise ValueError( + _( + "Couldn't claim the setup token, it may have been used " + "already: %(error)s" + ) + % {"error": e} + ) + value = response.text.strip() + _check_host(value) + return {**settings, "access_url": value} + + def __init__(self, settings: dict): + super().__init__(settings) + parts = urlsplit(self.settings.get("access_url", "")) + self.session = requests.Session() + # Credentials go in a header, not the URL, so errors never show them + self.session.auth = (unquote(parts.username or ""), unquote(parts.password or "")) + netloc = (parts.hostname or "") + (f":{parts.port}" if parts.port else "") + self.base_url = urlunsplit((parts.scheme, netloc, parts.path.rstrip("/"), "", "")) + + def _get_accounts(self, **params) -> dict: + # Settings can also arrive through a backup restore, skipping the form + _check_host(self.base_url) + response = self.session.get( + f"{self.base_url}/accounts", + params={"version": "2", **params}, + # Slow: the server can fetch from every bank on each request + timeout=60, + ) + response.raise_for_status() + return response.json() + + def list_accounts(self) -> list[RemoteAccount]: + data = self._get_accounts(**{"balances-only": "1"}) + connections = {c["conn_id"]: c.get("name", "") for c in data.get("connections", [])} + account_errors, connection_errors = {}, {} + for error in data.get("errlist", []): + if error.get("account_id"): + account_errors.setdefault(error["account_id"], error.get("msg", "")) + elif error.get("conn_id"): + connection_errors.setdefault(error["conn_id"], error.get("msg", "")) + + accounts = [] + for account in data["accounts"]: + institution = connections.get(account.get("conn_id")) or ( + account.get("org") or {} + ).get("name") + currency = account.get("currency") or "" + accounts.append( + RemoteAccount( + id=str(account["id"]), + name=" - ".join(filter(None, [institution, account["name"]])), + # Custom currencies are URLs, nothing to match them against + currencies=(currency.upper(),) if currency and "/" not in currency else (), + error=account_errors.get(account["id"]) + or connection_errors.get(account.get("conn_id"), ""), + ) + ) + return accounts + + def get_transactions( + self, account_id: str, start_date: date + ) -> list[RemoteTransaction]: + transactions = [] + start, stop = start_date, timezone.localdate() + timedelta(days=1) + while start < stop: + end = min(start + timedelta(days=MAX_RANGE_DAYS), stop) + data = self._get_accounts( + account=account_id, + **{"start-date": _timestamp(start), "end-date": _timestamp(end)}, + ) + for account in data["accounts"]: + if str(account["id"]) != account_id: + continue + currency = account.get("currency") or "" + for tx in account.get("transactions", []): + if tx.get("pending"): + continue + payee = (tx.get("payee") or "").strip() + description = (tx.get("description") or "").strip() + transactions.append( + RemoteTransaction( + id=str(tx["id"]), + date=datetime.fromtimestamp( + tx["posted"], tz=timezone.get_current_timezone() + ).date(), + amount=Decimal(str(tx["amount"])), + description=payee or description, + notes=description if payee and description != payee else "", + currency=currency.upper() if "/" not in currency else "", + ) + ) + start = end + return transactions diff --git a/app/apps/bank_sync/tests/test_bank_sync.py b/app/apps/bank_sync/tests/test_bank_sync.py index 346bef10..104f01da 100644 --- a/app/apps/bank_sync/tests/test_bank_sync.py +++ b/app/apps/bank_sync/tests/test_bank_sync.py @@ -1,3 +1,4 @@ +import base64 import io import os import zipfile @@ -6,6 +7,7 @@ from decimal import Decimal from unittest.mock import MagicMock, patch from django.contrib.auth import get_user_model +from django.core.exceptions import ValidationError from django.core.files.uploadedfile import SimpleUploadedFile from django.test import TestCase from django.urls import reverse @@ -16,6 +18,8 @@ from apps.bank_sync.forms import BankSyncAccountForm, BankSyncServiceForm from apps.bank_sync.models import BankSyncAccount, BankSyncService from apps.bank_sync.providers.base import RemoteAccount, RemoteTransaction from apps.bank_sync.providers.lunchflow import LunchFlowProvider +from apps.bank_sync.providers.simplefin import SimpleFINProvider +from apps.bank_sync.providers.simplefin import _timestamp as _ts from apps.bank_sync.sync import is_due, refresh_remote_accounts, sync_service from apps.common.middleware.thread_local import delete_current_user, write_current_user from apps.currencies.models import Currency @@ -96,6 +100,115 @@ class LunchFlowProviderTests(TestCase): ) +class SimpleFINProviderTests(TestCase): + def setUp(self): + self.enterContext(patch.dict("os.environ")) + os.environ.pop("SIMPLEFIN_ALLOWED_HOSTS", None) + self.provider = SimpleFINProvider( + {"access_url": "https://us%40r:p%3Ass@bridge.simplefin.org/simplefin/"} + ) + + def test_credentials_are_kept_out_of_the_url(self): + self.assertEqual(self.provider.session.auth, ("us@r", "p:ss")) + self.assertEqual(self.provider.base_url, "https://bridge.simplefin.org/simplefin") + + def test_claims_setup_token(self): + token = base64.b64encode(b"https://bridge.simplefin.org/simplefin/claim/abc").decode() + response = MagicMock(text="https://u:p@bridge.simplefin.org/simplefin\n") + with patch("requests.post", return_value=response) as post: + settings = SimpleFINProvider.clean_settings({"access_url": token}) + post.assert_called_once_with( + "https://bridge.simplefin.org/simplefin/claim/abc", timeout=30 + ) + self.assertEqual(settings, {"access_url": "https://u:p@bridge.simplefin.org/simplefin"}) + + def test_claimed_url_is_not_claimed_again(self): + with patch("requests.post") as post: + SimpleFINProvider.clean_settings({"access_url": "https://u:p@bridge.simplefin.org/x"}) + post.assert_not_called() + + def test_rejects_hosts_not_allowed(self): + token = base64.b64encode(b"http://db:5432/claim").decode() + with patch("requests.post") as post: + for value in (token, "https://u:p@evil.test/x", "not base64!"): + with self.assertRaises(ValidationError): + SimpleFINProvider.clean_settings({"access_url": value}) + post.assert_not_called() + with patch.dict("os.environ", {"SIMPLEFIN_ALLOWED_HOSTS": "sfin.local"}): + SimpleFINProvider.clean_settings({"access_url": "http://u:p@sfin.local/x"}) + # Restored settings skip the form, the request itself is checked too + provider = SimpleFINProvider({"access_url": "https://u:p@evil.test/x"}) + with self.assertRaises(ValueError): + provider.list_accounts() + + def test_list_accounts(self): + payload = { + "errlist": [ + {"code": "con.auth", "msg": "Reconnect", "conn_id": "c2"}, + {"code": "gen.api", "msg": "Slow down"}, + ], + "connections": [{"conn_id": "c1", "name": "Bank"}, {"conn_id": "c2", "name": "Other"}], + "accounts": [ + {"id": "a1", "name": "Checking", "conn_id": "c1", "currency": "usd"}, + {"id": "a2", "name": "Card", "conn_id": "c2", "currency": "https://x.test/pts"}, + ], + } + with patch.object(self.provider.session, "get", return_value=_response(payload)) as get: + accounts = self.provider.list_accounts() + get.assert_called_once_with( + "https://bridge.simplefin.org/simplefin/accounts", + params={"version": "2", "balances-only": "1"}, + timeout=60, + ) + self.assertEqual( + accounts, + [ + RemoteAccount("a1", "Bank - Checking", ("USD",), ""), + RemoteAccount("a2", "Other - Card", (), "Reconnect"), + ], + ) + + def test_get_transactions_in_90_day_windows(self): + today = timezone.localdate() + start = today - timedelta(days=100) + + def get(url, params, timeout): + if params["start-date"] != _ts(start): + return _response({"accounts": []}) + return _response( + { + "accounts": [ + { + "id": "a1", + "currency": "USD", + "transactions": [ + {"id": "t1", "posted": _ts(start) + 3600, "amount": "-5.10", "description": "CARD COFFEE", "payee": "Coffee"}, + {"id": "t2", "posted": 0, "amount": "-1", "description": "Pending", "pending": True}, + ], + }, + {"id": "other", "transactions": [{"id": "x", "posted": 0, "amount": "1"}]}, + ] + } + ) + + with patch.object(self.provider.session, "get", side_effect=get) as mock_get: + transactions = self.provider.get_transactions("a1", start) + + windows = [(c.kwargs["params"]["start-date"], c.kwargs["params"]["end-date"]) for c in mock_get.call_args_list] + self.assertEqual( + windows, + [ + (_ts(start), _ts(start + timedelta(days=90))), + (_ts(start + timedelta(days=90)), _ts(today + timedelta(days=1))), + ], + ) + self.assertEqual(mock_get.call_args.kwargs["params"]["account"], "a1") + self.assertEqual( + transactions, + [RemoteTransaction("t1", start, Decimal("-5.10"), "Coffee", "CARD COFFEE", "USD")], + ) + + class FakeProvider: def __init__(self, transactions=(), accounts=()): self.transactions = list(transactions) @@ -401,6 +514,31 @@ class FormTests(TestCase): self.assertFalse(form.is_valid()) self.assertIn("setting_api_key", form.errors) + def test_provider_minimum_sync_interval(self): + data = {"name": "SF", "service_type": "simplefin", "setting_access_url": "x"} + with patch.object(SimpleFINProvider, "clean_settings", side_effect=lambda s: s): + form = BankSyncServiceForm({**data, "sync_interval": "5"}) + self.assertIn("sync_interval", form.errors) + self.assertTrue(BankSyncServiceForm({**data, "sync_interval": "6"}).is_valid()) + + @patch.object(SimpleFINProvider, "clean_settings") + def test_clean_settings_runs_last_and_its_errors_show(self, clean_settings): + data = {"service_type": "simplefin", "sync_interval": "24", "setting_access_url": "tok"} + # Another field is invalid: a one-time token must not be spent + self.assertFalse(BankSyncServiceForm(data).is_valid()) + clean_settings.assert_not_called() + + clean_settings.side_effect = ValidationError("Token used") + form = BankSyncServiceForm({**data, "name": "SF"}) + self.assertFalse(form.is_valid()) + self.assertEqual(form.non_field_errors(), ["Token used"]) + + clean_settings.side_effect = None + clean_settings.return_value = {"access_url": "https://u:p@bridge.simplefin.org/x"} + form = BankSyncServiceForm({**data, "name": "SF"}) + self.assertTrue(form.is_valid()) + self.assertEqual(form.save().settings, clean_settings.return_value) + def test_account_link_form(self): eur = Currency.objects.create(code="€", name="Euro") service = BankSyncService.objects.create(name="LF", service_type="lunchflow")