Simplify refresh_credential functionality

CHANGELOG.md

@@ -2,6 +2,19 @@
 
 ## UNRELEASED
 
+Use dedicated devise hook for refreshing credentials.
+
+Details:
+- Add dedicated hook and column for refresh_credential functionality;
+- Move/simplify check/redirection to refresh_credentials! helper method;
+- Remove "refresh_otp_credentials" method from session hook (no longer needed);
+- Remove comments regarding cookie/persistence scope (no longer needed);
+
+Breaking Changes:
+- Requires adding the credentials_refreshed_at field to the database;
+
+## UNRELEASED
+
 Summary:
 - Require confirmation token before enabling Two Factor Authentication (2FA) to ensure that user has added OTP token properly to their device
 - Update system to populate OTP secrets as needed

app/controllers/devise_otp/devise/otp_tokens_controller.rb

@@ -3,7 +3,7 @@ module Devise
     class OtpTokensController < DeviseController
       include ::Devise::Controllers::Helpers
 
-      prepend_before_action :ensure_credentials_refresh
+      prepend_before_action :refresh_credentials!
       prepend_before_action :authenticate_scope!
 
       protect_from_forgery except: [:clear_persistence, :delete_persistence]
@@ -97,15 +97,6 @@ def recovery
 
       private
 
-      def ensure_credentials_refresh
-        ensure_resource!
-
-        if needs_credentials_refresh?(resource)
-          otp_set_flash_message :notice, :need_to_refresh_credentials
-          redirect_to refresh_otp_credential_path_for(resource)
-        end
-      end
-
       def scope
         resource_name.to_sym
       end

lib/devise_otp_authenticatable/controllers/helpers.rb

@@ -39,24 +39,34 @@ def ensure_resource!
         end
       end
 
-      # fixme do cookies and persistence need to be scoped? probably
+      # check if the resource needs a credentials refresh, and redirect if needed
+      # this resource.
       #
+      def refresh_credentials!
+        ensure_resource!
+
+        if needs_credentials_refresh?(resource)
+          otp_set_refresh_return_url
+          otp_set_flash_message :notice, :need_to_refresh_credentials
+          redirect_to refresh_otp_credential_path_for(resource)
+        end
+      end
+
       # check if the resource needs a credentials refresh. IE, they need to be asked a password again to access
       # this resource.
       #
       def needs_credentials_refresh?(resource)
-        return false unless resource.class.otp_credentials_refresh
+        return false unless resource.class.otp_credentials_refresh and resource.respond_to?(:credentials_refreshed_at)
 
-        (!session[otp_scoped_refresh_property].present? ||
-            (session[otp_scoped_refresh_property] < DateTime.now)).tap { |need| otp_set_refresh_return_url if need }
+        resource.credentials_refreshed_at.blank? or resource.credentials_refreshed_at + resource.class.otp_credentials_refresh < DateTime.now
       end
 
       #
       # credentials are refreshed
       #
       def otp_refresh_credentials_for(resource)
         return false unless resource.class.otp_credentials_refresh
-        session[otp_scoped_refresh_property] = (Time.now + resource.class.otp_credentials_refresh)
+        resource.update(:credentials_refreshed_at => DateTime.now)
       end
 
       #

lib/devise_otp_authenticatable/hooks/refreshable.rb

@@ -0,0 +1,7 @@
+# After each sign in, update credentials refreshed at time
+Warden::Manager.after_set_user except: :fetch do |record, warden, options|
+  if record.class.otp_credentials_refresh && record.respond_to?(:credentials_refreshed_at) && warden.authenticated?(options[:scope])
+    record.update(:credentials_refreshed_at => DateTime.now)
+  end
+end
+

lib/devise_otp_authenticatable/hooks/sessions.rb

@@ -16,8 +16,6 @@ def create_with_otp
       devise_stored_location = stored_location_for(resource) # Grab the current stored location before it gets lost by warden.logout
       store_location_for(resource, devise_stored_location) # Restore it since #stored_location_for removes it
 
-      otp_refresh_credentials_for(resource)
-
       yield resource if block_given?
       if otp_challenge_required_on?(resource)
         challenge = resource.generate_otp_challenge!

lib/devise_otp_authenticatable/models/otp_authenticatable.rb

@@ -1,4 +1,5 @@
 require "rotp"
+require 'devise_otp_authenticatable/hooks/refreshable'
 
 module Devise::Models
   module OtpAuthenticatable

lib/generators/active_record/templates/migration.rb

@@ -12,6 +12,8 @@ def self.up
 
       t.string    :otp_session_challenge
       t.datetime  :otp_challenge_expires
+
+      t.datetime  :credentials_refreshed_at
     end
     add_index :<%= table_name %>, :otp_session_challenge,  :unique => true
     add_index :<%= table_name %>, :otp_challenge_expires

test/dummy/db/migrate/20130131160351_devise_otp_add_to_users.rb

@@ -13,6 +13,8 @@ def self.up
 
       t.string :otp_session_challenge
       t.datetime :otp_challenge_expires
+
+      t.datetime :credentials_refreshed_at
     end
 
     add_index :users, :otp_session_challenge, unique: true