Refactor SessionsController

Also adds test case for providing an invalid 2FA code and then a valid
one without re-entering username and password.

1 files changed, 25 insertions, 22 deletions

diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 21505442e35..e2a5c612579 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -29,10 +29,7 @@ class SessionsController < Devise::SessionsController
 
   def create
     super do |resource|
-      # Remove any lingering user data from login
-      session.delete(:user)
-
-      # User has successfully signed in, so clear any unused reset tokens
+      # User has successfully signed in, so clear any unused reset token
       if resource.reset_password_token.present?
         resource.update_attributes(reset_password_token: nil,
                                    reset_password_sent_at: nil)
@@ -46,34 +43,40 @@ class SessionsController < Devise::SessionsController
     params.require(:user).permit(:login, :password, :remember_me, :otp_attempt)
   end
 
+  def find_user
+    if user_params[:login]
+      User.by_login(user_params[:login])
+    elsif user_params[:otp_attempt] && session[:otp_user_id]
+      User.find(session[:otp_user_id])
+    end
+  end
+
   def authenticate_with_two_factor
-    @user = User.by_login(user_params[:login])
+    user = self.resource = find_user
+
+    return unless user && user.otp_required_for_login
 
-    if user_params[:otp_attempt].present? && session[:user]
-      if valid_otp_attempt?
-        # Insert the saved params from the session into the request parameters
-        # so they're available to Devise::Strategies::DatabaseAuthenticatable
-        request.params[:user].merge!(session[:user])
+    if user_params[:otp_attempt].present? && session[:otp_user_id]
+      if valid_otp_attempt?(user)
+        # Remove any lingering user data from login
+        session.delete(:otp_user_id)
+
+        sign_in(user)
       else
         @error = 'Invalid two-factor code'
         render :two_factor and return
       end
     else
-      if @user && @user.valid_password?(user_params[:password])
-        self.resource = @user
-
-        if resource.otp_required_for_login
-          # Login is valid, save the values to the session so we can prompt the
-          # user for a one-time password.
-          session[:user] = user_params
-          render :two_factor and return
-        end
+      if user && user.valid_password?(user_params[:password])
+        # Save the user's ID to session so we can ask for a one-time password
+        session[:otp_user_id] = user.id
+        render :two_factor and return
       end
     end
   end
 
-  def valid_otp_attempt?
-    @user.valid_otp?(user_params[:otp_attempt]) ||
-    @user.invalidate_otp_backup_code!(user_params[:otp_attempt])
+  def valid_otp_attempt?(user)
+    user.valid_otp?(user_params[:otp_attempt]) ||
+    user.invalidate_otp_backup_code!(user_params[:otp_attempt])
   end
 end