Support OIDC-only mode without :database_authenticatable#3
Open
Fivell wants to merge 7 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.
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).