fix(dashboard-auth): BasicAuthProvider 500 on /auth/login and auto-SSO redirect - #65013
fix(dashboard-auth): BasicAuthProvider 500 on /auth/login and auto-SSO redirect#65013wen0531 wants to merge 1 commit into
Conversation
…O redirect Two bugs prevented BasicAuthProvider (username/password) from working on a non-loopback bind: 1. middleware.py _auto_sso_response: when a single session provider is registered, it auto-redirects to /auth/login?provider=N, which calls start_login(). BasicAuthProvider.start_login() raises NotImplementedError, returning HTTP 500. Fix: check supports_password and skip auto-SSO, falling through to /login which renders the password form. 2. routes.py auth_login: the /auth/login?provider=basic endpoint calls p.start_login() unconditionally. For password-only providers this raises NotImplementedError -> 500. Fix: when supports_password is True, redirect to /login instead of calling start_login(). Both fixes only add a supports_password check before the OAuth-specific code path; OAuth providers are completely unaffected.
Duplicate of #58044 — same both-site |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Looks Good
- Fix(dashboard-auth): BasicAuthProvider 500 on /auth/login and auto-SSO redirect
- 16 additions, 3 deletions — targeted fix
- No issues detected
Reviewed by Hermes Agent
|
Thanks for the focused report and implementation. This is already implemented on current
This also resolves the duplicate cluster noted in the prior triage discussion. |
Problem
Two bugs prevent BasicAuthProvider (username/password dashboard auth) from working on a non-loopback bind (
0.0.0.0):Bug 1: Auto-SSO redirect crashes on password providers
_auto_sso_responseinmiddleware.pychecks if there's exactly one session provider, and if so, auto-redirects to/auth/login?provider=N. ButBasicAuthProvider.start_login()raisesNotImplementedErrorbecause it's a password-only provider with no OAuth redirect flow. Result: HTTP 500 on first page load.Bug 2: /auth/login endpoint crashes on password providers
The
/auth/login?provider=basicroute callsp.start_login()unconditionally. For password-only providers this also raisesNotImplementedError→ 500.Fix
Both fixes add a
supports_passwordcheck before the OAuth-specific code path:/loginwhich renders the password form./logininstead of callingstart_login().OAuth providers are completely unaffected — the checks only gate on
supports_password=True.Verification
0.0.0.0bind confirms:/→ redirect to/login→ 200 (was 500)/auth/login?provider=basic→ redirect to/login→ 200 (was 500)POST /auth/password-login→ 200 with session cookies set