Backport 'Refactor amendment permissions' to v0.30 (decidim#15168)

Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>

Author: alecslupu

decidim-core/app/commands/decidim/amendable/accept.rb

@@ -62,7 +62,8 @@ def update_amendable!
           @amendable.save!
           @amendable
         end
-        @amendable.add_coauthor(@amender, user_group: @user_group)
+
+        @amendable.add_coauthor(@amender, user_group: @user_group) if @amendable.is_a?(Decidim::Coauthorable)
       end
 
       def notify_emendation_state_change!

decidim-core/app/controllers/decidim/amendments_controller.rb

@@ -107,7 +107,7 @@ def publish_draft
     end
 
     def reject
-      enforce_permission_to :reject, :amendment, current_component: amendable.component
+      enforce_permission_to :reject, :amendment, amendable:, current_component: amendable.component
 
       @form = form(Decidim::Amendable::RejectForm).from_model(amendment)
 
@@ -143,13 +143,13 @@ def promote
     end
 
     def review
-      enforce_permission_to :accept, :amendment, current_component: amendable.component
+      enforce_permission_to :accept, :amendment, amendable:, current_component: amendable.component
 
       @form = form(Decidim::Amendable::ReviewForm).from_params(params)
     end
 
     def accept
-      enforce_permission_to :accept, :amendment, current_component: amendable.component
+      enforce_permission_to :accept, :amendment, amendable:, current_component: amendable.component
 
       @form = form(Decidim::Amendable::ReviewForm).from_params(params)
 

decidim-core/app/helpers/decidim/amendments_helper.rb

@@ -61,8 +61,9 @@ def can_react_to_emendation?(emendation)
     # Checks if the user can accept and reject the emendation
     def allowed_to_accept_and_reject?(emendation)
       return unless emendation.amendment.evaluating?
+      return current_user.admin? if emendation.amendable.respond_to?(:official?) && emendation.amendable.official?
 
-      emendation.amendable.created_by?(current_user) || current_user.admin?
+      emendation.amendable.created_by?(current_user)
     end
 
     # Checks if the user can promote the emendation

decidim-core/app/permissions/decidim/permissions.rb

@@ -106,7 +106,19 @@ def amend_action?
         return allow! if component.current_settings.amendment_creation_enabled
       when :accept,
           :reject
-        return allow! if component.current_settings.amendment_reaction_enabled
+        return disallow! unless component.current_settings.amendment_reaction_enabled
+
+        amendable = context.fetch(:amendable, nil)
+
+        if amendable.respond_to?(:official?) && amendable.official?
+          return allow! if user.admin?
+
+          return disallow!
+        end
+
+        return disallow! unless amendable.authored_by?(user)
+
+        return allow!
       when :promote
         return allow! if component.current_settings.amendment_promotion_enabled
       end

decidim-core/spec/controllers/amendments_controller_spec.rb

@@ -234,5 +234,132 @@ module Decidim
         end
       end
     end
+
+    describe "GET review" do
+      context "when the user is NOT the creator of the resource" do
+        let(:user) { other_user }
+
+        it "does not allow access" do
+          get(:review, params:)
+
+          expect(response).to have_http_status(:redirect)
+          expect(flash[:alert]).to eq("You are not authorized to perform this action.")
+        end
+      end
+
+      context "when the user is the creator of the resource" do
+        let(:user) { amendable.author }
+
+        it "allows access" do
+          get(:review, params:)
+
+          expect(response).to have_http_status(:ok)
+        end
+      end
+    end
+
+    describe "PATCH accept" do
+      let(:emendation_params) { { title: emendation.title, body: emendation.body } }
+
+      context "when the user is NOT the creator of the resource" do
+        let(:user) { other_user }
+
+        it "does not accept the amendment" do
+          patch :accept, params: params.merge(emendation_params:)
+
+          expect(response).to have_http_status(:redirect)
+          expect(flash[:alert]).to eq("You are not authorized to perform this action.")
+        end
+      end
+
+      context "when the user is the creator of the resource" do
+        let(:user) { amendable.author }
+
+        it "accepts the amendment" do
+          patch :accept, params: params.merge(emendation_params:)
+
+          expect(response).to have_http_status(:redirect)
+          expect(flash[:notice]).to eq("The amendment has been accepted successfully.")
+        end
+      end
+
+      context "when the resource is official" do
+        let!(:amendable) { create(:dummy_resource, component:, author: component.organization) }
+
+        context "and the user is NOT an admin" do
+          let(:user) { amendment.amender }
+
+          it "does not accept the amendment" do
+            patch :accept, params: params.merge(emendation_params:)
+
+            expect(response).to have_http_status(:redirect)
+            expect(flash[:alert]).to eq("You are not authorized to perform this action.")
+          end
+        end
+
+        context "and the user is an admin" do
+          let(:user) { create(:user, :confirmed, :admin, organization: component.organization) }
+
+          it "accepts the amendment" do
+            patch :accept, params: params.merge(emendation_params:)
+
+            expect(response).to have_http_status(:redirect)
+            expect(flash[:notice]).to eq("The amendment has been accepted successfully.")
+          end
+        end
+      end
+    end
+
+    describe "PATCH reject" do
+      let(:emendation_params) { { title: emendation.title, body: emendation.body } }
+
+      context "when the user is NOT the creator of the resource" do
+        let(:user) { other_user }
+
+        it "does not accept the amendment" do
+          patch :reject, params: params.merge(emendation_params:)
+
+          expect(response).to have_http_status(:redirect)
+          expect(flash[:alert]).to eq("You are not authorized to perform this action.")
+        end
+      end
+
+      context "when the user is the creator of the resource" do
+        let(:user) { amendable.author }
+
+        it "accepts the amendment" do
+          patch :reject, params: params.merge(emendation_params:)
+
+          expect(response).to have_http_status(:redirect)
+          expect(flash[:notice]).to eq("The amendment has been successfully rejected.")
+        end
+      end
+
+      context "when the resource is official" do
+        let!(:amendable) { create(:dummy_resource, component:, author: component.organization) }
+
+        context "and the user is NOT an admin" do
+          let(:user) { amendment.amender }
+
+          it "does not accept the amendment" do
+            patch :reject, params: params.merge(emendation_params:)
+
+            expect(response).to have_http_status(:redirect)
+            expect(flash[:alert]).to eq("You are not authorized to perform this action.")
+          end
+        end
+
+        context "and the user is an admin" do
+          let(:user) { create(:user, :confirmed, :admin, organization: component.organization) }
+
+          it "accepts the amendment" do
+            patch :reject, params: params.merge(emendation_params:)
+
+            expect(response).to have_http_status(:redirect)
+            expect(flash[:notice]).to eq("The amendment has been successfully rejected.")
+          end
+        end
+      end
+    end
   end
 end

decidim-core/spec/permissions/decidim/permissions_spec.rb

@@ -197,9 +197,11 @@
   context "when an amend action" do
     let(:component) { create(:component, :published, organization: user.organization, settings:) }
     let(:settings) { { amendments_enabled: } }
-    let(:amendment) { create(:amendment) }
-    let(:user) { amendment.amender }
-    let(:context) { { current_component: component } }
+    let(:amendable) { build(:dummy_resource, component:) }
+    let(:emendation) { build(:dummy_resource, component:) }
+    let(:amendment) { create(:amendment, emendation:, amendable:, amender: user) }
+    let(:user) { create(:user) }
+    let(:context) { { amendable: amendment.amendable, current_component: component } }
     let(:action_name) { nil }
     let(:action) do
       { scope: :public, action: action_name, subject: :amendment }
@@ -217,13 +219,49 @@
       context "with a accept action" do
         let(:action_name) { :accept }
 
-        it { is_expected.to be true }
+        it { is_expected.to be false }
+
+        context "when the amendable is authored by the user" do
+          before { amendment.amendable.update!(author: user) }
+
+          it { is_expected.to be true }
+        end
+
+        context "with an official amendable" do
+          let(:amendable) { build(:dummy_resource, component:, author: component.organization) }
+
+          it { is_expected.to be false }
+
+          context "and the user is an admin" do
+            let(:user) { create(:user, :confirmed, :admin) }
+
+            it { is_expected.to be true }
+          end
+        end
       end
 
       context "with a reject action" do
         let(:action_name) { :reject }
 
-        it { is_expected.to be true }
+        it { is_expected.to be false }
+
+        context "when the amendable is authored by the user" do
+          before { amendment.amendable.update!(author: user) }
+
+          it { is_expected.to be true }
+        end
+
+        context "with an official amendable" do
+          let(:amendable) { build(:dummy_resource, component:, author: component.organization) }
+
+          it { is_expected.to be false }
+
+          context "and the user is an admin" do
+            let(:user) { create(:user, :confirmed, :admin) }
+
+            it { is_expected.to be true }
+          end
+        end
       end
 
       context "with a promote action" do

decidim-dev/app/models/decidim/dev/dummy_resource.rb

@@ -42,6 +42,16 @@ class DummyResource < ApplicationRecord
 
       component_manifest_name "dummy"
 
+      alias creator_author author
+
+      def creator
+        self
+      end
+
+      def authors
+        [author]
+      end
+
       def presenter
         Decidim::Dev::DummyResourcePresenter.new(self)
       end

decidim-proposals/spec/system/amendable/amend_proposal_spec.rb

@@ -207,6 +207,8 @@
           let!(:user_group) { create(:user_group, :verified, organization: user.organization, users: [user]) }
 
           before do
+            expect(page).to have_content("Log in")
+            switch_to_host(component.organization.host)
             login_as user, scope: :user
             visit proposal_path
             expect(page).to have_content(proposal_title)