From 24bef5e67a81c5edf9dacb65ecc091cac1f4c528 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Mon, 11 May 2015 14:31:31 -0400
Subject: [PATCH] Handle password reset for users with 2FA enabled

---
 app/controllers/passwords_controller.rb | 21 ++++++++++
 spec/features/login_spec.rb             |  4 +-
 spec/features/password_reset_spec.rb    | 53 +++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 spec/features/password_reset_spec.rb

diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index dcbbe5baa4b2c..88459d4080a9c 100644
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -15,4 +15,25 @@ def create
       respond_with(resource)
     end
   end
+
+  # After a user resets their password, prompt for 2FA code if enabled instead
+  # of signing in automatically
+  #
+  # See http://git.io/vURrI
+  def update
+    super do |resource|
+      # TODO (rspeicher): In Devise master (> 3.4.1), we can set
+      # `Devise.sign_in_after_reset_password = false` and avoid this mess.
+      if resource.errors.empty? && resource.try(:otp_required_for_login?)
+        resource.unlock_access! if unlockable?(resource)
+
+        # Since we are not signing this user in, we use the :updated_not_active
+        # message which only contains "Your password was changed successfully."
+        set_flash_message(:notice, :updated_not_active) if is_flashing_format?
+
+        # Redirect to sign in so they can enter 2FA code
+        respond_with(resource, location: new_session_path(resource)) and return
+      end
+    end
+  end
 end
diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb
index 61defb8a33350..046a9f6191df4 100644
--- a/spec/features/login_spec.rb
+++ b/spec/features/login_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 feature 'Login' do
-  context 'with two-factor authentication' do
+  describe 'with two-factor authentication' do
     context 'with valid username/password' do
       let(:user) { create(:user, :two_factor) }
 
@@ -78,7 +78,7 @@ def enter_code(code)
     end
   end
 
-  context 'without two-factor authentication' do
+  describe 'without two-factor authentication' do
     let(:user) { create(:user) }
 
     it 'allows basic login' do
diff --git a/spec/features/password_reset_spec.rb b/spec/features/password_reset_spec.rb
new file mode 100644
index 0000000000000..a34efce09ef2c
--- /dev/null
+++ b/spec/features/password_reset_spec.rb
@@ -0,0 +1,53 @@
+require 'spec_helper'
+
+feature 'Password reset' do
+  def forgot_password
+    click_on 'Forgot your password?'
+    fill_in 'Email', with: user.email
+    click_button 'Reset password'
+    user.reload
+  end
+
+  def get_reset_token
+    mail = ActionMailer::Base.deliveries.last
+    body = mail.body.encoded
+    body.scan(/reset_password_token=(.+)\"/).flatten.first
+  end
+
+  def reset_password(password = 'password')
+    visit edit_user_password_path(reset_password_token: get_reset_token)
+
+    fill_in 'New password', with: password
+    fill_in 'Confirm new password', with: password
+    click_button 'Change your password'
+  end
+
+  describe 'with two-factor authentication' do
+    let(:user) { create(:user, :two_factor) }
+
+    it 'requires login after password reset' do
+      visit root_path
+
+      forgot_password
+      reset_password
+
+      expect(page).to have_content("Your password was changed successfully.")
+      expect(page).not_to have_content("You are now signed in.")
+      expect(current_path).to eq new_user_session_path
+    end
+  end
+
+  describe 'without two-factor authentication' do
+    let(:user) { create(:user) }
+
+    it 'automatically logs in after password reset' do
+      visit root_path
+
+      forgot_password
+      reset_password
+
+      expect(current_path).to eq root_path
+      expect(page).to have_content("Your password was changed successfully. You are now signed in.")
+    end
+  end
+end
-- 
GitLab