Merge pull request #17 from senid231/fix-after-sign-in-path

Derive every mounted path from ActiveAdmin's namespace

Author: Fivell

.github/workflows/ci.yml

@@ -26,5 +26,14 @@ jobs:
         with:
           ruby-version: ${{ matrix.ruby }}
           bundler-cache: true
-      - name: Run all spec suites (default + engine + isolated)
-        run: bundle exec rake spec:all
+      # One step per dummy app: each boots a different Rails app, so they
+      # cannot share a process — and a separate step names which host
+      # shape broke instead of burying it in a combined run.
+      - name: Default suite (spec/dummy — ActiveAdmin at /admin)
+        run: bundle exec rake spec
+      - name: Engine-mounted Devise suite (spec/dummy_engine)
+        run: bundle exec rake spec:engine
+      - name: Isolated-engine suite (spec/dummy_isolated)
+        run: bundle exec rake spec:isolated
+      - name: Root-mounted suite (spec/dummy_root — ActiveAdmin at /)
+        run: bundle exec rake spec:root

.gitignore

@@ -21,9 +21,12 @@
 /spec/dummy_engine/tmp/
 /spec/dummy_isolated/log/
 /spec/dummy_isolated/tmp/
+/spec/dummy_root/log/
+/spec/dummy_root/tmp/
 
 # The dummy app's database.yml is checked in — override a global
 # ignore rule that excludes "database.yml" everywhere by default.
 !/spec/dummy/config/database.yml
 !/spec/dummy_engine/config/database.yml
 !/spec/dummy_isolated/config/database.yml
+!/spec/dummy_root/config/database.yml

.rspec

@@ -3,6 +3,6 @@
 --color
 # `bundle exec rspec` runs the default suite (spec/dummy). The engine-
 # and isolated-engine suites each boot their own dummy Rails app and
-# must run in separate processes — invoke them via `rake spec:engine`,
-# `rake spec:isolated`, or all three with `rake spec:all`.
+# must run in separate processes — invoke them via `rake spec:engine`
+# and `rake spec:isolated`, or all three with `rake spec:all`.
 --exclude-pattern "spec/{engine,isolated,dummy_engine,dummy_isolated}/**/*"

README.md

@@ -50,16 +50,25 @@ OIDC is the only authentication mechanism — `:database_authenticatable`, `encr
 
 If `devise_for :admin_users` lives inside a Rails engine (not the main app routes), set `Devise.router_name = :<engine_name>` in `config/initializers/devise.rb` and pass the same option to `devise_for`. The gem reads `Devise.available_router_name` and mounts its session routes inside that engine's route set, so `<Engine>.routes.url_helpers.new_<scope>_session_path` resolves correctly.
 
-For **isolated** engines (`isolate_namespace ...`) mounted at a prefix (e.g. `mount AdminPanel::Engine => '/admin'`), the engine prepends its mount path to every internal route. The gem's default `login_path = '/admin/login'` would then become `/admin/admin/login`. Configure engine-relative paths in `config/initializers/activeadmin_oidc.rb`:
+For **isolated** engines (`isolate_namespace ...`) mounted at a prefix (e.g. `mount AdminPanel::Engine => '/admin'`), the engine prepends its mount path to every internal route, so the derived `login_path` of `/admin/login` would become `/admin/admin/login`. Configure engine-relative paths in `config/initializers/activeadmin_oidc.rb`:
 
 ```ruby
 ActiveAdmin::Oidc.configure do |c|
-  c.login_path  = '/login'
-  c.logout_path = '/logout'
+  c.login_path            = '/login'
+  c.logout_path           = '/logout'
+  c.omniauth_route_prefix = '/auth'
 end
 ```
 
-Non-isolated engines don't need this override.
+`omniauth_route_prefix` is the same idea applied to the routes Devise draws for
+OmniAuth. It is separate from `omniauth_path_prefix` because the two are not the
+same string here: the OmniAuth middleware sits in the *application's* Rack stack
+and sees `/admin/auth/oidc` with the mount prefix still attached, while the route
+Devise declares for the callback is inside the engine and gets `/admin` prepended
+to it. Set only `omniauth_path_prefix` and the callback route lands on
+`/admin/admin/auth/oidc/callback`, so the redirect the middleware issues 404s.
+
+Non-isolated engines mounted at `/` don't need any of these overrides.
 
 ### 3. `config/initializers/activeadmin_oidc.rb` (generated)
 
@@ -73,7 +82,7 @@ The gem's Rails engine handles several things so host apps don't have to:
 * **Callback controller** — the engine patches `ActiveAdmin::Devise.controllers` to route OmniAuth callbacks to the gem's controller. No manual `controllers: { omniauth_callbacks: ... }` needed in `routes.rb`.
 * **Login view override** — the engine prepends an SSO-only login page (no email/password fields) to the sessions controller's view path. If your host app ships its own `app/views/active_admin/devise/sessions/new.html.erb`, the gem detects it and backs off — your view wins.
 * **Session routes** — the engine mounts `GET /admin/login` (renders the SSO landing page) and `DELETE /admin/logout` under `devise_scope`, with the scope name derived from `config.admin_user_class`. Devise normally generates session routes as a side effect of `:database_authenticatable`; without that module the route helpers would not exist and ActiveAdmin's login redirect would 404.
-* **Path prefix** — the engine sets `Devise.omniauth_path_prefix` and `OmniAuth.config.path_prefix` to `/admin/auth` so the middleware intercepts requests under ActiveAdmin's mount point. Compatible with Rails 7.2+ and Rails 8's lazy route loading.
+* **Path prefix** — the engine registers the strategy with `path_prefix: '/admin/auth'` so the middleware intercepts requests under ActiveAdmin's mount point, and sets `Devise.omniauth_path_prefix` to the prefix Devise declares its routes with. Compatible with Rails 7.2+ and Rails 8's lazy route loading.
 * **Parameter filtering** — `code`, `id_token`, `access_token`, `refresh_token`, `state`, and `nonce` are added to `Rails.application.config.filter_parameters`.
 
 ## Configuration
@@ -137,12 +146,41 @@ end
 | `identity_attribute` | `:email` | AdminUser column used for lookup/adoption |
 | `identity_claim` | `:email` | Claim key read from the id_token/userinfo |
 | `admin_user_class` | `"AdminUser"` | String or Class for the host's admin user model |
+| `login_path` | `/<namespace>/login` | SSO landing page path; derived from ActiveAdmin's namespace |
+| `logout_path` | `/<namespace>/logout` | Sign-out path; derived from ActiveAdmin's namespace |
+| `omniauth_path_prefix` | `/<namespace>/auth` | Browser-visible path the OmniAuth middleware listens on; derived from ActiveAdmin's namespace |
+| `omniauth_route_prefix` | `omniauth_path_prefix` | Prefix Devise declares its OmniAuth routes with; differs only for engine-mounted hosts |
 | `login_button_label` | `"Sign in with SSO"` | Label on the login-page button |
 | `access_denied_message` | generic | Flash shown on any denial |
 | `on_login` | — (required) | Authorization hook; see below |
 
 `stub_dev_env_login!` is a method, not an option — see "Stub login" below.
 
+## ActiveAdmin's namespace
+
+Everything the gem mounts hangs off ActiveAdmin's namespace, and all of it is
+derived from `ActiveAdmin.application.default_namespace` rather than assumed to
+be `admin`. A host that renames it:
+
+```ruby
+# config/initializers/active_admin.rb
+config.default_namespace = :backoffice
+```
+
+gets `/backoffice/login`, `/backoffice/logout`, the OmniAuth middleware at
+`/backoffice/auth`, and a post-sign-in redirect to `/backoffice` — no gem
+configuration needed. ActiveAdmin's root namespace (`config.default_namespace =
+false`) mounts everything at the top level: `/login`, `/auth`, `/`.
+
+Each is still overridable. Isolated engines *have* to override them, since the
+engine's mount prefix is prepended to every path declared inside it — see
+[Engine-mounted Devise](#engine-mounted-devise).
+
+`omniauth_route_prefix` is what the gem assigns to `Devise.omniauth_path_prefix`,
+and it is skipped entirely if your app already assigned that in
+`config/initializers/devise.rb`. `omniauth_path_prefix` is passed to the OmniAuth
+strategy directly, so it stays correct regardless.
+
 ## The `on_login` hook
 
 `on_login` is the **only** place authorization lives. The gem handles authentication (the user proved who they are via the IdP); deciding whether that user is allowed into the admin panel — and what they can see once they are in — is the host application's problem. The gem does not ship a role model.
@@ -247,7 +285,7 @@ AdminUser.last.oidc_raw_info
 
 * A login button is added to the ActiveAdmin sessions page via a prepended view override — no templates to edit.
 * Clicking it POSTs to `/admin/auth/oidc` with a Rails CSRF token. The gem loads `omniauth-rails_csrf_protection` so OmniAuth 2.x delegates its authenticity check to Rails' forgery protection and `button_to` just works.
-* After a successful callback the user is signed in and redirected to `/admin` (not the host app's `/`, which may not exist).
+* After a successful callback the user is signed in and redirected to ActiveAdmin's namespace root (not the host app's `/`, which may not exist). The path comes from ActiveAdmin's own route helper, so a renamed `config.default_namespace` or an engine-mounted ActiveAdmin lands correctly; `/admin` is only the fallback when that helper cannot be resolved.
 * **Disabled/locked users are rejected.** Devise's `active_for_authentication?` is checked after provisioning but before sign-in. If your model overrides this method (e.g. to check an `enabled` flag or Devise's `:lockable` module), the guard fires on OIDC sign-in too — the user sees an appropriate flash and is redirected to the login page.
 * In development, `stub_dev_env_login!` repoints that same button at a local sign-in that never contacts the IdP — see "Stub login" below.
 * Logout goes through Devise's stock session destroy. No RP-initiated single-logout ping to the IdP — override the destroy action in your host app if you need that.

Rakefile

@@ -7,7 +7,7 @@ begin
 
   # Default spec suite — boots spec/dummy/ (main-app OIDC-only setup).
   RSpec::Core::RakeTask.new(:spec) do |t|
-    t.exclude_pattern = "spec/{engine,isolated,dummy_engine,dummy_isolated}/**/*"
+    t.exclude_pattern = "spec/{engine,isolated,root,dummy_engine,dummy_isolated,dummy_root}/**/*"
   end
 
   namespace :spec do
@@ -24,8 +24,13 @@ begin
       sh "bundle exec rspec --options /dev/null --require spec_helper -I spec/isolated spec/isolated"
     end
 
-    desc "Run every spec suite (default + engine + isolated)"
-    task all: [:spec, :engine, :isolated]
+    desc "Run root-mounted-ActiveAdmin specs (boots spec/dummy_root/)"
+    task :root do
+      sh "bundle exec rspec --options /dev/null --require spec_helper -I spec/root spec/root"
+    end
+
+    desc "Run every spec suite (default + engine + isolated + root)"
+    task all: %i[spec engine isolated root]
   end
 
   task default: :spec

app/controllers/active_admin/oidc/devise/omniauth_callbacks_controller.rb

@@ -105,28 +105,53 @@ def provision_and_sign_in(claims, kind: 'OIDC')
         # sign-in instead of Devise's default (host app root). Hosts
         # that don't define a `/` route would otherwise hit a routing
         # error immediately after login, and even when `/` does exist
-        # it's rarely what an admin user wants to see. ActiveAdmin
-        # always mounts at `/admin`, so we go there directly.
+        # it's rarely what an admin user wants to see.
         def after_sign_in_path_for(resource)
-          stored_location_for(resource) || '/admin'
+          stored_location_for(resource) || active_admin_root_path
+        end
+
+        # Resolved from ActiveAdmin's own route helper rather than
+        # assumed to be '/admin'. Two things move it: a host can rename
+        # the namespace (`config.default_namespace = :backoffice`), and
+        # an engine-mounted ActiveAdmin prefixes every path it declares
+        # with the engine's mount point -- so the real root can be
+        # '/admin/admin' or anything else entirely. Routing a signed-in
+        # admin to a 404 is a poor reward for a successful login.
+        def active_admin_root_path
+          send(devise_router_name(resource_name)).public_send(active_admin_root_helper)
+        rescue NameError, ::ActionController::UrlGenerationError
+          # The host has ActiveAdmin's routes somewhere we can't see (or
+          # hasn't drawn them at all). Its namespace prefix is the best
+          # remaining guess at where the admin panel lives -- except for
+          # the root namespace, where that prefix is '' and would make an
+          # empty, unredirectable Location.
+          ActiveAdmin::Oidc.config.active_admin_namespace_prefix.presence || '/'
+        end
+
+        # ActiveAdmin names its namespace root helper `<namespace>_root_path`,
+        # except for the root namespace where it is plain `root_path`.
+        def active_admin_root_helper
+          namespace = ActiveAdmin::Oidc.config.active_admin_namespace
+          namespace ? :"#{namespace}_root_path" : :root_path
         end
 
         # Devise's `new_session_path(scope)` is only generated when
         # `: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 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).
+        # `new_<scope>_session_path` itself, but on whichever route set
+        # `devise_router_name` resolves to.
         def after_omniauth_failure_path_for(scope)
-          router_name = ::Devise.mappings[scope].router_name ||
-                        ::Devise.available_router_name
-          send(router_name).public_send(:"new_#{scope}_session_path")
+          send(devise_router_name(scope)).public_send(:"new_#{scope}_session_path")
+        end
+
+        # Devise's URL helpers live on the per-mapping `router_name` (set
+        # by `devise_for :scope, router_name: :engine`) when present,
+        # otherwise on the global `Devise.available_router_name` (set by
+        # `Devise.router_name = :engine`), which defaults to `:main_app`.
+        # Replicate that dispatcher here so helpers resolve on the right
+        # context (Rails.application proxy or mounted engine proxy).
+        def devise_router_name(scope)
+          ::Devise.mappings[scope]&.router_name || ::Devise.available_router_name
         end
       end
     end

gemfiles/activeadmin_3.5.gemfile

@@ -14,3 +14,11 @@ gem "omniauth_openid_connect", "~> 0.6.0"
 
 gem "sprockets-rails", ">= 3.4"
 gem "sassc-rails",     ">= 2.1"
+
+# json 3.0 dropped the `quirks_mode` keyword that ActiveSupport::JSON
+# still passes to JSON.parse/JSON.generate (activesupport 7.2 and 8.0,
+# lib/active_support/json/{decoding,encoding}.rb). CI has no lockfile, so
+# it picks up json 3.x and every request spec dies with
+# `ArgumentError: unknown keyword: quirks_mode`. Drop this once Rails
+# ships a version that no longer passes it.
+gem "json", "< 3"

gemfiles/activeadmin_4.0.gemfile

@@ -17,3 +17,7 @@ gem "propshaft"
 gem "importmap-rails"
 gem "cssbundling-rails"
 gem "tailwindcss-rails", "~> 4.0"
+
+# See the note in activeadmin_3.5.gemfile: json 3.0 removed the
+# `quirks_mode` keyword ActiveSupport::JSON still passes.
+gem "json", "< 3"