Fix Mandatory OTP Issue

CHANGELOG.md

@@ -2,6 +2,17 @@
 
 ## UNRELEASED
 
+Summary: Move mandatory OTP functionality to the helper layer to ensure that it is enforced throughout application (rather than one time at log in).
+
+Details:
+- Add PublicHelpers class, and add to Devise @@helpers variable to generate per-scope ensure\_mandatory\_{scope}\_otp! methods;
+- Update order of module definitions and "require" statements in devise-otp.rb (required for adding DeviseOtpAuthenticable PublicHelpers to Devise @@helpers variable);
+
+Breaking Changes:
+- Requires adding "ensure\_mandatory\_{scope}\_otp! to controllers;
+
+## 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

README.md

@@ -94,6 +94,12 @@ The install generator adds some options to the end of your Devise config file (`
 * `config.otp_issuer`: The name of the token issuer, to be added to the provisioning url. Display will vary based on token application. (defaults to the Rails application class)
 * `config.otp_controller_path`: The view path for Devise OTP controllers. The default being 'devise' to match Devise default installation.
 
+## Mandatory OTP
+Enforcing mandatory OTP requires adding the ensure\_mandatory\_{scope}\_otp! method to the desired controller(s) to ensure that the user is redirected to the Enable Two-Factor Authentication form before proceeding to other parts of the application. This functions the same way as the authenticate\_{scope}! methods, and can be included inline with them in the controllers, e.g.:
+
+    before_action :authenticate_user!
+    before_action :ensure_mandatory_user_otp!
+
 ## Authors
 
 The project was originally started by Lele Forzani by forking [devise_google_authenticator](https://github.com/AsteriskLabs/devise_google_authenticator) and still contains some devise_google_authenticator code. It's now maintained by [Josef Strzibny](https://github.com/strzibny/).

lib/devise-otp.rb

@@ -9,6 +9,25 @@
 
 require "devise"
 
+#
+# define DeviseOtpAuthenticatable module, and autoload hooks and helpers
+#
+module DeviseOtpAuthenticatable
+  autoload :Hooks, "devise_otp_authenticatable/hooks"
+
+  module Controllers
+    autoload :Helpers, "devise_otp_authenticatable/controllers/helpers"
+    autoload :UrlHelpers, "devise_otp_authenticatable/controllers/url_helpers"
+    autoload :PublicHelpers, "devise_otp_authenticatable/controllers/public_helpers"
+  end
+end
+
+require "devise_otp_authenticatable/routes"
+require "devise_otp_authenticatable/engine"
+
+#
+# update Devise module with additions needed for DeviseOtpAuthenticatable
+#
 module Devise
   mattr_accessor :otp_mandatory
   @@otp_mandatory = false
@@ -49,22 +68,23 @@ module Devise
   mattr_accessor :otp_controller_path
   @@otp_controller_path = "devise"
 
+  #
+  # add PublicHelpers to helpers class variable to ensure that per-mapping helpers are present.
+  # this integrates with the "define_helpers," which is run when adding each mapping in the Devise gem (lib/devise.rb#541)
+  #
+  @@helpers << DeviseOtpAuthenticatable::Controllers::PublicHelpers
+
   module Otp
   end
-end
 
-module DeviseOtpAuthenticatable
-  autoload :Hooks, "devise_otp_authenticatable/hooks"
-
-  module Controllers
-    autoload :Helpers, "devise_otp_authenticatable/controllers/helpers"
-    autoload :UrlHelpers, "devise_otp_authenticatable/controllers/url_helpers"
-  end
 end
 
-require "devise_otp_authenticatable/routes"
-require "devise_otp_authenticatable/engine"
-
 Devise.add_module :otp_authenticatable,
   controller: :tokens,
   model: "devise_otp_authenticatable/models/otp_authenticatable", route: :otp
+#
+# add PublicHelpers after adding Devise module to ensure that per-mapping routes from above are included
+#
+ActiveSupport.on_load(:action_controller) do
+  include DeviseOtpAuthenticatable::Controllers::PublicHelpers
+end

lib/devise_otp_authenticatable/controllers/public_helpers.rb

@@ -0,0 +1,36 @@
+module DeviseOtpAuthenticatable
+  module Controllers
+    module PublicHelpers
+      extend ActiveSupport::Concern
+
+      def self.generate_helpers!
+        Devise.mappings.each do |key, mapping|
+          self.define_helpers(mapping)
+        end
+      end
+
+      def self.define_helpers(mapping) #:nodoc:
+        mapping = mapping.name
+
+        class_eval <<-METHODS, __FILE__, __LINE__ + 1
+          def ensure_mandatory_#{mapping}_otp!
+            resource = current_#{mapping}
+            if !devise_controller?
+              if otp_mandatory_on?(resource)
+                redirect_to edit_#{mapping}_otp_token_path
+              end
+            end
+          end
+        METHODS
+      end
+
+      def otp_mandatory_on?(resource)
+        return true if resource.class.otp_mandatory && !resource.otp_enabled
+        return false unless resource.respond_to?(:otp_mandatory)
+
+        resource.otp_mandatory && !resource.otp_enabled
+      end
+
+    end
+  end
+end

lib/devise_otp_authenticatable/hooks/sessions.rb

@@ -24,10 +24,6 @@ def create_with_otp
         warden.logout
         store_location_for(resource, devise_stored_location) # restore the stored location
         respond_with resource, location: otp_credential_path_for(resource, {challenge: challenge})
-      elsif otp_mandatory_on?(resource) # if mandatory, log in user but send him to the must activate otp
-        set_flash_message(:notice, :signed_in_but_otp) if is_navigational_format?
-        sign_in(resource_name, resource)
-        respond_with resource, location: otp_token_path_for(resource)
       else
         sign_in(resource_name, resource)
         respond_with resource, location: after_sign_in_path_for(resource)
@@ -45,14 +41,5 @@ def otp_challenge_required_on?(resource)
       resource.otp_enabled && !is_otp_trusted_browser_for?(resource)
     end
 
-    #
-    # the resource -should- have otp turned on, but it isn't
-    #
-    def otp_mandatory_on?(resource)
-      return true if resource.class.otp_mandatory && !resource.otp_enabled
-      return false unless resource.respond_to?(:otp_mandatory)
-
-      resource.otp_mandatory && !resource.otp_enabled
-    end
   end
 end