LG-12712 Use phone fingerprint as rate limit key for phone confirmation

app/controllers/users/phone_setup_controller.rb

@@ -36,7 +36,10 @@ def create
       if result.success?
         handle_create_success(@new_phone_form.phone)
       else
-        flash.now[:error] = result.first_error_message(:recaptcha_token)
+        flash.now[:error] = result.first_error_message(
+          :recaptcha_token,
+          :phone_fingerprint,
+        )
         render :index
       end
     end

app/forms/new_phone_form.rb

@@ -4,6 +4,7 @@ class NewPhoneForm
   include ActiveModel::Model
   include FormPhoneValidator
   include OtpDeliveryPreferenceValidator
+  include ActionView::Helpers::DateHelper
 
   BLOCKED_PHONE_TYPES = [
     :premium_rate,
@@ -17,6 +18,7 @@ class NewPhoneForm
   validate :validate_not_premium_rate
   validate :validate_recaptcha_token
   validate :validate_allowed_carrier
+  validate :validate_phone_submission_limit
 
   attr_reader :phone,
               :international_code,
@@ -177,4 +179,27 @@ def ingest_submitted_params(params)
   def confirmed_phone?
     false
   end
+
+  def validate_phone_submission_limit
+    fingerprint = Pii::Fingerprinter.fingerprint(Phonelib.parse(phone).e164.to_s)
+    submission_rate_limiter ||= RateLimiter.new(
+      target: fingerprint,
+      rate_limit_type: :phone_fingerprint_confirmation,
+    )
+    submission_rate_limiter.increment!
+    if submission_rate_limiter.maxed?
+      errors.add(
+        :phone_fingerprint,
+        I18n.t(
+          'errors.messages.phone_confirmation_limited',
+          timeout: distance_of_time_in_words(
+            Time.zone.now,
+            [submission_rate_limiter.expires_at, Time.zone.now].compact.max,
+            except: :seconds,
+          ),
+        ),
+        type: :locked_phone_fingerprint,
+      )
+    end
+  end
 end

app/services/form_response.rb

@@ -39,10 +39,16 @@ def merge(other)
     end
   end
 
-  def first_error_message(key = nil)
+  def first_error_message(*keys)
     return if errors.blank?
-    key ||= errors.keys.first
-    errors[key].first
+    if keys.blank?
+      errors.values.first.first
+    else
+      keys.each do |key|
+        return errors[key].first if errors.key?(key)
+      end
+      nil
+    end
   end
 
   def ==(other)

app/services/rate_limiter.rb

@@ -214,6 +214,11 @@ def self.load_rate_limit_config
         max_attempts: IdentityConfig.store.phone_confirmation_max_attempts,
         attempt_window: IdentityConfig.store.phone_confirmation_max_attempt_window_in_minutes,
       },
+      phone_fingerprint_confirmation: {
+        max_attempts: IdentityConfig.store.phone_submissions_per_fingerprint_limit,
+        attempt_window: IdentityConfig.store.
+          phone_submissions_per_fingerprint_max_attempts_window_in_minutes,
+      },
       phone_otp: {
         max_attempts: IdentityConfig.store.otp_delivery_blocklist_maxretry + 1,
         attempt_window: IdentityConfig.store.otp_delivery_blocklist_findtime,

config/application.yml.default

@@ -245,6 +245,8 @@ phone_recaptcha_country_score_overrides: '{"AS":0.0,"GU":0.0,"MP":0.0,"PR":0.0,"
 phone_setups_per_ip_limit: 25
 phone_setups_per_ip_period: 300
 phone_setups_per_ip_track_only_mode: false
+phone_submissions_per_fingerprint_limit: 20
+phone_submissions_per_fingerprint_max_attempts_window_in_minutes: 10
 pii_lock_timeout_in_minutes: 30
 pinpoint_sms_sender_id: 'aaa'
 pinpoint_sms_configs: '[]'

lib/identity_config.rb

@@ -299,6 +299,8 @@ def self.store
     config.add(:phone_setups_per_ip_limit, type: :integer)
     config.add(:phone_setups_per_ip_period, type: :integer)
     config.add(:phone_setups_per_ip_track_only_mode, type: :boolean)
+    config.add(:phone_submissions_per_fingerprint_limit, type: :integer)
+    config.add(:phone_submissions_per_fingerprint_max_attempts_window_in_minutes, type: :integer)
     config.add(:pii_lock_timeout_in_minutes, type: :integer)
     config.add(:pinpoint_sms_configs, type: :json)
     config.add(:pinpoint_sms_sender_id, type: :string, allow_nil: true)

spec/controllers/users/phone_setup_controller_spec.rb

@@ -234,6 +234,38 @@
         expect(subject.user_session[:context]).to eq 'confirmation'
       end
     end
+
+    context 'with rate phone fingerprint rate limit' do
+      before do
+        @user = create(:user)
+        @user2 = create(:user)
+        @unconfirmed_phone = '+1 (202) 555-1213'
+        @unconfirmed_phone2 = '+1 (202) 555-1215'
+      end
+      it 'displays an error banner' do
+        sign_in_before_2fa(@user)
+        allow(IdentityConfig.store).to receive(
+          :phone_submissions_per_fingerprint_max_attempts_window_in_minutes,
+        ).and_return(2)
+        IdentityConfig.store.phone_submissions_per_fingerprint_limit.times do
+          post(:create, params: { new_phone_form: { phone: @unconfirmed_phone } })
+        end
+
+        expect(flash[:error]).to eq(
+          t('errors.messages.phone_confirmation_limited', timeout: '1 minute'),
+        )
+
+        sign_in_before_2fa(@user2)
+        post(:create, params: { new_phone_form: { phone: @unconfirmed_phone } })
+        expect(flash[:error]).to eq(
+          t('errors.messages.phone_confirmation_limited', timeout: '1 minute'),
+        )
+
+        sign_in_before_2fa(@user)
+        post(:create, params: { new_phone_form: { phone: @unconfirmed_phone2 } })
+        expect(flash[:error]).to eq(nil)
+      end
+    end
   end
 
   describe 'before_actions' do

spec/forms/new_phone_form_spec.rb

@@ -2,6 +2,7 @@
 
 RSpec.describe NewPhoneForm do
   include Shoulda::Matchers::ActiveModel
+  include ActionView::Helpers::DateHelper
 
   let(:user) { build(:user, :fully_registered) }
   let(:phone) { '703-555-5000' }
@@ -164,6 +165,38 @@
       expect(result.success?).to eq(true)
     end
 
+    context 'validating phone fingerprint submission rate limit' do
+      let(:params) do
+        {
+          phone: '703-555-1212',
+          international_code: 'US',
+        }
+      end
+      it 'enforces phone fingerprint rate limit' do
+        allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(999)
+
+        freeze_time do
+          IdentityConfig.store.phone_submissions_per_fingerprint_limit.times do
+            subject.submit(params)
+          end
+
+          result = subject.submit(params)
+          expect(result).to be_kind_of(FormResponse)
+          expect(result.success?).to eq(false)
+          expect(result.errors[:phone_fingerprint]).to eq(
+            [
+              I18n.t(
+                'errors.messages.phone_confirmation_limited',
+                timeout: distance_of_time_in_words(
+                  RateLimiter.attempt_window_in_minutes(:phone_fingerprint_confirmation).minutes,
+                ),
+              ),
+            ],
+          )
+        end
+      end
+    end
+
     context 'when the user has already added the number' do
       it 'is invalid' do
         phone = PhoneFormatter.format('+1 (954) 555-0100', country_code: 'US')

spec/services/form_response_spec.rb

@@ -112,10 +112,8 @@
   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
+      subject(:first_error_message) { form_response.first_error_message }
       let(:errors) { {} }
 
       it { expect(first_error_message).to be_nil }
@@ -125,19 +123,35 @@
       let(:errors) { { email: ['invalid', 'too_short'], language: ['blank'] } }
 
       context 'without specified key' do
-        let(:key) { nil }
+        subject(:first_error_message) { form_response.first_error_message }
 
         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 }
+        subject(:first_error_message) { form_response.first_error_message(:language) }
 
         it 'returns the first error of the specified field' do
           expect(first_error_message).to eq('blank')
         end
+
+        context 'with key that does not exist in set of errors' do
+          subject(:first_error_message) { form_response.first_error_message(:foo) }
+
+          it 'returns nil' do
+            expect(first_error_message).to be_nil
+          end
+        end
+      end
+
+      context 'with multiple specified keys' do
+        subject(:first_error_message) { form_response.first_error_message(:foo, :language, :email) }
+
+        it 'returns the first error of the key which exists as an error' do
+          expect(first_error_message).to eq('blank')
+        end
       end
     end
   end