diff --git a/app/components/captcha_submit_button_component.html.erb b/app/components/captcha_submit_button_component.html.erb index ca6fd5534d1..175257297aa 100644 --- a/app/components/captcha_submit_button_component.html.erb +++ b/app/components/captcha_submit_button_component.html.erb @@ -1,11 +1,10 @@ <%= content_tag( :'lg-captcha-submit-button', **tag_options, - 'recaptcha-site-key': IdentityConfig.store.recaptcha_site_key_v3, + 'recaptcha-site-key': IdentityConfig.store.recaptcha_site_key, 'recaptcha-action': action, 'recaptcha-enterprise': recaptcha_enterprise?, ) do %> - <%= f.error(:recaptcha_token) %> <% if show_mock_score_field? %> <%= f.input(:recaptcha_token, as: :hidden, input_html: { value: 'mock_token' }) %> <%= render AlertComponent.new(text_tag: :div, class: 'margin-top-0 margin-bottom-2') do %> diff --git a/app/components/captcha_submit_button_component.rb b/app/components/captcha_submit_button_component.rb index 1ea00130bac..9c1a30c5cd6 100644 --- a/app/components/captcha_submit_button_component.rb +++ b/app/components/captcha_submit_button_component.rb @@ -18,16 +18,15 @@ def show_mock_score_field? def recaptcha_script_src return @recaptcha_script_src if defined?(@recaptcha_script_src) - @recaptcha_script_src = begin - if IdentityConfig.store.recaptcha_site_key_v3.present? + @recaptcha_script_src = + if IdentityConfig.store.recaptcha_site_key.present? UriService.add_params( recaptcha_enterprise? ? 'https://www.google.com/recaptcha/enterprise.js' : 'https://www.google.com/recaptcha/api.js', - render: IdentityConfig.store.recaptcha_site_key_v3, + render: IdentityConfig.store.recaptcha_site_key, ) end - end end def recaptcha_enterprise? diff --git a/app/controllers/concerns/recaptcha_concern.rb b/app/controllers/concerns/recaptcha_concern.rb index ff5d58b07f0..5d60ad88c50 100644 --- a/app/controllers/concerns/recaptcha_concern.rb +++ b/app/controllers/concerns/recaptcha_concern.rb @@ -11,10 +11,6 @@ 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 9dfe68896ee..41f9ff5508f 100644 --- a/app/controllers/users/phone_setup_controller.rb +++ b/app/controllers/users/phone_setup_controller.rb @@ -35,9 +35,8 @@ def create if result.success? handle_create_success(@new_phone_form.phone) - elsif recoverable_recaptcha_error?(result) - render :spam_protection else + flash.now[:error] = result.first_error_message(:recaptcha_token) render :index end end @@ -131,7 +130,6 @@ def new_phone_form_params :otp_delivery_preference, :otp_make_default_number, :recaptcha_token, - :recaptcha_version, :recaptcha_mock_score, ) end diff --git a/app/forms/new_phone_form.rb b/app/forms/new_phone_form.rb index c79ea4a8420..a12f2c73afa 100644 --- a/app/forms/new_phone_form.rb +++ b/app/forms/new_phone_form.rb @@ -24,7 +24,6 @@ class NewPhoneForm :otp_make_default_number, :setup_voice_preference, :recaptcha_token, - :recaptcha_version, :recaptcha_mock_score alias_method :setup_voice_preference?, :setup_voice_preference @@ -35,7 +34,6 @@ 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) @@ -145,7 +143,7 @@ def recaptcha_validator end def recaptcha_validator_args - args = { recaptcha_version:, analytics: } + args = { analytics: } if IdentityConfig.store.phone_recaptcha_mock_validator args.merge(validator_class: RecaptchaMockValidator, score: recaptcha_mock_score) elsif FeatureManagement.recaptcha_enterprise? @@ -173,7 +171,6 @@ 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 @recaptcha_mock_score = params[:recaptcha_mock_score].to_f if params.key?(:recaptcha_mock_score) end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index f7b56a7af0e..2990cd45fc8 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4609,7 +4609,6 @@ def reactivate_account_visit # @param [String] validator_class Class name of validator # @param [String, nil] exception_class Class name of exception, if error occurred # @param [String, nil] phone_country_code Country code associated with reCAPTCHA phone result - # @param [String] recaptcha_version def recaptcha_verify_result_received( recaptcha_result:, score_threshold:, @@ -4617,7 +4616,6 @@ def recaptcha_verify_result_received( validator_class:, exception_class:, phone_country_code: nil, - recaptcha_version: nil, **extra ) track_event( @@ -4629,7 +4627,6 @@ def recaptcha_verify_result_received( validator_class:, exception_class:, phone_country_code:, - recaptcha_version:, **extra, }.compact, ) diff --git a/app/services/form_response.rb b/app/services/form_response.rb index 5c1f405e602..e62f67c9613 100644 --- a/app/services/form_response.rb +++ b/app/services/form_response.rb @@ -39,10 +39,10 @@ def merge(other) end end - def first_error_message + def first_error_message(key = nil) return if errors.blank? - _key, message_or_messages = errors.first - Array(message_or_messages).first + key ||= errors.keys.first + errors[key].first end def ==(other) diff --git a/app/services/recaptcha_enterprise_validator.rb b/app/services/recaptcha_enterprise_validator.rb index 8eddb0d9b10..2430751c450 100644 --- a/app/services/recaptcha_enterprise_validator.rb +++ b/app/services/recaptcha_enterprise_validator.rb @@ -22,7 +22,7 @@ def recaptcha_result(recaptcha_token) { event: { token: recaptcha_token, - siteKey: recaptcha_site_key, + siteKey: IdentityConfig.store.recaptcha_site_key, expectedAction: recaptcha_action, }, }, @@ -52,13 +52,4 @@ def faraday conn.response :json end end - - def recaptcha_site_key - case recaptcha_version - when 2 - IdentityConfig.store.recaptcha_site_key_v2 - when 3 - IdentityConfig.store.recaptcha_site_key_v3 - end - end end diff --git a/app/services/recaptcha_validator.rb b/app/services/recaptcha_validator.rb index b9af5e24232..ceee4414ca9 100644 --- a/app/services/recaptcha_validator.rb +++ b/app/services/recaptcha_validator.rb @@ -3,10 +3,8 @@ class RecaptchaValidator VERIFICATION_ENDPOINT = 'https://www.google.com/recaptcha/api/siteverify' RESULT_ERRORS = ['missing-input-secret', 'invalid-input-secret'].freeze - VALID_RECAPTCHA_VERSIONS = [2, 3].freeze - attr_reader :recaptcha_version, - :recaptcha_action, + attr_reader :recaptcha_action, :score_threshold, :analytics, :extra_analytics_properties @@ -20,20 +18,13 @@ def initialize(success:, score: nil, errors: [], reasons: []) end def initialize( - recaptcha_version: 3, recaptcha_action: nil, score_threshold: 0.0, analytics: nil, extra_analytics_properties: {} ) - if !VALID_RECAPTCHA_VERSIONS.include?(recaptcha_version) - raise ArgumentError, "Invalid reCAPTCHA version #{recaptcha_version}, expected one of " \ - "#{VALID_RECAPTCHA_VERSIONS}" - end - @score_threshold = score_threshold @analytics = analytics - @recaptcha_version = recaptcha_version @recaptcha_action = recaptcha_action @extra_analytics_properties = extra_analytics_properties end @@ -79,21 +70,12 @@ def recaptcha_result_valid?(result) return true if result.blank? if result.success? - recaptcha_score_meets_threshold?(result.score) + result.score >= score_threshold else result.errors.present? end end - def recaptcha_score_meets_threshold?(score) - case recaptcha_version - when 2 - true - when 3 - score >= score_threshold - end - end - def is_result_error?(error_code) RESULT_ERRORS.include?(error_code) end @@ -102,7 +84,6 @@ def log_analytics(result: nil, error: nil) analytics&.recaptcha_verify_result_received( recaptcha_result: result.to_h.presence, score_threshold:, - recaptcha_version:, evaluated_as_valid: recaptcha_result_valid?(result), exception_class: error&.class&.name, validator_class: self.class.name, @@ -111,11 +92,6 @@ def log_analytics(result: nil, error: nil) 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 + IdentityConfig.store.recaptcha_secret_key end end diff --git a/app/views/users/phone_setup/spam_protection.html.erb b/app/views/users/phone_setup/spam_protection.html.erb deleted file mode 100644 index 67768b19906..00000000000 --- a/app/views/users/phone_setup/spam_protection.html.erb +++ /dev/null @@ -1,66 +0,0 @@ -<% self.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 %> - <% if IdentityConfig.store.recaptcha_site_key_v2.present? %> -
- <% if FeatureManagement.recaptcha_enterprise? %> - - <% else %> - - <% end %> - <% elsif IdentityConfig.store.phone_recaptcha_mock_validator %> - <%= f.input :recaptcha_mock_score, as: :hidden, input_html: { value: 1.0 } %> - <%= render ButtonComponent.new(id: :submit_button, type: :button, big: true, wide: true, class: 'display-block margin-y-5') do %> - <%= t('forms.buttons.continue') %> - <% end %> - <%= javascript_tag(nonce: true) do %> - document.getElementById('submit_button').addEventListener('click', () => onCaptchaResponse('mock_token')); - <% end %> - <% end %> -<% end %> - -<%= render TroubleshootingOptionsComponent.new do |c| %> - <% c.with_header { t('components.troubleshooting_options.default_heading') } %> - <% if in_multi_mfa_selection_flow? %> - <% c.with_option( - url: authentication_methods_setup_path, - ).with_content(t('two_factor_authentication.login_options_link_text')) %> - <% end %> - <% c.with_option( - url: help_center_redirect_path( - category: 'get-started', - article: 'authentication-options', - flow: :two_factor_authentication, - step: :phone_setup_spam_protection, - ), - new_tab: true, - ).with_content(t('two_factor_authentication.learn_more')) %> -<% end %> - -<% unless in_multi_mfa_selection_flow? %> - <%= render PageFooterComponent.new do %> - <%= link_to t('links.cancel'), account_path %> - <% end %> -<% end %> diff --git a/config/application.yml.default b/config/application.yml.default index d289730f772..fe253c12e16 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -268,10 +268,8 @@ raise_on_missing_title: false reauthn_window: 1200 recaptcha_enterprise_api_key: '' recaptcha_enterprise_project_id: '' -recaptcha_site_key_v2: '' -recaptcha_site_key_v3: '' -recaptcha_secret_key_v2: '' -recaptcha_secret_key_v3: '' +recaptcha_site_key: '' +recaptcha_secret_key: '' recovery_code_length: 4 redis_throttle_url: redis://localhost:6379/1 redis_url: redis://localhost:6379/0 diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index 711e2a1f6a5..c7d5b29a2c4 100644 --- a/config/locales/errors/en.yml +++ b/config/locales/errors/en.yml @@ -69,7 +69,9 @@ en: invalid_phone_number: international: Enter a phone number with the correct number of digits. us: Enter a 10 digit phone number. - invalid_recaptcha_token: You must complete the spam prevention challenge. + invalid_recaptcha_token: We’re sorry, but your computer or network may be + sending automated queries. To protect our users, we can’t process your + request right now. invalid_sms_number: The phone number entered doesn’t support text messaging. Try the Phone call option. invalid_voice_number: Invalid phone number. Check that you’ve entered the diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index bd5943484ca..22a745cb3fe 100644 --- a/config/locales/errors/es.yml +++ b/config/locales/errors/es.yml @@ -74,7 +74,9 @@ es: invalid_phone_number: international: Ingrese un número de teléfono con el número correcto de dígitos. us: Ingrese un número de teléfono de 10 dígitos. - invalid_recaptcha_token: Debes superar el desafío de prevención de spam. + invalid_recaptcha_token: Lo sentimos, pero es posible que tu computadora o red + te estén enviando consultas automáticas. Para proteger a nuestros + usuarios, no podemos procesar tu solicitud en este momento. invalid_sms_number: El número de teléfono ingresado no admite mensajes de texto. Pruebe la opción de llamada telefónica. invalid_voice_number: Numero de telefono invalido. Verifique que haya ingresado diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index 570122627c4..2b6c23f2de7 100644 --- a/config/locales/errors/fr.yml +++ b/config/locales/errors/fr.yml @@ -82,7 +82,9 @@ fr: invalid_phone_number: international: Saisissez un numéro de téléphone avec le nombre correct de chiffres. us: Entrez un numéro de téléphone à 10 chiffres. - invalid_recaptcha_token: Vous devez relever le défi de la prévention du pourriel. + invalid_recaptcha_token: Désolé, il est possible que votre ordinateur ou votre + réseau envoie des requêtes automatiques. Pour protéger nos utilisateurs, + nous ne pouvons pas traiter votre demande pour le moment. invalid_sms_number: Le numéro de téléphone saisi ne prend pas en charge les messages textuels. Veuillez essayer l’option d’appel téléphonique. invalid_voice_number: Numéro de téléphone invalide. Vérifiez que vous avez entré diff --git a/config/locales/forms/en.yml b/config/locales/forms/en.yml index 3f8038ec6ae..47823b2a346 100644 --- a/config/locales/forms/en.yml +++ b/config/locales/forms/en.yml @@ -93,9 +93,6 @@ 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_setup: diff --git a/config/locales/forms/es.yml b/config/locales/forms/es.yml index 84994051f13..64fdd91dee8 100644 --- a/config/locales/forms/es.yml +++ b/config/locales/forms/es.yml @@ -98,9 +98,6 @@ 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_setup: diff --git a/config/locales/forms/fr.yml b/config/locales/forms/fr.yml index d52bf4b5cef..ac4f4b2abbe 100644 --- a/config/locales/forms/fr.yml +++ b/config/locales/forms/fr.yml @@ -99,9 +99,6 @@ 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_setup: diff --git a/config/locales/titles/en.yml b/config/locales/titles/en.yml index e960b51410e..35e42c0ed31 100644 --- a/config/locales/titles/en.yml +++ b/config/locales/titles/en.yml @@ -76,7 +76,6 @@ en: completion_new_sp: You are now signing in for the first time completion_reverified_consent: Share your updated information with %{sp} 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 11b409ab388..7e75cc4ece4 100644 --- a/config/locales/titles/es.yml +++ b/config/locales/titles/es.yml @@ -78,7 +78,6 @@ es: completion_new_sp: Acabas de iniciar sesión por primera vez completion_reverified_consent: Proporciónale tu información actualizada a %{sp} 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 93940a033d3..848f0acdeec 100644 --- a/config/locales/titles/fr.yml +++ b/config/locales/titles/fr.yml @@ -77,7 +77,6 @@ fr: completion_new_sp: Vous vous connectez pour la première fois completion_reverified_consent: Partagez vos informations mises à jour avec %{sp} 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/lib/feature_management.rb b/lib/feature_management.rb index 7e628fea0ba..9f079db3f8a 100644 --- a/lib/feature_management.rb +++ b/lib/feature_management.rb @@ -102,14 +102,10 @@ def self.log_to_stdout? end def self.phone_recaptcha_enabled? - return false if IdentityConfig.store.recaptcha_site_key_v2.blank? || - IdentityConfig.store.recaptcha_site_key_v3.blank? || + return false if IdentityConfig.store.recaptcha_site_key.blank? || !IdentityConfig.store.phone_recaptcha_score_threshold.positive? - recaptcha_enterprise? || ( - IdentityConfig.store.recaptcha_secret_key_v2.present? && - IdentityConfig.store.recaptcha_secret_key_v3.present? - ) + recaptcha_enterprise? || IdentityConfig.store.recaptcha_secret_key.present? end def self.recaptcha_enterprise? diff --git a/lib/identity_config.rb b/lib/identity_config.rb index f3b4ea3ffc9..7f76b012fb9 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -402,10 +402,8 @@ def self.build_store(config_map) config.add(:reauthn_window, type: :integer) config.add(:recaptcha_enterprise_api_key, type: :string) config.add(:recaptcha_enterprise_project_id, type: :string) - config.add(:recaptcha_secret_key_v2, type: :string) - config.add(:recaptcha_secret_key_v3, type: :string) - config.add(:recaptcha_site_key_v2, type: :string) - config.add(:recaptcha_site_key_v3, type: :string) + config.add(:recaptcha_secret_key, type: :string) + config.add(:recaptcha_site_key, type: :string) config.add(:recovery_code_length, type: :integer) config.add(:redis_pool_size, type: :integer) config.add(:redis_throttle_pool_size, type: :integer) diff --git a/spec/components/captcha_submit_button_component_spec.rb b/spec/components/captcha_submit_button_component_spec.rb index 05cbea1a01d..1deb19acd57 100644 --- a/spec/components/captcha_submit_button_component_spec.rb +++ b/spec/components/captcha_submit_button_component_spec.rb @@ -20,20 +20,9 @@ expect(rendered).to have_content(content) end - context 'with recaptcha token input errors' do - let(:error_message) { 'Invalid token' } - before do - form_object.errors.add(:recaptcha_token, error_message) - end - - it 'renders recaptcha token errors' do - expect(rendered).to have_content(error_message) - end - end - context 'without configured recaptcha site key' do before do - allow(IdentityConfig.store).to receive(:recaptcha_site_key_v3).and_return(nil) + allow(IdentityConfig.store).to receive(:recaptcha_site_key).and_return(nil) end it 'renders without recaptcha site key attribute' do @@ -48,7 +37,7 @@ context 'with configured recaptcha site key' do let(:recaptcha_site_key) { 'site_key' } before do - allow(IdentityConfig.store).to receive(:recaptcha_site_key_v3).and_return(recaptcha_site_key) + allow(IdentityConfig.store).to receive(:recaptcha_site_key).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 a1390838b8b..6d0c12984a4 100644 --- a/spec/controllers/concerns/recaptcha_concern_spec.rb +++ b/spec/controllers/concerns/recaptcha_concern_spec.rb @@ -20,34 +20,4 @@ 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 7c22f889843..20dc76dd601 100644 --- a/spec/controllers/users/phone_setup_controller_spec.rb +++ b/spec/controllers/users/phone_setup_controller_spec.rb @@ -94,19 +94,24 @@ } expect(response).to render_template(:index) + expect(flash[:error]).to be_blank end - context 'with recoverable recaptcha error' do - it 'renders spam protection template' do - stub_sign_in + context 'with recaptcha error' do + before do + allow(FeatureManagement).to receive(:phone_recaptcha_enabled?).and_return(true) + allow(IdentityConfig.store).to receive(:phone_recaptcha_country_score_overrides). + and_return({}) + allow(IdentityConfig.store).to receive(:phone_recaptcha_score_threshold).and_return(0.6) + end - allow(controller).to receive(:recoverable_recaptcha_error?) do |result| - result.is_a?(FormResponse) - end + it 'renders form with error message' do + stub_sign_in - post :create, params: { new_phone_form: { international_code: 'CA' } } + post :create, params: { new_phone_form: { phone: '3065550100', international_code: 'CA' } } - expect(response).to render_template(:spam_protection) + expect(response).to render_template(:index) + expect(flash[:error]).to eq(t('errors.messages.invalid_recaptcha_token')) end end diff --git a/spec/features/phone/add_phone_spec.rb b/spec/features/phone/add_phone_spec.rb index 1b14f111958..d45de1f5cb5 100644 --- a/spec/features/phone/add_phone_spec.rb +++ b/spec/features/phone/add_phone_spec.rb @@ -214,17 +214,12 @@ sign_in_and_2fa_user(user) within('.sidenav') { click_on t('account.navigation.add_phone_number') } - # Failing international should display spam protection screen + # Failing international should display error message fill_in t('two_factor_authentication.phone_label'), with: '+61 0491 570 006' fill_in t('components.captcha_submit_button.mock_score_label'), with: '0.5' click_send_one_time_code - expect(page).to have_content(t('titles.spam_protection'), wait: 5) - expect(page).not_to have_link(t('two_factor_authentication.login_options_link_text')) - expect(page).to have_link(t('links.cancel')) - click_continue - expect(page).to have_content(t('two_factor_authentication.header_text')) - visit account_path - within('.sidenav') { click_on t('account.navigation.add_phone_number') } + expect(page).to have_current_path(phone_setup_path, wait: 5) + expect(page).to have_content(t('errors.messages.invalid_recaptcha_token')) expect(fake_analytics).to have_logged_event( 'reCAPTCHA verify result received', hash_including( @@ -236,7 +231,6 @@ }, evaluated_as_valid: false, score_threshold: 0.6, - recaptcha_version: 3, phone_country_code: 'AU', ), ) diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 5e4c376801b..674321b43f9 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -179,9 +179,8 @@ fill_in t('two_factor_authentication.phone_label'), with: '+61 0491 570 006' fill_in t('components.captcha_submit_button.mock_score_label'), with: '0.5' click_send_one_time_code - expect(page).to have_content(t('titles.spam_protection'), wait: 5) - expect(page).to have_link(t('two_factor_authentication.login_options_link_text')) - expect(page).not_to have_link(t('links.cancel')) + expect(page).to have_current_path(phone_setup_path, wait: 5) + expect(page).to have_content(t('errors.messages.invalid_recaptcha_token')) end context 'with js', js: true do diff --git a/spec/forms/new_phone_form_spec.rb b/spec/forms/new_phone_form_spec.rb index 1497c03f3d2..b15a95b3cee 100644 --- a/spec/forms/new_phone_form_spec.rb +++ b/spec/forms/new_phone_form_spec.rb @@ -375,7 +375,7 @@ context 'with recaptcha enabled' do let(:valid) { nil } - let(:validator) { PhoneRecaptchaValidator.new(recaptcha_version: 3, parsed_phone: nil) } + let(:validator) { PhoneRecaptchaValidator.new(parsed_phone: nil) } let(:recaptcha_token) { 'token' } let(:phone) { '3065550100' } let(:international_code) { 'CA' } @@ -397,40 +397,9 @@ 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 - context 'with recaptcha enterprise' do let(:validator) do PhoneRecaptchaValidator.new( - recaptcha_version: 3, validator_class: RecaptchaEnterpriseValidator, parsed_phone: nil, ) diff --git a/spec/lib/feature_management_spec.rb b/spec/lib/feature_management_spec.rb index 1542af8a2b1..937f6a93b1d 100644 --- a/spec/lib/feature_management_spec.rb +++ b/spec/lib/feature_management_spec.rb @@ -377,10 +377,8 @@ end describe '.phone_recaptcha_enabled?' do - let(:recaptcha_site_key_v2) { '' } - let(:recaptcha_site_key_v3) { '' } - let(:recaptcha_secret_key_v2) { '' } - let(:recaptcha_secret_key_v3) { '' } + let(:recaptcha_site_key) { '' } + let(:recaptcha_secret_key) { '' } let(:recaptcha_enterprise_api_key) { '' } let(:recaptcha_enterprise_project_id) { '' } let(:phone_recaptcha_score_threshold) { 0.0 } @@ -388,14 +386,10 @@ subject(:phone_recaptcha_enabled) { FeatureManagement.phone_recaptcha_enabled? } before do - 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(: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(:phone_recaptcha_score_threshold). and_return(phone_recaptcha_score_threshold) allow(IdentityConfig.store).to receive(:recaptcha_enterprise_api_key). @@ -406,43 +400,31 @@ it { expect(phone_recaptcha_enabled).to eq(false) } - context 'with configured recaptcha v2 site key' do - let(:recaptcha_site_key_v2) { 'key' } + context 'with configured recaptcha site key' do + let(:recaptcha_site_key) { 'key' } it { expect(phone_recaptcha_enabled).to eq(false) } - context 'with configured recaptcha v3 site key' do - let(:recaptcha_site_key_v3) { 'key' } + 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(false) } - context 'with configured default success rate threshold greater than 0' do - let(:phone_recaptcha_score_threshold) { 1.0 } + context 'with configured recaptcha secret key' do + let(:recaptcha_secret_key) { 'key' } - it { expect(phone_recaptcha_enabled).to eq(false) } - - context 'with configured recaptcha v2 secret key' do - let(:recaptcha_secret_key_v2) { 'key' } - - 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(true) } - end - end + it { expect(phone_recaptcha_enabled).to eq(true) } + end - context 'with configured recaptcha enterprise api key' do - let(:recaptcha_enterprise_api_key) { 'key' } + context 'with configured recaptcha enterprise api key' do + let(:recaptcha_enterprise_api_key) { 'key' } - it { expect(phone_recaptcha_enabled).to eq(false) } + it { expect(phone_recaptcha_enabled).to eq(false) } - context 'with configured recaptcha enterprise project id' do - let(:recaptcha_enterprise_project_id) { 'project-id' } + context 'with configured recaptcha enterprise project id' do + let(:recaptcha_enterprise_project_id) { 'project-id' } - it { expect(phone_recaptcha_enabled).to eq(true) } - end + it { expect(phone_recaptcha_enabled).to eq(true) } end end end diff --git a/spec/services/form_response_spec.rb b/spec/services/form_response_spec.rb index 76f3659702d..5efcefb17ac 100644 --- a/spec/services/form_response_spec.rb +++ b/spec/services/form_response_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' RSpec.describe FormResponse do + let(:success) { true } + let(:errors) { {} } + subject(:form_response) { FormResponse.new(success:, errors:) } + describe '#success?' do context 'when the success argument is true' do it 'returns true' do @@ -107,6 +111,37 @@ end end + describe '#first_error_message' do + let(:key) { nil } + subject(:first_error_message) { form_response.first_error_message(*[key].compact) } + + context 'with no errors' do + let(:errors) { {} } + + it { expect(first_error_message).to be_nil } + end + + context 'with errors' do + let(:errors) { { email: ['invalid', 'too_short'], language: ['blank'] } } + + context 'without specified key' do + let(:key) { nil } + + it 'returns the first error of the first field' do + expect(first_error_message).to eq('invalid') + end + end + + context 'with specified key' do + let(:key) { :language } + + it 'returns the first error of the specified field' do + expect(first_error_message).to eq('blank') + end + end + end + end + describe '#to_h' do context 'when the extra argument is nil' do it 'returns a hash with success and errors keys' do diff --git a/spec/services/phone_recaptcha_validator_spec.rb b/spec/services/phone_recaptcha_validator_spec.rb index 688ee0a7aba..ec62fc93a50 100644 --- a/spec/services/phone_recaptcha_validator_spec.rb +++ b/spec/services/phone_recaptcha_validator_spec.rb @@ -4,9 +4,8 @@ let(:country_score_overrides_config) { {} } let(:score_threshold_config) { 0.2 } let(:parsed_phone) { Phonelib.parse('+15135551234') } - let(:recaptcha_version) { 3 } let(:analytics) { FakeAnalytics.new } - subject(:validator) { described_class.new(parsed_phone:, recaptcha_version:, analytics:) } + subject(:validator) { described_class.new(parsed_phone:, analytics:) } before do allow(IdentityConfig.store).to receive(:phone_recaptcha_country_score_overrides). and_return(country_score_overrides_config) @@ -20,7 +19,6 @@ with( score_threshold: score_threshold_config, analytics:, - recaptcha_version:, recaptcha_action: described_class::RECAPTCHA_ACTION, extra_analytics_properties: { phone_country_code: parsed_phone.country, @@ -35,7 +33,6 @@ subject(:validator) do described_class.new( parsed_phone:, - recaptcha_version:, analytics:, validator_class: RecaptchaMockValidator, ) diff --git a/spec/services/recaptcha_enterprise_validator_spec.rb b/spec/services/recaptcha_enterprise_validator_spec.rb index e7a3a46d80f..f2e191866d2 100644 --- a/spec/services/recaptcha_enterprise_validator_spec.rb +++ b/spec/services/recaptcha_enterprise_validator_spec.rb @@ -7,7 +7,7 @@ let(:action) { 'example_action' } let(:recaptcha_enterprise_api_key) { 'recaptcha_enterprise_api_key' } let(:recaptcha_enterprise_project_id) { 'project_id' } - let(:recaptcha_site_key_v3) { 'recaptcha_site_key_v3' } + let(:recaptcha_site_key) { 'recaptcha_site_key' } let(:assessment_url) do format( '%{base_endpoint}/%{project_id}/assessments?key=%{api_key}', @@ -31,8 +31,8 @@ and_return(recaptcha_enterprise_project_id) allow(IdentityConfig.store).to receive(:recaptcha_enterprise_api_key). and_return(recaptcha_enterprise_api_key) - allow(IdentityConfig.store).to receive(:recaptcha_site_key_v3). - and_return(recaptcha_site_key_v3) + allow(IdentityConfig.store).to receive(:recaptcha_site_key). + and_return(recaptcha_site_key) end describe '#exempt?' do @@ -122,7 +122,6 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaEnterpriseValidator', ) end @@ -156,7 +155,6 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaEnterpriseValidator', ) end @@ -178,7 +176,6 @@ 'reCAPTCHA verify result received', evaluated_as_valid: true, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaEnterpriseValidator', exception_class: 'Faraday::ConnectionFailed', ) @@ -216,7 +213,6 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaEnterpriseValidator', ) end @@ -253,7 +249,6 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaEnterpriseValidator', ) end @@ -290,7 +285,6 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaEnterpriseValidator', extra: true, ) @@ -307,7 +301,7 @@ end end - def stub_recaptcha_response(body:, action:, site_key: recaptcha_site_key_v3, token: nil) + def stub_recaptcha_response(body:, action:, site_key: recaptcha_site_key, token: nil) stub_request(:post, assessment_url). with do |req| req.body == { event: { token:, siteKey: site_key, expectedAction: action } }.to_json diff --git a/spec/services/recaptcha_mock_validator_spec.rb b/spec/services/recaptcha_mock_validator_spec.rb index b60ce10b5cf..84b461f0a47 100644 --- a/spec/services/recaptcha_mock_validator_spec.rb +++ b/spec/services/recaptcha_mock_validator_spec.rb @@ -4,7 +4,9 @@ let(:score_threshold) { 0.2 } let(:analytics) { FakeAnalytics.new } let(:score) { nil } - subject(:validator) { RecaptchaMockValidator.new(score_threshold:, analytics:, score:) } + subject(:validator) do + RecaptchaMockValidator.new(score_threshold:, analytics:, score:) + end around do |example| freeze_time { example.run } @@ -32,7 +34,6 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaMockValidator', ) end @@ -57,7 +58,6 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaMockValidator', ) end diff --git a/spec/services/recaptcha_validator_spec.rb b/spec/services/recaptcha_validator_spec.rb index 3b2459269b9..d475410e00e 100644 --- a/spec/services/recaptcha_validator_spec.rb +++ b/spec/services/recaptcha_validator_spec.rb @@ -4,18 +4,15 @@ let(:score_threshold) { 0.2 } let(:analytics) { FakeAnalytics.new } let(:extra_analytics_properties) { {} } - let(:recaptcha_secret_key_v2) { 'recaptcha_secret_key_v2' } - let(:recaptcha_secret_key_v3) { 'recaptcha_secret_key_v3' } + let(:recaptcha_secret_key) { 'recaptcha_secret_key' } subject(:validator) do RecaptchaValidator.new(score_threshold:, analytics:, extra_analytics_properties:) end 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) + allow(IdentityConfig.store).to receive(:recaptcha_secret_key). + and_return(recaptcha_secret_key) end describe '#exempt?' do @@ -101,7 +98,6 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaValidator', ) end @@ -130,7 +126,6 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaValidator', ) end @@ -159,7 +154,6 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaValidator', ) end @@ -183,7 +177,6 @@ 'reCAPTCHA verify result received', evaluated_as_valid: true, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaValidator', exception_class: 'Faraday::ConnectionFailed', ) @@ -213,7 +206,6 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaValidator', ) end @@ -242,7 +234,6 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaValidator', ) end @@ -263,7 +254,6 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - recaptcha_version: 3, validator_class: 'RecaptchaValidator', extra: true, ) @@ -277,54 +267,10 @@ 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:, - errors: [], - reasons: [], - }, - evaluated_as_valid: true, - score_threshold: score_threshold, - recaptcha_version: 2, - validator_class: 'RecaptchaValidator', - ) - 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:, secret: recaptcha_secret_key_v3, token: nil) + def stub_recaptcha_response(body:, secret: recaptcha_secret_key, 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) diff --git a/spec/views/phone_setup/spam_protection.html.erb_spec.rb b/spec/views/phone_setup/spam_protection.html.erb_spec.rb deleted file mode 100644 index 24beaf790b8..00000000000 --- a/spec/views/phone_setup/spam_protection.html.erb_spec.rb +++ /dev/null @@ -1,98 +0,0 @@ -require 'rails_helper' - -RSpec.describe 'users/phone_setup/spam_protection.html.erb' do - let(:user) { build_stubbed(:user) } - let(:form) { NewPhoneForm.new(user:) } - let(:in_multi_mfa_selection_flow) { false } - - subject(:rendered) { render(template: 'users/phone_setup/spam_protection') } - - before do - @new_phone_form = form - allow(view).to receive(:in_multi_mfa_selection_flow?).and_return(in_multi_mfa_selection_flow) - 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 'has expected troubleshooting options' do - expect(rendered).to have_link( - t('two_factor_authentication.learn_more'), - href: help_center_redirect_path( - category: 'get-started', - article: 'authentication-options', - flow: :two_factor_authentication, - step: :phone_setup_spam_protection, - ), - ) - expect(rendered).not_to have_link(t('two_factor_authentication.login_options_link_text')) - end - - context 'in multi mfa selectino flow' do - let(:in_multi_mfa_selection_flow) { true } - - it 'renders additional troubleshooting option to two factor options' do - expect(rendered).to have_link( - t('two_factor_authentication.login_options_link_text'), - href: authentication_methods_setup_path, - ) - end - - it 'does not render cancel option' do - expect(rendered).to_not have_link( - t('links.cancel'), - href: account_path, - ) - end - end - - context 'fully registered user adding new phone' do - let(:user) { create(:user, :fully_registered) } - - it 'does not render additional troubleshooting option to two factor options' do - expect(rendered).to_not have_link( - t('two_factor_authentication.login_options_link_text'), - href: authentication_methods_setup_path, - ) - end - - it 'renders cancel option' do - expect(rendered).to have_link( - t('links.cancel'), - href: account_path, - ) - end - end - - context 'with configured recaptcha site key' do - before do - allow(IdentityConfig.store).to receive(:recaptcha_site_key_v2).and_return('key') - end - - it 'renders recaptcha script' do - expect(rendered).to have_css( - 'script[src="https://www.google.com/recaptcha/api.js"]', - visible: :all, - ) - end - - context 'with recaptcha enterprise' do - before do - allow(FeatureManagement).to receive(:recaptcha_enterprise?).and_return(true) - end - - it 'renders recaptcha script' do - expect(rendered).to have_css( - 'script[src="https://www.google.com/recaptcha/enterprise.js"]', - visible: :all, - ) - end - end - end -end