Skip to content

OAuth sign-in error redirects carry error/reason params; check-oauth drops them and shows one generic "try again" toast #1674

Description

@heskew

Summary

When an OAuth sign-in fails after the browser has left the studio, @harperfast/oauth redirects back to postLoginRedirect (/#/check-oauth for the studio) with error and reason query params that say why. The studio never reads them. CheckOAuth calls getCurrentUser() and, on failure, shows the same toast for every case:

We were not able to verify your sign-in. Please try signing in again.

For several of the reasons the plugin sends, retrying is exactly the wrong advice.

What the plugin sends today

Callback-side failures in handleCallback (oauth src/lib/handlers.ts @ 0aa719cc2d640db618295f9c2f63ce6b3e714d6d) redirect to the original URL with:

Redirect When
error=auth_failed&reason=csrf state / browser-binding mismatch
error=auth_failed&reason=token_exchange|user_mapping|user_info|login_hook|unknown thrown during code exchange, userinfo, or the onLogin hook
error=oauth_failed&reason=<idp error> the IdP returned an error, e.g. access_denied
error=access_denied&reason=<app reason> the application's onLogin hook denied the login; the Fabric control plane's hook returns email_not_verified, provider_not_authorized, login_not_allowed, internal_error
error=invalid_request missing state or code

Login-side storage failures (the CSRF state write 503ing during a core write wedge, HarperFast/harper#2450) currently surface as a raw JSON error page instead of a redirect; HarperFast/oauth#227 tracks turning those into a redirect too, so the studio will start receiving error=server_error&reason=state_storage as well.

Why the studio can't see them even if it wanted to

The plugin appends the params with the URL API, which puts them before the hash: /?error=auth_failed&reason=csrf#/check-oauth. The studio router is createHashHistory() (src/router/useNewRouter.ts:13 @ 037a2fdb60ab25b334a2fb87c850bd6834e57e0c), so useSearch only sees the hash's own query string, which is empty. Reading them needs window.location.search, the pattern ProcessSetupIntent.tsx:14 already uses.

Where it shows

src/features/auth/CheckOAuth.tsx:26-31 @ 037a2fdb60ab25b334a2fb87c850bd6834e57e0c: getCurrentUser().catch(() => null) → generic toast → navigate to /sign-in. SignIn.tsx only reads me from search.

Proposed fix

  1. In CheckOAuth (or the auth layout), parse window.location.search for error / reason before calling getCurrentUser, and pick the message from the reason:
    • email_not_verified → "Verify your email address before signing in with this provider."
    • provider_not_authorized, login_not_allowed → "This sign-in method isn't allowed for your account."
    • csrf → "Your sign-in session expired. Please try again."
    • server_error, internal_error, unknown, token_exchange, user_info → "Sign-in is temporarily unavailable. Please try again in a few minutes."
    • IdP access_denied → "Sign-in was cancelled."
  2. Strip the params from the URL after reading them (as clearUtmParamsFromUrl does) so a reload doesn't re-toast.
  3. Keep the existing generic toast as the fallback when there is no error param but getCurrentUser still fails.

Context

Found while looking at a sign-in failure during a control-plane write wedge (HarperFast/harper#2450). The raw 503 the user saw on /oauth/google/login was at least honest; any redirect-with-reason from the plugin would have collapsed into the generic "try again" toast. Making sign-in degrade legibly is therefore a studio change first.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Fields

    Priority

    P3

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions