From 95b435a15a057b8fb25a62ed3434bf5aa0d77362 Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Fri, 27 Mar 2026 19:23:58 -0300 Subject: [PATCH 1/6] feat: add two-factor method registration and base model module Add Devise.register_two_factor_method API for extensions to register 2FA methods (analogous to config.omniauth). Register a single :two_factor_authenticatable module in modules.rb. Extend mapping strategies to include 2FA methods for Warden scope defaults. Provide TwoFactorAuthenticatable base model module with per-model two_factor_methods config, enabled_two_factors discovery, and automatic inclusion of extension model concerns. --- lib/devise.rb | 39 +++++++++++++++ lib/devise/mapping.rb | 16 ++++++- lib/devise/models.rb | 1 + .../models/two_factor_authenticatable.rb | 47 +++++++++++++++++++ lib/devise/modules.rb | 3 ++ 5 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 lib/devise/models/two_factor_authenticatable.rb diff --git a/lib/devise.rb b/lib/devise.rb index 8e0c85e77d..9782de509d 100644 --- a/lib/devise.rb +++ b/lib/devise.rb @@ -18,6 +18,7 @@ module Devise autoload :ParameterSanitizer, 'devise/parameter_sanitizer' autoload :TimeInflector, 'devise/time_inflector' autoload :TokenGenerator, 'devise/token_generator' + autoload :TwoFactor, 'devise/two_factor' module Controllers autoload :Helpers, 'devise/controllers/helpers' @@ -40,6 +41,7 @@ module Mailers module Strategies autoload :Base, 'devise/strategies/base' autoload :Authenticatable, 'devise/strategies/authenticatable' + autoload :TwoFactor, 'devise/strategies/two_factor' end module Test @@ -58,6 +60,13 @@ module Test # Strategies that do not require user input. NO_INPUT = [] + # Global default for two_factor_methods per-model config. + mattr_accessor :two_factor_methods + @@two_factor_methods = [] + + # Registry of two-factor method configs set via register_two_factor_method. + mattr_reader :two_factor_method_configs, default: {} + # True values used to check params TRUE_VALUES = [true, 1, '1', 'on', 'ON', 't', 'T', 'true', 'TRUE'] @@ -439,6 +448,36 @@ def self.add_module(module_name, options = {}) Devise::Mapping.add_module module_name end + def self.register_two_factor_method(name, options = {}) + options.assert_valid_keys(:model, :strategy, :controller, :route) + two_factor_method_configs[name.to_sym] = options + STRATEGIES[name.to_sym] = options[:strategy] if options[:strategy] + + if controller = options[:controller] + controller = (controller == true ? name : controller) + CONTROLLERS[name.to_sym] = controller + end + + if route = options[:route] + case route + when TrueClass + key, value = name, [] + when Symbol + key, value = route, [] + when Hash + key, value = route.keys.first, route.values.flatten + else + raise ArgumentError, ":route should be true, a Symbol or a Hash" + end + + URL_HELPERS[key] ||= [] + URL_HELPERS[key].concat(value) + URL_HELPERS[key].uniq! + + ROUTES[name.to_sym] = key + end + end + # Sets warden configuration using a block that will be invoked on warden # initialization. # diff --git a/lib/devise/mapping.rb b/lib/devise/mapping.rb index 8b1f94ced2..c0d57aaa43 100644 --- a/lib/devise/mapping.rb +++ b/lib/devise/mapping.rb @@ -84,7 +84,13 @@ def to end def strategies - @strategies ||= STRATEGIES.values_at(*self.modules).compact.uniq.reverse + @strategies ||= begin + keys = self.modules + if to.respond_to?(:two_factor_methods) && to.two_factor_methods + keys = keys + Array(to.two_factor_methods) + end + STRATEGIES.values_at(*keys).compact.uniq.reverse + end end def no_input_strategies @@ -92,7 +98,13 @@ def no_input_strategies end def routes - @routes ||= ROUTES.values_at(*self.modules).compact.uniq + @routes ||= begin + keys = self.modules + if to.respond_to?(:two_factor_methods) && to.two_factor_methods + keys = keys + Array(to.two_factor_methods) + end + ROUTES.values_at(*keys).compact.uniq + end end def authenticatable? diff --git a/lib/devise/models.rb b/lib/devise/models.rb index fb7dd89b06..6ddb9427d8 100644 --- a/lib/devise/models.rb +++ b/lib/devise/models.rb @@ -120,3 +120,4 @@ def devise_modules_hook! end require 'devise/models/authenticatable' +require 'devise/models/two_factor_authenticatable' diff --git a/lib/devise/models/two_factor_authenticatable.rb b/lib/devise/models/two_factor_authenticatable.rb new file mode 100644 index 0000000000..370be111ab --- /dev/null +++ b/lib/devise/models/two_factor_authenticatable.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Devise + module Models + module TwoFactorAuthenticatable + extend ActiveSupport::Concern + + def self.required_fields(klass) + [] + end + + module ClassMethods + Devise::Models.config(self, :two_factor_methods) + + def two_factor_methods=(methods) + @two_factor_methods = methods + Array(methods).each do |method_name| + config = Devise.two_factor_method_configs[method_name] + raise "Unknown two-factor method: #{method_name}. " \ + "Did you call Devise.register_two_factor_method?" unless config + begin + require config[:model] + rescue LoadError + raise unless config[:model].camelize.safe_constantize + end + mod = config[:model].camelize.constantize + include mod + end + end + + def two_factor_modules + Array(two_factor_methods) + end + end + + def enabled_two_factors + self.class.two_factor_modules.select do |method_name| + send(:"#{method_name}_two_factor_enabled?") + end + end + + def two_factor_enabled? + enabled_two_factors.any? + end + end + end +end diff --git a/lib/devise/modules.rb b/lib/devise/modules.rb index d8cde834c1..b539cf87e7 100644 --- a/lib/devise/modules.rb +++ b/lib/devise/modules.rb @@ -13,6 +13,9 @@ # Other authentications d.add_module :omniauthable, controller: :omniauth_callbacks, route: :omniauth_callback + # Two-factor authentication + d.add_module :two_factor_authenticatable, controller: :two_factor, route: :two_factor + # Misc after routes = [nil, :new, :edit] d.add_module :recoverable, controller: :passwords, route: { password: routes } From 5817f4dc09c3f64d803dff5f5cac9eedee0c4400 Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Fri, 27 Mar 2026 19:24:05 -0300 Subject: [PATCH 2/6] feat: add TwoFactor base Warden strategy Provide a base strategy that handles shared 2FA boilerplate: finding the pending resource from session, calling verify_two_factor!, restoring remember_me, and cleaning up session state on success. Extensions subclass and implement valid? + verify_two_factor!. The base strategy returns valid? false to prevent accidental use. --- lib/devise/strategies/two_factor.rb | 50 +++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 lib/devise/strategies/two_factor.rb diff --git a/lib/devise/strategies/two_factor.rb b/lib/devise/strategies/two_factor.rb new file mode 100644 index 0000000000..fa96eece1f --- /dev/null +++ b/lib/devise/strategies/two_factor.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'devise/strategies/base' + +module Devise + module Strategies + class TwoFactor < Base + # The base strategy is never used directly — extensions subclass it. + def valid? + false + end + + def authenticate! + resource = find_pending_resource + return fail!(:two_factor_session_expired) unless resource + + verify_two_factor!(resource) + + unless halted? + restore_remember_me(resource) + resource.after_database_authentication + cleanup_two_factor_session! + success!(resource) + end + end + + # Extensions must override. Should call fail! with a specific + # message on failure — this halts execution and triggers recall. + def verify_two_factor!(resource) + raise NotImplementedError + end + + private + + def find_pending_resource + return unless session[:devise_two_factor_resource_id] + mapping.to.where(id: session[:devise_two_factor_resource_id]).first + end + + def restore_remember_me(resource) + resource.remember_me = session[:devise_two_factor_remember_me] if resource.respond_to?(:remember_me=) + end + + def cleanup_two_factor_session! + session.delete(:devise_two_factor_resource_id) + session.delete(:devise_two_factor_remember_me) + end + end + end +end From de55046ac8918cfabff45f3e2e50459c0acdb001 Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Fri, 27 Mar 2026 19:24:12 -0300 Subject: [PATCH 3/6] feat: add TwoFactorController with OmniAuth-style routes and URL helpers Add TwoFactorController following the OmniAuth callbacks pattern: a single controller with per-method new_ actions, a central POST create endpoint, and an ActiveSupport.on_load hook for extensions. Generate per-method challenge routes from mapping.to.two_factor_methods. Add generic URL helpers (new_two_factor_challenge_path, two_factor_path) included via engine initializer when 2FA methods are registered. --- .../devise/two_factor_controller.rb | 54 +++++++++++++++++++ lib/devise/rails.rb | 8 +++ lib/devise/rails/routes.rb | 19 +++++++ lib/devise/two_factor.rb | 7 +++ lib/devise/two_factor/url_helpers.rb | 27 ++++++++++ 5 files changed, 115 insertions(+) create mode 100644 app/controllers/devise/two_factor_controller.rb create mode 100644 lib/devise/two_factor.rb create mode 100644 lib/devise/two_factor/url_helpers.rb diff --git a/app/controllers/devise/two_factor_controller.rb b/app/controllers/devise/two_factor_controller.rb new file mode 100644 index 0000000000..7e4a2b777c --- /dev/null +++ b/app/controllers/devise/two_factor_controller.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +class Devise::TwoFactorController < DeviseController + prepend_before_action :require_no_authentication + prepend_before_action :ensure_sign_in_initiated + + # Extensions can inject custom actions or override defaults via on_load + ActiveSupport.run_load_hooks(:devise_two_factor_controller, self) + + # Auto-generate default new_ actions for each registered 2FA module. + # Extensions that injected a custom action via on_load won't be overwritten. + Devise.two_factor_method_configs.each_key do |mod| + unless method_defined?(:"new_#{mod}") + define_method(:"new_#{mod}") do + @resource = find_pending_resource + end + end + end + + # POST /users/two_factor + # All methods POST here. Warden picks the right strategy via valid?. + def create + self.resource = warden.authenticate!(auth_options) + set_flash_message!(:notice, :signed_in, scope: :"devise.sessions") + sign_in(resource_name, resource) + yield resource if block_given? + respond_with resource, location: after_sign_in_path_for(resource) + end + + protected + + def auth_options + resource = find_pending_resource + default_method = resource.enabled_two_factors.first + { scope: resource_name, recall: "#{controller_path}#new_#{default_method}" } + end + + def translation_scope + 'devise.two_factor' + end + + def find_pending_resource + return unless session[:devise_two_factor_resource_id] + resource_class.where(id: session[:devise_two_factor_resource_id]).first + end + + private + + def ensure_sign_in_initiated + return if session[:devise_two_factor_resource_id].present? + set_flash_message!(:alert, :sign_in_not_initiated, scope: :"devise.failure") + redirect_to new_session_path(resource_name) + end +end diff --git a/lib/devise/rails.rb b/lib/devise/rails.rb index b5738853fe..c3ffaf48f3 100644 --- a/lib/devise/rails.rb +++ b/lib/devise/rails.rb @@ -37,6 +37,14 @@ class Engine < ::Rails::Engine end end + initializer "devise.two_factor" do + config.after_initialize do + if Devise.two_factor_method_configs.any? + Devise.include_helpers(Devise::TwoFactor) + end + end + end + initializer "devise.secret_key" do |app| Devise.secret_key ||= app.secret_key_base diff --git a/lib/devise/rails/routes.rb b/lib/devise/rails/routes.rb index f43e62fea7..80a893171a 100644 --- a/lib/devise/rails/routes.rb +++ b/lib/devise/rails/routes.rb @@ -398,6 +398,25 @@ def devise_unlock(mapping, controllers) #:nodoc: end end + def devise_two_factor(mapping, controllers) #:nodoc: + return unless mapping.to.respond_to?(:two_factor_methods) && mapping.to.two_factor_methods.present? + + controller = controllers[:two_factor] || "devise/two_factor" + two_factor_path = mapping.path_names[:two_factor] || "two_factor" + + # Central POST endpoint — all methods submit here + post two_factor_path, + to: "#{controller}#create", + as: "two_factor" + + # Per-method challenge routes + Array(mapping.to.two_factor_methods).each do |method_name| + get "#{two_factor_path}/#{method_name}/new", + to: "#{controller}#new_#{method_name}", + as: "new_two_factor_#{method_name}" + end + end + def devise_registration(mapping, controllers) #:nodoc: path_names = { new: mapping.path_names[:sign_up], diff --git a/lib/devise/two_factor.rb b/lib/devise/two_factor.rb new file mode 100644 index 0000000000..4e948c131a --- /dev/null +++ b/lib/devise/two_factor.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Devise + module TwoFactor + autoload :UrlHelpers, "devise/two_factor/url_helpers" + end +end diff --git a/lib/devise/two_factor/url_helpers.rb b/lib/devise/two_factor/url_helpers.rb new file mode 100644 index 0000000000..23c0ab98dc --- /dev/null +++ b/lib/devise/two_factor/url_helpers.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Devise + module TwoFactor + module UrlHelpers + def new_two_factor_challenge_path(resource_or_scope, method, *args) + scope = Devise::Mapping.find_scope!(resource_or_scope) + _devise_route_context.send(:"#{scope}_new_two_factor_#{method}_path", *args) + end + + def new_two_factor_challenge_url(resource_or_scope, method, *args) + scope = Devise::Mapping.find_scope!(resource_or_scope) + _devise_route_context.send(:"#{scope}_new_two_factor_#{method}_url", *args) + end + + def two_factor_path(resource_or_scope, *args) + scope = Devise::Mapping.find_scope!(resource_or_scope) + _devise_route_context.send(:"#{scope}_two_factor_path", *args) + end + + def two_factor_url(resource_or_scope, *args) + scope = Devise::Mapping.find_scope!(resource_or_scope) + _devise_route_context.send(:"#{scope}_two_factor_url", *args) + end + end + end +end From 7bc147d6df944b81225989af9ff6c73fc60a2d9a Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Fri, 27 Mar 2026 19:24:19 -0300 Subject: [PATCH 4/6] feat: intercept DatabaseAuthenticatable and PasswordsController for 2FA Modify DatabaseAuthenticatable strategy to detect 2FA-enabled users after password validation and redirect to the default 2FA method's challenge page instead of signing in. Update PasswordsController to require 2FA verification after password reset when sign_in_after_reset_password is enabled. --- .../devise/passwords_controller.rb | 8 +++++++ .../strategies/database_authenticatable.rb | 24 ++++++++++++++++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/app/controllers/devise/passwords_controller.rb b/app/controllers/devise/passwords_controller.rb index 68b8dc8773..86d7db9503 100644 --- a/app/controllers/devise/passwords_controller.rb +++ b/app/controllers/devise/passwords_controller.rb @@ -37,6 +37,14 @@ def update if resource.errors.empty? resource.unlock_access! if unlockable?(resource) if sign_in_after_reset_password? + if resource.respond_to?(:two_factor_enabled?) && resource.two_factor_enabled? + session[:devise_two_factor_resource_id] = resource.id + default_method = resource.enabled_two_factors.first + set_flash_message!(:notice, :updated_two_factor_required) + respond_with resource, location: new_two_factor_challenge_path(resource_name, default_method) + return + end + flash_message = resource.active_for_authentication? ? :updated : :updated_not_active set_flash_message!(:notice, flash_message) resource.after_database_authentication diff --git a/lib/devise/strategies/database_authenticatable.rb b/lib/devise/strategies/database_authenticatable.rb index f7e007d144..50cb515f7f 100644 --- a/lib/devise/strategies/database_authenticatable.rb +++ b/lib/devise/strategies/database_authenticatable.rb @@ -11,9 +11,13 @@ def authenticate! hashed = false if validate(resource){ hashed = true; resource.valid_password?(password) } - remember_me(resource) - resource.after_database_authentication - success!(resource) + if resource.respond_to?(:two_factor_enabled?) && resource.two_factor_enabled? + initiate_two_factor_authentication!(resource) + else + remember_me(resource) + resource.after_database_authentication + success!(resource) + end end # In paranoid mode, hash the password even when a resource doesn't exist for the given authentication key. @@ -24,6 +28,20 @@ def authenticate! Devise.paranoid ? fail(:invalid) : fail(:not_found_in_database) end end + + private + + def initiate_two_factor_authentication!(resource) + session[:devise_two_factor_resource_id] = resource.id + session[:devise_two_factor_remember_me] = remember_me? + default_method = resource.enabled_two_factors.first + redirect!(new_two_factor_challenge_path(scope, default_method)) + end + + def new_two_factor_challenge_path(scope, method) + Rails.application.routes.url_helpers + .send(:"#{scope}_new_two_factor_#{method}_path") + end end end end From f9f22cf7bfe00f23e03f09e0a7156964337c1f72 Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Fri, 27 Mar 2026 19:24:31 -0300 Subject: [PATCH 5/6] feat: add two-factor view helpers and locale strings Add two_factor_method_links helper to DeviseHelper for rendering switch links between 2FA methods. Add locale strings for session expired, sign-in not initiated, and password reset 2FA required. --- app/helpers/devise_helper.rb | 4 ++++ app/views/devise/two_factor/_test_otp.html.erb | 4 ++++ app/views/devise/two_factor/_test_otp_link.html.erb | 1 + config/locales/en.yml | 3 +++ 4 files changed, 12 insertions(+) create mode 100644 app/views/devise/two_factor/_test_otp.html.erb create mode 100644 app/views/devise/two_factor/_test_otp_link.html.erb diff --git a/app/helpers/devise_helper.rb b/app/helpers/devise_helper.rb index 0bfcb06308..97457de8d2 100644 --- a/app/helpers/devise_helper.rb +++ b/app/helpers/devise_helper.rb @@ -2,4 +2,8 @@ # Keeping the helper around for backward compatibility. module DeviseHelper + def two_factor_method_links(resource, current_method) + methods = resource.enabled_two_factors - [current_method] + safe_join(methods.map { |method| render "devise/two_factor/#{method}_link" }) + end end diff --git a/app/views/devise/two_factor/_test_otp.html.erb b/app/views/devise/two_factor/_test_otp.html.erb new file mode 100644 index 0000000000..909fbd7350 --- /dev/null +++ b/app/views/devise/two_factor/_test_otp.html.erb @@ -0,0 +1,4 @@ +<%= form_tag(two_factor_path(resource_name), method: :post) do %> + <%= text_field_tag :otp_attempt %> + <%= submit_tag "Verify" %> +<% end %> diff --git a/app/views/devise/two_factor/_test_otp_link.html.erb b/app/views/devise/two_factor/_test_otp_link.html.erb new file mode 100644 index 0000000000..e3cd753fcd --- /dev/null +++ b/app/views/devise/two_factor/_test_otp_link.html.erb @@ -0,0 +1 @@ +<%= link_to "Use OTP instead", new_two_factor_challenge_path(resource_name, :test_otp) %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 260e1c4ba6..a16d8462eb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -16,6 +16,8 @@ en: timeout: "Your session expired. Please sign in again to continue." unauthenticated: "You need to sign in or sign up before continuing." unconfirmed: "You have to confirm your email address before continuing." + two_factor_session_expired: "Your two-factor authentication session has expired. Please sign in again." + sign_in_not_initiated: "Please sign in first." mailer: confirmation_instructions: subject: "Confirmation instructions" @@ -36,6 +38,7 @@ en: send_paranoid_instructions: "If your email address exists in our database, you will receive a password recovery link at your email address in a few minutes." updated: "Your password has been changed successfully. You are now signed in." updated_not_active: "Your password has been changed successfully." + updated_two_factor_required: "Your password has been changed successfully. Please complete two-factor authentication." registrations: destroyed: "Bye! Your account has been successfully cancelled. We hope to see you again soon." signed_up: "Welcome! You have signed up successfully." From 0a4c6683b392bc0489dd829320e3817b62437d9a Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Fri, 27 Mar 2026 19:24:38 -0300 Subject: [PATCH 6/6] test: add two-factor test infrastructure and integration tests Add test_otp 2FA method for integration testing with a simple OTP check. Add model tests for TwoFactorAuthenticatable, strategy tests for the base TwoFactor strategy, and end-to-end integration tests covering the full sign-in flow, failure recall, and URL helpers. --- test/devise_test.rb | 41 +++++++ test/helpers/devise_helper_test.rb | 12 ++ test/integration/two_factor_test.rb | 114 ++++++++++++++++++ test/models/serializable_test.rb | 2 +- .../models/two_factor_authenticatable_test.rb | 98 +++++++++++++++ .../app/active_record/user_with_two_factor.rb | 9 ++ .../app/mongoid/user_with_two_factor.rb | 14 +++ .../devise/two_factor/new_test_otp.html.erb | 6 + .../config/initializers/test_two_factor.rb | 42 +++++++ test/rails_app/config/routes.rb | 2 + .../20260303000000_add_otp_secret_to_users.rb | 7 ++ test/rails_app/db/schema.rb | 74 ++++++------ .../lib/shared_user_with_two_factor.rb | 12 ++ test/strategies/two_factor_test.rb | 21 ++++ 14 files changed, 414 insertions(+), 40 deletions(-) create mode 100644 test/integration/two_factor_test.rb create mode 100644 test/models/two_factor_authenticatable_test.rb create mode 100644 test/rails_app/app/active_record/user_with_two_factor.rb create mode 100644 test/rails_app/app/mongoid/user_with_two_factor.rb create mode 100644 test/rails_app/app/views/devise/two_factor/new_test_otp.html.erb create mode 100644 test/rails_app/config/initializers/test_two_factor.rb create mode 100644 test/rails_app/db/migrate/20260303000000_add_otp_secret_to_users.rb create mode 100644 test/rails_app/lib/shared_user_with_two_factor.rb create mode 100644 test/strategies/two_factor_test.rb diff --git a/test/devise_test.rb b/test/devise_test.rb index a46be0d527..3f8b4fdbc0 100644 --- a/test/devise_test.rb +++ b/test/devise_test.rb @@ -86,6 +86,47 @@ class DeviseTest < ActiveSupport::TestCase Devise::CONTROLLERS.delete(:kivi) end + test 'register_two_factor_method stores config and populates STRATEGIES' do + Devise.register_two_factor_method(:fake_2fa, model: 'devise/models/fake_2fa', strategy: :fake_2fa_strategy) + assert_equal({ model: 'devise/models/fake_2fa', strategy: :fake_2fa_strategy }, Devise.two_factor_method_configs[:fake_2fa]) + assert_equal :fake_2fa_strategy, Devise::STRATEGIES[:fake_2fa] + ensure + Devise.two_factor_method_configs.delete(:fake_2fa) + Devise::STRATEGIES.delete(:fake_2fa) + end + + test 'register_two_factor_method with controller populates CONTROLLERS' do + Devise.register_two_factor_method(:fake_mgmt, model: 'x', controller: :fake_mgmt) + assert_equal :fake_mgmt, Devise::CONTROLLERS[:fake_mgmt] + ensure + Devise.two_factor_method_configs.delete(:fake_mgmt) + Devise::CONTROLLERS.delete(:fake_mgmt) + end + + test 'register_two_factor_method with route populates ROUTES and URL_HELPERS' do + Devise.register_two_factor_method(:fake_rt, model: 'x', route: { fake_rt: [nil, :new] }) + assert_equal :fake_rt, Devise::ROUTES[:fake_rt] + assert_equal [nil, :new], Devise::URL_HELPERS[:fake_rt] + ensure + Devise.two_factor_method_configs.delete(:fake_rt) + Devise::ROUTES.delete(:fake_rt) + Devise::URL_HELPERS.delete(:fake_rt) + end + + test 'register_two_factor_method rejects unknown options' do + assert_raises(ArgumentError) do + Devise.register_two_factor_method(:bad, model: 'x', unknown: true) + end + end + + test 'add_module no longer accepts two_factor option' do + assert_raises(ArgumentError) do + Devise.add_module(:test_mod, two_factor: true) + end + ensure + Devise::ALL.delete(:test_mod) + end + test 'Devise.secure_compare fails when comparing different strings or nil' do [nil, ""].each do |empty| assert_not Devise.secure_compare(empty, "something") diff --git a/test/helpers/devise_helper_test.rb b/test/helpers/devise_helper_test.rb index b9fac7da37..98bcec60cb 100644 --- a/test/helpers/devise_helper_test.rb +++ b/test/helpers/devise_helper_test.rb @@ -44,4 +44,16 @@ class DeviseHelperTest < Devise::IntegrationTest assert_have_selector '#error_explanation' assert_contain "Can't save the user because of 2 errors" end + + test 'two_factor_method_links returns empty string when no other methods' do + resource = mock('resource') + resource.stubs(:enabled_two_factors).returns([:test_two_factor]) + + helper = Class.new(ActionView::Base) do + include DeviseHelper + end.new(ActionView::LookupContext.new([]), {}, nil) + + result = helper.two_factor_method_links(resource, :test_two_factor) + assert_equal '', result + end end diff --git a/test/integration/two_factor_test.rb b/test/integration/two_factor_test.rb new file mode 100644 index 0000000000..3008429697 --- /dev/null +++ b/test/integration/two_factor_test.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require 'test_helper' + +class TwoFactorAuthenticationTest < Devise::IntegrationTest + test 'sign in redirects to two factor challenge when 2FA is enabled' do + user = create_user_with_two_factor(otp_secret: '123456') + + visit new_user_with_two_factor_session_path + fill_in 'email', with: user.email + fill_in 'password', with: '12345678' + click_button 'Log In' + + assert_not warden.authenticated?(:user_with_two_factor) + assert_equal user.id, session[:devise_two_factor_resource_id] + end + + test 'sign in without 2FA enabled proceeds normally' do + user = create_user_with_two_factor(otp_secret: nil) + + visit new_user_with_two_factor_session_path + fill_in 'email', with: user.email + fill_in 'password', with: '12345678' + click_button 'Log In' + + assert warden.authenticated?(:user_with_two_factor) + assert_nil session[:devise_two_factor_resource_id] + end + + test 'password reset with 2FA enabled redirects to two factor challenge' do + user = create_user_with_two_factor(otp_secret: '123456') + raw_token = user.send_reset_password_instructions + + visit edit_user_with_two_factor_password_path(reset_password_token: raw_token) + fill_in 'New password', with: 'newpassword123' + fill_in 'Confirm new password', with: 'newpassword123' + click_button 'Change my password' + + assert_not warden.authenticated?(:user_with_two_factor) + assert session[:devise_two_factor_resource_id] + end + + test 'password reset without 2FA signs in directly' do + user = create_user_with_two_factor(otp_secret: nil) + raw_token = user.send_reset_password_instructions + + visit edit_user_with_two_factor_password_path(reset_password_token: raw_token) + fill_in 'New password', with: 'newpassword123' + fill_in 'Confirm new password', with: 'newpassword123' + click_button 'Change my password' + + assert warden.authenticated?(:user_with_two_factor) + end + + test 'two-factor routes generate correct paths' do + assert_equal '/user_with_two_factors/two_factor/test_otp/new', + user_with_two_factor_new_two_factor_test_otp_path + assert_equal '/user_with_two_factors/two_factor', + user_with_two_factor_two_factor_path + end + + test 'full two-factor sign-in: password -> challenge -> OTP -> authenticated' do + user = create_user_with_two_factor(otp_secret: '123456') + + # Step 1: Submit password + post user_with_two_factor_session_path, params: { + user_with_two_factor: { email: user.email, password: '12345678' } + } + + # Step 2: Redirected to the default 2FA method's challenge page + assert_redirected_to user_with_two_factor_new_two_factor_test_otp_path + follow_redirect! + assert_response :success + + # Step 3: Submit correct OTP + post user_with_two_factor_two_factor_path, params: { + otp_attempt: user.otp_secret + } + + # Step 4: Authenticated and redirected to after_sign_in_path + assert_response :redirect + assert warden.authenticated?(:user_with_two_factor) + end + + test 'two-factor sign-in with wrong OTP recalls challenge page' do + user = create_user_with_two_factor(otp_secret: '123456') + + post user_with_two_factor_session_path, params: { + user_with_two_factor: { email: user.email, password: '12345678' } + } + assert_redirected_to user_with_two_factor_new_two_factor_test_otp_path + + # Submit wrong OTP + post user_with_two_factor_two_factor_path, params: { + otp_attempt: 'wrong' + } + + # Should recall (re-render) the challenge page, not redirect + assert_response :success + assert_not warden.authenticated?(:user_with_two_factor) + end + + private + + def create_user_with_two_factor(attributes = {}) + UserWithTwoFactor.create!( + username: 'usertest', + email: generate_unique_email, + password: '12345678', + password_confirmation: '12345678', + **attributes + ) + end +end diff --git a/test/models/serializable_test.rb b/test/models/serializable_test.rb index 024ccf4497..52c5fcccdf 100644 --- a/test/models/serializable_test.rb +++ b/test/models/serializable_test.rb @@ -9,7 +9,7 @@ class SerializableTest < ActiveSupport::TestCase test 'should not include unsafe keys on JSON' do keys = from_json().keys.select{ |key| !key.include?("id") } - assert_equal %w(created_at email facebook_token updated_at username), keys.sort + assert_equal %w(created_at email facebook_token otp_secret updated_at username), keys.sort end test 'should not include unsafe keys on JSON even if a new except is provided' do diff --git a/test/models/two_factor_authenticatable_test.rb b/test/models/two_factor_authenticatable_test.rb new file mode 100644 index 0000000000..d305c09ce9 --- /dev/null +++ b/test/models/two_factor_authenticatable_test.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require 'test_helper' + +class TwoFactorAuthenticatableTest < ActiveSupport::TestCase + test '.two_factor_modules returns the configured two_factor_methods' do + klass = Class.new do + extend Devise::Models::TwoFactorAuthenticatable::ClassMethods + end + klass.instance_variable_set(:@two_factor_methods, [:fake_method]) + + assert_equal [:fake_method], klass.two_factor_modules + end + + test '.two_factor_modules returns empty array when no methods configured' do + klass = Class.new do + extend Devise::Models::TwoFactorAuthenticatable::ClassMethods + end + + assert_equal [], klass.two_factor_modules + end + + test '#two_factor_enabled? returns true when any method reports enabled' do + klass = Class.new do + include Devise::Models::TwoFactorAuthenticatable + end + klass.instance_variable_set(:@two_factor_methods, [:fake_method]) + + instance = klass.new + instance.define_singleton_method(:fake_method_two_factor_enabled?) { true } + + assert instance.two_factor_enabled? + assert_equal [:fake_method], instance.enabled_two_factors + end + + test '#two_factor_enabled? returns false when no method reports enabled' do + klass = Class.new do + include Devise::Models::TwoFactorAuthenticatable + end + klass.instance_variable_set(:@two_factor_methods, [:fake_method]) + + instance = klass.new + instance.define_singleton_method(:fake_method_two_factor_enabled?) { false } + + assert_not instance.two_factor_enabled? + assert_empty instance.enabled_two_factors + end + + test '#enabled_two_factors returns only enabled methods' do + klass = Class.new do + include Devise::Models::TwoFactorAuthenticatable + end + klass.instance_variable_set(:@two_factor_methods, [:method_a, :method_b]) + + instance = klass.new + instance.define_singleton_method(:method_a_two_factor_enabled?) { true } + instance.define_singleton_method(:method_b_two_factor_enabled?) { false } + + assert_equal [:method_a], instance.enabled_two_factors + end + + test '.two_factor_methods= raises on unknown method' do + klass = Class.new do + extend Devise::Models::TwoFactorAuthenticatable::ClassMethods + end + + assert_raises(RuntimeError, /Unknown two-factor method/) do + klass.two_factor_methods = [:nonexistent] + end + end + + test '.two_factor_methods= includes model concern from registry' do + # Register a fake method + Devise.register_two_factor_method(:includable_test, + model: 'devise/models/test_otp', + strategy: :test_strategy) + + klass = Class.new do + extend Devise::Models::TwoFactorAuthenticatable::ClassMethods + + # Stub include to track what gets included + def self.included_modules_tracker + @included_modules_tracker ||= [] + end + + def self.include(mod) + included_modules_tracker << mod + super + end + end + + klass.two_factor_methods = [:includable_test] + assert_equal [:includable_test], Array(klass.instance_variable_get(:@two_factor_methods)) + ensure + Devise.two_factor_method_configs.delete(:includable_test) + Devise::STRATEGIES.delete(:includable_test) + end +end diff --git a/test/rails_app/app/active_record/user_with_two_factor.rb b/test/rails_app/app/active_record/user_with_two_factor.rb new file mode 100644 index 0000000000..b3fd42f669 --- /dev/null +++ b/test/rails_app/app/active_record/user_with_two_factor.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'shared_user_with_two_factor' + +class UserWithTwoFactor < ActiveRecord::Base + self.table_name = 'users' + include Shim + include SharedUserWithTwoFactor +end diff --git a/test/rails_app/app/mongoid/user_with_two_factor.rb b/test/rails_app/app/mongoid/user_with_two_factor.rb new file mode 100644 index 0000000000..7e598e9c7c --- /dev/null +++ b/test/rails_app/app/mongoid/user_with_two_factor.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'shared_user_with_two_factor' + +class UserWithTwoFactor + include Mongoid::Document + include Shim + include SharedUserWithTwoFactor + + field :username, type: String + field :email, type: String, default: "" + field :encrypted_password, type: String, default: "" + field :otp_secret, type: String +end diff --git a/test/rails_app/app/views/devise/two_factor/new_test_otp.html.erb b/test/rails_app/app/views/devise/two_factor/new_test_otp.html.erb new file mode 100644 index 0000000000..e19e6a2828 --- /dev/null +++ b/test/rails_app/app/views/devise/two_factor/new_test_otp.html.erb @@ -0,0 +1,6 @@ +

Two-Factor Authentication

+ +<%= form_tag(two_factor_path(resource_name), method: :post) do %> + <%= text_field_tag :otp_attempt %> + <%= submit_tag "Verify" %> +<% end %> diff --git a/test/rails_app/config/initializers/test_two_factor.rb b/test/rails_app/config/initializers/test_two_factor.rb new file mode 100644 index 0000000000..9e972fd9c9 --- /dev/null +++ b/test/rails_app/config/initializers/test_two_factor.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +# Test-only two-factor method for integration testing. +# Simulates a real 2FA extension with a simple OTP check. + +require 'devise/models/two_factor_authenticatable' + +module Devise + module Models + module TestOtp + extend ActiveSupport::Concern + + def test_otp_two_factor_enabled? + respond_to?(:otp_secret) && otp_secret.present? + end + end + end +end + +module Devise + module Strategies + class TestOtp < Devise::Strategies::TwoFactor + def valid? + params[:otp_attempt].present? && + session[:devise_two_factor_resource_id].present? + end + + def verify_two_factor!(resource) + unless resource.respond_to?(:otp_secret) && params[:otp_attempt] == resource.otp_secret + fail!(:invalid_otp) + return + end + end + end + end +end + +Warden::Strategies.add(:test_otp, Devise::Strategies::TestOtp) + +Devise.register_two_factor_method :test_otp, + model: 'devise/models/test_otp', + strategy: :test_otp diff --git a/test/rails_app/config/routes.rb b/test/rails_app/config/routes.rb index 0b748f3fd7..87ccfd1a32 100644 --- a/test/rails_app/config/routes.rb +++ b/test/rails_app/config/routes.rb @@ -22,6 +22,8 @@ # Users scope devise_for :users, controllers: { omniauth_callbacks: "users/omniauth_callbacks" } + devise_for :user_with_two_factors + devise_for :user_on_main_apps, class_name: 'UserOnMainApp', router_name: :main_app, diff --git a/test/rails_app/db/migrate/20260303000000_add_otp_secret_to_users.rb b/test/rails_app/db/migrate/20260303000000_add_otp_secret_to_users.rb new file mode 100644 index 0000000000..4e1e716987 --- /dev/null +++ b/test/rails_app/db/migrate/20260303000000_add_otp_secret_to_users.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddOtpSecretToUsers < ActiveRecord::Migration[6.0] + def change + add_column :users, :otp_secret, :string + end +end diff --git a/test/rails_app/db/schema.rb b/test/rails_app/db/schema.rb index c435f6b96e..b18d970e6d 100644 --- a/test/rails_app/db/schema.rb +++ b/test/rails_app/db/schema.rb @@ -1,57 +1,53 @@ -# encoding: UTF-8 -# frozen_string_literal: true - # This file is auto-generated from the current state of the database. Instead # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. # -# Note that this schema.rb definition is the authoritative source for your -# database schema. If you need to create the application database on another -# system, you should be using db:schema:load, not running all the migrations -# from scratch. The latter is a flawed and unsustainable approach (the more migrations -# you'll amass, the slower it'll run and the greater likelihood for issues). +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20100401102949) do - - create_table "admins", force: true do |t| - t.string "email" - t.string "encrypted_password" - t.string "reset_password_token" - t.datetime "reset_password_sent_at" - t.datetime "remember_created_at" - t.string "confirmation_token" - t.datetime "confirmed_at" +ActiveRecord::Schema[8.1].define(version: 2026_03_03_000000) do + create_table "admins", force: :cascade do |t| + t.boolean "active", default: false t.datetime "confirmation_sent_at" - t.string "unconfirmed_email" - t.datetime "locked_at" - t.boolean "active", default: false + t.string "confirmation_token" + t.datetime "confirmed_at" t.datetime "created_at" + t.string "email" + t.string "encrypted_password" + t.datetime "locked_at" + t.datetime "remember_created_at" + t.datetime "reset_password_sent_at" + t.string "reset_password_token" + t.string "unconfirmed_email" t.datetime "updated_at" end - create_table "users", force: true do |t| - t.string "username" - t.string "facebook_token" - t.string "email", default: "", null: false - t.string "encrypted_password", default: "", null: false - t.string "reset_password_token" - t.datetime "reset_password_sent_at" - t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0 + create_table "users", force: :cascade do |t| + t.datetime "confirmation_sent_at" + t.string "confirmation_token" + t.datetime "confirmed_at" + t.datetime "created_at" t.datetime "current_sign_in_at" + t.string "current_sign_in_ip" + t.string "email", default: "", null: false + t.string "encrypted_password", default: "", null: false + t.string "facebook_token" + t.integer "failed_attempts", default: 0 t.datetime "last_sign_in_at" - t.string "current_sign_in_ip" - t.string "last_sign_in_ip" - t.string "confirmation_token" - t.datetime "confirmed_at" - t.datetime "confirmation_sent_at" - t.integer "failed_attempts", default: 0 - t.string "unlock_token" + t.string "last_sign_in_ip" t.datetime "locked_at" - t.datetime "created_at" + t.string "otp_secret" + t.datetime "remember_created_at" + t.datetime "reset_password_sent_at" + t.string "reset_password_token" + t.integer "sign_in_count", default: 0 + t.string "unlock_token" t.datetime "updated_at" + t.string "username" end - end diff --git a/test/rails_app/lib/shared_user_with_two_factor.rb b/test/rails_app/lib/shared_user_with_two_factor.rb new file mode 100644 index 0000000000..253326cd1b --- /dev/null +++ b/test/rails_app/lib/shared_user_with_two_factor.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module SharedUserWithTwoFactor + extend ActiveSupport::Concern + + included do + devise :database_authenticatable, :registerable, :recoverable, + :two_factor_authenticatable, two_factor_methods: [:test_otp] + + validates_uniqueness_of :email, allow_blank: true, if: :devise_will_save_change_to_email? + end +end diff --git a/test/strategies/two_factor_test.rb b/test/strategies/two_factor_test.rb new file mode 100644 index 0000000000..1224bff145 --- /dev/null +++ b/test/strategies/two_factor_test.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'test_helper' + +class TwoFactorStrategyTest < ActiveSupport::TestCase + test 'TwoFactor strategy can be loaded' do + assert defined?(Devise::Strategies::TwoFactor) + end + + test 'TwoFactor base strategy is never valid' do + strategy = Devise::Strategies::TwoFactor.new(nil) + assert_equal false, strategy.valid? + end + + test 'verify_two_factor! raises NotImplementedError by default' do + strategy = Devise::Strategies::TwoFactor.new(nil) + assert_raises(NotImplementedError) do + strategy.verify_two_factor!(Object.new) + end + end +end