fix(auth): redirect OAuth login failures to login page instead of raw JSON (#1273)#2955
fix(auth): redirect OAuth login failures to login page instead of raw JSON (#1273)#2955dokterbob wants to merge 5 commits into
Conversation
… JSON (#1273) All user-facing failure paths in oauth_callback and oauth_azure_hf_callback now redirect to /login?error=oauthSignin instead of returning raw JSON or a bare 500. Developer-facing config errors (missing oauth_callback, unknown provider) remain HTTPException. The frontend i18n lookup now tries the raw error key before lowercasing so camelCase keys (oauthSignin) resolve to their specific messages. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR improves OAuth failure UX by redirecting OAuth callback errors to the login page (instead of returning raw JSON/500s), and updates the login error i18n lookup to correctly resolve camelCase error keys like oauthSignin. It also adds backend unit tests and a Cypress spec to cover key redirect/error-display paths.
Changes:
- Backend: Redirect more OAuth callback failure paths to
/login?error=oauthSignininstead of raisingHTTPException(JSON) or returning 500s. - Frontend: i18n lookup now tries the exact error key before attempting a lowercased fallback.
- Tests: Adds backend unit tests and a Cypress E2E spec for OAuth error redirects and friendly messaging.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
backend/chainlit/server.py |
Redirects additional OAuth failure paths to the login page instead of JSON/500 responses. |
backend/tests/test_server.py |
Adds regression tests for OAuth callback redirect behavior (including Azure hybrid flow). |
frontend/src/components/LoginForm.tsx |
Improves translation-key resolution for OAuth error query params (camelCase keys). |
cypress/e2e/oauth_auth/spec.cy.ts |
Adds E2E coverage for redirect-on-error and user-friendly login error message rendering. |
cypress/e2e/oauth_auth/main.py |
Provides a Chainlit app entrypoint configuring OAuth env vars for the new Cypress spec. |
…1273) - Rework `_get_oauth_redirect_error` to accept a `status_code` param (default 302); POST azure-hybrid handler now uses 303 (See Other) so the browser issues a GET to /login instead of re-POSTing. - Map all provider-returned OAuth errors (including access_denied) to the friendly `oauthSignin` key; raw provider code is logged for debugging. - Add `asyncio.CancelledError: raise` guard before the broad `except Exception` in both OAuth callback handlers. - Reword `oauthSignin` across all 23 locale files: "Sign in failed. Please try again, or use a different sign-in method." - Add `role="alert"` to `Alert.tsx` (ARIA + stable Cypress selector). - Fix Cypress `oauth_auth/main.py`: remove unused `# noqa: E402` that caused RUF100 lint failure in CI. - Update `spec.cy.ts`: assert new message text, fix ineffective body assertion (use content-type check), assert provider-error redirects to `error=oauthSignin`. - Tighten `test_server.py` redirect-status assertions to exact codes (302 for GET handler, 303 for POST handler) and check `oauthSignin` key for provider-error case. - Follow-up issue for access_denied-specific UX: #2956. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…n log to warning - Alert: apply role="alert" only for error variant; info uses role="status" (assertive announcement is appropriate only for errors, not notices) - oauth_callback: demote logger.exception → logger.warning for invalid state cookie (expected user-facing failure, not an application error) Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…apture logger.warning with exc_info=True keeps the traceback (needed by Sentry) while logging at WARNING rather than ERROR level, since an invalid OAuth state cookie is an expected user-facing event, not an application error. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…Signin key All redirect location assertions now check for `error=oauthSignin` rather than the generic `/login?error=`, and Cypress status assertions use exact 302 instead of oneOf([3xx]). This prevents regressions in error-key mapping that a looser check would miss. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
|
The Windows e2e shards are red ("Cypress binary is missing" on windows-latest-5, fail-fast cancellation on the other four), but this is pre-existing, repo-wide infrastructure — not introduced by this PR. The identical failure signature appears on the unrelated Root cause and proposed fixes are tracked in #2957. This PR can be reviewed and merged independently of that infra issue. |
|
Nice, this is a solid fix for #1273. OAuth failures redirect back to One thing though: I don't think it fully closes #2956. Happy to put up a small follow-up for that on top of this if you'd want it. |
Summary
oauth_callbackandoauth_azure_hf_callback) now redirect to/login?error=oauthSignininstead of returning raw JSON or a bare 500 — fixes the screenshot in UX bug: raw JSON on OAuth errors / Exception handling in auth #1273HTTPException(400/401), now redirect; developer-config errors (missingoauth_callback, unknown provider) remainHTTPExceptionoauthSigninnow resolve to their specific message ("Try signing in with a different account") instead of always falling back to the generic defaultTest plan
backend/tests/test_server.py) — all failure paths for both callback handlerscypress/e2e/oauth_auth/) — provider error redirect, invalid-state redirect, friendly message render at/login?error=oauthSigninuv run pytest tests/test_server.py— 61 passed)Closes #1273
🤖 Generated with Claude Code
Summary by cubic
Redirects all OAuth login failures to the login page with a clear, localized message and correct redirect codes. Fixes #1273.
oauth_callback(GET) andoauth_azure_hf_callback(POST) now redirect to/login?error=oauthSigninon user-facing failures; GET returns 302, POST returns 303 to avoid re-POST.Noneuser are all mapped tooauthSignin; raw provider codes are logged,asyncio.CancelledErroris re-raised, and state-validation logs at warning withexc_info=True.oauthSigninshows “Sign in failed. Please try again, or use a different sign-in method.”; login i18n checks the raw key before lowercasing;Alertusesrole="alert"for errors androle="status"otherwise; tests now assert exact 302/303 codes anderror=oauthSigninin redirect locations.Written for commit 1083c04. Summary will update on new commits.