Skip to content

Commit 69827da

Browse files
authored
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.
1 parent aacbb5e commit 69827da

2 files changed

Lines changed: 28 additions & 7 deletions

File tree

app/models/transaction/search.rb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ def transactions_scope
2929
# This already joins entries + accounts. To avoid expensive double-joins, don't join them again (causes full table scan)
3030
query = family.transactions.merge(Entry.excluding_split_parents)
3131

32-
# Scope to accessible accounts when provided
33-
query = query.where(entries: { account_id: accessible_account_ids }) if accessible_account_ids&.any?
32+
# Scope to accessible accounts when provided (including an empty array, which should yield no results)
33+
query = query.where(entries: { account_id: accessible_account_ids }) unless accessible_account_ids.nil?
3434

3535
query = apply_active_accounts_filter(query, active_accounts_only)
3636
query = apply_category_filter(query, categories)
@@ -88,11 +88,11 @@ def totals
8888
.take
8989

9090
Totals.new(
91-
count: result.transactions_count.to_i,
92-
income_money: Money.new(result.income_total, family.currency),
93-
expense_money: Money.new(result.expense_total, family.currency),
94-
transfer_inflow_money: Money.new(result.transfer_inflow_total, family.currency),
95-
transfer_outflow_money: Money.new(result.transfer_outflow_total, family.currency)
91+
count: result&.transactions_count.to_i,
92+
income_money: Money.new((result&.income_total || 0), family.currency),
93+
expense_money: Money.new((result&.expense_total || 0), family.currency),
94+
transfer_inflow_money: Money.new((result&.transfer_inflow_total || 0), family.currency),
95+
transfer_outflow_money: Money.new((result&.transfer_outflow_total || 0), family.currency)
9696
)
9797
end
9898
end

test/models/transaction/search_test.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,4 +625,25 @@ class Transaction::SearchTest < ActiveSupport::TestCase
625625
end
626626
end
627627
end
628+
629+
test "empty accessible_account_ids yields no visible transactions" do
630+
create_transaction(account: @checking_account, amount: 100)
631+
632+
search = Transaction::Search.new(@family, filters: {}, accessible_account_ids: [])
633+
634+
assert_empty search.transactions_scope
635+
end
636+
637+
test "totals handles empty accessible_account_ids without raising" do
638+
create_transaction(account: @checking_account, amount: 100)
639+
640+
search = Transaction::Search.new(@family, filters: {}, accessible_account_ids: [])
641+
totals = search.totals
642+
643+
assert_equal 0, totals.count
644+
assert_equal Money.new(0, @family.currency), totals.expense_money
645+
assert_equal Money.new(0, @family.currency), totals.income_money
646+
assert_equal Money.new(0, @family.currency), totals.transfer_inflow_money
647+
assert_equal Money.new(0, @family.currency), totals.transfer_outflow_money
648+
end
628649
end

0 commit comments

Comments
 (0)