diff --git a/apps/auth/src/refresh-db.ts b/apps/auth/src/refresh-db.ts index 811de40..64f736f 100644 --- a/apps/auth/src/refresh-db.ts +++ b/apps/auth/src/refresh-db.ts @@ -47,24 +47,55 @@ export async function issueRefreshToken(db: D1Database, accountId: number): Prom return token } +/** + * Attempts per redemption. D1 occasionally answers a perfectly good statement with + * `D1_ERROR: internal error` — a storage-side hiccup carrying a support reference, not a + * verdict on the query. Unretried, one of those logs a player out: the grant throws, the + * shared error handler turns it into a 500, and the client falls back to the login + * screen with a refresh token it never got to spend. + * + * Deliberately small, like the Meta nonce retry next door: a refresh blocks the client on + * a loading screen, so two quick retries ride out a blip and a longer outage fails fast + * rather than hanging. + */ +const MAX_ATTEMPTS = 3 + /** * Redeem a refresh token: if it exists and hasn't expired, delete it (single-use * rotation) and return the account it logs in; otherwise return null. The delete is * atomic (`DELETE ... RETURNING`), so a token can't be redeemed twice — a * concurrent second attempt finds no row. An expired token is deleted and rejected. + * + * A D1 error is retried (see {@link MAX_ATTEMPTS}). That is safe precisely BECAUSE the + * statement is atomic and single-use: an attempt that actually committed before failing + * to answer leaves no row, so the retry returns null and the player re-logs in — exactly + * what a replayed token does. There is no interleaving in which retrying redeems one + * token twice, and none in which it lands worse than the 500 it replaces. + * + * Only the D1 call is retried; the hashing around it is pure. */ -export async function consumeRefreshToken( - db: D1Database, - token: string -): Promise { +export async function consumeRefreshToken(db: D1Database, token: string): Promise { const now = Math.floor(Date.now() / 1000) - const row = await db + const statement = db .prepare( `DELETE FROM refresh_tokens WHERE token_hash = ?1 RETURNING account_id AS accountId, expires_at AS expiresAt` ) .bind(await hashToken(token)) - .first<{ accountId: number; expiresAt: number }>() + + let row: { accountId: number; expiresAt: number } | null = null + for (let attempt = 1; ; attempt++) { + try { + row = await statement.first<{ accountId: number; expiresAt: number }>() + break + } catch (err) { + // The last attempt rethrows: a D1 outage is a 500, not a silent "bad token" that + // would tell the player to log in again over something that isn't their fault. + if (attempt === MAX_ATTEMPTS) throw err + await new Promise((resolve) => setTimeout(resolve, attempt * attempt * 250)) + } + } + if (!row || row.expiresAt < now) return null return row.accountId } diff --git a/apps/auth/src/test/integration/api.test.ts b/apps/auth/src/test/integration/api.test.ts index 08e9e67..3d1ee86 100644 --- a/apps/auth/src/test/integration/api.test.ts +++ b/apps/auth/src/test/integration/api.test.ts @@ -27,7 +27,7 @@ import { PLATFORM_BACKFILL_SQL, PLATFORM_SCHEMA_DDL, } from '../../platform-db' -import { REFRESH_SCHEMA_DDL } from '../../refresh-db' +import { consumeRefreshToken, issueRefreshToken, REFRESH_SCHEMA_DDL } from '../../refresh-db' import type { Env } from '../../context' @@ -1083,6 +1083,44 @@ describe('auth worker routes', () => { expect(reuse.json.error).toBe('invalid_grant') }) + /** + * A D1 that throws the storage-side `internal error` on its first `n` reads and then + * behaves. Only what `consumeRefreshToken` touches is wrapped — prepare/bind/first. + */ + const flakyDb = (failures: number): D1Database => { + let remaining = failures + const wrap = (stmt: D1PreparedStatement): D1PreparedStatement => + ({ + bind: (...values: unknown[]) => wrap(stmt.bind(...values)), + first: async () => { + if (remaining > 0) { + remaining-- + throw new Error('D1_ERROR: internal error; reference = testref0000') + } + return stmt.first() + }, + }) as unknown as D1PreparedStatement + return { prepare: (sql: string) => wrap(env.DB.prepare(sql)) } as unknown as D1Database + } + + test('consumeRefreshToken rides out a transient D1 error rather than logging the player out', async () => { + const token = await issueRefreshToken(env.DB, 77) + // Two hiccups, then the real thing — the redemption still succeeds, so the player + // keeps their session instead of being bounced to the login screen by a 500. + expect(await consumeRefreshToken(flakyDb(2), token)).toBe(77) + // And it was genuinely consumed: the retry didn't leave the row behind. + expect(await consumeRefreshToken(env.DB, token)).toBeNull() + }) + + test('consumeRefreshToken rethrows once the retries are spent, never a silent null', async () => { + const token = await issueRefreshToken(env.DB, 77) + // A real D1 outage has to surface as a 500. Answering null would tell the player + // their token was bad and make them log in again over something that isn't theirs. + await expect(consumeRefreshToken(flakyDb(99), token)).rejects.toThrow('internal error') + // The token survived, so a later attempt still works. + expect(await consumeRefreshToken(env.DB, token)).toBe(77) + }) + test('POST /connect/token 400s on an unknown refresh_token', async () => { const res = await postToken('grant_type=refresh_token&refresh_token=NOPE-1') expect(res.status).toBe(400)