Skip to content

fix(horde): load OAuth admin page when OAuth is not configured#134

Closed
TDannhauer wants to merge 1 commit into
FRAMEWORK_6_0from
fix/oauth-admin-graceful
Closed

fix(horde): load OAuth admin page when OAuth is not configured#134
TDannhauer wants to merge 1 commit into
FRAMEWORK_6_0from
fix/oauth-admin-graceful

Conversation

@TDannhauer

@TDannhauer TDannhauer commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Make RouteUrlWriter optional in OAuthProviderController so list/edit pages can load without full URL-generation dependencies
  • Show the existing storage-unavailable notice when the null OAuth provider repository is active
  • Fall back OAuth callback URLs to the webroot path when named routes are unavailable

Motivation

The OAuth provider admin page should be usable on instances without OAuth configured. Controller construction and the list template should degrade gracefully instead of failing hard when storage or route URL generation is incomplete.

This complements horde/Core#198 (DbServiceFactory Unix-socket fix), which resolves the fatal PDO error on PostgreSQL socket deployments.

Changes

  • OAuthProviderController: optional RouteUrlWriter, resolveCallbackUrl(), storage-unavailable view data
  • templates/admin/oauthprovider/list.html.php: notice when provider storage is unavailable

Test plan

  • OAuth provider list renders HTTP 200 with null repository forced
  • Storage-unavailable notice appears when NullOAuthProviderConfigRepository is active
  • Manual: open /horde/admin/authentication/provider/ without OAuth providers configured

Make RouteUrlWriter optional so controller construction does not fail when URL generation dependencies are incomplete. Show an existing storage-unavailable notice when the null OAuth provider repository is active, and fall back callback URLs to the webroot path.
@TDannhauer TDannhauer requested a review from ralflang July 9, 2026 22:22

@ralflang ralflang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not really convinced - Nothing in RouteUrlWriter depends on Oauth being configured so it's questionable whatever this fixes for you is really the described behaviour.

Please document how exactly RouteUrlWrite becomes unavailable by disabling / not configuring OAuth.

Generally:

The UrlWriter subsystem is fundamental - if it is not available, we should bail out in a "system temporarily not available" fashion regardless of which endpoint was asked for rather than try to rescue this.

Also if DB is configured but unavailable this is fundamental and "gracefully" starting the system with some resources available and others seemingly disappeared (rather than "currently not available" if the protocol supports it) is actively dangerous in anything synchronizing/consuming

I agree the oauth admin page should survive conditions related to OAuth not yet being configured sufficiently.

@TDannhauer

Copy link
Copy Markdown
Contributor Author

Closing after re-investigation — the review was right on both counts.

  1. RouteUrlWriter is never unavailable in real web requests. The failure I attributed to it came from a CLI test harness running without HTTP_HOST; in the actual request path RouteUrlWriterFactory always resolves. Nothing here depends on OAuth being configured.
  2. The real fatal was the Unix-socket config bug fixed in fix(core): preserve Unix socket SQL config in DbServiceFactory Core#198 (PermissionService -> DbServiceFactory forcing localhost:3306 onto a PostgreSQL socket deployment). With that fix, this page loads fine on an instance without OAuth configured: horde_oauth_providers is simply empty and the list renders with no providers — no controller changes needed.
  3. A deployment whose horde_oauth_providers migration never ran should fail loudly, consistent with every other admin page (perms, prefs, ... would be equally broken there).

@TDannhauer TDannhauer closed this Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants