Bump devise to 5.0, widen omniauth_openid_connect support#2
Merged
Conversation
omniauth_openid_connect 0.7+ pulls in openid_connect 2.x which requires faraday ~> 2.0. Host apps still on faraday 1.x (large legacy Gemfiles with pinned transitives like azure-storage-common, faraday_middleware, pipedrive forks) cannot adopt the gem at all. omniauth_openid_connect 0.6.x pairs with openid_connect 1.x, which uses httpclient internally and has no faraday dependency. activeadmin-oidc itself only touches the standard OmniAuth strategy registration API (devise.omniauth :openid_connect, ...) — identical across both lines — so the floor can safely move down to 0.6 with no runtime changes.
…vise mappings OmniAuth.config.on_failure walks Devise.mappings.find_by_path! to resolve the resource scope from the request path. On Rails 8 routes load lazily — first OmniAuth callback can fire before devise_for runs, which leaves Devise.mappings empty and raises 'Could not find a valid mapping for path /admin/auth/oidc' that masks the real underlying error (CSRF check, bad id_token signature, etc). Move the workaround into the engine so every host app gets it for free instead of every consumer copy-pasting Rails.application.routes_reloader. execute_unless_loaded into their activeadmin-oidc initializer.
execute_unless_loaded is a Rails 8 API; on Rails 7.x the engine crashed at boot. Guarded with respond_to? — Rails 7.x loads routes eagerly so the hook is a no-op there. Added a high-level spec that simulates the empty-Devise.mappings state and verifies the fix.
The previous spec poked at routes_reloader and Devise.mappings internals to demonstrate the fix. Replaced with a feature-level Capybara spec that exercises the OIDC failure flow end-to-end on Rails 8 only (skipped on 7.x where the bug doesnt manifest). Adds capybara as a dev dependency.
0.6.x uses openid_connect 1.x (httpclient, no faraday dep) so host apps still on faraday 1.x can stay on this line. 0.7.x uses openid_connect 2.x (faraday 2.x). Both must keep working since the gemspec floor is >= 0.6. OOIDC env var overrides the omniauth_openid_connect version in the Gemfile; CI exercises both lines across the Ruby/Rails/AA matrix.
Devise 5.0 wraps Devise.mappings with reload_routes_unless_loaded (heartcombo/devise#5728) so OmniAuth's failure handler resolves the scope correctly under Rails 8 lazy route loading without engine-side workarounds. Removes: engine after_initialize hook, capybara dev dep, the Rails 8 sanity spec that existed only to cover the bug Devise now handles upstream.
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
Bump
devisefloor to>= 5.0and widenomniauth_openid_connectsupport to>= 0.6so host apps on either the faraday-1 (0.6.x) or faraday-2 (0.7.x/0.8.x) line can install the gem.CI matrix now exercises all three
omniauth_openid_connectlines across Ruby 3.2–3.4 and Rails 7.2 / 8.0.Background — why no Rails 8 route-loader workaround in the engine
Originally this branch added an engine-side
after_initializehook to force Rails 8 route loading so OmniAuth's failure handler wouldn't hit an emptyDevise.mappings. On investigation, Devise 5.0 already handles this upstream (heartcombo/devise#5728) — itsDevise.mappingsgetter wrapsRails.application.try(:reload_routes_unless_loaded), which is functionally identical to our workaround. So instead of carrying a duplicate, this PR bumps thedevisefloor to the version that has the fix.Changes
devise >= 5.0(was>= 4.9) with a comment pointing to the Devise PR.omniauth_openid_connect >= 0.6(was>= 0.7) — keeps faraday-1 host apps supported.omniauth_openid_connectaxis added:['0.6.0', '0.7.0', '0.8.0'].OOIDCenv var so CI can pin per matrix cell.lib/.Compatibility
activeadmin-oidcto the previous version.omniauth_openid_connectconsumers — floor was lowered, not raised.Test plan