Skip to content

fix(dashboard-auth): skip auto-SSO redirect for password-only providers - #58980

Closed
jahartling wants to merge 1 commit into
NousResearch:mainfrom
jahartling:fix/auto-sso-password-providers
Closed

fix(dashboard-auth): skip auto-SSO redirect for password-only providers#58980
jahartling wants to merge 1 commit into
NousResearch:mainfrom
jahartling:fix/auto-sso-password-providers

Conversation

@jahartling

Copy link
Copy Markdown

The auto-initiated SSO redirect added in f5ecbe1 assumes every interactive provider speaks OAuth (has a start_login() method). When the only registered provider is BasicAuthProvider (supports_password=True), the redirect to /auth/login?provider=basic calls start_login() which raises NotImplementedError — password-only providers have no OAuth redirect flow.

Root cause: _auto_sso_response() checks whether the single provider is interactive (supports_session=True) but doesn't distinguish password-based providers from OAuth-based ones.

Fix: Check supports_password on the single provider. When True, return None so the call falls through to the /login interstitial, which correctly renders the password form.

Reproduction:

  1. Configure dashboard.basic_auth with username and password_hash in config.yaml
  2. Visit http://localhost:9119/
  3. Dashboard returns 500 Internal Server Error instead of the login page

cc @benbarclay @teknium1

The auto-initiated SSO redirect added in f5ecbe1 assumes every interactive
provider speaks OAuth (has a start_login() method). When the only registered
provider is BasicAuthProvider (supports_password=True), the redirect to
/auth/login?provider=basic calls start_login() which raises
NotImplementedError — password-only providers have no OAuth redirect flow.

Fix: check supports_password on the single provider in _auto_sso_response().
When True, return None so the call falls through to the /login interstitial
which correctly renders the password form.
@alt-glitch alt-glitch added type/bug Something isn't working comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jul 5, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #54887 — same file (hermes_cli/dashboard_auth/middleware.py), same function (_auto_sso_response), same supports_password guard so a sole password-only BasicAuthProvider falls through to the /login form instead of 500ing on start_login(). #54887 is the earliest open canonical fix for this cluster. Related: #55130 (the anchor issue), #58905 (another open sibling fix for the same bug). Regression traces to merged #54846.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. This automated hermes-sweeper review found that the exact behavior is already on current main.

  • hermes_cli/dashboard_auth/middleware.py:212-213 now returns None for a sole provider with supports_password=True, so the existing fallback reaches /login instead of beginning an OAuth redirect.
  • git blame attributes that guard to 3e24b16f566045399012bc1185fe0cdb6e1a1be9 (fix(dashboard): support mobile OAuth login).
  • The downstream password form remains implemented at hermes_cli/dashboard_auth/login_page.py:485-487.

Closing as implemented on main; the PR's change is redundant with the current implementation.

@teknium1 teknium1 closed this Jul 15, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/dashboard Web dashboard / control panel UI (dashboard/, landing) duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants