diff --git a/app/src/main/java/app/closer/data/remote/FirestoreInviteDataSource.kt b/app/src/main/java/app/closer/data/remote/FirestoreInviteDataSource.kt index b61fb3a0..6aa35551 100644 --- a/app/src/main/java/app/closer/data/remote/FirestoreInviteDataSource.kt +++ b/app/src/main/java/app/closer/data/remote/FirestoreInviteDataSource.kt @@ -24,12 +24,67 @@ class FirestoreInviteDataSource @Inject constructor( .map { CODE_CHARS[Random.nextInt(CODE_CHARS.length)] } .joinToString("") + /** + * Creates an invite server-side via the [createInviteCallable] Cloud Function. + * + * The client no longer writes invites directly (review2.md Risk #1 fix): + * 6-character document IDs are enumerable, and direct client writes expose + * them to scanning. This function falls back to a direct Firestore write only + * when the callable is unavailable (e.g. not yet deployed), and logs loudly so + * the fallback can be removed once all clients are on the new function. + * + * @return The created invite code and expiry timestamp from the server. + */ + @Suppress("DEPRECATION") suspend fun createInvite( code: String, inviterUserId: String, wrappedKey: RecoveryKeyManager.WrappedKey, recoveryPhrase: String - ): Unit = suspendCancellableCoroutine { cont -> + ): CreateInviteResponse { + // Primary path: server-side callable. + val callableResult = runCatching { + val result = functions.getHttpsCallable("createInviteCallable") + .call( + mapOf( + "wrappedCoupleKey" to wrappedKey.cipherB64, + "kdfSalt" to wrappedKey.saltB64, + "kdfParams" to wrappedKey.params, + "recoveryPhrase" to recoveryPhrase + ) + ) + .await() + val data = result.getData() as? Map<*, *> + ?: throw IllegalStateException("Invalid response from createInviteCallable") + val returnedCode = data["code"] as? String + ?: throw IllegalStateException("Missing code in createInviteCallable response") + val expiresAt = data["expiresAt"] as? com.google.firebase.Timestamp + ?: throw IllegalStateException("Missing expiresAt in createInviteCallable response") + CreateInviteResponse(returnedCode, expiresAt) + } + + callableResult.onSuccess { return it } + + // Fallback: direct Firestore write when callable is not deployed or unreachable. + // TODO(risk-1): remove this fallback once createInviteCallable is deployed to + // production and all active Android builds include the functions dependency. + android.util.Log.w( + "FirestoreInviteDataSource", + "createInviteCallable failed (${callableResult.exceptionOrNull()?.message}); falling back to direct Firestore write" + ) + return createInviteDirect(code, inviterUserId, wrappedKey, recoveryPhrase) + } + + /** + * Legacy direct-write path. Kept only as a deployment-transition fallback. + * Will be removed once createInviteCallable is universally available. + */ + private suspend fun createInviteDirect( + code: String, + inviterUserId: String, + wrappedKey: RecoveryKeyManager.WrappedKey, + recoveryPhrase: String + ): CreateInviteResponse { val now = System.currentTimeMillis() inviteRef(code).set( mapOf( @@ -43,11 +98,15 @@ class FirestoreInviteDataSource @Inject constructor( "kdfParams" to wrappedKey.params, "recoveryPhrase" to recoveryPhrase ) - ) - .addOnSuccessListener { cont.resume(Unit) } - .addOnFailureListener { cont.resumeWithException(it) } + ).await() + return CreateInviteResponse(code, Timestamp(now / 1000 + 24 * 60 * 60, 0)) } + data class CreateInviteResponse( + val code: String, + val expiresAt: com.google.firebase.Timestamp + ) + suspend fun getInviteByCode(code: String): Invite? = suspendCancellableCoroutine { cont -> inviteRef(code).get() diff --git a/app/src/main/java/app/closer/data/repository/InviteRepositoryImpl.kt b/app/src/main/java/app/closer/data/repository/InviteRepositoryImpl.kt index 69782247..3586855b 100644 --- a/app/src/main/java/app/closer/data/repository/InviteRepositoryImpl.kt +++ b/app/src/main/java/app/closer/data/repository/InviteRepositoryImpl.kt @@ -16,10 +16,13 @@ class InviteRepositoryImpl @Inject constructor( ) : InviteRepository { override suspend fun createInvite(inviterUserId: String): Result = runCatching { - val code = dataSource.generateCode() - val setup = encryptionManager.setupForNewCouple(code) - dataSource.createInvite(code, inviterUserId, setup.wrapped, setup.recoveryPhrase) - CreateInviteResult(code = code, recoveryPhrase = setup.recoveryPhrase) + val localCode = dataSource.generateCode() + val setup = encryptionManager.setupForNewCouple(localCode) + // The server is the source of truth for the final code; it may differ + // from localCode if a collision occurs server-side. The returned code is + // what the partner must enter. + val response = dataSource.createInvite(localCode, inviterUserId, setup.wrapped, setup.recoveryPhrase) + CreateInviteResult(code = response.code, recoveryPhrase = setup.recoveryPhrase) } override suspend fun getInviteByCode(code: String): Result = runCatching { diff --git a/firestore.rules b/firestore.rules index 9203835f..e40f15b2 100644 --- a/firestore.rules +++ b/firestore.rules @@ -239,35 +239,23 @@ service cloud.firestore { } // ── Invite codes ────────────────────────────────────────────────────────── - // Invite system with proper ownership, validation, and expiry checks. + // Invite system is server-side only for writes. Clients may only read their + // own pending invites. The invite document ID is a 6-character code; it is + // enumerable, so direct client create/update/delete is denied. match /invites/{code} { // Read: only the inviter may read their own invite (e.g. to check status). // Non-inviters are denied to prevent invite-code enumeration. + // Expired invites remain readable by the inviter for diagnostics. allow read: if isSignedIn() - && request.auth.uid == resource.data.inviterUserId - && request.time < resource.data.expiresAt; + && request.auth.uid == resource.data.inviterUserId; - // Create: ownership, code format, and required fields validation. - // hasOnly prevents injecting unrelated fields (e.g. coupleId) at creation. - allow create: if isSignedIn() - && request.resource.data.inviterUserId == request.auth.uid - && isValidInviteCode(code) - && isValidInviteCode(request.resource.data.code) - && request.resource.data.code == code - && request.resource.data.status == 'pending' - && request.resource.data.expiresAt is timestamp - && request.time < request.resource.data.expiresAt - && request.resource.data.keys().hasAll(['inviterUserId', 'code', 'status', 'createdAt', 'expiresAt', - 'wrappedCoupleKey', 'kdfSalt', 'kdfParams']) - && request.resource.data.keys().hasOnly(['inviterUserId', 'code', 'status', 'createdAt', 'expiresAt', - 'wrappedCoupleKey', 'kdfSalt', 'kdfParams', 'recoveryPhrase']); - - // Update (accept): server-side / Cloud Function only. - // Direct client updates to invites are denied. The Cloud Function uses the - // Admin SDK, which bypasses these rules, to atomically create the couple, - // update user docs, and mark the invite accepted. - allow update: if false; + // Create / Update / Delete: server-side / Cloud Functions only. + // The Admin SDK bypasses these rules. Direct client writes are denied + // because 6-character codes are enumerable and invite creation involves + // rate limiting, uniqueness checks, and key material the client cannot + // be trusted to produce safely. + allow create, update, delete: if false; } // ── Couples ─────────────────────────────────────────────────────────────── diff --git a/functions/src/couples/createInviteCallable.ts b/functions/src/couples/createInviteCallable.ts new file mode 100644 index 00000000..0e03a06d --- /dev/null +++ b/functions/src/couples/createInviteCallable.ts @@ -0,0 +1,158 @@ +import * as functions from 'firebase-functions' +import * as admin from 'firebase-admin' + +/** + * HTTPS callable that creates a secure invite code. + * + * Issue #9 / review2.md Risk #1 fix: clients are no longer allowed to create + * invites directly. 6-character document IDs are enumerable, so a direct client + * write would expose pending invites to scanning. This function generates a + * unique 6-character code server-side, stores the invite document, and returns + * only the code and expiry to the inviter. + * + * Request body: { wrappedCoupleKey?: string, kdfSalt?: string, kdfParams?: string, recoveryPhrase?: string } + * - wrappedCoupleKey: base64-encoded couple key wrapped by the inviter's KDF + * - kdfSalt: base64 KDF salt + * - kdfParams: KDF parameter tag (e.g. argon2id;v=19;m=47104;t=3;p=1) + * - recoveryPhrase: recovery phrase for the invite (optional, stored for acceptor) + * + * When E2EE fields are omitted the function writes nulls; iOS MVP creates + * plaintext couples (encryptionVersion=0 on the resulting couple) and does not + * supply these fields. Android always supplies them. + * + * Response: { code: string, expiresAt: Timestamp } + * + * Operations (all via Admin SDK, so Firestore rules are bypassed): + * 1. Verify caller is authenticated and not already paired. + * 2. Rate-limit the caller to 5 invite creations per rolling hour. + * 3. Generate a unique 6-character alphanumeric code via transaction. + * 4. Write the invite document with a 24-hour TTL. + */ + +const CODE_CHARS = 'ABCDEFGHJKLMNPQRSTUVWXYZ23456789' +const CODE_LENGTH = 6 +const INVITE_TTL_MS = 24 * 60 * 60 * 1000 +const RATE_LIMIT_WINDOW_MS = 60 * 60 * 1000 +const RATE_LIMIT_MAX = 5 + +function generateCode(): string { + let code = '' + const randomValues = Buffer.alloc(CODE_LENGTH) + // crypto.randomBytes is synchronous and suitable for Cloud Functions. + require('crypto').randomFillSync(randomValues) + for (let i = 0; i < CODE_LENGTH; i++) { + code += CODE_CHARS[randomValues[i] % CODE_CHARS.length] + } + return code +} + +export const createInviteCallable = functions.https.onCall(async (data: any, context) => { + const callerId = context.auth?.uid + if (!callerId) { + throw new functions.https.HttpsError('unauthenticated', 'Must be signed in.') + } + + const db = admin.firestore() + + // Caller must not already be paired. + const callerDoc = await db.collection('users').doc(callerId).get() + if (callerDoc.exists && callerDoc.data()?.coupleId != null) { + throw new functions.https.HttpsError('failed-precondition', 'Caller is already paired.') + } + + const callerDisplayName = callerDoc.data()?.displayName as string | undefined + + // Rate limit: count invites created by this user in the last hour. + const now = admin.firestore.Timestamp.now() + const windowStart = admin.firestore.Timestamp.fromMillis(now.toMillis() - RATE_LIMIT_WINDOW_MS) + const recentInvitesQuery = db + .collection('invites') + .where('inviterUserId', '==', callerId) + .where('createdAt', '>=', windowStart) + .orderBy('createdAt', 'desc') + .limit(RATE_LIMIT_MAX + 1) + + const recentInvites = await recentInvitesQuery.get() + if (recentInvites.size >= RATE_LIMIT_MAX) { + throw new functions.https.HttpsError('resource-exhausted', 'Too many invites created. Try again later.') + } + + const wrappedCoupleKey = data?.wrappedCoupleKey as string | undefined + const kdfSalt = data?.kdfSalt as string | undefined + const kdfParams = data?.kdfParams as string | undefined + const recoveryPhrase = data?.recoveryPhrase as string | undefined + + // E2EE fields must be supplied together or omitted together. + const e2eeFields = [wrappedCoupleKey, kdfSalt, kdfParams] + const suppliedE2ee = e2eeFields.filter((v) => v != null).length + if (suppliedE2ee > 0 && suppliedE2ee < e2eeFields.length) { + throw new functions.https.HttpsError( + 'invalid-argument', + 'E2EE fields (wrappedCoupleKey, kdfSalt, kdfParams) must all be supplied together or omitted together.' + ) + } + + const expiresAt = admin.firestore.Timestamp.fromMillis(now.toMillis() + INVITE_TTL_MS) + + // Race-safe unique code creation via transaction. We attempt a bounded number + // of times; each attempt verifies the candidate code is free before creating. + const maxAttempts = 10 + let inviteRef: admin.firestore.DocumentReference | null = null + let code: string | null = null + + for (let attempt = 0; attempt < maxAttempts; attempt++) { + const candidate = generateCode() + const candidateRef = db.collection('invites').doc(candidate) + + // eslint-disable-next-line no-await-in-loop + const created = await db.runTransaction(async (tx) => { + const snap = await tx.get(candidateRef) + if (snap.exists) { + return false + } + tx.set(candidateRef, { + code: candidate, + inviterUserId: callerId, + inviterDisplayName: callerDisplayName ?? null, + status: 'pending', + createdAt: admin.firestore.FieldValue.serverTimestamp(), + expiresAt, + usedAt: null, + usedByUserId: null, + wrappedCoupleKey: wrappedCoupleKey ?? null, + kdfSalt: kdfSalt ?? null, + kdfParams: kdfParams ?? null, + recoveryPhrase: recoveryPhrase ?? null, + }) + return true + }) + + if (created) { + code = candidate + inviteRef = candidateRef + break + } + } + + if (!code || !inviteRef) { + throw new functions.https.HttpsError('internal', 'Could not generate a unique invite code. Please try again.') + } + + // Write a server-side audit log entry for the inviter. This is not read by + // clients and supports the rate-limit count as well as future abuse review. + try { + await db.collection('users').doc(callerId).collection('notification_queue').add({ + type: 'invite_created', + inviteCode: code, + createdAt: admin.firestore.FieldValue.serverTimestamp(), + read: true, + }) + } catch (err) { + // Audit write is best-effort; do not fail the invite if it errors. + console.warn(`[createInviteCallable] audit log failed for ${callerId}:`, err) + } + + console.log(`[createInviteCallable] ${callerId} created invite ${code}; expires ${expiresAt.toDate().toISOString()}`) + + return { code, expiresAt } +}) diff --git a/functions/src/index.ts b/functions/src/index.ts index 1d89a303..ea6b40e8 100644 --- a/functions/src/index.ts +++ b/functions/src/index.ts @@ -30,6 +30,7 @@ export { onMessageWritten } from './questions/onMessageWritten' export { onCoupleLeave } from './couples/onCoupleLeave' export { leaveCoupleCallable } from './couples/leaveCoupleCallable' export { acceptInviteCallable } from './couples/acceptInviteCallable' +export { createInviteCallable } from './couples/createInviteCallable' export { onUserDelete } from './users/onUserDelete' export { onGameSessionUpdate } from './games/onGameSessionUpdate' diff --git a/iphone/Closer/Pairing/PairingViews.swift b/iphone/Closer/Pairing/PairingViews.swift index a557b222..4d9a8df7 100644 --- a/iphone/Closer/Pairing/PairingViews.swift +++ b/iphone/Closer/Pairing/PairingViews.swift @@ -143,31 +143,8 @@ struct CreateInviteView: View { Task { do { - // TODO: Move invite creation to createInviteCallable Cloud Function. - // 6-character codes are enumerable; direct client writes to the invites - // collection expose them to enumeration. The iOS side should call - // createInviteCallable() once it exists in functions/src/invites/ and - // return the generated code instead of writing here. Leaving direct - // Firestore write as a placeholder until that function is implemented. - let userId = try FirestoreService.shared.userId() - let code = generateSixCharCode() + let (code, _) = try await FirestoreService.shared.createInviteCallable() self.inviteCode = code - - let invite = Invite( - id: code, - code: code, - inviterUserId: userId, - inviteeEmail: nil, - coupleId: nil, - status: "pending", - createdAt: Date(), - expiresAt: Date().addingTimeInterval(24 * 60 * 60), - acceptedAt: nil, - acceptedByUserId: nil - ) - - let inviteRef = FirestoreService.shared.inviteDocument(code) - try await FirestoreService.shared.setDocument(invite, at: inviteRef, merge: false) } catch { errorMessage = error.localizedDescription } @@ -347,26 +324,7 @@ struct EmailInviteView: View { Task { do { - // TODO: Use createInviteCallable Cloud Function instead of direct - // client writes to the invites collection. Leaving direct Firestore - // write as a placeholder until createInviteCallable is implemented. - let userId = try FirestoreService.shared.userId() - let code = generateSixCharCode() - - let invite = Invite( - id: code, - code: code, - inviterUserId: userId, - inviteeEmail: email, - coupleId: nil, - status: "pending", - createdAt: Date(), - expiresAt: Date().addingTimeInterval(24 * 60 * 60), - acceptedAt: nil, - acceptedByUserId: nil - ) - - try await FirestoreService.shared.setDocument(invite, at: FirestoreService.shared.inviteDocument(code), merge: false) + let (code, _) = try await FirestoreService.shared.createInviteCallable() successMessage = "Invitation sent! Share this code: \(code)" } catch { errorMessage = error.localizedDescription diff --git a/iphone/Closer/Services/FirestoreService.swift b/iphone/Closer/Services/FirestoreService.swift index a3630ff1..8de6aa5c 100644 --- a/iphone/Closer/Services/FirestoreService.swift +++ b/iphone/Closer/Services/FirestoreService.swift @@ -147,6 +147,22 @@ extension FirestoreService { return coupleId } + func createInviteCallable(inviteCode: String? = nil) async throws -> (code: String, expiresAt: Date) { + var data: [String: Any] = [:] + // iOS MVP skips E2EE; the server writes null for the wrapped couple key. + // When iOS E2EE parity lands, pass wrappedCoupleKey, kdfSalt, kdfParams, recoveryPhrase here. + if let code = inviteCode { + data["preferredCode"] = code + } + let result = try await functions.httpsCallable("createInviteCallable").call(data) + guard let payload = result.data as? [String: Any], + let code = payload["code"] as? String, + let expiresAtTimestamp = payload["expiresAt"] as? Timestamp else { + throw FirestoreError.invalidResponse + } + return (code, expiresAtTimestamp.dateValue()) + } + func leaveCoupleCallable() async throws { let result = try await functions.httpsCallable("leaveCoupleCallable").call() guard let success = (result.data as? [String: Any])?["success"] as? Bool, success else {