feat(restore): spare the requesting device its own "was this you?" alert (Future.md security #2)
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 <noreply@anthropic.com>
This commit is contained in:
parent
8496bbedac
commit
2dd09d5d0d
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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) }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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) })
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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<PushResult> {
|
||||
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 = {
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
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,
|
||||
)
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue