Skip to content

/login propagates CSRF-state storage failures as a raw error response; /callback redirects with a reason code #227

Description

@heskew

Summary

handleLogin awaits provider.generateCSRFToken()csrfTokenManager.set()oauth.csrf_tokens.put() with no error handling (src/lib/handlers.ts:158-163 @ 0aa719cc2d640db618295f9c2f63ce6b3e714d6d). CSRFTokenManager.set logs and rethrows (CSRFTokenManager.ts:61-64), and the resource dispatch returns handleLogin(...) directly (resource.ts:405-406), so a table-write failure escapes to Harper's default error serializer. The browser, which reached /oauth/<provider>/login via a plain <a href>, lands on a JSON document:

{"type":"error:ServerError","code":"ServerError","title":"Outstanding write transactions have too long of queue, please try again later","status":503,"instance":"/oauth/google/login?redirect=%2F%23%2Fcheck-oauth"}

handleCallback treats the same class of failure differently: its catch (handlers.ts:514-531) logs the error and redirects to originalUrl || postLoginRedirect with error=auth_failed&reason=<generic code>, on the stated principle that details belong in the server log, not the browser. /login is the only human-flow endpoint that doesn't follow it.

Observed

Harper core's checkOverloaded() rejecting writes on a wedged worker thread (HarperFast/harper#2450). Every /login that landed on an affected thread returned the 503 above. Sign-in could not have proceeded regardless, since the state token has to be persisted before the redirect to the IdP, but the failure surfaced as a raw error page instead of the app's sign-in page. Observed on 2.5.1; the login path is unchanged on main @ 0aa719c.

Proposed change

Wrap the generateCSRFToken call in handleLogin:

let csrfToken: string;
try {
	csrfToken = await provider.generateCSRFToken({ originalUrl, sessionId: request.session?.id, providerName, browserNonceHash: hashBrowserSecret(browserSecret) });
} catch (error) {
	logger?.error?.('OAuth login: failed to store CSRF state:', error);
	return { status: 302, headers: { Location: buildErrorRedirect(originalUrl, { error: 'server_error', reason: 'state_storage' }) } };
}
  • Redirect target is originalUrl (already sanitized), matching the callback's tokenData.originalUrl || postLoginRedirect.
  • No Set-Cookie on the error path; the browser secret is only useful once a state token exists.
  • The raw error (which carries statusCode=503 from core) stays in the server log via the existing CSRFTokenManager.set error log.
  • Unit test: csrfTokenManager.set rejecting → 302 to originalUrl with error=server_error&reason=state_storage, no Set-Cookie, error logged.

Trade-off

A 302 hides the 503 from HTTP-level monitoring of /login. The server-side error log already covers it, and the app's sign-in page is the right place to explain a transient outage.

Not a fix for the outage

Nothing in the plugin can make sign-in work while core is rejecting writes; this only makes the failure land on the app's sign-in page with a reason code. Whether the app renders that reason is the app's problem: the Fabric studio currently drops every error / reason the plugin already sends (HarperFast/studio#1674), so on its own this change swaps an honest JSON 503 for a generic "try again" toast there. Land the studio change first, or together.

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