Fix after_omniauth_failure_path_for for engine-mounted Devise#6
Merged
Conversation
Devise's own new_session_path(scope) helper is only generated when :database_authenticatable is in the mapping's used_helpers — OIDC-only models never get it. The engine mounts new_<scope>_session_path itself, but the helper lives on whichever route set the mapping's router_name points at: Rails.application.routes by default, or <engine_name> routes for hosts that mount Devise inside an engine. Replicates Devise's URL-helper dispatch in the OmniauthCallbacks failure handler: look up the mapping's router_name, send it on the controller to get the engine proxy (or fall back to self for main_app), then public_send the concrete session helper on that context. Without this fix the failure handler raised NoMethodError on hosts with Devise.router_name set (e.g. mount Devise inside a Console/AdminPanel engine).
Both suites previously only tested the happy path (URL helpers resolve, GET /admin/login renders the SSO button). The failure handler in OmniauthCallbacksController#after_omniauth_failure_path_for was only exercised by spec/requests/omniauth_callback_spec.rb, which boots the default dummy where Devise.router_name is unset — so the engine-mounted regression that the previous commit fixes was uncovered by CI. Adds an :invalid_credentials Capybara scenario in each engine dummy: visit /admin/login, click the SSO button, expect to land back on /admin/login. Both fail without the controller's router_name-aware dispatch and pass with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes
after_omniauth_failure_path_forfor hosts that mount Devise inside a Rails engine and pin URL helpers to it viaDevise.router_name = :<engine_name>.Why it was broken
after_omniauth_failure_path_for(scope)was callingpublic_send(:"new_#{scope}_session_path")directly on the controller. That works when the gem's session route is mounted onRails.application.routes(the default), because controllers include the main_app url helpers. But for engine-mounted hosts the gem mounts the route inside<Engine>.routes, so the concrete helper lives on the engine proxy (e.g.console_engine.new_admin_user_session_path), not directly on the controller — calling it raised:Devise's own
new_session_path(scope)doesn't help either — it's only generated when:database_authenticatableis inDevise::Mapping#used_helpers, and OIDC-only models never load that module.Fix
Replicate Devise's URL-helper dispatch (see
devise/controllers/url_helpers.rb#generate_helpers!) — look up the mapping'srouter_name, send it on the controller to get the engine proxy (or fall back toselffor main_app), thenpublic_sendthe concrete session helper on that context:Test plan
rake spec:allis green — covers default, engine-mounted, and isolated-engine setups.Devise.router_name = :<engine>that an OmniAuth failure redirects to the SSO landing page instead of 500-ing.