Refactor reCAPTCHA validator as form model

app/forms/new_phone_form.rb

@@ -130,12 +130,9 @@ def validate_not_premium_rate
   end
 
   def validate_recaptcha_token
-    return if !validate_recaptcha_token? || recaptcha_validator.valid?(recaptcha_token)
-    errors.add(
-      :recaptcha_token,
-      I18n.t('errors.messages.invalid_recaptcha_token'),
-      type: :invalid_recaptcha_token,
-    )
+    return if !validate_recaptcha_token?
+    recaptcha_validator.submit(recaptcha_token)
+    errors.merge!(recaptcha_validator)
   end
 
   def recaptcha_validator

app/services/phone_recaptcha_validator.rb

@@ -5,7 +5,7 @@ class PhoneRecaptchaValidator
 
   attr_reader :parsed_phone, :validator_class, :validator_args
 
-  delegate :valid?, :exempt?, to: :validator
+  delegate :submit, :errors, to: :validator
 
   def initialize(parsed_phone:, validator_class: RecaptchaValidator, **validator_args)
     @parsed_phone = parsed_phone

app/services/recaptcha_enterprise_validator.rb

@@ -16,7 +16,7 @@ def assessment_url
     )
   end
 
-  def recaptcha_result(recaptcha_token)
+  def recaptcha_result
     response = faraday.post(
       assessment_url,
       {

app/services/recaptcha_mock_validator.rb

@@ -10,7 +10,7 @@ def initialize(score:, **kwargs)
 
   private
 
-  def recaptcha_result(_recaptcha_token)
+  def recaptcha_result
     RecaptchaResult.new(success: true, score:)
   end
 end

app/services/recaptcha_validator.rb

@@ -1,14 +1,21 @@
 # frozen_string_literal: true
 
 class RecaptchaValidator
+  include ActiveModel::Model
+  include ActionView::Helpers::TranslationHelper
+
   VERIFICATION_ENDPOINT = 'https://www.google.com/recaptcha/api/siteverify'
   RESULT_ERRORS = ['missing-input-secret', 'invalid-input-secret'].freeze
 
   attr_reader :recaptcha_action,
+              :recaptcha_token,
               :score_threshold,
               :analytics,
               :extra_analytics_properties
 
+  validate :validate_token_exists
+  validate :validate_recaptcha_result
+
   RecaptchaResult = Struct.new(:success, :score, :errors, :reasons, keyword_init: true) do
     alias_method :success?, :success
 
@@ -33,20 +40,30 @@ def exempt?
     !score_threshold.positive?
   end
 
-  def valid?(recaptcha_token)
-    return true if exempt?
-    return false if recaptcha_token.blank?
-    result = recaptcha_result(recaptcha_token)
-    log_analytics(result:)
-    recaptcha_result_valid?(result)
+  def submit(recaptcha_token)
+    @recaptcha_token = recaptcha_token
+    @recaptcha_result = recaptcha_result if !exempt? && recaptcha_token.present?
+
+    log_analytics(result: @recaptcha_result) if @recaptcha_result
+    FormResponse.new(success: valid?, errors:, serialize_error_details_only: true)
   rescue Faraday::Error => error
     log_analytics(error:)
-    true
+    FormResponse.new(success: true, serialize_error_details_only: true)
   end
 
   private
 
-  def recaptcha_result(recaptcha_token)
+  def validate_token_exists
+    return if exempt? || recaptcha_token.present?
+    errors.add(:recaptcha_token, :blank, message: t('errors.messages.invalid_recaptcha_token'))
+  end
+
+  def validate_recaptcha_result
+    return if @recaptcha_result.blank? || recaptcha_result_valid?(@recaptcha_result)
+    errors.add(:recaptcha_token, :invalid, message: t('errors.messages.invalid_recaptcha_token'))
+  end
+
+  def recaptcha_result
     response = faraday.post(
       VERIFICATION_ENDPOINT,
       URI.encode_www_form(secret: recaptcha_secret_key, response: recaptcha_token),

spec/forms/new_phone_form_spec.rb

@@ -374,7 +374,7 @@
     end
 
     context 'with recaptcha enabled' do
-      let(:valid) { nil }
+      let(:errors) { ActiveModel::Errors.new(RecaptchaValidator.new) }
       let(:validator) { PhoneRecaptchaValidator.new(parsed_phone: nil) }
       let(:recaptcha_token) { 'token' }
       let(:phone) { '3065550100' }
@@ -385,13 +385,12 @@
 
       before do
         allow(FeatureManagement).to receive(:phone_recaptcha_enabled?).and_return(true)
-        allow(validator).to receive(:valid?).with(recaptcha_token).and_return(valid)
+        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)
       end
 
       context 'with valid recaptcha result' do
-        let(:valid) { true }
-
         it 'is valid' do
           expect(result.success?).to eq(true)
           expect(result.errors).to be_blank
@@ -417,7 +416,9 @@
       end
 
       context 'with invalid recaptcha result' do
-        let(:valid) { false }
+        before do
+          errors.add(:recaptcha_token, :fail, message: 'fail')
+        end
 
         it 'is invalid' do
           expect(result.success?).to eq(false)

spec/services/phone_recaptcha_validator_spec.rb

@@ -14,7 +14,10 @@
   end
 
   it 'passes instance variables to validator' do
-    recaptcha_validator = instance_double(RecaptchaValidator, valid?: true)
+    recaptcha_validator = instance_double(
+      RecaptchaValidator,
+      submit: FormResponse.new(success: true),
+    )
     expect(RecaptchaValidator).to receive(:new).
       with(
         score_threshold: score_threshold_config,
@@ -26,7 +29,7 @@
       ).
       and_return(recaptcha_validator)
 
-    validator.valid?('token')
+    validator.submit('token')
   end
 
   context 'with custom recaptcha validator class' do
@@ -39,31 +42,37 @@
     end
 
     it 'delegates to validator instance of the given class' do
-      recaptcha_validator = instance_double(RecaptchaMockValidator, valid?: true)
+      recaptcha_validator = instance_double(
+        RecaptchaValidator,
+        submit: FormResponse.new(success: true),
+      )
       expect(RecaptchaMockValidator).to receive(:new).and_return(recaptcha_validator)
-      expect(recaptcha_validator).to receive(:valid?)
+      expect(recaptcha_validator).to receive(:submit)
 
-      validator.valid?('token')
+      validator.submit('token')
     end
   end
 
-  describe '#valid?' do
+  describe '#submit' do
     it 'is delegated to recaptcha validator' do
-      recaptcha_validator = instance_double(RecaptchaValidator, valid?: true)
+      recaptcha_validator = instance_double(
+        RecaptchaValidator,
+        submit: FormResponse.new(success: true),
+      )
       expect(validator).to receive(:validator).and_return(recaptcha_validator)
-      expect(recaptcha_validator).to receive(:valid?)
+      expect(recaptcha_validator).to receive(:submit)
 
-      validator.valid?('token')
+      validator.submit('token')
     end
   end
 
-  describe '#exempt?' do
+  describe '#errors' do
     it 'is delegated to recaptcha validator' do
-      recaptcha_validator = instance_double(RecaptchaValidator, exempt?: true)
+      recaptcha_validator = instance_double(RecaptchaValidator, errors: ActiveModel::Errors.new({}))
       expect(validator).to receive(:validator).and_return(recaptcha_validator)
-      expect(recaptcha_validator).to receive(:exempt?)
+      expect(recaptcha_validator).to receive(:errors)
 
-      validator.exempt?
+      validator.errors
     end
   end
 

spec/services/recaptcha_enterprise_validator_spec.rb

@@ -51,19 +51,21 @@
     end
   end
 
-  describe '#valid?' do
+  describe '#submit' do
     let(:token) { nil }
-    subject(:valid) { validator.valid?(token) }
+    subject(:response) { validator.submit(token) }
 
     context 'with exemption' do
       before do
         allow(validator).to receive(:exempt?).and_return(true)
       end
 
-      it { expect(valid).to eq(true) }
+      it 'is successful' do
+        expect(response.to_h).to eq(success: true)
+      end
 
       it 'does not log analytics' do
-        valid
+        response
 
         expect(analytics).not_to have_logged_event('reCAPTCHA verify result received')
       end
@@ -72,10 +74,15 @@
     context 'with missing token' do
       let(:token) { nil }
 
-      it { expect(valid).to eq(false) }
+      it 'is unsuccessful with error for blank token' do
+        expect(response.to_h).to eq(
+          success: false,
+          error_details: { recaptcha_token: { blank: true } },
+        )
+      end
 
       it 'does not log analytics' do
-        valid
+        response
 
         expect(analytics).not_to have_logged_event('reCAPTCHA verify result received')
       end
@@ -84,10 +91,15 @@
     context 'with blank token' do
       let(:token) { '' }
 
-      it { expect(valid).to eq(false) }
+      it 'is unsuccessful with error for blank token' do
+        expect(response.to_h).to eq(
+          success: false,
+          error_details: { recaptcha_token: { blank: true } },
+        )
+      end
 
       it 'does not log analytics' do
-        valid
+        response
 
         expect(analytics).not_to have_logged_event('reCAPTCHA verify result received')
       end
@@ -107,10 +119,15 @@
         )
       end
 
-      it { expect(valid).to eq(false) }
+      it 'is unsuccessful with error for invalid token' do
+        expect(response.to_h).to eq(
+          success: false,
+          error_details: { recaptcha_token: { invalid: true } },
+        )
+      end
 
       it 'logs analytics of the body' do
-        valid
+        response
 
         expect(analytics).to have_logged_event(
           'reCAPTCHA verify result received',
@@ -140,10 +157,12 @@
         )
       end
 
-      it { expect(valid).to eq(true) }
+      it 'is successful' do
+        expect(response.to_h).to eq(success: true)
+      end
 
       it 'logs analytics of the body' do
-        valid
+        response
 
         expect(analytics).to have_logged_event(
           'reCAPTCHA verify result received',
@@ -167,10 +186,12 @@
         stub_request(:post, assessment_url).to_timeout
       end
 
-      it { expect(valid).to eq(true) }
+      it 'is successful' do
+        expect(response.to_h).to eq(success: true)
+      end
 
       it 'logs analytics of the body' do
-        valid
+        response
 
         expect(analytics).to have_logged_event(
           'reCAPTCHA verify result received',
@@ -198,10 +219,15 @@
         )
       end
 
-      it { expect(valid).to eq(false) }
+      it 'is unsuccessful with error for invalid token' do
+        expect(response.to_h).to eq(
+          success: false,
+          error_details: { recaptcha_token: { invalid: true } },
+        )
+      end
 
       it 'logs analytics of the body' do
-        valid
+        response
 
         expect(analytics).to have_logged_event(
           'reCAPTCHA verify result received',
@@ -222,8 +248,8 @@
       let(:token) { 'token' }
       let(:score) { score_threshold + 0.1 }
 
-      before do
-        stub_recaptcha_response(
+      around do |example|
+        stubbed_request = stub_recaptcha_response(
           body: {
             tokenProperties: { valid: true, action: },
             riskAnalysis: { score:, reasons: ['LOW_CONFIDENCE'] },
@@ -232,12 +258,16 @@
           action:,
           token:,
         )
+        example.run
+        expect(stubbed_request).to have_been_made.once
       end
 
-      it { expect(valid).to eq(true) }
+      it 'is successful' do
+        expect(response.to_h).to eq(success: true)
+      end
 
       it 'logs analytics of the body' do
-        valid
+        response
 
         expect(analytics).to have_logged_event(
           'reCAPTCHA verify result received',
@@ -266,14 +296,19 @@
           )
         end
 
-        it { expect(valid).to eq(false) }
+        it 'is unsuccessful with error for invalid token' do
+          expect(response.to_h).to eq(
+            success: false,
+            error_details: { recaptcha_token: { invalid: true } },
+          )
+        end
       end
 
       context 'with extra analytics properties', allowed_extra_analytics: [:extra] do
         let(:extra_analytics_properties) { { extra: true } }
 
         it 'logs analytics of the body' do
-          valid
+          response
 
           expect(analytics).to have_logged_event(
             'reCAPTCHA verify result received',
@@ -295,7 +330,7 @@
         let(:analytics) { nil }
 
         it 'validates gracefully without analytics logging' do
-          valid
+          response
         end
       end
     end