diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index 2800eca2585ae3feae9042e81d3e8a49e3dcd36d..cb82c2d8208caeacb0e58bda606e47bf16ffd2bd 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -2181,7 +2181,6 @@ Layout/LineLength: - 'ee/spec/support/shared_examples/requests/api/project_approval_rules_api_shared_examples.rb' - 'ee/spec/support/shared_examples/services/boards/base_service_shared_examples.rb' - 'ee/spec/support/shared_examples/services/build_execute_shared_examples.rb' - - 'ee/spec/support/shared_examples/services/ci/play_job_service_shared_examples.rb' - 'ee/spec/support/shared_examples/services/dast_on_demand_scans_shared_examples.rb' - 'ee/spec/support/shared_examples/services/geo/geo_request_service_shared_examples.rb' - 'ee/spec/support/shared_examples/services/group_saml/saml_provider/base_service_shared_examples.rb' diff --git a/app/assets/javascripts/ci/pipeline_new/constants.js b/app/assets/javascripts/ci/pipeline_new/constants.js index 43f7634083b9e9817162f666e88cee7575210676..1e5f2986ff1c9e74553b92701857130fa2212df2 100644 --- a/app/assets/javascripts/ci/pipeline_new/constants.js +++ b/app/assets/javascripts/ci/pipeline_new/constants.js @@ -10,5 +10,5 @@ export const TAG_REF_TYPE = 'tag'; // must match pipeline/chain/validate/after_config.rb export const CC_VALIDATION_REQUIRED_ERROR = __( - 'Credit card required to be on file in order to create a pipeline', + 'Credit card required to be on file in order to run CI jobs', ); diff --git a/ee/app/models/concerns/identity_verifiable.rb b/ee/app/models/concerns/identity_verifiable.rb index e3ec1d95c1d17ac65674df32353bd152e73e63e6..7b4a6682dfb23318d8e7d891840be21fb5890e0d 100644 --- a/ee/app/models/concerns/identity_verifiable.rb +++ b/ee/app/models/concerns/identity_verifiable.rb @@ -18,6 +18,7 @@ module IdentityVerifiable MEDIUM_RISK_USER_METHODS = %w[email phone].freeze LOW_RISK_USER_METHODS = %w[email].freeze ACTIVE_USER_METHODS = %w[phone].freeze + IDENTITY_VERIFICATION_RELEASE_DATE = Date.new(2024, 6, 3) def signup_identity_verification_enabled? return false unless ::Gitlab::Saas.feature_available?(:identity_verification) @@ -66,8 +67,9 @@ def identity_verification_enabled? end def identity_verified? - return false unless active_user? return true unless identity_verification_enabled? + return false unless active_user? + return true if created_at < IDENTITY_VERIFICATION_RELEASE_DATE # Allow an existing credit card validation to override the identity verification state if # credit_card is not a required verification method. diff --git a/ee/app/services/ee/ci/play_bridge_service.rb b/ee/app/services/ee/ci/play_bridge_service.rb index 04edb53323a652f3bee6e103369423cb581d3b4c..9e821d0a2e1a52678abd7322b75a0b929e9e3b20 100644 --- a/ee/app/services/ee/ci/play_bridge_service.rb +++ b/ee/app/services/ee/ci/play_bridge_service.rb @@ -11,15 +11,10 @@ module PlayBridgeService def check_access!(bridge) super - if current_user && !current_user.has_required_credit_card_to_run_pipelines?(project) - ::Gitlab::AppLogger.info( - message: 'Credit card required to be on file in order to play a job', - project_path: project.full_path, - user_id: current_user.id, - plan: project.root_namespace.actual_plan_name - ) - - raise ::Gitlab::Access::AccessDeniedError, 'Credit card required to be on file in order to play a job' + begin + ::Users::IdentityVerification::AuthorizeCi.new(user: current_user, project: project).authorize_run_jobs! + rescue ::Users::IdentityVerification::Error => e + raise ::Gitlab::Access::AccessDeniedError, e end end end diff --git a/ee/app/services/ee/ci/play_build_service.rb b/ee/app/services/ee/ci/play_build_service.rb index 8d90d8f68aba04aac400ef5fef8680c94bb98598..0b11daa5f37a74de89c9f7122b2433420baa100a 100644 --- a/ee/app/services/ee/ci/play_build_service.rb +++ b/ee/app/services/ee/ci/play_build_service.rb @@ -11,15 +11,10 @@ module PlayBuildService def check_access!(build, job_variables_attributes) super - if current_user && !current_user.has_required_credit_card_to_run_pipelines?(project) - ::Gitlab::AppLogger.info( - message: 'Credit card required to be on file in order to play a job', - project_path: project.full_path, - user_id: current_user.id, - plan: project.root_namespace.actual_plan_name - ) - - raise ::Gitlab::Access::AccessDeniedError, 'Credit card required to be on file in order to play a job' + begin + ::Users::IdentityVerification::AuthorizeCi.new(user: current_user, project: project).authorize_run_jobs! + rescue ::Users::IdentityVerification::Error => e + raise ::Gitlab::Access::AccessDeniedError, e end end end diff --git a/ee/app/services/ee/ci/retry_job_service.rb b/ee/app/services/ee/ci/retry_job_service.rb index cc39d4fa69cdcaf6552537e47bb05e2b371fe68c..d5a93c388c36ddc140fd555236c9c8ffcac23e8f 100644 --- a/ee/app/services/ee/ci/retry_job_service.rb +++ b/ee/app/services/ee/ci/retry_job_service.rb @@ -12,15 +12,10 @@ module RetryJobService def check_access!(build) super - if current_user && !current_user.has_required_credit_card_to_run_pipelines?(project) - ::Gitlab::AppLogger.info( - message: 'Credit card required to be on file in order to retry build', - project_path: project.full_path, - user_id: current_user.id, - plan: project.root_namespace.actual_plan_name - ) - - raise ::Gitlab::Access::AccessDeniedError, 'Credit card required to be on file in order to retry a build' + begin + ::Users::IdentityVerification::AuthorizeCi.new(user: current_user, project: project).authorize_run_jobs! + rescue ::Users::IdentityVerification::Error => e + raise ::Gitlab::Access::AccessDeniedError, e end end diff --git a/ee/app/services/ee/ci/retry_pipeline_service.rb b/ee/app/services/ee/ci/retry_pipeline_service.rb index c2bf46c68373388c4722cc62facb3f6d8d6a9285..8a3941aae0a8b9f1bf25768868da9901d1dd6f6b 100644 --- a/ee/app/services/ee/ci/retry_pipeline_service.rb +++ b/ee/app/services/ee/ci/retry_pipeline_service.rb @@ -7,11 +7,13 @@ module RetryPipelineService override :check_access def check_access(pipeline) - if current_user && !current_user.has_required_credit_card_to_run_pipelines?(project) - ServiceResponse.error(message: 'Credit card required to be on file in order to retry a pipeline', http_status: :forbidden) - else - super + begin + ::Users::IdentityVerification::AuthorizeCi.new(user: current_user, project: project).authorize_run_jobs! + rescue ::Users::IdentityVerification::Error => e + return ServiceResponse.error(message: e.message, http_status: :forbidden) end + + super end private diff --git a/ee/config/feature_flags/gitlab_com_derisk/ci_requires_identity_verification_on_free_plan.yml b/ee/config/feature_flags/gitlab_com_derisk/ci_requires_identity_verification_on_free_plan.yml new file mode 100644 index 0000000000000000000000000000000000000000..cd89f0db6ffae7a71266a7d86eb974594627b63a --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/ci_requires_identity_verification_on_free_plan.yml @@ -0,0 +1,9 @@ +--- +name: ci_requires_identity_verification_on_free_plan +feature_issue_url: https://gitlab.com/gitlab-org/modelops/anti-abuse/team-tasks/-/issues/669 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/151834 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/460157 +milestone: '17.0' +group: group::anti-abuse +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/lib/ee/gitlab/ci/pipeline/chain/validate/after_config.rb b/ee/lib/ee/gitlab/ci/pipeline/chain/validate/after_config.rb index 2c89415f9d3ed56f496d8b6af292969df7876627..3f7cd088096338350db1b01d2a8f7bde9b329fa9 100644 --- a/ee/lib/ee/gitlab/ci/pipeline/chain/validate/after_config.rb +++ b/ee/lib/ee/gitlab/ci/pipeline/chain/validate/after_config.rb @@ -11,15 +11,11 @@ module AfterConfig override :perform! def perform! - if current_user && !current_user.has_required_credit_card_to_run_pipelines?(project) - ::Gitlab::AppLogger.info( - message: 'Credit card required to be on file in order to create a pipeline', - project_path: project.full_path, - user_id: current_user.id, - plan: project.root_namespace.actual_plan_name - ) - - return error('Credit card required to be on file in order to create a pipeline', drop_reason: :user_not_verified) + begin + ::Users::IdentityVerification::AuthorizeCi.new(user: current_user, project: project) + .authorize_run_jobs! + rescue ::Users::IdentityVerification::Error => e + return error(e.message, drop_reason: :user_not_verified) end super diff --git a/ee/lib/users/identity_verification/authorize_ci.rb b/ee/lib/users/identity_verification/authorize_ci.rb new file mode 100644 index 0000000000000000000000000000000000000000..62dfc54cd1d9c44380bac6d357933498159bde8e --- /dev/null +++ b/ee/lib/users/identity_verification/authorize_ci.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +module Users + module IdentityVerification + Error = Class.new(StandardError) + + class AuthorizeCi + attr_reader :user, :project + + def initialize(user:, project:) + @user = user + @project = project + end + + def authorize_run_jobs! + return unless user + + authorize_credit_card! + authorize_identity_verification! + end + + private + + def authorize_credit_card! + return if user.has_required_credit_card_to_run_pipelines?(project) + + ci_access_denied!('Credit card required to be on file in order to run CI jobs') + end + + def authorize_identity_verification! + return unless identity_verification_enabled_for_ci? + return if user.identity_verified? + return unless project_requires_identity_verification_to_run_pipelines? + + ci_access_denied!('Identity verification is required in order to run CI jobs') + end + + def identity_verification_enabled_for_ci? + ::Feature.enabled?(:ci_requires_identity_verification_on_free_plan, project, type: :gitlab_com_derisk) + end + + def project_requires_identity_verification_to_run_pipelines? + return false unless project.shared_runners_enabled + + root_namespace = project.root_namespace + return false if root_namespace.actual_plan.paid_excluding_trials? + + ci_usage = root_namespace.ci_minutes_usage + return false if ci_usage.quota_enabled? && ci_usage.quota.any_purchased? + + true + end + + def ci_access_denied!(message) + log_ci_access_denied(message) + + raise ::Users::IdentityVerification::Error, message + end + + def log_ci_access_denied(message) + ::Gitlab::AppLogger.info( + message: message, + class: self.class.name, + project_path: project.full_path, + user_id: user.id, + plan: project.root_namespace.actual_plan_name + ) + end + end + end +end diff --git a/ee/spec/features/users/identity_verification_spec.rb b/ee/spec/features/users/identity_verification_spec.rb index 1ce4081151840d53c775e9104fb3e221744f0c91..e9bc827dffdcac2dd0addb467593510ce00393d2 100644 --- a/ee/spec/features/users/identity_verification_spec.rb +++ b/ee/spec/features/users/identity_verification_spec.rb @@ -6,7 +6,9 @@ include IdentityVerificationHelpers include ListboxHelpers - let_it_be_with_reload(:user) { create(:user) } + let_it_be_with_reload(:user) do + create(:user, created_at: IdentityVerifiable::IDENTITY_VERIFICATION_RELEASE_DATE + 1.day) + end before do stub_saas_features(identity_verification: true) @@ -39,6 +41,16 @@ expect_to_see_dashboard_page end + context 'when the user was created before the feature relase date' do + let_it_be(:user) do + create(:user, created_at: IdentityVerifiable::IDENTITY_VERIFICATION_RELEASE_DATE - 1.day) + end + + it 'does not verify the user' do + expect_to_see_dashboard_page + end + end + context 'when the user requests a phone verification exemption' do it 'verifies the user' do expect_to_see_identity_verification_page diff --git a/ee/spec/lib/ee/gitlab/ci/pipeline/chain/validate/after_config_spec.rb b/ee/spec/lib/ee/gitlab/ci/pipeline/chain/validate/after_config_spec.rb index 80221f8c4c57bcd8303a297e6c6f2461d91596c9..a70d05dbffead47106295ec6a1ef7e6c60c149b5 100644 --- a/ee/spec/lib/ee/gitlab/ci/pipeline/chain/validate/after_config_spec.rb +++ b/ee/spec/lib/ee/gitlab/ci/pipeline/chain/validate/after_config_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::AfterConfig, feature_category: :continuous_integration do - let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository, developers: user) } let(:pipeline) do build(:ci_pipeline, project: project) @@ -19,57 +19,36 @@ let(:ref) { 'master' } describe '#perform!' do - before do - project.add_developer(user) - end - - describe 'credit card requirement', :saas do - context 'when user does not have credit card for pipelines in project' do - before do - allow(user) - .to receive(:has_required_credit_card_to_run_pipelines?) - .with(project) - .and_return(false) - end - - it 'breaks the chain with an error' do - step.perform! - - expect(step.break?).to be_truthy - expect(pipeline.errors.to_a) - .to include('Credit card required to be on file in order to create a pipeline') - expect(pipeline.failure_reason).to eq('user_not_verified') - expect(pipeline).to be_persisted # when passing a failure reason the pipeline is persisted + context 'when the user is not authorized' do + before do + allow_next_instance_of(::Users::IdentityVerification::AuthorizeCi) do |instance| + allow(instance).to receive(:authorize_run_jobs!).and_raise( + ::Users::IdentityVerification::Error, 'authorization error') end + end - it 'logs the event' do - allow(Gitlab).to receive(:com?).and_return(true).at_least(:once) - allow(Gitlab::AppLogger).to receive(:info) - - expect(Gitlab::AppLogger).to receive(:info).with( - message: 'Credit card required to be on file in order to create a pipeline', - project_path: project.full_path, - user_id: user.id, - plan: 'free') + it 'breaks the chain with an error' do + step.perform! - step.perform! - end + expect(step.break?).to be_truthy + expect(pipeline.errors.to_a).to include('authorization error') + expect(pipeline.failure_reason).to eq('user_not_verified') + expect(pipeline).to be_persisted # when passing a failure reason the pipeline is persisted end + end - context 'when user has credit card for pipelines in project' do - before do - allow(user) - .to receive(:has_required_credit_card_to_run_pipelines?) - .with(project) - .and_return(true) + context 'when the user is authorized' do + before do + allow_next_instance_of(::Users::IdentityVerification::AuthorizeCi) do |instance| + allow(instance).to receive(:authorize_run_jobs!) end + end - it 'succeeds the step' do - step.perform! + it 'succeeds the step' do + step.perform! - expect(step.break?).to be_falsey - expect(pipeline.errors).to be_empty - end + expect(step.break?).to be_falsey + expect(pipeline.errors).to be_empty end end end diff --git a/ee/spec/lib/users/identity_verification/authorize_ci_spec.rb b/ee/spec/lib/users/identity_verification/authorize_ci_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..4d23a3d573464181858e33ec8928b6b2c2a49d52 --- /dev/null +++ b/ee/spec/lib/users/identity_verification/authorize_ci_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::IdentityVerification::AuthorizeCi, :saas, feature_category: :instance_resiliency do + let_it_be(:user) { create(:user, :with_sign_ins) } + let_it_be_with_reload(:project) { create(:project) } + + def stub_verifications(credit_card:, identity_verification:) + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:authorize_credit_card!) unless credit_card + allow(instance).to receive(:authorize_identity_verification!) unless identity_verification + end + end + + describe '#authorize_run_jobs!' do + subject(:authorize) { described_class.new(user: user, project: project).authorize_run_jobs! } + + shared_examples 'logs the failure and raises an exception' do + before do + allow(::Gitlab::AppLogger).to receive(:info) + end + + specify :aggregate_failures do + expect(::Gitlab::AppLogger) + .to receive(:info) + .with( + message: error_message, + class: described_class.name, + project_path: project.full_path, + user_id: user.id, + plan: 'free') + + expect { authorize }.to raise_error(::Users::IdentityVerification::Error, error_message) + end + end + + shared_examples 'credit card verification' do + let(:error_message) { 'Credit card required to be on file in order to run CI jobs' } + + context 'when the user has validated a credit card' do + before do + build(:credit_card_validation, user: user) + end + + it { expect { authorize }.not_to raise_error } + end + + context 'when the user has not validated a credit card' do + before do + allow(user).to receive(:has_required_credit_card_to_run_pipelines?).with(project).and_return(false) + end + + it_behaves_like 'logs the failure and raises an exception' + end + end + + context 'when the user is nil' do + let(:user) { nil } + + before do + allow(described_class).to receive(:authorize_credit_card!).and_raise(::Users::IdentityVerification::Error) + allow(described_class).to receive(:authorize_identity_verification!).and_raise( + ::Users::IdentityVerification::Error) + end + + it { expect { authorize }.not_to raise_error } + end + + context 'when credit card verification is required' do + before do + stub_verifications(credit_card: true, identity_verification: false) + end + + it_behaves_like 'credit card verification' + end + + context 'when credit card and identity verification are required' do + before do + stub_verifications(credit_card: true, identity_verification: true) + end + + it_behaves_like 'credit card verification' + end + + context 'when identity verification is required' do + before do + stub_verifications(credit_card: false, identity_verification: true) + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(ci_requires_identity_verification_on_free_plan: false) + end + + it { expect { authorize }.not_to raise_error } + end + + context 'when user identity is verified' do + before do + allow(user).to receive(:identity_verified?).and_return(true) + end + + it { expect { authorize }.not_to raise_error } + end + + context 'when user identity is not verified' do + before do + allow(user).to receive(:identity_verified?).and_return(false) + end + + it_behaves_like 'logs the failure and raises an exception' do + let(:error_message) { 'Identity verification is required in order to run CI jobs' } + end + + context 'when project shared runners are disabled' do + before do + allow(project).to receive(:shared_runners_enabled).and_return(false) + end + + it { expect { authorize }.not_to raise_error } + end + + context 'when root namespace has a paid plan' do + let_it_be(:ultimate_group) { create(:group_with_plan, :public, plan: :ultimate_plan) } + let_it_be(:project) { create(:project, group: ultimate_group) } + + it { expect { authorize }.not_to raise_error } + end + + context 'when root namespace has purchased CI minutes' do + before do + project.namespace.update!(extra_shared_runners_minutes_limit: 100) + project.namespace.clear_memoization(:ci_minutes_usage) + end + + it { expect { authorize }.not_to raise_error } + end + end + end + end +end diff --git a/ee/spec/models/concerns/identity_verifiable_spec.rb b/ee/spec/models/concerns/identity_verifiable_spec.rb index 66c0aea87790f115e95e7dc107196d7dde2886f3..c410970e4de032a055c61e0b6f0485d82f9527ca 100644 --- a/ee/spec/models/concerns/identity_verifiable_spec.rb +++ b/ee/spec/models/concerns/identity_verifiable_spec.rb @@ -84,7 +84,9 @@ def assume_high_risk(user) end describe('#identity_verified?') do - let_it_be(:user) { create(:user, :with_sign_ins) } + let_it_be(:user) do + create(:user, :with_sign_ins, created_at: described_class::IDENTITY_VERIFICATION_RELEASE_DATE + 1.day) + end subject(:identity_verified?) { user.identity_verified? } @@ -155,6 +157,14 @@ def assume_high_risk(user) it { is_expected.to eq(result) } end end + + context 'when the user was created before the release date' do + let_it_be(:user) do + create(:user, :with_sign_ins, created_at: described_class::IDENTITY_VERIFICATION_RELEASE_DATE - 1.day) + end + + it { is_expected.to eq true } + end end describe('#active_for_authentication?') do diff --git a/ee/spec/services/ci/create_pipeline_service_spec.rb b/ee/spec/services/ci/create_pipeline_service_spec.rb index 48fc0530f91fc511a27965a187a7e0aeadd79b98..c3d6082dae084efd63ed27c89de8d0ba7ef8b982 100644 --- a/ee/spec/services/ci/create_pipeline_service_spec.rb +++ b/ee/spec/services/ci/create_pipeline_service_spec.rb @@ -160,69 +160,6 @@ end end - describe 'credit card requirement' do - shared_examples 'creates a successful pipeline' do - it 'creates a successful pipeline', :aggregate_failures do - response, pipeline = create_pipeline! - - expect(response).to be_success - expect(pipeline).to be_created_successfully - end - end - - context 'when credit card is required' do - context 'when project is on free plan' do - before do - allow(::Gitlab).to receive(:com?).and_return(true) - namespace.gitlab_subscription.update!(hosted_plan: create(:free_plan)) - user.created_at = ::Users::CreditCardValidation::RELEASE_DAY - end - - context 'when user has credit card' do - before do - allow(user).to receive(:credit_card_validated_at).and_return(Time.current) - end - - it_behaves_like 'creates a successful pipeline' - end - - context 'when user does not have credit card' do - it 'creates a pipeline with errors', :aggregate_failures do - _, pipeline = create_pipeline! - - expect(pipeline).not_to be_created_successfully - expect(pipeline.failure_reason).to eq('user_not_verified') - end - - context 'when config is blank' do - before do - stub_ci_pipeline_yaml_file(nil) - end - - it 'does not create a pipeline', :aggregate_failures do - response, pipeline = create_pipeline! - - expect(response).to be_error - expect(pipeline).not_to be_persisted - end - end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(ci_require_credit_card_on_free_plan: false) - end - - it_behaves_like 'creates a successful pipeline' - end - end - end - end - - context 'when credit card is not required' do - it_behaves_like 'creates a successful pipeline' - end - end - def create_pipeline! response = service.execute(:push) diff --git a/ee/spec/services/ci/play_bridge_service_spec.rb b/ee/spec/services/ci/play_bridge_service_spec.rb index b64b2987d2e9555cf8f8ed5ee7ccdca183b65260..22bb185b46d8d0a1035348f8bd1b8ffedd1a12f0 100644 --- a/ee/spec/services/ci/play_bridge_service_spec.rb +++ b/ee/spec/services/ci/play_bridge_service_spec.rb @@ -3,13 +3,13 @@ require 'spec_helper' RSpec.describe Ci::PlayBridgeService, '#execute', feature_category: :continuous_integration do - it_behaves_like 'prevents playing job when credit card is required' do - let(:user) { create(:user, maintainer_of: [project, downstream_project]) } - let(:project) { create(:project) } - let(:pipeline) { create(:ci_pipeline, project: project) } - let(:downstream_project) { create(:project) } - let(:job) { create(:ci_bridge, :playable, pipeline: pipeline, downstream: downstream_project) } - - subject { described_class.new(project, user).execute(job) } - end + let_it_be(:project) { create(:project) } + let_it_be(:downstream_project) { create(:project) } + let_it_be(:user) { create(:user, maintainer_of: [project, downstream_project]) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be_with_reload(:job) { create(:ci_bridge, :playable, pipeline: pipeline, downstream: downstream_project) } + + subject { described_class.new(project, user).execute(job) } + + it_behaves_like 'authorizing CI jobs' end diff --git a/ee/spec/services/ci/play_build_service_spec.rb b/ee/spec/services/ci/play_build_service_spec.rb index 162ee70bf6110e41164a7f3bd0c56c025c80103f..f006a681c7ab5466a9d0a7bd546344b2ec12d2bd 100644 --- a/ee/spec/services/ci/play_build_service_spec.rb +++ b/ee/spec/services/ci/play_build_service_spec.rb @@ -7,12 +7,12 @@ subject { service.execute(build) } end - it_behaves_like 'prevents playing job when credit card is required' do - let(:user) { create(:user, maintainer_of: project) } - let(:project) { create(:project) } - let(:pipeline) { create(:ci_pipeline, project: project) } - let(:job) { create(:ci_build, :manual, pipeline: pipeline) } + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user, maintainer_of: project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be(:job) { create(:ci_build, :manual, pipeline: pipeline) } - subject { described_class.new(project, user).execute(job) } - end + subject { described_class.new(project, user).execute(job) } + + it_behaves_like 'authorizing CI jobs' end diff --git a/ee/spec/services/ci/retry_job_service_spec.rb b/ee/spec/services/ci/retry_job_service_spec.rb index 7471838de58f0d1ffaf7e6595850d615fd063c5c..10bea3e48634054eba1b4226817d020cc683339c 100644 --- a/ee/spec/services/ci/retry_job_service_spec.rb +++ b/ee/spec/services/ci/retry_job_service_spec.rb @@ -69,61 +69,8 @@ end end - describe 'credit card requirement' do - shared_examples 'creates a retried build' do - it 'creates a retried build' do - build - - expect { new_build }.to change { Ci::Build.count }.by(1) - - expect(new_build.name).to eq build.name - expect(new_build).to be_latest - expect(build).to be_retried - expect(build).to be_processed - end - end - - context 'when credit card is required', :saas do - let_it_be(:ultimate_plan) { create(:ultimate_plan) } - let_it_be(:plan_limits) { create(:plan_limits, plan: ultimate_plan) } - - before do - create(:gitlab_subscription, namespace: namespace, hosted_plan: ultimate_plan) - end - - context 'when project is on free plan' do - before do - namespace.gitlab_subscription.update!(hosted_plan: create(:free_plan)) - user.created_at = ::Users::CreditCardValidation::RELEASE_DAY - end - - context 'when user has credit card' do - before do - allow(user).to receive(:credit_card_validated_at).and_return(Time.current) - end - - it_behaves_like 'creates a retried build' - end - - context 'when user does not have credit card' do - it 'raises an exception', :aggregate_failures do - expect { new_build }.to raise_error Gitlab::Access::AccessDeniedError - end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(ci_require_credit_card_on_free_plan: false) - end - - it_behaves_like 'creates a retried build' - end - end - end - end - - context 'when credit card is not required' do - it_behaves_like 'creates a retried build' - end + it_behaves_like 'authorizing CI jobs' do + subject { new_build } end end end diff --git a/ee/spec/services/ci/retry_pipeline_service_spec.rb b/ee/spec/services/ci/retry_pipeline_service_spec.rb index abecf4c7430221146b0a9eabea61e4295c7ef422..f4e993d03cf8e6b90890a8dfe8d393f3528a8ea8 100644 --- a/ee/spec/services/ci/retry_pipeline_service_spec.rb +++ b/ee/spec/services/ci/retry_pipeline_service_spec.rb @@ -37,17 +37,19 @@ end end - context 'when user is not allowed to retry pipeline because of missing credit card' do - it 'returns an error' do - allow(user) - .to receive(:has_required_credit_card_to_run_pipelines?) - .with(project) - .and_return(false) + context 'when the user is not authorized to run jobs' do + before do + allow_next_instance_of(::Users::IdentityVerification::AuthorizeCi) do |instance| + allow(instance).to receive(:authorize_run_jobs!) + .and_raise(::Users::IdentityVerification::Error, 'authorization error') + end + end + it 'returns an error' do response = service.execute(pipeline) expect(response.http_status).to eq(:forbidden) - expect(response.errors).to include('Credit card required to be on file in order to retry a pipeline') + expect(response.errors).to include('authorization error') expect(pipeline.reload).not_to be_running end end diff --git a/ee/spec/support/shared_examples/lib/users/identity_verification/authorize_ci_shared_examples.rb b/ee/spec/support/shared_examples/lib/users/identity_verification/authorize_ci_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..3a90a08e7da136cc0c48961714bc04658c8512f6 --- /dev/null +++ b/ee/spec/support/shared_examples/lib/users/identity_verification/authorize_ci_shared_examples.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'authorizing CI jobs' do + context 'when the user is not authorized to run jobs' do + before do + allow_next_instance_of(::Users::IdentityVerification::AuthorizeCi) do |instance| + allow(instance).to receive(:authorize_run_jobs!).and_raise(::Users::IdentityVerification::Error) + end + end + + it 'raises an exception' do + expect { subject }.to raise_error do |error| + expect(error.cause).to be_a(::Users::IdentityVerification::Error) + end + end + end + + context 'when the user is authorized to run jobs' do + before do + allow_next_instance_of(::Users::IdentityVerification::AuthorizeCi) do |instance| + allow(instance).to receive(:authorize_run_jobs!) + end + end + + it 'does not raise an exception' do + expect { subject }.not_to raise_error + end + end +end diff --git a/ee/spec/support/shared_examples/requests/identity_verification_shared_examples.rb b/ee/spec/support/shared_examples/requests/identity_verification_shared_examples.rb index 073542629d854472e9c75c9b1ba73c03655d224b..33e316719ef8ff18d40c75a9b90e10ecaf586d63 100644 --- a/ee/spec/support/shared_examples/requests/identity_verification_shared_examples.rb +++ b/ee/spec/support/shared_examples/requests/identity_verification_shared_examples.rb @@ -71,7 +71,9 @@ def mock_rate_limit(limit_name, method, result, scope: nil) end RSpec.shared_examples 'it requires a signed in user' do - let_it_be(:confirmed_user) { create(:user) } + let_it_be(:confirmed_user) do + create(:user, created_at: IdentityVerifiable::IDENTITY_VERIFICATION_RELEASE_DATE + 1.day) + end before do stub_session(session_data: { verification_user_id: nil }) @@ -80,12 +82,24 @@ def mock_rate_limit(limit_name, method, result, scope: nil) do_request end - it 'sets the user instance variable' do - expect(assigns(:user)).to eq(confirmed_user) + context 'when the user was created after the release day' do + it 'sets the user instance variable' do + expect(assigns(:user)).to eq(confirmed_user) + end + + it 'does not redirect to root path' do + expect(response).not_to redirect_to(root_path) + end end - it 'does not redirect to root path' do - expect(response).not_to redirect_to(root_path) + context 'when the user was created before the release day' do + let_it_be(:confirmed_user) do + create(:user, created_at: IdentityVerifiable::IDENTITY_VERIFICATION_RELEASE_DATE - 1.day) + + it 'redirects to root path' do + expect(response).to redirect_to(root_path) + end + end end end diff --git a/ee/spec/support/shared_examples/services/ci/play_job_service_shared_examples.rb b/ee/spec/support/shared_examples/services/ci/play_job_service_shared_examples.rb deleted file mode 100644 index 92f66fe94841bf7981859f677471b42bca48059e..0000000000000000000000000000000000000000 --- a/ee/spec/support/shared_examples/services/ci/play_job_service_shared_examples.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -RSpec.shared_examples 'prevents playing job when credit card is required' do - before do - allow(::Gitlab).to receive(:com?).and_return(true) - end - - context 'when user has required credit card' do - before do - allow(user) - .to receive(:has_required_credit_card_to_run_pipelines?) - .with(project) - .and_return(true) - end - - it 'does not raise any exception' do - expect { subject }.not_to raise_error - end - end - - context 'when user does not have required credit card' do - before do - allow(::Gitlab::AppLogger).to receive(:info) - allow(user) - .to receive(:has_required_credit_card_to_run_pipelines?) - .with(project) - .and_return(false) - end - - it 'raises an exception and logs the failure' do - expect(::Gitlab::AppLogger) - .to receive(:info) - .with( - message: 'Credit card required to be on file in order to play a job', - project_path: project.full_path, - user_id: user.id, - plan: 'free') - - expect { subject } - .to raise_error(::Gitlab::Access::AccessDeniedError, 'Credit card required to be on file in order to play a job') - end - end -end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d7ea0be2f9162162201ad7d0c99abeb81e8220ec..d12be553409f2a60feb337905d947b6dcf953c59 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15593,7 +15593,7 @@ msgstr "" msgid "CredentialsInventory|SSH Keys" msgstr "" -msgid "Credit card required to be on file in order to create a pipeline" +msgid "Credit card required to be on file in order to run CI jobs" msgstr "" msgid "Credit card validation record saved" diff --git a/spec/frontend/ci/pipeline_new/mock_data.js b/spec/frontend/ci/pipeline_new/mock_data.js index 72a491bb946c0a28068daf33c5e2b4f51ec642ab..8389447ca15d9934a53fe20884e7fb3b71426084 100644 --- a/spec/frontend/ci/pipeline_new/mock_data.js +++ b/spec/frontend/ci/pipeline_new/mock_data.js @@ -38,7 +38,7 @@ export const mockError = { }; export const mockCreditCardValidationRequiredError = { - errors: ['Credit card required to be on file in order to create a pipeline'], + errors: ['Credit card required to be on file in order to run CI jobs'], warnings: [], total_warnings: 0, };