Merge remote-tracking branch 'upstream/master' into 30634-protected-pipeline

* upstream/master:
  Make membership required for create_user_by_membership
  Fix sa
  use go instead of let proc
  Fixed typo and hash alighment
  Improve description_for
  Fix static analysis
  IMprove access_matchers
  Remove author. Replace Result to Got.
  Fix static analysys
  Resolve static analysis
  Add changelog. Add AccessMatchersForController
  Use authorize_update_pipeline_schedule in PipelineSchedulesController

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]

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:

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

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