From 2dd09d5d0d15a5b04a2e6557cdfb4069f2e758eb Mon Sep 17 00:00:00 2001 From: null Date: Wed, 15 Jul 2026 02:39:20 -0500 Subject: [PATCH] feat(restore): spare the requesting device its own "was this you?" alert (Future.md security #2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The restore self-alerts fan out to every device the recipient owns — including the new device that is doing the requesting. "Was this you?" sent to the asker is noise; the copies that matter go to their partner and to any OTHER device the real owner still holds (the phished-password-without-device-loss case), and those are untouched. For a legit single-device owner the self-alert becomes a clean no-op, which was the point. The requesting device identifies itself: RestoreManager fetches its own FCM token at request time and the doc carries it as a create-only, optional requesterFcmToken. Doc-embedded on purpose — MainActivity's token registration races the restore flow on a fresh device, so cross-referencing fcmTokens server-side can't reliably name the requester. Strictly best-effort on the client (runCatching → null → field omitted, never written hollow): a restore must never block or fail over a notification nicety, pinned by test. sendPushToUser gains optional excludeTokens (filtered after merge/dedupe; excluding everything is a clean zero no-op via the existing empty-list guard), threaded through the shared queueAndPush — the notification_queue record is still written, so the in-app alert history stays complete — and applied to the two self-alerts only; the partner "help them restore" push is deliberately unfiltered. Rules: requesterFcmToken joins the create allowlist as an optional plain string (opaque device identifier, no format to pin); partner-update and status-flip rules are unaffected since the field is create-only. Old clients never send it; the server reads it only if present — no deploy-order coupling. Tests: 3 new push.test.ts cases (exclusion, exclude-all no-op, absent-list unchanged) — mutation check on the filter kills exactly those; 2 new RestoreManagerTest cases (token embedded; FCM failure never blocks the request). Functions 101/101, tsc clean; Android suite + assembleDebug green. Deploy (scoped): firebase deploy --only firestore:rules, then --only functions:onRestoreRequested,functions:onRestoreFulfilled. Co-Authored-By: Claude Opus 4.8 --- .../app/closer/data/backup/RestoreManager.kt | 11 ++++- .../data/remote/FirestoreBackupDataSource.kt | 22 ++++++---- .../closer/data/backup/RestoreManagerTest.kt | 34 +++++++++++++- firestore.rules | 7 ++- functions/src/backup/onRestoreRequested.ts | 8 ++++ functions/src/notifications/push.test.ts | 44 +++++++++++++++++++ functions/src/notifications/push.ts | 10 ++++- functions/src/notifications/queueAndPush.ts | 5 ++- 8 files changed, 125 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/app/closer/data/backup/RestoreManager.kt b/app/src/main/java/app/closer/data/backup/RestoreManager.kt index 25f1c287..7c69a859 100644 --- a/app/src/main/java/app/closer/data/backup/RestoreManager.kt +++ b/app/src/main/java/app/closer/data/backup/RestoreManager.kt @@ -9,6 +9,8 @@ import app.closer.domain.model.RestoreRequest import app.closer.domain.model.RestoreStatus import app.closer.domain.repository.AuthRepository import app.closer.domain.repository.CoupleRepository +import com.google.firebase.messaging.FirebaseMessaging +import kotlinx.coroutines.tasks.await import java.security.SecureRandom import java.util.Base64 import javax.inject.Inject @@ -32,7 +34,8 @@ class RestoreManager @Inject constructor( private val coupleKeyTransfer: CoupleKeyTransfer, private val backupDataSource: FirestoreBackupDataSource, private val backupManager: BackupManager, - private val backupRestoreManager: BackupRestoreManager + private val backupRestoreManager: BackupRestoreManager, + private val messaging: FirebaseMessaging ) { /** What A shows on screen while waiting: the code to read aloud + who to observe. */ data class RestoreSession( @@ -57,7 +60,11 @@ class RestoreManager @Inject constructor( // status-only update can't touch keys) → PERMISSION_DENIED. Clear it first so a retry always works. // The recipient may delete their own request, and a fresh pubkey/nonce fully supersedes the old one. backupDataSource.deleteRestoreRequest(couple.id, uid) - backupDataSource.createRestoreRequest(couple.id, uid, pubKey, nonce, expiresAt) + // Identify this device so the server can skip it for the "was this you?" self-alert. + // Strictly best-effort: FCM may not have a token yet on a fresh install (registration + // races this flow), and a restore must never block or fail over a notification nicety. + val requesterToken = runCatching { messaging.token.await() }.getOrNull() + backupDataSource.createRestoreRequest(couple.id, uid, pubKey, nonce, expiresAt, requesterToken) RestoreSession( coupleId = couple.id, recipientUid = uid, diff --git a/app/src/main/java/app/closer/data/remote/FirestoreBackupDataSource.kt b/app/src/main/java/app/closer/data/remote/FirestoreBackupDataSource.kt index cfebe003..dbeaff36 100644 --- a/app/src/main/java/app/closer/data/remote/FirestoreBackupDataSource.kt +++ b/app/src/main/java/app/closer/data/remote/FirestoreBackupDataSource.kt @@ -189,17 +189,21 @@ class FirestoreBackupDataSource @Inject constructor( recipientUid: String, recipientPublicKey: String, requestNonce: String, - expiresAt: Long + expiresAt: Long, + requesterFcmToken: String? = null ) { restoreRequestRef(coupleId, recipientUid).set( - mapOf( - "recipientUid" to recipientUid, - "recipientPublicKey" to recipientPublicKey, - "requestNonce" to requestNonce, - "status" to RestoreStatus.REQUESTED.name, - "createdAt" to System.currentTimeMillis(), - "expiresAt" to expiresAt - ) + buildMap { + put("recipientUid", recipientUid) + put("recipientPublicKey", recipientPublicKey) + put("requestNonce", requestNonce) + put("status", RestoreStatus.REQUESTED.name) + put("createdAt", System.currentTimeMillis()) + put("expiresAt", expiresAt) + // The requesting device self-identifies so the server can spare it its own + // "was this you?" alert. Best-effort and never written hollow. + if (!requesterFcmToken.isNullOrBlank()) put("requesterFcmToken", requesterFcmToken) + } ).await() } diff --git a/app/src/test/java/app/closer/data/backup/RestoreManagerTest.kt b/app/src/test/java/app/closer/data/backup/RestoreManagerTest.kt index 3135ea02..9b1f219c 100644 --- a/app/src/test/java/app/closer/data/backup/RestoreManagerTest.kt +++ b/app/src/test/java/app/closer/data/backup/RestoreManagerTest.kt @@ -37,12 +37,13 @@ class RestoreManagerTest { private val backupDataSource: FirestoreBackupDataSource = mockk(relaxed = true) private val backupManager: BackupManager = mockk(relaxed = true) private val backupRestoreManager: BackupRestoreManager = mockk(relaxed = true) + private val messaging: com.google.firebase.messaging.FirebaseMessaging = mockk() private val couple = Couple(id = "c1", userIds = listOf("uidSam", "uidQA")) private fun manager() = RestoreManager( authRepository, coupleRepository, encryptionManager, userKeyManager, - coupleKeyTransfer, backupDataSource, backupManager, backupRestoreManager + coupleKeyTransfer, backupDataSource, backupManager, backupRestoreManager, messaging ) @Test @@ -51,13 +52,14 @@ class RestoreManagerTest { coEvery { coupleRepository.getCoupleForUser("uidQA") } returns couple every { userKeyManager.getOrCreatePrivateKey() } returns mockk() every { userKeyManager.publicKeyB64(any()) } returns "pub:v1:x" + every { messaging.token } returns com.google.android.gms.tasks.Tasks.forResult("tokQA") val result = manager().requestRestore() assertTrue(result.isSuccess) coVerifyOrder { backupDataSource.deleteRestoreRequest("c1", "uidQA") - backupDataSource.createRestoreRequest("c1", "uidQA", "pub:v1:x", any(), any()) + backupDataSource.createRestoreRequest("c1", "uidQA", "pub:v1:x", any(), any(), any()) } } @@ -109,4 +111,32 @@ class RestoreManagerTest { assertTrue(result.isFailure) assertFalse(result.exceptionOrNull()?.message?.contains("expired", ignoreCase = true) == true) } + + @Test + fun `requestRestore embeds this device's FCM token so the self-alert can skip it`() = runTest { + every { authRepository.currentUserId } returns "uidQA" + coEvery { coupleRepository.getCoupleForUser("uidQA") } returns couple + every { userKeyManager.getOrCreatePrivateKey() } returns mockk() + every { userKeyManager.publicKeyB64(any()) } returns "pub:v1:x" + every { messaging.token } returns com.google.android.gms.tasks.Tasks.forResult("tokQA") + + assertTrue(manager().requestRestore().isSuccess) + + coVerify { backupDataSource.createRestoreRequest("c1", "uidQA", "pub:v1:x", any(), any(), "tokQA") } + } + + @Test + fun `an FCM failure never blocks a restore - the token is a nicety, the request is the point`() = runTest { + // Fresh installs race token registration; FirebaseMessaging can also just be unavailable. + every { authRepository.currentUserId } returns "uidQA" + coEvery { coupleRepository.getCoupleForUser("uidQA") } returns couple + every { userKeyManager.getOrCreatePrivateKey() } returns mockk() + every { userKeyManager.publicKeyB64(any()) } returns "pub:v1:x" + every { messaging.token } returns com.google.android.gms.tasks.Tasks.forException(RuntimeException("fcm down")) + + val result = manager().requestRestore() + + assertTrue(result.isSuccess) + coVerify { backupDataSource.createRestoreRequest("c1", "uidQA", "pub:v1:x", any(), any(), null) } + } } diff --git a/firestore.rules b/firestore.rules index cc9d1c83..cb5dcc72 100644 --- a/firestore.rules +++ b/firestore.rules @@ -737,8 +737,13 @@ service cloud.firestore { && isPublicKey(request.resource.data.recipientPublicKey) && request.resource.data.status == 'REQUESTED' && !('keybox' in request.resource.data) + // Optional: the requesting device's own FCM token, so the self-alert can skip it. An + // opaque device string — deliberately NOT validated beyond type (no format to pin). + && (!('requesterFcmToken' in request.resource.data) + || request.resource.data.requesterFcmToken is string) && request.resource.data.keys().hasOnly( - ['recipientUid', 'recipientPublicKey', 'requestNonce', 'status', 'createdAt', 'expiresAt']); + ['recipientUid', 'recipientPublicKey', 'requestNonce', 'status', 'createdAt', 'expiresAt', + 'requesterFcmToken']); // The PARTNER (not the recipient) writes the keybox + flips status to READY; pubkey/nonce immutable. allow update: if isCouplesMember(coupleId) diff --git a/functions/src/backup/onRestoreRequested.ts b/functions/src/backup/onRestoreRequested.ts index 98ab09d7..380b944a 100644 --- a/functions/src/backup/onRestoreRequested.ts +++ b/functions/src/backup/onRestoreRequested.ts @@ -34,6 +34,10 @@ export const onRestoreRequested = onDocumentCreated( async (event) => { const { coupleId, recipientUid } = event.params const db = admin.firestore() + // The device that created the request identifies itself (create-only field, best-effort on the + // client). Used ONLY to spare that device its own "was this you?" — all other devices still get it. + const requesterToken = event.data?.data()?.requesterFcmToken + const excludeTokens = typeof requesterToken === 'string' && requesterToken ? [requesterToken] : undefined const coupleRef = db.collection('couples').doc(coupleId) const coupleDoc = await coupleRef.get() @@ -75,6 +79,7 @@ export const onRestoreRequested = onDocumentCreated( body: 'If this wasn’t you, secure your account now.', coupleId, bypassQuietHours: true, + excludeTokens, }) } } catch (e) { @@ -94,6 +99,8 @@ export const onRestoreFulfilled = onDocumentUpdated( const before = event.data?.before.data() const after = event.data?.after.data() if (!isRestoreReadyTransition(before?.status, after?.status)) return + const requesterToken = after?.requesterFcmToken + const excludeTokens = typeof requesterToken === 'string' && requesterToken ? [requesterToken] : undefined const { coupleId, recipientUid } = event.params const db = admin.firestore() @@ -110,6 +117,7 @@ export const onRestoreFulfilled = onDocumentUpdated( body: 'A new device now has access. If this wasn’t you, secure your account now.', coupleId, bypassQuietHours: true, + excludeTokens, }) } catch (e) { logger.warn('[onRestoreFulfilled] self-alert failed', { error: String(e) }) diff --git a/functions/src/notifications/push.test.ts b/functions/src/notifications/push.test.ts index 0c7f597e..4395023b 100644 --- a/functions/src/notifications/push.test.ts +++ b/functions/src/notifications/push.test.ts @@ -99,6 +99,50 @@ describe('batchResponseToResults', () => { }) describe('sendPushToUser', () => { + it('skips excluded tokens — the requesting device must not get its own "was this you?"', async () => { + const { db, deleted } = makeFakeDb({}, ['A', 'B', 'C']) + const sent: string[][] = [] + const messaging = { + sendEachForMulticast: async (msg: any) => { + sent.push(msg.tokens) + return batch(msg.tokens.map((_: string) => ({ success: true, messageId: 'm' }))) + }, + } as any + + const result = await sendPushToUser(db, messaging, 'u1', { notification: { title: 't', body: 'b' } }, undefined, ['B']) + + expect(sent).toEqual([['A', 'C']]) + expect(result).toEqual({ sent: 2, failed: 0, pruned: 0 }) + expect(deleted).toEqual([]) + }) + + it('excluding every token is a clean zero no-op, never a throw', async () => { + const { db } = makeFakeDb({}, ['A']) + const messaging = { + sendEachForMulticast: async () => { + throw new Error('must not be called with zero tokens') + }, + } as any + + const result = await sendPushToUser(db, messaging, 'u1', { notification: { title: 't', body: 'b' } }, undefined, ['A']) + + expect(result).toEqual({ sent: 0, failed: 0, pruned: 0 }) + }) + + it('an absent exclude list changes nothing', async () => { + const { db } = makeFakeDb({}, ['A', 'B']) + const sent: string[][] = [] + const messaging = { + sendEachForMulticast: async (msg: any) => { + sent.push(msg.tokens) + return batch(msg.tokens.map(() => ({ success: true, messageId: 'm' }))) + }, + } as any + + await sendPushToUser(db, messaging, 'u1', { notification: { title: 't', body: 'b' } }) + expect(sent).toEqual([['A', 'B']]) + }) + it('sends to every token and prunes exactly the permanently-dead ones', async () => { const { db, deleted, commits } = makeFakeDb({ fcmToken: 'LEG' }, ['LEG', 'DEAD', 'GOOD']) const messaging = makeFakeMessaging( diff --git a/functions/src/notifications/push.ts b/functions/src/notifications/push.ts index cb891d1b..c8aabf5b 100644 --- a/functions/src/notifications/push.ts +++ b/functions/src/notifications/push.ts @@ -65,6 +65,10 @@ export function batchResponseToResults( * round-trip, then prune any tokens FCM reports as permanently dead. Best-effort: never throws; * returns a per-user summary. A whole-batch failure (auth/quota, not a per-token death) is logged * and does not prune anything. + * + * `excludeTokens` skips specific devices — e.g. the restore self-alert must not go to the very + * device that requested the restore ("was this you?" to the asker is noise; their partner's and + * their other devices' copies are the signal). Excluding everything yields a clean zero no-op. */ export async function sendPushToUser( db: admin.firestore.Firestore, @@ -72,8 +76,12 @@ export async function sendPushToUser( uid: string, content: PushContent, userData?: admin.firestore.DocumentData, + excludeTokens?: string[], ): Promise { - const tokens = await getUserTokens(db, uid, userData) + let tokens = await getUserTokens(db, uid, userData) + if (excludeTokens && excludeTokens.length > 0) { + tokens = tokens.filter((t) => !excludeTokens.includes(t)) + } if (tokens.length === 0) return { sent: 0, failed: 0, pruned: 0 } const message: admin.messaging.MulticastMessage = { diff --git a/functions/src/notifications/queueAndPush.ts b/functions/src/notifications/queueAndPush.ts index 669e67c4..ca540f67 100644 --- a/functions/src/notifications/queueAndPush.ts +++ b/functions/src/notifications/queueAndPush.ts @@ -20,9 +20,11 @@ export async function queueAndPush( coupleId: string bypassQuietHours?: boolean channelId?: string + /** Devices to skip — see [sendPushToUser]. The queue entry is still written for them. */ + excludeTokens?: string[] } ): Promise { - const { type, title, body, coupleId, bypassQuietHours, channelId = 'partner_activity' } = opts + const { type, title, body, coupleId, bypassQuietHours, channelId = 'partner_activity', excludeTokens } = opts await db.collection('users').doc(uid).collection('notification_queue').add({ type, title, body, read: false, createdAt: admin.firestore.FieldValue.serverTimestamp(), }) @@ -42,5 +44,6 @@ export async function queueAndPush( android: { notification: { channelId } }, }, userData, + excludeTokens, ) }