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

@@ -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 });
   }

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();
+  });
+});

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<string, unknown>;
+    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 () => {

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);