Skip to content

Commit

Permalink
πŸžπŸ”¨Authentication Prevent race condition when double-tapping sign-in (…
Browse files Browse the repository at this point in the history
…#1892)

* πŸžπŸ”¨`Authentication` Prevent a condition when double-tapping sign-in

- #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.
  • Loading branch information
zspencer authored Oct 14, 2023
1 parent ab517f8 commit 8c6c880
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 25 deletions.
25 changes: 15 additions & 10 deletions app/models/authenticated_session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
47 changes: 32 additions & 15 deletions spec/models/authenticated_session_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: "[email protected]").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
Expand Down

0 comments on commit 8c6c880

Please sign in to comment.