From 4276f0048ac1d060e1d8536eddf45c2cc5f2ca98 Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Wed, 3 Apr 2024 06:51:07 +0000
Subject: [PATCH] Remove arkose_labs_signup_challenge feature flag

Changelog: other
---
 app/controllers/registrations_controller.rb   |  5 +---
 doc/development/identity_verification.md      |  1 -
 doc/integration/arkose.md                     |  1 -
 .../pages/registrations/new/index.js          |  4 +--
 .../concerns/arkose/token_verifiable.rb       |  2 +-
 .../ee/registrations_controller.rb            | 16 ++++-------
 .../users/identity_verification_controller.rb |  6 ++--
 .../arkose/blocked_users_report_worker.rb     |  2 --
 .../arkose_labs_signup_challenge.yml          |  8 ------
 ee/lib/arkose/settings.rb                     |  2 ++
 .../ee/registrations_controller_spec.rb       |  4 ---
 ee/spec/features/invites_spec.rb              |  1 -
 .../registrations/email_confirmation_spec.rb  |  1 -
 ...external_site_without_confirmation_spec.rb |  1 -
 ...external_site_without_confirmation_spec.rb |  1 -
 ee/spec/features/signup_spec.rb               |  1 -
 ee/spec/lib/arkose/settings_spec.rb           | 17 +++++------
 .../trial_registrations_controller_spec.rb    |  1 -
 .../saas_registration_settings_context.rb     |  6 +---
 ...signup_arkose_challenge_shared_examples.rb |  1 -
 ...signup_arkose_challenge_shared_examples.rb | 12 --------
 .../blocked_users_report_worker_spec.rb       | 28 +++----------------
 .../registrations_controller_spec.rb          |  1 -
 spec/features/invites_spec.rb                 |  1 -
 spec/features/users/signup_spec.rb            |  1 -
 25 files changed, 28 insertions(+), 96 deletions(-)
 delete mode 100644 ee/config/feature_flags/development/arkose_labs_signup_challenge.yml

diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index eb461520d4355..a1acc23daf972 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 fe15813301acd..caf1a8ba34ff9 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 08d606f5cc145..0b1e0f291f9ba 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 e69b37a99717d..b203f575024c2 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 7f5b616f1a80a..3fb3ec7cb668b 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 15a7af670e840..998b2e4af563e 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 c700c21994490..0304c0a48f0c7 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 44a5633a746bc..60dab7fd34894 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 8b40ce5d0294c..0000000000000
--- 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 3255b6062f56d..59e29be973f29 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 c0c2ee200b5c8..fc1750b8bc162 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 b3b4a8c63469b..0c414239badab 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 20442080bdd85..220f4a5a844b1 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 4312a3824c0c0..ca6fcaf75392a 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 7919636f1a51f..8f0043b89b3e1 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 dbfb0faff4c36..d97d52784b319 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 e9ec9372ab712..976ad41b2805d 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 e100d94eb3b2c..2f4622dc779cf 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 8ce2f5b395882..a9284ddb42544 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 800d608d3a5d9..bf59276e99e18 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 9182ae683d333..45a416c22f453 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 54a3d9e91534d..00d04c4cf66df 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 156479c7044ed..468ecd617d968 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 47ca74ec092bf..1ea3ad019d927 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 6f13dfa08cc0b..a01ebb44412b0 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
 
-- 
GitLab