Derive every mounted path from ActiveAdmin's namespace by senid231 · Pull Request #17 · activeadmin-plugins/activeadmin-oidc

senid231 · 2026-08-25T11:08:59Z

The gem hardcoded /admin for the post-sign-in landing page, the SSO login/logout routes and the OmniAuth prefix. A host that renames the namespace (config.default_namespace = :backoffice) has no /admin anywhere, so each of those either 404'd or raised. Derive them from ActiveAdmin.application.default_namespace instead, and resolve the post-sign-in path through ActiveAdmin's own root helper on whichever route set holds the host's Devise mapping.

Engine-mounted hosts need one more distinction. Devise reuses a single setting, Devise.omniauth_path_prefix, both to declare its OmniAuth routes and to tell the middleware where to listen; for an engine mounted at a prefix those are different strings, so one value put the callback route at /admin/admin/auth/oidc/callback while the middleware redirected to /admin/auth/oidc/callback. Drive them separately: path_prefix: goes to the strategy directly (honoured over OmniAuth.config.path_prefix, which Devise owns), and omniauth_route_prefix -- defaulting to omniauth_path_prefix, so main-app hosts see no change -- feeds Devise's global. Registering the strategy after the host's config/initializers makes the namespace readable there, so no after_initialize hook or Rails 8 lazy-route workaround is needed.

Covered by unit specs for the namespace derivation and a full OmniAuth round trip against spec/dummy_isolated.

Copilot

Pull request overview

This PR removes hardcoded /admin assumptions by deriving login/logout/OmniAuth paths (and the post-sign-in landing path) from ActiveAdmin’s configured namespace, with special handling for engine-mounted hosts where Devise’s OmniAuth route prefix and the middleware listening prefix must differ.

Changes:

Derive login_path, logout_path, and OmniAuth prefixes from ActiveAdmin.application.default_namespace, while preserving explicit overrides.
Split OmniAuth configuration into a per-strategy middleware path_prefix: and a Devise route declaration prefix (omniauth_route_prefix) to support mounted engines correctly.
Add/adjust request + isolated-engine specs and update docs/templates to use ActiveAdmin::Oidc.config.omniauth_path_prefix as the source of truth for browser-visible OmniAuth request paths.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
spec/unit/configuration_spec.rb	Adds unit coverage for namespace-derived paths and fallback behavior.
spec/requests/login_path_helper_spec.rb	Updates regression spec to assert login view uses configured OmniAuth prefix (not hardcoded).
spec/requests/disabled_user_persistence_spec.rb	Switches callback posting to configured OmniAuth prefix.
spec/requests/after_sign_in_path_spec.rb	Adds specs for post-sign-in landing path resolution via ActiveAdmin root helper.
spec/isolated/requests/isolated_engine_callback_spec.rb	Adds full round-trip coverage for isolated engine mounts with split prefixes.
spec/dummy_isolated/config/initializers/activeadmin_oidc.rb	Configures isolated dummy to override `omniauth_route_prefix` engine-relatively.
README.md	Documents namespace derivation and the new `omniauth_route_prefix` behavior for engines.
Rakefile	Minor task definition syntax update for `spec:all`.
lib/generators/active_admin/oidc/install/templates/sessions_new.html.erb	Uses gem config for OmniAuth request path in generated view.
lib/generators/active_admin/oidc/install/templates/sessions_new_v4.html.erb	Uses gem config for OmniAuth request path in generated v4 view.
lib/activeadmin/oidc/engine.rb	Registers strategy using per-strategy `path_prefix:` and sets Devise route prefix from config.
lib/activeadmin/oidc/configuration.rb	Introduces derived path helpers + `omniauth_route_prefix` with fallback namespace behavior.
app/views/active_admin/devise/sessions/new.html.erb	Uses gem config for OmniAuth request path instead of OmniAuth global.
app/controllers/active_admin/oidc/devise/omniauth_callbacks_controller.rb	Redirects post-sign-in via ActiveAdmin’s root helper; adds route-set dispatch helper.
.rspec	Updates guidance comment about running spec suites.
.gitignore	Adds ignore entries and db.yml exception for new dummy app paths.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The gem hardcoded `/admin` for the post-sign-in landing page, the SSO login/logout routes and the OmniAuth prefix. A host that renames the namespace (`config.default_namespace = :backoffice`) has no /admin anywhere, so each of those either 404'd or raised. Derive them from `ActiveAdmin.application.default_namespace` instead, and resolve the post-sign-in path through ActiveAdmin's own root helper on whichever route set holds the host's Devise mapping. Engine-mounted hosts need one more distinction. Devise reuses a single setting, `Devise.omniauth_path_prefix`, both to declare its OmniAuth routes and to tell the middleware where to listen; for an engine mounted at a prefix those are different strings, so one value put the callback route at `/admin/admin/auth/oidc/callback` while the middleware redirected to `/admin/auth/oidc/callback`. Drive them separately: `path_prefix:` goes to the strategy directly (honoured over `OmniAuth.config.path_prefix`, which Devise owns), and `omniauth_route_prefix` -- defaulting to `omniauth_path_prefix`, so main-app hosts see no change -- feeds Devise's global. Registering the strategy after the host's config/initializers makes the namespace readable there, so no `after_initialize` hook or Rails 8 lazy-route workaround is needed. Covered by unit specs for the namespace derivation and a full OmniAuth round trip against spec/dummy_isolated. Co-Authored-By: Clanker

The namespace derivation above is exercised by unit specs that stub `ActiveAdmin.application.default_namespace`. This adds the other half: a host that really boots with `config.default_namespace = false`, so the derived login path, OmniAuth prefix and post-sign-in redirect are asserted against a live route table rather than a double. spec/dummy_root/ is spec/dummy/ with that one config line flipped, and no host `root` route — anything the gem redirects to outside the ActiveAdmin route table 404s instead of quietly resolving. The suite pins down exactly what broke in production on such a host: /login (not /admin/login) serves the SSO landing page, /auth is where the middleware listens, and a callback with no stored location lands on / instead of a 404 at /admin. CI now runs one step per dummy app rather than a single combined `spec:all`, so a failure names the host shape that broke.

json 3.0.2 removed the `quirks_mode` keyword that ActiveSupport::JSON.decode/encode still passes to JSON.parse and JSON.generate (activesupport 7.2 and 8.0). CI resolves without a lockfile, so it picked json 3.x up as soon as it was released and every request spec fails with `ArgumentError: unknown keyword: quirks_mode`, regardless of what a branch changes.

Fivell · 2026-09-30T11:40:44Z

Rebased onto main and picked up the parts of #19 that were not already here; #19 is closed as superseded.

Conflict resolutions (against main, which gained stub login #16 and the dark-mode login screen #18 after this branch was written):

The three login views now go through ActiveAdmin::Oidc.config.login_submit_path, which main introduced and which already handles the stub-login route. This branch's intent — post to the configured OmniAuth prefix rather than the OmniAuth.config.path_prefix global — moved one level down, into login_submit_path itself:
```
def login_submit_path
  return "#{login_path}/stub" if stub_dev_env_login_enabled?
  "#{omniauth_path_prefix}/#{Engine::PROVIDER_NAME}"
end
```
Same outcome for engine-mounted hosts, and the stub button keeps working. Worth noting the pre-rebase version of the two generator templates had <%%= escaping, but the generator uses copy_file, not template — that would have emitted a literal <%= into the host's view. Taking main's unescaped side fixes that too.
Configuration: kept FALLBACK_NAMESPACE from this branch and DEFAULT_STUB_DEV_ENV_LOGIN_CLAIMS from main; DEFAULT_LOGIN_PATH / DEFAULT_LOGOUT_PATH stay deleted.
.gitignore: the entries for spec/dummy_namespaced/ were dropped — no such directory exists in this branch or on main.

Added from #19:

spec/dummy_root/ + spec/root/ — a host that really boots with config.default_namespace = false, asserting the derived paths against a live route table rather than a stubbed default_namespace. It reproduces the exact production symptom this PR fixes: /login serves the SSO landing page (not /admin/login, which 404s), /auth is where the middleware listens, and a callback with no stored location lands on / instead of a 404 at /admin.

These specs were originally written against the hardcoded /admin/auth/oidc and failed on this branch — correctly, since the prefix is now derived. They read it from config now.
CI runs one step per dummy app instead of a single spec:all, so a failure names the host shape that broke.
gem "json", "< 3" in both CI gemfiles. Unrelated to this PR, but without it nothing runs: json 3.0.2 removed the quirks_mode keyword that ActiveSupport::JSON.decode/encode still passes (activesupport 7.2 and 8.0), and CI resolves without a lockfile. Every request spec was failing with ArgumentError: unknown keyword: quirks_mode regardless of branch content.

Suites, all green on 6/6 matrix legs:

spec           161 examples, 0 failures, 1 pending
spec:engine      7 examples, 0 failures
spec:isolated    8 examples, 0 failures
spec:root        6 examples, 0 failures

Copilot

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Review effort: Lite
Findings: 3

Open (3)

Exclude root dummy suites from the default RSpec run · New
Update login path test for the new omniauth prefix contract · New
Keep Devise and strategy OAuth prefixes consistent · New

Resolved since last review (1)

In the fallback path for active_admin_root_path, returning `ActiveAdmin::Oidc.config.active_admin_…

+# 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}/**/*"


+        # `omniauth_path_prefix`, not `OmniAuth.config.path_prefix`:
+        # under an engine mount those two differ by the mount prefix,
+        # and this one is the browser-visible path the form posts to.
+        "#{omniauth_path_prefix}/#{Engine::PROVIDER_NAME}"


+        ::Devise.omniauth_path_prefix ||= cfg.omniauth_route_prefix

+        ::Devise.setup do |devise|
          devise.omniauth :openid_connect,
                          name: PROVIDER_NAME,
+                          path_prefix: cfg.omniauth_path_prefix,


senid231 requested review from Fivell and a lite review from Copilot August 25, 2026 11:08

senid231 self-assigned this Aug 25, 2026

senid231 force-pushed the fix-after-sign-in-path branch from 44defde to f95a7e2 Compare August 25, 2026 11:09

Copilot started reviewing on behalf of senid231 August 25, 2026 11:09 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Comment thread app/controllers/active_admin/oidc/devise/omniauth_callbacks_controller.rb Outdated

senid231 force-pushed the fix-after-sign-in-path branch from f95a7e2 to 08c1c26 Compare August 25, 2026 14:47

Fivell mentioned this pull request Sep 30, 2026

Follow the host's default_namespace after SSO sign-in #19

Closed

Fivell added 2 commits September 30, 2026 13:37

Fivell force-pushed the fix-after-sign-in-path branch from 08c1c26 to 734488f Compare September 30, 2026 11:38

Fivell approved these changes Sep 30, 2026

View reviewed changes

Fivell merged commit bf425a4 into activeadmin-plugins:main Sep 30, 2026
6 checks passed

senid231 deleted the fix-after-sign-in-path branch September 30, 2026 12:18

Fivell requested a lite review from Copilot September 30, 2026 12:25

Copilot started reviewing on behalf of Fivell September 30, 2026 12:26 View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

This was referenced Sep 30, 2026

Exclude the root-mounted suite from a bare rspec run #20

Merged

Follow a host-set Devise.omniauth_path_prefix in the strategy too #21

Merged

senid231 mentioned this pull request Oct 1, 2026

Release 3.0.0 #25

Merged

Navigation Menu

Derive every mounted path from ActiveAdmin's namespace - #17

Derive every mounted path from ActiveAdmin's namespace#17
Fivell merged 3 commits into
activeadmin-plugins:mainfrom
senid231:fix-after-sign-in-path

senid231 commented Aug 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Fivell commented Sep 30, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

senid231 commented Aug 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Fivell commented Sep 30, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants