diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index eb461520d4355afb7e00ce39036e2fc0694bfe5d..a1acc23daf972201945deec0e82396f61e083e42 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -205,9 +205,6 @@ def check_captcha end def ensure_first_name_and_last_name_not_empty - # The key here will be affected by feature flag 'arkose_labs_signup_challenge' - # When flag is disabled, the key will be 'user' because #check_captcha will remove 'new_' prefix - # When flag is enabled, #check_captcha will be skipped, so the key will have 'new_' prefix first_name = params.dig(resource_name, :first_name) || params.dig("new_#{resource_name}", :first_name) last_name = params.dig(resource_name, :last_name) || params.dig("new_#{resource_name}", :last_name) @@ -317,7 +314,7 @@ def identity_verification_redirect_path # overridden by EE module end - def arkose_labs_enabled? + def arkose_labs_enabled?(user: nil) # rubocop:disable Lint/UnusedMethodArgument -- Param is unused here but used in EE override false end end diff --git a/doc/development/identity_verification.md b/doc/development/identity_verification.md index fe15813301acd59de126f0f614bc58379b67452f..caf1a8ba34ff9a8a2c03085f88bae7e339a3a8d5 100644 --- a/doc/development/identity_verification.md +++ b/doc/development/identity_verification.md @@ -19,7 +19,6 @@ Before you enable these features, ensure [hard email confirmation](../security/u | `identity_verification` | Turns on email verification for all registration paths | | `identity_verification_phone_number` | Turns on phone verification for medium risk users for all flows (the Arkose challenge flag for the specific flow and the `identity_verification` flag must be enabled for this to have effect) | | `identity_verification_credit_card` | Turns on credit card verification for high risk users for all flows (the Arkose challenge flag for the specific flow and the `identity_verification` flag must be enabled for this to have effect) | -| `arkose_labs_signup_challenge` | Enables Arkose challenge for all flows | ## Logging diff --git a/doc/integration/arkose.md b/doc/integration/arkose.md index 08d606f5cc1450106840ef6585cd8608b5acd11e..0b1e0f291f9ba67e43fa4d5cbaa4afdd328de8ca 100644 --- a/doc/integration/arkose.md +++ b/doc/integration/arkose.md @@ -57,7 +57,6 @@ To enable Arkose Protect: 1. Enable the ArkoseLabs login challenge. Run the following commands in the Rails console, replacing `<your_public_api_key>` and `<your_private_api_key>` with your own API keys. ```ruby - Feature.enable(:arkose_labs_signup_challenge) ApplicationSetting.current.update(arkose_labs_public_api_key: '<your_public_api_key>') ApplicationSetting.current.update(arkose_labs_private_api_key: '<your_private_api_key>') ``` diff --git a/ee/app/assets/javascripts/pages/registrations/new/index.js b/ee/app/assets/javascripts/pages/registrations/new/index.js index e69b37a99717da42240ca2cb5c02e2b45d9ecb40..b203f575024c20f738a65a6b9af290e5a872764e 100644 --- a/ee/app/assets/javascripts/pages/registrations/new/index.js +++ b/ee/app/assets/javascripts/pages/registrations/new/index.js @@ -9,6 +9,4 @@ trackNewRegistrations(); // (which is executed when '~/pages/registrations/new' is imported) initPasswordValidator(); -if (gon.features.arkoseLabsSignupChallenge) { - setupArkoseLabsForSignup(); -} +setupArkoseLabsForSignup(); diff --git a/ee/app/controllers/concerns/arkose/token_verifiable.rb b/ee/app/controllers/concerns/arkose/token_verifiable.rb index 7f5b616f1a80a65ab518381688eb5d47943c3838..3fb3ec7cb668b818a9d6bed32d172787361cf09a 100644 --- a/ee/app/controllers/concerns/arkose/token_verifiable.rb +++ b/ee/app/controllers/concerns/arkose/token_verifiable.rb @@ -8,7 +8,7 @@ module TokenVerifiable private def verify_arkose_labs_token(user: nil) - return true unless arkose_labs_enabled? + return true unless arkose_labs_enabled?(user: user) return true if arkose_labs_verify_response(user: user).present? if arkose_down? diff --git a/ee/app/controllers/ee/registrations_controller.rb b/ee/app/controllers/ee/registrations_controller.rb index 15a7af670e84006a69b73487c7fae0619d196d1d..998b2e4af563ef4205dec89a83f42b5aa3b3b261 100644 --- a/ee/app/controllers/ee/registrations_controller.rb +++ b/ee/app/controllers/ee/registrations_controller.rb @@ -17,9 +17,6 @@ module RegistrationsController skip_before_action :check_captcha, if: -> { arkose_labs_enabled? } before_action :restrict_registration, only: [:new, :create] - before_action only: [:new, :create] do - push_frontend_feature_flag(:arkose_labs_signup_challenge) - end before_action :ensure_can_remove_self, only: [:destroy] before_action :verify_arkose_labs_challenge!, only: :create end @@ -78,7 +75,7 @@ def after_successful_create_hook(user) # RegistrationsController#after_inactive_sign_up_path_for is correctly called with the custom_attributes # that are added by this action so that the IdentityVerifiable module observation of them is correct. # Identity Verification feature specs cover this ordering. - record_arkose_data + record_arkose_data(user) super @@ -154,20 +151,19 @@ def registration_path_params glm_tracking_params.to_h end - def record_arkose_data - return unless arkose_labs_enabled? + def record_arkose_data(user) + return unless arkose_labs_enabled?(user: user) return unless arkose_labs_verify_response Arkose::RecordUserDataService.new( response: arkose_labs_verify_response, - user: resource + user: user ).execute end override :arkose_labs_enabled? - def arkose_labs_enabled? - ::Feature.enabled?(:arkose_labs_signup_challenge) && - ::Arkose::Settings.enabled?(user: resource, user_agent: request.user_agent) + def arkose_labs_enabled?(user: nil) + ::Arkose::Settings.enabled?(user: user, user_agent: request.user_agent) end def allow_account_deletion? diff --git a/ee/app/controllers/users/identity_verification_controller.rb b/ee/app/controllers/users/identity_verification_controller.rb index c700c219944906a67ed319de0b007f71ddf7d375..0304c0a48f0c79a47ca465a98ff631df4a7ff9f8 100644 --- a/ee/app/controllers/users/identity_verification_controller.rb +++ b/ee/app/controllers/users/identity_verification_controller.rb @@ -254,7 +254,7 @@ def redirect_banned_user end def require_arkose_verification! - return unless arkose_labs_enabled? + return unless arkose_labs_enabled?(user: @user) return unless @user.identities.any? return if @user.arkose_verified? @@ -346,8 +346,8 @@ def ensure_challenge_completed(category) true end - def arkose_labs_enabled? - ::Arkose::Settings.enabled?(user: @user, user_agent: request.user_agent) + def arkose_labs_enabled?(user: nil) + ::Arkose::Settings.enabled?(user: user, user_agent: request.user_agent) end def username diff --git a/ee/app/workers/arkose/blocked_users_report_worker.rb b/ee/app/workers/arkose/blocked_users_report_worker.rb index 44a5633a746bc7b521b3bd2de23cb2453f86344e..60dab7fd3489476d4ab43293b27c754d46e786da 100644 --- a/ee/app/workers/arkose/blocked_users_report_worker.rb +++ b/ee/app/workers/arkose/blocked_users_report_worker.rb @@ -14,8 +14,6 @@ class BlockedUsersReportWorker feature_category :system_access def perform - return unless ::Feature.enabled?(:arkose_labs_signup_challenge) - ::Arkose::BlockedUsersReportService.new.execute end end diff --git a/ee/config/feature_flags/development/arkose_labs_signup_challenge.yml b/ee/config/feature_flags/development/arkose_labs_signup_challenge.yml deleted file mode 100644 index 8b40ce5d0294c083939b01a133fc56553dc47e23..0000000000000000000000000000000000000000 --- a/ee/config/feature_flags/development/arkose_labs_signup_challenge.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: arkose_labs_signup_challenge -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/95560 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/370932 -milestone: '15.4' -type: development -group: group::anti-abuse -default_enabled: false diff --git a/ee/lib/arkose/settings.rb b/ee/lib/arkose/settings.rb index 3255b6062f56d4aca39a9fb974d724318a4b699d..59e29be973f2962aa4678b8f576e5669105a3df8 100644 --- a/ee/lib/arkose/settings.rb +++ b/ee/lib/arkose/settings.rb @@ -35,6 +35,8 @@ def self.enabled?(user:, user_agent:) end def self.group_saml_user(user) + return unless user + user.group_saml_identities.with_provider(::Users::BuildService::GROUP_SAML_PROVIDER).any? end private_class_method :group_saml_user diff --git a/ee/spec/controllers/ee/registrations_controller_spec.rb b/ee/spec/controllers/ee/registrations_controller_spec.rb index c0c2ee200b5c8a2d384b534ecbb5afaa034ad525..fc1750b8bc1623a7d8bb82fb1a3f693a1a84be8f 100644 --- a/ee/spec/controllers/ee/registrations_controller_spec.rb +++ b/ee/spec/controllers/ee/registrations_controller_spec.rb @@ -122,10 +122,6 @@ let(:params) { {} } let(:session) { {} } - before do - stub_feature_flags(arkose_labs_signup_challenge: false) - end - subject(:post_create) { post :create, params: params.merge(user_params), session: session } it_behaves_like 'geo-ip restriction' diff --git a/ee/spec/features/invites_spec.rb b/ee/spec/features/invites_spec.rb index b3b4a8c63469b8974ba826e5adffd009672d9f5f..0c414239badab711017b270fadce11c032c1715c 100644 --- a/ee/spec/features/invites_spec.rb +++ b/ee/spec/features/invites_spec.rb @@ -10,7 +10,6 @@ let(:com) { true } before do - stub_feature_flags(arkose_labs_signup_challenge: false) stub_application_setting(require_admin_approval_after_user_signup: false) allow(::Gitlab).to receive(:com?).and_return(com) diff --git a/ee/spec/features/registrations/email_confirmation_spec.rb b/ee/spec/features/registrations/email_confirmation_spec.rb index 20442080bdd851195e68eb288679103da3151dc7..220f4a5a844b168874f6628f2840974436c51166 100644 --- a/ee/spec/features/registrations/email_confirmation_spec.rb +++ b/ee/spec/features/registrations/email_confirmation_spec.rb @@ -19,7 +19,6 @@ with_them do before do - stub_feature_flags(arkose_labs_signup_challenge: false) stub_feature_flags(identity_verification_credit_card: false) stub_feature_flags(identity_verification: identity_verification) diff --git a/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb b/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb index 4312a3824c0c0b9f6cd3061afc4c6281140b119d..ca6fcaf75392aa0749f929da6a5cfc431dcff328 100644 --- a/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb +++ b/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb @@ -10,7 +10,6 @@ before do stub_application_setting(require_admin_approval_after_user_signup: false) - stub_feature_flags(arkose_labs_signup_challenge: false) stub_application_setting(import_sources: %w[github gitlab_project]) # The groups_and_projects_controller (on `click_on 'Create project'`) is over diff --git a/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb b/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb index 7919636f1a51ffcafbaa322175d696806291ec23..8f0043b89b3e1c56ea85681ec3dde608c7d70776 100644 --- a/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb +++ b/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb @@ -9,7 +9,6 @@ before do stub_application_setting(require_admin_approval_after_user_signup: false) - stub_feature_flags(arkose_labs_signup_challenge: false) stub_application_setting(import_sources: %w[github gitlab_project]) # The groups_and_projects_controller (on `click_on 'Create project'`) is over diff --git a/ee/spec/features/signup_spec.rb b/ee/spec/features/signup_spec.rb index dbfb0faff4c36b2705b97a90c91cd9149e48f36f..d97d52784b319747c35c58b44999c2242e8dc80f 100644 --- a/ee/spec/features/signup_spec.rb +++ b/ee/spec/features/signup_spec.rb @@ -7,7 +7,6 @@ before do stub_application_setting(require_admin_approval_after_user_signup: false) - stub_feature_flags(arkose_labs_signup_challenge: false) end context 'for SaaS', :saas do diff --git a/ee/spec/lib/arkose/settings_spec.rb b/ee/spec/lib/arkose/settings_spec.rb index e9ec9372ab712ac82034aa8020f1772969a609ed..976ad41b2805d1e877811eb294dfbd21d55d06c7 100644 --- a/ee/spec/lib/arkose/settings_spec.rb +++ b/ee/spec/lib/arkose/settings_spec.rb @@ -86,17 +86,18 @@ end describe '.enabled?' do - let_it_be(:user) { create(:user) } + let_it_be(:a_user) { create(:user) } subject { described_class.enabled?(user: user, user_agent: 'user_agent') } - where(:private_key, :public_key, :namespace, :qa_request, :group_saml_user, :result) do - nil | 'public' | 'namespace' | false | false | false - 'private' | nil | 'namespace' | false | false | false - 'private' | 'public' | nil | false | false | false - 'private' | 'public' | 'namespace' | true | false | false - 'private' | 'public' | 'namespace' | false | true | false - 'private' | 'public' | 'namespace' | false | false | true + where(:private_key, :public_key, :namespace, :qa_request, :user, :group_saml_user, :result) do + nil | 'public' | 'namespace' | false | ref(:a_user) | false | false + 'private' | nil | 'namespace' | false | ref(:a_user) | false | false + 'private' | 'public' | nil | false | ref(:a_user) | false | false + 'private' | 'public' | 'namespace' | true | ref(:a_user) | false | false + 'private' | 'public' | 'namespace' | false | ref(:a_user) | true | false + 'private' | 'public' | 'namespace' | false | ref(:a_user) | false | true + 'private' | 'public' | 'namespace' | false | nil | false | true end with_them do diff --git a/ee/spec/requests/trial_registrations_controller_spec.rb b/ee/spec/requests/trial_registrations_controller_spec.rb index e100d94eb3b2c7bbadb92d57663b97330dccfec9..2f4622dc779cf8f93808e1e16b7136ebd2936c61 100644 --- a/ee/spec/requests/trial_registrations_controller_spec.rb +++ b/ee/spec/requests/trial_registrations_controller_spec.rb @@ -47,7 +47,6 @@ before do allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(false) - stub_feature_flags(arkose_labs_signup_challenge: false) end context 'with onboarding' do diff --git a/ee/spec/support/shared_contexts/saas_registration_settings_context.rb b/ee/spec/support/shared_contexts/saas_registration_settings_context.rb index 8ce2f5b395882faaf86f2b09bba2f38dff7ebf0e..a9284ddb42544c65bdff5f94d2057cb51b76a719 100644 --- a/ee/spec/support/shared_contexts/saas_registration_settings_context.rb +++ b/ee/spec/support/shared_contexts/saas_registration_settings_context.rb @@ -20,11 +20,7 @@ # SaaS always requires confirmation, since the default is set to `off` we want to ensure SaaS is set to `hard` stub_application_setting_enum('email_confirmation_setting', 'hard') - stub_feature_flags( - # our focus isn't around the arkose challenge, so we'll omit it - arkose_labs_signup_challenge: false, - identity_verification: true - ) + stub_feature_flags(identity_verification: true) stub_config(extra: { 'google_tag_manager_nonce_id' => 'key' }) diff --git a/ee/spec/support/shared_examples/features/signup_arkose_challenge_shared_examples.rb b/ee/spec/support/shared_examples/features/signup_arkose_challenge_shared_examples.rb index 800d608d3a5d9a08f9b2a1e9d98878bef25006d4..bf59276e99e18680c80b160e638ca059d4541712 100644 --- a/ee/spec/support/shared_examples/features/signup_arkose_challenge_shared_examples.rb +++ b/ee/spec/support/shared_examples/features/signup_arkose_challenge_shared_examples.rb @@ -8,7 +8,6 @@ end before do - stub_feature_flags(arkose_labs_signup_challenge: true) stub_application_setting( arkose_labs_public_api_key: 'public_key', arkose_labs_private_api_key: mock_arkose_labs_key diff --git a/ee/spec/support/shared_examples/requests/signup_arkose_challenge_shared_examples.rb b/ee/spec/support/shared_examples/requests/signup_arkose_challenge_shared_examples.rb index 9182ae683d33350f6aa606308a6b48e1a3911af9..45a416c22f453984f329f6687db4b7d3b0149686 100644 --- a/ee/spec/support/shared_examples/requests/signup_arkose_challenge_shared_examples.rb +++ b/ee/spec/support/shared_examples/requests/signup_arkose_challenge_shared_examples.rb @@ -22,8 +22,6 @@ let(:arkose_status_response) { ServiceResponse.success } before do - stub_feature_flags(arkose_labs_signup_challenge: true) - allow(::Arkose::Settings).to receive(:enabled?).and_return(true) allow_next_instance_of(Arkose::TokenVerificationService) do |instance| @@ -99,16 +97,6 @@ end end - context 'when the feature flag is disabled' do - before do - stub_feature_flags(arkose_labs_signup_challenge: false) - end - - it_behaves_like 'creates the user' - - it_behaves_like 'skips verification and data recording' - end - context 'when feature is disabled' do before do allow(::Arkose::Settings).to receive(:enabled?).and_return(false) diff --git a/ee/spec/workers/ee/arkose/blocked_users_report_worker_spec.rb b/ee/spec/workers/ee/arkose/blocked_users_report_worker_spec.rb index 54a3d9e91534d91621bd97739c0ee7bb72687862..00d04c4cf66df459499a865f22acd07b80578b98 100644 --- a/ee/spec/workers/ee/arkose/blocked_users_report_worker_spec.rb +++ b/ee/spec/workers/ee/arkose/blocked_users_report_worker_spec.rb @@ -5,35 +5,15 @@ RSpec.describe Arkose::BlockedUsersReportWorker, '#perform', feature_category: :insider_threat do subject(:worker) { described_class.new } - context 'when the feature flag arkose_labs_signup_challenge is disabled' do + context 'when the blocked users are reported' do before do - stub_feature_flags(arkose_labs_signup_challenge: false) - end - - it 'does not report the blocked users' do allow_next_instance_of(::Arkose::BlockedUsersReportService) do |instance| - expect(instance).not_to receive(:execute) + allow(instance).to receive(:execute).and_return(true) end - - expect(subject.perform).to be_nil - end - end - - context 'when the feature flag arkose_labs_signup_challenge is enabled' do - before do - stub_feature_flags(arkose_labs_signup_challenge: true) end - context 'when the blocked users are reported' do - before do - allow_next_instance_of(::Arkose::BlockedUsersReportService) do |instance| - allow(instance).to receive(:execute).and_return(true) - end - end - - it 'reports the blocked users' do - expect(subject.perform).to be_truthy - end + it 'reports the blocked users' do + expect(subject.perform).to be_truthy end end end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 156479c7044ed1b4606606f8d12e73db0e069874..468ecd617d96813e21365d14d83d82dbec38c8cd 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -8,7 +8,6 @@ before do stub_application_setting(require_admin_approval_after_user_signup: false) - stub_feature_flags(arkose_labs_signup_challenge: false) end describe '#new' do diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb index 47ca74ec092bf9718a02968fcba8469bc902b081..1ea3ad019d927789aaf299b2a4ea41033e07a4a4 100644 --- a/spec/features/invites_spec.rb +++ b/spec/features/invites_spec.rb @@ -11,7 +11,6 @@ let(:group_invite) { group.group_members.invite.last } before do - stub_feature_flags(arkose_labs_signup_challenge: false) stub_application_setting(require_admin_approval_after_user_signup: false) project.add_maintainer(owner) group.add_owner(owner) diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb index 6f13dfa08cc0b1a7252382eae4868efd0df17f7b..a01ebb44412b043cda0bed99649060788a85047b 100644 --- a/spec/features/users/signup_spec.rb +++ b/spec/features/users/signup_spec.rb @@ -53,7 +53,6 @@ end before do - stub_feature_flags(arkose_labs_signup_challenge: false) stub_application_setting(require_admin_approval_after_user_signup: false) end