From cdd26dd568a35c59ac7f8902df041a07f6c11036 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?= <15726+Senen@users.noreply.github.com> Date: Fri, 13 Jan 2023 15:29:37 +0100 Subject: [PATCH] Fix access restriction in valuation budget investments controller Since we allow many active budgets at the same time, the controller should now check the budget given by params. Before this change the controller was checking the latest published budget, ignoring the request parameter `budget_id`. --- .../valuation/budget_investments_controller.rb | 4 ++-- spec/system/valuation/budget_investments_spec.rb | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/controllers/valuation/budget_investments_controller.rb b/app/controllers/valuation/budget_investments_controller.rb index 6fe23df54..f1ff5d834 100644 --- a/app/controllers/valuation/budget_investments_controller.rb +++ b/app/controllers/valuation/budget_investments_controller.rb @@ -4,9 +4,9 @@ class Valuation::BudgetInvestmentsController < Valuation::BaseController feature_flag :budgets + before_action :load_budget before_action :restrict_access_to_assigned_items, only: [:show, :edit, :valuate] before_action :restrict_access, only: [:edit, :valuate] - before_action :load_budget before_action :load_investment, only: [:show, :edit, :valuate] has_orders %w[oldest], only: [:show, :edit] @@ -110,7 +110,7 @@ class Valuation::BudgetInvestmentsController < Valuation::BaseController end def restrict_access - unless current_user.administrator? || current_budget.valuating? + unless current_user.administrator? || @budget.valuating? raise CanCan::AccessDenied, I18n.t("valuation.budget_investments.not_in_valuating_phase") end end diff --git a/spec/system/valuation/budget_investments_spec.rb b/spec/system/valuation/budget_investments_spec.rb index 69a5188d0..e02ea6b5d 100644 --- a/spec/system/valuation/budget_investments_spec.rb +++ b/spec/system/valuation/budget_investments_spec.rb @@ -501,6 +501,17 @@ describe "Valuation budget investments" do expect(page).to have_content("Investments can only be valuated when Budget is in valuating phase") end + scenario "restric access to the budget given by params when is not in valuating phase" do + budget.update!(phase: "publishing_prices") + create(:budget, :valuating) + investment = create(:budget_investment, budget: budget, valuators: [valuator]) + + login_as(valuator.user) + visit edit_valuation_budget_budget_investment_path(budget, investment) + + expect(page).to have_content("Investments can only be valuated when Budget is in valuating phase") + end + scenario "visible to admins regardless of not being in valuating phase" do budget.update!(phase: "publishing_prices")