HarperFast / HarperFast/oauth

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

Open
#227 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
1
Forks
1
Avg merge
2d 16h
Merged PRs (30d)
12

Description

## Summary

`handleLogin` awaits `provider.generateCSRFToken()` → `csrfTokenManager.set()` → `oauth.csrf_tokens.put()` with no error handling ([`src/lib/handlers.ts:158-163` @ 0aa719cc2d640db618295f9c2f63ce6b3e714d6d](https://github.com/HarperFast/oauth/blob/0aa719cc2d640db618295f9c2f63ce6b3e714d6d/src/lib/handlers.ts#L158-L163)). `CSRFTokenManager.set` logs and rethrows ([`CSRFTokenManager.ts:61-64`](https://github.com/HarperFast/oauth/blob/0aa719cc2d640db618295f9c2f63ce6b3e714d6d/src/lib/CSRFTokenManager.ts#L61-L64)), and the resource dispatch returns `handleLogin(...)` directly ([`resource.ts:405-406`](https://github.com/HarperFast/oauth/blob/0aa719cc2d640db618295f9c2f63ce6b3e714d6d/src/lib/resource.ts#L405-L406)), so a table-write failure escapes to Harper's default error serializer. The browser, which reached `/oauth//login` via a plain ``, lands on a JSON document:

```json
{"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`](https://github.com/HarperFast/oauth/blob/0aa719cc2d640db618295f9c2f63ce6b3e714d6d/src/lib/handlers.ts#L514-L531)) logs the error and redirects to `originalUrl || postLoginRedirect` with `error=auth_failed&reason=`, 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` @ 0aa719cc2d640db618295f9c2f63ce6b3e714d6d.

## Proposed change

Wrap the `generateCSRFToken` call in `handleLogin`:

```ts
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.

Contributor guide

Open the contributing guide

Research direction

Start in src/lib/handlers.ts around handleLogin and compare its CSRF-token failure path with handleCallback in src/lib/handlers.ts:514-531. Read CSRFTokenManager.ts and the dispatch in src/lib/resource.ts, then add or update the unit test for a rejecting csrfTokenManager.set. Done means the failure redirects with the stated reason, omits Set-Cookie, and logs the error.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
authentication, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.