diff options
22 files changed, 214 insertions, 170 deletions
diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 966e80de174..b4700bd8a7a 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -1153,8 +1153,6 @@ RSpec/MissingFeatureCategory: - 'ee/spec/models/ee/project_group_link_spec.rb' - 'ee/spec/models/ee/project_statistics_spec.rb' - 'ee/spec/models/ee/project_wiki_spec.rb' - - 'ee/spec/models/ee/protected_branch_spec.rb' - - 'ee/spec/models/ee/protected_ref_access_spec.rb' - 'ee/spec/models/ee/protected_ref_spec.rb' - 'ee/spec/models/ee/release_spec.rb' - 'ee/spec/models/ee/resource_label_event_spec.rb' @@ -4479,7 +4477,6 @@ RSpec/MissingFeatureCategory: - 'spec/lib/gitlab/usage_data_non_sql_metrics_spec.rb' - 'spec/lib/gitlab/usage_data_queries_spec.rb' - 'spec/lib/gitlab/user_access_snippet_spec.rb' - - 'spec/lib/gitlab/user_access_spec.rb' - 'spec/lib/gitlab/utils/batch_loader_spec.rb' - 'spec/lib/gitlab/utils/deep_size_spec.rb' - 'spec/lib/gitlab/utils/delegator_override/error_spec.rb' @@ -4806,7 +4803,6 @@ RSpec/MissingFeatureCategory: - 'spec/models/concerns/project_api_compatibility_spec.rb' - 'spec/models/concerns/project_features_compatibility_spec.rb' - 'spec/models/concerns/prometheus_adapter_spec.rb' - - 'spec/models/concerns/protected_ref_access_spec.rb' - 'spec/models/concerns/reactive_caching_spec.rb' - 'spec/models/concerns/redactable_spec.rb' - 'spec/models/concerns/redis_cacheable_spec.rb' @@ -5109,8 +5105,6 @@ RSpec/MissingFeatureCategory: - 'spec/models/prometheus_alert_spec.rb' - 'spec/models/prometheus_metric_spec.rb' - 'spec/models/protectable_dropdown_spec.rb' - - 'spec/models/protected_branch/merge_access_level_spec.rb' - - 'spec/models/protected_branch/push_access_level_spec.rb' - 'spec/models/protected_tag_spec.rb' - 'spec/models/push_event_payload_spec.rb' - 'spec/models/push_event_spec.rb' diff --git a/app/assets/javascripts/invite_members/components/group_select.vue b/app/assets/javascripts/invite_members/components/group_select.vue index 5bb3b6b98e6..13902b1f156 100644 --- a/app/assets/javascripts/invite_members/components/group_select.vue +++ b/app/assets/javascripts/invite_members/components/group_select.vue @@ -103,11 +103,11 @@ export default { }, fetchGroups() { if (this.isProject) { - return new Promise((resolve, reject) => { - getProjectShareLocations(this.sourceId, { search: this.searchTerm }) - .then(({ data }) => resolve(data)) - .catch(reject); - }); + return getProjectShareLocations(this.sourceId, { search: this.searchTerm }) + .then(({ data }) => data) + .catch((error) => { + throw error; + }); } switch (this.groupsFilter) { diff --git a/app/models/ci/pending_build.rb b/app/models/ci/pending_build.rb index 2b1eb67d4f2..14050a1e78e 100644 --- a/app/models/ci/pending_build.rb +++ b/app/models/ci/pending_build.rb @@ -14,7 +14,6 @@ module Ci validates :namespace, presence: true scope :ref_protected, -> { where(protected: true) } - scope :queued_before, ->(time) { where(arel_table[:created_at].lt(time)) } scope :with_instance_runners, -> { where(instance_runners_enabled: true) } scope :for_tags, ->(tag_ids) do if tag_ids.present? diff --git a/app/models/project.rb b/app/models/project.rb index 5271fbb138d..fcbef23bc4c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2143,8 +2143,8 @@ class Project < ApplicationRecord pages_metadatum&.deployed? end - def pages_url - return pages_unique_url if pages_unique_domain_enabled? + def pages_url(with_unique_domain: false) + return pages_unique_url if with_unique_domain && pages_unique_domain_enabled? url = pages_namespace_url url_path = full_path.partition('/').last diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 105d22201df..09a0cfc91dc 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -29,7 +29,7 @@ class ProtectedBranch < ApplicationRecord # Maintainers, owners and admins are allowed to create the default branch if project.empty_repo? && project.default_branch_protected? - return true if user.admin? || project.team.max_member_access(user.id) > Gitlab::Access::DEVELOPER + return true if user.admin? || user.can?(:admin_project, project) end super diff --git a/app/services/ci/queue/build_queue_service.rb b/app/services/ci/queue/build_queue_service.rb index 2deebc1d725..d6a252df82f 100644 --- a/app/services/ci/queue/build_queue_service.rb +++ b/app/services/ci/queue/build_queue_service.rb @@ -34,10 +34,6 @@ module Ci order(relation) end - def builds_queued_before(relation, time) - relation.queued_before(time) - end - def builds_for_protected_runner(relation) relation.ref_protected end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index e5b0683d601..68ebb376ccd 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -129,11 +129,6 @@ module Ci builds = queue.builds_with_any_tags(builds) end - # pick builds that older than specified age - if params.key?(:job_age) && Feature.disabled?(:remove_job_age_from_jobs_api) - builds = queue.builds_queued_before(builds, params[:job_age].seconds.ago) - end - build_ids = retrieve_queue(-> { queue.execute(builds) }) @metrics.observe_queue_size(-> { build_ids.size }, @runner.runner_type) diff --git a/app/views/projects/pages/_access.html.haml b/app/views/projects/pages/_access.html.haml index 28f04d78861..7f2588134ba 100644 --- a/app/views/projects/pages/_access.html.haml +++ b/app/views/projects/pages/_access.html.haml @@ -1,4 +1,6 @@ - if @project.pages_deployed? + - pages_url = @project.pages_url(with_unique_domain: true) + = render Pajamas::CardComponent.new(card_options: { class: 'gl-mb-5', data: { qa_selector: 'access_page_container' } }, footer_options: { class: 'gl-alert-warning' }) do |c| - c.header do = s_('GitLabPages|Access pages') @@ -8,7 +10,7 @@ = s_('GitLabPages|Your pages are served under:') %p - = external_link(@project.pages_url, @project.pages_url) + = external_link(pages_url, pages_url) - @project.pages_domains.each do |domain| %p diff --git a/config/feature_flags/development/remove_job_age_from_jobs_api.yml b/config/feature_flags/development/remove_job_age_from_jobs_api.yml deleted file mode 100644 index 2f03f71a757..00000000000 --- a/config/feature_flags/development/remove_job_age_from_jobs_api.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: remove_job_age_from_jobs_api -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118045 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/406662 -milestone: '16.0' -type: development -group: group::pipeline execution -default_enabled: true diff --git a/data/deprecations/16-0-mobsf-android-manifests.yml b/data/deprecations/16-0-mobsf-android-manifests.yml new file mode 100644 index 00000000000..c3bca0a6db1 --- /dev/null +++ b/data/deprecations/16-0-mobsf-android-manifests.yml @@ -0,0 +1,22 @@ +- title: "Changing MobSF-based SAST analyzer behavior in multi-module Android projects" + removal_milestone: "16.0" + announcement_milestone: "16.0" + breaking_change: true + reporter: connorgilbert + stage: Secure + issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/408396 + body: | + We'll change how the MobSF-based analyzer in GitLab SAST handles multi-module Android projects. + This analyzer only runs if you [enable Experimental features](https://docs.gitlab.com/ee/user/application_security/sast/#experimental-features) for SAST. + + The analyzer currently searches for `AndroidManifest.xml` files and scans only the first one it finds. + This manifest often is not the main manifest for the app, so the scan checks less of the app's source code for vulnerabilities. + + Starting in GitLab 16.0, the analyzer will always use `app/src/main/AndroidManifest.xml` as the manifest, and use `app/src/main/` as the project root directory. + The new behavior matches standard Android project layouts and addresses bug reports from customers, so we expect it will improve scan coverage for most apps. + + If you relied on the previous behavior, you can [pin the MobSF analyzer](https://docs.gitlab.com/ee/user/application_security/sast/#pinning-to-minor-image-version) to version 4.0.0, which uses the old behavior. + Then, please comment on [the deprecation issue](https://gitlab.com/gitlab-org/gitlab/-/issues/408396) so we can consider new configuration options to accommodate your use case. + + This change doesn't affect scans you run in GitLab 15.11 or previous versions, since this change is only included in the [new major version](#secure-analyzers-major-version-update) of the MobSF-based analyzer. + documentation_url: https://docs.gitlab.com/ee/user/application_security/sast/ diff --git a/doc/ci/cloud_services/index.md b/doc/ci/cloud_services/index.md index d2d609196e4..6b58fe50059 100644 --- a/doc/ci/cloud_services/index.md +++ b/doc/ci/cloud_services/index.md @@ -21,7 +21,7 @@ but it is not customizable. In GitLab 14.6 an earlier you must use the `CI_JOB_J NOTE: `CI_JOB_JWT` and `CI_JOB_JWT_V2` were [deprecated in GitLab 15.9](../../update/deprecations.md#old-versions-of-json-web-tokens-are-deprecated) -and are scheduled to be removed in GitLab 16.0. Use [ID tokens](../yaml/index.md#id_tokens) instead. +and are scheduled to be removed in GitLab 16.5. Use [ID tokens](../yaml/index.md#id_tokens) instead. ## Requirements diff --git a/doc/development/application_secrets.md b/doc/development/application_secrets.md index e640a94f0a6..5b0755e97e3 100644 --- a/doc/development/application_secrets.md +++ b/doc/development/application_secrets.md @@ -17,7 +17,7 @@ This page is a development guide for application secrets. |`db_key_base` | The base key to encrypt the data for `attr_encrypted` columns | |`openid_connect_signing_key` | The signing key for OpenID Connect | | `encrypted_settings_key_base` | The base key to encrypt settings files with | -| `ci_jwt_signing_key` | The base key for encrypting the `CI_JOB_JWT` and `CI_JOB_JWT_V2` predefined CI/CD variables. `CI_JOB_JWT` and `CI_JOB_JWT_V2` were [deprecated in GitLab 15.9](../update/deprecations.md#old-versions-of-json-web-tokens-are-deprecated) and are scheduled to be removed in GitLab 16.0. | +| `ci_jwt_signing_key` | The base key for encrypting the `CI_JOB_JWT` and `CI_JOB_JWT_V2` predefined CI/CD variables. `CI_JOB_JWT` and `CI_JOB_JWT_V2` were [deprecated in GitLab 15.9](../update/deprecations.md#old-versions-of-json-web-tokens-are-deprecated) and are scheduled to be removed in GitLab 16.5. | ## Where the secrets are stored diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index fc21f4692dc..88803b16825 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -705,6 +705,31 @@ When using the native HashiCorp Vault integration, CI/CD jobs will fail when no <div class="deprecation breaking-change" data-milestone="16.0"> +### Changing MobSF-based SAST analyzer behavior in multi-module Android projects + +<div class="deprecation-notes"> +- Announced in: GitLab <span class="milestone">16.0</span> +- [Breaking change](https://docs.gitlab.com/ee/development/deprecation_guidelines/) +</div> + +We'll change how the MobSF-based analyzer in GitLab SAST handles multi-module Android projects. +This analyzer only runs if you [enable Experimental features](https://docs.gitlab.com/ee/user/application_security/sast/#experimental-features) for SAST. + +The analyzer currently searches for `AndroidManifest.xml` files and scans only the first one it finds. +This manifest often is not the main manifest for the app, so the scan checks less of the app's source code for vulnerabilities. + +Starting in GitLab 16.0, the analyzer will always use `app/src/main/AndroidManifest.xml` as the manifest, and use `app/src/main/` as the project root directory. +The new behavior matches standard Android project layouts and addresses bug reports from customers, so we expect it will improve scan coverage for most apps. + +If you relied on the previous behavior, you can [pin the MobSF analyzer](https://docs.gitlab.com/ee/user/application_security/sast/#pinning-to-minor-image-version) to version 4.0.0, which uses the old behavior. +Then, please comment on [the deprecation issue](https://gitlab.com/gitlab-org/gitlab/-/issues/408396) so we can consider new configuration options to accommodate your use case. + +This change doesn't affect scans you run in GitLab 15.11 or previous versions, since this change is only included in the [new major version](#secure-analyzers-major-version-update) of the MobSF-based analyzer. + +</div> + +<div class="deprecation breaking-change" data-milestone="16.0"> + ### Changing merge request approvals with the `/approvals` API endpoint <div class="deprecation-notes"> diff --git a/lib/api/ci/runner.rb b/lib/api/ci/runner.rb index a8fa7c8a0e4..fcf283491ca 100644 --- a/lib/api/ci/runner.rb +++ b/lib/api/ci/runner.rb @@ -160,7 +160,6 @@ module API optional :certificate, type: String, desc: %q(Session's certificate) optional :authorization, type: String, desc: %q(Session's authorization) end - optional :job_age, type: Integer, desc: %q(DEPRECATED and will be REMOVED in GitLab 16.0. Job should be older than passed age in seconds to be ran on runner) end # Since we serialize the build output ourselves to ensure Gitaly @@ -186,10 +185,6 @@ module API runner_params = declared_params(include_missing: false) - if Feature.enabled?(:remove_job_age_from_jobs_api) - runner_params.delete(:job_age) - end - if current_runner.runner_queue_value_latest?(runner_params[:last_update]) header 'X-GitLab-Last-Update', runner_params[:last_update] Gitlab::Metrics.add_event(:build_not_found_cached) diff --git a/spec/models/concerns/protected_ref_access_spec.rb b/spec/models/concerns/protected_ref_access_spec.rb deleted file mode 100644 index 3cf195704ce..00000000000 --- a/spec/models/concerns/protected_ref_access_spec.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ProtectedRefAccess do - include ExternalAuthorizationServiceHelpers - - subject(:protected_ref_access) do - create(:protected_branch, :maintainers_can_push).push_access_levels.first - end - - let(:project) { protected_ref_access.project } - - describe '#check_access' do - it 'is not bypassed for admins' do - admin = create(:admin) - - expect(protected_ref_access.check_access(admin)).to be_falsey - end - - it 'is true for maintainers' do - maintainer = create(:user) - project.add_maintainer(maintainer) - - expect(protected_ref_access.check_access(maintainer)).to be_truthy - end - - it 'is for developers of the project' do - developer = create(:user) - project.add_developer(developer) - - expect(protected_ref_access.check_access(developer)).to be_falsy - end - - context 'external authorization' do - it 'is false if external authorization denies access' do - maintainer = create(:user) - project.add_maintainer(maintainer) - external_service_deny_access(maintainer, project) - - expect(protected_ref_access.check_access(maintainer)).to be_falsey - end - end - end -end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 412b72f0819..7959412e6dd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2727,34 +2727,73 @@ RSpec.describe Project, factory_default: :keep, feature_category: :projects do .and_return(['http://example.com', port].compact.join(':')) end - context 'when pages_unique_domain feature flag is disabled' do - before do - stub_feature_flags(pages_unique_domain: false) + context 'when not using pages_unique_domain' do + subject { project.pages_url(with_unique_domain: false) } + + context 'when pages_unique_domain feature flag is disabled' do + before do + stub_feature_flags(pages_unique_domain: false) + end + + it { is_expected.to eq('http://group.example.com/project') } end - it { is_expected.to eq('http://group.example.com/project') } - end + context 'when pages_unique_domain feature flag is enabled' do + before do + stub_feature_flags(pages_unique_domain: true) - context 'when pages_unique_domain feature flag is enabled' do - before do - stub_feature_flags(pages_unique_domain: true) + project.project_setting.update!( + pages_unique_domain_enabled: pages_unique_domain_enabled, + pages_unique_domain: 'unique-domain' + ) + end - project.project_setting.update!( - pages_unique_domain_enabled: pages_unique_domain_enabled, - pages_unique_domain: 'unique-domain' - ) + context 'when pages_unique_domain_enabled is false' do + let(:pages_unique_domain_enabled) { false } + + it { is_expected.to eq('http://group.example.com/project') } + end + + context 'when pages_unique_domain_enabled is true' do + let(:pages_unique_domain_enabled) { true } + + it { is_expected.to eq('http://group.example.com/project') } + end end + end + + context 'when using pages_unique_domain' do + subject { project.pages_url(with_unique_domain: true) } - context 'when pages_unique_domain_enabled is false' do - let(:pages_unique_domain_enabled) { false } + context 'when pages_unique_domain feature flag is disabled' do + before do + stub_feature_flags(pages_unique_domain: false) + end it { is_expected.to eq('http://group.example.com/project') } end - context 'when pages_unique_domain_enabled is false' do - let(:pages_unique_domain_enabled) { true } + context 'when pages_unique_domain feature flag is enabled' do + before do + stub_feature_flags(pages_unique_domain: true) - it { is_expected.to eq('http://unique-domain.example.com') } + project.project_setting.update!( + pages_unique_domain_enabled: pages_unique_domain_enabled, + pages_unique_domain: 'unique-domain' + ) + end + + context 'when pages_unique_domain_enabled is false' do + let(:pages_unique_domain_enabled) { false } + + it { is_expected.to eq('http://group.example.com/project') } + end + + context 'when pages_unique_domain_enabled is true' do + let(:pages_unique_domain_enabled) { true } + + it { is_expected.to eq('http://unique-domain.example.com') } + end end end diff --git a/spec/models/protected_branch/merge_access_level_spec.rb b/spec/models/protected_branch/merge_access_level_spec.rb index b6c2d527d1b..7b003e369ee 100644 --- a/spec/models/protected_branch/merge_access_level_spec.rb +++ b/spec/models/protected_branch/merge_access_level_spec.rb @@ -2,6 +2,6 @@ require 'spec_helper' -RSpec.describe ProtectedBranch::MergeAccessLevel do - it { is_expected.to validate_inclusion_of(:access_level).in_array([Gitlab::Access::MAINTAINER, Gitlab::Access::DEVELOPER, Gitlab::Access::NO_ACCESS]) } +RSpec.describe ProtectedBranch::MergeAccessLevel, feature_category: :source_code_management do + include_examples 'protected branch access' end diff --git a/spec/models/protected_branch/push_access_level_spec.rb b/spec/models/protected_branch/push_access_level_spec.rb index 008ae6275f0..837703cd2b3 100644 --- a/spec/models/protected_branch/push_access_level_spec.rb +++ b/spec/models/protected_branch/push_access_level_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' -RSpec.describe ProtectedBranch::PushAccessLevel do - it { is_expected.to validate_inclusion_of(:access_level).in_array([Gitlab::Access::MAINTAINER, Gitlab::Access::DEVELOPER, Gitlab::Access::NO_ACCESS]) } +RSpec.describe ProtectedBranch::PushAccessLevel, feature_category: :source_code_management do + include_examples 'protected branch access' describe 'associations' do it { is_expected.to belong_to(:deploy_key) } diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index 0a75250b68c..d14a7dd1a7e 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -453,6 +453,62 @@ RSpec.describe ProtectedBranch, feature_category: :source_code_management do end end + describe '.protected_ref_accessible_to?' do + let_it_be(:project) { create(:project) } + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:developer) { create(:user) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:owner) { create(:user) } + let_it_be(:admin) { create(:user, :admin) } + + before do + project.add_guest(guest) + project.add_reporter(reporter) + project.add_developer(developer) + project.add_maintainer(maintainer) + project.add_owner(owner) + end + + subject { described_class.protected_ref_accessible_to?(anything, current_user, project: project, action: :push) } + + context 'with guest' do + let(:current_user) { guest } + + it { is_expected.to eq(false) } + end + + context 'with reporter' do + let(:current_user) { reporter } + + it { is_expected.to eq(false) } + end + + context 'with developer' do + let(:current_user) { developer } + + it { is_expected.to eq(false) } + end + + context 'with maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to eq(true) } + end + + context 'with owner' do + let(:current_user) { owner } + + it { is_expected.to eq(true) } + end + + context 'with admin' do + let(:current_user) { admin } + + it { is_expected.to eq(true) } + end + end + describe '.by_name' do let!(:protected_branch) { create(:protected_branch, name: 'master') } let!(:another_protected_branch) { create(:protected_branch, name: 'stable') } diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index e26f4ac8a2a..39b8b489019 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -343,65 +343,6 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end end - # TODO: Remove this with https://gitlab.com/gitlab-org/gitlab/-/issues/334253 - context 'when job filtered by job_age' do - let!(:job) do - create(:ci_build, :pending, :queued, :tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0, queued_at: 60.seconds.ago) - end - - before do - job.queuing_entry&.update!(created_at: 60.seconds.ago) - end - - context 'job is queued less than job_age parameter' do - let(:job_age) { 120 } - - it 'ignores the param and picks the job' do - request_job(job_age: job_age) - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['id']).to eq(job.id) - end - end - - context 'job is queued more than job_age parameter' do - let(:job_age) { 30 } - - it 'ignores the param and picks the job' do - request_job(job_age: job_age) - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['id']).to eq(job.id) - end - end - - context 'when FF remove_job_age_from_jobs_api is disabled' do - before do - stub_feature_flags(remove_job_age_from_jobs_api: false) - end - - context 'job is queued less than job_age parameter' do - let(:job_age) { 120 } - - it 'gives 204' do - request_job(job_age: job_age) - - expect(response).to have_gitlab_http_status(:no_content) - end - end - - context 'job is queued more than job_age parameter' do - let(:job_age) { 30 } - - it 'picks a job' do - request_job(job_age: job_age) - - expect(response).to have_gitlab_http_status(:created) - end - end - end - end - context 'when job is made for branch' do it 'sets tag as ref_type' do request_job diff --git a/spec/support/shared_examples/models/concerns/protected_branch_access_examples.rb b/spec/support/shared_examples/models/concerns/protected_branch_access_examples.rb new file mode 100644 index 00000000000..dd27ff3844f --- /dev/null +++ b/spec/support/shared_examples/models/concerns/protected_branch_access_examples.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'protected branch access' do + include_examples 'protected ref access', :protected_branch + + it { is_expected.to belong_to(:protected_branch) } + + describe '#project' do + before do + allow(protected_ref).to receive(:project) + end + + it 'delegates project to protected_branch association' do + described_class.new(protected_branch: protected_ref).project + + expect(protected_ref).to have_received(:project) + end + end +end diff --git a/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb b/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb index 53ebc207128..f6ca2b91616 100644 --- a/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb +++ b/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true RSpec.shared_examples 'protected ref access' do |association| + include ExternalAuthorizationServiceHelpers + let_it_be(:project) { create(:project) } let_it_be(:protected_ref) { create(association, project: project) } # rubocop:disable Rails/SaveBang @@ -22,7 +24,7 @@ RSpec.shared_examples 'protected ref access' do |association| let(:access_level) { ::Gitlab::Access::DEVELOPER } before_all do - project.add_maintainer(current_user) + project.add_developer(current_user) end subject do @@ -42,12 +44,24 @@ RSpec.shared_examples 'protected ref access' do |association| it { expect(subject.check_access(current_user)).to eq(false) } end - context 'when current_user can push_code to project and access_level is permitted' do - before do - allow(current_user).to receive(:can?).with(:push_code, project).and_return(true) + context 'when current_user can push_code to project' do + context 'and member access is high enough' do + it { expect(subject.check_access(current_user)).to eq(true) } + + context 'when external authorization denies access' do + before do + external_service_deny_access(current_user, project) + end + + it { expect(subject.check_access(current_user)).to be_falsey } + end end - it { expect(subject.check_access(current_user)).to eq(true) } + context 'and member access is too low' do + let(:access_level) { ::Gitlab::Access::MAINTAINER } + + it { expect(subject.check_access(current_user)).to eq(false) } + end end context 'when current_user cannot push_code to project' do |