Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
10 changes: 2 additions & 8 deletions app/controllers/concerns/two_factor_authenticatable_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,18 @@ 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(:sign_in_recaptcha_assessment_id) if sign_in_recaptcha_annotation_enabled?
session.delete(:sign_in_recaptcha_assessment_id)
else
handle_invalid_verification_for_authentication_context
end
end

def annotate_recaptcha(reason)
if sign_in_recaptcha_annotation_enabled?
RecaptchaAnnotator.annotate(assessment_id: session[:sign_in_recaptcha_assessment_id], reason:)
end
RecaptchaAnnotator.annotate(assessment_id: session[:sign_in_recaptcha_assessment_id], reason:)
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
6 changes: 2 additions & 4 deletions app/services/recaptcha_annotator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@ def annotate(assessment_id:, reason: nil, annotation: nil)
return if assessment_id.blank?

if FeatureManagement.recaptcha_enterprise?
submit_annotation(assessment_id:, reason:, annotation:)
# Future:
# assessment = create_or_update_assessment!(assessment_id:, reason:, annotation:)
# RecaptchaAnnotateJob.perform_later(assessment:)
assessment = create_or_update_assessment!(assessment_id:, reason:, annotation:)
RecaptchaAnnotateJob.perform_later(assessment:)
end

{ assessment_id:, reason:, annotation: }
Expand Down
3 changes: 0 additions & 3 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,6 @@ 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 @@ -508,7 +507,6 @@ 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 @@ -609,7 +607,6 @@ 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: 0 additions & 1 deletion lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,6 @@ 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
117 changes: 34 additions & 83 deletions spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,53 +168,28 @@
context 'when there is a sign_in_recaptcha_assessment_id in the session' do
let(:assessment_id) { 'projects/project-id/assessments/assessment-id' }

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 PASSED_TWO_FACTOR and clears assessment id from session' do
recaptcha_annotation = {
assessment_id:,
reason: RecaptchaAnnotator::AnnotationReasons::PASSED_TWO_FACTOR,
}

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

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

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

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

expect { result }
.to change { controller.session[:sign_in_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[:sign_in_recaptcha_assessment_id] = assessment_id

expect(RecaptchaAnnotator).not_to receive(:annotate)

stub_analytics

expect { result }
.not_to change { controller.session[:sign_in_recaptcha_assessment_id] }
end
expect(@analytics).to have_logged_event(
'Multi-Factor Authentication',
hash_including(recaptcha_annotation:),
)
end
end
end
Expand Down Expand Up @@ -253,52 +228,28 @@
context 'when there is a sign_in_recaptcha_assessment_id in the session' do
let(:assessment_id) { 'projects/project-id/assessments/assessment-id' }

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[:sign_in_recaptcha_assessment_id] = assessment_id

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

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

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

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[:sign_in_recaptcha_assessment_id] = assessment_id
expect(RecaptchaAnnotator).to receive(:annotate)
.with(**recaptcha_annotation)
.and_return(recaptcha_annotation)

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

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

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