diff options
author | Luke Bennett <lbennett@gitlab.com> | 2019-05-13 17:04:09 +0100 |
---|---|---|
committer | Luke Bennett <lbennett@gitlab.com> | 2019-05-16 12:59:14 +0100 |
commit | 9d3676846c860dcd7865b339d58588184b2f9e6c (patch) | |
tree | e71e4006e091ea51cb6384fc3d83c18db4009c9e | |
parent | 3061eee6ed00708225931bab566c20d91a06a5c0 (diff) | |
download | gitlab-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.rb | 25 | ||||
-rw-r--r-- | locale/gitlab.pot | 3 | ||||
-rw-r--r-- | spec/controllers/registrations_controller_spec.rb | 19 |
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 |