Skip to content

Commit

Permalink
Move race-condition handling to the Model
Browse files Browse the repository at this point in the history
  • Loading branch information
zspencer committed Oct 12, 2023
1 parent d8b04cf commit e068c24
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 22 deletions.
7 changes: 0 additions & 7 deletions app/controllers/authenticated_sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,6 @@ def create
else
render :create, status: :unprocessable_entity
end
rescue ActiveRecord::RecordInvalid
@authenticated_session = nil
if authenticated_session.save
redirect_to(current_space.presence || :root)
else
render :create, status: :unprocessable_entity
end
end

alias_method :update, :create
Expand Down
10 changes: 10 additions & 0 deletions app/models/authenticated_session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ def save
end

false
rescue ActiveRecord::RecordInvalid
self.authentication_method = nil
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

# If we don't have a OTP _or_ a way of issuing one, there's nothin' we can do.
Expand Down
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 e068c24

Please sign in to comment.