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
8 changes: 7 additions & 1 deletion app/services/phone_recaptcha_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ def score_threshold
private

def validator
@validator ||= validator_class.new(score_threshold:, **validator_args)
@validator ||= validator_class.new(
score_threshold:,
extra_analytics_properties: {
phone_country_code: parsed_phone.country,
},
**validator_args,
)
end

def score_threshold_country_override
Expand Down
11 changes: 9 additions & 2 deletions app/services/recaptcha_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@ class RecaptchaValidator
EXEMPT_ERROR_CODES = ['missing-input-secret', 'invalid-input-secret']
VALID_RECAPTCHA_VERSIONS = [2, 3]

attr_reader :recaptcha_version, :score_threshold, :analytics
attr_reader :recaptcha_version, :score_threshold, :analytics, :extra_analytics_properties

def initialize(recaptcha_version: 3, score_threshold: 0.0, analytics: nil)
def initialize(
recaptcha_version: 3,
score_threshold: 0.0,
analytics: nil,
extra_analytics_properties: {}
)
if !VALID_RECAPTCHA_VERSIONS.include?(recaptcha_version)
raise ArgumentError, "Invalid reCAPTCHA version #{recaptcha_version}, expected one of " \
"#{VALID_RECAPTCHA_VERSIONS}"
Expand All @@ -14,6 +19,7 @@ def initialize(recaptcha_version: 3, score_threshold: 0.0, analytics: nil)
@score_threshold = score_threshold
@analytics = analytics
@recaptcha_version = recaptcha_version
@extra_analytics_properties = extra_analytics_properties
end

def exempt?
Expand Down Expand Up @@ -80,6 +86,7 @@ def log_analytics(recaptcha_result: nil, error: nil)
recaptcha_version:,
evaluated_as_valid: recaptcha_result_valid?(recaptcha_result),
exception_class: error&.class&.name,
**extra_analytics_properties,
)
end

Expand Down
22 changes: 21 additions & 1 deletion spec/features/phone/add_phone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,13 @@
end

scenario 'adding a phone with a reCAPTCHA challenge', js: true do
user = create(:user, :signed_up)

allow(IdentityConfig.store).to receive(:phone_recaptcha_mock_validator).and_return(true)
allow(IdentityConfig.store).to receive(:phone_recaptcha_score_threshold).and_return(0.6)
fake_analytics = FakeAnalytics.new(user:)
allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(fake_analytics)

user = create(:user, :signed_up)
sign_in_and_2fa_user(user)
within('.sidenav') { click_on t('account.navigation.add_phone_number') }

Expand All @@ -221,6 +224,23 @@
expect(page).to have_content(t('two_factor_authentication.header_text'))
visit account_path
within('.sidenav') { click_on t('account.navigation.add_phone_number') }
expect(fake_analytics).to have_logged_event(
'reCAPTCHA verify result received',
hash_including(
recaptcha_result: {
'success' => true,
'score' => 0.5,
'error-codes' => [],
'challenge_ts' => kind_of(String),
'hostname' => anything,
},
evaluated_as_valid: false,
score_threshold: 0.6,
recaptcha_version: 3,
exception_class: nil,
phone_country_code: 'CA',
),
)

# Passing international should display OTP confirmation
fill_in t('two_factor_authentication.phone_label'), with: '3065550100'
Expand Down
11 changes: 10 additions & 1 deletion spec/services/phone_recaptcha_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,16 @@

it 'passes instance variables to validator' do
recaptcha_validator = instance_double(RecaptchaValidator, valid?: true)
expect(RecaptchaValidator).to receive(:new).and_return(recaptcha_validator)
expect(RecaptchaValidator).to receive(:new).
with(
score_threshold: score_threshold_config,
analytics:,
recaptcha_version:,
extra_analytics_properties: {
phone_country_code: parsed_phone.country,
},
).
and_return(recaptcha_validator)

validator.valid?('token')
end
Expand Down
27 changes: 26 additions & 1 deletion spec/services/recaptcha_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@
describe RecaptchaValidator do
let(:score_threshold) { 0.2 }
let(:analytics) { FakeAnalytics.new }
let(:extra_analytics_properties) { {} }
let(:recaptcha_secret_key_v2) { 'recaptcha_secret_key_v2' }
let(:recaptcha_secret_key_v3) { 'recaptcha_secret_key_v3' }
subject(:validator) { RecaptchaValidator.new(score_threshold:, analytics:) }

subject(:validator) do
RecaptchaValidator.new(score_threshold:, analytics:, extra_analytics_properties:)
end

before do
allow(IdentityConfig.store).to receive(:recaptcha_secret_key_v2).
Expand Down Expand Up @@ -233,6 +237,27 @@
)
end

context 'with extra analytics properties' do
let(:extra_analytics_properties) { { extra: true } }

it 'logs analytics of the body' do
valid

expect(analytics).to have_logged_event(
'reCAPTCHA verify result received',
recaptcha_result: {
'success' => true,
'score' => score,
},
evaluated_as_valid: true,
score_threshold: score_threshold,
recaptcha_version: 3,
exception_class: nil,
extra: true,
)
end
end

context 'without analytics' do
let(:analytics) { nil }

Expand Down