fix(dashboard-auth): catch NotImplementedError in auth_login for password-only providers - #57445
Closed
benskls wants to merge 1 commit into
Closed
fix(dashboard-auth): catch NotImplementedError in auth_login for password-only providers#57445benskls wants to merge 1 commit into
benskls wants to merge 1 commit into
Conversation
…word-only providers BasicAuthProvider.start_login() raises NotImplementedError because it does not support OAuth redirect flows. When GET /auth/login?provider=basic is reached (e.g. by a browser bookmark, a redirect, or the auth gate landing on a password-only setup), the unhandled exception produces an HTTP 500 with no useful message. Catch NotImplementedError and redirect to the server-rendered /login page instead, which renders the password form for providers that declare supports_password=True. The next= parameter is preserved through the redirect so the user lands where they originally intended. Fixes the root cause of dashboard inaccessibility from mobile browsers when only basic_auth is configured.
Collaborator
Duplicate of #55993 — same code site ( |
Author
|
Closing as duplicate of #55993. Added a suggestion for URL query preservation on the original PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When only
dashboard.basic_authis configured (no OAuth provider), navigating toGET /auth/login?provider=basiccrashes with an HTTP 500 — no error page, no redirect, just "Internal Server Error".The root cause:
BasicAuthProvider.start_login()intentionally raisesNotImplementedErrorbecause password-only providers have no OAuth redirect flow. Theauth_loginroute catchesProviderErrorbut notNotImplementedError.This is reproducible from any mobile browser that bookmarks or auto-completes the
/auth/login?provider=basicURL, or when the auth gate lands on a password-only setup.Fix
Catch
NotImplementedErrorinauth_login()and redirect to the server-rendered/loginpage, which correctly renders the password form for providers withsupports_password=True. Thenext=parameter is preserved through the redirect.Steps to reproduce
dashboard.basic_authinconfig.yaml(no OAuth)hermes dashboard --host 0.0.0.0 --port 9119http://<ip>:9119/auth/login?provider=basic/login-> password form renders correctlyTesting
Verified locally with:
curl -s -o /dev/null -w "%{http_code}" http://localhost:9119/auth/login?provider=basic-> 302 (was 500)curl -s -o /dev/null -w "%{redirect_url}" "http://localhost:9119/auth/login?provider=basic&next=/projects"->/login?next=%2Fprojects(next= preserved)