Skip to content

fix: avoid 500 when password-only auth provider hits /auth/login - #553

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56666
Open

hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56666

Conversation

@hashbender

Copy link
Copy Markdown
Owner

Problem

The dashboard login flow calls GET /auth/login?provider=<name> for interactive providers. In hermes_cli/dashboard_auth/routes.py, auth_login() unconditionally calls p.start_login(...) once the provider exists and supports_session is true.

That breaks for BasicAuthProvider, whose start_login() intentionally raises:

NotImplementedError: BasicAuthProvider is password-only; there is no OAuth redirect flow.

Result: clicking the dashboard login button for the basic/password provider returns 500 Internal Server Error instead of a clean client error, and the stack trace bubbles all the way to ASGI.

Root cause

auth_login() only catches ProviderError, but BasicAuthProvider.start_login() raises NotImplementedError, so the request falls through as an unhandled exception.

Fix

  • Reject password-capable providers in GET /auth/login with a clean 400 explaining that the client must POST /auth/password-login instead.
  • Also catch NotImplementedError defensively and convert it into a 400 rather than a 500.

This preserves the intended split:

  • OAuth/redirect providers → GET /auth/login
  • Password providers → POST /auth/password-login

Repro

  1. Configure Hermes dashboard auth with the basic provider.
  2. Open /login.
  3. Click sign-in for that provider.
  4. Current behavior: 500 with BasicAuthProvider is password-only; there is no OAuth redirect flow.
  5. With this patch: clean 400 response instead of an internal server error.

Mirror-of: NousResearch#56666
NousResearch#56666

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant