Merge branch 'reg-captcha-flag' into 'master'

Add :registrations_recaptcha feature flag

Closes gitlab-ee#11389

See merge request gitlab-org/gitlab-ce!28296

3 files changed, 33 insertions, 14 deletions

diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 0fa4677ced1..07b38371ab9 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -4,6 +4,7 @@ class RegistrationsController < Devise::RegistrationsController
   include Recaptcha::Verify
   include AcceptsPendingInvitations
 
+  prepend_before_action :check_captcha, only: :create
   before_action :whitelist_query_limiting, only: [:destroy]
   before_action :ensure_terms_accepted,
                 if: -> { Gitlab::CurrentSettings.current_application_settings.enforce_terms? },
@@ -21,15 +22,10 @@ class RegistrationsController < Devise::RegistrationsController
       params[resource_name] = params.delete(:"new_#{resource_name}")
     end
 
-    if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha
-      accept_pending_invitations
-      super do |new_user|
-        persist_accepted_terms_if_required(new_user)
-      end
-    else
-      flash[:alert] = s_('Profiles|There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.')
-      flash.delete :recaptcha_error
-      render action: 'new'
+    accept_pending_invitations
+
+    super do |new_user|
+      persist_accepted_terms_if_required(new_user)
     end
   rescue Gitlab::Access::AccessDeniedError
     redirect_to(new_user_session_path)
@@ -89,6 +85,17 @@ class RegistrationsController < Devise::RegistrationsController
 
   private
 
+  def check_captcha
+    return unless Feature.enabled?(:registrations_recaptcha, default_enabled: true)
+    return unless Gitlab::Recaptcha.load_configurations!
+
+    return if verify_recaptcha
+
+    flash[:alert] = _('There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.')
+    flash.delete :recaptcha_error
+    render action: 'new'
+  end
+
   def sign_up_params
     params.require(:user).permit(:username, :email, :email_confirmation, :name, :password)
   end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 9883622669c..49f9bb964a5 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -7280,9 +7280,6 @@ msgstr ""
 msgid "Profiles|The maximum file size allowed is 200KB."
 msgstr ""
 
-msgid "Profiles|There was an error with the reCAPTCHA. Please solve the reCAPTCHA again."
-msgstr ""
-
 msgid "Profiles|This doesn't look like a public SSH key, are you sure you want to add it?"
 msgstr ""
 
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 088c515c3a6..9a598790ff2 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -46,13 +46,17 @@ describe RegistrationsController do
     end
 
     context 'when reCAPTCHA is enabled' do
+      def fail_recaptcha
+        # Without this, `verify_recaptcha` arbitrarily returns true in test env
+        Recaptcha.configuration.skip_verify_env.delete('test')
+      end
+
       before do
         stub_application_setting(recaptcha_enabled: true)
       end
 
       it 'displays an error when the reCAPTCHA is not solved' do
-        # Without this, `verify_recaptcha` arbitrarily returns true in test env
-        Recaptcha.configuration.skip_verify_env.delete('test')
+        fail_recaptcha
 
         post(:create, params: user_params)
 
@@ -70,6 +74,17 @@ describe RegistrationsController do
 
         expect(flash[:notice]).to include 'Welcome! You have signed up successfully.'
       end
+
+      it 'does not require reCAPTCHA if disabled by feature flag' do
+        stub_feature_flags(registrations_recaptcha: false)
+        fail_recaptcha
+
+        post(:create, params: user_params)
+
+        expect(controller).not_to receive(:verify_recaptcha)
+        expect(flash[:alert]).to be_nil
+        expect(flash[:notice]).to include 'Welcome! You have signed up successfully.'
+      end
     end
 
     context 'when terms are enforced' do