Skip to content

fix(dashboard): don't auto-SSO password-only providers into the OAuth route - #57927

Closed
jconover wants to merge 1 commit into
NousResearch:mainfrom
jconover:fix/dashboard-auto-sso-password-provider
Closed

fix(dashboard): don't auto-SSO password-only providers into the OAuth route#57927
jconover wants to merge 1 commit into
NousResearch:mainfrom
jconover:fix/dashboard-auto-sso-password-provider

Conversation

@jconover

@jconover jconover commented Jul 3, 2026

Copy link
Copy Markdown

Summary

A dashboard configured with only basic (username/password) auth returns HTTP 500 on every unauthenticated page load instead of showing the login form.

Root cause

_auto_sso_response in hermes_cli/dashboard_auth/middleware.py is an OAuth-only optimization: when exactly one interactive provider is registered, it skips the /login chooser and 302-redirects the browser straight to /auth/login?provider=…. That route runs the OAuth authorization-code flow via provider.start_login().

A password-only provider (BasicAuthProvider, supports_password = True) has no OAuth redirect flow — its start_login raises:

NotImplementedError: BasicAuthProvider is password-only; there is no OAuth redirect flow.
The login page POSTs to /auth/password-login instead.

So a single-provider BasicAuth dashboard 500s on every unauthenticated document load. It's masked on loopback (already-authenticated sessions), but surfaces immediately when the dashboard is exposed and hit fresh (e.g. bound to a LAN address).

Fix

Guard the auto-SSO shortcut on the sole provider actually supporting the OAuth redirect. When it's password-based, return None so the gate falls through to the /login password form. OAuth providers are unaffected.

provider = providers[0]
if getattr(provider, "supports_password", False):
    return None   # password form, not OAuth redirect

Test Plan

  • Added test_single_password_provider_renders_login_not_oauth to TestAutoSsoRedirect: a single password-only provider must 302 to /login, never /auth/login.
  • Verified via fastapi.testclient.TestClient against the real app:
    • password-only provider → 302 /login?next=%2Fsessions ✅ (was 500)
    • OAuth provider (control) → 302 /auth/login?provider=stub&next=%2Fsessions ✅ (unchanged)
  • Existing TestAutoSsoRedirect cases (OAuth auto-redirect, loop guard, multi-provider chooser, API-path 401) preserved.

Notes

Minimal footprint: one guard line in the middleware + one regression test. No new config, no schema change, no impact on the OAuth path.

_auto_sso_response redirects an unauthenticated HTML load straight to
/auth/login when exactly one interactive provider is registered. That
route runs the OAuth authorization-code flow (provider.start_login). A
password-only provider (e.g. BasicAuthProvider) has no OAuth redirect
flow — its start_login raises NotImplementedError — so a single-provider
BasicAuth dashboard 500s on every unauthenticated document load instead
of rendering the /login password form.

Guard the auto-SSO shortcut on the provider actually supporting the
OAuth redirect: when the sole provider is password-based
(supports_password), fall through to /login so the credential form
renders. OAuth providers are unaffected.

Add a regression test asserting a single password-only provider yields a
302 to /login (not /auth/login).
@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 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #54887 — this adds the same supports_password early-return guard in _auto_sso_response() (hermes_cli/dashboard_auth/middleware.py) as the earliest open canonical fix #54887 (verified open). It's part of a large auto-SSO/password-provider fix cluster (#54958, #57402, #57797, #57214, …) all addressing the same HTTP 500 (anchor issue #55130, regression from merged #54846). Marking as a duplicate so maintainers can converge on one PR.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression report and test coverage. This is an automated hermes-sweeper review; the requested behavior is already present on current main.

  • hermes_cli/dashboard_auth/middleware.py:211-213 returns None for a sole supports_password provider, so the gate falls through to /login rather than auto-starting OAuth.
  • tests/hermes_cli/test_dashboard_auth_password_login.py:201-209 already verifies the password-provider redirect and rendered password form.
  • The matching implementation arrived in 3e24b16f566045399012bc1185fe0cdb6e1a1be9 (fix(dashboard): support mobile OAuth login).

The earlier duplicate-cluster note is consistent with this result; this PR is now redundant.

@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