From 56c06b47906d0ed5032312217c74b23109dd7dec Mon Sep 17 00:00:00 2001 From: Kai Lieth Date: Wed, 24 Jun 2026 13:17:55 -0700 Subject: [PATCH] Allow early configuration of devise models As of Rails 8, which enables lazy-loaded routes by default, Devise configuration particularly in test environments consistently breaks because tests begin running before routes have been loaded. This causes Devise (and therefore Warden) to operate with only partial configuration, particularly, a stale default_scope among other things. This change includes a new method to create a skeleton Mapping object which can be used to indicate a Devise-enabled model class that will be used for authentication before the route loading step has been completed. This object will have its configuration finalized once routes have been completed, and the Devise routes for that object are properly drawn. This is backwards compatible with the current route-centric configuration. --- lib/devise.rb | 32 +++++++---- lib/devise/mapping.rb | 21 ++++++-- lib/devise/rails.rb | 5 ++ lib/devise/rails/devise_model.rb | 25 +++++++++ lib/devise/rails/routes.rb | 2 +- test/mapping_test.rb | 56 ++++++++++++++++++++ test/rails_app/config/initializers/devise.rb | 6 +++ test/routes_test.rb | 51 ++++++++++++++++++ test/test_helper.rb | 5 ++ 9 files changed, 188 insertions(+), 15 deletions(-) create mode 100644 lib/devise/rails/devise_model.rb diff --git a/lib/devise.rb b/lib/devise.rb index 8e0c85e77d..2e3f7ce705 100644 --- a/lib/devise.rb +++ b/lib/devise.rb @@ -273,14 +273,8 @@ module Test # PRIVATE CONFIGURATION # Store scopes mappings. + mattr_accessor :mappings @@mappings = {} - def self.mappings - # Starting from Rails 8.0, routes are lazy-loaded by default in test and development environments. - # However, Devise's mappings are built during the routes loading phase. - # To ensure it works correctly, we need to load the routes first before accessing @@mappings. - Rails.application.try(:reload_routes_unless_loaded) - @@mappings - end # OmniAuth configurations. mattr_reader :omniauth_configs @@ -360,11 +354,29 @@ def self.mailer=(class_name) end self.mailer = "Devise::Mailer" - # Small method that adds a mapping to Devise. + # Registers a mapping with Devise. This initializes a mapping that only + # contains information about the model; routing information must be added + # later (typically while routes are loaded) via + # `Devise::Mapping#add_routes_options!`. + # + # This is now idempotent, so that `devise_for` can still create a mapping if + # it was not initialized before route loading, or use the existing one if it + # was previously initialized. def self.add_mapping(resource, options) + _scoped_path, name = Devise::Mapping.mapping_name(resource, as: options[:as], singular: options[:singular]) + + if (mapping = @@mappings[name]) + requested = (options[:class_name] || resource.to_s.classify).to_s + if options.key?(:class_name) && requested != mapping.class_name + raise ArgumentError, "conflicting class_name for the #{name.inspect} scope: " \ + "already mapped to #{mapping.class_name.inspect} but got #{requested.inspect}" + end + return mapping + end + mapping = Devise::Mapping.new(resource, options) - @@mappings[mapping.name] = mapping - @@default_scope ||= mapping.name + @@mappings[name] = mapping + @@default_scope ||= name @@helpers.each { |h| h.define_helpers(mapping) } mapping end diff --git a/lib/devise/mapping.rb b/lib/devise/mapping.rb index 8b1f94ced2..1c6e1616c9 100644 --- a/lib/devise/mapping.rb +++ b/lib/devise/mapping.rb @@ -47,18 +47,26 @@ def self.find_scope!(obj) end def self.find_by_path!(path, path_type = :fullpath) - Devise.mappings.each_value { |m| return m if path.include?(m.send(path_type)) } + Devise.mappings.each_value { |m| return m if m.finalized? && path.include?(m.send(path_type)) } raise "Could not find a valid mapping for path #{path.inspect}" end + def self.mapping_name(resource, as: nil, singular: nil) + scoped_path = as ? "#{as}/#{resource}" : resource.to_s + singular = (singular || scoped_path.tr('/', '_').singularize).to_sym + [scoped_path, singular] + end + def initialize(name, options) #:nodoc: - @scoped_path = options[:as] ? "#{options[:as]}/#{name}" : name.to_s - @singular = (options[:singular] || @scoped_path.tr('/', '_').singularize).to_sym + @resource = name + @scoped_path, @singular = self.class.mapping_name(name, as: options[:as], singular: options[:singular]) @class_name = (options[:class_name] || name.to_s.classify).to_s @klass = Devise.ref(@class_name) + end - @path = (options[:path] || name).to_s + def add_routes_options!(options) + @path = (options[:path] || @resource).to_s @path_prefix = options[:path_prefix] @sign_out_via = options[:sign_out_via] || Devise.sign_out_via @@ -71,6 +79,11 @@ def initialize(name, options) #:nodoc: default_path_names(options) default_used_route(options) default_used_helpers(options) + @finalized = true + end + + def finalized? + !!@finalized end # Return modules for the mapping. diff --git a/lib/devise/rails.rb b/lib/devise/rails.rb index b5738853fe..958cd1b259 100644 --- a/lib/devise/rails.rb +++ b/lib/devise/rails.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'devise/rails/routes' +require 'devise/rails/devise_model' require 'devise/rails/warden_compat' module Devise @@ -17,6 +18,10 @@ class Engine < ::Rails::Engine app.reload_routes! if Devise.reload_routes end + config.after_initialize do + Devise.configure_warden! + end + initializer "devise.deprecator" do |app| app.deprecators[:devise] = Devise.deprecator if app.respond_to?(:deprecators) end diff --git a/lib/devise/rails/devise_model.rb b/lib/devise/rails/devise_model.rb new file mode 100644 index 0000000000..c33c702bb5 --- /dev/null +++ b/lib/devise/rails/devise_model.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Devise + ALLOWED_DEVISE_MODEL_OPTIONS = [:class_name, :singular, :as].freeze + + # Registers the model phase of a Devise mapping during Rails initialization, + # before routes are loaded. This makes Devise.mappings[scope] available early + # (with its model, scope and strategies) even when routes are lazy-loaded, + # which Rails does by default in development and test starting from Rails 8. + # + # Devise.devise_model :user + # Devise.devise_model :admin, class_name: "Account", singular: :manager + def self.devise_model(resource, options = {}) + options = options.symbolize_keys + invalid_options = options.keys - ALLOWED_DEVISE_MODEL_OPTIONS + + unless invalid_options.empty? + raise ArgumentError, "Devise.devise_model only accepts model options " \ + "(#{ALLOWED_DEVISE_MODEL_OPTIONS.join(', ')}); got: #{invalid_options.join(', ')}. " \ + "Routing options must be given to devise_for." + end + + add_mapping(resource, options) + end +end diff --git a/lib/devise/rails/routes.rb b/lib/devise/rails/routes.rb index f43e62fea7..5b249d891f 100644 --- a/lib/devise/rails/routes.rb +++ b/lib/devise/rails/routes.rb @@ -16,7 +16,6 @@ def finalize! " to :main_app as well in case you want to keep the current behavior." end - Devise.configure_warden! Devise.regenerate_helpers! true end @@ -240,6 +239,7 @@ def devise_for(*resources) resources.each do |resource| mapping = Devise.add_mapping(resource, options) + mapping.add_routes_options!(options) begin raise_no_devise_method_error!(mapping.class_name) unless mapping.to.respond_to?(:devise) diff --git a/test/mapping_test.rb b/test/mapping_test.rb index 9d60287cd4..839e531efc 100644 --- a/test/mapping_test.rb +++ b/test/mapping_test.rb @@ -133,4 +133,60 @@ def user.devise_scope; :special_scope; end Devise::Mapping.find_by_path!('/accounts/facebook/callback', :path) end end + + test 'mapping_name computes the scoped path and scope from the resource' do + assert_equal ["users", :user], Devise::Mapping.mapping_name(:users) + assert_equal ["user", :user], Devise::Mapping.mapping_name(:user) + end + + test 'mapping_name honors the :singular option' do + assert_equal ["accounts", :manager], Devise::Mapping.mapping_name(:accounts, singular: :manager) + end + + test 'mapping_name honors the :as option' do + assert_equal ["publisher/account", :publisher_account], Devise::Mapping.mapping_name(:account, as: "publisher") + end + + test 'a mapping built from devise_for is finalized' do + assert Devise.mappings[:user].finalized? + end + + test 'a partial mapping exposes the model phase but is not finalized' do + mapping = Devise::Mapping.new(:partial_admin, class_name: "Admin", singular: :partial_admin) + + assert_not mapping.finalized? + assert_equal Admin, mapping.to + assert_equal :partial_admin, mapping.name + assert_equal Admin.devise_modules, mapping.modules + assert_equal [:database_authenticatable], mapping.strategies + end + + test 'add_routes_options! finalizes a partial mapping with the routing options' do + mapping = Devise::Mapping.new(:partial_admin, class_name: "Admin") + mapping.add_routes_options!(path: "admins_area", sign_out_via: :get) + + assert mapping.finalized? + assert_equal "admins_area", mapping.path + assert_equal :get, mapping.sign_out_via + assert_not_nil mapping.used_routes + end + + test 'add_routes_options! defaults the path from the resource, not the singular' do + mapping = Devise::Mapping.new(:accounts, class_name: "Admin", singular: :manager) + mapping.add_routes_options!({}) + + assert_equal "accounts", mapping.path + assert_equal :manager, mapping.name + end + + test 'find_by_path! ignores mappings that are not finalized' do + partial = Devise::Mapping.new(:partial_admin, class_name: "Admin") + Devise.mappings[:partial_admin] = partial + + assert_raise RuntimeError do + Devise::Mapping.find_by_path!("/partial_admin/sign_in") + end + ensure + Devise.mappings.delete(:partial_admin) + end end diff --git a/test/rails_app/config/initializers/devise.rb b/test/rails_app/config/initializers/devise.rb index 7414bc1f94..2075996e5c 100644 --- a/test/rails_app/config/initializers/devise.rb +++ b/test/rails_app/config/initializers/devise.rb @@ -15,6 +15,12 @@ config.secret_key = "d9eb5171c59a4c817f68b0de27b8c1e340c2341b52cdbc60d3083d4e8958532" \ "18dcc5f589cafde048faec956b61f864b9b5513ff9ce29bf9e5d58b0f234f8e3b" + # An example of configuring a devise-enabled model via early initialization, + # before route loading. This same resource name still needs to be configured + # again in the routes, via `devise_for`, in order to register its routes and + # finalize the mapping configuration. + config.devise_model :users + # ==> Mailer Configuration # Configure the e-mail address which will be shown in Devise::Mailer, # note that it will be overwritten if you use your own mailer class with default "from" parameter. diff --git a/test/routes_test.rb b/test/routes_test.rb index 20ba311727..0db2b22499 100644 --- a/test/routes_test.rb +++ b/test/routes_test.rb @@ -274,3 +274,54 @@ class ScopedRoutingTest < ActionController::TestCase assert_equal '/publisher/accounts/get_in', @routes.url_helpers.new_publisher_account_session_path end end + +class DeviseModelTest < ActiveSupport::TestCase + test 'registers a partial mapping that is not finalized' do + mapping = Devise.devise_model :registered_admin, class_name: "Admin" + + assert_equal mapping, Devise.mappings[:registered_admin] + assert_not mapping.finalized? + assert_equal Admin, mapping.to + ensure + Devise.mappings.delete(:registered_admin) + end + + test 'derives the class_name from the resource name when not given' do + mapping = Devise.devise_model :registered_admin + + assert_equal "RegisteredAdmin", mapping.class_name + ensure + Devise.mappings.delete(:registered_admin) + end + + test 'is idempotent for an already registered scope' do + first = Devise.devise_model :registered_admin, class_name: "Admin" + second = Devise.devise_model :registered_admin, class_name: "Admin" + + assert_same first, second + ensure + Devise.mappings.delete(:registered_admin) + end + + test 'rejects routing options' do + e = assert_raise ArgumentError do + Devise.devise_model :registered_admin, path: "admins_area" + end + assert_match "only accepts model options", e.message + assert_nil Devise.mappings[:registered_admin] + end + + test 'devise_for raises on a class_name conflicting with the partial mapping' do + Devise.devise_model :registered_admin, class_name: "Admin" + + e = assert_raise ArgumentError do + routes = ActionDispatch::Routing::RouteSet.new + routes.draw do + devise_for :registered_admin, class_name: "User" + end + end + assert_match "conflicting class_name", e.message + ensure + Devise.mappings.delete(:registered_admin) + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index e6df812037..ccbcc561f9 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -10,6 +10,11 @@ require "rails/test_help" require "orm/#{DEVISE_ORM}" +# Routes are lazy-loaded by default in development and test from Rails 8, so the +# Devise mappings declared by devise_for are not built until something loads the +# routes. Load them now so unit tests can rely on Devise.mappings being present. +Rails.application.reload_routes_unless_loaded + I18n.load_path.concat Dir["#{File.dirname(__FILE__)}/support/locale/*.yml"] require 'mocha/minitest'