fix(crypto): stop telling users "waiting for partner" when their key is gone
decryptPartnerAnswer returned a nullable payload, and null meant two opposite things: the partner hasn't released their key yet (wait — all is well), or the keybox is sitting right there and this device cannot open it (your key is gone). The reveal UI rendered WAITING_FOR_PARTNER for both. Someone whose key was lost was told to keep waiting for something that had already happened — and could never report the bug accurately, because the app described it wrongly. A LOST_LOCAL_KEY phase with honest copy has existed the whole time; this path just never reached it. Now a typed PartnerAnswerResult: WaitingForPartner (no keybox), Decrypted, or KeyUnavailable (keybox present, no local key and no usable escrow — or a payload we hold the key for and still can't open, which is broken, not pending). Mutation-checked: collapsing KeyUnavailable back into WaitingForPartner fails exactly the three tests that assert the distinction. Repaired collateral from my own edit: the replacement over-cut and deleted releaseOwnKeyForThread / decryptOwnThreadAnswer / decryptPartnerThreadAnswer. Compile caught it; restored verbatim from git, then diffed the function list before/after to prove nothing else went missing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
3c74aec82d
commit
4952b129c5
|
|
@ -79,6 +79,20 @@ class SealedRevealManager @Inject constructor(
|
|||
* @return The decrypted [SealedAnswerEncryptor.AnswerPayload], or null if the
|
||||
* partner has not yet released their key.
|
||||
*/
|
||||
/**
|
||||
* Why this is a typed result and not a nullable payload: null conflated two opposite situations —
|
||||
* "the partner hasn't released their key yet" (wait, all is well) and "the keybox is right there
|
||||
* but we can't open it" (this device's key is gone). The UI showed WAITING_FOR_PARTNER for both,
|
||||
* so a user whose key was lost was told to keep waiting for something that had already happened.
|
||||
*/
|
||||
sealed interface PartnerAnswerResult {
|
||||
/** No keybox yet — the partner genuinely hasn't released their key. */
|
||||
data object WaitingForPartner : PartnerAnswerResult
|
||||
data class Decrypted(val payload: SealedAnswerEncryptor.AnswerPayload) : PartnerAnswerResult
|
||||
/** The keybox exists but this device can't open it: no local key and no usable escrow. */
|
||||
data object KeyUnavailable : PartnerAnswerResult
|
||||
}
|
||||
|
||||
suspend fun decryptPartnerAnswer(
|
||||
coupleId: String,
|
||||
date: String,
|
||||
|
|
@ -86,13 +100,13 @@ class SealedRevealManager @Inject constructor(
|
|||
partnerId: String,
|
||||
userId: String,
|
||||
encryptedPayload: String
|
||||
): SealedAnswerEncryptor.AnswerPayload? {
|
||||
): PartnerAnswerResult {
|
||||
val keybox = releaseKeyDataSource.readReleaseKey(
|
||||
coupleId = coupleId,
|
||||
date = date,
|
||||
senderUserId = partnerId,
|
||||
recipientUserId = userId
|
||||
) ?: return null
|
||||
) ?: return PartnerAnswerResult.WaitingForPartner
|
||||
|
||||
// loadPrivateKey — NOT getOrCreatePrivateKey. Generating a fresh keypair here would mismatch
|
||||
// the published public key the partner sealed to, turning a recoverable state into garbage.
|
||||
|
|
@ -100,7 +114,8 @@ class SealedRevealManager @Inject constructor(
|
|||
// first: that is exactly the case this used to fail silently on.
|
||||
val myPrivateKey = userKeyManager.loadPrivateKey()
|
||||
?: restorePrivateKeyFromEscrow(userId, coupleId)
|
||||
?: return null
|
||||
?: return PartnerAnswerResult.KeyUnavailable
|
||||
|
||||
val oneTimeKey = releaseKeyEncryptor.unwrapFromSender(
|
||||
keyboxB64 = keybox,
|
||||
recipientPrivateKey = myPrivateKey,
|
||||
|
|
@ -110,20 +125,17 @@ class SealedRevealManager @Inject constructor(
|
|||
recipientUserId = userId
|
||||
)
|
||||
|
||||
return sealedAnswerEncryptor.open(
|
||||
val payload = sealedAnswerEncryptor.open(
|
||||
encryptedPayload = encryptedPayload,
|
||||
keyHandle = oneTimeKey,
|
||||
coupleId = coupleId,
|
||||
questionId = questionId,
|
||||
userId = partnerId
|
||||
)
|
||||
// A keybox we hold the key for but still can't open is a broken/foreign payload, not a wait.
|
||||
return payload?.let { PartnerAnswerResult.Decrypted(it) } ?: PartnerAnswerResult.KeyUnavailable
|
||||
}
|
||||
|
||||
// ── Thread reveal ─────────────────────────────────────────────────────────────
|
||||
// Thread answers use the threadId (not a date) as the key-store identifier and
|
||||
// as the AAD "questionId", so daily-question and thread keys for the same
|
||||
// question ID are always cryptographically distinct.
|
||||
|
||||
suspend fun releaseOwnKeyForThread(
|
||||
coupleId: String,
|
||||
threadId: String,
|
||||
|
|
|
|||
|
|
@ -386,7 +386,7 @@ class AnswerRevealViewModel @Inject constructor(
|
|||
return
|
||||
}
|
||||
|
||||
val payload = runCatching {
|
||||
val result = runCatching {
|
||||
sealedRevealManager.decryptPartnerAnswer(
|
||||
coupleId = coupleId,
|
||||
date = effectiveDate(state.partnerAnswer),
|
||||
|
|
@ -395,11 +395,21 @@ class AnswerRevealViewModel @Inject constructor(
|
|||
userId = userId,
|
||||
encryptedPayload = encryptedPayload
|
||||
)
|
||||
}.onFailure { crashReporter.recordException(it) }.getOrNull()
|
||||
}.onFailure { crashReporter.recordException(it) }
|
||||
.getOrDefault(SealedRevealManager.PartnerAnswerResult.KeyUnavailable)
|
||||
|
||||
if (payload == null) {
|
||||
_uiState.update { it.copy(sealedRevealPhase = SealedRevealPhase.WAITING_FOR_PARTNER) }
|
||||
return
|
||||
val payload = when (result) {
|
||||
is SealedRevealManager.PartnerAnswerResult.Decrypted -> result.payload
|
||||
// Say the true thing. "Waiting for partner" here used to mean "your key is gone" as often
|
||||
// as it meant waiting, and the user had no way to tell — or to report it.
|
||||
SealedRevealManager.PartnerAnswerResult.KeyUnavailable -> {
|
||||
_uiState.update { it.copy(sealedRevealPhase = SealedRevealPhase.LOST_LOCAL_KEY) }
|
||||
return
|
||||
}
|
||||
SealedRevealManager.PartnerAnswerResult.WaitingForPartner -> {
|
||||
_uiState.update { it.copy(sealedRevealPhase = SealedRevealPhase.WAITING_FOR_PARTNER) }
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// The sealed payload carries only option IDs; map them to labels via the question so the
|
||||
|
|
|
|||
|
|
@ -0,0 +1,86 @@
|
|||
package app.closer.crypto
|
||||
|
||||
import app.closer.core.crash.CrashReporter
|
||||
import app.closer.data.remote.FirestoreDeviceKeyDataSource
|
||||
import app.closer.data.remote.FirestoreReleaseKeyDataSource
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* "Waiting for your partner" must mean the partner. Nothing else.
|
||||
*
|
||||
* This used to be a nullable payload, and null meant two opposite things: the partner hasn't released
|
||||
* their key yet (wait — all is well), or the keybox is right there and this device can't open it
|
||||
* (your key is gone). The UI said WAITING_FOR_PARTNER for both, so someone whose key was lost was
|
||||
* told to keep waiting for something that had already happened — a bug they could never report
|
||||
* accurately, because the app described it wrongly.
|
||||
*/
|
||||
class PartnerAnswerResultTest {
|
||||
|
||||
private val userKeyManager: UserKeyManager = mockk()
|
||||
private val pendingAnswerKeyStore: PendingAnswerKeyStore = mockk()
|
||||
private val releaseKeyEncryptor: ReleaseKeyEncryptor = mockk()
|
||||
private val sealedAnswerEncryptor: SealedAnswerEncryptor = mockk()
|
||||
private val deviceKeyDataSource: FirestoreDeviceKeyDataSource = mockk()
|
||||
private val releaseKeyDataSource: FirestoreReleaseKeyDataSource = mockk()
|
||||
private val encryptionManager: CoupleEncryptionManager = mockk()
|
||||
private val fieldEncryptor: FieldEncryptor = mockk()
|
||||
private val crashReporter: CrashReporter = mockk(relaxUnitFun = true)
|
||||
|
||||
private val manager = SealedRevealManager(
|
||||
userKeyManager, pendingAnswerKeyStore, releaseKeyEncryptor, sealedAnswerEncryptor,
|
||||
deviceKeyDataSource, releaseKeyDataSource, encryptionManager, fieldEncryptor, crashReporter
|
||||
)
|
||||
|
||||
private suspend fun decrypt() = manager.decryptPartnerAnswer(
|
||||
coupleId = "c1",
|
||||
date = "2026-07-16",
|
||||
questionId = "q1",
|
||||
partnerId = "partner",
|
||||
userId = "me",
|
||||
encryptedPayload = "sealed:v1:whatever"
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `no keybox yet means we really are waiting for the partner`() = runTest {
|
||||
coEvery { releaseKeyDataSource.readReleaseKey(any(), any(), any(), any()) } returns null
|
||||
|
||||
assertEquals(SealedRevealManager.PartnerAnswerResult.WaitingForPartner, decrypt())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `keybox present but no local key and no escrow is LOST, not waiting`() = runTest {
|
||||
// The exact second-device case. Saying "waiting for partner" here is the lie.
|
||||
coEvery { releaseKeyDataSource.readReleaseKey(any(), any(), any(), any()) } returns "keybox:v1:x"
|
||||
every { userKeyManager.loadPrivateKey() } returns null
|
||||
coEvery { deviceKeyDataSource.getPrivateKeyEscrow("me") } returns null
|
||||
|
||||
assertEquals(SealedRevealManager.PartnerAnswerResult.KeyUnavailable, decrypt())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `keybox present and escrow unreadable is LOST, not waiting`() = runTest {
|
||||
// Escrow exists but the couple key isn't here (or the blob is corrupt) — still lost, still
|
||||
// must not claim we're waiting.
|
||||
coEvery { releaseKeyDataSource.readReleaseKey(any(), any(), any(), any()) } returns "keybox:v1:x"
|
||||
every { userKeyManager.loadPrivateKey() } returns null
|
||||
coEvery { deviceKeyDataSource.getPrivateKeyEscrow("me") } returns "enc:v1:blob"
|
||||
every { encryptionManager.aeadFor("c1") } returns null
|
||||
|
||||
assertEquals(SealedRevealManager.PartnerAnswerResult.KeyUnavailable, decrypt())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `escrow failure is recorded, never thrown into the reveal flow`() = runTest {
|
||||
coEvery { releaseKeyDataSource.readReleaseKey(any(), any(), any(), any()) } returns "keybox:v1:x"
|
||||
every { userKeyManager.loadPrivateKey() } returns null
|
||||
coEvery { deviceKeyDataSource.getPrivateKeyEscrow("me") } throws RuntimeException("offline")
|
||||
|
||||
assertEquals(SealedRevealManager.PartnerAnswerResult.KeyUnavailable, decrypt())
|
||||
io.mockk.coVerify { crashReporter.recordException(any()) }
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue