summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLuke Bennett <lbennett@gitlab.com>2019-05-13 17:04:09 +0100
committerLuke Bennett <lbennett@gitlab.com>2019-05-16 12:59:14 +0100
commit9d3676846c860dcd7865b339d58588184b2f9e6c (patch)
treee71e4006e091ea51cb6384fc3d83c18db4009c9e
parent3061eee6ed00708225931bab566c20d91a06a5c0 (diff)
downloadgitlab-ce-9d3676846c860dcd7865b339d58588184b2f9e6c.tar.gz
Add :registrations_recaptcha feature flag
Allows instance owners to toggle the recaptcha requirement on the user registration page by feature flag. Allows GitLab Growth team to measure reCAPTCHA's impact on registrations.
-rw-r--r--app/controllers/registrations_controller.rb25
-rw-r--r--locale/gitlab.pot3
-rw-r--r--spec/controllers/registrations_controller_spec.rb19
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 71ec8bcb9ba..0c2396bae90 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -7238,9 +7238,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