From 8026f47d67546d48139cb35a0dc16bbac47b1bb7 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 5 Jun 2017 22:19:23 +0900 Subject: [PATCH] Add changelog. Add AccessMatchersForController --- ...nership-in-pipelineschedulescontroller.yml | 4 + .../pipeline_schedules_controller_spec.rb | 58 ++++++++++++ .../access_matchers_for_controller.rb | 88 +++++++++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 changelogs/unreleased/33082-use-update_pipeline_schedule-for-edit-and-take_ownership-in-pipelineschedulescontroller.yml create mode 100644 spec/support/matchers/access_matchers_for_controller.rb 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..3769940feb9 --- /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: dosuken123 diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index f8f95dd9bc8..abbf74235ef 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -84,4 +84,62 @@ describe Projects::PipelineSchedulesController do end end end + + describe 'security' do + include AccessMatchersForController + + describe 'GET edit' do + let(:action) do + Proc.new do |user| + get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + end + end + + specify { expect(action).to be_allowed_for(:admin) } + specify { expect(action).to be_allowed_for(:owner).of(project) } + specify { expect(action).to be_allowed_for(:master).of(project) } + specify { expect(action).to be_allowed_for(:developer).of(project) } + specify { expect(action).to be_denied_for(:reporter).of(project) } + specify { expect(action).to be_denied_for(:guest).of(project) } + specify { expect(action).to be_denied_for(:user) } + specify { expect(action).to be_denied_for(:external) } + specify { expect(action).to be_denied_for(:visitor) } + end + + describe 'GET take_ownership' do + let(:action) do + Proc.new do |user| + post :take_ownership, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + end + end + + specify { expect(action).to be_allowed_for(:admin) } + specify { expect(action).to be_allowed_for(:owner).of(project) } + specify { expect(action).to be_allowed_for(:master).of(project) } + specify { expect(action).to be_allowed_for(:developer).of(project) } + specify { expect(action).to be_denied_for(:reporter).of(project) } + specify { expect(action).to be_denied_for(:guest).of(project) } + specify { expect(action).to be_denied_for(:user) } + specify { expect(action).to be_denied_for(:external) } + specify { expect(action).to be_denied_for(:visitor) } + end + + describe 'PUT update' do + let(:action) do + Proc.new do |user| + put :update, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + end + end + + specify { expect(action).to be_allowed_for(:admin) } + specify { expect(action).to be_allowed_for(:owner).of(project) } + specify { expect(action).to be_allowed_for(:master).of(project) } + specify { expect(action).to be_allowed_for(:developer).of(project) } + specify { expect(action).to be_denied_for(:reporter).of(project) } + specify { expect(action).to be_denied_for(:guest).of(project) } + specify { expect(action).to be_denied_for(:user) } + specify { expect(action).to be_denied_for(:external) } + specify { expect(action).to be_denied_for(:visitor) } + 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..4da10247801 --- /dev/null +++ b/spec/support/matchers/access_matchers_for_controller.rb @@ -0,0 +1,88 @@ +# AccessMatchersForController +# +# For testing authorize_xxx in controller. +module AccessMatchersForController + extend RSpec::Matchers::DSL + include Warden::Test::Helpers + + EXPECTED_STATUS_CODE_ALLOWED = [200, 302] + EXPECTED_STATUS_CODE_DENIED = [404] + + def emulate_user(role, membership = nil) + case role + when :admin + user = create(:admin) + 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 + + if role == :owner && membership.owner + user = membership.owner + else + user = create(:user) + membership.public_send(:"add_#{role}", user) + end + + sign_in(user) + when :user + user = create(:user) + sign_in(user) + when :external + user = create(:user, external: true) + sign_in(user) + when :visitor + # no-op + else + raise ArgumentError, "cannot emulate user #{role}" + end + + user + end + + def description_for(role, type, expected, result) + "be #{type} for #{role}." \ + " Expected: #{expected.join(',')} Result: #{result}" + end + + matcher :be_allowed_for do |role| + match do |action| + user = emulate_user(role, @membership) + begin + action.call(user) + rescue Exception => e + # Ignore internal exceptions which will be caused in the controller + # In such cases, response.status will be 200. + end + + 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| + user = emulate_user(role, @membership) + begin + action.call(user) + rescue Exception => e + # Ignore internal exceptions which will be caused in the controller + # In such cases, response.status will be 200. + end + + 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