diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index 5d594d2f52..3fc1d09820 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -19,4 +19,4 @@ This code of conduct applies both within project spaces and in public spaces whe Instances of abusive, harassing, or otherwise unacceptable behavior may be reported by sending an email to [heartcombo.oss@gmail.com](heartcombo.oss@gmail.com) or contacting one or more of the project maintainers. -This Code of Conduct is adapted from the [Contributor Covenant](http://contributor-covenant.org), version 1.2.0, available at [http://contributor-covenant.org/version/1/2/0/](http://contributor-covenant.org/version/1/2/0/) +This Code of Conduct is adapted from the [Contributor Covenant](https://contributor-covenant.org), version 1.2.0, available at [https://contributor-covenant.org/version/1/2/0/](https://contributor-covenant.org/version/1/2/0/) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 336d614f40..6327e4ddb7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,7 +27,7 @@ internationalization. Avoid opening new issues to ask questions in our issues tracker. Please go through the project wiki, documentation and source code first, or try to ask your question -on [Stack Overflow](http://stackoverflow.com/questions/tagged/devise). +on [Stack Overflow](https://stackoverflow.com/questions/tagged/devise). **If you find a security bug, do not report it through GitHub. Please send an e-mail to [heartcombo.oss@gmail.com](mailto:heartcombo.oss@gmail.com) diff --git a/Gemfile.lock b/Gemfile.lock index 8b339d7add..305f561c10 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -21,31 +21,31 @@ PATH GEM remote: https://rubygems.org/ specs: - action_text-trix (2.1.17) + action_text-trix (2.1.18) railties - actioncable (8.1.2) - actionpack (= 8.1.2) - activesupport (= 8.1.2) + actioncable (8.1.3) + actionpack (= 8.1.3) + activesupport (= 8.1.3) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (8.1.2) - actionpack (= 8.1.2) - activejob (= 8.1.2) - activerecord (= 8.1.2) - activestorage (= 8.1.2) - activesupport (= 8.1.2) + actionmailbox (8.1.3) + actionpack (= 8.1.3) + activejob (= 8.1.3) + activerecord (= 8.1.3) + activestorage (= 8.1.3) + activesupport (= 8.1.3) mail (>= 2.8.0) - actionmailer (8.1.2) - actionpack (= 8.1.2) - actionview (= 8.1.2) - activejob (= 8.1.2) - activesupport (= 8.1.2) + actionmailer (8.1.3) + actionpack (= 8.1.3) + actionview (= 8.1.3) + activejob (= 8.1.3) + activesupport (= 8.1.3) mail (>= 2.8.0) rails-dom-testing (~> 2.2) - actionpack (8.1.2) - actionview (= 8.1.2) - activesupport (= 8.1.2) + actionpack (8.1.3) + actionview (= 8.1.3) + activesupport (= 8.1.3) nokogiri (>= 1.8.5) rack (>= 2.2.4) rack-session (>= 1.0.1) @@ -53,36 +53,36 @@ GEM rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) - actiontext (8.1.2) + actiontext (8.1.3) action_text-trix (~> 2.1.15) - actionpack (= 8.1.2) - activerecord (= 8.1.2) - activestorage (= 8.1.2) - activesupport (= 8.1.2) + actionpack (= 8.1.3) + activerecord (= 8.1.3) + activestorage (= 8.1.3) + activesupport (= 8.1.3) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (8.1.2) - activesupport (= 8.1.2) + actionview (8.1.3) + activesupport (= 8.1.3) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - activejob (8.1.2) - activesupport (= 8.1.2) + activejob (8.1.3) + activesupport (= 8.1.3) globalid (>= 0.3.6) - activemodel (8.1.2) - activesupport (= 8.1.2) - activerecord (8.1.2) - activemodel (= 8.1.2) - activesupport (= 8.1.2) + activemodel (8.1.3) + activesupport (= 8.1.3) + activerecord (8.1.3) + activemodel (= 8.1.3) + activesupport (= 8.1.3) timeout (>= 0.4.0) - activestorage (8.1.2) - actionpack (= 8.1.2) - activejob (= 8.1.2) - activerecord (= 8.1.2) - activesupport (= 8.1.2) + activestorage (8.1.3) + actionpack (= 8.1.3) + activejob (= 8.1.3) + activerecord (= 8.1.3) + activesupport (= 8.1.3) marcel (~> 1.0) - activesupport (8.1.2) + activesupport (8.1.3) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.3.1) @@ -96,7 +96,7 @@ GEM tzinfo (~> 2.0, >= 2.0.5) uri (>= 0.13.1) base64 (0.3.0) - bcrypt (3.1.21) + bcrypt (3.1.22) bigdecimal (4.0.1) bson (5.2.0) builder (3.3.0) @@ -125,11 +125,11 @@ GEM prism (>= 1.3.0) rdoc (>= 4.0.0) reline (>= 0.4.2) - json (2.19.1) + json (2.19.3) jwt (3.1.2) base64 logger (1.7.0) - loofah (2.25.0) + loofah (2.25.1) crass (~> 1.0.2) nokogiri (>= 1.12.0) mail (2.9.0) @@ -161,7 +161,7 @@ GEM net-smtp (0.5.1) net-protocol nio4r (2.7.5) - nokogiri (1.19.1) + nokogiri (1.19.2) mini_portile2 (~> 2.8.2) racc (~> 1.4) oauth2 (2.0.18) @@ -213,20 +213,20 @@ GEM rack (>= 1.3) rackup (2.3.1) rack (>= 3) - rails (8.1.2) - actioncable (= 8.1.2) - actionmailbox (= 8.1.2) - actionmailer (= 8.1.2) - actionpack (= 8.1.2) - actiontext (= 8.1.2) - actionview (= 8.1.2) - activejob (= 8.1.2) - activemodel (= 8.1.2) - activerecord (= 8.1.2) - activestorage (= 8.1.2) - activesupport (= 8.1.2) + rails (8.1.3) + actioncable (= 8.1.3) + actionmailbox (= 8.1.3) + actionmailer (= 8.1.3) + actionpack (= 8.1.3) + actiontext (= 8.1.3) + actionview (= 8.1.3) + activejob (= 8.1.3) + activemodel (= 8.1.3) + activerecord (= 8.1.3) + activestorage (= 8.1.3) + activesupport (= 8.1.3) bundler (>= 1.15.0) - railties (= 8.1.2) + railties (= 8.1.3) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) actionview (>= 5.0.1.rc1) @@ -238,9 +238,9 @@ GEM rails-html-sanitizer (1.7.0) loofah (~> 2.25) nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) - railties (8.1.2) - actionpack (= 8.1.2) - activesupport (= 8.1.2) + railties (8.1.3) + actionpack (= 8.1.3) + activesupport (= 8.1.3) irb (~> 1.13) rackup (>= 1.0.0) rake (>= 12.2) @@ -264,7 +264,7 @@ GEM snaky_hash (2.0.3) hashie (>= 0.1.0, < 6) version_gem (>= 1.1.8, < 3) - sqlite3 (2.9.1) + sqlite3 (2.9.2) mini_portile2 (~> 2.8.0) stringio (3.2.0) thor (1.5.0) diff --git a/README.md b/README.md index 0c4278de33..37e2d31601 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ If you have discovered a security related bug, please do *NOT* use the GitHub is If you have any questions, comments, or concerns, please use StackOverflow instead of the GitHub issue tracker: -http://stackoverflow.com/questions/tagged/devise +https://stackoverflow.com/questions/tagged/devise The deprecated mailing lists can still be read on: @@ -90,7 +90,7 @@ https://groups.google.com/group/heartcombo You can view the Devise documentation in RDoc format here: -http://rubydoc.info/github/heartcombo/devise/main/frames +https://rubydoc.info/github/heartcombo/devise/main/frames If you need to use Devise with previous versions of Rails, you can always run "gem server" from the command line after you install the gem to access the old documentation. @@ -745,7 +745,7 @@ config.http_authenticatable = [:database] ``` This restriction does not limit you from implementing custom warden strategies, either in your application or via gem-based extensions for devise. -A common authentication strategy for APIs is token-based authentication. For more information on extending devise to support this type of authentication and others, see the wiki article for [Simple Token Authentication Examples and alternatives](https://github.com/heartcombo/devise/wiki/How-To:-Simple-Token-Authentication-Example#alternatives) or this blog post on [Custom authentication methods with Devise](http://blog.plataformatec.com.br/2019/01/custom-authentication-methods-with-devise/). +A common authentication strategy for APIs is token-based authentication. For more information on extending devise to support this type of authentication and others, see the wiki article for [Simple Token Authentication Examples and alternatives](https://github.com/heartcombo/devise/wiki/How-To:-Simple-Token-Authentication-Example#alternatives) or this blog post on [Custom authentication methods with Devise](https://blog.plataformatec.com.br/2019/01/custom-authentication-methods-with-devise/). #### Testing API Mode changes the order of the middleware stack, and this can cause problems for `Devise::Test::IntegrationHelpers`. This problem usually surfaces as an ```undefined method `[]=' for nil:NilClass``` error when using integration test helpers, such as `#sign_in`. The solution is simply to reorder the middlewares by adding the following to test.rb: 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/app/controllers/devise/two_factor_controller.rb b/app/controllers/devise/two_factor_controller.rb new file mode 100644 index 0000000000..f14e1ed104 --- /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/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/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." diff --git a/lib/devise.rb b/lib/devise.rb index 8e0c85e77d..ba85f6faa9 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 @@ -312,6 +314,14 @@ def self.mappings mattr_accessor :sign_in_after_change_password @@sign_in_after_change_password = true + # 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 + @@two_factor_method_configs = {} + # Default way to set up Devise. Run rails generate devise_install to create # a fresh initializer with all configuration values. def self.setup @@ -439,6 +449,59 @@ def self.add_module(module_name, options = {}) Devise::Mapping.add_module module_name end + + # Register available devise two factor methods. + # Third-party modules that intend to add a 2FA method need to be added explicitly using this method. + # + # Note that adding a module using this method does not cause it to be used in the authentication + # process. That requires the `:two_factor_authenticatable` module to be listed in the arguments passed + # to the 'devise' method in the model class definition along with the two factor method name listed under + # the `:two_factor_methods` argument passed to the 'devise' method. + # + # == Options: + # + # +name+ - String representing the name of the 2FA method. This will be used to identify it. + # +model+ - String representing the load path to a custom *model* for this 2FA method (to autoload.) + # +strategy+ - Symbol representing if this module got a custom *strategy*. + # +route+ - Generates extension-specific routes and URL helpers (e.g., credential management + # endpoints). This is separate from the core challenge/create routes that Devise + # generates automatically from +two_factor_methods+. Accepts true (defaults route + # name to the method name), a Symbol, or a Hash. Works the same as the +:route+ + # option in +add_module+. + # + # == Examples: + # + # Devise.register_two_factor_method(:my_two_factor_method) + # Devise.register_two_factor_method(:my_two_factor_method, model: 'my_two_factor_method/model') + # Devise.register_two_factor_method(:my_two_factor_method, model: 'my_two_factor_method/model', strategy: :my_two_factor_method, route: true) + # + def self.register_two_factor_method(name, options = {}) + options.assert_valid_keys(:model, :strategy, :route) + + two_factor_method_configs[name.to_sym] = options + + STRATEGIES[name.to_sym] = options[:strategy] if options[:strategy] + + 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 } 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/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 diff --git a/lib/devise/strategies/two_factor.rb b/lib/devise/strategies/two_factor.rb new file mode 100644 index 0000000000..763b1c210e --- /dev/null +++ b/lib/devise/strategies/two_factor.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'devise/strategies/base' + +module Devise + module Strategies + class TwoFactor < Base + def valid? + session[:devise_two_factor_resource_id].present? + 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 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 diff --git a/test/devise_test.rb b/test/devise_test.rb index a46be0d527..6eafe9ab9a 100644 --- a/test/devise_test.rb +++ b/test/devise_test.rb @@ -86,6 +86,29 @@ 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] + Devise.two_factor_method_configs.delete(:fake_2fa) + Devise::STRATEGIES.delete(:fake_2fa) + 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] + 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 '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..4b5bb07992 100644 --- a/test/helpers/devise_helper_test.rb +++ b/test/helpers/devise_helper_test.rb @@ -44,4 +44,33 @@ 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 + + test 'two_factor_method_links renders link partials for other enabled methods' do + resource = mock('resource') + resource.stubs(:enabled_two_factors).returns([:webauthn, :totp, :backup_codes]) + + helper = Class.new(ActionView::Base) do + include DeviseHelper + end.new(ActionView::LookupContext.new([]), {}, nil) + + helper.stubs(:render).with("devise/two_factor/totp_link").returns('Use TOTP'.html_safe) + helper.stubs(:render).with("devise/two_factor/backup_codes_link").returns('Use backup codes'.html_safe) + + result = helper.two_factor_method_links(resource, :webauthn) + assert_includes result, "Use TOTP" + assert_includes result, "Use backup codes" + assert_not_includes result, "webauthn" + end end diff --git a/test/integration/two_factor_test.rb b/test/integration/two_factor_test.rb new file mode 100644 index 0000000000..9cab486efe --- /dev/null +++ b/test/integration/two_factor_test.rb @@ -0,0 +1,139 @@ +# 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 + + test 'two-factor sign-in with expired session does not authenticate' 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 + + # Simulate session expiration between password and OTP submission + reset! + + post user_with_two_factor_two_factor_path, params: { + otp_attempt: user.otp_secret + } + + assert_not warden.authenticated?(:user_with_two_factor) + end + + test 'visiting two-factor challenge page without sign-in redirects to login' do + get user_with_two_factor_new_two_factor_test_otp_path + + assert_redirected_to new_user_with_two_factor_session_path + 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..ddb25e07c9 --- /dev/null +++ b/test/models/two_factor_authenticatable_test.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'test_helper' + +class TwoFactorAuthenticatableTest < ActiveSupport::TestCase + test '.two_factor_modules returns the configured two_factor_methods' do + assert_equal [:test_otp], UserWithTwoFactor.two_factor_modules + end + + test '#two_factor_enabled? returns true when any method reports enabled' do + user = new_user_with_two_factor + user.stubs(:test_otp_two_factor_enabled?).returns(true) + assert user.two_factor_enabled? + assert_equal [:test_otp], user.enabled_two_factors + end + + test '#two_factor_enabled? returns false when no method reports enabled' do + user = new_user_with_two_factor + user.stubs(:test_otp_two_factor_enabled?).returns(false) + assert_not user.two_factor_enabled? + assert_empty user.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 + + private + + def new_user_with_two_factor(attributes = {}) + UserWithTwoFactor.new(valid_attributes(attributes)) + 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..83468a555b --- /dev/null +++ b/test/rails_app/config/initializers/test_two_factor.rb @@ -0,0 +1,41 @@ +# 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? + super && params[:otp_attempt].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..6fb8f219d0 100644 --- a/test/rails_app/db/schema.rb +++ b/test/rails_app/db/schema.rb @@ -13,7 +13,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20100401102949) do +ActiveRecord::Schema.define(version: 20260303000000) do create_table "admins", force: true do |t| t.string "email" @@ -50,6 +50,7 @@ t.integer "failed_attempts", default: 0 t.string "unlock_token" t.datetime "locked_at" + t.string "otp_secret" t.datetime "created_at" t.datetime "updated_at" 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..02679cb116 --- /dev/null +++ b/test/strategies/two_factor_test.rb @@ -0,0 +1,27 @@ +# 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 not valid without a pending session' do + strategy = Devise::Strategies::TwoFactor.new(env_with_session) + assert_not strategy.valid? + end + + test 'verify_two_factor! raises NotImplementedError by default' do + strategy = Devise::Strategies::TwoFactor.new(env_with_session) + assert_raises(NotImplementedError) do + strategy.verify_two_factor!(Object.new) + end + end + + private + + def env_with_session(session = {}) + { 'rack.session' => session } + end +end