Support OIDC-only mode without :database_authenticatable#3
Open
Fivell wants to merge 12 commits into
Open
Conversation
When the host AdminUser model uses devise :omniauthable alone (no :database_authenticatable), Devise doesn't mount session routes, so ActiveAdmin's redirect to new_admin_user_session_path 404s. Engine now detects this configuration via Engine.oidc_only? and mounts GET /admin/login + DELETE /admin/logout under the existing devise scope. /admin/login renders the same SSO landing view; /admin/logout signs out via warden. Backwards compatible: when :database_authenticatable is present, the auto-mount is skipped and Devise's own routes win. Dummy app now exercises OIDC-only mode (no encrypted_password column), proving the new path end-to-end. README updated with both setup options.
The gate existed to avoid clashing with Devise's session routes when the host kept :database_authenticatable. In practice OIDC consumers either use :omniauthable alone (route mount needed) or want SSO-only UX anyway. Mixed mode (SSO + password on same login page) is exotic — recoverable/lockable/confirmable belong to the IdP under SSO, migration periods just disable :database_authenticatable while keeping the column, and a host that genuinely needs both forms is already customising the view itself. Now the engine always mounts GET /admin/login and DELETE /admin/logout. POST /admin/login (Devise password sign-in) is unaffected when :database_authenticatable is loaded — same path, different verb.
Reproduces the pbx-api / yeti-web pattern: AdminPanel::Engine (non-isolated) hosts devise_for :admin_users + ActiveAdmin inside its own routes, with Devise.router_name = :admin_panel pinning URL helpers to the engine. Without fixing the gem, AdminPanel::Engine.routes.url_helpers.new_admin_user_session_path is undefined and Capybara visit to /admin/login fails because the gem currently mounts on Rails.application.routes, not the engine. Adds spec/dummy_engine/ (full second dummy app) and spec/engine/ (separate spec dir loaded by engine_rails_helper.rb). Default rspec run is unaffected via .rspec --exclude-pattern.
Hosts that mount Devise inside a Rails engine (pbx-api / yeti-web) set Devise.router_name = :admin_panel so Devise looks up URL helpers on AdminPanel::Engine.routes instead of Rails.application.routes. Previously the gem mounted /admin/login + /admin/logout unconditionally on Rails.application.routes, so AdminPanel::Engine.routes.url_helpers.new_admin_user_session_path stayed undefined and any host-side Devise::FailureApp subclass that redirected via the engine's helpers (e.g. pbx-api's PbxDevise#redirect_url) raised NoMethodError. New Engine.session_routes_target(app) resolves the right route set by walking Rails::Engine.subclasses for the one whose engine_name matches Devise.available_router_name; falls back to Rails.application.routes when router_name is unset or :main_app.
spec/engine/ uses spec/dummy_engine/ via a uniquely-named engine_rails_helper.rb. Default rspec excludes it; CI runs it as its own step so the second dummy app boots in a fresh process.
Adds config.login_path (default /admin/login) and config.logout_path (default /admin/logout). The engine reads them when mounting session routes, so hosts whose Devise lives inside an isolated Rails engine can configure engine-relative paths (e.g. /login when the engine is mounted at /admin) and avoid mount-prefix duplication that would otherwise turn /admin/login into /admin/admin/login. Adds spec/dummy_isolated/ (third dummy app exercising isolate_namespace AdminPanel mounted at /admin) and spec/isolated/ specs verifying the engine's url helpers and route table see the correct paths. CI runs the isolated suite in its own step.
Global gitignore excludes database.yml; the existing dummy app un-ignored its own copy via !/spec/dummy/config/database.yml. Add the same un-ignore for the two new dummies so CI can boot them.
Fivell
added a commit
that referenced
this pull request
May 29, 2026
Critical: engine's session-route mount moved from to_prepare to after_initialize. RouteSet#clear! preserves the append queue across reloads, so to_prepare re-running in dev was enqueueing duplicate append blocks and crashing the second draw with 'Invalid route name, already in use'. after_initialize fires once at boot, leaving the single append callback to re-evaluate cleanly on every reload.
Removed dead ActiveAdmin::Oidc::Devise::SessionsController subclass. Devise's own SessionsController#new is safe for omniauth-only models: sign_in_params returns {} when no params submitted, and clean_up_passwords in DeviseController is guarded by respond_to?, which an omniauth-only model never satisfies. The route now points at ActiveAdmin::Devise::SessionsController#new directly.
Scope name and route helper names derived from config.admin_user_class.model_name.singular instead of hardcoded :admin_user, so hosts with differently-named admin models (User, Staff, etc.) work without patching.
OmniauthCallbacksController#after_omniauth_failure_path_for drops the respond_to? guard — after the mount-fix above the helper is always defined when OIDC is enabled.
Removed spec/unit/engine_spec.rb — it was a mock-heavy unit test for a one-line Array#include? predicate already covered by the dummy-app integration suites.
test_helpers.rb no longer auto-installs RSpec hooks on require. Hosts where OIDC is mandatory (yeti-web / pbx-api / didww-rs) just include TestHelpers directly without the filter machinery they would otherwise have to strip back. Optional-OIDC hosts call ActiveAdmin::Oidc::RSpecSupport.install! explicitly.
README documents the mixed-mode silent-no-op (Devise's GET /admin/login wins recognition because host routes draw first) and the engine-mounted setup with the isolated-engine login_path workaround.
Rakefile gains spec:engine, spec:isolated, spec:all tasks; CI now runs the unified rake spec:all instead of three hand-rolled rspec invocations.
Three intertwined routing fixes: (1) Move the session-route mount from to_prepare to after_initialize. RouteSet#clear! deliberately preserves the @Append queue across reloads (railties 8.0.5 routes_reloader.rb:23-30 + actionpack route_set.rb:466-498), so to_prepare re-running in dev was enqueueing duplicate append blocks and crashing the second draw with 'Invalid route name, already in use: new_admin_user_session'. after_initialize fires once at boot, registering the single append callback exactly once. (2) Remove ActiveAdmin::Oidc::Devise::SessionsController subclass — Devise's own SessionsController#new is safe for omniauth-only models: sign_in_params returns {} when no params submitted, and clean_up_passwords in DeviseController is guarded by respond_to? which an omniauth-only model never satisfies. The route now points at ActiveAdmin::Devise::SessionsController#new directly. (3) Derive scope name and route helper names from config.admin_user_class.model_name.singular instead of hardcoded :admin_user, so hosts with User/Staff/etc. admin models work without patching. Drops the now-dead respond_to? guard in OmniauthCallbacksController#after_omniauth_failure_path_for since the helper is always defined after the mount fix.
It was a mock-heavy unit test for a one-line Array#include? predicate (Engine.oidc_enabled? checks klass.devise_modules.include?(:omniauthable)). The dummy-app integration suites already exercise the predicate end-to-end via the auto-mount path.
Hosts where OIDC is mandatory (yeti-web / pbx-api / didww-rs) were silently getting filter_run_excluding oidc_mode: true installed on every require of activeadmin/oidc/test_helpers, forcing them to either set CI_RUN_OIDC=true or strip the filter from @filter_manager — both are reverse-engineering of an undocumented side effect. test_helpers.rb now only defines the modules; hosts that want the filter machinery call ActiveAdmin::Oidc::RSpecSupport.install! explicitly. The doc block on RSpecSupport spells out both setups.
Mixed mode (:database_authenticatable + :omniauthable on the same model): clarify that Devise's GET /admin/login wins recognition because host routes draw before the gem's after_initialize append — the gem's SSO landing is a silent no-op for these hosts. Direct them to customise the sessions/new view if they want a specific landing UI. Engine-mounted Devise (yeti-web / pbx-api pattern): document Devise.router_name pinning and the isolated-engine login_path/logout_path workaround for hosts using isolate_namespace + mount Engine => '/admin'.
Rakefile gains spec:engine, spec:isolated and spec:all tasks. Each suite boots its own dummy Rails app and therefore must run in a separate process — spec:all shells out three times sequentially. CI replaces three hand-rolled rspec invocations with a single rake spec:all step. .rspec keeps the exclude-pattern for the default rspec invocation (so bundle exec rspec runs the dummy suite alone without booting the other two dummies) and adds an inline comment pointing developers at the rake tasks for the full run.
Fivell
force-pushed
the
oidc-only-without-database-authenticatable
branch
from
5117cd8 to
7e217d6
Compare
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
1 participant
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
Host apps can now use SSO-only without keeping
:database_authenticatableas dead code:— and the
encrypted_passwordcolumn is no longer required.Why
Previously the README told hosts to include
:database_authenticatableeven when the app only used SSO. The reason was not password login — it was that Devise only generates session routes (new_admin_user_session_path,destroy_admin_user_session_path) as a side effect of:database_authenticatable. Without those, ActiveAdmin redirect to/admin/login404s.Host apps then carried defensive scaffolding to neutralise the unused module:
encrypted_passwordcolumn,valid_password?override returningfalse,password_required?override returningfalse. That is busywork to load a module the app does not want.How
activeadmin_oidc.mount_oidc_sessions_routes— when the gem detects:omniauthableon the AdminUser model, it appends to the host route set insidedevise_scope :admin_user:ActiveAdmin::Oidc::Devise::SessionsController < ActiveAdmin::Devise::SessionsController—newrenders the existing SSO landing view;destroyis inherited from Devise (warden logout, independent of auth modules).OmniauthCallbacksController#after_omniauth_failure_path_foroverridden to usenew_admin_user_session_path(always defined now) instead of Devisenew_session_pathproxy, which only exists when:database_authenticatableis loaded.Mixed mode
The mount is unconditional — there is no
oidc_only?gate. Hosts that DO keep:database_authenticatableget our SSO landing on GET/admin/login. Devise's POST/admin/login(password sign-in) is unaffected because it lives on the same path with a different verb. If someone genuinely wants both SSO button and password form on one page, they override the view (app/views/active_admin/devise/sessions/new.html.erb) themselves — that is a presentation concern, not routing.Why no gate:
:recoverable/:lockable/:confirmablebelong to the IdP under SSO — recovery flows there, not in the app.:database_authenticatablewhile keeping the column for old records; the module is already off.Tests
devise :omniauthable, schema: noencrypted_password).:database_authenticatable + :omniauthablealso passes — backwards compatibility preserved.spec/unit/engine_spec.rbcover theoidc_enabled?predicate.Test plan
devise :omniauthableonly (pbx-api).