diff --git a/app/controllers/authenticated_sessions_controller.rb b/app/controllers/authenticated_sessions_controller.rb index 535617440..998917f0f 100644 --- a/app/controllers/authenticated_sessions_controller.rb +++ b/app/controllers/authenticated_sessions_controller.rb @@ -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 diff --git a/app/models/authenticated_session.rb b/app/models/authenticated_session.rb index e5b4b59d3..ec1ac29e7 100644 --- a/app/models/authenticated_session.rb +++ b/app/models/authenticated_session.rb @@ -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. 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