LG-15655: Submit reCAPTCHA annotations through worker job (Part 2 of 2)

app/controllers/concerns/two_factor_authenticatable_methods.rb

@@ -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)

app/services/recaptcha_annotator.rb

@@ -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: }
@@ -36,6 +34,7 @@ def submit_assessment(assessment)
         annotation: assessment.annotation_before_type_cast,
         reason: assessment.annotation_reason_before_type_cast,
       )
+      assessment.destroy
     end
 
     private

config/application.yml.default

@@ -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
@@ -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"]'
@@ -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'

db/primary_migrate/20250228141538_add_timestamps_to_recaptcha_assessments.rb

@@ -0,0 +1,7 @@
+class AddTimestampsToRecaptchaAssessments < ActiveRecord::Migration[8.0]
+  def change
+    add_timestamps :recaptcha_assessments
+    change_column_comment :recaptcha_assessments, :created_at, from: nil, to: 'sensitive=false'
+    change_column_comment :recaptcha_assessments, :updated_at, from: nil, to: 'sensitive=false'
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.0].define(version: 2025_02_24_134110) do
+ActiveRecord::Schema[8.0].define(version: 2025_02_28_141538) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
   enable_extension "pg_catalog.plpgsql"
@@ -482,6 +482,8 @@
   create_table "recaptcha_assessments", id: :string, force: :cascade do |t|
     t.string "annotation", comment: "sensitive=false"
     t.string "annotation_reason", comment: "sensitive=false"
+    t.datetime "created_at", null: false, comment: "sensitive=false"
+    t.datetime "updated_at", null: false, comment: "sensitive=false"
   end
 
   create_table "registration_logs", force: :cascade do |t|

lib/identity_config.rb

@@ -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)

spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb

@@ -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
@@ -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

spec/services/recaptcha_annotator_spec.rb

@@ -52,21 +52,18 @@
           .and_return(recaptcha_enterprise_project_id)
         allow(IdentityConfig.store).to receive(:recaptcha_enterprise_api_key)
           .and_return(recaptcha_enterprise_api_key)
-        stub_request(:post, annotation_url)
-          .with do |req|
-            parsed_body = JSON.parse(req.body)
-            next if reason && parsed_body['reasons'] != [reason.to_s]
-            next if !reason && parsed_body.key?('reasons')
-            next if annotation && parsed_body['annotation'] != annotation.to_s
-            true
-          end
-          .to_return(headers: { 'Content-Type': 'application/json' }, body: '{}')
+        allow(RecaptchaAnnotateJob).to receive(:perform_later)
       end
 
-      it 'submits annotation' do
-        annotate
+      it 'schedules the annotation to be submitted' do
+        expect(RecaptchaAnnotateJob).to receive(:perform_later) do |assessment:|
+          expect(assessment).to be_kind_of(RecaptchaAssessment)
+          expect(assessment.annotation_before_type_cast).to eq(annotation)
+          expect(assessment.annotation_reason_before_type_cast).to eq(reason)
+          expect(assessment).to be_persisted
+        end
 
-        expect(WebMock).to have_requested(:post, annotation_url)
+        annotate
       end
 
       it 'logs analytics' do
@@ -83,11 +80,15 @@
         let(:annotation) { nil }
         subject(:annotate) { RecaptchaAnnotator.annotate(assessment_id:, reason:) }
 
-        it 'submits only what is provided' do
-          annotate
+        it 'schedules the annotation to be with only what is provided' do
+          expect(RecaptchaAnnotateJob).to receive(:perform_later) do |assessment:|
+            expect(assessment).to be_kind_of(RecaptchaAssessment)
+            expect(assessment.annotation_before_type_cast).to be_nil
+            expect(assessment.annotation_reason_before_type_cast).to eq(reason)
+            expect(assessment).to be_persisted
+          end
 
-          expect(WebMock).to have_requested(:post, annotation_url)
-            .with(body: { reasons: [reason] }.to_json)
+          annotate
         end
 
         it 'returns a hash describing annotation' do
@@ -98,34 +99,6 @@
           )
         end
       end
-
-      context 'with nil assessment id' do
-        let(:assessment_id) { nil }
-
-        it 'does not submit annotation' do
-          annotate
-
-          expect(WebMock).not_to have_requested(:post, annotation_url)
-        end
-
-        it { expect(annotate).to be_nil }
-      end
-
-      context 'with connection error' do
-        before do
-          stub_request(:post, annotation_url).to_timeout
-        end
-
-        it 'fails gracefully' do
-          annotate
-        end
-
-        it 'notices the error to NewRelic' do
-          expect(NewRelic::Agent).to receive(:notice_error).with(Faraday::Error)
-
-          annotate
-        end
-      end
     end
   end
 
@@ -153,9 +126,9 @@
         .to_return(headers: { 'Content-Type': 'application/json' }, body: '{}')
     end
 
-    it 'submits annotation' do
-      result
-
+    it 'submits annotation and destroys the record' do
+      assessment
+      expect { result }.to change { RecaptchaAssessment.count }.by(-1)
       expect(WebMock).to have_requested(:post, annotation_url)
     end