From fe47c918bbc8d4a9d26b0c7e5de40f1452363c54 Mon Sep 17 00:00:00 2001 From: Tobias Rahloff Date: Thu, 4 Jun 2026 19:53:52 +0200 Subject: [PATCH] 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. --- .../enable_banking_items_controller.rb | 45 ++------ app/models/enable_banking_item.rb | 109 +++++++++++++++++- app/models/enable_banking_item/importer.rb | 22 +++- app/models/enable_banking_item/syncer.rb | 17 ++- app/models/provider/enable_banking.rb | 6 +- .../locales/views/enable_banking_items/ca.yml | 2 - .../locales/views/enable_banking_items/en.yml | 1 - .../locales/views/enable_banking_items/fr.yml | 1 - .../locales/views/enable_banking_items/hu.yml | 1 - .../locales/views/enable_banking_items/vi.yml | 1 - .../views/enable_banking_items/zh-CN.yml | 1 - .../enable_banking_items_controller_test.rb | 24 ++++ .../importer_error_handling_test.rb | 52 +++++++-- .../models/enable_banking_item/syncer_test.rb | 31 +++++ test/models/enable_banking_item_test.rb | 106 +++++++++++++++++ test/models/provider/enable_banking_test.rb | 43 +++++++ 16 files changed, 393 insertions(+), 69 deletions(-) create mode 100644 test/models/enable_banking_item/syncer_test.rb create mode 100644 test/models/enable_banking_item_test.rb diff --git a/app/controllers/enable_banking_items_controller.rb b/app/controllers/enable_banking_items_controller.rb index 0bfc0dabb..901d0589c 100644 --- a/app/controllers/enable_banking_items_controller.rb +++ b/app/controllers/enable_banking_items_controller.rb @@ -131,39 +131,6 @@ class EnableBankingItemsController < ApplicationController return end - # Re-fetch ASPSP list from provider to avoid session cookie overflow. - # We do not store full ASPSP metadata in the session to stay within the 4KB limit; - # instead, we re-query the provider here for the final authorization parameters. - aspsp_data = nil - begin - provider_for_lookup = @enable_banking_item.enable_banking_provider - if provider_for_lookup - response = provider_for_lookup.get_aspsps(country: @enable_banking_item.country_code) - raw_aspsps = response[:aspsps] || response["aspsps"] || [] - found = raw_aspsps.find { |a| a[:name] == aspsp_name || a["name"] == aspsp_name } - aspsp_data = found&.with_indifferent_access - end - rescue Provider::EnableBanking::EnableBankingError => e - Rails.logger.warn "Enable Banking: could not fetch ASPSP metadata in authorize: #{e.message}" - end - - # Block DECOUPLED banks — our OAuth redirect flow doesn't support them - if aspsp_data.present? - # Adjust psu_type if the bank does not support the requested type - supported_types = Array(aspsp_data[:psu_types]).map(&:to_s) - if supported_types.any? && !supported_types.include?(psu_type) - psu_type = supported_types.first - end - - first_method = Array(aspsp_data[:auth_methods]).first - approach = first_method&.dig(:approach) || first_method&.dig("approach") - if approach == "DECOUPLED" - redirect_to settings_providers_path, alert: t(".decoupled_not_supported", - default: "This bank uses a separate device authentication method which is not yet supported. Please add this account manually.") - return - end - end - begin target_item = if params[:new_connection] == "true" Current.family.enable_banking_items.create!( @@ -181,12 +148,14 @@ class EnableBankingItemsController < ApplicationController language = I18n.locale.to_s.split("-").first - redirect_url = target_item.start_authorization( + # begin_authorization! re-fetches ASPSP metadata and auto-selects the best + # auth method (REDIRECT > DECOUPLED > EMBEDDED). Decoupled/MFA banks proceed + # through Enable Banking's hosted SCA page rather than being blocked. + redirect_url = target_item.begin_authorization!( aspsp_name: aspsp_name, redirect_url: enable_banking_callback_url, state: target_item.id, psu_type: psu_type, - aspsp_data: aspsp_data, language: language ) @@ -269,11 +238,11 @@ class EnableBankingItemsController < ApplicationController begin language = I18n.locale.to_s.split("-").first - redirect_url = @enable_banking_item.start_authorization( - aspsp_name: @enable_banking_item.aspsp_name, + # Route through the shared path so reauthorization re-selects the same auth + # method (decoupled banks included) instead of falling back to a default. + redirect_url = @enable_banking_item.begin_authorization!( redirect_url: enable_banking_callback_url, state: @enable_banking_item.id, - psu_type: @enable_banking_item.psu_type || "personal", language: language ) diff --git a/app/models/enable_banking_item.rb b/app/models/enable_banking_item.rb index 7f3b6a540..87af6ec78 100644 --- a/app/models/enable_banking_item.rb +++ b/app/models/enable_banking_item.rb @@ -73,19 +73,31 @@ class EnableBankingItem < ApplicationRecord raise StandardError.new("Enable Banking provider is not configured") unless provider validated_psu_type = psu_type + selected_method = nil # Store ASPSP metadata before calling provider so it's available even if auth fails if aspsp_data.present? aspsp_data = aspsp_data.with_indifferent_access - first_auth_method = aspsp_data.dig(:auth_methods, 0) || aspsp_data.dig("auth_methods", 0) - aspsp_types = aspsp_data[:psu_types] || [] + aspsp_types = Array(aspsp_data[:psu_types]).map(&:to_s) + + # If the requested PSU type isn't supported by this ASPSP, fall back to the + # first type it advertises rather than failing outright. + validated_psu_type = if psu_type.present? && aspsp_types.include?(psu_type) + psu_type + elsif aspsp_types.any? + aspsp_types.first + else + psu_type + end + + selected_method = select_auth_method(aspsp_data, validated_psu_type) + update!( aspsp_required_psu_headers: aspsp_data[:required_psu_headers] || [], aspsp_maximum_consent_validity: aspsp_data[:maximum_consent_validity], - aspsp_auth_approach: first_auth_method&.dig(:approach) || first_auth_method&.dig("approach"), + aspsp_auth_approach: selected_method&.dig(:approach), aspsp_psu_types: aspsp_types ) - validated_psu_type = psu_type.present? && aspsp_types.include?(psu_type) ? psu_type : nil end result = provider.start_authorization( @@ -95,7 +107,8 @@ class EnableBankingItem < ApplicationRecord state: state, psu_type: validated_psu_type, maximum_consent_validity: aspsp_maximum_consent_validity, - language: language + language: language, + auth_method: selected_method&.dig(:name) ) attributes = { @@ -109,6 +122,26 @@ class EnableBankingItem < ApplicationRecord result[:url] end + # Shared entry point for both initial authorization and reauthorization. + # Re-fetches ASPSP metadata (so the auth method / PSU type selection and the + # stored approach stay accurate) and starts the provider authorization. The + # re-fetch — rather than caching the full ASPSP object in the session — keeps + # us under the 4KB session cookie limit. + # @return [String] Redirect URL for the user + def begin_authorization!(redirect_url:, state:, language: nil, psu_type: nil, aspsp_name: nil) + name = aspsp_name.presence || self.aspsp_name + raise StandardError.new("No bank selected for this connection") if name.blank? + + start_authorization( + aspsp_name: name, + redirect_url: redirect_url, + state: state, + psu_type: psu_type.presence || self.psu_type || "personal", + aspsp_data: fetch_aspsp_data(name), + language: language + ) + end + # Complete the authorization flow with the code from callback def complete_authorization(code:) provider = enable_banking_provider @@ -130,6 +163,27 @@ class EnableBankingItem < ApplicationRecord result end + # Reconcile the locally-stored session expiry with what the API reports. + # The session info returned by GET /sessions carries the authoritative + # access.valid_until; persisting it on every sync keeps session_valid? accurate + # and avoids both premature "expired" states and stale "still valid" states. + def reconcile_session_expiry!(session_data) + return unless session_data.is_a?(Hash) + + valid_until = session_data.dig(:access, :valid_until) || session_data.dig("access", "valid_until") + return if valid_until.blank? + + parsed = Time.zone.parse(valid_until.to_s) + return if parsed.nil? || parsed == session_expires_at + + update!(session_expires_at: parsed) + rescue ArgumentError, TypeError, ActiveRecord::ActiveRecordError => e + # Best-effort reconciliation: swallow bad timestamps (ArgumentError/TypeError) + # as well as validation/locking failures from update! (RecordInvalid, + # StaleObjectError) so a sync is never derailed by expiry bookkeeping. + Rails.logger.warn "EnableBankingItem #{id} - Failed to reconcile session expiry: #{e.message}" + end + def import_latest_enable_banking_data provider = enable_banking_provider unless provider @@ -288,6 +342,51 @@ class EnableBankingItem < ApplicationRecord private + # Authentication approach preference, lowest number wins. + # REDIRECT is the smoothest (PSU authenticates entirely on the ASPSP page). + # DECOUPLED works through Enable Banking's hosted page (push-to-app / photoTAN + # / chipTAN). EMBEDDED is last resort (handled by the hosted page too). + AUTH_APPROACH_PRIORITY = { "REDIRECT" => 0, "DECOUPLED" => 1, "EMBEDDED" => 2 }.freeze + + # Choose the best authentication method for the given PSU type. + # Returns a hash with :name and :approach, or nil when the ASPSP exposes no + # API-selectable methods (Enable Banking then falls back to its default). + def select_auth_method(aspsp_data, psu_type) + methods = Array(aspsp_data[:auth_methods]).map(&:with_indifferent_access) + return nil if methods.empty? + + # Hidden methods aren't surfaced on Enable Banking's hosted page, so we don't + # auto-select one (the PSU couldn't complete it). If every method is hidden, + # return nil and let /auth fall back to the ASPSP's default rather than + # forcing a non-selectable method. + methods = methods.reject { |m| ActiveModel::Type::Boolean.new.cast(m[:hidden_method]) } + return nil if methods.empty? + + # Prefer methods that match the chosen PSU type; if none declare a psu_type + # (or none match), consider all of them. + matching = methods.select { |m| m[:psu_type].blank? || m[:psu_type].to_s == psu_type.to_s } + candidates = matching.presence || methods + + best = candidates.min_by { |m| AUTH_APPROACH_PRIORITY.fetch(m[:approach].to_s, 99) } + return nil unless best + + { name: best[:name], approach: best[:approach] } + end + + # Fetch the ASPSP object for a given name from the provider's /aspsps list. + # Returns a HashWithIndifferentAccess, or nil if unavailable. + def fetch_aspsp_data(aspsp_name) + provider = enable_banking_provider + return nil unless provider + + response = provider.get_aspsps(country: country_code) + raw_aspsps = response[:aspsps] || response["aspsps"] || [] + raw_aspsps.find { |a| (a[:name] || a["name"]) == aspsp_name }&.with_indifferent_access + rescue Provider::EnableBanking::EnableBankingError => e + Rails.logger.warn "EnableBankingItem #{id} - could not fetch ASPSP metadata for #{aspsp_name}: #{e.message}" + nil + end + def parse_session_expiry(session_result) if session_result[:access].present? && session_result[:access][:valid_until].present? parsed = Time.zone.parse(session_result[:access][:valid_until]) diff --git a/app/models/enable_banking_item/importer.rb b/app/models/enable_banking_item/importer.rb index 696b4f673..d08f0c5f5 100644 --- a/app/models/enable_banking_item/importer.rb +++ b/app/models/enable_banking_item/importer.rb @@ -124,14 +124,20 @@ class EnableBankingItem::Importer private - def handle_sync_error(exception) + # @param session_level [Boolean] true only for the top-level GET /sessions call. + # A session-level 401/404 means the consent is genuinely dead and the user + # must re-authorize. Per-account 401/404 (a stale account UID, a transient + # hiccup on one account) must NOT mark the whole connection requires_update — + # doing so is what made every sync report "session expired". Those are recorded + # as ordinary sync errors and retried on the next sync. + def handle_sync_error(exception, session_level: false) # Check the underlying cause first, then the exception itself exceptions = [ exception.cause, exception ].compact provider_error = exceptions.find { |ex| ex.is_a?(Provider::EnableBanking::EnableBankingError) } - # Handle session expiration status update - if provider_error && [ :unauthorized, :not_found ].include?(provider_error.error_type) + # Handle session expiration status update (session-level failures only) + if session_level && provider_error && [ :unauthorized, :not_found ].include?(provider_error.error_type) enable_banking_item.update!(status: :requires_update) return I18n.t("enable_banking_items.errors.session_invalid") end @@ -151,14 +157,18 @@ class EnableBankingItem::Importer end def fetch_session_data - enable_banking_provider.get_session(session_id: enable_banking_item.session_id) + session_data = enable_banking_provider.get_session(session_id: enable_banking_item.session_id) + # Keep the local expiry in sync with the authoritative value from the API so + # session_valid? doesn't drift (premature "expired" or stale "still valid"). + enable_banking_item.reconcile_session_expiry!(session_data) + session_data rescue Provider::EnableBanking::EnableBankingError => e Rails.logger.error "EnableBankingItem::Importer - Enable Banking API error: #{e.message}" - @session_error = handle_sync_error(e) + @session_error = handle_sync_error(e, session_level: true) nil rescue => e Rails.logger.error "EnableBankingItem::Importer - Unexpected error fetching session: #{e.class} - #{e.message}" - @session_error = handle_sync_error(e) + @session_error = handle_sync_error(e, session_level: true) nil end diff --git a/app/models/enable_banking_item/syncer.rb b/app/models/enable_banking_item/syncer.rb index dfe8ce984..3049a05eb 100644 --- a/app/models/enable_banking_item/syncer.rb +++ b/app/models/enable_banking_item/syncer.rb @@ -8,11 +8,14 @@ class EnableBankingItem::Syncer end def perform_sync(sync) - # Check if session is valid before syncing + # An expired/missing session is an expected state that needs user action, not a + # hard failure. Mark the connection requires_update and finish the sync + # gracefully so the UI surfaces the "Reconnect" CTA instead of a red sync error. unless enable_banking_item.session_valid? sync.update!(status_text: "Session expired - re-authorization required") if sync.respond_to?(:status_text) enable_banking_item.update!(status: :requires_update) - raise StandardError.new("Enable Banking session has expired. Please re-authorize.") + collect_health_stats(sync, errors: nil) + return end # Phase 1: Import data from Enable Banking API @@ -20,6 +23,16 @@ class EnableBankingItem::Syncer import_result = enable_banking_item.import_latest_enable_banking_data unless import_result[:success] + # A session-level auth failure detected mid-import flips the item to + # requires_update — surface that as a graceful reconnect state, not a red + # error. Transient/per-account failures leave status good and fall through + # to a normal sync error that retries next time. + if enable_banking_item.requires_update? + sync.update!(status_text: "Re-authorization required") if sync.respond_to?(:status_text) + collect_health_stats(sync, errors: nil) + return + end + error_msg = import_result[:error] if error_msg.blank? && (import_result[:accounts_failed].to_i > 0 || import_result[:transactions_failed].to_i > 0) parts = [] diff --git a/app/models/provider/enable_banking.rb b/app/models/provider/enable_banking.rb index 88df7ec62..da6a2ea06 100644 --- a/app/models/provider/enable_banking.rb +++ b/app/models/provider/enable_banking.rb @@ -39,9 +39,12 @@ class Provider::EnableBanking # @param psu_type [String] "personal" or "business" # @param maximum_consent_validity [Integer, nil] Max consent duration in seconds from ASPSP (nil = use 90 days) # @param language [String, nil] Two-letter language code (e.g. "fr", "en") + # @param auth_method [String, nil] Name of a specific authentication method to use (from the ASPSP's + # auth_methods list). Required to drive DECOUPLED/EMBEDDED banks that expose several methods; when nil + # Enable Banking falls back to the ASPSP's default method. # @return [Hash] Contains :url and :authorization_id def start_authorization(aspsp_name:, aspsp_country:, redirect_url:, state: nil, - psu_type: "personal", maximum_consent_validity: nil, language: nil) + psu_type: "personal", maximum_consent_validity: nil, language: nil, auth_method: nil) max_seconds = maximum_consent_validity ? [ maximum_consent_validity, 1 ].max : 90.days.to_i valid_until = [ Time.current + max_seconds.seconds, Time.current + 90.days ].min @@ -60,6 +63,7 @@ class Provider::EnableBanking psu_type: psu_type } body[:language] = language if language.present? + body[:auth_method] = auth_method if auth_method.present? body = body.compact response = self.class.post( diff --git a/config/locales/views/enable_banking_items/ca.yml b/config/locales/views/enable_banking_items/ca.yml index 432240782..f05a0386f 100644 --- a/config/locales/views/enable_banking_items/ca.yml +++ b/config/locales/views/enable_banking_items/ca.yml @@ -4,8 +4,6 @@ ca: authorize: authorization_failed: 'No s''ha pogut iniciar l''autorització: %{message}' bank_required: Selecciona un banc. - decoupled_not_supported: Aquest banc utilitza un mètode d'autenticació amb dispositiu - separat que encara no està admès. Afegeix aquest compte manualment. invalid_redirect: L'URL d'autorització rebuda no és vàlida. Torna-ho a provar. redirect_uri_not_allowed: Redirecció no permesa. Configura `%{callback_url}` a la configuració de la teva aplicació Enable Banking. diff --git a/config/locales/views/enable_banking_items/en.yml b/config/locales/views/enable_banking_items/en.yml index ab6061433..b3eb1f15f 100644 --- a/config/locales/views/enable_banking_items/en.yml +++ b/config/locales/views/enable_banking_items/en.yml @@ -9,7 +9,6 @@ en: authorize: authorization_failed: "Failed to initiate authorization: %{message}" bank_required: Please select a bank. - decoupled_not_supported: This bank uses a separate device authentication method which is not yet supported. Please add this account manually. invalid_redirect: The authorization URL received is invalid. Please try again. redirect_uri_not_allowed: Redirect not allowed. Please configure `%{callback_url}` in your Enable Banking app settings. unexpected_error: An unexpected error occurred. Please try again. diff --git a/config/locales/views/enable_banking_items/fr.yml b/config/locales/views/enable_banking_items/fr.yml index 1c6684aac..2b012a169 100644 --- a/config/locales/views/enable_banking_items/fr.yml +++ b/config/locales/views/enable_banking_items/fr.yml @@ -9,7 +9,6 @@ fr: authorize: authorization_failed: Échec de l'initiation de l'autorisation bank_required: Veuillez sélectionner une banque. - decoupled_not_supported: Cette banque utilise une méthode d'authentification sur un appareil séparé qui n'est pas encore prise en charge. Veuillez ajouter ce compte manuellement. invalid_redirect: L'URL d'autorisation reçue est invalide. Veuillez réessayer. redirect_uri_not_allowed: Redirection non autorisée. Veuillez configurer `%{callback_url}` dans les paramètres de votre application Enable Banking. unexpected_error: Une erreur inattendue s'est produite. Veuillez réessayer. diff --git a/config/locales/views/enable_banking_items/hu.yml b/config/locales/views/enable_banking_items/hu.yml index 79a3282f0..634c44f3c 100644 --- a/config/locales/views/enable_banking_items/hu.yml +++ b/config/locales/views/enable_banking_items/hu.yml @@ -9,7 +9,6 @@ hu: authorize: authorization_failed: "Nem sikerült elindítani az engedélyezést: %{message}" bank_required: Kérjük, válassz bankot. - decoupled_not_supported: Ez a bank egy különálló eszközös hitelesítési módszert használ, amely jelenleg nem támogatott. Kérjük, add hozzá ezt a számlát manuálisan. invalid_redirect: A kapott engedélyezési URL érvénytelen. Kérjük, próbáld újra. redirect_uri_not_allowed: Az átirányítás nem engedélyezett. Kérjük, konfiguráld a `%{callback_url}` címet az Enable Banking alkalmazás beállításaiban. unexpected_error: Váratlan hiba történt. Kérjük, próbáld újra. diff --git a/config/locales/views/enable_banking_items/vi.yml b/config/locales/views/enable_banking_items/vi.yml index 138ba872d..8d395d14f 100644 --- a/config/locales/views/enable_banking_items/vi.yml +++ b/config/locales/views/enable_banking_items/vi.yml @@ -9,7 +9,6 @@ vi: authorize: authorization_failed: "Không thể khởi tạo ủy quyền: %{message}" bank_required: Vui lòng chọn ngân hàng. - decoupled_not_supported: Ngân hàng này sử dụng phương thức xác thực thiết bị riêng biệt chưa được hỗ trợ. Vui lòng thêm tài khoản này thủ công. invalid_redirect: URL ủy quyền nhận được không hợp lệ. Vui lòng thử lại. redirect_uri_not_allowed: Chuyển hướng không được phép. Vui lòng cấu hình `%{callback_url}` trong cài đặt ứng dụng Enable Banking của bạn. unexpected_error: Đã xảy ra lỗi không mong muốn. Vui lòng thử lại. diff --git a/config/locales/views/enable_banking_items/zh-CN.yml b/config/locales/views/enable_banking_items/zh-CN.yml index 3bf277767..7038de650 100644 --- a/config/locales/views/enable_banking_items/zh-CN.yml +++ b/config/locales/views/enable_banking_items/zh-CN.yml @@ -9,7 +9,6 @@ zh-CN: authorize: authorization_failed: 启动授权失败:%{message} bank_required: 请选择一家银行。 - decoupled_not_supported: 该银行使用独立设备认证方式,目前尚不支持。请手动添加此账户。 invalid_redirect: 收到的授权 URL 无效。请重试。 redirect_uri_not_allowed: 不允许重定向。请在 Enable Banking 应用设置中配置 `%{callback_url}`。 unexpected_error: 发生意外错误。请重试。 diff --git a/test/controllers/enable_banking_items_controller_test.rb b/test/controllers/enable_banking_items_controller_test.rb index 06005bc9a..424a5aa64 100644 --- a/test/controllers/enable_banking_items_controller_test.rb +++ b/test/controllers/enable_banking_items_controller_test.rb @@ -39,4 +39,28 @@ class EnableBankingItemsControllerTest < ActionDispatch::IntegrationTest assert_includes haystack, "ing-diba ag", "Expected the searchable data attribute to still include the bank name (existing name-search behavior)" end + + test "authorize no longer blocks decoupled banks and proceeds to the hosted auth page" do + Provider::EnableBanking.any_instance.stubs(:get_aspsps).returns( + aspsps: [ + { + name: "VR Bank in Holstein", + country: "DE", + psu_types: [ "personal" ], + auth_methods: [ { name: "decoupled_app", approach: "DECOUPLED" } ] + } + ] + ) + Provider::EnableBanking.any_instance.stubs(:start_authorization).returns( + url: "https://api.enablebanking.com/auth/redirect/abc", + authorization_id: "auth_1" + ) + + post authorize_enable_banking_item_url(@item), + params: { aspsp_name: "VR Bank in Holstein", psu_type: "personal" } + + assert_redirected_to "https://api.enablebanking.com/auth/redirect/abc" + assert_nil flash[:alert] + assert_equal "DECOUPLED", @item.reload.aspsp_auth_approach + end end diff --git a/test/models/enable_banking_item/importer_error_handling_test.rb b/test/models/enable_banking_item/importer_error_handling_test.rb index e5b9192cb..006ce127a 100644 --- a/test/models/enable_banking_item/importer_error_handling_test.rb +++ b/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 diff --git a/test/models/enable_banking_item/syncer_test.rb b/test/models/enable_banking_item/syncer_test.rb new file mode 100644 index 000000000..861db6e6c --- /dev/null +++ b/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 diff --git a/test/models/enable_banking_item_test.rb b/test/models/enable_banking_item_test.rb new file mode 100644 index 000000000..83e101119 --- /dev/null +++ b/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 diff --git a/test/models/provider/enable_banking_test.rb b/test/models/provider/enable_banking_test.rb index e09c90c7e..16c3a8d8a 100644 --- a/test/models/provider/enable_banking_test.rb +++ b/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