From d7dc4af2998dfb28a8b7049168e6780dbd8436f2 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Fri, 24 May 2024 10:06:44 +0900 Subject: [PATCH 01/10] move ensuring of mandatory otp to ApplicationController to allow enforcement throughout application; --- lib/devise-otp.rb | 9 +++++ .../controllers/public_helpers.rb | 36 +++++++++++++++++++ .../hooks/sessions.rb | 13 ------- 3 files changed, 45 insertions(+), 13 deletions(-) create mode 100644 lib/devise_otp_authenticatable/controllers/public_helpers.rb diff --git a/lib/devise-otp.rb b/lib/devise-otp.rb index fe158aa..c5c706e 100644 --- a/lib/devise-otp.rb +++ b/lib/devise-otp.rb @@ -59,9 +59,18 @@ module DeviseOtpAuthenticatable 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 +ActiveSupport.on_load(:action_controller) do + Devise.mappings.each do |key, mapping| + DeviseOtpAuthenticatable::Controllers::PublicHelpers.define_helpers(mapping) + end + + include DeviseOtpAuthenticatable::Controllers::PublicHelpers +end + require "devise_otp_authenticatable/routes" require "devise_otp_authenticatable/engine" diff --git a/lib/devise_otp_authenticatable/controllers/public_helpers.rb b/lib/devise_otp_authenticatable/controllers/public_helpers.rb new file mode 100644 index 0000000..2ecce94 --- /dev/null +++ b/lib/devise_otp_authenticatable/controllers/public_helpers.rb @@ -0,0 +1,36 @@ +module DeviseOtpAuthenticatable + module Controllers + module PublicHelpers + extend ActiveSupport::Concern + + 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 + + ActiveSupport.on_load(:action_controller) do + if respond_to?(:helper_method) + helper_method "ensure_mandatory_#{mapping}_otp!" + end + end + 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 diff --git a/lib/devise_otp_authenticatable/hooks/sessions.rb b/lib/devise_otp_authenticatable/hooks/sessions.rb index d2713a1..47c9ef4 100644 --- a/lib/devise_otp_authenticatable/hooks/sessions.rb +++ b/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 From 5433c6f8ef12ce325ec38ac47e42b22e9521d085 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Fri, 24 May 2024 10:23:21 +0900 Subject: [PATCH 02/10] add test_method for troubleshooting; --- lib/devise_otp_authenticatable/controllers/public_helpers.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/devise_otp_authenticatable/controllers/public_helpers.rb b/lib/devise_otp_authenticatable/controllers/public_helpers.rb index 2ecce94..135dfae 100644 --- a/lib/devise_otp_authenticatable/controllers/public_helpers.rb +++ b/lib/devise_otp_authenticatable/controllers/public_helpers.rb @@ -24,6 +24,10 @@ def ensure_mandatory_#{mapping}_otp! end end + def test_method + raise "Test method included successfully." + end + def otp_mandatory_on?(resource) return true if resource.class.otp_mandatory && !resource.otp_enabled return false unless resource.respond_to?(:otp_mandatory) From 9bed4a314e0a67656da7496e9594e4ac3da1c06c Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Fri, 24 May 2024 10:29:08 +0900 Subject: [PATCH 03/10] add per-mapping test methods; --- .../controllers/public_helpers.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/devise_otp_authenticatable/controllers/public_helpers.rb b/lib/devise_otp_authenticatable/controllers/public_helpers.rb index 135dfae..71b863d 100644 --- a/lib/devise_otp_authenticatable/controllers/public_helpers.rb +++ b/lib/devise_otp_authenticatable/controllers/public_helpers.rb @@ -6,6 +6,12 @@ module PublicHelpers def self.define_helpers(mapping) #:nodoc: mapping = mapping.name + class_eval <<-TEST_METHOD, __FILE__, __LINE__ + 1 + def test_#{mapping}_method + raise "Test method for #{mapping} included successfully." + end + TEST_METHOD + class_eval <<-METHODS, __FILE__, __LINE__ + 1 def ensure_mandatory_#{mapping}_otp! resource = current_#{mapping} From 2caf0c4b035191b6e2105dd3a16667c56b741b69 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Fri, 24 May 2024 11:35:18 +0900 Subject: [PATCH 04/10] move PublicHelper method generation to regenerate_helpers! method to ensure that Devise mappings are populated before calling define_helpers; --- lib/devise-otp.rb | 24 ++++++++++++------- .../controllers/public_helpers.rb | 6 ----- lib/devise_otp_authenticatable/engine.rb | 1 + 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/devise-otp.rb b/lib/devise-otp.rb index c5c706e..d275167 100644 --- a/lib/devise-otp.rb +++ b/lib/devise-otp.rb @@ -51,6 +51,22 @@ module Devise module Otp end + + # Regenerates url helpers considering Devise.mapping + def self.regenerate_helpers! + Devise::Controllers::UrlHelpers.remove_helpers! + Devise::Controllers::UrlHelpers.generate_helpers! + + ActiveSupport.on_load(:action_controller) do + Devise.mappings.each do |key, mapping| + DeviseOtpAuthenticatable::Controllers::PublicHelpers.define_helpers(mapping) + end + + include DeviseOtpAuthenticatable::Controllers::PublicHelpers + end + + end + end module DeviseOtpAuthenticatable @@ -63,14 +79,6 @@ module Controllers end end -ActiveSupport.on_load(:action_controller) do - Devise.mappings.each do |key, mapping| - DeviseOtpAuthenticatable::Controllers::PublicHelpers.define_helpers(mapping) - end - - include DeviseOtpAuthenticatable::Controllers::PublicHelpers -end - require "devise_otp_authenticatable/routes" require "devise_otp_authenticatable/engine" diff --git a/lib/devise_otp_authenticatable/controllers/public_helpers.rb b/lib/devise_otp_authenticatable/controllers/public_helpers.rb index 71b863d..b4ff13f 100644 --- a/lib/devise_otp_authenticatable/controllers/public_helpers.rb +++ b/lib/devise_otp_authenticatable/controllers/public_helpers.rb @@ -22,12 +22,6 @@ def ensure_mandatory_#{mapping}_otp! end end METHODS - - ActiveSupport.on_load(:action_controller) do - if respond_to?(:helper_method) - helper_method "ensure_mandatory_#{mapping}_otp!" - end - end end def test_method diff --git a/lib/devise_otp_authenticatable/engine.rb b/lib/devise_otp_authenticatable/engine.rb index 035c83a..48bbfdb 100644 --- a/lib/devise_otp_authenticatable/engine.rb +++ b/lib/devise_otp_authenticatable/engine.rb @@ -12,6 +12,7 @@ class Engine < ::Rails::Engine ActiveSupport.on_load(:devise_controller) do include DeviseOtpAuthenticatable::Controllers::UrlHelpers include DeviseOtpAuthenticatable::Controllers::Helpers + include DeviseOtpAuthenticatable::Controllers::PublicHelpers end ActiveSupport.on_load(:action_view) do From 7abb4bde40ba6e1fb467bb0b4d930dbbff0341fa Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Fri, 24 May 2024 11:36:38 +0900 Subject: [PATCH 05/10] remove test methods; --- .../controllers/public_helpers.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/devise_otp_authenticatable/controllers/public_helpers.rb b/lib/devise_otp_authenticatable/controllers/public_helpers.rb index b4ff13f..72f670f 100644 --- a/lib/devise_otp_authenticatable/controllers/public_helpers.rb +++ b/lib/devise_otp_authenticatable/controllers/public_helpers.rb @@ -6,12 +6,6 @@ module PublicHelpers def self.define_helpers(mapping) #:nodoc: mapping = mapping.name - class_eval <<-TEST_METHOD, __FILE__, __LINE__ + 1 - def test_#{mapping}_method - raise "Test method for #{mapping} included successfully." - end - TEST_METHOD - class_eval <<-METHODS, __FILE__, __LINE__ + 1 def ensure_mandatory_#{mapping}_otp! resource = current_#{mapping} @@ -24,10 +18,6 @@ def ensure_mandatory_#{mapping}_otp! METHODS end - def test_method - raise "Test method included successfully." - end - def otp_mandatory_on?(resource) return true if resource.class.otp_mandatory && !resource.otp_enabled return false unless resource.respond_to?(:otp_mandatory) From f73fa41b36b1babdb54e36671978ff6890c7e071 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Fri, 24 May 2024 11:47:07 +0900 Subject: [PATCH 06/10] add generate_helpers! method for consistency, and comment changes to regenerate_helpers! method; --- lib/devise-otp.rb | 12 ++++++------ .../controllers/public_helpers.rb | 6 ++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/devise-otp.rb b/lib/devise-otp.rb index d275167..7801fb2 100644 --- a/lib/devise-otp.rb +++ b/lib/devise-otp.rb @@ -52,19 +52,19 @@ module Devise module Otp end - # Regenerates url helpers considering Devise.mapping + # + # tapping into regenerate_helpers! method, to ensure that Devise mappings are present when generating public helpers + # def self.regenerate_helpers! + # Existing (Devise) Devise::Controllers::UrlHelpers.remove_helpers! Devise::Controllers::UrlHelpers.generate_helpers! + # Additions (Devise OTP) + DeviseOtpAuthenticatable::Controllers::PublicHelpers.generate_helpers! ActiveSupport.on_load(:action_controller) do - Devise.mappings.each do |key, mapping| - DeviseOtpAuthenticatable::Controllers::PublicHelpers.define_helpers(mapping) - end - include DeviseOtpAuthenticatable::Controllers::PublicHelpers end - end end diff --git a/lib/devise_otp_authenticatable/controllers/public_helpers.rb b/lib/devise_otp_authenticatable/controllers/public_helpers.rb index 72f670f..3ce7293 100644 --- a/lib/devise_otp_authenticatable/controllers/public_helpers.rb +++ b/lib/devise_otp_authenticatable/controllers/public_helpers.rb @@ -3,6 +3,12 @@ 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 From 0a8ffe1a64146cc48cf23e6461cafd525234a5a7 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Fri, 24 May 2024 12:03:12 +0900 Subject: [PATCH 07/10] utilize existing @@helpers method to avoid overwriting regenerate_helpers! method; move DeviseOtpAuthenticable module above Devise module modifications to ensure that PublicHelpers class is available; append "on_load" callback to devise-otp file to ensure that updated helpers are included; --- lib/devise-otp.rb | 48 +++++++++++++++++++++-------------------------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/lib/devise-otp.rb b/lib/devise-otp.rb index 7801fb2..3f838f1 100644 --- a/lib/devise-otp.rb +++ b/lib/devise-otp.rb @@ -9,6 +9,19 @@ require "devise" +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" + module Devise mattr_accessor :otp_mandatory @@otp_mandatory = false @@ -49,39 +62,20 @@ module Devise mattr_accessor :otp_controller_path @@otp_controller_path = "devise" - module Otp - end - # - # tapping into regenerate_helpers! method, to ensure that Devise mappings are present when generating public helpers + # add PublicHelpers to helpers class variable to ensure that "define_helpers" is run when adding mapping in Devise gem (lib/devise.rb#541) # - def self.regenerate_helpers! - # Existing (Devise) - Devise::Controllers::UrlHelpers.remove_helpers! - Devise::Controllers::UrlHelpers.generate_helpers! - - # Additions (Devise OTP) - DeviseOtpAuthenticatable::Controllers::PublicHelpers.generate_helpers! - ActiveSupport.on_load(:action_controller) do - include DeviseOtpAuthenticatable::Controllers::PublicHelpers - end - end + @@helpers << DeviseOtpAuthenticatable::Controllers::PublicHelpers -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" - autoload :PublicHelpers, "devise_otp_authenticatable/controllers/public_helpers" + module Otp end -end -require "devise_otp_authenticatable/routes" -require "devise_otp_authenticatable/engine" +end Devise.add_module :otp_authenticatable, controller: :tokens, model: "devise_otp_authenticatable/models/otp_authenticatable", route: :otp + +ActiveSupport.on_load(:action_controller) do + include DeviseOtpAuthenticatable::Controllers::PublicHelpers +end From 0178bfe44b6fa4eac14b9c102627f732fb589b87 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Fri, 24 May 2024 12:20:41 +0900 Subject: [PATCH 08/10] remove PublicHelpers from engine.rb; comment changes to devise-otp.rb file; --- lib/devise-otp.rb | 13 +++++++++++-- lib/devise_otp_authenticatable/engine.rb | 1 - 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/devise-otp.rb b/lib/devise-otp.rb index 3f838f1..17ea0d3 100644 --- a/lib/devise-otp.rb +++ b/lib/devise-otp.rb @@ -9,6 +9,9 @@ require "devise" +# +# define DeviseOtpAuthenticatable module, and autoload hooks and helpers +# module DeviseOtpAuthenticatable autoload :Hooks, "devise_otp_authenticatable/hooks" @@ -22,6 +25,9 @@ module Controllers 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 @@ -63,7 +69,8 @@ module Devise @@otp_controller_path = "devise" # - # add PublicHelpers to helpers class variable to ensure that "define_helpers" is run when adding mapping in Devise gem (lib/devise.rb#541) + # 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 @@ -75,7 +82,9 @@ module Otp 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 diff --git a/lib/devise_otp_authenticatable/engine.rb b/lib/devise_otp_authenticatable/engine.rb index 48bbfdb..035c83a 100644 --- a/lib/devise_otp_authenticatable/engine.rb +++ b/lib/devise_otp_authenticatable/engine.rb @@ -12,7 +12,6 @@ class Engine < ::Rails::Engine ActiveSupport.on_load(:devise_controller) do include DeviseOtpAuthenticatable::Controllers::UrlHelpers include DeviseOtpAuthenticatable::Controllers::Helpers - include DeviseOtpAuthenticatable::Controllers::PublicHelpers end ActiveSupport.on_load(:action_view) do From a115f53ecbf6aa9b955ae6858e6b88006fd08a41 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Sat, 25 May 2024 21:53:09 +0900 Subject: [PATCH 09/10] update CHANGELOG and README regarding new ensure_mandatory_{scope}_otp! method; --- CHANGELOG.md | 11 +++++++++++ README.md | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d1d330..a1b7af6 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index cb29bd9..66413c7 100644 --- a/README.md +++ b/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.: + + authenticate_user! + 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/). From 88869249ec92de3debf02526b0824a669d197c48 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Sun, 26 May 2024 05:29:34 +0900 Subject: [PATCH 10/10] add before_action to README example; --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 66413c7..064c178 100644 --- a/README.md +++ b/README.md @@ -97,8 +97,8 @@ The install generator adds some options to the end of your Devise config file (` ## 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.: - authenticate_user! - ensure_mandatory_user_otp! + before_action :authenticate_user! + before_action :ensure_mandatory_user_otp! ## Authors