Merge branch 'revert-f7fa7c1f' into 'master'

Revert "Merge branch 'arkose-labs-captcha-header-guard' into 'master'"

See merge request gitlab-org/gitlab!84177

app/controllers/sessions_controller.rb

@@ -39,7 +39,7 @@ class SessionsController < Devise::SessionsController
   after_action :log_failed_login, if: :action_new_and_failed_login?
   after_action :verify_known_sign_in, only: [:create]
 
-  helper_method :captcha_enabled?, :captcha_on_login_required?, :arkose_labs_enabled?
+  helper_method :captcha_enabled?, :captcha_on_login_required?
 
   # protect_from_forgery is already prepended in ApplicationController but
   # authenticate_with_two_factor which signs in the user is prepended before
@@ -111,10 +111,6 @@ class SessionsController < Devise::SessionsController
     Gitlab::Recaptcha.enabled_on_login? && unverified_anonymous_user?
   end
 
-  def arkose_labs_enabled?
-    false
-  end
-
   # From https://github.com/plataformatec/devise/wiki/How-To:-Use-Recaptcha-with-Devise#devisepasswordscontroller
   def check_captcha
     return unless user_params[:password].present?

app/views/devise/sessions/_new_base.html.haml

@@ -16,7 +16,7 @@
         - else
           = link_to _('Forgot your password?'), new_password_path(:user)
     %div
-    - if arkose_labs_enabled?
+    - if Feature.enabled?(:arkose_labs_login_challenge)
       = render_if_exists 'devise/sessions/arkose_labs'
     - elsif captcha_enabled? || captcha_on_login_required?
       = recaptcha_tags nonce: content_security_policy_nonce

ee/app/controllers/ee/sessions_controller.rb

@@ -79,11 +79,6 @@ module EE
       super
     end
 
-    override :arkose_labs_enabled?
-    def arkose_labs_enabled?
-      ::Feature.enabled?(:arkose_labs_login_challenge, default_enabled: :yaml) && request.headers[::SessionsController::CAPTCHA_HEADER]
-    end
-
     override :check_captcha
     def check_captcha
       if ::Feature.enabled?(:arkose_labs_login_challenge, default_enabled: :yaml)

ee/spec/features/users/arkose_labs_csp_spec.rb

@@ -5,6 +5,10 @@ require 'spec_helper'
 RSpec.describe 'ArkoseLabs content security policy' do
   let(:user) { create(:user) }
 
+  before do
+    stub_feature_flags(arkose_labs_login_challenge: true)
+  end
+
   it 'has proper Content Security Policy headers' do
     visit root_path
 

ee/spec/requests/api/captcha_check_spec.rb

@@ -21,6 +21,10 @@ RSpec.describe API::CaptchaCheck do
     end
 
     context 'when the feature flag arkose_labs_login_challenge is enabled' do
+      before do
+        stub_feature_flags(arkose_labs_login_challenge: true)
+      end
+
       context 'when the username is invalid' do
         let(:invalid_username) { 'invalidUsername' }
 

ee/spec/services/users/captcha_challenge_service_spec.rb

@@ -23,6 +23,10 @@ RSpec.describe Users::CaptchaChallengeService do
     end
 
     context 'when feature flag arkose_labs_login_challenge is enabled' do
+      before do
+        stub_feature_flags(arkose_labs_login_challenge: true)
+      end
+
       context 'when the user has never logged in previously' do
         before do
           user.last_sign_in_at = nil

ee/spec/views/devise/sessions/new.html.haml_spec.rb

@@ -16,9 +16,9 @@ RSpec.describe 'devise/sessions/new' do
       allow(Gitlab).to receive(:com?).and_return(true)
     end
 
-    context 'when arkose_labs_enabled? is enabled' do
+    context 'when the :arkose_labs_login_challenge feature flag is enabled' do
       before do
-        stub_arkose_labs(enabled: true)
+        stub_feature_flags(arkose_labs_login_challenge: true)
 
         subject
       end
@@ -32,9 +32,9 @@ RSpec.describe 'devise/sessions/new' do
       end
     end
 
-    context 'when arkose_labs_enabled? is disabled' do
+    context 'when the :arkose_labs_login_challenge feature flag is disabled' do
       before do
-        stub_arkose_labs(enabled: false)
+        stub_feature_flags(arkose_labs_login_challenge: false)
 
         subject
       end
@@ -55,8 +55,4 @@ RSpec.describe 'devise/sessions/new' do
     allow(view).to receive(:captcha_enabled?).and_return(false)
     allow(view).to receive(:captcha_on_login_required?).and_return(false)
   end
-
-  def stub_arkose_labs(enabled:)
-    allow(view).to receive(:arkose_labs_enabled?).and_return(enabled)
-  end
 end

spec/frontend/fixtures/sessions.rb

@@ -12,7 +12,6 @@ RSpec.describe 'Sessions (JavaScript fixtures)' do
 
     before do
       set_devise_mapping(context: @request)
-      allow(controller).to receive(:arkose_labs_enabled?).and_return(true)
     end
 
     it 'sessions/new.html' do

spec/spec_helper.rb

@@ -329,6 +329,10 @@ RSpec.configure do |config|
       stub_feature_flags(disable_anonymous_search: false)
       stub_feature_flags(disable_anonymous_project_search: false)
 
+      # Specs should not get a CAPTCHA challenge by default, this makes the sign-in flow simpler in
+      # most cases. We do test the CAPTCHA flow in the appropriate specs.
+      stub_feature_flags(arkose_labs_login_challenge: false)
+
       allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_return(enable_rugged)
     else
       unstub_all_feature_flags

spec/views/devise/sessions/new.html.haml_spec.rb

@@ -9,7 +9,6 @@ RSpec.describe 'devise/sessions/new' do
     before do
       stub_devise
       disable_captcha
-      allow(view).to receive(:arkose_labs_enabled?).and_return(false)
       allow(Gitlab).to receive(:com?).and_return(true)
     end
 

spec/views/devise/shared/_signin_box.html.haml_spec.rb

@@ -11,7 +11,6 @@ RSpec.describe 'devise/shared/_signin_box' do
       allow(view).to receive(:captcha_enabled?).and_return(false)
       allow(view).to receive(:captcha_on_login_required?).and_return(false)
       allow(view).to receive(:experiment_enabled?).and_return(false)
-      allow(view).to receive(:arkose_labs_enabled?).and_return(false)
     end
 
     it 'is shown when Crowd is enabled' do