Merge branch '33082-use-update_pipeline_schedule-for-edit-and-take_ownership-in-pipelineschedulescontroller' into 'master'

Use authorize_update_pipeline_schedule in PipelineSchedulesController

Closes #33082

See merge request !11846

4 files changed, 143 insertions, 1 deletions

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index ef4f083b98f..60db179277b 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -1,6 +1,7 @@
 class Projects::PipelineSchedulesController < Projects::ApplicationController
   before_action :authorize_read_pipeline_schedule!
-  before_action :authorize_create_pipeline_schedule!, only: [:new, :create, :edit, :take_ownership, :update]
+  before_action :authorize_create_pipeline_schedule!, only: [:new, :create]
+  before_action :authorize_update_pipeline_schedule!, only: [:edit, :take_ownership, :update]
   before_action :authorize_admin_pipeline_schedule!, only: [:destroy]
 
   before_action :schedule, only: [:edit, :update, :destroy, :take_ownership]
diff --git a/changelogs/unreleased/33082-use-update_pipeline_schedule-for-edit-and-take_ownership-in-pipelineschedulescontroller.yml b/changelogs/unreleased/33082-use-update_pipeline_schedule-for-edit-and-take_ownership-in-pipelineschedulescontroller.yml
new file mode 100644
index 00000000000..d3172c405c3
--- /dev/null
+++ b/changelogs/unreleased/33082-use-update_pipeline_schedule-for-edit-and-take_ownership-in-pipelineschedulescontroller.yml
@@ -0,0 +1,4 @@
+---
+title: Use authorize_update_pipeline_schedule in PipelineSchedulesController
+merge_request: 11846
+author:
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index f8f95dd9bc8..a8c44d5c313 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -84,4 +84,57 @@ describe Projects::PipelineSchedulesController do
       end
     end
   end
+
+  describe 'security' do
+    include AccessMatchersForController
+
+    describe 'GET edit' do
+      it { expect { go }.to be_allowed_for(:admin) }
+      it { expect { go }.to be_allowed_for(:owner).of(project) }
+      it { expect { go }.to be_allowed_for(:master).of(project) }
+      it { expect { go }.to be_allowed_for(:developer).of(project) }
+      it { expect { go }.to be_denied_for(:reporter).of(project) }
+      it { expect { go }.to be_denied_for(:guest).of(project) }
+      it { expect { go }.to be_denied_for(:user) }
+      it { expect { go }.to be_denied_for(:external) }
+      it { expect { go }.to be_denied_for(:visitor) }
+
+      def go
+        get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+      end
+    end
+
+    describe 'GET take_ownership' do
+      it { expect { go }.to be_allowed_for(:admin) }
+      it { expect { go }.to be_allowed_for(:owner).of(project) }
+      it { expect { go }.to be_allowed_for(:master).of(project) }
+      it { expect { go }.to be_allowed_for(:developer).of(project) }
+      it { expect { go }.to be_denied_for(:reporter).of(project) }
+      it { expect { go }.to be_denied_for(:guest).of(project) }
+      it { expect { go }.to be_denied_for(:user) }
+      it { expect { go }.to be_denied_for(:external) }
+      it { expect { go }.to be_denied_for(:visitor) }
+
+      def go
+        post :take_ownership, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+      end
+    end
+
+    describe 'PUT update' do
+      it { expect { go }.to be_allowed_for(:admin) }
+      it { expect { go }.to be_allowed_for(:owner).of(project) }
+      it { expect { go }.to be_allowed_for(:master).of(project) }
+      it { expect { go }.to be_allowed_for(:developer).of(project) }
+      it { expect { go }.to be_denied_for(:reporter).of(project) }
+      it { expect { go }.to be_denied_for(:guest).of(project) }
+      it { expect { go }.to be_denied_for(:user) }
+      it { expect { go }.to be_denied_for(:external) }
+      it { expect { go }.to be_denied_for(:visitor) }
+
+      def go
+        put :update, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id,
+                     schedule: { description: 'a' }
+      end
+    end
+  end
 end
diff --git a/spec/support/matchers/access_matchers_for_controller.rb b/spec/support/matchers/access_matchers_for_controller.rb
new file mode 100644
index 00000000000..fb43f51c70c
--- /dev/null
+++ b/spec/support/matchers/access_matchers_for_controller.rb
@@ -0,0 +1,84 @@
+# AccessMatchersForController
+#
+# For testing authorize_xxx in controller. 
+module AccessMatchersForController
+  extend RSpec::Matchers::DSL
+  include Warden::Test::Helpers
+
+  EXPECTED_STATUS_CODE_ALLOWED = [200, 201, 302].freeze
+  EXPECTED_STATUS_CODE_DENIED = [401, 404].freeze
+
+  def emulate_user(role, membership = nil)
+    case role
+    when :admin
+      user = create(:admin)
+      sign_in(user)
+    when :user
+      user = create(:user)
+      sign_in(user)
+    when :external
+      user = create(:user, external: true)
+      sign_in(user)
+    when :visitor
+      user = nil
+    when User
+      user = role
+      sign_in(user)
+    when *Gitlab::Access.sym_options_with_owner.keys # owner, master, developer, reporter, guest
+      raise ArgumentError, "cannot emulate #{role} without membership parent" unless membership
+
+      user = create_user_by_membership(role, membership)
+      sign_in(user)
+    else
+      raise ArgumentError, "cannot emulate user #{role}"
+    end
+
+    user
+  end
+
+  def create_user_by_membership(role, membership)
+    if role == :owner && membership.owner
+      user = membership.owner
+    else
+      user = create(:user)
+      membership.public_send(:"add_#{role}", user)
+    end
+    user
+  end
+
+  def description_for(role, type, expected, result)
+    "be #{type} for #{role}. Expected: #{expected.join(',')} Got: #{result}"
+  end
+
+  matcher :be_allowed_for do |role|
+    match do |action|
+      emulate_user(role, @membership)
+      action.call
+
+      EXPECTED_STATUS_CODE_ALLOWED.include?(response.status)
+    end
+
+    chain :of do |membership|
+      @membership = membership
+    end
+
+    description { description_for(role, 'allowed', EXPECTED_STATUS_CODE_ALLOWED, response.status) }
+    supports_block_expectations
+  end
+
+  matcher :be_denied_for do |role|
+    match do |action|
+      emulate_user(role, @membership)
+      action.call
+
+      EXPECTED_STATUS_CODE_DENIED.include?(response.status)
+    end
+
+    chain :of do |membership|
+      @membership = membership
+    end
+
+    description { description_for(role, 'denied', EXPECTED_STATUS_CODE_DENIED, response.status) }
+    supports_block_expectations
+  end
+end