From 20dfdc962f5cdd9310ffe95f2dcebfbd713630ed Mon Sep 17 00:00:00 2001
From: Jessica Dembe <jessica.dembe@gsa.gov>
Date: Mon, 5 May 2025 15:39:43 -0400
Subject: [PATCH] Ensure new device is evaluated consistently in sign-in
 reCAPTCHA

changelog: Bug Fixes, Authentication, Ensure new device is evaluated consistently in sign-in reCAPTCHA

Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>
---
 app/controllers/users/sessions_controller.rb |  8 ++++++--
 app/forms/sign_in_recaptcha_form.rb          | 16 ++++++----------
 spec/forms/sign_in_recaptcha_form_spec.rb    | 17 +++++------------
 3 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb
index 456aa195f45..6e5a6ff4176 100644
--- a/app/controllers/users/sessions_controller.rb
+++ b/app/controllers/users/sessions_controller.rb
@@ -115,9 +115,13 @@ def recaptcha_assessment_id
     end
 
     def recaptcha_form
+      return @recaptcha_form if defined?(@recaptcha_form)
+      existing_device = User.find_with_confirmed_email(auth_params[:email])&.devices&.exists?(
+        cookie_uuid: cookies[:device],
+      )
+
       @recaptcha_form ||= SignInRecaptchaForm.new(
-        email: auth_params[:email],
-        device_cookie: cookies[:device],
+        existing_device: existing_device,
         ab_test_bucket: ab_test_bucket(:RECAPTCHA_SIGN_IN, user: user_from_params),
         **recaptcha_form_args,
       )
diff --git a/app/forms/sign_in_recaptcha_form.rb b/app/forms/sign_in_recaptcha_form.rb
index 9bbdb422ef8..584fc345682 100644
--- a/app/forms/sign_in_recaptcha_form.rb
+++ b/app/forms/sign_in_recaptcha_form.rb
@@ -5,20 +5,20 @@ class SignInRecaptchaForm
 
   RECAPTCHA_ACTION = 'sign_in'
 
-  attr_reader :form_class, :form_args, :email, :recaptcha_token, :device_cookie, :ab_test_bucket,
+  attr_reader :form_class, :form_args, :recaptcha_token, :ab_test_bucket,
               :assessment_id
 
+  attr_writer :existing_device
+
   validate :validate_recaptcha_result
 
   def initialize(
-    email:,
-    device_cookie:,
+    existing_device:,
     ab_test_bucket:,
     form_class: RecaptchaForm,
     **form_args
   )
-    @email = email
-    @device_cookie = device_cookie
+    @existing_device = existing_device
     @ab_test_bucket = ab_test_bucket
     @form_class = form_class
     @form_args = form_args
@@ -34,7 +34,7 @@ def submit(recaptcha_token:)
   def exempt?
     IdentityConfig.store.sign_in_recaptcha_score_threshold.zero? ||
       ab_test_bucket != :sign_in_recaptcha ||
-      device.present?
+      @existing_device
   end
 
   private
@@ -44,10 +44,6 @@ def validate_recaptcha_result
     errors.merge!(recaptcha_form) if !recaptcha_response.success?
   end
 
-  def device
-    User.find_with_confirmed_email(email)&.devices&.find_by(cookie_uuid: device_cookie)
-  end
-
   def score_threshold
     if exempt?
       0.0
diff --git a/spec/forms/sign_in_recaptcha_form_spec.rb b/spec/forms/sign_in_recaptcha_form_spec.rb
index 428c3cc7593..4915924d5c7 100644
--- a/spec/forms/sign_in_recaptcha_form_spec.rb
+++ b/spec/forms/sign_in_recaptcha_form_spec.rb
@@ -5,15 +5,13 @@
   let(:user) { create(:user, :with_authenticated_device) }
   let(:score_threshold_config) { 0.2 }
   let(:analytics) { FakeAnalytics.new }
-  let(:email) { user.email }
+  let(:existing_device) { false }
   let(:ab_test_bucket) { :sign_in_recaptcha }
   let(:recaptcha_token) { 'token' }
-  let(:device_cookie) { Random.hex }
   let(:score) { 1.0 }
   subject(:form) do
     described_class.new(
-      email:,
-      device_cookie:,
+      existing_device:,
       ab_test_bucket:,
       form_class: RecaptchaMockForm,
       analytics:,
@@ -45,8 +43,7 @@
   context 'with custom recaptcha form class' do
     subject(:form) do
       described_class.new(
-        email:,
-        device_cookie:,
+        existing_device:,
         ab_test_bucket:,
         analytics:,
         form_class: RecaptchaForm,
@@ -74,8 +71,6 @@
       let(:ab_test_bucket) { nil }
 
       it { is_expected.to eq(true) }
-
-      it { expect(queries_database?).to eq(false) }
     end
 
     context 'score threshold configured at zero' do
@@ -87,11 +82,9 @@
     end
 
     context 'existing device for user' do
-      let(:device_cookie) { user.devices.first.cookie_uuid }
+      let(:existing_device) { true }
 
       it { is_expected.to eq(true) }
-
-      it { expect(queries_database?).to eq(true) }
     end
 
     def queries_database?
@@ -108,7 +101,7 @@ def queries_database?
       let(:score) { 0.0 }
 
       context 'existing device for user' do
-        let(:device_cookie) { user.devices.first.cookie_uuid }
+        let(:existing_device) { true }
 
         it 'is successful' do
           expect(response.to_h).to eq(success: true)