diff --git a/app/components/captcha_submit_button_component.rb b/app/components/captcha_submit_button_component.rb index 948d8104d6a..d759bf40df8 100644 --- a/app/components/captcha_submit_button_component.rb +++ b/app/components/captcha_submit_button_component.rb @@ -17,7 +17,7 @@ def call :'lg-captcha-submit-button', safe_join([input_errors_tag, input_tag, spinner_button_tag, recaptcha_script_tag]), **tag_options, - 'recaptcha-site-key': IdentityConfig.store.recaptcha_site_key, + 'recaptcha-site-key': IdentityConfig.store.recaptcha_site_key_v3, 'recaptcha-action': action, ) end @@ -42,11 +42,11 @@ def input_tag end def recaptcha_script_tag - return if IdentityConfig.store.recaptcha_site_key.blank? + return if IdentityConfig.store.recaptcha_site_key_v3.blank? content_tag(:script, '', src: recaptcha_script_src, async: true) end def recaptcha_script_src - UriService.add_params(RECAPTCHA_SCRIPT_SRC, render: IdentityConfig.store.recaptcha_site_key) + UriService.add_params(RECAPTCHA_SCRIPT_SRC, render: IdentityConfig.store.recaptcha_site_key_v3) end end diff --git a/app/controllers/concerns/recaptcha_concern.rb b/app/controllers/concerns/recaptcha_concern.rb index 997d56217de..12e7aa6463e 100644 --- a/app/controllers/concerns/recaptcha_concern.rb +++ b/app/controllers/concerns/recaptcha_concern.rb @@ -9,6 +9,10 @@ module RecaptchaConcern 'https://recaptcha.google.com/recaptcha/', ].freeze + def recoverable_recaptcha_error?(result) + result.errors.keys == [:recaptcha_token] + end + def allow_csp_recaptcha_src policy = current_content_security_policy policy.script_src(*policy.script_src, *RECAPTCHA_SCRIPT_SRC) diff --git a/app/controllers/users/phone_setup_controller.rb b/app/controllers/users/phone_setup_controller.rb index a9f10a4f128..55c8678f27d 100644 --- a/app/controllers/users/phone_setup_controller.rb +++ b/app/controllers/users/phone_setup_controller.rb @@ -28,6 +28,8 @@ def create if result.success? handle_create_success(@new_phone_form.phone) + elsif recoverable_recaptcha_error?(result) + render :spam_protection, locals: { two_factor_options_path: two_factor_options_path } else render :index end @@ -84,6 +86,7 @@ def new_phone_form_params :otp_delivery_preference, :otp_make_default_number, :recaptcha_token, + :recaptcha_version, ) end end diff --git a/app/controllers/users/phones_controller.rb b/app/controllers/users/phones_controller.rb index 8fe1ec27a78..386a412ccc3 100644 --- a/app/controllers/users/phones_controller.rb +++ b/app/controllers/users/phones_controller.rb @@ -15,9 +15,12 @@ def add def create @new_phone_form = NewPhoneForm.new(user: current_user, analytics: analytics) - if @new_phone_form.submit(user_params).success? + result = @new_phone_form.submit(user_params) + if result.success? confirm_phone bypass_sign_in current_user + elsif recoverable_recaptcha_error?(result) + render 'users/phone_setup/spam_protection' else render :add end @@ -37,6 +40,7 @@ def user_params :otp_delivery_preference, :otp_make_default_number, :recaptcha_token, + :recaptcha_version, ) end diff --git a/app/forms/new_phone_form.rb b/app/forms/new_phone_form.rb index c9d01b4dd3c..1ace391ffe7 100644 --- a/app/forms/new_phone_form.rb +++ b/app/forms/new_phone_form.rb @@ -21,7 +21,8 @@ class NewPhoneForm :otp_delivery_preference, :otp_make_default_number, :setup_voice_preference, - :recaptcha_token + :recaptcha_token, + :recaptcha_version alias_method :setup_voice_preference?, :setup_voice_preference @@ -31,6 +32,7 @@ def initialize(user:, analytics: nil, setup_voice_preference: false) @otp_delivery_preference = user.otp_delivery_preference @otp_make_default_number = false @setup_voice_preference = setup_voice_preference + @recaptcha_version = 3 end def submit(params) @@ -129,7 +131,7 @@ def validate_not_premium_rate end def validate_recaptcha_token - return if !FeatureManagement.phone_recaptcha_enabled? + return if !phone_recaptcha_enabled? return if recaptcha_validator.valid?(recaptcha_token) errors.add( :recaptcha_token, @@ -139,7 +141,15 @@ def validate_recaptcha_token end def recaptcha_validator - @recaptcha_validator ||= PhoneRecaptchaValidator.new(parsed_phone:, analytics:) + @recaptcha_validator ||= PhoneRecaptchaValidator.new( + parsed_phone:, + recaptcha_version:, + analytics:, + ) + end + + def phone_recaptcha_enabled? + FeatureManagement.phone_recaptcha_enabled? end def parsed_phone @@ -155,6 +165,7 @@ def ingest_submitted_params(params) @otp_delivery_preference = delivery_prefs if delivery_prefs @otp_make_default_number = true if default_prefs @recaptcha_token = params[:recaptcha_token] + @recaptcha_version = 2 if params[:recaptcha_version].to_i == 2 end def confirmed_phone? diff --git a/app/services/phone_recaptcha_validator.rb b/app/services/phone_recaptcha_validator.rb index 5cc669e08fe..c029b4ed009 100644 --- a/app/services/phone_recaptcha_validator.rb +++ b/app/services/phone_recaptcha_validator.rb @@ -1,11 +1,12 @@ class PhoneRecaptchaValidator - attr_reader :parsed_phone, :analytics + attr_reader :parsed_phone, :recaptcha_version, :analytics delegate :valid?, :exempt?, to: :validator - def initialize(parsed_phone:, analytics: nil) + def initialize(parsed_phone:, recaptcha_version:, analytics: nil) @parsed_phone = parsed_phone @analytics = analytics + @recaptcha_version = recaptcha_version end def self.exempt_countries @@ -23,7 +24,7 @@ def score_threshold private def validator - @validator ||= RecaptchaValidator.new(score_threshold:, analytics:) + @validator ||= RecaptchaValidator.new(score_threshold:, recaptcha_version:, analytics:) end def score_threshold_country_override diff --git a/app/services/recaptcha_validator.rb b/app/services/recaptcha_validator.rb index 07bdb8e1c14..ff4ac6fec06 100644 --- a/app/services/recaptcha_validator.rb +++ b/app/services/recaptcha_validator.rb @@ -1,13 +1,19 @@ class RecaptchaValidator VERIFICATION_ENDPOINT = 'https://www.google.com/recaptcha/api/siteverify'.freeze - EXEMPT_ERROR_CODES = ['missing-input-secret', 'invalid-input-secret'] + VALID_RECAPTCHA_VERSIONS = [2, 3] + + attr_reader :recaptcha_version, :score_threshold, :analytics - attr_reader :score_threshold, :analytics + def initialize(recaptcha_version: 3, score_threshold: 0.0, analytics: nil) + if !VALID_RECAPTCHA_VERSIONS.include?(recaptcha_version) + raise ArgumentError, "Invalid reCAPTCHA version #{recaptcha_version}, expected one of " \ + "#{VALID_RECAPTCHA_VERSIONS}" + end - def initialize(score_threshold: 0.0, analytics: nil) @score_threshold = score_threshold @analytics = analytics + @recaptcha_version = recaptcha_version end def exempt? @@ -20,10 +26,7 @@ def valid?(recaptcha_token) response = faraday.post( VERIFICATION_ENDPOINT, - URI.encode_www_form( - secret: IdentityConfig.store.recaptcha_secret_key, - response: recaptcha_token, - ), + URI.encode_www_form(secret: recaptcha_secret_key, response: recaptcha_token), ) do |request| request.options.context = { service_name: 'recaptcha' } end @@ -45,21 +48,45 @@ def faraday end def recaptcha_result_valid?(recaptcha_result) - if recaptcha_result.blank? - true - elsif recaptcha_result['success'] - recaptcha_result['score'] >= score_threshold + return true if recaptcha_result.blank? + + success, score, error_codes = recaptcha_result.values_at('success', 'score', 'error-codes') + if success + recaptcha_score_meets_threshold?(score) else - (recaptcha_result['error-codes'] - EXEMPT_ERROR_CODES).blank? + recaptcha_errors_exempt?(error_codes) + end + end + + def recaptcha_score_meets_threshold?(score) + case recaptcha_version + when 2 + true + when 3 + score >= score_threshold end end + def recaptcha_errors_exempt?(error_codes) + (error_codes - EXEMPT_ERROR_CODES).blank? + end + def log_analytics(recaptcha_result: nil, error: nil) analytics&.recaptcha_verify_result_received( recaptcha_result:, score_threshold:, + recaptcha_version:, evaluated_as_valid: recaptcha_result_valid?(recaptcha_result), exception_class: error&.class&.name, ) end + + def recaptcha_secret_key + case recaptcha_version + when 2 + IdentityConfig.store.recaptcha_secret_key_v2 + when 3 + IdentityConfig.store.recaptcha_secret_key_v3 + end + end end diff --git a/app/views/users/phone_setup/spam_protection.html.erb b/app/views/users/phone_setup/spam_protection.html.erb new file mode 100644 index 00000000000..40489f3e3c3 --- /dev/null +++ b/app/views/users/phone_setup/spam_protection.html.erb @@ -0,0 +1,43 @@ +<% title t('titles.spam_protection') %> + +<%= render PageHeadingComponent.new.with_content(t('titles.spam_protection')) %> + +

<%= t('forms.spam_protection.description') %>

+ +<%= simple_form_for(@new_phone_form, url: request.url, method: :post) do |f| %> + <%= f.input :phone, as: :hidden %> + <%= f.input :international_code, as: :hidden %> + <%= f.input :otp_delivery_preference, as: :hidden %> + <%= f.input :otp_make_default_number, as: :hidden %> + <%= f.input :recaptcha_version, as: :hidden, input_html: { value: 2 } %> + <%= f.input :recaptcha_token, as: :hidden, input_html: { id: :recaptcha_token } %> + <%= javascript_tag(nonce: true) do %> + function onCaptchaResponse(token) { + const input = document.getElementById('recaptcha_token'); + input.value = token; + input.closest('form').submit(); + } + <% end %> +
+ +<% end %> + +<%= render TroubleshootingOptionsComponent.new do |c| %> + <% c.header { t('components.troubleshooting_options.default_heading') } %> + <% if local_assigns[:two_factor_options_path].present? %> + <% c.option( + url: two_factor_options_path, + ).with_content(t('two_factor_authentication.login_options_link_text')) %> + <% end %> + <% c.option( + url: MarketingSite.help_center_article_url( + category: 'get-started', + article: 'authentication-options', + ), + new_tab: true, + ).with_content(t('two_factor_authentication.phone_verification.troubleshooting.learn_more')) %> +<% end %> diff --git a/config/application.yml.default b/config/application.yml.default index beac4fb5e0e..4b0740a8919 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -251,8 +251,10 @@ rack_mini_profiler: false rack_timeout_service_timeout_seconds: 15 rails_mailer_previews_enabled: false reauthn_window: 120 -recaptcha_site_key: '' -recaptcha_secret_key: '' +recaptcha_site_key_v2: '' +recaptcha_site_key_v3: '' +recaptcha_secret_key_v2: '' +recaptcha_secret_key_v3: '' recovery_code_length: 4 redis_irs_attempt_api_url: redis://localhost:6379/2 redis_throttle_url: redis://localhost:6379/1 diff --git a/config/locales/forms/en.yml b/config/locales/forms/en.yml index e419da73a71..06fd81f4b70 100644 --- a/config/locales/forms/en.yml +++ b/config/locales/forms/en.yml @@ -111,6 +111,9 @@ en: labels: email: Enter your email address email_language: Select your email language preference + spam_protection: + description: We use reCAPTCHA to protect against automated spam. Check the box + below to continue. ssn: show: Show Social Security number totp_delete: diff --git a/config/locales/forms/es.yml b/config/locales/forms/es.yml index 309a2642399..6047a7a3d74 100644 --- a/config/locales/forms/es.yml +++ b/config/locales/forms/es.yml @@ -119,6 +119,9 @@ es: labels: email: Ingrese su dirección de correo electrónico email_language: Seleccione su preferencia de idioma de correo electrónico + spam_protection: + description: Utilizamos reCAPTCHA como protección contra el correo no deseado + automatizado. Marque la casilla de abajo para continuar. ssn: show: Mostrar Número de Seguro Social totp_delete: diff --git a/config/locales/forms/fr.yml b/config/locales/forms/fr.yml index 396d0c49004..54a7ce63d29 100644 --- a/config/locales/forms/fr.yml +++ b/config/locales/forms/fr.yml @@ -120,6 +120,9 @@ fr: labels: email: Entrez votre adresse email email_language: Sélectionnez votre préférence de langue pour les e-mails + spam_protection: + description: Nous utilisons reCAPTCHA pour nous protéger contre les pourriels + automatisés. Cochez la case ci-dessous pour continuer. ssn: show: Afficher le numéro de sécurité sociale totp_delete: diff --git a/config/locales/titles/en.yml b/config/locales/titles/en.yml index 9bbb89a1c39..783638a1ef9 100644 --- a/config/locales/titles/en.yml +++ b/config/locales/titles/en.yml @@ -80,6 +80,7 @@ en: completion_new_attributes: '%{sp} is requesting new information' completion_new_sp: You are now signing in for the first time confirmation: Continue to sign in + spam_protection: Protecting against spam totp_setup: new: Add authentication app two_factor_setup: Two-factor authentication setup diff --git a/config/locales/titles/es.yml b/config/locales/titles/es.yml index 9e64e1c56c2..458620a62ee 100644 --- a/config/locales/titles/es.yml +++ b/config/locales/titles/es.yml @@ -82,6 +82,7 @@ es: completion_new_attributes: '%{sp} está solicitando nueva información' completion_new_sp: Acabas de iniciar sesión por primera vez confirmation: Continuar para iniciar sesión + spam_protection: Protección contra el correo no deseado totp_setup: new: Agregar aplicación de autenticación two_factor_setup: Configuración de autenticación de dos factores diff --git a/config/locales/titles/fr.yml b/config/locales/titles/fr.yml index b18978dc62f..5175de061b2 100644 --- a/config/locales/titles/fr.yml +++ b/config/locales/titles/fr.yml @@ -82,6 +82,7 @@ fr: completion_new_attributes: '%{sp} demande de nouvelles informations' completion_new_sp: Vous vous connectez pour la première fois confirmation: Continuer à vous connecter + spam_protection: Protection contre les pourriels totp_setup: new: Ajouter une application d’authentification two_factor_setup: Configuration de l’authentification à deux facteurs diff --git a/config/routes.rb b/config/routes.rb index 8f31eb5ac83..a525c55d77e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -251,7 +251,8 @@ get '/second_mfa_setup' => 'users/mfa_selection#index' patch '/second_mfa_setup' => 'users/mfa_selection#update' get '/phone_setup' => 'users/phone_setup#index' - patch '/phone_setup' => 'users/phone_setup#create' + patch '/phone_setup' => 'users/phone_setup#create' # TODO: Remove after next deploy + post '/phone_setup' => 'users/phone_setup#create' get '/users/two_factor_authentication' => 'users/two_factor_authentication#show', as: :user_two_factor_authentication # route name is used by two_factor_authentication gem get '/backup_code_refreshed' => 'users/backup_code_setup#refreshed' diff --git a/lib/feature_management.rb b/lib/feature_management.rb index a355668f66f..7763cc92c32 100644 --- a/lib/feature_management.rb +++ b/lib/feature_management.rb @@ -113,8 +113,10 @@ def self.log_to_stdout? end def self.phone_recaptcha_enabled? - IdentityConfig.store.recaptcha_site_key.present? && - IdentityConfig.store.recaptcha_secret_key.present? && + IdentityConfig.store.recaptcha_site_key_v2.present? && + IdentityConfig.store.recaptcha_site_key_v3.present? && + IdentityConfig.store.recaptcha_secret_key_v2.present? && + IdentityConfig.store.recaptcha_secret_key_v3.present? && IdentityConfig.store.phone_recaptcha_score_threshold.positive? end diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 8b0e9ee36f1..141e3958e76 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -352,8 +352,10 @@ def self.build_store(config_map) config.add(:rack_timeout_service_timeout_seconds, type: :integer) config.add(:rails_mailer_previews_enabled, type: :boolean) config.add(:reauthn_window, type: :integer) - config.add(:recaptcha_site_key, type: :string) - config.add(:recaptcha_secret_key, type: :string) + config.add(:recaptcha_site_key_v2, type: :string) + config.add(:recaptcha_site_key_v3, type: :string) + config.add(:recaptcha_secret_key_v2, type: :string) + config.add(:recaptcha_secret_key_v3, type: :string) config.add(:recovery_code_length, type: :integer) config.add(:recurring_jobs_disabled_names, type: :json) config.add(:redis_irs_attempt_api_url) diff --git a/spec/components/captcha_submit_button_component_spec.rb b/spec/components/captcha_submit_button_component_spec.rb index 1bd03054015..c9f9350b954 100644 --- a/spec/components/captcha_submit_button_component_spec.rb +++ b/spec/components/captcha_submit_button_component_spec.rb @@ -34,7 +34,7 @@ context 'without configured recaptcha site key' do before do - allow(IdentityConfig.store).to receive(:recaptcha_site_key).and_return(nil) + allow(IdentityConfig.store).to receive(:recaptcha_site_key_v3).and_return(nil) end it 'renders without recaptcha site key attribute' do @@ -49,7 +49,7 @@ context 'with configured recaptcha site key' do let(:recaptcha_site_key) { 'site_key' } before do - allow(IdentityConfig.store).to receive(:recaptcha_site_key).and_return(recaptcha_site_key) + allow(IdentityConfig.store).to receive(:recaptcha_site_key_v3).and_return(recaptcha_site_key) end it 'renders with recaptcha site key attribute' do diff --git a/spec/controllers/concerns/recaptcha_concern_spec.rb b/spec/controllers/concerns/recaptcha_concern_spec.rb index 965be1dd521..a1390838b8b 100644 --- a/spec/controllers/concerns/recaptcha_concern_spec.rb +++ b/spec/controllers/concerns/recaptcha_concern_spec.rb @@ -1,17 +1,17 @@ require 'rails_helper' RSpec.describe RecaptchaConcern, type: :controller do - describe '#allow_csp_recaptcha_src' do - controller ApplicationController do - include RecaptchaConcern + controller ApplicationController do + include RecaptchaConcern - before_action :allow_csp_recaptcha_src + before_action :allow_csp_recaptcha_src - def index - render plain: '' - end + def index + render plain: '' end + end + describe '#allow_csp_recaptcha_src' do it 'overrides csp to add directives for recaptcha' do get :index @@ -20,4 +20,34 @@ def index expect(csp.frame_src).to include(*RecaptchaConcern::RECAPTCHA_FRAME_SRC) end end + + describe '#recoverable_recaptcha_error?' do + let(:form) { NewPhoneForm.new(user: build_stubbed(:user)) } + let(:errors) { ActiveModel::Errors.new(form) } + let(:result) { FormResponse.new(success: true, errors:) } + + subject(:recoverable_recaptcha_error) { controller.recoverable_recaptcha_error?(result) } + + it { expect(recoverable_recaptcha_error).to eq(false) } + + context 'with recaptcha token error' do + before do + errors.add( + :recaptcha_token, + t('errors.messages.invalid_recaptcha_token'), + type: :invalid_recaptcha_token, + ) + end + + it { expect(recoverable_recaptcha_error).to eq(true) } + + context 'with error unrelated to recaptcha token' do + before do + errors.add(:phone, :blank, message: t('errors.messages.blank')) + end + + it { expect(recoverable_recaptcha_error).to eq(false) } + end + end + end end diff --git a/spec/controllers/users/phone_setup_controller_spec.rb b/spec/controllers/users/phone_setup_controller_spec.rb index 508093b4a7e..df7ee2ad4de 100644 --- a/spec/controllers/users/phone_setup_controller_spec.rb +++ b/spec/controllers/users/phone_setup_controller_spec.rb @@ -69,7 +69,7 @@ expect(@analytics).to receive(:track_event). with('Multi-Factor Authentication: phone setup', result) - patch :create, params: { + post :create, params: { new_phone_form: { phone: '703-555-010', international_code: 'US', @@ -79,6 +79,20 @@ expect(response).to render_template(:index) end + context 'with recoverable recaptcha error' do + it 'renders spam protection template' do + stub_sign_in + + allow(controller).to receive(:recoverable_recaptcha_error?) do |result| + result.is_a?(FormResponse) + end + + post :create, params: { new_phone_form: { international_code: 'CA' } } + + expect(response).to render_template(:spam_protection) + end + end + context 'with voice' do let(:user) { create(:user, otp_delivery_preference: 'voice') } @@ -101,7 +115,7 @@ expect(@analytics).to receive(:track_event). with('Multi-Factor Authentication: phone setup', result) - patch( + post( :create, params: { new_phone_form: { phone: '703-555-0100', @@ -141,7 +155,7 @@ expect(@analytics).to receive(:track_event). with('Multi-Factor Authentication: phone setup', result) - patch( + post( :create, params: { new_phone_form: { phone: '703-555-0100', diff --git a/spec/controllers/users/phones_controller_spec.rb b/spec/controllers/users/phones_controller_spec.rb index e79838ebaf3..ed817cf245b 100644 --- a/spec/controllers/users/phones_controller_spec.rb +++ b/spec/controllers/users/phones_controller_spec.rb @@ -89,4 +89,20 @@ end end end + + describe '#create' do + context 'with recoverable recaptcha error' do + it 'renders spam protection template' do + stub_sign_in + + allow(controller).to receive(:recoverable_recaptcha_error?) do |result| + result.is_a?(FormResponse) + end + + post :create, params: { new_phone_form: { international_code: 'CA' } } + + expect(response).to render_template('users/phone_setup/spam_protection') + end + end + end end diff --git a/spec/forms/new_phone_form_spec.rb b/spec/forms/new_phone_form_spec.rb index 23056b8c116..1ce459011a3 100644 --- a/spec/forms/new_phone_form_spec.rb +++ b/spec/forms/new_phone_form_spec.rb @@ -361,7 +361,7 @@ context 'with recaptcha enabled' do let(:valid) { nil } - let(:validator) { PhoneRecaptchaValidator.new(parsed_phone: nil) } + let(:validator) { PhoneRecaptchaValidator.new(recaptcha_version: 3, parsed_phone: nil) } let(:recaptcha_token) { 'token' } let(:params) { super().merge(recaptcha_token:) } subject(:result) { form.submit(params) } @@ -379,6 +379,36 @@ expect(result.success?).to eq(true) expect(result.errors).to be_blank end + + it 'does not override default recaptcha version' do + result + + expect(form.recaptcha_version).to eq(3) + end + + context 'with recaptcha_version parameter' do + let(:params) { super().merge(recaptcha_version:) } + + context 'with v2 parameter' do + let(:recaptcha_version) { '2' } + + it 'overrides default recaptcha version' do + result + + expect(form.recaptcha_version).to eq(2) + end + end + + context 'with invalid parameter' do + let(:recaptcha_version) { '4' } + + it 'does not override default recaptcha version' do + result + + expect(form.recaptcha_version).to eq(3) + end + end + end end context 'with invalid recaptcha result' do diff --git a/spec/lib/feature_management_spec.rb b/spec/lib/feature_management_spec.rb index 8892d8fadc5..2af24ac3a32 100644 --- a/spec/lib/feature_management_spec.rb +++ b/spec/lib/feature_management_spec.rb @@ -408,35 +408,55 @@ end describe '.phone_recaptcha_enabled?' do - let(:recaptcha_site_key) { '' } - let(:recaptcha_secret_key) { '' } + let(:recaptcha_site_key_v2) { '' } + let(:recaptcha_site_key_v3) { '' } + let(:recaptcha_secret_key_v2) { '' } + let(:recaptcha_secret_key_v3) { '' } let(:phone_recaptcha_score_threshold) { 0.0 } subject(:phone_recaptcha_enabled) { FeatureManagement.phone_recaptcha_enabled? } before do - allow(IdentityConfig.store).to receive(:recaptcha_site_key).and_return(recaptcha_site_key) - allow(IdentityConfig.store).to receive(:recaptcha_secret_key).and_return(recaptcha_secret_key) + allow(IdentityConfig.store).to receive(:recaptcha_site_key_v2). + and_return(recaptcha_site_key_v2) + allow(IdentityConfig.store).to receive(:recaptcha_site_key_v3). + and_return(recaptcha_site_key_v3) + allow(IdentityConfig.store).to receive(:recaptcha_secret_key_v2). + and_return(recaptcha_secret_key_v2) + allow(IdentityConfig.store).to receive(:recaptcha_secret_key_v3). + and_return(recaptcha_secret_key_v3) allow(IdentityConfig.store).to receive(:phone_recaptcha_score_threshold). and_return(phone_recaptcha_score_threshold) end - it { expect(subject).to eq(false) } + it { expect(phone_recaptcha_enabled).to eq(false) } - context 'with configured recaptcha site key' do - let(:recaptcha_site_key) { 'key' } + context 'with configured recaptcha v2 site key' do + let(:recaptcha_site_key_v2) { 'key' } - it { expect(subject).to eq(false) } + it { expect(phone_recaptcha_enabled).to eq(false) } - context 'with configured recaptcha secret key' do - let(:recaptcha_secret_key) { 'key' } + context 'with configured recaptcha v2 secret key' do + let(:recaptcha_secret_key_v2) { 'key' } - it { expect(subject).to eq(false) } + it { expect(phone_recaptcha_enabled).to eq(false) } - context 'with configured default success rate threshold greater than 0' do - let(:phone_recaptcha_score_threshold) { 1.0 } + context 'with configured recaptcha v3 site key' do + let(:recaptcha_site_key_v3) { 'key' } - it { expect(subject).to eq(true) } + it { expect(phone_recaptcha_enabled).to eq(false) } + + context 'with configured recaptcha v2 secret key' do + let(:recaptcha_secret_key_v3) { 'key' } + + it { expect(phone_recaptcha_enabled).to eq(false) } + + context 'with configured default success rate threshold greater than 0' do + let(:phone_recaptcha_score_threshold) { 1.0 } + + it { expect(phone_recaptcha_enabled).to eq(true) } + end + end end end end diff --git a/spec/services/phone_recaptcha_validator_spec.rb b/spec/services/phone_recaptcha_validator_spec.rb index 6f35b8fa05a..e7fb60cb165 100644 --- a/spec/services/phone_recaptcha_validator_spec.rb +++ b/spec/services/phone_recaptcha_validator_spec.rb @@ -4,7 +4,9 @@ let(:country_score_overrides_config) { {} } let(:score_threshold_config) { 0.2 } let(:parsed_phone) { Phonelib.parse('+15135551234') } - subject(:validator) { described_class.new(parsed_phone:) } + let(:recaptcha_version) { 3 } + let(:analytics) { FakeAnalytics.new } + subject(:validator) { described_class.new(parsed_phone:, recaptcha_version:, analytics:) } before do allow(IdentityConfig.store).to receive(:phone_recaptcha_country_score_overrides). and_return(country_score_overrides_config) @@ -12,19 +14,26 @@ and_return(score_threshold_config) end + it 'passes instance variables to validator' do + recaptcha_validator = instance_double(RecaptchaValidator, valid?: true) + expect(RecaptchaValidator).to receive(:new).and_return(recaptcha_validator) + + validator.valid?('token') + end + describe '#valid?' do it 'is delegated to recaptcha validator' do - recaptcha_validator = RecaptchaValidator.new + recaptcha_validator = instance_double(RecaptchaValidator, valid?: true) expect(validator).to receive(:validator).and_return(recaptcha_validator) expect(recaptcha_validator).to receive(:valid?) - validator.valid? + validator.valid?('token') end end describe '#exempt?' do it 'is delegated to recaptcha validator' do - recaptcha_validator = RecaptchaValidator.new + recaptcha_validator = instance_double(RecaptchaValidator, exempt?: true) expect(validator).to receive(:validator).and_return(recaptcha_validator) expect(recaptcha_validator).to receive(:exempt?) diff --git a/spec/services/recaptcha_validator_spec.rb b/spec/services/recaptcha_validator_spec.rb index 8fd7ffaf3c3..72eaf2edfad 100644 --- a/spec/services/recaptcha_validator_spec.rb +++ b/spec/services/recaptcha_validator_spec.rb @@ -3,8 +3,17 @@ describe RecaptchaValidator do let(:score_threshold) { 0.2 } let(:analytics) { FakeAnalytics.new } + let(:recaptcha_secret_key_v2) { 'recaptcha_secret_key_v2' } + let(:recaptcha_secret_key_v3) { 'recaptcha_secret_key_v3' } subject(:validator) { RecaptchaValidator.new(score_threshold:, analytics:) } + before do + allow(IdentityConfig.store).to receive(:recaptcha_secret_key_v2). + and_return(recaptcha_secret_key_v2) + allow(IdentityConfig.store).to receive(:recaptcha_secret_key_v3). + and_return(recaptcha_secret_key_v3) + end + describe '#exempt?' do subject(:exempt) { validator.exempt? } @@ -67,7 +76,10 @@ let(:token) { 'token' } before do - stub_recaptcha_response_body(success: false, 'error-codes': ['timeout-or-duplicate']) + stub_recaptcha_response( + body: { success: false, 'error-codes': ['timeout-or-duplicate'] }, + token:, + ) end it { expect(valid).to eq(false) } @@ -83,6 +95,7 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, + recaptcha_version: 3, exception_class: nil, ) end @@ -90,7 +103,10 @@ context 'with unsuccessful response due to misconfiguration' do context 'with missing input secret' do before do - stub_recaptcha_response_body(success: false, 'error-codes': ['missing-input-secret']) + stub_recaptcha_response( + body: { success: false, 'error-codes': ['missing-input-secret'] }, + token:, + ) end it { expect(valid).to eq(true) } @@ -106,6 +122,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, + recaptcha_version: 3, exception_class: nil, ) end @@ -113,7 +130,10 @@ context 'with invalid input secret' do before do - stub_recaptcha_response_body(success: false, 'error-codes': ['invalid-input-secret']) + stub_recaptcha_response( + body: { success: false, 'error-codes': ['invalid-input-secret'] }, + token:, + ) end it { expect(valid).to eq(true) } @@ -129,6 +149,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, + recaptcha_version: 3, exception_class: nil, ) end @@ -153,6 +174,7 @@ recaptcha_result: nil, evaluated_as_valid: true, score_threshold: score_threshold, + recaptcha_version: 3, exception_class: 'Faraday::ConnectionFailed', ) end @@ -163,7 +185,7 @@ let(:score) { score_threshold - 0.1 } before do - stub_recaptcha_response_body(success: true, score:) + stub_recaptcha_response(body: { success: true, score: }, token:) end it { expect(valid).to eq(false) } @@ -179,6 +201,7 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, + recaptcha_version: 3, exception_class: nil, ) end @@ -189,7 +212,7 @@ let(:score) { score_threshold + 0.1 } before do - stub_recaptcha_response_body(success: true, score:) + stub_recaptcha_response(body: { success: true, score: }, token:) end it { expect(valid).to eq(true) } @@ -205,6 +228,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, + recaptcha_version: 3, exception_class: nil, ) end @@ -216,13 +240,54 @@ valid end end + + context 'with recaptcha v2' do + before do + stub_recaptcha_response( + body: { success: true, score: }, + secret: recaptcha_secret_key_v2, + token:, + ) + end + + subject(:validator) do + RecaptchaValidator.new(recaptcha_version: 2, score_threshold:, analytics:) + end + + it { expect(valid).to eq(true) } + + it 'logs analytics of the body' do + valid + + expect(analytics).to have_logged_event( + 'reCAPTCHA verify result received', + recaptcha_result: { + 'success' => true, + 'score' => score, + }, + evaluated_as_valid: true, + score_threshold: score_threshold, + recaptcha_version: 2, + exception_class: nil, + ) + end + end + end + end + + context 'with invalid recaptcha_version' do + subject(:validator) do + RecaptchaValidator.new(recaptcha_version: 4, score_threshold:, analytics:) + end + + it 'raises an error during initialization' do + expect { validator }.to raise_error(ArgumentError) end end - def stub_recaptcha_response_body(body) - stub_request(:post, RecaptchaValidator::VERIFICATION_ENDPOINT).to_return( - headers: { 'Content-Type': 'application/json' }, - body: body.to_json, - ) + def stub_recaptcha_response(body:, secret: recaptcha_secret_key_v3, token: nil) + stub_request(:post, RecaptchaValidator::VERIFICATION_ENDPOINT). + with { |req| req.body == URI.encode_www_form(secret:, response: token) }. + to_return(headers: { 'Content-Type': 'application/json' }, body: body.to_json) end end diff --git a/spec/views/phone_setup/spam_protection.html.erb_spec.rb b/spec/views/phone_setup/spam_protection.html.erb_spec.rb new file mode 100644 index 00000000000..24aad92c352 --- /dev/null +++ b/spec/views/phone_setup/spam_protection.html.erb_spec.rb @@ -0,0 +1,38 @@ +require 'rails_helper' + +describe 'users/phone_setup/spam_protection.html.erb' do + let(:user) { build_stubbed(:user) } + let(:form) { NewPhoneForm.new(user:) } + let(:locals) { {} } + + subject(:rendered) { render(template: 'users/phone_setup/spam_protection', locals:) } + + before do + @new_phone_form = form + end + + it 'renders hidden form inputs' do + expect(rendered).to have_field('new_phone_form[phone]', type: :hidden) + expect(rendered).to have_field('new_phone_form[international_code]', type: :hidden) + expect(rendered).to have_field('new_phone_form[otp_delivery_preference]', type: :hidden) + expect(rendered).to have_field('new_phone_form[otp_make_default_number]', type: :hidden) + expect(rendered).to have_field('new_phone_form[recaptcha_version]', type: :hidden, with: '2') + expect(rendered).to have_field('new_phone_form[recaptcha_token]', type: :hidden) + end + + it 'does not render link to two factor options' do + expect(rendered).not_to have_link(t('two_factor_authentication.login_options_link_text')) + end + + context 'with two factor options path' do + let(:two_factor_options_path) { root_path } + let(:locals) { { two_factor_options_path: } } + + it 'renders additional troubleshooting option' do + expect(rendered).to have_link( + t('two_factor_authentication.login_options_link_text'), + href: two_factor_options_path, + ) + end + end +end