From 3334c3fc7026497fc9da258824795c0ce23a8ffd Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 29 Feb 2016 13:56:40 -0500 Subject: [PATCH 1/2] Reset `otp_grace_period_started_at` after disabling 2FA Prior, if the user enabled 2FA, then disabled it and came back some time after the grace period expired, they would be forced to enable 2FA immediately. --- app/models/user.rb | 11 ++++++----- spec/factories.rb | 1 + spec/models/user_spec.rb | 2 ++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 6baf2468ad..4bc4ab8de7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -362,11 +362,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 264e3ed2c8..cd57661c1c 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 88821dd0da..7ad7aab2ee 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -268,6 +268,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! @@ -276,6 +277,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 From b0ec9529e218ba577404c48453ea1818e28fd382 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 29 Feb 2016 13:58:36 -0500 Subject: [PATCH 2/2] 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. --- .../profiles/two_factor_auths_controller.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index f3bfede435..8f83fdd02b 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