LG-12555 Use 10-digit voice OTP for all IdV users

app/controllers/concerns/idv/ab_test_analytics_concern.rb

@@ -6,7 +6,7 @@ module AbTestAnalyticsConcern
     include OptInHelper
 
     def ab_test_analytics_buckets
-      buckets = { ab_tests: {} }
+      buckets = {}
 
       if defined?(idv_session)
         buckets[:skip_hybrid_handoff] = idv_session&.skip_hybrid_handoff
@@ -19,23 +19,7 @@ def ab_test_analytics_buckets
         buckets = buckets.merge(lniv_args)
       end
 
-      if defined?(idv_session)
-        phone_confirmation_session = idv_session.user_phone_confirmation_session ||
-                                     PhoneConfirmationSession.new(
-                                       code: nil,
-                                       phone: nil,
-                                       sent_at: nil,
-                                       delivery_method: :sms,
-                                       user: current_user,
-                                     )
-        buckets[:ab_tests].merge!(
-          phone_confirmation_session.ab_test_analytics_args,
-        )
-      end
-
-      buckets.merge!(acuant_sdk_ab_test_analytics_args)
-      buckets.delete(:ab_tests) if buckets[:ab_tests].blank?
-      buckets
+      buckets.merge(acuant_sdk_ab_test_analytics_args)
     end
   end
 end

app/controllers/concerns/two_factor_authenticatable.rb

@@ -7,7 +7,8 @@ module TwoFactorAuthenticatable
   NEED_AUTHENTICATION = 'need_two_factor_authentication'
   OTP_LENGTH = 6
   DIRECT_OTP_LENGTH = 6
-  PROOFING_DIRECT_OTP_LENGTH = 6
+  PROOFING_SMS_DIRECT_OTP_LENGTH = 6
+  PROOFING_VOICE_DIRECT_OTP_LENGTH = 10
   ALLOWED_OTP_DRIFT_SECONDS = 30
   DIRECT_OTP_VALID_FOR_MINUTES = IdentityConfig.store.otp_valid_for.freeze
   DIRECT_OTP_VALID_FOR_SECONDS = DIRECT_OTP_VALID_FOR_MINUTES * 60

app/controllers/idv/otp_verification_controller.rb

@@ -100,16 +100,11 @@ def phone_confirmation_otp_verification_form
     end
 
     def code_length
-      if ten_digit_otp?
-        10
+      if idv_session.user_phone_confirmation_session.delivery_method == :voice
+        TwoFactorAuthenticatable::PROOFING_VOICE_DIRECT_OTP_LENGTH
       else
-        TwoFactorAuthenticatable::PROOFING_DIRECT_OTP_LENGTH
+        TwoFactorAuthenticatable::PROOFING_SMS_DIRECT_OTP_LENGTH
       end
     end
-
-    def ten_digit_otp?
-      AbTests::IDV_TEN_DIGIT_OTP.bucket(current_user.uuid) == :ten_digit_otp &&
-        idv_session.user_phone_confirmation_session.delivery_method == :voice
-    end
   end
 end

app/services/idv/phone_confirmation_session.rb

@@ -4,13 +4,14 @@ module Idv
   class PhoneConfirmationSession
     attr_reader :code, :phone, :sent_at, :delivery_method, :user
 
-    def self.generate_code(user:, delivery_method:)
-      bucket = AbTests::IDV_TEN_DIGIT_OTP.bucket(user&.uuid)
-      if delivery_method == :voice && bucket == :ten_digit_otp
-        OtpCodeGenerator.generate_digits(10)
-      else # original, bucket defaults to :six_alphanumeric_otp
+    def self.generate_code(delivery_method:)
+      if delivery_method == :voice
+        OtpCodeGenerator.generate_digits(
+          TwoFactorAuthenticatable::PROOFING_VOICE_DIRECT_OTP_LENGTH,
+        )
+      else
         OtpCodeGenerator.generate_alphanumeric_digits(
-          TwoFactorAuthenticatable::PROOFING_DIRECT_OTP_LENGTH,
+          TwoFactorAuthenticatable::PROOFING_SMS_DIRECT_OTP_LENGTH,
         )
       end
     end
@@ -25,27 +26,17 @@ def initialize(code:, phone:, sent_at:, delivery_method:, user:)
 
     def self.start(phone:, delivery_method:, user:)
       new(
-        code: generate_code(user: user, delivery_method: delivery_method),
+        code: generate_code(delivery_method: delivery_method),
         phone: phone,
         sent_at: Time.zone.now,
         delivery_method: delivery_method,
         user: user,
       )
     end
 
-    def ab_test_analytics_args
-      return {} unless IdentityConfig.store.ab_testing_idv_ten_digit_otp_enabled
-
-      {
-        AbTests::IDV_TEN_DIGIT_OTP.experiment_name => {
-          bucket: AbTests::IDV_TEN_DIGIT_OTP.bucket(user.uuid),
-        },
-      }
-    end
-
     def regenerate_otp
       self.class.new(
-        code: self.class.generate_code(user: user, delivery_method: delivery_method),
+        code: self.class.generate_code(delivery_method: delivery_method),
         phone: phone,
         sent_at: Time.zone.now,
         delivery_method: delivery_method,

app/services/idv/send_phone_confirmation_otp.rb

@@ -61,6 +61,13 @@ def otp_rate_limiter
     end
 
     def send_otp
+      length, format = case delivery_method
+                       when :voice
+                         ['ten', 'digit']
+                       else
+                         ['six', 'character']
+                       end
+
       idv_session.user_phone_confirmation_session = user_phone_confirmation_session.regenerate_otp
       @telephony_response = Telephony.send_confirmation_otp(
         otp: code,
@@ -80,48 +87,21 @@ def send_otp
       otp_sent_response
     end
 
-    def bucket
-      @bucket ||= AbTests::IDV_TEN_DIGIT_OTP.bucket(
-        idv_session.user_phone_confirmation_session.user.uuid,
-      )
-    end
-
-    def format
-      return 'digit' if delivery_method == :voice && bucket == :ten_digit_otp
-
-      'character'
-    end
-
-    def length
-      return 'ten' if delivery_method == :voice && bucket == :ten_digit_otp
-
-      'six'
-    end
-
     def otp_sent_response
       FormResponse.new(
         success: telephony_response.success?, extra: extra_analytics_attributes,
       )
     end
 
     def extra_analytics_attributes
-      attributes = {
+      {
         otp_delivery_preference: delivery_method,
         country_code: parsed_phone.country,
         area_code: parsed_phone.area_code,
         phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164),
         rate_limit_exceeded: rate_limit_exceeded?,
         telephony_response: @telephony_response,
       }
-      if IdentityConfig.store.ab_testing_idv_ten_digit_otp_enabled
-        attributes[:ab_tests] = {
-          AbTests::IDV_TEN_DIGIT_OTP.experiment_name => {
-            bucket: bucket,
-          },
-        }
-      end
-
-      attributes
     end
 
     def parsed_phone

config/application.yml.default

@@ -20,8 +20,6 @@ aamva_cert_enabled: true
 aamva_supported_jurisdictions: '["AL","AR","AZ","CO","CT","DC","DE","FL","GA","HI","IA","ID","IL","IN","KS","KY","MA","MD","ME","MI","MO","MS","MT","NC","ND","NE","NJ","NM","NV","OH","OR","PA","RI","SC","SD","TN","TX","VA","VT","WA","WI","WV","WY"]'
 aamva_verification_request_timeout: 5.0
 aamva_verification_url: https://example.org:12345/verification/url
-ab_testing_idv_ten_digit_otp_enabled: false
-ab_testing_idv_ten_digit_otp_percent: 0
 all_redirect_uris_cache_duration_minutes: 2
 allowed_ialmax_providers: '[]'
 allowed_verified_within_providers: '[]'

config/initializers/ab_tests.rb

@@ -30,15 +30,4 @@ module AbTests
           0,
     },
   ).freeze
-
-  IDV_TEN_DIGIT_OTP = AbTestBucket.new(
-    experiment_name: 'idv_ten_digit_otp',
-    default_bucket: :six_alphanumeric_otp,
-    buckets: {
-      ten_digit_otp:
-        IdentityConfig.store.ab_testing_idv_ten_digit_otp_enabled ?
-          IdentityConfig.store.ab_testing_idv_ten_digit_otp_percent :
-          0,
-    },
-  ).freeze
 end

lib/identity_config.rb

@@ -35,8 +35,6 @@ def self.store
     config.add(:aamva_supported_jurisdictions, type: :json)
     config.add(:aamva_verification_request_timeout, type: :float)
     config.add(:aamva_verification_url)
-    config.add(:ab_testing_idv_ten_digit_otp_enabled, type: :boolean)
-    config.add(:ab_testing_idv_ten_digit_otp_percent, type: :integer)
     config.add(:account_reset_token_valid_for_days, type: :integer)
     config.add(:account_reset_wait_period_days, type: :integer)
     config.add(:account_reset_fraud_user_wait_period_days, type: :integer, allow_nil: true)

spec/controllers/users/two_factor_authentication_controller_spec.rb

@@ -595,38 +595,6 @@ def index
         )
       end
 
-      it 'sends a 6-digit OTP when the idv_ten_digit_otp A/B test is in progress' do
-        stub_const(
-          'AbTests::IDV_TEN_DIGIT_OTP',
-          FakeAbTestBucket.new.tap { |ab| ab.assign(@user.uuid => :ten_digit_otp) },
-        )
-
-        sign_in_before_2fa(@user)
-        subject.user_session[:context] = 'confirmation'
-        subject.user_session[:unconfirmed_phone] = @unconfirmed_phone
-        parsed_phone = Phonelib.parse(@unconfirmed_phone)
-
-        allow(Telephony).to receive(:send_confirmation_otp).and_call_original
-
-        get :send_code, params: otp_delivery_form_sms
-
-        expect(Telephony).to have_received(:send_confirmation_otp).with(
-          otp: subject.current_user.direct_otp,
-          to: @unconfirmed_phone,
-          expiration: 10,
-          channel: :sms,
-          otp_format: 'digit',
-          otp_length: '6',
-          domain: IdentityConfig.store.domain_name,
-          country_code: 'US',
-          extra_metadata: {
-            area_code: parsed_phone.area_code,
-            phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164),
-            resend: nil,
-          },
-        )
-      end
-
       it 'tracks the enrollment attempt event' do
         sign_in_before_2fa(@user)
         subject.user_session[:context] = 'confirmation'

spec/services/idv/phone_confirmation_session_spec.rb

@@ -38,63 +38,16 @@
   end
 
   describe '.generate_code' do
-    let(:ab_test_enabled) { false }
-    before do
-      allow(IdentityConfig.store).to receive(:ab_testing_idv_ten_digit_otp_enabled).
-        and_return(ab_test_enabled)
-    end
-
-    context 'A/B test not enabled' do
-      it 'generates a six-character alphanumeric code' do
-        code = described_class.generate_code(user: user, delivery_method: :voice)
+    it 'generates a six-character alphanumeric code for sms' do
+      code = described_class.generate_code(delivery_method: :sms)
 
-        expect(code).to match(six_char_alphanumeric)
-      end
+      expect(code).to match(six_char_alphanumeric)
     end
-    context '10-digit A/B test enabled' do
-      let(:ab_test_enabled) { true }
-
-      context '10-digit A/B test puts user in :six_alphanumeric_otp bucket' do
-        before do
-          stub_const(
-            'AbTests::IDV_TEN_DIGIT_OTP',
-            FakeAbTestBucket.new.tap { |ab| ab.assign(user.uuid => :six_alphanumeric_otp) },
-          )
-        end
 
-        it 'generates a six-character alphanumeric code for sms' do
-          code = described_class.generate_code(user: user, delivery_method: :sms)
+    it 'generates a ten-digit numeric code for voice' do
+      code = described_class.generate_code(delivery_method: :voice)
 
-          expect(code).to match(six_char_alphanumeric)
-        end
-
-        it 'generates a six-character alphanumeric code for voice' do
-          code = described_class.generate_code(user: user, delivery_method: :voice)
-
-          expect(code).to match(six_char_alphanumeric)
-        end
-      end
-
-      context '10-digit A/B test puts user in :ten_digit_otp bucket' do
-        before do
-          stub_const(
-            'AbTests::IDV_TEN_DIGIT_OTP',
-            FakeAbTestBucket.new.tap { |ab| ab.assign(user.uuid => :ten_digit_otp) },
-          )
-        end
-
-        it 'generates a six-character alphanumeric code for sms' do
-          code = described_class.generate_code(user: user, delivery_method: :sms)
-
-          expect(code).to match(six_char_alphanumeric)
-        end
-
-        it 'generates a ten-digit numeric code for voice' do
-          code = described_class.generate_code(user: user, delivery_method: :voice)
-
-          expect(code).to match(ten_digit_numeric)
-        end
-      end
+      expect(code).to match(ten_digit_numeric)
     end
   end
 

spec/services/idv/send_phone_confirmation_otp_spec.rb

@@ -94,8 +94,8 @@
           to: phone,
           expiration: 10,
           channel: :voice,
-          otp_format: 'character',
-          otp_length: '6',
+          otp_format: 'digit',
+          otp_length: '10',
           domain: IdentityConfig.store.domain_name,
           country_code: 'US',
           extra_metadata: {