Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions app/forms/new_phone_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,33 +60,28 @@ def ingest_phone_number(params)
def extra_analytics_attributes
{
otp_delivery_preference: otp_delivery_preference,
phone_type: @phone_info&.type, # comes from pinpoint API
phone_type: phone_info&.type, # comes from pinpoint API
types: parsed_phone.types, # comes from Phonelib gem
carrier: @phone_info&.carrier,
carrier: phone_info&.carrier,
country_code: parsed_phone.country,
area_code: parsed_phone.area_code,
pii_like_keypaths: [[:errors, :phone], [:error_details, :phone]],
}.tap do |extra|
extra[:redacted_phone] = @redacted_phone if @redacted_phone
extra[:warn] = @warning_message if @warning_message
end
end

def validate_not_voip
return if phone.blank? || !IdentityConfig.store.voip_check

@phone_info = Telephony.phone_info(phone)

return unless IdentityConfig.store.voip_block

if @phone_info.type == :voip &&
if phone_info.type == :voip &&
!FeatureManagement.voip_allowed_phones.include?(parsed_phone.e164)
errors.add(:phone, I18n.t('errors.messages.voip_phone'))
elsif @phone_info.error
elsif phone_info.error
errors.add(:phone, I18n.t('errors.messages.voip_check_error'))
end
rescue Aws::Pinpoint::Errors::BadRequestException
errors.add(:phone, :improbable_phone)
@redacted_phone = redact(phone)
end

def validate_not_duplicate
Expand All @@ -104,6 +99,24 @@ def validate_not_premium_rate
end
end

# @return [Telephony::PhoneNumberInfo, nil]
def phone_info
return @phone_info if defined?(@phone_info)

if phone.blank? || !IdentityConfig.store.voip_check
@phone_info = nil
else
@phone_info = Telephony.phone_info(phone)
end
rescue Aws::Pinpoint::Errors::TooManyRequestsException
@warning_message = 'AWS pinpoint phone info rate limit'
@phone_info = Telephony::PhoneNumberInfo.new(type: :unknown)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does :unknown mean we accept the number?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, we only fail on :voip:

if @phone_info.type == :voip &&
!FeatureManagement.voip_allowed_phones.include?(parsed_phone.e164)
errors.add(:phone, I18n.t('errors.messages.voip_phone'))
elsif @phone_info.error
errors.add(:phone, I18n.t('errors.messages.voip_check_error'))
end

rescue Aws::Pinpoint::Errors::BadRequestException
errors.add(:phone, :improbable_phone)
@redacted_phone = redact(phone)
@phone_info = Telephony::PhoneNumberInfo.new(type: :unknown)
end

def parsed_phone
@parsed_phone ||= Phonelib.parse(phone)
end
Expand Down
14 changes: 14 additions & 0 deletions spec/forms/new_phone_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,20 @@
end
end

context 'when AWS rate limits info type checks' do
before do
expect(Telephony).to receive(:phone_info).
and_raise(Aws::Pinpoint::Errors::TooManyRequestsException.new(nil, 'error message'))
end

it 'logs a warning and fails open' do
expect(result.extra[:warn]).to include('AWS pinpoint phone info rate limit')

expect(result.success?).to eq(true)
expect(result.errors).to be_blank
end
end

context 'when voip checks are disabled' do
let(:voip_check) { false }

Expand Down