Context
Follow-up from the code review of the fix for #1053. That change adds prompt=select_account to the Google and GitHub authorization URLs, so the account chooser is now shown on every sign-in. This makes three pre-existing problems in the OAuth callback path much more reachable, because users will cancel or abandon the chooser far more often than they previously cancelled the one-time consent screen.
None of these are regressions introduced by the fix, and none block it.
1. Cancelling the chooser returns a raw axum 400 on the API origin
OAuthCallback (crates/api/src/routes/auth.rs:89-93) has code: String as a required field and no error field. When a user backs out of the provider's account chooser, Google and GitHub redirect to:
GET /v1/auth/callback?error=access_denied&state=...
with no code. Query<OAuthCallback> extraction fails before the handler runs, so the browser shows:
400 Failed to deserialize query string: missing field `code`
on the API domain. The oauth_states row and its frontend_callback are never read, so the UI's error page is never reached and the row is orphaned (see item 3).
Suggested fix: make code optional and add error / error_description to OAuthCallback. When error is present, look up and delete the state row, then redirect to frontend_callback with an error indicator instead of returning JSON.
2. Post-state-lookup failures return JSON instead of redirecting to the frontend
Only the success arm of the callback redirects to frontend_callback. The 401 OAuth authentication failed: {e} and 500 User creation failed: {e} / Session creation failed: {e} arms (crates/api/src/routes/auth.rs:437-465 and the session-creation arm below it) return JSON bodies on the API origin, and they interpolate raw internal error strings into the response.
Example scenario: with the chooser now always shown, a user picks a second GitHub account that has no public or verified email. handle_github_callback returns AuthFailed, the browser lands on raw 401 JSON, and because get_and_delete already consumed the state row, refreshing yields "Invalid or expired state".
Suggested fix: in every failure arm after the state row has been loaded, redirect to frontend_callback with a stable error code rather than returning JSON. Do not surface internal error text to the browser.
3. oauth_states rows from abandoned logins are never purged
OAuthStateRepository::cleanup_expired (crates/database/src/repositories/oauth_state.rs:104) has no production caller. Both cleanup_expired() calls in crates/services/src/auth/mod.rs (around lines 408 and 600) target session_repository, and there is no database-side TTL on the table.
Every login that does not reach get_and_delete (tab closed, browser Back, or chooser cancel, which per item 1 never hits the handler at all) leaves a row holding state, pkce_verifier, and frontend_callback forever. The table and its index grow without bound, and the forced chooser increases the abandonment rate.
Suggested fix: wire OAuthStateRepository::cleanup_expired into the same periodic cleanup that runs cleanup_expired_sessions.
Steps to reproduce (item 1)
- Click "Sign in with Google".
- On the Google account chooser, click "Cancel" or use the browser Back button after the chooser loads and choose a "return to app" link.
- Observe the raw 400 JSON on the API origin instead of the frontend error page.
Context
Follow-up from the code review of the fix for #1053. That change adds
prompt=select_accountto the Google and GitHub authorization URLs, so the account chooser is now shown on every sign-in. This makes three pre-existing problems in the OAuth callback path much more reachable, because users will cancel or abandon the chooser far more often than they previously cancelled the one-time consent screen.None of these are regressions introduced by the fix, and none block it.
1. Cancelling the chooser returns a raw axum 400 on the API origin
OAuthCallback(crates/api/src/routes/auth.rs:89-93) hascode: Stringas a required field and noerrorfield. When a user backs out of the provider's account chooser, Google and GitHub redirect to:with no
code.Query<OAuthCallback>extraction fails before the handler runs, so the browser shows:on the API domain. The
oauth_statesrow and itsfrontend_callbackare never read, so the UI's error page is never reached and the row is orphaned (see item 3).Suggested fix: make
codeoptional and adderror/error_descriptiontoOAuthCallback. Whenerroris present, look up and delete the state row, then redirect tofrontend_callbackwith an error indicator instead of returning JSON.2. Post-state-lookup failures return JSON instead of redirecting to the frontend
Only the success arm of the callback redirects to
frontend_callback. The 401OAuth authentication failed: {e}and 500User creation failed: {e}/Session creation failed: {e}arms (crates/api/src/routes/auth.rs:437-465and the session-creation arm below it) return JSON bodies on the API origin, and they interpolate raw internal error strings into the response.Example scenario: with the chooser now always shown, a user picks a second GitHub account that has no public or verified email.
handle_github_callbackreturnsAuthFailed, the browser lands on raw 401 JSON, and becauseget_and_deletealready consumed the state row, refreshing yields "Invalid or expired state".Suggested fix: in every failure arm after the state row has been loaded, redirect to
frontend_callbackwith a stable error code rather than returning JSON. Do not surface internal error text to the browser.3.
oauth_statesrows from abandoned logins are never purgedOAuthStateRepository::cleanup_expired(crates/database/src/repositories/oauth_state.rs:104) has no production caller. Bothcleanup_expired()calls incrates/services/src/auth/mod.rs(around lines 408 and 600) targetsession_repository, and there is no database-side TTL on the table.Every login that does not reach
get_and_delete(tab closed, browser Back, or chooser cancel, which per item 1 never hits the handler at all) leaves a row holdingstate,pkce_verifier, andfrontend_callbackforever. The table and its index grow without bound, and the forced chooser increases the abandonment rate.Suggested fix: wire
OAuthStateRepository::cleanup_expiredinto the same periodic cleanup that runscleanup_expired_sessions.Steps to reproduce (item 1)