From f5b60e03e122be0171b3c67ab7694bb4ff1a879f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 18 Oct 2019 16:24:09 +0200 Subject: [PATCH] Don't let valuators update investments There were some confusing definitions regarding the valuation of budget investments. In the controller, `CommentableActions` was included, which includes the update action. In the abilities, a valuator was given permission to update an investment. However, the action to update an investment didn't work because there is no route defined to do so. The ability was defined so valuators could access the "edit" action, which will not call the "update" action but the "valuate" action. Since internally "edit" and "update" use the same permission, it worked. But then we added permission for regular users to update budget investments, and these permissions were allowing valuators to update another user's investment. After this change, everything seems to work properly since we check authorization in the controller itself instead of using abilities. --- app/models/abilities/valuator.rb | 4 ++-- spec/models/abilities/valuator_spec.rb | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/app/models/abilities/valuator.rb b/app/models/abilities/valuator.rb index dde522b6a..5a15011a6 100644 --- a/app/models/abilities/valuator.rb +++ b/app/models/abilities/valuator.rb @@ -7,9 +7,9 @@ module Abilities assigned_investment_ids = valuator.assigned_investment_ids finished = { phase: "finished" } - can [:read, :update], Budget::Investment, id: assigned_investment_ids + can [:read], Budget::Investment, id: assigned_investment_ids can [:valuate], Budget::Investment, { id: assigned_investment_ids, valuation_finished: false } - cannot [:update, :valuate, :comment_valuation], Budget::Investment, budget: finished + cannot [:valuate, :comment_valuation], Budget::Investment, budget: finished if valuator.can_edit_dossier? can [:edit_dossier], Budget::Investment, id: assigned_investment_ids diff --git a/spec/models/abilities/valuator_spec.rb b/spec/models/abilities/valuator_spec.rb index 379001a25..3ef93c799 100644 --- a/spec/models/abilities/valuator_spec.rb +++ b/spec/models/abilities/valuator_spec.rb @@ -18,16 +18,12 @@ describe Abilities::Valuator do should_not be_able_to(:valuate, assigned_investment) end - it { should_not be_able_to(:update, non_assigned_investment) } - it { should_not be_able_to(:valuate, non_assigned_investment) } + it { should_not be_able_to(:update, assigned_investment) } - it { should be_able_to(:update, assigned_investment) } it { should be_able_to(:valuate, assigned_investment) } - - it { should be_able_to(:update, group_assigned_investment) } it { should be_able_to(:valuate, group_assigned_investment) } - it { should_not be_able_to(:update, finished_assigned_investment) } + it { should_not be_able_to(:valuate, non_assigned_investment) } it { should_not be_able_to(:valuate, finished_assigned_investment) } it "can update dossier information if not set can_edit_dossier attribute" do