diff --git a/firestore.indexes.json b/firestore.indexes.json index 529da8d7..d0b1f2d1 100644 --- a/firestore.indexes.json +++ b/firestore.indexes.json @@ -4,32 +4,56 @@ "collectionGroup": "invites", "queryScope": "COLLECTION", "fields": [ - { "fieldPath": "inviterUserId", "order": "ASCENDING" }, - { "fieldPath": "createdAt", "order": "DESCENDING" } + { + "fieldPath": "inviterUserId", + "order": "ASCENDING" + }, + { + "fieldPath": "createdAt", + "order": "DESCENDING" + } ] }, { "collectionGroup": "capsules", "queryScope": "COLLECTION_GROUP", "fields": [ - { "fieldPath": "status", "order": "ASCENDING" }, - { "fieldPath": "unlockAt", "order": "ASCENDING" } + { + "fieldPath": "status", + "order": "ASCENDING" + }, + { + "fieldPath": "unlockAt", + "order": "ASCENDING" + } ] }, { "collectionGroup": "questions", "queryScope": "COLLECTION", "fields": [ - { "fieldPath": "active", "order": "ASCENDING" }, - { "fieldPath": "isPremium", "order": "ASCENDING" } + { + "fieldPath": "active", + "order": "ASCENDING" + }, + { + "fieldPath": "isPremium", + "order": "ASCENDING" + } ] }, { "collectionGroup": "bucket_list", "queryScope": "COLLECTION", "fields": [ - { "fieldPath": "category", "order": "ASCENDING" }, - { "fieldPath": "addedAt", "order": "DESCENDING" } + { + "fieldPath": "category", + "order": "ASCENDING" + }, + { + "fieldPath": "addedAt", + "order": "DESCENDING" + } ] } ], @@ -39,6 +63,16 @@ "fieldPath": "expiresAt", "ttl": true, "indexes": [] + }, + { + "collectionGroup": "restore_requests", + "fieldPath": "expiresAt", + "indexes": [ + { + "queryScope": "COLLECTION_GROUP", + "order": "ASCENDING" + } + ] } ] } diff --git a/functions/src/backup/cleanupRestoreRequests.test.ts b/functions/src/backup/cleanupRestoreRequests.test.ts new file mode 100644 index 00000000..e7960cdf --- /dev/null +++ b/functions/src/backup/cleanupRestoreRequests.test.ts @@ -0,0 +1,164 @@ +import { + GRACE_MS, + FALLBACK_TTL_MS, + NOTIFY_WINDOW_MS, + shouldReapRestoreRequest, + shouldNotifyExpiry, + sweepExpiredRestoreRequests, +} from './cleanupRestoreRequests' +import { queueAndPush } from '../notifications/queueAndPush' + +jest.mock('../notifications/queueAndPush', () => ({ queueAndPush: jest.fn().mockResolvedValue(undefined) })) + +const NOW = 1_800_000_000_000 + +describe('shouldReapRestoreRequest', () => { + it('keeps a fresh request', () => { + expect(shouldReapRestoreRequest({ status: 'REQUESTED', expiresAt: NOW + 60_000 }, NOW)).toBe(false) + }) + + it('keeps an expired request still inside the grace window', () => { + // The client already treats it as expired; we wait out the grace so we can never race a + // mid-completion restore. + expect(shouldReapRestoreRequest({ status: 'READY', expiresAt: NOW - GRACE_MS + 1_000 }, NOW)).toBe(false) + }) + + it('reaps expired requests regardless of status — DECLINED-after-READY still carries the keybox', () => { + const expiresAt = NOW - GRACE_MS - 1_000 + for (const status of ['REQUESTED', 'READY', 'DECLINED', 'RESTORED', 'EXPIRED']) { + expect(shouldReapRestoreRequest({ status, expiresAt }, NOW)).toBe(true) + } + }) + + it('falls back to createdAt when expiresAt is unusable', () => { + expect(shouldReapRestoreRequest({ createdAt: NOW - FALLBACK_TTL_MS - 1 }, NOW)).toBe(true) + expect(shouldReapRestoreRequest({ expiresAt: 0, createdAt: NOW - FALLBACK_TTL_MS - 1 }, NOW)).toBe(true) + expect(shouldReapRestoreRequest({ createdAt: NOW - 60_000 }, NOW)).toBe(false) + }) + + it('leaves a doc with neither field — delete only on positive evidence', () => { + expect(shouldReapRestoreRequest({}, NOW)).toBe(false) + expect(shouldReapRestoreRequest({ expiresAt: 'soon', createdAt: null }, NOW)).toBe(false) + }) +}) + +describe('shouldNotifyExpiry', () => { + const justExpired = NOW - GRACE_MS - 60_000 + + it('notifies for a recently expired request still waiting on someone', () => { + expect(shouldNotifyExpiry({ status: 'REQUESTED', expiresAt: justExpired }, NOW)).toBe(true) + expect(shouldNotifyExpiry({ status: 'READY', expiresAt: justExpired }, NOW)).toBe(true) + }) + + it('stays silent for terminal states — RESTORED succeeded, DECLINED was answered', () => { + expect(shouldNotifyExpiry({ status: 'RESTORED', expiresAt: justExpired }, NOW)).toBe(false) + expect(shouldNotifyExpiry({ status: 'DECLINED', expiresAt: justExpired }, NOW)).toBe(false) + }) + + it('stays silent for backlog debris — first deploy must not blast ancient expiries', () => { + expect(shouldNotifyExpiry({ status: 'REQUESTED', expiresAt: NOW - NOTIFY_WINDOW_MS - GRACE_MS - 1_000 }, NOW)).toBe(false) + }) + + it('stays silent for the createdAt-fallback branch (no real expiry to report)', () => { + expect(shouldNotifyExpiry({ status: 'REQUESTED', createdAt: NOW - FALLBACK_TTL_MS - 1 }, NOW)).toBe(false) + }) + + it('never notifies for something it would not reap', () => { + expect(shouldNotifyExpiry({ status: 'REQUESTED', expiresAt: NOW + 60_000 }, NOW)).toBe(false) + expect(shouldNotifyExpiry({ status: 'REQUESTED', expiresAt: NOW - GRACE_MS + 1_000 }, NOW)).toBe(false) + }) +}) + +describe('sweepExpiredRestoreRequests', () => { + const mockQueueAndPush = queueAndPush as jest.Mock + + function fakeDoc(opts: { + id: string + data: Record + coupleExists?: boolean + deleteError?: Error + }) { + const coupleRef = { + id: `couple_${opts.id}`, + get: jest.fn().mockResolvedValue({ exists: opts.coupleExists ?? true }), + } + return { + id: opts.id, + data: () => opts.data, + ref: { + path: `couples/couple_${opts.id}/restore_requests/${opts.id}`, + parent: { parent: coupleRef }, + delete: opts.deleteError + ? jest.fn().mockRejectedValue(opts.deleteError) + : jest.fn().mockResolvedValue(undefined), + }, + } + } + + function fakeDb(docs: ReturnType[]) { + return { + collectionGroup: jest.fn().mockReturnValue({ + where: jest.fn().mockReturnValue({ + limit: jest.fn().mockReturnValue({ + get: jest.fn().mockResolvedValue({ size: docs.length, docs }), + }), + }), + }), + } as unknown as FirebaseFirestore.Firestore + } + + beforeEach(() => mockQueueAndPush.mockClear()) + + it('deletes the expired doc and nudges the requester', async () => { + const doc = fakeDoc({ id: 'uidA', data: { status: 'READY', recipientUid: 'uidA', expiresAt: NOW - GRACE_MS - 60_000 } }) + const counts = await sweepExpiredRestoreRequests(fakeDb([doc]), NOW) + + expect(doc.ref.delete).toHaveBeenCalledTimes(1) + expect(mockQueueAndPush).toHaveBeenCalledWith(expect.anything(), 'uidA', expect.objectContaining({ type: 'restore_request_expired' })) + expect(counts).toMatchObject({ scanned: 1, reaped: 1, notified: 1, failed: 0 }) + }) + + it('leaves a query hit that fails re-verify', async () => { + // The index says expired; the doc says otherwise (clock skew, weird data). The predicate wins. + const doc = fakeDoc({ id: 'uidB', data: { status: 'REQUESTED', expiresAt: NOW + 60_000 } }) + const counts = await sweepExpiredRestoreRequests(fakeDb([doc]), NOW) + + expect(doc.ref.delete).not.toHaveBeenCalled() + expect(counts).toMatchObject({ reaped: 0, skipped: 1 }) + }) + + it('skips the nudge for an orphan reap (couple gone) but still deletes', async () => { + const doc = fakeDoc({ + id: 'uidC', + data: { status: 'READY', expiresAt: NOW - GRACE_MS - 60_000 }, + coupleExists: false, + }) + const counts = await sweepExpiredRestoreRequests(fakeDb([doc]), NOW) + + expect(doc.ref.delete).toHaveBeenCalledTimes(1) + expect(mockQueueAndPush).not.toHaveBeenCalled() + expect(counts).toMatchObject({ reaped: 1, notified: 0 }) + }) + + it('one failing doc never stops the sweep', async () => { + const bad = fakeDoc({ + id: 'uidD', + data: { status: 'READY', expiresAt: NOW - GRACE_MS - 60_000 }, + deleteError: new Error('firestore unavailable'), + }) + const good = fakeDoc({ id: 'uidE', data: { status: 'REQUESTED', recipientUid: 'uidE', expiresAt: NOW - GRACE_MS - 60_000 } }) + const counts = await sweepExpiredRestoreRequests(fakeDb([bad, good]), NOW) + + expect(good.ref.delete).toHaveBeenCalledTimes(1) + expect(counts).toMatchObject({ scanned: 2, reaped: 1, failed: 1 }) + }) + + it('a notify failure costs the nudge, never the reap or the pass', async () => { + mockQueueAndPush.mockRejectedValueOnce(new Error('fcm down')) + const doc = fakeDoc({ id: 'uidF', data: { status: 'READY', recipientUid: 'uidF', expiresAt: NOW - GRACE_MS - 60_000 } }) + const counts = await sweepExpiredRestoreRequests(fakeDb([doc]), NOW) + + expect(doc.ref.delete).toHaveBeenCalledTimes(1) + expect(counts).toMatchObject({ reaped: 1, notified: 0, failed: 0 }) + }) +}) diff --git a/functions/src/backup/cleanupRestoreRequests.ts b/functions/src/backup/cleanupRestoreRequests.ts new file mode 100644 index 00000000..84a02253 --- /dev/null +++ b/functions/src/backup/cleanupRestoreRequests.ts @@ -0,0 +1,137 @@ +import * as admin from 'firebase-admin' +import { onSchedule } from 'firebase-functions/v2/scheduler' +import { queueAndPush } from '../notifications/queueAndPush' +import { logger } from '../log' + +/** + * Cloud Function: cleanupExpiredRestoreRequests + * + * A `restore_requests` doc left behind (partner wrapped the couple key but the recipient never + * completed, or nobody ever answered) keeps its ECIES `keybox` — ciphertext sealed only to the + * recipient, useless to anyone else, but key material has no business lying around. Expiry is + * otherwise enforced only client-side (fulfil-time check, delete-before-re-request, delete-on- + * complete), so a request whose device disappeared lives forever. + * + * Hourly sweep: requests expire 30 minutes after creation, so this bounds a stranded keybox's life + * to ~1.5 h. Uses a collectionGroup query on purpose — it also reaps requests orphaned under + * already-deleted couple docs, which an iterate-the-couples sweep can never see (deleted parents + * don't list). Needs the COLLECTION_GROUP fieldOverride on `restore_requests.expiresAt` in + * `firestore.indexes.json`. + * + * Admin SDK bypasses the recipient-only delete rule, which is the point: nobody else CAN clean these. + */ + +/** Never race a restore that is mid-completion: the client treats these as expired well before us. */ +export const GRACE_MS = 5 * 60 * 1000 +/** Reap docs with no usable `expiresAt` once they are unambiguously ancient. */ +export const FALLBACK_TTL_MS = 24 * 60 * 60 * 1000 +/** + * Only tell the requester about expiries that just happened (~2 sweep cycles). Without this bound, + * the first deploy would blast a notification for every ancient doc in the backlog at once. + */ +export const NOTIFY_WINDOW_MS = 2 * 60 * 60 * 1000 + +interface RestoreRequestFields { + status?: unknown + expiresAt?: unknown + createdAt?: unknown + recipientUid?: unknown +} + +const asMillis = (v: unknown): number => (typeof v === 'number' && v > 0 ? v : 0) + +/** + * Pure reap decision, re-verified per doc even though the query already filtered — the query is an + * index scan, this is the contract. Deletes only on positive evidence of staleness: + * a real `expiresAt` past the grace window, or (defensively — the create rule doesn't validate field + * values) no usable `expiresAt` but a `createdAt` at least a day old. A doc with neither is left + * alone for a human to wonder about. + */ +export function shouldReapRestoreRequest(data: RestoreRequestFields, nowMs: number): boolean { + const expiresAt = asMillis(data.expiresAt) + if (expiresAt > 0) return nowMs > expiresAt + GRACE_MS + const createdAt = asMillis(data.createdAt) + return createdAt > 0 && nowMs > createdAt + FALLBACK_TTL_MS +} + +/** + * Pure notify decision: only for a request that expired *recently* while still waiting on someone + * (REQUESTED: partner never approved; READY: recipient never completed). Terminal states are debris — + * RESTORED already succeeded and DECLINED was answered; telling anyone again is noise. + */ +export function shouldNotifyExpiry(data: RestoreRequestFields, nowMs: number): boolean { + const expiresAt = asMillis(data.expiresAt) + if (expiresAt <= 0 || nowMs <= expiresAt + GRACE_MS) return false + if (nowMs - expiresAt > NOTIFY_WINDOW_MS) return false + return data.status === 'REQUESTED' || data.status === 'READY' +} + +/** + * The sweep, with `db` injectable for tests. Per-doc failures are isolated (one bad doc can't stop + * the pass); the doc is deleted BEFORE the requester is notified so a notify failure costs a nudge, + * never a duplicate (the doc it would re-trigger on is already gone). + */ +export async function sweepExpiredRestoreRequests( + db: admin.firestore.Firestore, + nowMs: number +): Promise<{ scanned: number; reaped: number; notified: number; skipped: number; failed: number }> { + const snap = await db + .collectionGroup('restore_requests') + .where('expiresAt', '<=', nowMs - GRACE_MS) + .limit(200) + .get() + + const counts = { scanned: snap.size, reaped: 0, notified: 0, skipped: 0, failed: 0 } + + const results = await Promise.allSettled( + snap.docs.map(async (doc) => { + const data = (doc.data() ?? {}) as RestoreRequestFields + if (!shouldReapRestoreRequest(data, nowMs)) { + counts.skipped++ + logger.warn('[cleanupRestoreRequests] query hit failed re-verify; leaving doc', { path: doc.ref.path }) + return + } + const notify = shouldNotifyExpiry(data, nowMs) + await doc.ref.delete() + counts.reaped++ + + if (!notify) return + // Best-effort: a missed nudge is fine, a failed delete is not — hence delete-first above. + // Skip when the couple itself is gone (an orphan reap has nobody meaningful to notify). + try { + const coupleRef = doc.ref.parent.parent + const recipientUid = typeof data.recipientUid === 'string' && data.recipientUid ? data.recipientUid : doc.id + if (!coupleRef) return + const coupleDoc = await coupleRef.get() + if (!coupleDoc.exists) return + await queueAndPush(db, recipientUid, { + type: 'restore_request_expired', + title: 'Your restore request expired', + body: 'No worries — start a new one whenever you’re ready.', + coupleId: coupleRef.id, + }) + counts.notified++ + } catch (e) { + logger.warn('[cleanupRestoreRequests] expiry notice failed', { path: doc.ref.path, error: String(e) }) + } + }) + ) + counts.failed = results.filter((r) => r.status === 'rejected').length + return counts +} + +export const cleanupExpiredRestoreRequests = onSchedule( + { schedule: 'every 1 hours', timeoutSeconds: 180 }, + async () => { + try { + const counts = await sweepExpiredRestoreRequests(admin.firestore(), Date.now()) + logger.log( + `[cleanupRestoreRequests] scanned ${counts.scanned}; reaped ${counts.reaped}; ` + + `notified ${counts.notified}; skipped ${counts.skipped}; failed ${counts.failed}` + ) + } catch (e) { + // Never rethrow: a scheduled-function failure retries in a storm, and the next hourly run IS the retry. + logger.error('[cleanupRestoreRequests] sweep failed', { error: String(e) }) + } + } +) diff --git a/functions/src/backup/onRestoreRequested.ts b/functions/src/backup/onRestoreRequested.ts index 25db2545..98ab09d7 100644 --- a/functions/src/backup/onRestoreRequested.ts +++ b/functions/src/backup/onRestoreRequested.ts @@ -1,10 +1,8 @@ import * as admin from 'firebase-admin' import { onDocumentCreated, onDocumentUpdated } from 'firebase-functions/v2/firestore' -import { recipientInQuietHours } from '../notifications/quietHours' -import { sendPushToUser } from '../notifications/push' +import { queueAndPush } from '../notifications/queueAndPush' import { logger } from '../log' -const CHANNEL = 'partner_activity' // Suppress duplicate pushes when a request doc is rapidly deleted+recreated (a compromised account // could loop that to spam both partners) and when an event is redelivered. The in-app queue entry is // still written every time. A genuine re-request after the window still notifies — so this is a time @@ -22,38 +20,6 @@ export function isRestoreReadyTransition(beforeStatus: unknown, afterStatus: unk return afterStatus === 'READY' && beforeStatus !== 'READY' } -/** - * Write the durable in-app queue entry (always) and — unless quiet hours suppress it — push to every device. - * `bypassQuietHours` is for security signals (the "was this you?" self-alert) that must not be silenced. - */ -async function queueAndPush( - db: admin.firestore.Firestore, - uid: string, - opts: { type: string; title: string; body: string; coupleId: string; bypassQuietHours?: boolean } -): Promise { - const { type, title, body, coupleId, bypassQuietHours } = opts - await db.collection('users').doc(uid).collection('notification_queue').add({ - type, title, body, read: false, createdAt: admin.firestore.FieldValue.serverTimestamp(), - }) - - const userDoc = await db.collection('users').doc(uid).get() - if (!userDoc.exists) return - const userData = userDoc.data() - if (!bypassQuietHours && recipientInQuietHours(userData)) return - - await sendPushToUser( - db, - admin.messaging(), - uid, - { - notification: { title, body }, - data: { type, couple_id: coupleId }, - android: { notification: { channelId: CHANNEL } }, - }, - userData, - ) -} - /** * Fires when a member starts a partner-assisted restore * (`couples/{coupleId}/restore_requests/{recipientUid}` created on a new/wiped device). Sends TWO diff --git a/functions/src/index.ts b/functions/src/index.ts index 9f362f1b..70066816 100644 --- a/functions/src/index.ts +++ b/functions/src/index.ts @@ -33,6 +33,7 @@ export { onDateReflectionWritten } from './dates/onDateReflectionWritten' export { onDateReflectionRevealed } from './dates/onDateReflectionRevealed' export { onDateHistoryCreated } from './dates/onDateHistoryCreated' export { onRestoreRequested, onRestoreFulfilled } from './backup/onRestoreRequested' +export { cleanupExpiredRestoreRequests } from './backup/cleanupRestoreRequests' export { assignDailyQuestion, assignDailyQuestionCallable, diff --git a/functions/src/notifications/queueAndPush.ts b/functions/src/notifications/queueAndPush.ts new file mode 100644 index 00000000..669e67c4 --- /dev/null +++ b/functions/src/notifications/queueAndPush.ts @@ -0,0 +1,46 @@ +import * as admin from 'firebase-admin' +import { recipientInQuietHours } from './quietHours' +import { sendPushToUser } from './push' + +/** + * Write the durable in-app queue entry (always) and — unless quiet hours suppress it — push to every + * device. `bypassQuietHours` is for security signals (the "was this you?" self-alert class) that must + * not be silenced. + * + * Extracted from `backup/onRestoreRequested.ts` when the restore-request cleanup sweep needed the same + * behavior — one implementation, so queue-entry/quiet-hours/push semantics can't drift between callers. + */ +export async function queueAndPush( + db: admin.firestore.Firestore, + uid: string, + opts: { + type: string + title: string + body: string + coupleId: string + bypassQuietHours?: boolean + channelId?: string + } +): Promise { + const { type, title, body, coupleId, bypassQuietHours, channelId = 'partner_activity' } = opts + await db.collection('users').doc(uid).collection('notification_queue').add({ + type, title, body, read: false, createdAt: admin.firestore.FieldValue.serverTimestamp(), + }) + + const userDoc = await db.collection('users').doc(uid).get() + if (!userDoc.exists) return + const userData = userDoc.data() + if (!bypassQuietHours && recipientInQuietHours(userData)) return + + await sendPushToUser( + db, + admin.messaging(), + uid, + { + notification: { title, body }, + data: { type, couple_id: coupleId }, + android: { notification: { channelId } }, + }, + userData, + ) +}