From 69827dada87d4d8fcb1496712fd6725fae2e37cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Mata?= Date: Mon, 13 Apr 2026 21:23:59 +0200 Subject: [PATCH] Fix transaction search account scope bypass (#1460) Ensure accessible_account_ids filtering is applied whenever account scope is provided, including empty arrays, so users with no shared accounts cannot see family-wide transactions. Also make totals robust when scoped queries return no rows and add regression tests for both visibility and totals behavior with empty accessible account lists. --- app/models/transaction/search.rb | 14 +++++++------- test/models/transaction/search_test.rb | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/app/models/transaction/search.rb b/app/models/transaction/search.rb index 43dfdd78b..d9a87c18e 100644 --- a/app/models/transaction/search.rb +++ b/app/models/transaction/search.rb @@ -29,8 +29,8 @@ class Transaction::Search # This already joins entries + accounts. To avoid expensive double-joins, don't join them again (causes full table scan) query = family.transactions.merge(Entry.excluding_split_parents) - # Scope to accessible accounts when provided - query = query.where(entries: { account_id: accessible_account_ids }) if accessible_account_ids&.any? + # Scope to accessible accounts when provided (including an empty array, which should yield no results) + query = query.where(entries: { account_id: accessible_account_ids }) unless accessible_account_ids.nil? query = apply_active_accounts_filter(query, active_accounts_only) query = apply_category_filter(query, categories) @@ -88,11 +88,11 @@ class Transaction::Search .take Totals.new( - count: result.transactions_count.to_i, - income_money: Money.new(result.income_total, family.currency), - expense_money: Money.new(result.expense_total, family.currency), - transfer_inflow_money: Money.new(result.transfer_inflow_total, family.currency), - transfer_outflow_money: Money.new(result.transfer_outflow_total, family.currency) + count: result&.transactions_count.to_i, + income_money: Money.new((result&.income_total || 0), family.currency), + expense_money: Money.new((result&.expense_total || 0), family.currency), + transfer_inflow_money: Money.new((result&.transfer_inflow_total || 0), family.currency), + transfer_outflow_money: Money.new((result&.transfer_outflow_total || 0), family.currency) ) end end diff --git a/test/models/transaction/search_test.rb b/test/models/transaction/search_test.rb index 1d7521c0e..e3e899929 100644 --- a/test/models/transaction/search_test.rb +++ b/test/models/transaction/search_test.rb @@ -625,4 +625,25 @@ class Transaction::SearchTest < ActiveSupport::TestCase end end end + + test "empty accessible_account_ids yields no visible transactions" do + create_transaction(account: @checking_account, amount: 100) + + search = Transaction::Search.new(@family, filters: {}, accessible_account_ids: []) + + assert_empty search.transactions_scope + end + + test "totals handles empty accessible_account_ids without raising" do + create_transaction(account: @checking_account, amount: 100) + + search = Transaction::Search.new(@family, filters: {}, accessible_account_ids: []) + totals = search.totals + + assert_equal 0, totals.count + assert_equal Money.new(0, @family.currency), totals.expense_money + assert_equal Money.new(0, @family.currency), totals.income_money + assert_equal Money.new(0, @family.currency), totals.transfer_inflow_money + assert_equal Money.new(0, @family.currency), totals.transfer_outflow_money + end end