diff --git a/PARITY.md b/PARITY.md index 1da4068..354d529 100644 --- a/PARITY.md +++ b/PARITY.md @@ -95,6 +95,7 @@ From `E2E/E2EInbox.swift`, which is the real feature list. | Session restore across restarts | โœ… credential and device both survive | | Reactions | ๐ŸŸก codec done, no UI โ€” blocked on the transcript carrying a `mid` | | Edit a sent message (ยง6.15) | ๐ŸŸก codec done, in progress โ€” an inbound edit is recognised and not drawn as a duplicate bubble; applying it needs the same `mid` | +| Drafts that survive a relaunch | โœ… sealed in the vault, not SharedPreferences | | Photos | โœ… send via the system picker, render inline | | Voice notes (record + play) | โœ… AAC/MP4, tap to record, tap to send | | Stories, and seen-state | โœ… 24h, sent per-contact, expired bytes deleted | diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt b/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt index 5c56328..587da32 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt @@ -1619,7 +1619,18 @@ fun ColumnScope.ChatScreen( /** Pictures, so a received bubble can carry its author's. */ avatars: Map = emptyMap(), ) { - var draft by remember { mutableStateOf("") } + // โš ๏ธ Keyed on the thread, so switching conversations re-reads rather than + // carrying one person's half-written message into somebody else's composer. + var draft by remember(chat.threadId) { mutableStateOf(model.openDraft()) } + // ๐Ÿ”ด **Every write to `draft` goes through here.** Sending clears the + // composer by assigning to the state directly in three places, and each one + // that forgot to also clear the stored copy would bring the message back the + // next time the conversation was opened โ€” as a draft of something already + // sent, which is worse than not having drafts at all. + val setDraft: (String) -> Unit = { text -> + draft = text + model.rememberDraft(text) + } val listState = rememberLazyListState() // Open AT the newest message โ€” a jump, not a tour of the history. @@ -1691,7 +1702,7 @@ fun ColumnScope.ChatScreen( onClick = { model.requestCall(chat.peerId) }, ) } - if (chat.peerId != null) ConversationMenu(model, chat, draft) { draft = "" } + if (chat.peerId != null) ConversationMenu(model, chat, draft) { setDraft("") } chat.group?.let { group -> IconAction( icon = KitIcon.LeaveGroup, @@ -1848,7 +1859,7 @@ fun ColumnScope.ChatScreen( } BasicTextField( value = draft, - onValueChange = { draft = it }, + onValueChange = setDraft, textStyle = LocalTextStyle.current.copy( fontSize = 14.sp, color = MaterialTheme.colorScheme.onSurface, @@ -1863,7 +1874,7 @@ fun ColumnScope.ChatScreen( onSend = { if (draft.isNotBlank()) { model.sendMessage(draft) - draft = "" + setDraft("") } } ), @@ -1922,7 +1933,7 @@ fun ColumnScope.ChatScreen( Button( onClick = { model.sendMessage(draft) - draft = "" + setDraft("") }, shape = CircleShape, contentPadding = PaddingValues(0.dp), diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index 8d1f519..a3c72eb 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -2194,6 +2194,37 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { }.onFailure { failure -> update { it.copy(error = describe(failure)) } } } + /** + * How the open conversation is named in the vault โ€” the same ids the + * transcript uses, so a group, a local contact and somebody on another + * relay cannot collide. + */ + private fun Phase.Chat.vaultThreadId(): String = when { + group != null -> Vault.threadForGroup(group.id) + address != null -> Vault.threadForAddress(address) + peerId != null -> Vault.threadForUser(peerId) + else -> threadId + } + + /** What was half-written in the open conversation, restored on open. */ + fun openDraft(): String { + val chat = signedIn()?.openChat ?: return "" + return vault?.draft(chat.vaultThreadId()).orEmpty() + } + + /** + * Keep, or forget, what is in the composer. + * + * โš ๏ธ Called on every change including back to empty โ€” clearing the + * composer has to clear what was stored, or a draft somebody deliberately + * deleted reappears the next time they open the conversation. + */ + fun rememberDraft(text: String) { + val chat = signedIn()?.openChat ?: return + val vault = vault ?: return + viewModelScope.launch(Dispatchers.IO) { vault.saveDraft(chat.vaultThreadId(), text) } + } + fun sendMessage(text: String) = viewModelScope.launch { val current = session ?: return@launch val chat = signedIn()?.openChat ?: return@launch diff --git a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt index 553b339..7684fc8 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt @@ -88,6 +88,7 @@ class Vault(private val storage: Storage, private val key: SecretKey) { private const val REPLY_REMINDER = "reply-reminder" private const val ROSTER = "roster" private const val GROUP_NAMES = "group-names" + private const val DRAFTS = "drafts" private const val GROUP_SECRETS = "group-secrets" private const val DEVICE_OWNER_PREFIX = "devowner/" private const val KT_ISSUER_PIN = "kt-issuer-pin" @@ -551,6 +552,40 @@ class Vault(private val storage: Storage, private val key: SecretKey) { writeJson(GROUP_NAMES, json) } + /** + * A half-written message, kept across a relaunch. + * + * ๐Ÿ”ด **In the vault rather than in SharedPreferences, and that is the whole + * point.** A draft is plaintext the person has not decided to send โ€” very + * often the most sensitive text in the app, because it is the thing they + * are still weighing. `SharedPreferences` writes it as readable XML that + * survives into backups; here it is sealed under the same key as the + * conversation, with the record's own name bound into the AAD, and + * [forgetEverything] takes it on sign-out. + * + * โš ๏ธ Keyed by the same `threadFor*` ids the transcript uses, so a group, a + * local contact and somebody on another relay cannot collide โ€” see the note + * on [threadForAddress] for why that third case is not theoretical. + */ + fun draft(threadId: String): String? = + readJson(DRAFTS)?.optString(threadId)?.takeIf { it.isNotEmpty() } + + /** + * โš ๏ธ Blank is not a draft. Storing " " would restore a composer that + * looks empty and reports itself as having content, which on this screen + * means an enabled send button that sends nothing. + * + * โš ๏ธ Stored AS TYPED though, not trimmed: somebody who left a trailing + * space was mid-word, and handing back a subtly different string than the + * one they left is the kind of small wrongness that makes an app feel + * untrustworthy. + */ + fun saveDraft(threadId: String, text: String) { + val json = readJson(DRAFTS) ?: JSONObject() + if (text.isBlank()) json.remove(threadId) else json.put(threadId, text) + writeJson(DRAFTS, json) + } + /** * Record a contact's picture. * diff --git a/engine/src/test/kotlin/io/flamenet/e2e/DraftsThatSurviveTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/DraftsThatSurviveTest.kt new file mode 100644 index 0000000..830efb6 --- /dev/null +++ b/engine/src/test/kotlin/io/flamenet/e2e/DraftsThatSurviveTest.kt @@ -0,0 +1,202 @@ +package io.flamenet.e2e + +import javax.crypto.KeyGenerator +import javax.crypto.SecretKey +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * Half-written messages, kept across a relaunch. + * + * ๐Ÿ”ด **The reason this is not a two-line SharedPreferences change** is where a + * draft has to live. It is plaintext the person has not decided to send โ€” very + * often the most sensitive text in the app, because it is the thing they are + * still weighing. `SharedPreferences` writes readable XML that survives into + * backups. In the vault it is sealed under the same key as the conversation, + * with the record's own name bound into the AAD, and `forgetEverything` takes + * it on sign-out. + */ +class DraftsThatSurviveTest { + + private class MapStorage : Vault.Storage { + val records = LinkedHashMap() + override fun read(name: String) = records[name] + override fun write(name: String, bytes: ByteArray) { records[name] = bytes } + override fun delete(name: String) { records.remove(name) } + override fun names() = records.keys.toList() + } + + private fun key(): SecretKey = + KeyGenerator.getInstance("AES").apply { init(256) }.generateKey() + + // ---------------------------------------------------------------- surviving + + @Test + fun `a draft survives being stored and read back`() { + val storage = MapStorage() + val secret = key() + val thread = Vault.threadForUser(7) + + Vault(storage, secret).saveDraft(thread, "meet at 6") + assertEquals("meet at 6", Vault(storage, secret).draft(thread)) + } + + /** + * ๐Ÿ”ด **On disk it must be ciphertext.** This is the assertion the whole + * ticket is about: the same feature built on SharedPreferences would put + * the text in readable XML, and no other test here would notice. + */ + @Test + fun `a draft is not readable on disk`() { + val storage = MapStorage() + Vault(storage, key()).saveDraft(Vault.threadForUser(7), "the thing I have not sent") + + val onDisk = storage.records.values.joinToString("") { bytes -> + bytes.joinToString("") { byte -> (byte.toInt() and 0xFF).toChar().toString() } + } + assertFalse(onDisk.contains("the thing I have not sent"), + "the draft was written in the clear") + } + + /** โš ๏ธ Multi-line text and awkward characters survive the round trip. */ + @Test + fun `awkward text survives`() { + val storage = MapStorage() + val secret = key() + val awkward = "line one\n\"quoted\"\ttabbed \\ backslash ๐Ÿ™‚" + + Vault(storage, secret).saveDraft(Vault.threadForUser(1), awkward) + assertEquals(awkward, Vault(storage, secret).draft(Vault.threadForUser(1))) + } + + // ---------------------------------------------------------------- forgetting + + /** + * ๐Ÿ”ด Clearing the composer has to clear what was stored. A draft that + * outlives the message it became comes back as a draft of something already + * sent, which is worse than not having drafts at all. + */ + @Test + fun `clearing the composer forgets the draft`() { + val storage = MapStorage() + val secret = key() + val thread = Vault.threadForUser(7) + + Vault(storage, secret).saveDraft(thread, "meet at 6") + Vault(storage, secret).saveDraft(thread, "") + assertNull(Vault(storage, secret).draft(thread)) + } + + /** + * โš ๏ธ Blank is not a draft. Storing " " would restore a composer that + * looks empty and reports itself as having content, which on this screen + * means an enabled send button that sends nothing. + */ + @Test + fun `whitespace is not a draft`() { + val storage = MapStorage() + val secret = key() + val thread = Vault.threadForUser(7) + + for (blank in listOf("", " ", " ", "\n", "\t\n ")) { + Vault(storage, secret).saveDraft(thread, "something") + Vault(storage, secret).saveDraft(thread, blank) + assertNull(Vault(storage, secret).draft(thread), + "kept \"$blank\", which would enable send on an empty composer") + } + } + + /** + * โš ๏ธ But stored as typed, not trimmed. Somebody who left a trailing space + * was mid-word, and handing back a subtly different string than the one + * they left is the kind of small wrongness that makes an app feel + * untrustworthy. + */ + @Test + fun `text is kept exactly as it was typed`() { + val storage = MapStorage() + val secret = key() + Vault(storage, secret).saveDraft(Vault.threadForUser(1), "meet at ") + assertEquals("meet at ", Vault(storage, secret).draft(Vault.threadForUser(1))) + } + + // ---------------------------------------------------------------- which thread + + /** + * ๐Ÿ”ด Three kinds of conversation share one record, so the ids must not + * collide. A group called "7", the person who is user 7, and an address + * "7" are three different threads โ€” and the transcript already names them + * apart for exactly this reason. + */ + @Test + fun `three kinds of conversation cannot share a draft`() { + val storage = MapStorage() + val secret = key() + + Vault(storage, secret).saveDraft(Vault.threadForUser(7), "to the contact") + Vault(storage, secret).saveDraft(Vault.threadForGroup("7"), "to the group") + Vault(storage, secret).saveDraft(Vault.threadForAddress("7"), "to the address") + + val vault = Vault(storage, secret) + assertEquals("to the contact", vault.draft(Vault.threadForUser(7))) + assertEquals("to the group", vault.draft(Vault.threadForGroup("7"))) + assertEquals("to the address", vault.draft(Vault.threadForAddress("7"))) + } + + @Test + fun `a conversation with no draft has none`() { + assertNull(Vault(MapStorage(), key()).draft(Vault.threadForUser(99))) + } + + /** Writing one draft must not disturb another. */ + @Test + fun `drafts do not overwrite each other`() { + val storage = MapStorage() + val secret = key() + + Vault(storage, secret).saveDraft(Vault.threadForUser(1), "first") + Vault(storage, secret).saveDraft(Vault.threadForUser(2), "second") + Vault(storage, secret).saveDraft(Vault.threadForUser(1), "") + + assertNull(Vault(storage, secret).draft(Vault.threadForUser(1))) + assertEquals("second", Vault(storage, secret).draft(Vault.threadForUser(2))) + } + + // ---------------------------------------------------------------- sign-out + + /** + * ๐Ÿ”ด Signing out must take drafts with it. Leaving the one piece of + * plaintext the person never chose to send behind, after the credential + * that justified it is gone, is exactly the residue the vault exists to + * avoid. + */ + @Test + fun `signing out forgets drafts`() { + val storage = MapStorage() + val secret = key() + + Vault(storage, secret).saveDraft(Vault.threadForUser(7), "meet at 6") + Vault(storage, secret).forgetEverything() + + assertTrue(storage.records.isEmpty(), "something survived sign-out") + } + + /** + * โš ๏ธ A vault opened with a different key cannot read the draft โ€” the same + * guarantee as everything else in here, asserted because a draft is the + * newest thing in it and the easiest to get wrong. + */ + @Test + fun `another key cannot read a draft`() { + val storage = MapStorage() + Vault(storage, key()).saveDraft(Vault.threadForUser(7), "meet at 6") + + val other = Vault(storage, key()) + val read = runCatching { other.draft(Vault.threadForUser(7)) } + assertNotEquals("meet at 6", read.getOrNull()) + } +}