From 47ec029717955552d585532af82263e6eceda6db Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:03:19 +0100 Subject: [PATCH] fix: give a hand-off redeem back when the store fails, and hand out a sign-in whose tombstone couldn't be written #1066 --- app/api/auth/handoff/redeem/route.ts | 10 ++++++++-- lib/__tests__/auth-routes-store-down.test.ts | 16 ++++++++++++++++ lib/__tests__/shared-handoff-store.test.ts | 20 +++++++++++++++++--- lib/auth/handoff-store.ts | 10 ++++++++-- 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/app/api/auth/handoff/redeem/route.ts b/app/api/auth/handoff/redeem/route.ts index ce1856a68..eb18c4967 100644 --- a/app/api/auth/handoff/redeem/route.ts +++ b/app/api/auth/handoff/redeem/route.ts @@ -6,6 +6,7 @@ import { redeemHandoff, reserveHandoffRedeem, } from '@/lib/auth/handoff-store'; +import type { BudgetRefund } from '@/lib/auth/verify-budget'; import { loginClientAddress } from '@/lib/auth/client-address'; import { rejectCrossOriginRequest } from '@/lib/security/same-origin'; import { isStoreUnavailable, storeUnavailableResponse } from '@/lib/auth/store-unavailable'; @@ -26,8 +27,9 @@ const NO_STORE = { 'Cache-Control': 'no-store' }; export async function POST(request: NextRequest) { const crossOrigin = rejectCrossOriginRequest(request); if (crossOrigin) return crossOrigin; + let refund: BudgetRefund | null = null; try { - const refund = await reserveHandoffRedeem(loginClientAddress(request)); + refund = await reserveHandoffRedeem(loginClientAddress(request)); if (!refund) { return NextResponse.json({ error: 'too_many_requests' }, { status: 429, headers: NO_STORE }); } @@ -55,7 +57,11 @@ export async function POST(request: NextRequest) { logger.info('Mobile hand-off redeemed', { flow: result.bundle.flow }); return NextResponse.json({ ...result.bundle, state: result.state }, { headers: NO_STORE }); } catch (error) { - if (isStoreUnavailable(error)) return storeUnavailableResponse(NO_STORE); + if (isStoreUnavailable(error)) { + // Nothing was redeemed: the attempt doesn't count. + if (refund) await refund(); + return storeUnavailableResponse(NO_STORE); + } logger.error('Mobile hand-off redeem error', { error: error instanceof Error ? error.message : 'Unknown error' }); return NextResponse.json({ error: 'Internal server error' }, { status: 500, headers: NO_STORE }); } diff --git a/lib/__tests__/auth-routes-store-down.test.ts b/lib/__tests__/auth-routes-store-down.test.ts index 43872639b..d5b6bb427 100644 --- a/lib/__tests__/auth-routes-store-down.test.ts +++ b/lib/__tests__/auth-routes-store-down.test.ts @@ -152,3 +152,19 @@ describe('sign-in routes while the shared store is down', () => { expect(fetchSpy).toHaveBeenCalled(); }); }); + +describe('mobile hand-off redeem when the store fails after the budget was counted', () => { + it('gives the attempt back, so outages do not lock the app out', async () => { + store.down = false; + vi.stubEnv('SESSION_SECRET', 'a-test-session-secret-of-at-least-32-chars'); + const take = vi.spyOn(store.kv, 'take').mockRejectedValue(Object.assign(new Error('down (test)'), { name: 'StoreUnavailableError' })); + for (let i = 0; i < 12; i++) { + expect((await post('@/app/api/auth/handoff/redeem/route', { code: 'a'.repeat(64), code_verifier: 'v'.repeat(43) })).status).toBe(503); + } + take.mockRestore(); + const res = await post('@/app/api/auth/handoff/redeem/route', { code: 'a'.repeat(64), code_verifier: 'v'.repeat(43) }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_code'); + vi.unstubAllEnvs(); + }); +}); diff --git a/lib/__tests__/shared-handoff-store.test.ts b/lib/__tests__/shared-handoff-store.test.ts index 19e9a4409..2209e5a94 100644 --- a/lib/__tests__/shared-handoff-store.test.ts +++ b/lib/__tests__/shared-handoff-store.test.ts @@ -63,10 +63,24 @@ describe('mobile hand-off with a shared store', () => { }); it('spends an unconfirmed code on the first attempt, on any replica', async () => { - const code = await (await replica()).stashHandoff(PASSWORD_BUNDLE, { challenge: CHALLENGE, state: 's', awaitConfirm: true }, T0); + // Parked on A (the SSO callback), tried too early on B: A can't confirm it any more. + const a = await replica(); + const code = await a.stashHandoff(PASSWORD_BUNDLE, { challenge: CHALLENGE, state: 's', awaitConfirm: true }, T0); + const g = globalThis as Record; + const aMemory = g.__bulwarkMobileHandoffStore; expect(await (await replica()).redeemHandoff(code, VERIFIER, T0 + 1)).toEqual({ ok: false, reason: 'invalid' }); - expect(await (await replica()).confirmHandoff(code, T0 + 2)).toBe(false); - expect(await (await replica()).redeemHandoff(code, VERIFIER, T0 + 3)).toEqual({ ok: false, reason: 'invalid' }); + // Back on A, with A's own memory. + g.__bulwarkMobileHandoffStore = aMemory; + expect(await a.confirmHandoff(code, T0 + 2)).toBe(false); + expect(await a.redeemHandoff(code, VERIFIER, T0 + 3)).toEqual({ ok: false, reason: 'invalid' }); + }); + + it('still hands out the sign-in when the tombstone after the redeem cannot be written', async () => { + const code = await (await replica()).stashHandoff(PASSWORD_BUNDLE, { challenge: CHALLENGE, state: 's' }, T0); + const app = await replica(); + vi.spyOn(store.kv, 'set').mockRejectedValueOnce(Object.assign(new Error('down (test)'), { name: 'StoreUnavailableError' })); + expect(await app.redeemHandoff(code, VERIFIER, T0 + 1)).toMatchObject({ ok: true, bundle: PASSWORD_BUNDLE }); + vi.restoreAllMocks(); }); it('spends a code on a wrong verifier and tells a late attempt it expired', async () => { diff --git a/lib/auth/handoff-store.ts b/lib/auth/handoff-store.ts index 0b92eaf94..ba0a8fc0e 100644 --- a/lib/auth/handoff-store.ts +++ b/lib/auth/handoff-store.ts @@ -4,6 +4,7 @@ import { isMobileCodeChallenge } from '@/lib/auth/mobile-handoff'; import { createFailureBudget, type BudgetRefund } from '@/lib/auth/verify-budget'; import { decryptPayload, encryptPayload } from '@/lib/auth/crypto'; import { getStore, isSharedMode } from '@/lib/shared-store'; +import { logger } from '@/lib/logger'; // One-time codes for the browser sign-in hand-off to the mobile app // (lib/auth/mobile-handoff.ts, bulwarkmail.org/docs/development/mobile-handoff). The app opens the @@ -220,9 +221,14 @@ export async function redeemHandoff(code: string, verifier: string, now = Date.n const record = open(await kv.take(sharedKey(code)), now); if (!record) return { ok: false, reason: 'invalid' }; const bundle = record.expiresAt > now ? record.bundle : null; - // A tombstone goes back, so a late attempt is told "expired". + // A tombstone goes back, so a late attempt is told "expired". The code is + // spent already: if the tombstone can't be written, the answer stands. const spent = { ...record, bundle: null }; - await kv.set(sharedKey(code), seal(spent), { ttlMs: ttl(spent, now), ifAbsent: true }); + try { + await kv.set(sharedKey(code), seal(spent), { ttlMs: ttl(spent, now), ifAbsent: true }); + } catch (error) { + logger.warn('Mobile hand-off tombstone not written', { error: error instanceof Error ? error.message : 'Unknown error' }); + } return redeemOutcome(record, bundle, verifier, now); } sweep(now);