Merge branch 'ashmckenzie/new-user-signup-ab-test' into 'master'
CE port of new user signup with/without reCAPTCHA A/B test (no-op) See merge request gitlab-org/gitlab-ce!29341
This commit is contained in:
commit
6cae635cc7
|
@ -3,6 +3,7 @@
|
|||
class RegistrationsController < Devise::RegistrationsController
|
||||
include Recaptcha::Verify
|
||||
include AcceptsPendingInvitations
|
||||
include RecaptchaExperimentHelper
|
||||
|
||||
prepend_before_action :check_captcha, only: :create
|
||||
before_action :whitelist_query_limiting, only: [:destroy]
|
||||
|
@ -15,13 +16,6 @@ class RegistrationsController < Devise::RegistrationsController
|
|||
end
|
||||
|
||||
def create
|
||||
# To avoid duplicate form fields on the login page, the registration form
|
||||
# names fields using `new_user`, but Devise still wants the params in
|
||||
# `user`.
|
||||
if params["new_#{resource_name}"].present? && params[resource_name].blank?
|
||||
params[resource_name] = params.delete(:"new_#{resource_name}")
|
||||
end
|
||||
|
||||
accept_pending_invitations
|
||||
|
||||
super do |new_user|
|
||||
|
@ -74,19 +68,35 @@ class RegistrationsController < Devise::RegistrationsController
|
|||
end
|
||||
|
||||
def after_sign_up_path_for(user)
|
||||
Gitlab::AppLogger.info("User Created: username=#{user.username} email=#{user.email} ip=#{request.remote_ip} confirmed:#{user.confirmed?}")
|
||||
Gitlab::AppLogger.info(user_created_message(confirmed: user.confirmed?))
|
||||
user.confirmed? ? stored_location_for(user) || dashboard_projects_path : users_almost_there_path
|
||||
end
|
||||
|
||||
def after_inactive_sign_up_path_for(resource)
|
||||
Gitlab::AppLogger.info("User Created: username=#{resource.username} email=#{resource.email} ip=#{request.remote_ip} confirmed:false")
|
||||
Gitlab::AppLogger.info(user_created_message)
|
||||
users_almost_there_path
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def user_created_message(confirmed: false)
|
||||
"User Created: username=#{resource.username} email=#{resource.email} ip=#{request.remote_ip} confirmed:#{confirmed}"
|
||||
end
|
||||
|
||||
def ensure_correct_params!
|
||||
# To avoid duplicate form fields on the login page, the registration form
|
||||
# names fields using `new_user`, but Devise still wants the params in
|
||||
# `user`.
|
||||
if params["new_#{resource_name}"].present? && params[resource_name].blank?
|
||||
params[resource_name] = params.delete(:"new_#{resource_name}")
|
||||
end
|
||||
end
|
||||
|
||||
def check_captcha
|
||||
return unless Feature.enabled?(:registrations_recaptcha, default_enabled: true)
|
||||
ensure_correct_params!
|
||||
|
||||
return unless Feature.enabled?(:registrations_recaptcha, default_enabled: true) # reCAPTCHA on the UI will still display however
|
||||
return unless show_recaptcha_sign_up?
|
||||
return unless Gitlab::Recaptcha.load_configurations!
|
||||
|
||||
return if verify_recaptcha
|
||||
|
|
|
@ -0,0 +1,7 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module RecaptchaExperimentHelper
|
||||
def show_recaptcha_sign_up?
|
||||
!!Gitlab::Recaptcha.enabled?
|
||||
end
|
||||
end
|
|
@ -7,7 +7,10 @@
|
|||
= f.check_box :recaptcha_enabled, class: 'form-check-input'
|
||||
= f.label :recaptcha_enabled, class: 'form-check-label' do
|
||||
Enable reCAPTCHA
|
||||
%span.form-text.text-muted#recaptcha_help_block Helps prevent bots from creating accounts
|
||||
- recaptcha_v2_link_url = 'https://developers.google.com/recaptcha/docs/versions'
|
||||
- recaptcha_v2_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: recaptcha_v2_link_url }
|
||||
%span.form-text.text-muted#recaptcha_help_block
|
||||
= _('Helps prevent bots from creating accounts. We currently only support %{recaptcha_v2_link_start}reCAPTCHA v2%{recaptcha_v2_link_end}').html_safe % { recaptcha_v2_link_start: recaptcha_v2_link_start, recaptcha_v2_link_end: '</a>'.html_safe }
|
||||
|
||||
.form-group
|
||||
= f.label :recaptcha_site_key, 'reCAPTCHA Site Key', class: 'label-bold'
|
||||
|
|
|
@ -33,7 +33,7 @@
|
|||
= accept_terms_label.html_safe
|
||||
= render_if_exists 'devise/shared/email_opted_in', f: f
|
||||
%div
|
||||
- if Gitlab::Recaptcha.enabled?
|
||||
- if show_recaptcha_sign_up?
|
||||
= recaptcha_tags
|
||||
.submit-container
|
||||
= f.submit _("Register"), class: "btn-register btn qa-new-user-register-button"
|
||||
|
|
|
@ -5016,6 +5016,9 @@ msgstr ""
|
|||
msgid "Help page text and support page url."
|
||||
msgstr ""
|
||||
|
||||
msgid "Helps prevent bots from creating accounts. We currently only support %{recaptcha_v2_link_start}reCAPTCHA v2%{recaptcha_v2_link_end}"
|
||||
msgstr ""
|
||||
|
||||
msgid "Hide archived projects"
|
||||
msgstr ""
|
||||
|
||||
|
|
|
@ -6,7 +6,8 @@ describe RegistrationsController do
|
|||
include TermsHelper
|
||||
|
||||
describe '#create' do
|
||||
let(:user_params) { { user: { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } } }
|
||||
let(:base_user_params) { { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } }
|
||||
let(:user_params) { { user: base_user_params } }
|
||||
|
||||
context 'email confirmation' do
|
||||
around do |example|
|
||||
|
@ -105,6 +106,20 @@ describe RegistrationsController do
|
|||
expect(subject.current_user.terms_accepted?).to be(true)
|
||||
end
|
||||
end
|
||||
|
||||
it "logs a 'User Created' message" do
|
||||
stub_feature_flags(registrations_recaptcha: false)
|
||||
|
||||
expect(Gitlab::AppLogger).to receive(:info).with(/\AUser Created: username=new_username email=new@user.com.+\z/).and_call_original
|
||||
|
||||
post(:create, params: user_params)
|
||||
end
|
||||
|
||||
it 'handles when params are new_user' do
|
||||
post(:create, params: { new_user: base_user_params })
|
||||
|
||||
expect(subject.current_user).not_to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
describe '#destroy' do
|
||||
|
|
|
@ -0,0 +1,23 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'spec_helper'
|
||||
|
||||
describe RecaptchaExperimentHelper, type: :helper do
|
||||
describe '.show_recaptcha_sign_up?' do
|
||||
context 'when reCAPTCHA is disabled' do
|
||||
it 'returns false' do
|
||||
stub_application_setting(recaptcha_enabled: false)
|
||||
|
||||
expect(helper.show_recaptcha_sign_up?).to be(false)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when reCAPTCHA is enabled' do
|
||||
it 'returns true' do
|
||||
stub_application_setting(recaptcha_enabled: true)
|
||||
|
||||
expect(helper.show_recaptcha_sign_up?).to be(true)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue