From fc362ace3e04f9a147f8856d2c3555bc25a1fa3e Mon Sep 17 00:00:00 2001 From: Anurag Mahapatra Date: Mon, 6 Jul 2026 18:31:38 +0530 Subject: [PATCH] fix(dashboard-auth): skip auto-SSO redirect for password-only providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the sole registered auth provider is password-only (supports_password=True, e.g. BasicAuthProvider), _auto_sso_response redirected to /auth/login which calls start_login() — a method that pure-password providers implement as a NotImplementedError stub. This crashed the dashboard with HTTP 500 on first load. Now _auto_sso_response returns None for password-only providers, falling through to _unauth_response which correctly renders the /login page with the password form. Fixes #56067, #57211, #58166 --- hermes_cli/dashboard_auth/middleware.py | 10 ++++ .../test_dashboard_auth_middleware.py | 47 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/hermes_cli/dashboard_auth/middleware.py b/hermes_cli/dashboard_auth/middleware.py index 2c5f5b4f7b95..abb653e7424e 100644 --- a/hermes_cli/dashboard_auth/middleware.py +++ b/hermes_cli/dashboard_auth/middleware.py @@ -185,6 +185,16 @@ def _auto_sso_response(request: Request) -> Response | None: from hermes_cli.dashboard_auth.prefix import prefix_from_request provider = providers[0] + + # Auto-SSO is only meaningful for OAuth/redirect providers. A + # pure-password provider (supports_password=True, e.g. basic) has no + # IDP to silently bounce to — its start_login() raises + # NotImplementedError. Return None so the caller falls through to + # _unauth_response, which renders the /login interstitial with the + # password form. + if getattr(provider, "supports_password", False): + return None + prefix = prefix_from_request(request) next_param = _safe_next_target(request) from urllib.parse import quote diff --git a/tests/hermes_cli/test_dashboard_auth_middleware.py b/tests/hermes_cli/test_dashboard_auth_middleware.py index 7c1d6a9c2b21..4398edc5cd74 100644 --- a/tests/hermes_cli/test_dashboard_auth_middleware.py +++ b/tests/hermes_cli/test_dashboard_auth_middleware.py @@ -593,3 +593,50 @@ def test_unverifiable_token_with_reachable_providers_redirects(_gated_state): r = client.get("/api/auth/me") assert r.status_code == 401 assert "unreachable" not in r.text.lower() + + +def test_auto_sso_skips_password_only_provider(): + """When the sole provider is password-only (e.g. basic), auto-SSO must NOT + redirect to /auth/login (which would hit start_login() and crash with + NotImplementedError). Instead the middleware should fall through to + _unauth_response → /login, where the password form renders. + + Regression guard for #56067 / #57211 / #58166. + """ + from plugins.dashboard_auth.basic import BasicAuthProvider, hash_password + + clear_providers() + register_provider( + BasicAuthProvider( + username="admin", + password_hash=hash_password("test-password"), + secret=b"test-secret-at-least-16-bytes!!", + ) + ) + prev_host = getattr(web_server.app.state, "bound_host", None) + prev_port = getattr(web_server.app.state, "bound_port", None) + prev_required = getattr(web_server.app.state, "auth_required", None) + web_server.app.state.bound_host = "fly-app.fly.dev" + web_server.app.state.bound_port = 443 + web_server.app.state.auth_required = True + try: + client = TestClient( + web_server.app, base_url="https://fly-app.fly.dev" + ) + r = client.get("/", follow_redirects=False) + assert r.status_code == 302, ( + f"Expected 302, got {r.status_code}: {r.text}" + ) + # Must redirect to /login (password form), NOT /auth/login (OAuth). + location = r.headers["location"] + assert "/login" in location, f"Expected redirect to /login, got {location}" + assert "/auth/login" not in location, ( + f"auto-SSO must NOT redirect to /auth/login for a password-only " + f"provider — that would crash with NotImplementedError. " + f"Got: {location}" + ) + finally: + clear_providers() + web_server.app.state.bound_host = prev_host + web_server.app.state.bound_port = prev_port + web_server.app.state.auth_required = prev_required