Skip to content

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

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-56130-auto-sso-password-only
Closed

fix(dashboard): skip auto-SSO redirect for password-only providers#56191
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-56130-auto-sso-password-only

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes an HTTP 500 crash when the dashboard is bound to a non-loopback interface with basic_auth configured. The _auto_sso_response() middleware unconditionally redirects to /auth/login when exactly one session provider is registered, but BasicAuthProvider is password-only and crashes with NotImplementedError inside start_login().

The fix adds a guard: if the sole provider has supports_password=True, skip the auto-SSO redirect and fall through to /login which renders the password form.

Related Issue

Fixes #56130

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/dashboard_auth/middleware.py — In _auto_sso_response(), skip auto-SSO redirect when the single provider is password-only (supports_password=True), letting /login render the password form instead.
  • tests/hermes_cli/test_dashboard_auth_middleware.py — Add test_password_only_provider_skips_auto_sso regression test.

How to Test

  1. Run python -m pytest tests/hermes_cli/test_dashboard_auth_middleware.py -q — all 34 tests should pass (including the new regression test).
  2. Run python -m pytest tests/plugins/dashboard_auth/ -q — all 181 tests should pass.
  3. Manual: configure basic_auth in config.yaml, start dashboard on non-loopback host, visit / — should redirect to /login (password form), not crash with 500.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — N/A (middleware logic is platform-agnostic)
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

When BasicAuthProvider is the sole session provider, _auto_sso_response()
unconditionally redirected to /auth/login which crashes with
NotImplementedError (password-only providers don't support OAuth flow).

Guard on supports_password to fall through to /login instead, which
renders the password form. Regression test included.

Fixes NousResearch#56130
@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 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #54887 — byte-for-byte the same supports_password guard in the same _auto_sso_response() function. This is the 7th+ identical PR in the dashboard auto-SSO cluster (siblings #54958, #55133; canonical #54887). Fixes the same issue #56130.

@liuhao1024

Copy link
Copy Markdown
Contributor Author

This is a duplicate of #54887 by @sjfkai, which implements the identical fix — a supports_password guard in _auto_sso_response() that skips the auto-SSO redirect for password-only providers.

Diff comparison:

Since #54887 was filed first with the same approach, I'll recommend closing this one in favor of #54887. The regression test from this PR could be contributed separately if needed.

Note: #55988 is a complementary fix that adds defense-in-depth at the auth_login route handler level (returns 400 instead of crashing on direct /auth/login?provider=basic access). Both PRs can coexist — they address different layers.

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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard returns HTTP 500 on non-loopback bind with basic auth (BasicAuthProvider auto-redirect crash)

2 participants