diff --git a/app/forms/new_phone_form.rb b/app/forms/new_phone_form.rb index 62c6aa03f10..01944cadcfc 100644 --- a/app/forms/new_phone_form.rb +++ b/app/forms/new_phone_form.rb @@ -131,20 +131,20 @@ def validate_not_premium_rate def validate_recaptcha_token return if !validate_recaptcha_token? - recaptcha_validator.submit(recaptcha_token) - errors.merge!(recaptcha_validator) + recaptcha_form.submit(recaptcha_token) + errors.merge!(recaptcha_form) end - def recaptcha_validator - @recaptcha_validator ||= PhoneRecaptchaValidator.new(parsed_phone:, **recaptcha_validator_args) + def recaptcha_form + @recaptcha_form ||= PhoneRecaptchaForm.new(parsed_phone:, **recaptcha_form_args) end - def recaptcha_validator_args + def recaptcha_form_args args = { analytics: } if IdentityConfig.store.phone_recaptcha_mock_validator - args.merge(validator_class: RecaptchaMockValidator, score: recaptcha_mock_score) + args.merge(form_class: RecaptchaMockForm, score: recaptcha_mock_score) elsif FeatureManagement.recaptcha_enterprise? - args.merge(validator_class: RecaptchaEnterpriseValidator) + args.merge(form_class: RecaptchaEnterpriseForm) else args end diff --git a/app/services/phone_recaptcha_validator.rb b/app/forms/phone_recaptcha_form.rb similarity index 65% rename from app/services/phone_recaptcha_validator.rb rename to app/forms/phone_recaptcha_form.rb index 2878b1265d6..58c7206b8ab 100644 --- a/app/services/phone_recaptcha_validator.rb +++ b/app/forms/phone_recaptcha_form.rb @@ -1,16 +1,16 @@ # frozen_string_literal: true -class PhoneRecaptchaValidator +class PhoneRecaptchaForm RECAPTCHA_ACTION = 'phone_setup' - attr_reader :parsed_phone, :validator_class, :validator_args + attr_reader :parsed_phone, :form_class, :form_args - delegate :submit, :errors, to: :validator + delegate :submit, :errors, to: :form - def initialize(parsed_phone:, validator_class: RecaptchaValidator, **validator_args) + def initialize(parsed_phone:, form_class: RecaptchaForm, **form_args) @parsed_phone = parsed_phone - @validator_class = validator_class - @validator_args = validator_args + @form_class = form_class + @form_args = form_args end def self.country_score_overrides @@ -23,14 +23,14 @@ def score_threshold private - def validator - @validator ||= validator_class.new( + def form + @form ||= form_class.new( score_threshold:, recaptcha_action: RECAPTCHA_ACTION, extra_analytics_properties: { phone_country_code: parsed_phone.country, }, - **validator_args, + **form_args, ) end diff --git a/app/services/recaptcha_enterprise_validator.rb b/app/forms/recaptcha_enterprise_form.rb similarity index 96% rename from app/services/recaptcha_enterprise_validator.rb rename to app/forms/recaptcha_enterprise_form.rb index 5ab494efb94..b3439341c67 100644 --- a/app/services/recaptcha_enterprise_validator.rb +++ b/app/forms/recaptcha_enterprise_form.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class RecaptchaEnterpriseValidator < RecaptchaValidator +class RecaptchaEnterpriseForm < RecaptchaForm BASE_VERIFICATION_ENDPOINT = 'https://recaptchaenterprise.googleapis.com/v1/projects' private diff --git a/app/services/recaptcha_validator.rb b/app/forms/recaptcha_form.rb similarity index 98% rename from app/services/recaptcha_validator.rb rename to app/forms/recaptcha_form.rb index 6f1a0c8a91e..7254961c663 100644 --- a/app/services/recaptcha_validator.rb +++ b/app/forms/recaptcha_form.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class RecaptchaValidator +class RecaptchaForm include ActiveModel::Model include ActionView::Helpers::TranslationHelper @@ -103,7 +103,7 @@ def log_analytics(result: nil, error: nil) score_threshold:, evaluated_as_valid: recaptcha_result_valid?(result), exception_class: error&.class&.name, - validator_class: self.class.name, + form_class: self.class.name, **extra_analytics_properties, ) end diff --git a/app/services/recaptcha_mock_validator.rb b/app/forms/recaptcha_mock_form.rb similarity index 81% rename from app/services/recaptcha_mock_validator.rb rename to app/forms/recaptcha_mock_form.rb index eedc7c156c7..c4cded23c8e 100644 --- a/app/services/recaptcha_mock_validator.rb +++ b/app/forms/recaptcha_mock_form.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class RecaptchaMockValidator < RecaptchaValidator +class RecaptchaMockForm < RecaptchaForm attr_reader :score def initialize(score:, **kwargs) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index f8a2a05252f..7ac91a2a4fb 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4639,14 +4639,14 @@ def reactivate_account_visit # @param [Hash] recaptcha_result Full reCAPTCHA response body # @param [Float] score_threshold Minimum value for considering passing result # @param [Boolean] evaluated_as_valid Whether result was considered valid - # @param [String] validator_class Class name of validator + # @param [String] form_class Class name of form # @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 def recaptcha_verify_result_received( recaptcha_result:, score_threshold:, evaluated_as_valid:, - validator_class:, + form_class:, exception_class:, phone_country_code: nil, **extra @@ -4657,7 +4657,7 @@ def recaptcha_verify_result_received( recaptcha_result:, score_threshold:, evaluated_as_valid:, - validator_class:, + form_class:, exception_class:, phone_country_code:, **extra, diff --git a/app/views/users/phone_setup/index.html.erb b/app/views/users/phone_setup/index.html.erb index f4f1fa3aabd..321e6f3307d 100644 --- a/app/views/users/phone_setup/index.html.erb +++ b/app/views/users/phone_setup/index.html.erb @@ -37,7 +37,7 @@ <%= render CaptchaSubmitButtonComponent.new( form: f, - action: PhoneRecaptchaValidator::RECAPTCHA_ACTION, + action: PhoneRecaptchaForm::RECAPTCHA_ACTION, ).with_content(t('forms.buttons.send_one_time_code')) %> <% end %> diff --git a/knapsack_rspec_report.json b/knapsack_rspec_report.json index 6d0cc329492..70bf552a2c9 100644 --- a/knapsack_rspec_report.json +++ b/knapsack_rspec_report.json @@ -783,7 +783,7 @@ "spec/services/personal_key_generator_spec.rb": 0.320518645, "spec/services/phone_formatter_spec.rb": 0.030768014, "spec/services/phone_number_capabilities_spec.rb": 0.104795072, - "spec/services/phone_recaptcha_validator_spec.rb": 0.065012571, + "spec/services/phone_recaptcha_form_spec.rb": 0.065012571, "spec/services/pii/attributes_spec.rb": 0.026663207, "spec/services/pii/cacher_spec.rb": 0.328829647, "spec/services/pii/fingerprinter_spec.rb": 0.026187948, @@ -840,7 +840,7 @@ "spec/services/reactivate_account_session_spec.rb": 0.486191592, "spec/services/recaptcha_enterprise_validator_spec.rb": 0.184840574, "spec/services/recaptcha_mock_validator_spec.rb": 0.031384338, - "spec/services/recaptcha_validator_spec.rb": 0.207950607, + "spec/services/recaptcha_form_spec.rb": 0.207950607, "spec/services/redis_rate_limiter_spec.rb": 0.045844488, "spec/services/remember_device_cookie_spec.rb": 0.144756036, "spec/services/reporting/account_deletion_rate_report_spec.rb": 0.287856172, diff --git a/spec/forms/new_phone_form_spec.rb b/spec/forms/new_phone_form_spec.rb index e6fdbde9596..a27a231bd74 100644 --- a/spec/forms/new_phone_form_spec.rb +++ b/spec/forms/new_phone_form_spec.rb @@ -374,8 +374,8 @@ end context 'with recaptcha enabled' do - let(:errors) { ActiveModel::Errors.new(RecaptchaValidator.new) } - let(:validator) { PhoneRecaptchaValidator.new(parsed_phone: nil) } + let(:errors) { ActiveModel::Errors.new(RecaptchaForm.new) } + let(:recaptcha_form) { PhoneRecaptchaForm.new(parsed_phone: nil) } let(:recaptcha_token) { 'token' } let(:phone) { '3065550100' } let(:international_code) { 'CA' } @@ -385,9 +385,9 @@ before do allow(FeatureManagement).to receive(:phone_recaptcha_enabled?).and_return(true) - allow(validator).to receive(:submit).with(recaptcha_token) - allow(validator).to receive(:errors).and_return(errors) - allow(form).to receive(:recaptcha_validator).and_return(validator) + allow(recaptcha_form).to receive(:submit).with(recaptcha_token) + allow(recaptcha_form).to receive(:errors).and_return(errors) + allow(form).to receive(:recaptcha_form).and_return(recaptcha_form) end context 'with valid recaptcha result' do @@ -397,9 +397,9 @@ end context 'with recaptcha enterprise' do - let(:validator) do - PhoneRecaptchaValidator.new( - validator_class: RecaptchaEnterpriseValidator, + let(:recaptcha_form) do + PhoneRecaptchaForm.new( + form_class: RecaptchaEnterpriseForm, parsed_phone: nil, ) end diff --git a/spec/services/phone_recaptcha_validator_spec.rb b/spec/forms/phone_recaptcha_form_spec.rb similarity index 61% rename from spec/services/phone_recaptcha_validator_spec.rb rename to spec/forms/phone_recaptcha_form_spec.rb index 01b34ef3b2b..6ab1d72f91b 100644 --- a/spec/services/phone_recaptcha_validator_spec.rb +++ b/spec/forms/phone_recaptcha_form_spec.rb @@ -1,11 +1,11 @@ require 'rails_helper' -RSpec.describe PhoneRecaptchaValidator do +RSpec.describe PhoneRecaptchaForm do let(:country_score_overrides_config) { {} } let(:score_threshold_config) { 0.2 } let(:parsed_phone) { Phonelib.parse('+15135551234') } let(:analytics) { FakeAnalytics.new } - subject(:validator) { described_class.new(parsed_phone:, analytics:) } + subject(:form) { described_class.new(parsed_phone:, analytics:) } before do allow(IdentityConfig.store).to receive(:phone_recaptcha_country_score_overrides). and_return(country_score_overrides_config) @@ -13,12 +13,12 @@ and_return(score_threshold_config) end - it 'passes instance variables to validator' do - recaptcha_validator = instance_double( - RecaptchaValidator, + it 'passes instance variables to form' do + recaptcha_form = instance_double( + RecaptchaForm, submit: FormResponse.new(success: true), ) - expect(RecaptchaValidator).to receive(:new). + expect(RecaptchaForm).to receive(:new). with( score_threshold: score_threshold_config, analytics:, @@ -27,52 +27,52 @@ phone_country_code: parsed_phone.country, }, ). - and_return(recaptcha_validator) + and_return(recaptcha_form) - validator.submit('token') + form.submit('token') end - context 'with custom recaptcha validator class' do - subject(:validator) do + context 'with custom recaptcha form class' do + subject(:form) do described_class.new( parsed_phone:, analytics:, - validator_class: RecaptchaMockValidator, + form_class: RecaptchaMockForm, ) end - it 'delegates to validator instance of the given class' do - recaptcha_validator = instance_double( - RecaptchaValidator, + it 'delegates to form instance of the given class' do + recaptcha_form = instance_double( + RecaptchaForm, submit: FormResponse.new(success: true), ) - expect(RecaptchaMockValidator).to receive(:new).and_return(recaptcha_validator) - expect(recaptcha_validator).to receive(:submit) + expect(RecaptchaMockForm).to receive(:new).and_return(recaptcha_form) + expect(recaptcha_form).to receive(:submit) - validator.submit('token') + form.submit('token') end end describe '#submit' do - it 'is delegated to recaptcha validator' do - recaptcha_validator = instance_double( - RecaptchaValidator, + it 'is delegated to recaptcha form' do + recaptcha_form = instance_double( + RecaptchaForm, submit: FormResponse.new(success: true), ) - expect(validator).to receive(:validator).and_return(recaptcha_validator) - expect(recaptcha_validator).to receive(:submit) + expect(form).to receive(:form).and_return(recaptcha_form) + expect(recaptcha_form).to receive(:submit) - validator.submit('token') + form.submit('token') end end describe '#errors' do - it 'is delegated to recaptcha validator' do - recaptcha_validator = instance_double(RecaptchaValidator, errors: ActiveModel::Errors.new({})) - expect(validator).to receive(:validator).and_return(recaptcha_validator) - expect(recaptcha_validator).to receive(:errors) + it 'is delegated to recaptcha form' do + recaptcha_form = instance_double(RecaptchaForm, errors: ActiveModel::Errors.new({})) + expect(form).to receive(:form).and_return(recaptcha_form) + expect(recaptcha_form).to receive(:errors) - validator.errors + form.errors end end @@ -85,7 +85,7 @@ end describe '#score_threshold' do - subject(:score_threshold) { validator.score_threshold } + subject(:score_threshold) { form.score_threshold } context 'without country override' do it 'returns default score threshold configuration value' do diff --git a/spec/services/recaptcha_enterprise_validator_spec.rb b/spec/forms/recaptcha_enterprise_form_spec.rb similarity index 93% rename from spec/services/recaptcha_enterprise_validator_spec.rb rename to spec/forms/recaptcha_enterprise_form_spec.rb index 2bcd247e5b6..0d134fa2c66 100644 --- a/spec/services/recaptcha_enterprise_validator_spec.rb +++ b/spec/forms/recaptcha_enterprise_form_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.describe RecaptchaEnterpriseValidator do +RSpec.describe RecaptchaEnterpriseForm do let(:score_threshold) { 0.2 } let(:analytics) { FakeAnalytics.new } let(:extra_analytics_properties) { {} } @@ -17,7 +17,7 @@ ) end - subject(:validator) do + subject(:form) do described_class.new( recaptcha_action: action, score_threshold:, @@ -36,7 +36,7 @@ end describe '#exempt?' do - subject(:exempt) { validator.exempt? } + subject(:exempt) { form.exempt? } context 'with initialized score threshold of 0' do let(:score_threshold) { 0.0 } @@ -53,11 +53,11 @@ describe '#submit' do let(:token) { nil } - subject(:response) { validator.submit(token) } + subject(:response) { form.submit(token) } context 'with exemption' do before do - allow(validator).to receive(:exempt?).and_return(true) + allow(form).to receive(:exempt?).and_return(true) end it 'is successful' do @@ -139,7 +139,7 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, - validator_class: 'RecaptchaEnterpriseValidator', + form_class: 'RecaptchaEnterpriseForm', ) end end @@ -174,7 +174,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - validator_class: 'RecaptchaEnterpriseValidator', + form_class: 'RecaptchaEnterpriseForm', ) end end @@ -197,7 +197,7 @@ 'reCAPTCHA verify result received', evaluated_as_valid: true, score_threshold: score_threshold, - validator_class: 'RecaptchaEnterpriseValidator', + form_class: 'RecaptchaEnterpriseForm', exception_class: 'Faraday::ConnectionFailed', ) end @@ -239,7 +239,7 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, - validator_class: 'RecaptchaEnterpriseValidator', + form_class: 'RecaptchaEnterpriseForm', ) end end @@ -279,7 +279,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - validator_class: 'RecaptchaEnterpriseValidator', + form_class: 'RecaptchaEnterpriseForm', ) end @@ -320,7 +320,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - validator_class: 'RecaptchaEnterpriseValidator', + form_class: 'RecaptchaEnterpriseForm', extra: true, ) end diff --git a/spec/services/recaptcha_validator_spec.rb b/spec/forms/recaptcha_form_spec.rb similarity index 90% rename from spec/services/recaptcha_validator_spec.rb rename to spec/forms/recaptcha_form_spec.rb index 2b433756c00..7b5d1245247 100644 --- a/spec/services/recaptcha_validator_spec.rb +++ b/spec/forms/recaptcha_form_spec.rb @@ -1,13 +1,13 @@ require 'rails_helper' -RSpec.describe RecaptchaValidator do +RSpec.describe RecaptchaForm do let(:score_threshold) { 0.2 } let(:analytics) { FakeAnalytics.new } let(:extra_analytics_properties) { {} } let(:recaptcha_secret_key) { 'recaptcha_secret_key' } - subject(:validator) do - RecaptchaValidator.new(score_threshold:, analytics:, extra_analytics_properties:) + subject(:form) do + RecaptchaForm.new(score_threshold:, analytics:, extra_analytics_properties:) end before do @@ -16,7 +16,7 @@ end describe '#exempt?' do - subject(:exempt) { validator.exempt? } + subject(:exempt) { form.exempt? } context 'with initialized score threshold of 0' do let(:score_threshold) { 0.0 } @@ -33,11 +33,11 @@ describe '#submit' do let(:token) { nil } - subject(:response) { validator.submit(token) } + subject(:response) { form.submit(token) } context 'with exemption' do before do - allow(validator).to receive(:exempt?).and_return(true) + allow(form).to receive(:exempt?).and_return(true) end it 'is successful' do @@ -115,7 +115,7 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, - validator_class: 'RecaptchaValidator', + form_class: 'RecaptchaForm', ) end @@ -145,7 +145,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - validator_class: 'RecaptchaValidator', + form_class: 'RecaptchaForm', ) end end @@ -175,7 +175,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - validator_class: 'RecaptchaValidator', + form_class: 'RecaptchaForm', ) end end @@ -186,7 +186,7 @@ let(:token) { 'token' } before do - stub_request(:post, RecaptchaValidator::VERIFICATION_ENDPOINT).to_timeout + stub_request(:post, RecaptchaForm::VERIFICATION_ENDPOINT).to_timeout end it 'is successful' do @@ -200,7 +200,7 @@ 'reCAPTCHA verify result received', evaluated_as_valid: true, score_threshold: score_threshold, - validator_class: 'RecaptchaValidator', + form_class: 'RecaptchaForm', exception_class: 'Faraday::ConnectionFailed', ) end @@ -234,7 +234,7 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, - validator_class: 'RecaptchaValidator', + form_class: 'RecaptchaForm', ) end end @@ -266,7 +266,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - validator_class: 'RecaptchaValidator', + form_class: 'RecaptchaForm', ) end @@ -286,7 +286,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - validator_class: 'RecaptchaValidator', + form_class: 'RecaptchaForm', extra: true, ) end @@ -303,7 +303,7 @@ end def stub_recaptcha_response(body:, secret: recaptcha_secret_key, token: nil) - stub_request(:post, RecaptchaValidator::VERIFICATION_ENDPOINT). + stub_request(:post, RecaptchaForm::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 diff --git a/spec/services/recaptcha_mock_validator_spec.rb b/spec/forms/recaptcha_mock_form_spec.rb similarity index 85% rename from spec/services/recaptcha_mock_validator_spec.rb rename to spec/forms/recaptcha_mock_form_spec.rb index 7e81436972e..4fc249809f7 100644 --- a/spec/services/recaptcha_mock_validator_spec.rb +++ b/spec/forms/recaptcha_mock_form_spec.rb @@ -1,11 +1,11 @@ require 'rails_helper' -RSpec.describe RecaptchaMockValidator do +RSpec.describe RecaptchaMockForm do let(:score_threshold) { 0.2 } let(:analytics) { FakeAnalytics.new } let(:score) { nil } - subject(:validator) do - RecaptchaMockValidator.new(score_threshold:, analytics:, score:) + subject(:form) do + RecaptchaMockForm.new(score_threshold:, analytics:, score:) end around do |example| @@ -14,7 +14,7 @@ describe '#submit' do let(:token) { 'token' } - subject(:response) { validator.submit(token) } + subject(:response) { form.submit(token) } context 'with failing score from validation service' do let(:score) { score_threshold - 0.1 } @@ -39,7 +39,7 @@ }, evaluated_as_valid: false, score_threshold: score_threshold, - validator_class: 'RecaptchaMockValidator', + form_class: 'RecaptchaMockForm', ) end end @@ -65,7 +65,7 @@ }, evaluated_as_valid: true, score_threshold: score_threshold, - validator_class: 'RecaptchaMockValidator', + form_class: 'RecaptchaMockForm', ) end