Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,18 @@ def after_sign_in_path_for(resource)
# `:database_authenticatable` is in the mapping's `used_helpers`,
# so an OIDC-only model never gets it. The engine mounts
# `new_<scope>_session_path` itself, but the helper lives on
# whichever route set the mapping's `router_name` points at —
# main_app by default, or `<engine_name>` for hosts that mount
# Devise inside a Rails engine. Replicate Devise's own dispatch
# so the right context is asked.
# whichever route set Devise's URL helper dispatcher points at:
# the per-mapping `router_name` (set by
# `devise_for :scope, router_name: :engine`) when present,
# otherwise the global `Devise.available_router_name`
# (set by `Devise.router_name = :engine`), which defaults to
# `:main_app`. Replicate that dispatcher here so the helper is
# resolved on the right context (Rails.application proxy or
# mounted engine proxy).
def after_omniauth_failure_path_for(scope)
router_name = ::Devise.mappings[scope].router_name
context = router_name ? send(router_name) : self
context.public_send(:"new_#{scope}_session_path")
router_name = ::Devise.mappings[scope].router_name ||
::Devise.available_router_name
send(router_name).public_send(:"new_#{scope}_session_path")
end
end
end
Expand Down
8 changes: 6 additions & 2 deletions spec/dummy_engine/lib/admin_panel/config/routes.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
# frozen_string_literal: true

# Engine-mounted-Devise layout: devise_for + ActiveAdmin routes live
# inside the engine, pinned to it via `router_name`.
# inside the engine. The engine is pinned via the GLOBAL
# `Devise.router_name = :admin_panel` set in config/initializers/devise.rb
# rather than a per-mapping `router_name:` option here — this is the
# pattern hosts use most often (and the pattern under which the bug in
# OmniauthCallbacksController#after_omniauth_failure_path_for surfaces).
AdminPanel::Engine.routes.draw do
devise_for :admin_users, ActiveAdmin::Devise.config.merge(router_name: :admin_panel)
devise_for :admin_users, ActiveAdmin::Devise.config
ActiveAdmin.routes(self)
end
49 changes: 34 additions & 15 deletions spec/engine/features/engine_mounted_login_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,18 @@
# Engine-mounted Devise + OIDC scenario:
#
# - AdminPanel::Engine is a non-isolated Rails engine
# - `devise_for :admin_users, ..., router_name: :admin_panel` lives
# inside the engine's routes
# - `Devise.router_name = :admin_panel` pins Devise's URL helpers to
# `AdminPanel::Engine.routes`
# - `devise_for :admin_users` (NO `router_name:` option) lives inside
# the engine's routes
# - `Devise.router_name = :admin_panel` is set globally in the
# devise initializer — this pins Devise's URL helpers to the
# engine via `Devise.available_router_name` rather than the
# per-mapping option
#
# With AdminUser also OIDC-only (`devise :omniauthable`, no
# `:database_authenticatable`), Devise generates no session routes.
# activeadmin-oidc has to mount `/admin/login` and `/admin/logout`
# *inside the engine's route set*, otherwise
# `AdminPanel::Engine.routes.url_helpers.new_admin_user_session_path`
# stays undefined and host-side failure apps (Devise::FailureApp
# subclasses that redirect via the engine's helpers) blow up with
# NoMethodError.
# The global-only setup is the more common pattern in real hosts. The
# OmniauthCallbacks failure handler must therefore fall back to
# `Devise.available_router_name` when `Devise.mappings[scope].router_name`
# is nil — calling the helper directly on the controller blows up
# because the helper lives on the engine proxy, not on main_app.
RSpec.feature "Engine-mounted OIDC sessions", type: :feature do
it "AdminPanel::Engine.routes.url_helpers exposes new_admin_user_session_path" do
expect {
Expand Down Expand Up @@ -47,7 +46,15 @@
expect(page).to have_current_path("/admin/login")
end

context "OmniAuth failure path" do
context "OmniAuth failure path under global Devise.router_name" do
# Sanity-check the dummy setup so a future refactor that flips it
# back to per-mapping `router_name:` makes the regression scenario
# below stop covering the global-only code path explicitly.
it "dummy_engine has no per-mapping router_name (relies on the global setting)" do
expect(Devise.mappings[:admin_user].router_name).to be_nil
expect(Devise.available_router_name.to_sym).to eq(:admin_panel)
end

around do |ex|
saved = OmniAuth.config.mock_auth[:oidc]
OmniAuth.config.mock_auth[:oidc] = :invalid_credentials
Expand All @@ -58,11 +65,23 @@
scenario "failed OmniAuth callback lands back on the SSO landing page" do
# OmniauthCallbacksController#after_omniauth_failure_path_for has
# to resolve `new_<scope>_session_path` against the engine's
# route set (Devise.router_name pins helpers there) — calling the
# helper directly on the controller would raise NoMethodError.
# route set. When the host pins helpers via the GLOBAL
# `Devise.router_name = :admin_panel` (no per-mapping option),
# `Devise.mappings[scope].router_name` is nil and the handler has
# to fall back to `Devise.available_router_name` to find the
# engine proxy — otherwise it tries the helper on the controller
# itself and raises NoMethodError (500 response → driver
# exception, not a clean redirect).
visit "/admin/login"
click_button ActiveAdmin::Oidc.config.login_button_label

# Full Capybara assertions: real path + rendered DOM state. If
# the failure handler had raised, Capybara would never reach the
# `page` assertions — but we also check the SSO button + flash
# so a silent redirect to the wrong place can't pass either.
expect(page).to have_current_path("/admin/login")
expect(page).to have_button(ActiveAdmin::Oidc.config.login_button_label)
expect(page).to have_content(ActiveAdmin::Oidc.config.access_denied_message)
end
end
end
Loading