From 538082a50b662c68f0953016cd26269af2b31ee2 Mon Sep 17 00:00:00 2001 From: Rajendra Kadam Date: Tue, 20 Feb 2024 10:31:00 +0530 Subject: [PATCH 1/3] Add access keyword in artifacts Make sure access and public keyword are not used at same level. Add specs to cover cases with the values for new keyword Access supports values as 'all', 'developer' and 'none'. 'all' is default. 'developer' is for project members including guest and reporters. 'none' will not allow anyone to access. Add documentation related to the new `access` keyword. Changelog: added MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145206 --- app/assets/javascripts/editor/schema/ci.json | 10 ++++ app/models/ci/build.rb | 18 +++++-- app/models/ci/job_artifact.rb | 6 ++- app/policies/ci/build_policy.rb | 6 ++- app/policies/ci/job_artifact_policy.rb | 6 ++- .../ci/job_artifacts/create_service.rb | 2 +- doc/ci/yaml/index.md | 29 ++++++++++++ .../policies/ci/job_artifact_policy_spec.rb | 40 ++++++++++++++++ lib/gitlab/ci/config/entry/artifacts.rb | 9 +++- spec/factories/ci/builds.rb | 44 +++++++++++++++++ spec/factories/ci/job_artifacts.rb | 4 ++ .../yaml_tests/negative_tests/artifacts.yml | 9 ++++ .../yaml_tests/positive_tests/artifacts.yml | 12 +++++ .../gitlab/ci/config/entry/artifacts_spec.rb | 29 ++++++++++++ spec/models/ci/build_spec.rb | 38 ++++++++++++--- spec/models/ci/job_artifact_spec.rb | 16 +++++++ spec/policies/ci/build_policy_spec.rb | 47 +++++++++++++++++++ .../ci/job_artifacts/create_service_spec.rb | 16 +++++++ 18 files changed, 327 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/editor/schema/ci.json b/app/assets/javascripts/editor/schema/ci.json index b5b0fd9721a45a..fd3623162b8f94 100644 --- a/app/assets/javascripts/editor/schema/ci.json +++ b/app/assets/javascripts/editor/schema/ci.json @@ -239,6 +239,16 @@ "always" ] }, + "access": { + "markdownDescription": "Configure who can access the artifacts. [Learn More](https://docs.gitlab.com/ee/ci/yaml/#artifactsaccess).", + "default": "all", + "type": "string", + "enum": [ + "none", + "developer", + "all" + ] + }, "expire_in": { "type": "string", "markdownDescription": "How long artifacts should be kept. They are saved 30 days by default. Artifacts that have expired are removed periodically via cron job. Supports a wide variety of formats, e.g. '1 week', '3 mins 4 sec', '2 hrs 20 min', '2h20min', '6 mos 1 day', '47 yrs 6 mos and 4d', '3 weeks and 2 days'. [Learn More](https://docs.gitlab.com/ee/ci/yaml/#artifactsexpire_in).", diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 7bda535c72d61c..38c2227ae33bc0 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -726,12 +726,24 @@ def artifacts_public? job_artifacts_archive.public_access? end - def artifact_is_public_in_config? + def artifacts_no_access? + return false if job_artifacts_archive.nil? # To backward compatibility return false if no artifacts found + + job_artifacts_archive.none_access? + end + + def artifact_access_setting_in_config artifacts_public = options.dig(:artifacts, :public) + artifacts_access = options.dig(:artifacts, :access) + + raise ArgumentError, 'artifacts:public and artifacts:access are mutually exclusive' if !artifacts_public.nil? && !artifacts_access.nil? - return true if artifacts_public.nil? # Default artifacts:public to true + return :public if artifacts_public == true || artifacts_access == 'all' + return :private if artifacts_public == false || artifacts_access == 'developer' + return :none if artifacts_access == 'none' - artifacts_public + # default behaviour + :public end def artifacts_metadata_entry(path, **options) diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 5a08924fc4778c..ca7f539d386c03 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -19,7 +19,7 @@ class JobArtifact < Ci::ApplicationRecord self.primary_key = :id self.sequence_name = :ci_job_artifacts_id_seq - enum accessibility: { public: 0, private: 1 }, _suffix: true + enum accessibility: { public: 0, private: 1, none: 2 }, _suffix: true PLAN_LIMIT_PREFIX = 'ci_max_artifact_size_' @@ -227,6 +227,10 @@ def public_access? public_accessibility? end + def none_access? + none_accessibility? + end + private def store_file_in_transaction! diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index 71ea42e1f235cf..dcdd7d6668637a 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -40,6 +40,10 @@ class BuildPolicy < CommitStatusPolicy @subject.artifacts_public? end + condition(:artifacts_none, scope: :subject) do + @subject.artifacts_no_access? + end + condition(:terminal, scope: :subject) do @subject.has_terminal? end @@ -116,7 +120,7 @@ class BuildPolicy < CommitStatusPolicy prevent :create_build_service_proxy end - rule { can_read_project_build }.enable :read_job_artifacts + rule { can_read_project_build & ~artifacts_none }.enable :read_job_artifacts rule { ~artifacts_public & ~project_developer }.prevent :read_job_artifacts end end diff --git a/app/policies/ci/job_artifact_policy.rb b/app/policies/ci/job_artifact_policy.rb index 61c935af8badc4..8f0b9847d8ffcd 100644 --- a/app/policies/ci/job_artifact_policy.rb +++ b/app/policies/ci/job_artifact_policy.rb @@ -8,6 +8,10 @@ class JobArtifactPolicy < BasePolicy @subject.public_access? end + condition(:none_access, scope: :subject) do + @subject.none_access? + end + condition(:can_read_project_build, scope: :subject) do can?(:read_build, @subject.job.project) end @@ -16,7 +20,7 @@ class JobArtifactPolicy < BasePolicy can?(:developer_access, @subject.job.project) end - rule { can_read_project_build }.enable :read_job_artifacts + rule { can_read_project_build & ~none_access }.enable :read_job_artifacts rule { ~public_access & ~has_access_to_project }.prevent :read_job_artifacts end end diff --git a/app/services/ci/job_artifacts/create_service.rb b/app/services/ci/job_artifacts/create_service.rb index 0791fff8545a99..82f2c22adadc39 100644 --- a/app/services/ci/job_artifacts/create_service.rb +++ b/app/services/ci/job_artifacts/create_service.rb @@ -130,7 +130,7 @@ def accessibility(params) return accessibility if accessibility.present? - job.artifact_is_public_in_config? ? :public : :private + job.artifact_access_setting_in_config end def parse_artifact(artifact) diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index 7bedef67625cea..ee8d482010b75b 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -1424,6 +1424,10 @@ job: > - [Updated](https://gitlab.com/gitlab-org/gitlab/-/issues/322454) in GitLab 15.10. Artifacts created with `artifacts:public` before 15.10 are not guaranteed to remain private after this update. > - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/294503) in GitLab 16.7. Feature flag `non_public_artifacts` removed. +NOTE: +`artifacts:public` is now superceded by [`artifacts:access`](#artifactsaccess) which +has more options. + Use `artifacts:public` to determine whether the job artifacts should be publicly available. @@ -1448,6 +1452,31 @@ job: public: false ``` +### `artifacts:access` + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145206) in GitLab 16.10. + +Use `artifacts:access` to determine who can access the job artifacts. + +You cannot use [`artifacts:public`](#artifactspublic) and `artifacts:access` in the same job. + +**Keyword type**: Job keyword. You can use it only as part of a job. + +**Possible inputs**: + +- `all` (default): Artifacts in public pipelines are available for download by anyone, including anonymous, + guest, and reporter users. +- `developer`: Artifacts are only available for download by users with the Developer role or higher. +- `none`: Artifacts are not available for download by anyone. + +**Example of `artifacts:access`**: + +```yaml +job: + artifacts: + access: 'developer' +``` + #### `artifacts:reports` Use [`artifacts:reports`](artifacts_reports.md) to collect artifacts generated by diff --git a/ee/spec/policies/ci/job_artifact_policy_spec.rb b/ee/spec/policies/ci/job_artifact_policy_spec.rb index 6fbf9e376c60dd..56b1073b25482c 100644 --- a/ee/spec/policies/ci/job_artifact_policy_spec.rb +++ b/ee/spec/policies/ci/job_artifact_policy_spec.rb @@ -29,6 +29,14 @@ expect(policy).to be_allowed :read_job_artifacts end end + + context 'when job artifact access is set to none' do + let(:job_artifact) { create(:ci_job_artifact, :none, project: project) } + + it 'allows read_job_artifacts' do + expect(policy).to be_disallowed :read_job_artifacts + end + end end describe 'for user with access to the project' do @@ -51,6 +59,14 @@ expect(policy).to be_allowed :read_job_artifacts end end + + context 'when job artifact access is set to none' do + let(:job_artifact) { create(:ci_job_artifact, :none, project: project) } + + it 'allows read_job_artifacts' do + expect(policy).to be_disallowed :read_job_artifacts + end + end end describe 'for auditor user' do @@ -75,6 +91,14 @@ expect(policy).to be_allowed :read_job_artifacts end end + + context 'when job artifact access is set to none' do + let(:job_artifact) { create(:ci_job_artifact, :none, project: project) } + + it 'allows read_job_artifacts' do + expect(policy).to be_disallowed :read_job_artifacts + end + end end describe 'for reporter user' do @@ -97,6 +121,14 @@ expect(policy).to be_allowed :read_job_artifacts end end + + context 'when job artifact access is set to none' do + let(:job_artifact) { create(:ci_job_artifact, :none, project: project) } + + it 'allows read_job_artifacts' do + expect(policy).to be_disallowed :read_job_artifacts + end + end end describe 'for guest user' do @@ -118,6 +150,14 @@ expect(policy).to be_allowed :read_job_artifacts end end + + context 'when job artifact access is set to none' do + let(:job_artifact) { create(:ci_job_artifact, :none, project: project) } + + it 'allows read_job_artifacts' do + expect(policy).to be_disallowed :read_job_artifacts + end + end end end end diff --git a/lib/gitlab/ci/config/entry/artifacts.rb b/lib/gitlab/ci/config/entry/artifacts.rb index 3fd07811daffe5..688ce89501128c 100644 --- a/lib/gitlab/ci/config/entry/artifacts.rb +++ b/lib/gitlab/ci/config/entry/artifacts.rb @@ -13,7 +13,8 @@ class Artifacts < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Attributable ALLOWED_WHEN = %w[on_success on_failure always].freeze - ALLOWED_KEYS = %i[name untracked paths reports when expire_in expose_as exclude public].freeze + ALLOWED_ACCESS = %w[none developer all].freeze + ALLOWED_KEYS = %i[name untracked paths reports when expire_in expose_as exclude public access].freeze EXPOSE_AS_REGEX = /\A\w[-\w ]*\z/ EXPOSE_AS_ERROR_MESSAGE = "can contain only letters, digits, '-', '_' and spaces" @@ -26,6 +27,8 @@ class Artifacts < ::Gitlab::Config::Entry::Node validates :config, allowed_keys: ALLOWED_KEYS validates :paths, presence: true, if: :expose_as_present? + validates :config, mutually_exclusive_keys: %i[access public] + with_options allow_nil: true do validates :name, type: String validates :public, boolean: true @@ -44,6 +47,10 @@ class Artifacts < ::Gitlab::Config::Entry::Node message: "should be one of: #{ALLOWED_WHEN.join(', ')}" } validates :expire_in, duration: { parser: ::Gitlab::Ci::Build::DurationParser } + validates :access, type: String, inclusion: { + in: ALLOWED_ACCESS, + message: "should be one of: #{ALLOWED_ACCESS.join(', ')}" + } end end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 0556b42aa97e21..99a65ed13d2ce9 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -330,6 +330,14 @@ end end + trait :no_access_artifacts do + after(:create) do |build, evaluator| + create(:ci_job_artifact, :archive, :none, job: build, expire_at: build.artifacts_expire_at) + create(:ci_job_artifact, :metadata, :none, job: build, expire_at: build.artifacts_expire_at) + build.reload + end + end + trait :report_results do after(:build) do |build| build.report_results << build(:ci_build_report_result) @@ -597,6 +605,34 @@ end end + trait :with_developer_access_artifacts do + options do + { + artifacts: { access: 'developer' } + } + end + end + + # invalid case for access setting + trait :with_access_and_public_setting do + options do + { + artifacts: { + public: true, + access: 'all' + } + } + end + end + + trait :with_none_access_artifacts do + options do + { + artifacts: { access: 'none' } + } + end + end + trait :with_public_artifacts_config do options do { @@ -605,6 +641,14 @@ end end + trait :with_all_access_artifacts do + options do + { + artifacts: { access: 'all' } + } + end + end + trait :non_playable do status { 'created' } self.when { 'manual' } diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb index 292138d76dbcc4..26004ea929d152 100644 --- a/spec/factories/ci/job_artifacts.rb +++ b/spec/factories/ci/job_artifacts.rb @@ -182,6 +182,10 @@ accessibility { 'public' } end + trait :none do + accessibility { 'none' } + end + trait :accessibility do file_type { :accessibility } file_format { :raw } diff --git a/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/artifacts.yml b/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/artifacts.yml index ba4b0db908de2f..5b80511cbec4f6 100644 --- a/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/artifacts.yml +++ b/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/artifacts.yml @@ -67,3 +67,12 @@ artifacts-when-array: artifacts-when-boolean: artifacts: when: true + +artifacts-access-all: + artifacts: + public: true + accesss: all + +artifacts-access-invalid-value: + artifacts: + access: random diff --git a/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/artifacts.yml b/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/artifacts.yml index 70761a09b582b0..f37de22551d880 100644 --- a/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/artifacts.yml +++ b/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/artifacts.yml @@ -51,3 +51,15 @@ artifacts-no-when: artifacts: paths: - binaries/ + +artifacts-access-developer: + artifacts: + access: developer + +artifacts-access-all: + artifacts: + access: all + +artifacts-access-none: + artifacts: + access: none diff --git a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb index 6264e0c8e33f21..ffd5e902a509a3 100644 --- a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb @@ -142,6 +142,35 @@ end end + context 'when access keyword is used' do + context 'when used with public' do + let(:config) { { paths: %w[results.txt], public: true, access: 'all' } } + + it 'reports error' do + expect(entry.errors) + .to include "artifacts config these keys cannot be used together: public, access" + end + end + + context 'when value is not one from expected list' do + let(:config) { { paths: %w[results.txt], access: 'random' } } + + it 'reports error' do + expect(entry.errors) + .to include 'artifacts access should be one of: none, developer, all' + end + end + + context 'when value is correct' do + let(:config) { { paths: %w[results.txt], access: 'developer' } } + + it 'correctly parses the configuration' do + expect(entry).to be_valid + expect(entry.value).to eq(config) + end + end + end + context 'when the `when` keyword is not a string' do context 'when it is an array' do let(:config) { { paths: %w[results.txt], when: ['always'] } } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 78820c0f36977f..d7d06bee0d96b5 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1055,31 +1055,57 @@ end end - describe '#artifact_is_public_in_config?' do - subject { build.artifact_is_public_in_config? } + describe '#artifact_access_setting_in_config' do + subject { build.artifact_access_setting_in_config } context 'artifacts with defaults' do let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } - it { is_expected.to be_truthy } + it { is_expected.to eq(:public) } end context 'non public artifacts' do let(:build) { create(:ci_build, :with_private_artifacts_config, pipeline: pipeline) } - it { is_expected.to be_falsey } + it { is_expected.to eq(:private) } + end + + context 'non public artifacts via access' do + let(:build) { create(:ci_build, :with_developer_access_artifacts, pipeline: pipeline) } + + it { is_expected.to eq(:private) } + end + + context 'non public artifacts via access as none' do + let(:build) { create(:ci_build, :with_none_access_artifacts, pipeline: pipeline) } + + it { is_expected.to eq(:none) } end context 'public artifacts' do let(:build) { create(:ci_build, :with_public_artifacts_config, pipeline: pipeline) } - it { is_expected.to be_truthy } + it { is_expected.to eq(:public) } + end + + context 'public artifacts via access' do + let(:build) { create(:ci_build, :with_all_access_artifacts, pipeline: pipeline) } + + it { is_expected.to eq(:public) } end context 'no artifacts' do let(:build) { create(:ci_build, pipeline: pipeline) } - it { is_expected.to be_truthy } + it { is_expected.to eq(:public) } + end + + context 'when public and access are used together' do + let(:build) { create(:ci_build, :with_access_and_public_setting, pipeline: pipeline) } + + it 'raises ArgumentError' do + expect { subject }.to raise_error(ArgumentError, 'artifacts:public and artifacts:access are mutually exclusive') + end end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index a4214e1a93ee5a..76269868dcf7c9 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -179,6 +179,22 @@ end end + describe 'none_access?' do + subject { artifact.none_access? } + + context 'when job artifact created by default' do + let!(:artifact) { create(:ci_job_artifact) } + + it { is_expected.to be_falsey } + end + + context 'when job artifact created as none access' do + let!(:artifact) { create(:ci_job_artifact, :none) } + + it { is_expected.to be_truthy } + end + end + describe '.file_types_for_report' do it 'returns the report file types for the report type' do expect(described_class.file_types_for_report(:test)).to match_array(%w[junit]) diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb index ad568e60d5ce0a..00e78e29e639ed 100644 --- a/spec/policies/ci/build_policy_spec.rb +++ b/spec/policies/ci/build_policy_spec.rb @@ -19,6 +19,53 @@ end end + describe 'artifacts access config with access keyword' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:user) { create(:user) } + + include_context 'public pipelines disabled' + + before_all do + project.add_developer(user) + end + + context 'when job artifact access is set to all' do + let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } + + it 'disallows read_job_artifacts to project members' do + expect(policy).to be_allowed :read_job_artifacts + end + end + + context 'when job artifact is private to developers' do + let(:build) { create(:ci_build, :private_artifacts, pipeline: pipeline) } + + it 'allows read_job_artifacts to project members' do + expect(policy).to be_allowed :read_job_artifacts + end + + context 'when user is anon' do + let(:user2) { create(:user) } + + let(:policy2) do + described_class.new(user2, build) + end + + it 'disallows read_job_artifacts to anon user' do + expect(policy2).to be_disallowed :read_job_artifacts + end + end + end + + context 'when job artifact access is set to none' do + let(:build) { create(:ci_build, :no_access_artifacts, pipeline: pipeline) } + + it 'disallows read_job_artifacts to project members' do + expect(policy).to be_disallowed :read_job_artifacts + end + end + end + describe '#rules' do context 'when user does not have access to the project' do let(:project) { create(:project, :private) } diff --git a/spec/services/ci/job_artifacts/create_service_spec.rb b/spec/services/ci/job_artifacts/create_service_spec.rb index 8f7afcc0c74b6b..39bba6a2dd8f31 100644 --- a/spec/services/ci/job_artifacts/create_service_spec.rb +++ b/spec/services/ci/job_artifacts/create_service_spec.rb @@ -136,6 +136,22 @@ subject(:execute) { service.execute(artifacts_file, params, metadata_file: metadata_file) } + context 'when artifacts have none access setting' do + let(:artifacts_file) do + file_to_upload('spec/fixtures/ci_build_artifacts.zip', sha256: artifacts_sha256) + end + + let(:access_job) { create(:ci_build, :no_access_artifacts, project: project) } + let(:access_service) { described_class.new(access_job) } + + it 'sets accessibility to none' do + access_service.execute(artifacts_file, params, metadata_file: metadata_file) + + expect(access_job.job_artifacts).not_to be_empty + expect(access_job.job_artifacts).to all be_none_accessibility + end + end + shared_examples_for 'handling accessibility' do shared_examples 'public accessibility' do it 'sets accessibility to public level' do -- GitLab From 281e92b13d9f61966640cca7496f79b157a8f154 Mon Sep 17 00:00:00 2001 From: Albert Salim Date: Wed, 6 Mar 2024 08:48:45 +0000 Subject: [PATCH 2/3] Fix typo for specs --- ee/spec/policies/ci/job_artifact_policy_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/policies/ci/job_artifact_policy_spec.rb b/ee/spec/policies/ci/job_artifact_policy_spec.rb index 56b1073b25482c..9e71f2c15ed071 100644 --- a/ee/spec/policies/ci/job_artifact_policy_spec.rb +++ b/ee/spec/policies/ci/job_artifact_policy_spec.rb @@ -33,7 +33,7 @@ context 'when job artifact access is set to none' do let(:job_artifact) { create(:ci_job_artifact, :none, project: project) } - it 'allows read_job_artifacts' do + it 'does not allow read_job_artifacts' do expect(policy).to be_disallowed :read_job_artifacts end end -- GitLab From 85e3b9f744d3c0efd99c220ed7b377d4e002949b Mon Sep 17 00:00:00 2001 From: Rajendra Kadam Date: Wed, 6 Mar 2024 14:38:57 +0530 Subject: [PATCH 3/3] Fix typos in spec describe --- ee/spec/policies/ci/job_artifact_policy_spec.rb | 8 ++++---- spec/policies/ci/build_policy_spec.rb | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ee/spec/policies/ci/job_artifact_policy_spec.rb b/ee/spec/policies/ci/job_artifact_policy_spec.rb index 9e71f2c15ed071..0b1a713853b8d0 100644 --- a/ee/spec/policies/ci/job_artifact_policy_spec.rb +++ b/ee/spec/policies/ci/job_artifact_policy_spec.rb @@ -33,7 +33,7 @@ context 'when job artifact access is set to none' do let(:job_artifact) { create(:ci_job_artifact, :none, project: project) } - it 'does not allow read_job_artifacts' do + it 'disallows read_job_artifacts' do expect(policy).to be_disallowed :read_job_artifacts end end @@ -63,7 +63,7 @@ context 'when job artifact access is set to none' do let(:job_artifact) { create(:ci_job_artifact, :none, project: project) } - it 'allows read_job_artifacts' do + it 'disallows read_job_artifacts' do expect(policy).to be_disallowed :read_job_artifacts end end @@ -95,7 +95,7 @@ context 'when job artifact access is set to none' do let(:job_artifact) { create(:ci_job_artifact, :none, project: project) } - it 'allows read_job_artifacts' do + it 'disallows read_job_artifacts' do expect(policy).to be_disallowed :read_job_artifacts end end @@ -154,7 +154,7 @@ context 'when job artifact access is set to none' do let(:job_artifact) { create(:ci_job_artifact, :none, project: project) } - it 'allows read_job_artifacts' do + it 'disallows read_job_artifacts' do expect(policy).to be_disallowed :read_job_artifacts end end diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb index 00e78e29e639ed..3a928db67f7485 100644 --- a/spec/policies/ci/build_policy_spec.rb +++ b/spec/policies/ci/build_policy_spec.rb @@ -32,7 +32,7 @@ context 'when job artifact access is set to all' do let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } - it 'disallows read_job_artifacts to project members' do + it 'allows read_job_artifacts to project members' do expect(policy).to be_allowed :read_job_artifacts end end -- GitLab