Skip to content

Commit

Permalink
Add a feature flag for sign-in recaptcha annotation
Browse files Browse the repository at this point in the history
  • Loading branch information
vrajmohan committed Jan 31, 2025
1 parent 9133a7c commit fe271ca
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 38 deletions.
10 changes: 8 additions & 2 deletions app/controllers/concerns/two_factor_authenticatable_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,24 @@ def handle_verification_for_authentication_context(result:, auth_method:, extra_
if result.success?
handle_valid_verification_for_authentication_context(auth_method:)
user_session.delete(:mfa_attempts)
session.delete(:recaptcha_assessment_id)
session.delete(:recaptcha_assessment_id) if sign_in_recaptcha_annotation_enabled?
else
handle_invalid_verification_for_authentication_context
end
end

def annotate_recaptcha(reason)
RecaptchaAnnotator.annotate(assessment_id: session[:recaptcha_assessment_id], reason:)
if sign_in_recaptcha_annotation_enabled?
RecaptchaAnnotator.annotate(assessment_id: session[:recaptcha_assessment_id], reason:)
end
end

private

def sign_in_recaptcha_annotation_enabled?
IdentityConfig.store.sign_in_recaptcha_annotation_enabled
end

def handle_valid_verification_for_authentication_context(auth_method:)
mark_user_session_authenticated(auth_method:, authentication_type: :valid_2fa)
disavowal_event, disavowal_token = create_user_event_with_disavowal(:sign_in_after_2fa)
Expand Down
3 changes: 3 additions & 0 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ short_term_phone_otp_max_attempt_window_in_seconds: 10
short_term_phone_otp_max_attempts: 2
show_unsupported_passkey_platform_authentication_setup: false
show_user_attribute_deprecation_warnings: false
sign_in_recaptcha_annotation_enabled: false
sign_in_recaptcha_log_failures_only: false
sign_in_recaptcha_percent_tested: 0
sign_in_recaptcha_score_threshold: 0.0
Expand Down Expand Up @@ -495,6 +496,7 @@ development:
secret_key_base: development_secret_key_base
session_encryption_key: 27bad3c25711099429c1afdfd1890910f3b59f5a4faec1c85e945cb8b02b02f261ba501d99cfbb4fab394e0102de6fecf8ffe260f322f610db3e96b2a775c120
show_unsupported_passkey_platform_authentication_setup: true
sign_in_recaptcha_annotation_enabled: true
sign_in_recaptcha_percent_tested: 100
sign_in_recaptcha_score_threshold: 0.3
skip_encryption_allowed_list: '["urn:gov:gsa:SAML:2.0.profiles:sp:sso:localhost"]'
Expand Down Expand Up @@ -595,6 +597,7 @@ test:
secret_key_base: test_secret_key_base
session_encryption_key: 27bad3c25711099429c1afdfd1890910f3b59f5a4faec1c85e945cb8b02b02f261ba501d99cfbb4fab394e0102de6fecf8ffe260f322f610db3e96b2a775c120
short_term_phone_otp_max_attempts: 100
sign_in_recaptcha_annotation_enabled: true
skip_encryption_allowed_list: '[]'
socure_docv_document_request_endpoint: 'https://sandbox.socure.test/documnt-request'
socure_docv_webhook_secret_key: 'secret-key'
Expand Down
1 change: 1 addition & 0 deletions lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ def self.store
config.add(:sign_in_user_id_per_ip_attempt_window_in_minutes, type: :integer)
config.add(:sign_in_user_id_per_ip_attempt_window_max_minutes, type: :integer)
config.add(:sign_in_user_id_per_ip_max_attempts, type: :integer)
config.add(:sign_in_recaptcha_annotation_enabled, type: :boolean)
config.add(:sign_in_recaptcha_log_failures_only, type: :boolean)
config.add(:sign_in_recaptcha_percent_tested, type: :integer)
config.add(:sign_in_recaptcha_score_threshold, type: :float)
Expand Down
120 changes: 84 additions & 36 deletions spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,29 +169,53 @@
context 'when there is a recaptcha_assessment_id in the session' do
let(:assessment_id) { 'projects/project-id/assessments/assessment-id' }

it 'annotates the assessment with PASSED_TWO_FACTOR and logs the annotation' do
recaptcha_annotation = {
assessment_id:,
reason: RecaptchaAnnotator::AnnotationReasons::PASSED_TWO_FACTOR,
}
context 'when sign_in_recaptcha_annotation_enabled is true' do
before do
allow(IdentityConfig.store).to receive(:sign_in_recaptcha_annotation_enabled)
.and_return(true)
end

controller.session[:recaptcha_assessment_id] = assessment_id
controller.user_session[:phone_recaptcha_assessment_id] = assessment_id
it 'annotates assessment with PASSED_TWO_FACTOR and clears assessment id from session' do
recaptcha_annotation = {
assessment_id:,
reason: RecaptchaAnnotator::AnnotationReasons::PASSED_TWO_FACTOR,
}

expect(RecaptchaAnnotator).to receive(:annotate)
.with(**recaptcha_annotation)
.and_return(recaptcha_annotation)
controller.session[:recaptcha_assessment_id] = assessment_id

stub_analytics
expect(RecaptchaAnnotator).to receive(:annotate)
.with(**recaptcha_annotation)
.and_return(recaptcha_annotation)

expect { result }
.to change { controller.session[:recaptcha_assessment_id] }
.from(assessment_id).to(nil)
stub_analytics

expect(@analytics).to have_logged_event(
'Multi-Factor Authentication',
hash_including(recaptcha_annotation:),
)
expect { result }
.to change { controller.session[:recaptcha_assessment_id] }
.from(assessment_id).to(nil)

expect(@analytics).to have_logged_event(
'Multi-Factor Authentication',
hash_including(recaptcha_annotation:),
)
end
end

context 'when sign_in_recaptcha_annotation_enabled is false' do
before do
allow(IdentityConfig.store).to receive(:sign_in_recaptcha_annotation_enabled)
.and_return(false)
end

it 'does not annotate the assessment' do
controller.session[:recaptcha_assessment_id] = assessment_id

expect(RecaptchaAnnotator).not_to receive(:annotate)

stub_analytics

expect { result }
.not_to change { controller.session[:recaptcha_assessment_id] }
end
end
end
end
Expand Down Expand Up @@ -231,29 +255,53 @@
context 'when there is a recaptcha_assessment_id in the session' do
let(:assessment_id) { 'projects/project-id/assessments/assessment-id' }

it 'annotates the assessment with FAILED_TWO_FACTOR and logs the annotation' do
recaptcha_annotation = {
assessment_id:,
reason: RecaptchaAnnotator::AnnotationReasons::FAILED_TWO_FACTOR,
}
context 'when sign_in_recaptcha_annotation_enabled is true' do
before do
allow(IdentityConfig.store).to receive(:sign_in_recaptcha_annotation_enabled)
.and_return(true)
end

it 'annotates assessment with FAILED_TWO_FACTOR and clears assessment id from session' do
recaptcha_annotation = {
assessment_id:,
reason: RecaptchaAnnotator::AnnotationReasons::FAILED_TWO_FACTOR,
}

controller.session[:recaptcha_assessment_id] = assessment_id
controller.user_session[:phone_recaptcha_assessment_id] = assessment_id

expect(RecaptchaAnnotator).to receive(:annotate)
.with(**recaptcha_annotation)
.and_return(recaptcha_annotation)

stub_analytics

controller.session[:recaptcha_assessment_id] = assessment_id
controller.user_session[:phone_recaptcha_assessment_id] = assessment_id
expect { result }
.not_to change { controller.session[:recaptcha_assessment_id] }
.from(assessment_id)

expect(RecaptchaAnnotator).to receive(:annotate)
.with(**recaptcha_annotation)
.and_return(recaptcha_annotation)
expect(@analytics).to have_logged_event(
'Multi-Factor Authentication',
hash_including(recaptcha_annotation:),
)
end
end
context 'when sign_in_recaptcha_annotation_enabled is false' do
before do
allow(IdentityConfig.store).to receive(:sign_in_recaptcha_annotation_enabled)
.and_return(false)
end

it 'does not annotate the assessment' do
controller.session[:recaptcha_assessment_id] = assessment_id

stub_analytics
expect(RecaptchaAnnotator).not_to receive(:annotate)

expect { result }
.not_to change { controller.session[:recaptcha_assessment_id] }
.from(assessment_id)
stub_analytics

expect(@analytics).to have_logged_event(
'Multi-Factor Authentication',
hash_including(recaptcha_annotation:),
)
expect { result }
.not_to change { controller.session[:recaptcha_assessment_id] }
end
end
end
end
Expand Down

0 comments on commit fe271ca

Please sign in to comment.