fix(dashboard-auth): first unauthenticated page load returns HTTP 500 with a single password-only provider - #57265
Conversation
…providers
With a single interactive provider registered, the auth gate's auto-SSO
middleware unconditionally 302s any unauthenticated document load to
/auth/login?provider=<name>. For a password-only provider (the bundled
basic plugin), start_login() raises NotImplementedError, so every fresh
browser's FIRST page load returns a raw HTTP 500; only the second hit
within the 60s loop-guard window falls through to the /login form. This
affects every basic-auth-only deployment.
Two small, defensive changes:
* Add a supports_redirect_login capability flag to DashboardAuthProvider
(default True, alongside supports_password / supports_token /
supports_session), set it False in BasicAuthProvider, and have
_auto_sso_response fall through to the /login interstitial when the
single provider lacks a redirect flow. The credential form renders
immediately and next= is preserved — the right UX for a password
provider anyway (there is no interstitial click to save).
* Backstop in /auth/login: catch NotImplementedError from start_login
and 302 to {prefix}/login?next=... so direct or stale links to
/auth/login?provider=basic land on the form instead of a 500.
OAuth providers are unaffected (flag defaults True; regression test
pins the existing auto-SSO behaviour).
CronPage loaded its job list on mount and after the user's own mutations only — a cron job firing, failing, or being edited by the CLI/another process stayed invisible until a manual page reload. SessionsPage already polls for exactly this reason (separate processes share one session DB; no push channel yet). Add a silent 30s background refresh alongside the existing mount load, mirroring the SessionsPage pattern: errors in a background tick are swallowed rather than toasted, and the interval re-arms when the selected profile changes. Stopgap until a server-push state channel exists.
Duplicate of #54887 — the earliest open canonical fix for the dashboard 500 when the sole interactive provider is the password-only basic plugin ( |
|
Agreed — duplicate of #54887, which took the same guard via supports_password and predates this. Closing in its favor. FWIW this branch also carried 3 regression tests for the auto-SSO fall-through (middleware falls through to the /login interstitial, next= preserved, OAuth providers still auto-SSO) — happy to port them onto #54887 if useful. Splitting the unrelated CronPage auto-refresh into its own PR. |
Bug
On a dashboard deployment whose only interactive auth provider is the bundled
basic (username/password) plugin, a fresh browser's first hit to any gated
page returns a raw HTTP 500 instead of the login form:
_auto_sso_response(hermes_cli/dashboard_auth/middleware.py) fires on anyunauthenticated non-
/api/document load whenever exactly one interactiveprovider is registered, and unconditionally 302s to
/auth/login?provider=<name>&next=…— it never asks whether the provideractually has a redirect flow.
auth_login(hermes_cli/dashboard_auth/routes.py) wrapsp.start_login(...)intry/except ProviderErroronly.BasicAuthProvider.start_login(plugins/dashboard_auth/basic) raisesNotImplementedError("password-only; there is no OAuth redirect flow") →unhandled → 500.
Only the second request within the 60s
hermes_sso_attemptloop-guard windowfalls through to
/login, so the user sees "Internal Server Error" and reachesthe credential form after a manual reload. Long-lived sessions mask the bug for
existing users; every new browser/device hits it.
Repro: register only the basic provider, then from a cookie-less client
GET /logs→ 302 →GET /auth/login?provider=basic&next=%2Flogs→ 500.Fix (two small, defensive changes)
supports_redirect_login: bool = TruetoDashboardAuthProvider— same pattern as the existingsupports_password/supports_token/supports_sessionflags — and set itFalseinBasicAuthProvider. In_auto_sso_response, when the single interactiveprovider lacks a redirect flow, return
Noneso the gate falls through tothe ordinary
/logininterstitial: the credential form renders immediatelywith
next=preserved, which is the right UX for a password provider anyway(there's no interstitial click to save), and the pointless bounce +
loop-guard cookie are skipped entirely.
auth_login, catchNotImplementedErrorfromstart_loginand 302 to{prefix}/login?next=…, so direct or stale linksto
/auth/login?provider=basic(bookmarks, older clients, any othersingle-provider code path) land on the form instead of a 500.
OAuth providers are completely unaffected — the flag defaults
True, and anew regression test pins the existing auto-SSO redirect behaviour.
Second commit (optional to split out): CronPage 30s auto-refresh
CronPageloaded jobs on mount and after the user's own mutations only, so acron job firing/failing or being edited from the CLI stayed invisible until a
manual reload. This adds a silent 30s background refresh mirroring the
existing SessionsPage polling pattern (background errors swallowed, interval
re-arms on profile change). Happy to drop this commit into its own PR if you
prefer — it's an independent stopgap until a server-push state channel exists.
Tests
test_password_only_single_provider_lands_on_login_not_500— fresh client,single password-only provider:
GET /sessions→ 302/login?next=%2Fsessions→ 200, never 500.
test_auth_login_redirect_backstop_for_password_only_provider— directGET /auth/login?provider=pwstub&next=%2Flogs→ 302/login?next=%2Flogs(and
/loginwith nonext).test_oauth_provider_auto_sso_unchanged— pins the current auto-SSO redirectfor redirect-capable providers.
tests/hermes_cli/test_dashboard_auth_*+ basic-provider suites pass;web/:tsc --noEmitclean.