From bc77a6adefcd02a058e3d5a10718d74a5dad954e Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 29 Nov 2023 16:29:06 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@16-6-stable-ee --- .../projects/pipeline_schedules_controller_spec.rb | 4 + spec/helpers/groups_helper_spec.rb | 63 ++++ spec/models/ability_spec.rb | 39 ++- spec/policies/ci/pipeline_schedule_policy_spec.rb | 338 +++++++++++++++------ .../ci/pipeline_schedules/update_service_spec.rb | 56 +++- 5 files changed, 393 insertions(+), 107 deletions(-) (limited to 'spec') diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index cd828c956a0..7cd4f43d4da 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -65,6 +65,10 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu create(:protected_branch, *branch_access_levels, name: ref_name, project: project) end + after do + ProtectedBranches::CacheService.new(project).refresh + end + it { expect { go }.to try(maintainer_accessible, :maintainer).of(project) } it { expect { go }.to try(developer_accessible, :developer).of(project) } end diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index f5cadfc2761..93191cc956b 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -604,4 +604,67 @@ RSpec.describe GroupsHelper, feature_category: :groups_and_projects do end end end + + describe '#access_level_roles_user_can_assign' do + subject { helper.access_level_roles_user_can_assign(group) } + + let_it_be(:group) { create(:group) } + let_it_be_with_reload(:user) { create(:user) } + + context 'when user is provided' do + before do + allow(helper).to receive(:current_user).and_return(user) + end + + context 'when a user is a group member' do + before do + group.add_developer(user) + end + + it 'returns only the roles the provided user can assign' do + expect(subject).to eq( + { + 'Guest' => 10, + 'Reporter' => 20, + 'Developer' => 30 + } + ) + end + end + + context 'when a user is an admin', :enable_admin_mode do + before do + user.update!(admin: true) + end + + it 'returns all roles' do + expect(subject).to eq( + { + 'Guest' => 10, + 'Reporter' => 20, + 'Developer' => 30, + 'Maintainer' => 40, + 'Owner' => 50 + } + ) + end + end + + context 'when a user is not a group member' do + it 'returns the empty array' do + expect(subject).to be_empty + end + end + + context 'when user is not provided' do + before do + allow(helper).to receive(:current_user).and_return(nil) + end + + it 'returns the empty array' do + expect(subject).to be_empty + end + end + end + end end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index a808cb1c823..1f0e074d90b 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -3,9 +3,42 @@ require 'spec_helper' RSpec.describe Ability do - context 'using a nil subject' do - it 'has no permissions' do - expect(described_class.policy_for(nil, nil)).to be_banned + describe '#policy_for' do + subject(:policy) { described_class.policy_for(user, subject, **options) } + + let(:user) { User.new } + let(:subject) { :global } + let(:options) { {} } + + context 'using a nil subject' do + let(:user) { nil } + let(:subject) { nil } + + it 'has no permissions' do + expect(policy).to be_banned + end + end + + context 'with request store', :request_store do + before do + ::Gitlab::SafeRequestStore.write(:example, :value) # make request store different from {} + end + + it 'caches in the request store' do + expect(DeclarativePolicy).to receive(:policy_for).with(user, subject, cache: ::Gitlab::SafeRequestStore.storage) + + policy + end + + context 'when cache: false' do + let(:options) { { cache: false } } + + it 'uses a fresh cache each time' do + expect(DeclarativePolicy).to receive(:policy_for).with(user, subject, cache: {}) + + policy + end + end end end diff --git a/spec/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb index 8fc5c6ca296..1d353b9a35e 100644 --- a/spec/policies/ci/pipeline_schedule_policy_spec.rb +++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache, f using RSpec::Parameterized::TableSyntax let_it_be(:user) { create(:user) } + let_it_be(:other_user) { create(:user) } let_it_be_with_reload(:project) { create(:project, :repository, create_tag: tag_ref_name) } let_it_be_with_reload(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project) } let_it_be(:tag_ref_name) { "v1.0.0" } @@ -17,89 +18,180 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache, f describe 'rules' do describe 'rules for protected ref' do context 'for branch' do + subject(:policy) { described_class.new(user, pipeline_schedule) } + %w[refs/heads/master master].each do |branch_ref| context "with #{branch_ref}" do let_it_be(:branch_ref_name) { "master" } - let_it_be(:branch_pipeline_schedule) do + let_it_be(:pipeline_schedule) do create(:ci_pipeline_schedule, :nightly, project: project, ref: branch_ref) end - where(:push_access_level, :merge_access_level, :project_role, :accessible) do - :no_one_can_push | :no_one_can_merge | :owner | :be_disallowed - :no_one_can_push | :no_one_can_merge | :maintainer | :be_disallowed - :no_one_can_push | :no_one_can_merge | :developer | :be_disallowed - :no_one_can_push | :no_one_can_merge | :reporter | :be_disallowed - :no_one_can_push | :no_one_can_merge | :guest | :be_disallowed - - :maintainers_can_push | :no_one_can_merge | :owner | :be_allowed - :maintainers_can_push | :no_one_can_merge | :maintainer | :be_allowed - :maintainers_can_push | :no_one_can_merge | :developer | :be_disallowed - :maintainers_can_push | :no_one_can_merge | :reporter | :be_disallowed - :maintainers_can_push | :no_one_can_merge | :guest | :be_disallowed - - :developers_can_push | :no_one_can_merge | :owner | :be_allowed - :developers_can_push | :no_one_can_merge | :maintainer | :be_allowed - :developers_can_push | :no_one_can_merge | :developer | :be_allowed - :developers_can_push | :no_one_can_merge | :reporter | :be_disallowed - :developers_can_push | :no_one_can_merge | :guest | :be_disallowed - - :no_one_can_push | :maintainers_can_merge | :owner | :be_allowed - :no_one_can_push | :maintainers_can_merge | :maintainer | :be_allowed - :no_one_can_push | :maintainers_can_merge | :developer | :be_disallowed - :no_one_can_push | :maintainers_can_merge | :reporter | :be_disallowed - :no_one_can_push | :maintainers_can_merge | :guest | :be_disallowed - - :maintainers_can_push | :maintainers_can_merge | :owner | :be_allowed - :maintainers_can_push | :maintainers_can_merge | :maintainer | :be_allowed - :maintainers_can_push | :maintainers_can_merge | :developer | :be_disallowed - :maintainers_can_push | :maintainers_can_merge | :reporter | :be_disallowed - :maintainers_can_push | :maintainers_can_merge | :guest | :be_disallowed - - :developers_can_push | :maintainers_can_merge | :owner | :be_allowed - :developers_can_push | :maintainers_can_merge | :maintainer | :be_allowed - :developers_can_push | :maintainers_can_merge | :developer | :be_allowed - :developers_can_push | :maintainers_can_merge | :reporter | :be_disallowed - :developers_can_push | :maintainers_can_merge | :guest | :be_disallowed - - :no_one_can_push | :developers_can_merge | :owner | :be_allowed - :no_one_can_push | :developers_can_merge | :maintainer | :be_allowed - :no_one_can_push | :developers_can_merge | :developer | :be_allowed - :no_one_can_push | :developers_can_merge | :reporter | :be_disallowed - :no_one_can_push | :developers_can_merge | :guest | :be_disallowed - - :maintainers_can_push | :developers_can_merge | :owner | :be_allowed - :maintainers_can_push | :developers_can_merge | :maintainer | :be_allowed - :maintainers_can_push | :developers_can_merge | :developer | :be_allowed - :maintainers_can_push | :developers_can_merge | :reporter | :be_disallowed - :maintainers_can_push | :developers_can_merge | :guest | :be_disallowed - - :developers_can_push | :developers_can_merge | :owner | :be_allowed - :developers_can_push | :developers_can_merge | :maintainer | :be_allowed - :developers_can_push | :developers_can_merge | :developer | :be_allowed - :developers_can_push | :developers_can_merge | :reporter | :be_disallowed - :developers_can_push | :developers_can_merge | :guest | :be_disallowed + shared_examples_for 'allowed by those who can update the branch' do + where(:push_access_level, :merge_access_level, :project_role, :accessible) do + :no_one_can_push | :no_one_can_merge | :owner | :be_disallowed + :no_one_can_push | :no_one_can_merge | :maintainer | :be_disallowed + :no_one_can_push | :no_one_can_merge | :developer | :be_disallowed + :no_one_can_push | :no_one_can_merge | :reporter | :be_disallowed + :no_one_can_push | :no_one_can_merge | :guest | :be_disallowed + + :maintainers_can_push | :no_one_can_merge | :owner | :be_allowed + :maintainers_can_push | :no_one_can_merge | :maintainer | :be_allowed + :maintainers_can_push | :no_one_can_merge | :developer | :be_disallowed + :maintainers_can_push | :no_one_can_merge | :reporter | :be_disallowed + :maintainers_can_push | :no_one_can_merge | :guest | :be_disallowed + + :developers_can_push | :no_one_can_merge | :owner | :be_allowed + :developers_can_push | :no_one_can_merge | :maintainer | :be_allowed + :developers_can_push | :no_one_can_merge | :developer | :be_allowed + :developers_can_push | :no_one_can_merge | :reporter | :be_disallowed + :developers_can_push | :no_one_can_merge | :guest | :be_disallowed + + :no_one_can_push | :maintainers_can_merge | :owner | :be_allowed + :no_one_can_push | :maintainers_can_merge | :maintainer | :be_allowed + :no_one_can_push | :maintainers_can_merge | :developer | :be_disallowed + :no_one_can_push | :maintainers_can_merge | :reporter | :be_disallowed + :no_one_can_push | :maintainers_can_merge | :guest | :be_disallowed + + :maintainers_can_push | :maintainers_can_merge | :owner | :be_allowed + :maintainers_can_push | :maintainers_can_merge | :maintainer | :be_allowed + :maintainers_can_push | :maintainers_can_merge | :developer | :be_disallowed + :maintainers_can_push | :maintainers_can_merge | :reporter | :be_disallowed + :maintainers_can_push | :maintainers_can_merge | :guest | :be_disallowed + + :developers_can_push | :maintainers_can_merge | :owner | :be_allowed + :developers_can_push | :maintainers_can_merge | :maintainer | :be_allowed + :developers_can_push | :maintainers_can_merge | :developer | :be_allowed + :developers_can_push | :maintainers_can_merge | :reporter | :be_disallowed + :developers_can_push | :maintainers_can_merge | :guest | :be_disallowed + + :no_one_can_push | :developers_can_merge | :owner | :be_allowed + :no_one_can_push | :developers_can_merge | :maintainer | :be_allowed + :no_one_can_push | :developers_can_merge | :developer | :be_allowed + :no_one_can_push | :developers_can_merge | :reporter | :be_disallowed + :no_one_can_push | :developers_can_merge | :guest | :be_disallowed + + :maintainers_can_push | :developers_can_merge | :owner | :be_allowed + :maintainers_can_push | :developers_can_merge | :maintainer | :be_allowed + :maintainers_can_push | :developers_can_merge | :developer | :be_allowed + :maintainers_can_push | :developers_can_merge | :reporter | :be_disallowed + :maintainers_can_push | :developers_can_merge | :guest | :be_disallowed + + :developers_can_push | :developers_can_merge | :owner | :be_allowed + :developers_can_push | :developers_can_merge | :maintainer | :be_allowed + :developers_can_push | :developers_can_merge | :developer | :be_allowed + :developers_can_push | :developers_can_merge | :reporter | :be_disallowed + :developers_can_push | :developers_can_merge | :guest | :be_disallowed + end + + with_them do + before do + create(:protected_branch, push_access_level, merge_access_level, name: branch_ref_name, + project: project) + project.add_role(user, project_role) + end + + it { expect(policy).to try(accessible, :create_pipeline_schedule) } + end end - with_them do - before do - create(:protected_branch, push_access_level, merge_access_level, name: branch_ref_name, - project: project) - project.add_role(user, project_role) + shared_examples_for 'only allowed by schedule owners who can update the branch' do + where(:push_access_level, :merge_access_level, :schedule_owner, :project_role, :accessible) do + :no_one_can_push | :no_one_can_merge | :other_user | :owner | :be_disallowed + :no_one_can_push | :no_one_can_merge | :user | :owner | :be_disallowed + :no_one_can_push | :no_one_can_merge | :user | :maintainer | :be_disallowed + :no_one_can_push | :no_one_can_merge | :user | :developer | :be_disallowed + :no_one_can_push | :no_one_can_merge | :user | :reporter | :be_disallowed + :no_one_can_push | :no_one_can_merge | :user | :guest | :be_disallowed + + :maintainers_can_push | :no_one_can_merge | :other_user | :owner | :be_disallowed + :maintainers_can_push | :no_one_can_merge | :user | :owner | :be_allowed + :maintainers_can_push | :no_one_can_merge | :user | :maintainer | :be_allowed + :maintainers_can_push | :no_one_can_merge | :user | :developer | :be_disallowed + :maintainers_can_push | :no_one_can_merge | :user | :reporter | :be_disallowed + :maintainers_can_push | :no_one_can_merge | :user | :guest | :be_disallowed + + :developers_can_push | :no_one_can_merge | :other_user | :owner | :be_disallowed + :developers_can_push | :no_one_can_merge | :user | :owner | :be_allowed + :developers_can_push | :no_one_can_merge | :user | :maintainer | :be_allowed + :developers_can_push | :no_one_can_merge | :user | :developer | :be_allowed + :developers_can_push | :no_one_can_merge | :user | :reporter | :be_disallowed + :developers_can_push | :no_one_can_merge | :user | :guest | :be_disallowed + + :no_one_can_push | :maintainers_can_merge | :other_user | :owner | :be_disallowed + :no_one_can_push | :maintainers_can_merge | :user | :owner | :be_allowed + :no_one_can_push | :maintainers_can_merge | :user | :maintainer | :be_allowed + :no_one_can_push | :maintainers_can_merge | :user | :developer | :be_disallowed + :no_one_can_push | :maintainers_can_merge | :user | :reporter | :be_disallowed + :no_one_can_push | :maintainers_can_merge | :user | :guest | :be_disallowed + + :maintainers_can_push | :maintainers_can_merge | :other_user | :owner | :be_disallowed + :maintainers_can_push | :maintainers_can_merge | :user | :owner | :be_allowed + :maintainers_can_push | :maintainers_can_merge | :user | :maintainer | :be_allowed + :maintainers_can_push | :maintainers_can_merge | :user | :developer | :be_disallowed + :maintainers_can_push | :maintainers_can_merge | :user | :reporter | :be_disallowed + :maintainers_can_push | :maintainers_can_merge | :user | :guest | :be_disallowed + + :developers_can_push | :maintainers_can_merge | :other_user | :owner | :be_disallowed + :developers_can_push | :maintainers_can_merge | :user | :owner | :be_allowed + :developers_can_push | :maintainers_can_merge | :user | :maintainer | :be_allowed + :developers_can_push | :maintainers_can_merge | :user | :developer | :be_allowed + :developers_can_push | :maintainers_can_merge | :user | :reporter | :be_disallowed + :developers_can_push | :maintainers_can_merge | :user | :guest | :be_disallowed + + :no_one_can_push | :developers_can_merge | :other_user | :owner | :be_disallowed + :no_one_can_push | :developers_can_merge | :user | :owner | :be_allowed + :no_one_can_push | :developers_can_merge | :user | :maintainer | :be_allowed + :no_one_can_push | :developers_can_merge | :user | :developer | :be_allowed + :no_one_can_push | :developers_can_merge | :user | :reporter | :be_disallowed + :no_one_can_push | :developers_can_merge | :user | :guest | :be_disallowed + + :maintainers_can_push | :developers_can_merge | :other_user | :owner | :be_disallowed + :maintainers_can_push | :developers_can_merge | :user | :owner | :be_allowed + :maintainers_can_push | :developers_can_merge | :user | :maintainer | :be_allowed + :maintainers_can_push | :developers_can_merge | :user | :developer | :be_allowed + :maintainers_can_push | :developers_can_merge | :user | :reporter | :be_disallowed + :maintainers_can_push | :developers_can_merge | :user | :guest | :be_disallowed + + :developers_can_push | :developers_can_merge | :other_user | :owner | :be_disallowed + :developers_can_push | :developers_can_merge | :user | :owner | :be_allowed + :developers_can_push | :developers_can_merge | :user | :maintainer | :be_allowed + :developers_can_push | :developers_can_merge | :user | :developer | :be_allowed + :developers_can_push | :developers_can_merge | :user | :reporter | :be_disallowed + :developers_can_push | :developers_can_merge | :user | :guest | :be_disallowed end - context 'for create_pipeline_schedule' do - subject(:policy) { described_class.new(user, new_branch_pipeline_schedule) } + with_them do + before do + create(:protected_branch, push_access_level, merge_access_level, name: branch_ref_name, + project: project) + project.add_role(user, project_role) + project.add_role(other_user, project_role) - let(:new_branch_pipeline_schedule) { project.pipeline_schedules.new(ref: branch_ref) } + pipeline_schedule.owner = schedule_owner == :user ? user : other_user + end - it { expect(policy).to try(accessible, :create_pipeline_schedule) } + it { expect(policy).to try(accessible, ability_name) } end + end - context 'for play_pipeline_schedule' do - subject(:policy) { described_class.new(user, branch_pipeline_schedule) } + describe 'create_pipeline_schedule' do + let(:ability_name) { :create_pipeline_schedule } + let(:pipeline_schedule) { project.pipeline_schedules.new(ref: branch_ref) } - it { expect(policy).to try(accessible, :play_pipeline_schedule) } - end + it_behaves_like 'allowed by those who can update the branch' + end + + describe 'play_pipeline_schedule' do + let(:ability_name) { :play_pipeline_schedule } + + it_behaves_like 'allowed by those who can update the branch' + end + + describe 'update_pipeline_schedule' do + let(:ability_name) { :update_pipeline_schedule } + + it_behaves_like 'only allowed by schedule owners who can update the branch' end end end @@ -108,49 +200,97 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache, f context 'for tag' do %w[refs/tags/v1.0.0 v1.0.0].each do |tag_ref| context "with #{tag_ref}" do - let_it_be(:tag_pipeline_schedule) do + let_it_be(:pipeline_schedule) do create(:ci_pipeline_schedule, :nightly, project: project, ref: tag_ref) end - where(:access_level, :project_role, :accessible) do - :no_one_can_create | :owner | :be_disallowed - :no_one_can_create | :maintainer | :be_disallowed - :no_one_can_create | :developer | :be_disallowed - :no_one_can_create | :reporter | :be_disallowed - :no_one_can_create | :guest | :be_disallowed - - :maintainers_can_create | :owner | :be_allowed - :maintainers_can_create | :maintainer | :be_allowed - :maintainers_can_create | :developer | :be_disallowed - :maintainers_can_create | :reporter | :be_disallowed - :maintainers_can_create | :guest | :be_disallowed - - :developers_can_create | :owner | :be_allowed - :developers_can_create | :maintainer | :be_allowed - :developers_can_create | :developer | :be_allowed - :developers_can_create | :reporter | :be_disallowed - :developers_can_create | :guest | :be_disallowed + subject(:policy) { described_class.new(user, pipeline_schedule) } + + shared_examples_for 'allowed by those who can update the tag' do + where(:access_level, :project_role, :accessible) do + :no_one_can_create | :owner | :be_disallowed + :no_one_can_create | :maintainer | :be_disallowed + :no_one_can_create | :developer | :be_disallowed + :no_one_can_create | :reporter | :be_disallowed + :no_one_can_create | :guest | :be_disallowed + + :maintainers_can_create | :owner | :be_allowed + :maintainers_can_create | :maintainer | :be_allowed + :maintainers_can_create | :developer | :be_disallowed + :maintainers_can_create | :reporter | :be_disallowed + :maintainers_can_create | :guest | :be_disallowed + + :developers_can_create | :owner | :be_allowed + :developers_can_create | :maintainer | :be_allowed + :developers_can_create | :developer | :be_allowed + :developers_can_create | :reporter | :be_disallowed + :developers_can_create | :guest | :be_disallowed + end + + with_them do + before do + create(:protected_tag, access_level, name: tag_ref_name, project: project) + project.add_role(user, project_role) + end + + it { expect(policy).to try(accessible, ability_name) } + end end - with_them do - before do - create(:protected_tag, access_level, name: tag_ref_name, project: project) - project.add_role(user, project_role) + shared_examples_for 'only allowed by schedule owners who can update the tag' do + where(:access_level, :schedule_owner, :project_role, :accessible) do + :no_one_can_create | :other_user | :owner | :be_disallowed + :no_one_can_create | :user | :owner | :be_disallowed + :no_one_can_create | :user | :maintainer | :be_disallowed + :no_one_can_create | :user | :developer | :be_disallowed + :no_one_can_create | :user | :reporter | :be_disallowed + :no_one_can_create | :user | :guest | :be_disallowed + + :maintainers_can_create | :other_user | :owner | :be_disallowed + :maintainers_can_create | :user | :owner | :be_allowed + :maintainers_can_create | :user | :maintainer | :be_allowed + :maintainers_can_create | :user | :developer | :be_disallowed + :maintainers_can_create | :user | :reporter | :be_disallowed + :maintainers_can_create | :user | :guest | :be_disallowed + + :developers_can_create | :other_user | :owner | :be_disallowed + :developers_can_create | :user | :owner | :be_allowed + :developers_can_create | :user | :maintainer | :be_allowed + :developers_can_create | :user | :developer | :be_allowed + :developers_can_create | :user | :reporter | :be_disallowed + :developers_can_create | :user | :guest | :be_disallowed end - context 'for create_pipeline_schedule' do - subject(:policy) { described_class.new(user, new_tag_pipeline_schedule) } + with_them do + before do + create(:protected_tag, access_level, name: tag_ref_name, project: project) + project.add_role(user, project_role) + project.add_role(other_user, project_role) - let(:new_tag_pipeline_schedule) { project.pipeline_schedules.new(ref: tag_ref) } + pipeline_schedule.owner = schedule_owner == :user ? user : other_user + end - it { expect(policy).to try(accessible, :create_pipeline_schedule) } + it { expect(policy).to try(accessible, ability_name) } end + end - context 'for play_pipeline_schedule' do - subject(:policy) { described_class.new(user, tag_pipeline_schedule) } + describe 'create_pipeline_schedule' do + let(:ability_name) { :create_pipeline_schedule } + let(:pipeline_schedule) { project.pipeline_schedules.new(ref: tag_ref) } - it { expect(policy).to try(accessible, :play_pipeline_schedule) } - end + it_behaves_like 'allowed by those who can update the tag' + end + + describe 'play_pipeline_schedule' do + let(:ability_name) { :play_pipeline_schedule } + + it_behaves_like 'allowed by those who can update the tag' + end + + describe 'update_pipeline_schedule' do + let(:ability_name) { :update_pipeline_schedule } + + it_behaves_like 'only allowed by schedule owners who can update the tag' end end end diff --git a/spec/services/ci/pipeline_schedules/update_service_spec.rb b/spec/services/ci/pipeline_schedules/update_service_spec.rb index 834bbcfcfeb..b84afacdcff 100644 --- a/spec/services/ci/pipeline_schedules/update_service_spec.rb +++ b/spec/services/ci/pipeline_schedules/update_service_spec.rb @@ -7,16 +7,16 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo let_it_be_with_reload(:project) { create(:project, :public, :repository) } let_it_be_with_reload(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } let_it_be(:reporter) { create(:user) } + let_it_be(:project_owner) { create(:user) } let_it_be(:pipeline_schedule_variable) do create(:ci_pipeline_schedule_variable, key: 'foo', value: 'foovalue', pipeline_schedule: pipeline_schedule) end - subject(:service) { described_class.new(pipeline_schedule, user, params) } - before_all do project.add_maintainer(user) + project.add_owner(project_owner) project.add_reporter(reporter) pipeline_schedule.reload @@ -54,8 +54,10 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo subject(:service) { described_class.new(pipeline_schedule, user, params) } it 'updates database values with passed params' do - expect { service.execute } - .to change { pipeline_schedule.description }.from('pipeline schedule').to('updated_desc') + expect do + service.execute + pipeline_schedule.reload + end.to change { pipeline_schedule.description }.from('pipeline schedule').to('updated_desc') .and change { pipeline_schedule.ref }.from('master').to('patch-x') .and change { pipeline_schedule.active }.from(true).to(false) .and change { pipeline_schedule.cron }.from('0 1 * * *').to('*/1 * * * *') @@ -63,6 +65,48 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo .and change { pipeline_schedule.variables.last.value }.from('foovalue').to('barvalue') end + context 'when the new branch is protected', :request_store do + let(:maintainer_access) { :no_one_can_merge } + + before do + create(:protected_branch, :no_one_can_push, maintainer_access, name: 'patch-x', project: project) + end + + after do + ProtectedBranches::CacheService.new(project).refresh + end + + context 'when called by someone other than the schedule owner who can update the ref' do + let(:maintainer_access) { :maintainers_can_merge } + + subject(:service) { described_class.new(pipeline_schedule, project_owner, params) } + + it 'does not update the schedule' do + expect do + service.execute + pipeline_schedule.reload + end.not_to change { pipeline_schedule.description } + end + end + + context 'when called by the schedule owner' do + it 'does not update the schedule' do + expect do + service.execute + pipeline_schedule.reload + end.not_to change { pipeline_schedule.description } + end + + context 'when the owner can update the ref' do + let(:maintainer_access) { :maintainers_can_merge } + + it 'updates the schedule' do + expect { service.execute }.to change { pipeline_schedule.description } + end + end + end + end + context 'when creating a variable' do let(:params) do { @@ -126,6 +170,8 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo end end - it_behaves_like 'pipeline schedules checking variables permission' + it_behaves_like 'pipeline schedules checking variables permission' do + subject(:service) { described_class.new(pipeline_schedule, user, params) } + end end end -- cgit v1.2.3