LG-1995 Fix legacy idv throttling (#3302)

**Why**: After legacy or doc auth resolution or phone verification locks up it does not unlock **How**: Replace the legacy throttling mechanism on the user table with the new generic Throttler which does not require an explicit reset and therefore can't lock up.
18F · Sep 26, 2019 · 3848929 · 3848929
1 parent 51e8cd5
commit 3848929
Show file tree

Hide file tree

Showing 23 changed files with 149 additions and 233 deletions.
diff --git a/app/controllers/concerns/idv_session.rb b/app/controllers/concerns/idv_session.rb
@@ -6,12 +6,9 @@ def confirm_idv_session_started
   end
 
   def confirm_idv_attempts_allowed
-    if idv_attempter.exceeded?
-      analytics.track_event(Analytics::IDV_MAX_ATTEMPTS_EXCEEDED, request_path: request.path)
-      redirect_to failure_url(:fail)
-    elsif idv_attempter.reset_attempts?
-      idv_attempter.reset
-    end
+    return unless idv_attempter_throttled?
+    analytics.track_event(Analytics::IDV_MAX_ATTEMPTS_EXCEEDED, request_path: request.path)
+    redirect_to failure_url(:fail)
   end
 
   def confirm_idv_needed
@@ -31,7 +28,7 @@ def idv_session
     )
   end
 
-  def idv_attempter
-    @_idv_attempter ||= Idv::Attempter.new(current_user)
+  def idv_attempter_throttled?
+    Throttler::IsThrottled.call(current_user.id, :idv_resolution)
   end
 end
diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb
@@ -11,12 +11,16 @@ module IdvStepConcern
 
   private
 
+  def idv_max_attempts
+    Throttle::THROTTLE_CONFIG[:idv_resolution][:max_attempts]
+  end
+
   def remaining_step_attempts
-    Idv::Attempter.idv_max_attempts - idv_session.step_attempts[step_name]
+    idv_max_attempts - idv_session.step_attempts[step_name]
   end
 
   def step_attempts_exceeded?
-    idv_session.step_attempts[step_name] >= Idv::Attempter.idv_max_attempts
+    idv_session.step_attempts[step_name] >= idv_max_attempts
   end
 
   def confirm_step_allowed

diff --git a/app/controllers/idv/sessions_controller.rb b/app/controllers/idv/sessions_controller.rb
@@ -76,7 +76,7 @@ def step_name
     end
 
     def remaining_step_attempts
-      Idv::Attempter.idv_max_attempts - current_user.idv_attempts
+      Throttler::RemainingCount.call(current_user.id, :idv_resolution)
     end
 
     def set_idv_form

diff --git a/app/controllers/idv/usps_controller.rb b/app/controllers/idv/usps_controller.rb
@@ -146,8 +146,20 @@ def form_response(result, success)
       FormResponse.new(success: success, errors: result[:errors])
     end
 
+    def idv_throttle_params
+      [idv_session.current_user.id, :idv_resolution]
+    end
+
+    def idv_attempter_increment
+      Throttler::Increment.call(*idv_throttle_params)
+    end
+
+    def idv_attempter_throttled?
+      Throttler::IsThrottled.call(*idv_throttle_params)
+    end
+
     def throttle_failure
-      attempter.increment
+      idv_attempter_increment
       flash_error
     end
 
@@ -157,15 +169,11 @@ def flash_error
     end
 
     def max_attempts_reached
-      flash_error if attempter.exceeded?
+      flash_error if idv_attempter_throttled?
     end
 
     def error_message
-      I18n.t('idv.failure.sessions.' + (attempter.exceeded? ? 'fail' : 'heading'))
-    end
-
-    def attempter
-      @attempter ||= Idv::Attempter.new(idv_session.current_user)
+      I18n.t('idv.failure.sessions.' + (idv_attempter_throttled? ? 'fail' : 'heading'))
     end
 
     def send_reminder

diff --git a/app/controllers/idv_controller.rb b/app/controllers/idv_controller.rb
@@ -9,7 +9,7 @@ class IdvController < ApplicationController
   def index
     if active_profile?
       redirect_to idv_activated_url
-    elsif idv_attempter.exceeded?
+    elsif idv_attempter_throttled?
       redirect_to idv_fail_url
     elsif doc_auth_enabled_and_exclusive?
       redirect_to idv_doc_auth_url
@@ -21,12 +21,11 @@ def index
 
   def activated
     redirect_to idv_url unless active_profile?
-    idv_attempter.reset
     idv_session.clear
   end
 
   def fail
-    redirect_to idv_url and return unless idv_attempter.exceeded?
+    redirect_to idv_url and return unless idv_attempter_throttled?
     presenter = Idv::IdvFailurePresenter.new(
       view_context: view_context,
     )

diff --git a/app/models/throttle.rb b/app/models/throttle.rb
@@ -7,6 +7,7 @@ class Throttle < ApplicationRecord
     reg_unconfirmed_email: 2,
     reg_confirmed_email: 3,
     reset_password_email: 4,
+    idv_resolution: 5,
   }
 
   THROTTLE_CONFIG = {
@@ -26,12 +27,22 @@ class Throttle < ApplicationRecord
       max_attempts: (Figaro.env.reset_password_email_max_attempts || 20).to_i,
       attempt_window: (Figaro.env.reset_password_email_window_in_minutes || 60).to_i,
     },
+    idv_resolution: {
+      max_attempts: (Figaro.env.idv_max_attempts || 3).to_i,
+      attempt_window: (Figaro.env.idv_attempt_window_in_hours || 24).to_i * 60,
+    },
   }.freeze
 
   def throttled?
     !expired? && maxed?
   end
 
+  def remaining_count
+    return 0 if throttled?
+    max_attempts, _attempt_window_in_minutes = Throttle.config_values(throttle_type)
+    max_attempts - attempts
+  end
+
   def expired?
     return true if attempted_at.blank?
     _max_attempts, attempt_window_in_minutes = Throttle.config_values(throttle_type)

diff --git a/app/services/idv/attempter.rb b/app/services/idv/attempter.rb
diff --git a/app/services/idv/phone_step.rb b/app/services/idv/phone_step.rb
@@ -17,7 +17,7 @@ def submit(step_params)
     end
 
     def failure_reason
-      return :fail if idv_session.step_attempts[:phone] >= Idv::Attempter.idv_max_attempts
+      return :fail if idv_session.step_attempts[:phone] >= idv_max_attempts
       return :timeout if idv_result[:timed_out]
       return :jobfail if idv_result[:exception].present?
       return :warning if idv_result[:success] != true
@@ -27,6 +27,10 @@ def failure_reason
 
     attr_accessor :idv_session, :step_params, :idv_result
 
+    def idv_max_attempts
+      Throttle::THROTTLE_CONFIG[:idv_resolution][:max_attempts]
+    end
+
     def proof_address
       self.idv_result = Idv::Agent.new(applicant).proof(:address)
       add_proofing_cost
@@ -71,11 +75,8 @@ def update_idv_session
       idv_session.applicant = applicant
       idv_session.vendor_phone_confirmation = true
       idv_session.user_phone_confirmation = phone_matches_user_phone?
-      Db::ProofingComponent::Add.call(user_id, :address_check, 'lexis_nexis_address')
-    end
-
-    def user_id
-      idv_session.current_user.id
+      Db::ProofingComponent::Add.call(idv_session.current_user.id, :address_check,
+                                      'lexis_nexis_address')
     end
 
     def phone_matches_user_phone?

diff --git a/app/services/idv/profile_step.rb b/app/services/idv/profile_step.rb
@@ -7,7 +7,7 @@ def initialize(idv_session:)
     def submit(step_params)
       consume_step_params(step_params)
       self.idv_result = Idv::Agent.new(applicant).proof(:resolution, :state_id)
-      increment_attempts_count unless failed_due_to_timeout_or_exception?
+      Throttler::Increment.call(*idv_throttle_params) unless failed_due_to_timeout_or_exception?
       update_idv_session if success?
       FormResponse.new(
         success: success?, errors: idv_result[:errors],
@@ -17,7 +17,7 @@ def submit(step_params)
 
     def failure_reason
       return if success?
-      return :fail if attempter.exceeded?
+      return :fail if throttled?
       return :timeout if idv_result[:timed_out]
       return :jobfail if idv_result[:exception].present?
       :warning
@@ -27,6 +27,10 @@ def failure_reason
 
     attr_accessor :idv_session, :step_params, :idv_result
 
+    def throttled?
+      Throttler::IsThrottled.call(*idv_throttle_params)
+    end
+
     def consume_step_params(params)
       self.step_params = params.merge(state_id_jurisdiction: params[:state])
     end
@@ -35,8 +39,8 @@ def applicant
       step_params.merge(uuid: idv_session.current_user.uuid)
     end
 
-    def increment_attempts_count
-      attempter.increment
+    def idv_throttle_params
+      [idv_session.current_user.id, :idv_resolution]
     end
 
     def success?
@@ -56,10 +60,6 @@ def failed_due_to_timeout_or_exception?
       idv_result[:timed_out] || idv_result[:exception]
     end
 
-    def attempter
-      @attempter ||= Idv::Attempter.new(idv_session.current_user)
-    end
-
     def update_idv_session
       idv_session.applicant = applicant
       idv_session.profile_confirmation = true
@@ -68,7 +68,7 @@ def update_idv_session
 
     def extra_analytics_attributes
       {
-        idv_attempts_exceeded: attempter.exceeded?,
+        idv_attempts_exceeded: throttled?,
         vendor: idv_result.except(:errors, :success),
         ssn_is_unique: ssn_is_unique?,
       }

diff --git a/app/services/idv/steps/doc_auth_base_step.rb b/app/services/idv/steps/doc_auth_base_step.rb
@@ -33,13 +33,21 @@ def simulate?
         Figaro.env.acuant_simulator == 'true'
       end
 
-      def attempter
-        @attempter ||= Idv::Attempter.new(current_user)
+      def idv_throttle_params
+        [current_user.id, :idv_resolution]
+      end
+
+      def attempter_increment
+        Throttler::Increment.call(*idv_throttle_params)
+      end
+
+      def attempter_throttled?
+        Throttler::IsThrottled.call(*idv_throttle_params)
       end
 
       def idv_failure(result)
-        attempter.increment
-        type = attempter.exceeded? ? :fail : :warning
+        attempter_increment
+        type = attempter_throttled? ? :fail : :warning
         redirect_to idv_session_failure_url(reason: type)
         result
       end

diff --git a/app/services/throttler/is_throttled.rb b/app/services/throttler/is_throttled.rb
@@ -2,7 +2,7 @@ module Throttler
   class IsThrottled
     def self.call(user_id, throttle_type)
       throttle = FindOrCreate.call(user_id, throttle_type)
-      throttle.throttled? ? throttle : nil
+      throttle.throttled? ? throttle : false
     end
   end
 end
diff --git a/app/services/throttler/remaining_count.rb b/app/services/throttler/remaining_count.rb
@@ -0,0 +1,8 @@
+module Throttler
+  class RemainingCount
+    def self.call(user_id, throttle_type)
+      throttle = FindOrCreate.call(user_id, throttle_type)
+      throttle.remaining_count
+    end
+  end
+end
diff --git a/spec/controllers/concerns/idv_step_concern_spec.rb b/spec/controllers/concerns/idv_step_concern_spec.rb
@@ -34,8 +34,7 @@ def failure_url(_arg)
 
     context 'user has exceeded IdV max attempts in a single session' do
       before do
-        user.idv_attempts = 3
-        user.idv_attempted_at = Time.zone.now
+        create_maxed_throttle
         allow(subject).to receive(:confirm_idv_session_started).and_return(true)
         get :show
       end
@@ -48,8 +47,7 @@ def failure_url(_arg)
     context 'user has exceeded IdV max attempts in a single period' do
       before do
         allow(subject).to receive(:confirm_idv_session_started).and_return(true)
-        user.idv_attempts = 3
-        user.idv_attempted_at = Time.zone.now
+        create_maxed_throttle
         get :show
       end
 
@@ -61,8 +59,7 @@ def failure_url(_arg)
     context 'user attempts IdV after window has passed' do
       before do
         allow(subject).to receive(:confirm_idv_session_started).and_return(true)
-        user.idv_attempts = 3
-        user.idv_attempted_at = Time.zone.now - 25.hours
+        create_maxed_throttle(Time.zone.now - 25.hours)
         get :show
       end
 
@@ -165,4 +162,13 @@ def show
       end
     end
   end
+
+  def create_maxed_throttle(attempted_at = Time.zone.now)
+    Throttle.create(
+      throttle_type: 5,
+      user_id: user.id,
+      attempts: 3,
+      attempted_at: attempted_at,
+    )
+  end
 end