fix(dashboard-auth): skip auto-SSO redirect for password-only providers - #59643
fix(dashboard-auth): skip auto-SSO redirect for password-only providers#59643IAnuragMahapatra wants to merge 1 commit into
Conversation
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 NousResearch#56067, NousResearch#57211, NousResearch#58166
Duplicate of #54887 -- the same |
|
Hey! I noticed the bot marked this as a duplicate of #54887. I checked the fix and the code in `middleware.py` is exactly the same but this PR also adds a regression test (`test_auto_sso_skips_password_only_provider`). The older PR does not have tests so it might be better to merge this one to avoid breaking the route again in the future. Let me know what you guys think! |
|
It is perfectly fine if we merge #54887 instead. I just wanted to point out that it is missing a regression test so the route might break again in the future. I am happy to open a quick follow-up PR to add just the test to whichever fix gets merged if that helps. |
|
Thanks for the focused fix and regression test. This is already implemented on current
|
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
What does this PR do?
Hermes dashboard auto-SSO silently redirects to
/auth/loginwhen exactly one auth provider is registered, skipping the login chooser. However,BasicAuthProvideris password-only — itsstart_login()method raisesNotImplementedError(by design).When basic auth is the only provider, the redirect hits
start_login()and crashes the dashboard with an HTTP 500.This PR adds a
supports_passwordguard in_auto_sso_responseso password-only providers fall through to_unauth_response→/login. Users now see the password login form instead of a server crash.Related Issue
Fixes #56067
Fixes #57211
Fixes #58166
Type of Change
Changes Made
hermes_cli/dashboard_auth/middleware.py: Added an early return in_auto_sso_response()whenprovider.supports_password is True.tests/hermes_cli/test_dashboard_auth_middleware.py: Added the regression testtest_auto_sso_skips_password_only_providerto ensureBasicAuthProvidercorrectly routes to/loginwithout crashing.How to Test
hermes-agentwith exactly one auth provider:basic./) without an active session cookie./loginpage containing the basic auth form, rather than encountering an HTTP 500 crash.pytest tests/hermes_cli/test_dashboard_auth_middleware.py -vand observe that the new testtest_auto_sso_skips_password_only_providerpasses.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AAI Usage Disclosure