Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/transactions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ def update_preferences
end

def exchange_rate
account = Current.family.accounts.find(params[:account_id])
account = Current.user.accessible_accounts.find(params[:account_id])
currency_from = params[:currency]
date = params[:date]&.to_date || Date.current

Expand Down
6 changes: 3 additions & 3 deletions app/views/import/uploads/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@

<%= styled_form_with model: @import, scope: :import, url: import_upload_path(@import), multipart: true, class: "space-y-4" do |form| %>
<%= form.select :account_id,
@import.family.accounts.visible.alphabetically.pluck(:name, :id),
accessible_accounts.visible.alphabetically.pluck(:name, :id),
{ label: t(".qif_account_label"), include_blank: t(".qif_account_placeholder"), selected: @import.account_id },
required: true %>

Expand Down Expand Up @@ -112,7 +112,7 @@
<%= form.select :col_sep, Import.separator_options, label: true %>

<% if @import.type == "TransactionImport" || @import.type == "TradeImport" %>
<%= form.select :account_id, @import.family.accounts.visible.alphabetically.pluck(:name, :id), { label: t(".account_optional_label"), include_blank: t(".multi_account_import"), selected: @import.account_id } %>
<%= form.select :account_id, accessible_accounts.visible.alphabetically.pluck(:name, :id), { label: t(".account_optional_label"), include_blank: t(".multi_account_import"), selected: @import.account_id } %>
<% end %>

<label for="import_import_file_csv" class="flex flex-col items-center justify-center w-full h-64 border border-secondary border-dashed rounded-xl cursor-pointer" data-controller="file-upload" data-file-upload-target="uploadArea">
Expand Down Expand Up @@ -144,7 +144,7 @@
<%= form.select :col_sep, Import.separator_options, label: true %>

<% if @import.type == "TransactionImport" || @import.type == "TradeImport" %>
<%= form.select :account_id, @import.family.accounts.visible.alphabetically.pluck(:name, :id), { label: t(".account_optional_label"), include_blank: t(".multi_account_import"), selected: @import.account_id } %>
<%= form.select :account_id, accessible_accounts.visible.alphabetically.pluck(:name, :id), { label: t(".account_optional_label"), include_blank: t(".multi_account_import"), selected: @import.account_id } %>
<% end %>

<%= form.text_area :raw_file_str,
Expand Down
2 changes: 1 addition & 1 deletion app/views/transactions/_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%# locals: (entry:, categories:) %>

<% account_currencies = Current.family.accounts.map { |a| [a.id, a.currency] }.to_h.to_json %>
<% account_currencies = accessible_accounts.pluck(:id, :currency).to_h.to_json %>
<%= styled_form_with model: entry, url: transactions_path, class: "space-y-4", data: { controller: "transaction-form", transaction_form_exchange_rate_url_value: exchange_rate_path, transaction_form_account_currencies_value: account_currencies } do |f| %>
<% if entry.errors.any? %>
<%= render "shared/form_errors", model: entry %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/transfers/_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% account_currencies = Current.family.accounts.map { |a| [a.id, a.currency] }.to_h.to_json %>
<% account_currencies = accessible_accounts.pluck(:id, :currency).to_h.to_json %>
<%= styled_form_with model: transfer, class: "space-y-4", data: { turbo_frame: "_top", controller: "transfer-form", transfer_form_exchange_rate_url_value: exchange_rate_path, transfer_form_account_currencies_value: account_currencies } do |f| %>
<% if transfer.errors.present? %>
<div class="text-destructive flex items-center gap-2">
Expand Down
13 changes: 13 additions & 0 deletions test/controllers/import/uploads_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,19 @@ class Import::UploadsControllerTest < ActionDispatch::IntegrationTest
assert_response :success
end

test "csv upload dropdown only lists accounts accessible to the signed-in user" do
# family_member has shares for :depository (full_control) and :credit_card (read_only)
# but not for :investment, owned by family_admin. The optional account dropdown
# in the CSV upload form must not leak unshared account names.
sign_in users(:family_member)

get import_upload_url(@import)

assert_response :success
assert_includes response.body, accounts(:depository).name
refute_includes response.body, accounts(:investment).name
end

test "uploads valid csv by copy and pasting" do
patch import_upload_url(@import), params: {
import: {
Expand Down
15 changes: 15 additions & 0 deletions test/controllers/transactions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,21 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
assert_not entry.protected_from_sync?
end

test "new only exposes account ids accessible to the signed-in user" do
# family_member has shares for :depository (full_control) and :credit_card (read_only)
# but not for :investment or :loan. The currency JSON map at the top of the
# form should not leak ids/currencies of unshared accounts.
sign_in users(:family_member)

get new_transaction_url

assert_response :success
assert_includes response.body, accounts(:depository).id.to_s
assert_includes response.body, accounts(:credit_card).id.to_s
refute_includes response.body, accounts(:investment).id.to_s
refute_includes response.body, accounts(:loan).id.to_s
end

test "new with duplicate_entry_id pre-fills form from source transaction" do
@entry.reload

Expand Down
13 changes: 13 additions & 0 deletions test/controllers/transfers_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,19 @@ class TransfersControllerTest < ActionDispatch::IntegrationTest
assert_response :success
end

test "new only exposes accounts accessible to the signed-in user" do
# family_member has shares for :depository (full_control) and :credit_card (read_only)
# but not for :investment or :loan, which are owned solely by family_admin.
sign_in users(:family_member)

get new_transfer_url

assert_response :success
assert_includes response.body, accounts(:depository).id.to_s
refute_includes response.body, accounts(:investment).id.to_s
refute_includes response.body, accounts(:loan).id.to_s
end

test "can create transfers" do
assert_difference "Transfer.count", 1 do
post transfers_url, params: {
Expand Down
Loading