From 80229ae247a7a9af98aa1113b0fb35db002c438d Mon Sep 17 00:00:00 2001
From: space pudim <rogi@skylittlesystem.org>
Date: Tue, 30 Jul 2019 18:58:53 +0100
Subject: [PATCH] Use create date over expiry on remember 2fa cookie

If a remember 2fa cookie is issued expiring in 30 days and the server is
updated with a new value for `remember_otp_session_for_seconds`, the
cookie will still be valid for 30 days, as the final date was set on it.

To get around that, timestamp the cookie and compare that to the current
setting on the server.
---
 .../devise/two_factor_authentication_controller.rb  |  6 +++++-
 .../hooks/two_factor_authenticatable.rb             |  8 +++++---
 spec/features/two_factor_authenticatable_spec.rb    | 13 +++++++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/app/controllers/devise/two_factor_authentication_controller.rb b/app/controllers/devise/two_factor_authentication_controller.rb
index 0e7aed84..46c86299 100644
--- a/app/controllers/devise/two_factor_authentication_controller.rb
+++ b/app/controllers/devise/two_factor_authentication_controller.rb
@@ -46,7 +46,11 @@ def set_remember_two_factor_cookie(resource)
 
     if expires_seconds && expires_seconds > 0
       cookies.signed[TwoFactorAuthentication::REMEMBER_TFA_COOKIE_NAME] = {
-          value: "#{resource.class}-#{resource.public_send(Devise.second_factor_resource_id)}",
+          value: {
+            user_class: resource.class.to_s,
+            user_id: resource.public_send(Devise.second_factor_resource_id).to_s,
+            created_at: Time.current
+          },
           expires: expires_seconds.seconds.from_now
       }
     end
diff --git a/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb b/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb
index 3ff03415..87ed6b44 100644
--- a/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb
+++ b/lib/two_factor_authentication/hooks/two_factor_authenticatable.rb
@@ -1,8 +1,10 @@
 Warden::Manager.after_authentication do |user, auth, options|
   if auth.env["action_dispatch.cookies"]
-    expected_cookie_value = "#{user.class}-#{user.public_send(Devise.second_factor_resource_id)}"
-    actual_cookie_value = auth.env["action_dispatch.cookies"].signed[TwoFactorAuthentication::REMEMBER_TFA_COOKIE_NAME]
-    bypass_by_cookie = actual_cookie_value == expected_cookie_value
+    cookie = auth.env["action_dispatch.cookies"].signed[TwoFactorAuthentication::REMEMBER_TFA_COOKIE_NAME]
+    bypass_by_cookie   = cookie.present?
+    bypass_by_cookie &&= cookie["user_class"] == user.class.to_s
+    bypass_by_cookie &&= cookie["user_id"]    == user.id.to_s
+    bypass_by_cookie &&= Time.current         <= cookie["created_at"].to_datetime + user.class.remember_otp_session_for_seconds
   end
 
   if user.respond_to?(:need_two_factor_authentication?) && !bypass_by_cookie
diff --git a/spec/features/two_factor_authenticatable_spec.rb b/spec/features/two_factor_authenticatable_spec.rb
index d433d636..9a743bb7 100644
--- a/spec/features/two_factor_authenticatable_spec.rb
+++ b/spec/features/two_factor_authenticatable_spec.rb
@@ -136,6 +136,19 @@
         expect(page).to have_content("Enter the code that was sent to you")
       end
 
+      scenario "requires TFA code again after if expiry date changes" do
+        sms_sign_in
+
+        logout
+
+        User.remember_otp_session_for_seconds = 1.day
+        Timecop.travel(1.day.from_now)
+        login_as user
+        visit dashboard_path
+        expect(page).to have_content("You are signed in as Marissa")
+        expect(page).to have_content("Enter the code that was sent to you")
+      end
+
       scenario 'TFA should be different for different users' do
         sms_sign_in