Repository navigation
fix(server): map OAuth callback token-exchange failures per cause (424 vs 502) - #665
Merged
Merged
Conversation
…4 vs 502)
T-6087. GET /auth/:provider/cb answered 502 for every token-exchange failure.
Behind Cloudflare an origin 502 is replaced by Cloudflare's own text/plain
page, so a provider rejection lost its JSON body, AUTH_PROVIDER_ERROR code and
x-upup-request-id and looked like a Cloudflare outage.
Each cause is now mapped on its own:
- provider 4xx (expired/reused/forged code, or the integrator's client
id/secret/redirect URI rejected), or an OAuth `error` in a 200 body:
424 Failed Dependency. Retrying unchanged cannot succeed, clients do not
retry it, and Cloudflare passes it through.
- provider unreachable/timed out, provider 5xx/429/408, or a 200 with neither
a token nor an error: still 502, which clients retry.
The { error, code } body, the request id and the cleared nonce cookie are the
same for both. The statuses live in respond.ts as DEPENDENCY_REJECTED_STATUS /
UPSTREAM_UNAVAILABLE_STATUS + providerFailureStatus(); the response-contract
guard now allows no 502/504 literal in src/ except that one declaration.
Docs (server-http, error-codes, server README) and a patch changeset updated.
Builds on the preserved WIP branch wip/cf502-2026-10-26 (36d406d).
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
- providerFailureStatus: only a real 4xx (other than 408/429) is 424; an unfollowed 3xx or a 1xx is not a client rejection and is now 502. - A 200 whose body is not JSON is a malformed provider answer: 502 with the uniform body, instead of escaping to the router's generic 500. - Docs, changeset and constant comments no longer claim a 502 is "safe to retry": the callback URL is never replayable (state consumed, code single-use), so both statuses mean "start the sign-in again"; 424 = that cannot succeed until the cause is fixed, 502 = it may work. - Tests: helper edges (101/304/399), a 200 with an empty `error`, and a non-JSON 200.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes T-6087 (Cloudflare masks
@useupup/serverJSON 502s on the OAuth callback).Decision (MASTER rule, 2026-10-26 00:14Z)
GET /auth/:provider/cbhas two kinds of token-exchange failure, and the provider's answer tells them apart, so each is mapped separately:4xx(expired / reused / forged code, or the integrator's client id, secret or redirect URI rejected), or an OAutherrorin a200body5xx/429/408, or a200with neither a token nor an errorThe
{ error: 'Token exchange failed', code: 'AUTH_PROVIDER_ERROR' }body, thex-upup-request-idheader and the cleared nonce cookie are identical for both.onErrorgets the same status the client saw. The refresh path's report-only stall status is still 502, now through the named constant.Changes
respond.ts:DEPENDENCY_REJECTED_STATUS(424),UPSTREAM_UNAVAILABLE_STATUS(502) andproviderFailureStatus().oauth.ts: the callback's three failure branches use them.tests/oauth-callback-failure-status.test.ts: per-cause matrix covering 400/401/403/404/422 → 424; 500/502/503/504/429/408 → 502; timeout → 502; 200 with anerror→ 424; 200 with neither → 502. Each case checks the body, the request id, the cleared cookie, that no token is stored, and theonErrorstatus. It also pins the helper and re-checks that a forged callback is 403 and a bad state is 400.tests/response-contract.test.ts: the only 502/504 literal allowed insrc/is theUPSTREAM_UNAVAILABLE_STATUSdeclaration.server-modeandoauth-cookie-hardeningtests: a provider400is now 424.Trade-off: a transient 502 is still replaced by Cloudflare's page behind Cloudflare. That is by design under the rule, and the cause still reaches
onError.Verification
Built on devino-* (no local build or test), and committed and pushed through the normal husky hooks:
Builds on the preserved WIP
wip/cf502-2026-10-26(36d406d).