LG-9118: Throttle phone verification on submit

app/controllers/idv/phone_controller.rb

@@ -14,9 +14,14 @@ class PhoneController < ApplicationController
     def new
       analytics.idv_phone_use_different(step: params[:step]) if params[:step]
 
+      async_state = step.async_state
+
+      # It's possible that create redirected here after a success and left the
+      # throttle maxed out. Check for success before checking throttle.
+      return async_state_done(async_state) if async_state.done?
+
       redirect_to failure_url(:fail) and return if throttle.throttled?
 
-      async_state = step.async_state
       if async_state.none?
         Funnel::DocAuth::RegisterStep.new(current_user.id, current_sp&.issuer).
           call(:verify_phone, :view, true)
@@ -29,8 +34,6 @@ def new
         analytics.proofing_address_result_missing
         flash.now[:error] = I18n.t('idv.failure.timeout')
         render :new, locals: { gpo_letter_available: gpo_letter_available }
-      elsif async_state.done?
-        async_state_done(async_state)
       end
     end
 
@@ -57,13 +60,6 @@ def throttle
       @throttle ||= Throttle.new(user: current_user, throttle_type: :proof_address)
     end
 
-    def max_attempts_reached
-      analytics.throttler_rate_limit_triggered(
-        throttle_type: :proof_address,
-        step_name: step_name,
-      )
-    end
-
     def redirect_to_next_step
       if phone_confirmation_required?
         if VendorStatus.new.all_phone_vendor_outage?
@@ -113,7 +109,6 @@ def handle_send_phone_confirmation_otp_failure(result)
     end
 
     def handle_proofing_failure
-      max_attempts_reached if step.failure_reason == :fail
       redirect_to failure_url(step.failure_reason)
     end
 
@@ -125,6 +120,7 @@ def step
       @step ||= Idv::PhoneStep.new(
         idv_session: idv_session,
         trace_id: amzn_trace_id,
+        analytics: analytics,
         attempts_tracker: irs_attempts_api_tracker,
       )
     end

app/services/idv/phone_step.rb

@@ -1,19 +1,24 @@
 module Idv
   class PhoneStep
-    def initialize(idv_session:, trace_id:, attempts_tracker:)
+    def initialize(idv_session:, trace_id:, analytics:, attempts_tracker:)
       self.idv_session = idv_session
       @trace_id = trace_id
+      @analytics = analytics
       @attempts_tracker = attempts_tracker
     end
 
     def submit(step_params)
+      return throttled_result if throttle.throttled?
+      throttle.increment!
+
       self.step_params = step_params
       idv_session.previous_phone_step_params = step_params.slice(:phone, :otp_delivery_preference)
       proof_address
     end
 
     def failure_reason
       return :fail if throttle.throttled?
+      return :no_idv_result if idv_result.nil?
       return :timeout if idv_result[:timed_out]
       return :jobfail if idv_result[:exception].present?
       return :warning if idv_result[:success] != true
@@ -34,8 +39,6 @@ def async_state
     def async_state_done(async_state)
       @idv_result = async_state.result
 
-      throttle.increment! unless failed_due_to_timeout_or_exception?
-      @attempts_tracker.idv_phone_otp_sent_rate_limited if throttle.throttled?
       success = idv_result[:success]
       handle_successful_proofing_attempt if success
 
@@ -106,6 +109,12 @@ def throttle
       @throttle ||= Throttle.new(user: idv_session.current_user, throttle_type: :proof_address)
     end
 
+    def throttled_result
+      @attempts_tracker.idv_phone_otp_sent_rate_limited
+      @analytics.throttler_rate_limit_triggered(throttle_type: :proof_address, step_name: :phone)
+      FormResponse.new(success: false)
+    end
+
     def failed_due_to_timeout_or_exception?
       idv_result[:timed_out] || idv_result[:exception]
     end

spec/controllers/idv/phone_controller_spec.rb

@@ -479,28 +479,25 @@
 
       context 'when the user is throttled by submission' do
         before do
+          stub_analytics
+
           user = create(:user, with: { phone: '+1 (415) 555-0130' })
           stub_verify_steps_one_and_two(user)
+
           throttle = Throttle.new(throttle_type: :proof_address, user: user)
-          (max_attempts - 1).times do
-            throttle.increment!
-          end
+          throttle.increment_to_throttled!
+
+          put :create, params: { idv_phone_form: { phone: bad_phone } }
         end
 
         it 'tracks throttled event' do
-          stub_analytics
-          allow(@analytics).to receive(:track_event)
-
-          expect(@analytics).to receive(:track_event).with(
+          expect(@analytics).to have_logged_event(
             'Throttler Rate Limit Triggered',
             {
               throttle_type: :proof_address,
               step_name: :phone,
             },
           )
-
-          put :create, params: { idv_phone_form: { phone: bad_phone } }
-          get :new
         end
       end
     end

spec/services/idv/phone_step_spec.rb

@@ -40,11 +40,13 @@
   end
   let(:trace_id) { SecureRandom.uuid }
   let(:attempts_tracker) { @irs_attempts_api_tracker }
+  let(:analytics) { FakeAnalytics.new }
 
   subject do
     described_class.new(
       idv_session: idv_session,
       trace_id: trace_id,
+      analytics: analytics,
       attempts_tracker: attempts_tracker,
     )
   end
@@ -116,27 +118,26 @@
     it 'increments step attempts' do
       expect do
         subject.submit(phone: bad_phone)
-        expect(subject.async_state.done?).to eq true
-        subject.async_state_done(subject.async_state)
       end.to(change { throttle.fetch_state!.attempts }.by(1))
     end
 
-    it 'does not increment step attempts when the vendor request times out' do
-      expect { subject.submit(phone: timeout_phone) }.to_not change { throttle.attempts }
+    it 'increments step attempts when the vendor request times out' do
+      expect do
+        subject.submit(phone: timeout_phone)
+      end.to(change { throttle.fetch_state!.attempts }.by(1))
     end
 
     it 'does not increment step attempts when the vendor raises an exception' do
-      expect { subject.submit(phone: fail_phone) }.to_not change { throttle.attempts }
+      expect do
+        subject.submit(phone: fail_phone)
+      end.to(change { throttle.fetch_state!.attempts }.by(1))
     end
 
     it 'logs a throttled attempts_tracker event' do
       throttle.increment_to_throttled!
 
-      subject.submit(phone: bad_phone)
       expect(@irs_attempts_api_tracker).to receive(:idv_phone_otp_sent_rate_limited)
-      subject.async_state_done(subject.async_state)
-
-      throttle.reset!
+      subject.submit(phone: bad_phone)
     end
 
     it 'marks the phone as unconfirmed if it matches 2FA phone' do
@@ -200,9 +201,6 @@
         Throttle.new(throttle_type: :proof_address, user: user).increment_to_throttled!
 
         subject.submit(phone: bad_phone)
-        expect(subject.async_state.done?).to eq true
-        subject.async_state_done(subject.async_state)
-
         expect(subject.failure_reason).to eq(:fail)
       end
     end