diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-07-31 17:26:56 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-07-31 17:26:56 +0300 |
commit | 49db4c357bfc82792c80f47d545b799fc373a868 (patch) | |
tree | c7f863091c733da4b57bb9d63455305f909e3c26 /spec | |
parent | 9456b286c3e84d50dfb6bab23d9556c00480eaf0 (diff) |
Add latest changes from gitlab-org/security/gitlab@16-1-stable-ee
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/pipeline_schedules_controller_spec.rb | 69 | ||||
-rw-r--r-- | spec/factories/diff_position.rb | 4 | ||||
-rw-r--r-- | spec/features/merge_request/user_views_comment_on_diff_file_spec.rb | 49 | ||||
-rw-r--r-- | spec/lib/banzai/filter/autolink_filter_spec.rb | 16 | ||||
-rw-r--r-- | spec/lib/banzai/filter/references/project_reference_filter_spec.rb | 1 | ||||
-rw-r--r-- | spec/lib/gitlab/checks/branch_check_spec.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/harbor/query_spec.rb | 23 | ||||
-rw-r--r-- | spec/lib/gitlab/pages/virtual_host_finder_spec.rb | 29 | ||||
-rw-r--r-- | spec/models/project_setting_spec.rb | 26 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 58 | ||||
-rw-r--r-- | spec/policies/ci/pipeline_schedule_policy_spec.rb | 185 | ||||
-rw-r--r-- | spec/services/ci/pipeline_schedules/update_service_spec.rb | 4 |
12 files changed, 391 insertions, 79 deletions
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index 6d810fdcd51..4aa2bac5259 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Projects::PipelineSchedulesController, feature_category: :continuous_integration do include AccessMatchersForController + using RSpec::Parameterized::TableSyntax let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :public, :repository) } @@ -45,6 +46,43 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end end + shared_examples 'protecting ref' do + where(:branch_access_levels, :tag_access_level, :maintainer_accessible, :developer_accessible) do + [:no_one_can_push, :no_one_can_merge] | :no_one_can_create | \ + :be_denied_for | :be_denied_for + [:maintainers_can_push, :maintainers_can_merge] | :maintainers_can_create | \ + :be_allowed_for | :be_denied_for + [:developers_can_push, :developers_can_merge] | :developers_can_create | \ + :be_allowed_for | :be_allowed_for + end + + with_them do + context 'when branch is protected' do + let(:ref_prefix) { 'heads' } + let(:ref_name) { 'master' } + + before do + create(:protected_branch, *branch_access_levels, name: ref_name, project: project) + end + + it { expect { go }.to try(maintainer_accessible, :maintainer).of(project) } + it { expect { go }.to try(developer_accessible, :developer).of(project) } + end + + context 'when tag is protected' do + let(:ref_prefix) { 'tags' } + let(:ref_name) { 'v1.0.0' } + + before do + create(:protected_tag, tag_access_level, name: ref_name, project: project) + end + + it { expect { go }.to try(maintainer_accessible, :maintainer).of(project) } + it { expect { go }.to try(developer_accessible, :developer).of(project) } + end + end + end + describe 'GET #index' do render_views @@ -158,7 +196,9 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end describe 'security' do - let(:schedule) { attributes_for(:ci_pipeline_schedule) } + let(:schedule) { attributes_for(:ci_pipeline_schedule, ref: "refs/#{ref_prefix}/#{ref_name}") } + let(:ref_prefix) { 'heads' } + let(:ref_name) { "master" } it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) @@ -177,6 +217,8 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:external) } it { expect { go }.to be_denied_for(:visitor) } + + it_behaves_like 'protecting ref' end def go @@ -427,7 +469,7 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end describe 'POST #play', :clean_gitlab_redis_rate_limiting do - let(:ref) { 'master' } + let(:ref_name) { 'master' } before do project.add_developer(user) @@ -443,7 +485,7 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu it 'does not allow pipeline to be executed' do expect(RunPipelineScheduleWorker).not_to receive(:perform_async) - post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } + go expect(response).to have_gitlab_http_status(:not_found) end @@ -453,16 +495,14 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu it 'executes a new pipeline' do expect(RunPipelineScheduleWorker).to receive(:perform_async).with(pipeline_schedule.id, user.id).and_return('job-123') - post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } + go expect(flash[:notice]).to start_with 'Successfully scheduled a pipeline to run' expect(response).to have_gitlab_http_status(:found) end it 'prevents users from scheduling the same pipeline repeatedly' do - 2.times do - post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } - end + 2.times { go } expect(flash.to_a.size).to eq(2) expect(flash[:alert]).to eq _('You cannot play this scheduled pipeline at the moment. Please wait a minute.') @@ -470,17 +510,14 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end end - context 'when a developer attempts to schedule a protected ref' do - it 'does not allow pipeline to be executed' do - create(:protected_branch, project: project, name: ref) - protected_schedule = create(:ci_pipeline_schedule, project: project, ref: ref) - - expect(RunPipelineScheduleWorker).not_to receive(:perform_async) + describe 'security' do + let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, ref: "refs/#{ref_prefix}/#{ref_name}") } - post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: protected_schedule.id } + it_behaves_like 'protecting ref' + end - expect(response).to have_gitlab_http_status(:not_found) - end + def go + post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } end end diff --git a/spec/factories/diff_position.rb b/spec/factories/diff_position.rb index bd248452de8..87875e0fa4d 100644 --- a/spec/factories/diff_position.rb +++ b/spec/factories/diff_position.rb @@ -54,6 +54,10 @@ FactoryBot.define do end end + factory :file_diff_position do + position_type { 'file' } + end + factory :image_diff_position do position_type { 'image' } x { 1 } diff --git a/spec/features/merge_request/user_views_comment_on_diff_file_spec.rb b/spec/features/merge_request/user_views_comment_on_diff_file_spec.rb new file mode 100644 index 00000000000..26dbf5605ec --- /dev/null +++ b/spec/features/merge_request/user_views_comment_on_diff_file_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User views comment on a diff file', :js, feature_category: :code_review_workflow do + include MergeRequestDiffHelpers + include RepoHelpers + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:merge_request) do + create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test') + end + + let_it_be(:user) { create(:user) } + + before_all do + project.add_maintainer(user) + end + + before do + sign_in(user) + + visit(diffs_project_merge_request_path(project, merge_request)) + end + + context 'with invalid start_sha position' do + before do + diff_refs = Gitlab::Diff::DiffRefs.new( + base_sha: merge_request.diff_refs.base_sha, + start_sha: 'fakesha', + head_sha: merge_request.diff_refs.head_sha + ) + position = build(:file_diff_position, file: 'files/ruby/popen.rb', diff_refs: diff_refs) + create(:diff_note_on_merge_request, noteable: merge_request, project: project, position: position) + end + + it 'renders diffs' do + visit diffs_project_merge_request_path(project, merge_request) + + expect(page).to have_selector('.diff-file') + end + + it 'renders discussion on overview tab' do + visit project_merge_request_path(project, merge_request) + + expect(page).to have_selector('.note-discussion') + end + end +end diff --git a/spec/lib/banzai/filter/autolink_filter_spec.rb b/spec/lib/banzai/filter/autolink_filter_spec.rb index 2c75377ec42..e033567753c 100644 --- a/spec/lib/banzai/filter/autolink_filter_spec.rb +++ b/spec/lib/banzai/filter/autolink_filter_spec.rb @@ -226,6 +226,22 @@ RSpec.describe Banzai::Filter::AutolinkFilter, feature_category: :team_planning end end + it 'protects against malicious backtracking' do + doc = "http://#{'&' * 1_000_000}x" + + expect do + Timeout.timeout(30.seconds) { filter(doc) } + end.not_to raise_error + end + + it 'does not timeout with excessively long scheme' do + doc = "#{'h' * 1_000_000}://example.com" + + expect do + Timeout.timeout(30.seconds) { filter(doc) } + end.not_to raise_error + end + # Rinku does not escape these characters in HTML attributes, but content_tag # does. We don't care about that difference for these specs, though. def unescape(html) diff --git a/spec/lib/banzai/filter/references/project_reference_filter_spec.rb b/spec/lib/banzai/filter/references/project_reference_filter_spec.rb index 49c71d08364..b6d6ff2309a 100644 --- a/spec/lib/banzai/filter/references/project_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/project_reference_filter_spec.rb @@ -39,6 +39,7 @@ RSpec.describe Banzai::Filter::References::ProjectReferenceFilter, feature_categ it_behaves_like 'fails fast', 'A' * 50000 it_behaves_like 'fails fast', '/a' * 50000 + it_behaves_like 'fails fast', "mailto:#{'a-' * 499_000}@aaaaaaaa..aaaaaaaa.example.com" end it 'allows references with text after the > character' do diff --git a/spec/lib/gitlab/checks/branch_check_spec.rb b/spec/lib/gitlab/checks/branch_check_spec.rb index 7ce267c535f..9950d4dbd12 100644 --- a/spec/lib/gitlab/checks/branch_check_spec.rb +++ b/spec/lib/gitlab/checks/branch_check_spec.rb @@ -32,6 +32,12 @@ RSpec.describe Gitlab::Checks::BranchCheck, feature_category: :source_code_manag expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You cannot create a branch with a 40-character hexadecimal branch name.") end + it "prohibits 40-character hexadecimal branch names followed by a dash as the start of a path" do + allow(subject).to receive(:branch_name).and_return("267208abfe40e546f5e847444276f7d43a39503e-/test") + + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You cannot create a branch with a 40-character hexadecimal branch name.") + end + it "doesn't prohibit a nested hexadecimal in a branch name" do allow(subject).to receive(:branch_name).and_return("267208abfe40e546f5e847444276f7d43a39503e-fix") diff --git a/spec/lib/gitlab/harbor/query_spec.rb b/spec/lib/gitlab/harbor/query_spec.rb index dcb9a16b27b..fe05643e04a 100644 --- a/spec/lib/gitlab/harbor/query_spec.rb +++ b/spec/lib/gitlab/harbor/query_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::Harbor::Query do + using RSpec::Parameterized::TableSyntax + let_it_be(:harbor_integration) { create(:harbor_integration) } let(:params) { {} } @@ -111,19 +113,20 @@ RSpec.describe Gitlab::Harbor::Query do end context 'search' do - context 'with valid search' do - let(:params) { { search: 'name=desc' } } - - it 'initialize successfully' do - expect(query.valid?).to eq(true) - end + where(:search_param, :is_valid) do + "name=desc" | true + "name=value1,name=value-2" | true + "name=value1,name=value_2" | false + "name=desc,key=value" | false + "name=value1, name=value2" | false + "name" | false end - context 'with invalid search' do - let(:params) { { search: 'blabla' } } + with_them do + let(:params) { { search: search_param } } - it 'initialize failed' do - expect(query.valid?).to eq(false) + it "validates according to the regex" do + expect(query.valid?).to eq(is_valid) end end end diff --git a/spec/lib/gitlab/pages/virtual_host_finder_spec.rb b/spec/lib/gitlab/pages/virtual_host_finder_spec.rb index 4b584a45503..49eee772f8d 100644 --- a/spec/lib/gitlab/pages/virtual_host_finder_spec.rb +++ b/spec/lib/gitlab/pages/virtual_host_finder_spec.rb @@ -9,6 +9,10 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do project.update_pages_deployment!(create(:pages_deployment, project: project)) end + before do + stub_pages_setting(host: 'example.com') + end + it 'returns nil when host is empty' do expect(described_class.new(nil).execute).to be_nil expect(described_class.new('').execute).to be_nil @@ -69,7 +73,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end it 'returns the virual domain with no lookup_paths' do - virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}").execute + virtual_domain = described_class.new("#{project.namespace.path}.example.com").execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to match(/pages_domain_for_namespace_#{project.namespace.id}_/) @@ -82,7 +86,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end it 'returns the virual domain with no lookup_paths' do - virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}".downcase).execute + virtual_domain = described_class.new("#{project.namespace.path}.example.com".downcase).execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to be_nil @@ -104,7 +108,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end it 'returns the virual domain when there are pages deployed for the project' do - virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}").execute + virtual_domain = described_class.new("#{project.namespace.path}.example.com").execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to match(/pages_domain_for_namespace_#{project.namespace.id}_/) @@ -113,7 +117,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end it 'finds domain with case-insensitive' do - virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host.upcase}").execute + virtual_domain = described_class.new("#{project.namespace.path}.Example.com").execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to match(/pages_domain_for_namespace_#{project.namespace.id}_/) @@ -127,7 +131,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end it 'returns the virual domain when there are pages deployed for the project' do - virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}").execute + virtual_domain = described_class.new("#{project.namespace.path}.example.com").execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to be_nil @@ -143,7 +147,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do project.project_setting.update!(pages_unique_domain: 'unique-domain') end - subject(:virtual_domain) { described_class.new("unique-domain.#{Settings.pages.host.upcase}").execute } + subject(:virtual_domain) { described_class.new('unique-domain.example.com').execute } context 'when pages unique domain is enabled' do before_all do @@ -171,6 +175,19 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do expect(virtual_domain.lookup_paths.first.project_id).to eq(project.id) end + context 'when a project path conflicts with a unique domain' do + it 'prioritizes the unique domain project' do + group = create(:group, path: 'unique-domain') + other_project = build(:project, path: 'unique-domain.example.com', group: group) + other_project.save!(validate: false) + other_project.update_pages_deployment!(create(:pages_deployment, project: other_project)) + other_project.mark_pages_as_deployed + + expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) + expect(virtual_domain.lookup_paths.first.project_id).to eq(project.id) + end + end + context 'when :cache_pages_domain_api is disabled' do before do stub_feature_flags(cache_pages_domain_api: false) diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 4b2760d7699..c020609c26f 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -77,12 +77,36 @@ RSpec.describe ProjectSetting, type: :model, feature_category: :groups_and_proje expect(project_setting).not_to be_valid expect(project_setting.errors.full_messages).to include("Pages unique domain has already been taken") end + + it "validates if the pages_unique_domain already exist as a project path" do + stub_pages_setting(host: 'example.com') + + create(:project, path: "random-unique-domain.example.com") + project_setting = build(:project_setting, pages_unique_domain: "random-unique-domain") + + expect(project_setting).not_to be_valid + expect(project_setting.errors.full_messages_for(:pages_unique_domain)) + .to match(["Pages unique domain already in use"]) + end + + context "when updating" do + it "validates if the pages_unique_domain already exist as a project path" do + stub_pages_setting(host: 'example.com') + project_setting = create(:project_setting) + + create(:project, path: "random-unique-domain.example.com") + + expect(project_setting.update(pages_unique_domain: "random-unique-domain")).to eq(false) + expect(project_setting.errors.full_messages_for(:pages_unique_domain)) + .to match(["Pages unique domain already in use"]) + end + end end describe 'target_platforms=' do it 'stringifies and sorts' do project_setting = build(:project_setting, target_platforms: [:watchos, :ios]) - expect(project_setting.target_platforms).to eq %w(ios watchos) + expect(project_setting.target_platforms).to eq %w[ios watchos] end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f44331521e9..6bf86558f1f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -827,6 +827,37 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect(project).to be_valid end + context 'when validating if path already exist as pages unique domain' do + before do + stub_pages_setting(host: 'example.com') + end + + it 'rejects paths that match pages unique domain' do + create(:project_setting, pages_unique_domain: 'some-unique-domain') + + project = build(:project, path: 'some-unique-domain.example.com') + + expect(project).not_to be_valid + expect(project.errors.full_messages_for(:path)).to match(['Path already in use']) + end + + it 'accepts path when the host does not match' do + create(:project_setting, pages_unique_domain: 'some-unique-domain') + + project = build(:project, path: 'some-unique-domain.another-example.com') + + expect(project).to be_valid + end + + it 'accepts path when the domain does not match' do + create(:project_setting, pages_unique_domain: 'another-unique-domain') + + project = build(:project, path: 'some-unique-domain.example.com') + + expect(project).to be_valid + end + end + context 'path is unchanged' do let_it_be(:invalid_path_project) do project = create(:project, :repository, :public) @@ -4921,6 +4952,33 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) end + context 'when validating if path already exist as pages unique domain' do + before do + stub_pages_setting(host: 'example.com') + end + + it 'rejects paths that match pages unique domain' do + stub_pages_setting(host: 'example.com') + create(:project_setting, pages_unique_domain: 'some-unique-domain') + + expect(project.update(path: 'some-unique-domain.example.com')).to eq(false) + expect(project.errors.full_messages_for(:path)).to match(['Path already in use']) + end + + it 'accepts path when the host does not match' do + create(:project_setting, pages_unique_domain: 'some-unique-domain') + + expect(project.update(path: 'some-unique-domain.another-example.com')).to eq(true) + end + + it 'accepts path when the domain does not match' do + stub_pages_setting(host: 'example.com') + create(:project_setting, pages_unique_domain: 'another-unique-domain') + + expect(project.update(path: 'some-unique-domain.example.com')).to eq(true) + end + end + it 'does not validate the visibility' do expect(project).not_to receive(:visibility_level_allowed_as_fork).and_call_original expect(project).not_to receive(:visibility_level_allowed_by_group).and_call_original diff --git a/spec/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb index 7025eda1ba1..8fc5c6ca296 100644 --- a/spec/policies/ci/pipeline_schedule_policy_spec.rb +++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb @@ -2,10 +2,13 @@ require 'spec_helper' -RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache do +RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache, feature_category: :continuous_integration do + using RSpec::Parameterized::TableSyntax + let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :repository) } - let_it_be(:pipeline_schedule, reload: true) { create(:ci_pipeline_schedule, :nightly, project: project) } + 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" } let(:policy) do described_class.new(user, pipeline_schedule) @@ -13,51 +16,143 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache do describe 'rules' do describe 'rules for protected ref' do - before do - project.add_developer(user) - end - - context 'when no one can push or merge to the branch' do - before do - create(:protected_branch, :no_one_can_push, name: pipeline_schedule.ref, project: project) - end - - it 'does not include ability to play pipeline schedule' do - expect(policy).to be_disallowed :play_pipeline_schedule + context 'for branch' do + %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 + 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 + 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 + + context 'for create_pipeline_schedule' do + subject(:policy) { described_class.new(user, new_branch_pipeline_schedule) } + + let(:new_branch_pipeline_schedule) { project.pipeline_schedules.new(ref: branch_ref) } + + it { expect(policy).to try(accessible, :create_pipeline_schedule) } + end + + context 'for play_pipeline_schedule' do + subject(:policy) { described_class.new(user, branch_pipeline_schedule) } + + it { expect(policy).to try(accessible, :play_pipeline_schedule) } + end + end + end end end - context 'when developers can push to the branch' do - before do - create(:protected_branch, :developers_can_merge, name: pipeline_schedule.ref, project: project) - end - - it 'includes ability to update pipeline' do - expect(policy).to be_allowed :play_pipeline_schedule - end - end - - context 'when no one can create the tag' do - let(:tag) { 'v1.0.0' } - - before do - pipeline_schedule.update!(ref: tag) - - create(:protected_tag, :no_one_can_create, name: pipeline_schedule.ref, project: project) - end - - it 'does not include ability to play pipeline schedule' do - expect(policy).to be_disallowed :play_pipeline_schedule - end - end - - context 'when no one can create the tag but it is not a tag' do - before do - create(:protected_tag, :no_one_can_create, name: pipeline_schedule.ref, project: project) - end - - it 'includes ability to play pipeline schedule' do - expect(policy).to be_allowed :play_pipeline_schedule + 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 + 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 + end + + with_them do + before do + create(:protected_tag, access_level, name: tag_ref_name, project: project) + project.add_role(user, project_role) + end + + context 'for create_pipeline_schedule' do + subject(:policy) { described_class.new(user, new_tag_pipeline_schedule) } + + let(:new_tag_pipeline_schedule) { project.pipeline_schedules.new(ref: tag_ref) } + + it { expect(policy).to try(accessible, :create_pipeline_schedule) } + end + + context 'for play_pipeline_schedule' do + subject(:policy) { described_class.new(user, tag_pipeline_schedule) } + + it { expect(policy).to try(accessible, :play_pipeline_schedule) } + end + end + end 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 838f49f6dea..63c0c996a1a 100644 --- a/spec/services/ci/pipeline_schedules/update_service_spec.rb +++ b/spec/services/ci/pipeline_schedules/update_service_spec.rb @@ -6,7 +6,9 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo let_it_be(:user) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:project) { create(:project, :public, :repository) } - let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + let_it_be(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: user, ref: 'master') + end before_all do project.add_maintainer(user) |