feat(enable_banking): support MFA/decoupled banks and harden session handling (#2174)

Decoupled/MFA banks (e.g. VR Bank in Holstein) were hard-blocked because the
authorize flow aborted whenever auth_methods[0] was DECOUPLED. Enable Banking's
hosted /auth page actually coordinates decoupled SCA and redirects back with a
code, so route these banks through it instead:

- Provider#start_authorization accepts and forwards an auth_method param
- EnableBankingItem#select_auth_method picks the best method
  (REDIRECT > DECOUPLED > EMBEDDED), filtering by psu_type and skipping hidden
  methods
- Shared begin_authorization! re-fetches ASPSP metadata on each authorize and
  reauthorize, so the method is always re-derived (no persistence required)
- Remove the DECOUPLED block in the controller

Also stop the integration from constantly reporting "session expired":

- Only a session-level GET /sessions 401/404 flips the connection to
  requires_update; per-account 401/404 are retried and no longer kill the
  whole connection
- Reconcile session_expires_at from the API's access.valid_until on every sync
- Treat an expired session as a graceful requires_update state instead of
  raising a bare error

No schema changes. Adds covering tests.

test/models/enable_banking_item/importer_error_handling_test.rb

@@ -19,22 +19,43 @@ class EnableBankingItem::ImporterErrorHandlingTest < ActiveSupport::TestCase
     @importer = EnableBankingItem::Importer.new(@enable_banking_item, enable_banking_provider: @mock_provider)
   end
 
-  test "handle_sync_error handles unauthorized EnableBankingError" do
+  # Session-level auth failures (the top-level GET /sessions call) mean the consent
+  # is genuinely dead and the user must re-authorize.
+  test "handle_sync_error with session_level flips requires_update on unauthorized" do
     error = Provider::EnableBanking::EnableBankingError.new("Unauthorized", :unauthorized)
-    message = @importer.send(:handle_sync_error, error)
+    message = @importer.send(:handle_sync_error, error, session_level: true)
 
     assert_equal I18n.t("enable_banking_items.errors.session_invalid"), message
     assert @enable_banking_item.reload.requires_update?
   end
 
-  test "handle_sync_error handles not_found EnableBankingError" do
+  test "handle_sync_error with session_level flips requires_update on not_found" do
     error = Provider::EnableBanking::EnableBankingError.new("Not Found", :not_found)
-    message = @importer.send(:handle_sync_error, error)
+    message = @importer.send(:handle_sync_error, error, session_level: true)
 
     assert_equal I18n.t("enable_banking_items.errors.session_invalid"), message
     assert @enable_banking_item.reload.requires_update?
   end
 
+  # Per-account auth failures (a stale account UID, a transient hiccup on one
+  # account) must NOT kill the whole connection — that is what made every sync
+  # report "session expired". They surface as ordinary api errors and retry.
+  test "handle_sync_error per-account unauthorized does not flip requires_update" do
+    error = Provider::EnableBanking::EnableBankingError.new("Unauthorized", :unauthorized)
+    message = @importer.send(:handle_sync_error, error)
+
+    assert_equal I18n.t("enable_banking_items.errors.api_error"), message
+    assert_not @enable_banking_item.reload.requires_update?
+  end
+
+  test "handle_sync_error per-account not_found does not flip requires_update" do
+    error = Provider::EnableBanking::EnableBankingError.new("Not Found", :not_found)
+    message = @importer.send(:handle_sync_error, error)
+
+    assert_equal I18n.t("enable_banking_items.errors.api_error"), message
+    assert_not @enable_banking_item.reload.requires_update?
+  end
+
   test "handle_sync_error handles other EnableBankingError as api_error" do
     error = Provider::EnableBanking::EnableBankingError.new("Some API error", :internal_server_error)
     message = @importer.send(:handle_sync_error, error)
@@ -53,14 +74,24 @@ class EnableBankingItem::ImporterErrorHandlingTest < ActiveSupport::TestCase
     assert @enable_banking_item.reload.requires_update?
   end
 
-  test "fetch_and_store_transactions updates status to requires_update on unauthorized error" do
+  test "fetch_session_data reconciles session_expires_at from API access.valid_until" do
+    new_expiry = 45.days.from_now.change(usec: 0)
+    @mock_provider.stubs(:get_session).returns({ access: { valid_until: new_expiry.iso8601 } })
+
+    @importer.send(:fetch_session_data)
+
+    assert_equal new_expiry.to_i, @enable_banking_item.reload.session_expires_at.to_i
+  end
+
+  test "fetch_and_store_transactions does not flip whole connection on per-account unauthorized error" do
     enable_banking_account = EnableBankingAccount.new(uid: "test_uid")
     @importer.stubs(:determine_sync_start_date).returns(Date.today)
     @importer.expects(:fetch_paginated_transactions).raises(Provider::EnableBanking::EnableBankingError.new("Unauthorized", :unauthorized))
 
-    @importer.send(:fetch_and_store_transactions, enable_banking_account)
+    result = @importer.send(:fetch_and_store_transactions, enable_banking_account)
 
-    assert @enable_banking_item.reload.requires_update?
+    assert_not result[:success]
+    assert_not @enable_banking_item.reload.requires_update?
   end
 
   test "fetch_and_store_transactions succeeds and skips pending when ASPSP rejects PDNG transaction_status" do
@@ -99,14 +130,15 @@ class EnableBankingItem::ImporterErrorHandlingTest < ActiveSupport::TestCase
     assert_not result[:success]
   end
 
-  test "fetch_and_update_balance updates status to requires_update on unauthorized error" do
+  test "fetch_and_update_balance does not flip whole connection on per-account unauthorized error" do
     enable_banking_account = EnableBankingAccount.new(uid: "test_uid")
     def @mock_provider.get_account_balances(**args)
       raise Provider::EnableBanking::EnableBankingError.new("Unauthorized", :unauthorized)
     end
 
-    @importer.send(:fetch_and_update_balance, enable_banking_account)
+    result = @importer.send(:fetch_and_update_balance, enable_banking_account)
 
-    assert @enable_banking_item.reload.requires_update?
+    assert_not result
+    assert_not @enable_banking_item.reload.requires_update?
   end
 end

test/models/enable_banking_item/syncer_test.rb

@@ -0,0 +1,31 @@
+require "test_helper"
+
+class EnableBankingItem::SyncerTest < ActiveSupport::TestCase
+  setup do
+    @item = EnableBankingItem.create!(
+      family: families(:dylan_family),
+      name: "Test",
+      country_code: "DE",
+      application_id: "app",
+      client_certificate: "cert",
+      session_id: "sess",
+      session_expires_at: 1.day.ago, # expired
+      status: :good
+    )
+    @syncer = EnableBankingItem::Syncer.new(@item)
+  end
+
+  test "expired session marks requires_update and finishes gracefully without raising" do
+    sync = Sync.create!(syncable: @item)
+
+    assert_nothing_raised do
+      @syncer.perform_sync(sync)
+    end
+
+    assert @item.reload.requires_update?
+
+    stats = sync.reload.sync_stats || {}
+    assert_equal 0, (stats["total_errors"] || 0),
+      "Expired session should be a graceful reconnect state, not a red sync error"
+  end
+end

test/models/enable_banking_item_test.rb

@@ -0,0 +1,106 @@
+require "test_helper"
+
+class EnableBankingItemTest < ActiveSupport::TestCase
+  setup do
+    @item = EnableBankingItem.new(
+      family: families(:dylan_family),
+      name: "Test",
+      country_code: "DE",
+      application_id: "app",
+      client_certificate: "cert"
+    )
+  end
+
+  test "select_auth_method prefers REDIRECT over DECOUPLED and EMBEDDED" do
+    aspsp = {
+      auth_methods: [
+        { name: "decoupled_app", approach: "DECOUPLED" },
+        { name: "redirect_web", approach: "REDIRECT" },
+        { name: "embedded_form", approach: "EMBEDDED" }
+      ]
+    }.with_indifferent_access
+
+    selected = @item.send(:select_auth_method, aspsp, "personal")
+
+    assert_equal "redirect_web", selected[:name]
+    assert_equal "REDIRECT", selected[:approach]
+  end
+
+  test "select_auth_method falls back to DECOUPLED when no REDIRECT exists" do
+    aspsp = {
+      auth_methods: [
+        { name: "embedded_form", approach: "EMBEDDED" },
+        { name: "decoupled_app", approach: "DECOUPLED" }
+      ]
+    }.with_indifferent_access
+
+    selected = @item.send(:select_auth_method, aspsp, "personal")
+
+    assert_equal "decoupled_app", selected[:name]
+    assert_equal "DECOUPLED", selected[:approach]
+  end
+
+  test "select_auth_method filters by psu_type when methods declare one" do
+    aspsp = {
+      auth_methods: [
+        { name: "business_redirect", approach: "REDIRECT", psu_type: "business" },
+        { name: "personal_decoupled", approach: "DECOUPLED", psu_type: "personal" }
+      ]
+    }.with_indifferent_access
+
+    selected = @item.send(:select_auth_method, aspsp, "personal")
+
+    assert_equal "personal_decoupled", selected[:name]
+  end
+
+  test "select_auth_method ignores hidden methods" do
+    aspsp = {
+      auth_methods: [
+        { name: "hidden_redirect", approach: "REDIRECT", hidden_method: true },
+        { name: "decoupled_app", approach: "DECOUPLED" }
+      ]
+    }.with_indifferent_access
+
+    selected = @item.send(:select_auth_method, aspsp, "personal")
+
+    assert_equal "decoupled_app", selected[:name]
+  end
+
+  test "select_auth_method returns nil when no auth methods present" do
+    assert_nil @item.send(:select_auth_method, { auth_methods: [] }.with_indifferent_access, "personal")
+  end
+
+  test "select_auth_method returns nil when every method is hidden" do
+    aspsp = {
+      auth_methods: [
+        { name: "hidden_a", approach: "REDIRECT", hidden_method: true },
+        { name: "hidden_b", approach: "DECOUPLED", hidden_method: true }
+      ]
+    }.with_indifferent_access
+
+    # All methods hidden -> fall back to the ASPSP default rather than forcing one.
+    assert_nil @item.send(:select_auth_method, aspsp, "personal")
+  end
+
+  test "reconcile_session_expiry! updates session_expires_at from access.valid_until" do
+    @item.session_id = "sess"
+    @item.session_expires_at = 1.day.from_now
+    @item.save!
+    new_expiry = 60.days.from_now.change(usec: 0)
+
+    @item.reconcile_session_expiry!({ access: { valid_until: new_expiry.iso8601 } })
+
+    assert_equal new_expiry.to_i, @item.reload.session_expires_at.to_i
+  end
+
+  test "reconcile_session_expiry! is a no-op when valid_until is missing" do
+    @item.session_id = "sess"
+    original = 1.day.from_now.change(usec: 0)
+    @item.session_expires_at = original
+    @item.save!
+
+    @item.reconcile_session_expiry!({ access: {} })
+
+    assert_equal original.to_i, @item.reload.session_expires_at.to_i
+  end
+end

test/models/provider/enable_banking_test.rb

@@ -61,4 +61,47 @@ class Provider::EnableBankingTest < ActiveSupport::TestCase
     assert_equal Date.new(2026, 1, 17), error.corrected_date_from
     assert error.wrong_transactions_period?
   end
+
+  test "start_authorization includes auth_method in the request body when provided" do
+    captured_body = nil
+    response = OpenStruct.new(
+      code: 200,
+      body: { url: "https://api.enablebanking.com/auth/abc", authorization_id: "auth_1" }.to_json
+    )
+
+    Provider::EnableBanking.expects(:post).with do |_url, options|
+      captured_body = JSON.parse(options[:body])
+      true
+    end.returns(response)
+
+    @provider.start_authorization(
+      aspsp_name: "VR Bank in Holstein",
+      aspsp_country: "DE",
+      redirect_url: "https://app.example.com/callback",
+      auth_method: "decoupled_app"
+    )
+
+    assert_equal "decoupled_app", captured_body["auth_method"]
+  end
+
+  test "start_authorization omits auth_method when not provided" do
+    captured_body = nil
+    response = OpenStruct.new(
+      code: 200,
+      body: { url: "https://api.enablebanking.com/auth/abc", authorization_id: "auth_1" }.to_json
+    )
+
+    Provider::EnableBanking.expects(:post).with do |_url, options|
+      captured_body = JSON.parse(options[:body])
+      true
+    end.returns(response)
+
+    @provider.start_authorization(
+      aspsp_name: "ING-DiBa AG",
+      aspsp_country: "DE",
+      redirect_url: "https://app.example.com/callback"
+    )
+
+    assert_not captured_body.key?("auth_method")
+  end
 end