From 8c6c880a489b1ae2171400babd943b9025ae27c6 Mon Sep 17 00:00:00 2001
From: Zee <50284+zspencer@users.noreply.github.com>
Date: Fri, 13 Oct 2023 20:16:21 -0700
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9E=F0=9F=94=A8`Authentication`=20Prev?=
 =?UTF-8?q?ent=20race=20condition=20when=20double-tapping=20sign-in=20(#18?=
 =?UTF-8?q?92)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 🐞🔨`Authentication` Prevent a condition when double-tapping sign-in

- https://github.com/zinc-collective/convene/issues/1809

OK, so my working assumption is that the bug is caused by two requests
being processed at the same time for a new email address, and the first
one creates the `AuthenticationMethod` and the second one *tries* to,
and fails; resulting in the validation error.

This requeries the database and retries the rest of the work.
---
 app/models/authenticated_session.rb       | 25 +++++++-----
 spec/models/authenticated_session_spec.rb | 47 +++++++++++++++--------
 2 files changed, 47 insertions(+), 25 deletions(-)

diff --git a/app/models/authenticated_session.rb b/app/models/authenticated_session.rb
index e5b4b59d3..a0f7d3be7 100644
--- a/app/models/authenticated_session.rb
+++ b/app/models/authenticated_session.rb
@@ -35,16 +35,10 @@ def destroy
   def save
     return false if !valid? || !actionable?
 
-    if one_time_password.nil?
-      authentication_method.send_one_time_password!(space)
-      return false
-    elsif authentication_method.verify?(one_time_password)
-      session[:person_id] = authentication_method.person.id
-      authentication_method.confirm!
-      return true
-    end
-
-    false
+    verify_or_resend_otp
+  rescue ActiveRecord::RecordInvalid
+    self.authentication_method = nil
+    verify_or_resend_otp
   end
 
   # If we don't have a OTP _or_ a way of issuing one, there's nothin' we can do.
@@ -56,4 +50,15 @@ def save
     return if one_time_password.nil? || authentication_method.verify?(one_time_password)
     errors.add(:one_time_password, :invalid_one_time_password)
   end
+
+  private def verify_or_resend_otp
+    if one_time_password.nil?
+      authentication_method.send_one_time_password!(space)
+      false
+    elsif authentication_method.verify?(one_time_password)
+      session[:person_id] = authentication_method.person.id
+      authentication_method.confirm!
+      true
+    end
+  end
 end
diff --git a/spec/models/authenticated_session_spec.rb b/spec/models/authenticated_session_spec.rb
index 8218d5338..e88635c79 100644
--- a/spec/models/authenticated_session_spec.rb
+++ b/spec/models/authenticated_session_spec.rb
@@ -71,29 +71,46 @@
             expect { authenticated_session.save }.not_to change(AuthenticationMethod, :count)
           end
         end
-      end
 
-      context "and the correct otp is provided" do
-        let(:one_time_password) { "123456" }
+        context "when a race condition caused the authentication method was created in another thread" do
+          let(:authentication_method) { nil }
+
+          it "reloads the authentication method and tries again" do
+            sad_authentication_method = instance_double(AuthenticationMethod)
+            allow(sad_authentication_method).to receive(:send_one_time_password!).with(space).and_raise(ActiveRecord::RecordInvalid)
+            happy_authentication_method = instance_double(AuthenticationMethod)
+            allow(happy_authentication_method).to receive(:send_one_time_password!).with(space)
+            allow(AuthenticationMethod).to receive(:find_or_initialize_by).with(contact_method: :email, contact_location: "test@example.com").and_return(sad_authentication_method, happy_authentication_method)
 
-        before do
-          allow(authentication_method).to receive(:verify?)
-            .with(one_time_password).and_return(true)
+            authenticated_session.save
 
-          allow(authentication_method).to receive(:person)
-            .and_return(build_stubbed(:person, id: SecureRandom.uuid))
+            expect(sad_authentication_method).to have_received(:send_one_time_password!).with(space)
+            expect(happy_authentication_method).to have_received(:send_one_time_password!).with(space)
+          end
         end
 
-        it "confirms the authentication method" do
-          authenticated_session.save
+        context "and the correct otp is provided" do
+          let(:one_time_password) { "123456" }
 
-          expect(authentication_method).to have_received(:confirm!)
-        end
+          before do
+            allow(authentication_method).to receive(:verify?)
+              .with(one_time_password).and_return(true)
 
-        it "populates the persons id in the session" do
-          authenticated_session.save
+            allow(authentication_method).to receive(:person)
+              .and_return(build_stubbed(:person, id: SecureRandom.uuid))
+          end
 
-          expect(session[:person_id]).to eql(authentication_method.person.id)
+          it "confirms the authentication method" do
+            authenticated_session.save
+
+            expect(authentication_method).to have_received(:confirm!)
+          end
+
+          it "populates the persons id in the session" do
+            authenticated_session.save
+
+            expect(session[:person_id]).to eql(authentication_method.person.id)
+          end
         end
       end
     end