Merge branch 'rs-improve-grace-period' into 'master'

Don't show any "2FA required" message if it's not actually required

Prior, if the user had enabled and then disabled 2FA, they would be
shown a "You must enable Two-factor Authentication for your account."
message when going back to re-activate it, even if 2FA enforcement was
disabled.

See merge request !3014

5 files changed, 17 insertions, 10 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 24fa4e8a808..ec5a5a01636 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -6,6 +6,7 @@ v 8.5.2
   - Bring the "branded appearance" feature from EE to CE
   - Fix error 500 when commenting on a commit
   - Improve implementation to check read access to forks and add pagination
+  - Don't show any "2FA required" message if it's not actually required
   - Update Rails to 4.2.5.2
 
 v 8.5.1
diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb
index f3bfede4354..8f83fdd02bc 100644
--- a/app/controllers/profiles/two_factor_auths_controller.rb
+++ b/app/controllers/profiles/two_factor_auths_controller.rb
@@ -12,11 +12,13 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
 
     current_user.save! if current_user.changed?
 
-    if two_factor_grace_period_expired?
-      flash.now[:alert] = 'You must enable Two-factor Authentication for your account.'
-    else
-      grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours
-      flash.now[:alert] = "You must enable Two-factor Authentication for your account before #{l(grace_period_deadline)}."
+    if two_factor_authentication_required?
+      if two_factor_grace_period_expired?
+        flash.now[:alert] = 'You must enable Two-factor Authentication for your account.'
+      else
+        grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours
+        flash.now[:alert] = "You must enable Two-factor Authentication for your account before #{l(grace_period_deadline)}."
+      end
     end
 
     @qr_code = build_qr_code
diff --git a/app/models/user.rb b/app/models/user.rb
index 02ff2456f2b..2ef8d851b26 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -354,11 +354,12 @@ class User < ActiveRecord::Base
 
   def disable_two_factor!
     update_attributes(
-      two_factor_enabled:        false,
-      encrypted_otp_secret:      nil,
-      encrypted_otp_secret_iv:   nil,
-      encrypted_otp_secret_salt: nil,
-      otp_backup_codes:          nil
+      two_factor_enabled:          false,
+      encrypted_otp_secret:        nil,
+      encrypted_otp_secret_iv:     nil,
+      encrypted_otp_secret_salt:   nil,
+      otp_grace_period_started_at: nil,
+      otp_backup_codes:            nil
     )
   end
 
diff --git a/spec/factories.rb b/spec/factories.rb
index 2a81684dfcf..d6483ed6ce8 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -32,6 +32,7 @@ FactoryGirl.define do
       before(:create) do |user|
         user.two_factor_enabled = true
         user.otp_secret = User.generate_otp_secret(32)
+        user.otp_grace_period_started_at = Time.now
         user.generate_otp_backup_codes!
       end
     end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 32d4f39b04a..95188f518c1 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -256,6 +256,7 @@ describe User, models: true do
       expect(user).to be_two_factor_enabled
       expect(user.encrypted_otp_secret).not_to be_nil
       expect(user.otp_backup_codes).not_to be_nil
+      expect(user.otp_grace_period_started_at).not_to be_nil
 
       user.disable_two_factor!
 
@@ -264,6 +265,7 @@ describe User, models: true do
       expect(user.encrypted_otp_secret_iv).to be_nil
       expect(user.encrypted_otp_secret_salt).to be_nil
       expect(user.otp_backup_codes).to be_nil
+      expect(user.otp_grace_period_started_at).to be_nil
     end
   end