Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
path: root/spec
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2023-11-29 19:29:06 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2023-11-29 19:29:17 +0300
commitbc77a6adefcd02a058e3d5a10718d74a5dad954e (patch)
tree07969835b1e8ce3727fb74fa33f1ad8dabd37149 /spec
parent706c0db2fcea38033a4b23e9434461127deb87e1 (diff)
Add latest changes from gitlab-org/security/gitlab@16-6-stable-ee
Diffstat (limited to 'spec')
-rw-r--r--spec/controllers/projects/pipeline_schedules_controller_spec.rb4
-rw-r--r--spec/helpers/groups_helper_spec.rb63
-rw-r--r--spec/models/ability_spec.rb39
-rw-r--r--spec/policies/ci/pipeline_schedule_policy_spec.rb338
-rw-r--r--spec/services/ci/pipeline_schedules/update_service_spec.rb56
5 files changed, 393 insertions, 107 deletions
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