From e619fdcb410678f594cbcf3f3cba8d5c9a1d8964 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 14:10:26 +0000 Subject: [PATCH] fix: make the location-chat identity migration finish on its own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps left by 04ef4128, both of which would have surfaced as a half-finished release at the flag flip rather than as a failure now. The mirror ignored the retirement switch. LEGACY_WRITES_RETIRED was private to LocalPreferences, so GeohashChatIdentityState could not read it and wrote to its legacy file unconditionally. Flipping the flag would have retired the four documented mirrors and left this one running. It is `internal` now and both of that class's legacy writes are gated on it, so the flip is one switch that stops everything. Nothing would ever have deleted the identity's legacy file. It is `secret_keeper_`, not `secret_keeper_` — the writer passed signer.pubKey where every other caller passes an npub — and the cleanup enumerates npub-keyed files. It would have sat on disk holding a seed after every other legacy file was gone. LegacyAccountFiles.delete now clears and unlinks both, and verify() refuses while the hex file holds an identity the current store does not. That check reads geohashSource(), the hex file. An earlier version of it read the npub file and was reverted for being unable to fire; this is the corrected form, and it is paired with actually deleting the file it guards. The copy also had to stop being lazy, and that is the part that would have bitten users rather than the release. It ran only from GeohashChatIdentityState, so only for someone who opened a location chat. Combined with the new gate that is worse than the orphan it replaced: a user with an identity who never opens another location chat would never be copied, and their legacy files could then never be deleted at all. It now runs on the first account load after the upgrade, beside the cleanup call and before it, so every existing install converges whether or not location chat is ever touched again. It sits next to legacyCleanup rather than inside the loader because innerLoadCurrentAccountFromEncryptedStorage is at the JVM's 64KB method limit — AccountStoreData's KDoc already records that every suspend call in there costs a coroutine state, and adding one broke the build with "Method too large". Outside the lock, once per load, is also where it belongs: this is migration housekeeping, not part of building AccountSettings. Tests: four on the gate (an uncopied identity blocks deletion, a copied one does not, an account that never opened a location chat is not held hostage, and both files go together) and one on idempotence, which the copy now needs because it runs on every load. The plan doc is updated to match: the group and its two quirks, deletion covering both files, the fourth condition, the flip stopping this mirror too, and a device-pass step for the seed — the one migrated value whose loss is silent rather than visible. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AXvKXakvup4inNFfAhhr4L --- ...2026-09-23-encrypted-storage-retirement.md | 41 +++++++-- .../amethyst/AccountSecretsStore.kt | 8 ++ .../amethyst/LegacyPreferenceCleanup.kt | 54 ++++++++++++ .../amethyst/LocalPreferences.kt | 68 +++++++++++++- .../model/GeohashChatIdentityState.kt | 11 ++- .../amethyst/LegacyPreferenceCleanupTest.kt | 88 +++++++++++++++++++ .../preferences/AccountSecretsStoreTest.kt | 21 +++++ 7 files changed, 278 insertions(+), 13 deletions(-) diff --git a/amethyst/plans/2026-09-23-encrypted-storage-retirement.md b/amethyst/plans/2026-09-23-encrypted-storage-retirement.md index ceb220068c..b2eccd2986 100644 --- a/amethyst/plans/2026-09-23-encrypted-storage-retirement.md +++ b/amethyst/plans/2026-09-23-encrypted-storage-retirement.md @@ -42,10 +42,20 @@ to none of those fails that test at the commit that adds it. | NIP-46 material, wallets, payment source | the account's encrypted DataStore | **kept** | | current account, saved accounts | the encrypted roster store | **kept** | | UI settings (`shared_settings`) | `UiSharedPreferences`' own DataStore | none left | +| location-chat identity — seed, nickname | the account's encrypted DataStore, as its own `GeohashIdentitySecrets` group | **kept** | -The three stores that still mirror are the ones whose loss is not an -annoyance: an account that cannot be listed, signed with, or paid from. They -keep the rollback window open until the device pass below has happened. +The stores that still mirror are the ones whose loss is not an annoyance: an +account that cannot be listed, signed with, or paid from. They keep the +rollback window open until the device pass below has happened. + +The location-chat identity is the odd one, in two ways worth knowing before +step 4. It is its **own** group rather than fields on `AccountSecrets`: every +account save mirrors a whole `AccountSecrets` built from `AccountSettings`, +which does not hold these, and that group save removes keys whose value is +null — folded in, the seed would be deleted by the next unrelated save and +every geohash identity the user has would silently change. And its legacy home +is a **different file**, `secret_keeper_`, because its writer +passed `signer.pubKey` where every other caller passes an npub. The UI settings copy is **guarded** where the others are not. That store has been the real home of these settings for a while, so most installs already @@ -69,7 +79,11 @@ cleanup treat any *other* unclaimed key as a reason to keep the file. ## Deleting a legacy file `LegacyPreferenceCleanup` runs after every successful account load and deletes -that account's file only when it can prove nothing would be lost: +that account's files — `secret_keeper_` **and** the location-chat +identity's `secret_keeper_` — only when it can prove nothing would +be lost. Both, because nothing else would ever remove the second one: the +cleanup enumerates npub-keyed files, so left out of this it would sit on disk +holding a seed forever. 1. **Every key in the file is accounted for** — claimed by a table, one of the secrets, or on the accepted list. Driven from the file's own keys, not from @@ -83,9 +97,14 @@ that account's file only when it can prove nothing would be lost: 3. **The secrets and the private key read back identical** from the current stores. Those *are* still dual-written, so the stronger question is available and is asked. -4. A store that cannot be read is a reason, never a pass. +4. **The location-chat identity has been copied**, when its file holds one. + Read from the hex-keyed file, not the npub one — these two keys were never + in that one, so a check pointed at it would never fire. An account that + never opened a location chat holds neither key, which is a real answer and + must not hold the file hostage. +5. A store that cannot be read is a reason, never a pass. -It refuses today, and says so, because of the fifth condition: +It refuses today, and says so, because of the last condition: `LEGACY_WRITES_RETIRED` is false. While the app still mirrors into the file, deleting it achieves nothing — the next save recreates it — and would look like it had worked. @@ -97,7 +116,10 @@ like it had worked. 3. Do the device pass below. 4. Flip `LEGACY_WRITES_RETIRED` and drop the legacy writes for the identity, key, secret and roster stores. This ends the rollback window, so it is a - release of its own. + release of its own. The flag is `internal`, not private, so the + location-chat mirror in `GeohashChatIdentityState` reads the same switch — + flipping it stops that write too, and the cleanup then removes both of the + account's legacy files. One flip, nothing left behind. 5. Keep the reader, and `androidx.security.crypto`, indefinitely. ## Verification this needs and has not had @@ -110,6 +132,9 @@ the one irreversible step in the whole series. On a real device, before step 4 ships: upgrade an install holding accounts and confirm they all list; open one and sign; force-stop and relaunch; add and remove an account; pair a NIP-46 signer; pay from a wallet; check the -key-backup nudge stays dismissed; confirm UI settings survive the upgrade. +key-backup nudge stays dismissed; confirm UI settings survive the upgrade; +open a location chat under a bunker or external signer and confirm the +throwaway identity and nickname are the same ones as before the upgrade — the +seed is the one migrated value whose loss is silent rather than visible. Then let the cleanup run with the flag flipped, and confirm the files are gone and everything above still holds on the next cold start. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountSecretsStore.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountSecretsStore.kt index ee32dde855..2e73084664 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountSecretsStore.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AccountSecretsStore.kt @@ -127,6 +127,14 @@ class AccountSecretsStore( return legacy } + /** + * What the current store holds for the location-chat identity, with no + * fallback to the legacy file — the same distinction [stored] draws, and + * for the same reader: [LegacyPreferenceCleanup] has to tell "migrated" + * from "falling back and looking migrated". + */ + suspend fun storedGeohashIdentity(npub: String): GeohashIdentitySecrets? = stores.loadGeohashIdentity(npub) + /** Mirrors a save into the new store. The legacy write stays where it is. */ suspend fun mirrorGeohashIdentity( npub: String, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanup.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanup.kt index 9872ab3ac8..7a271b02ee 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanup.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanup.kt @@ -25,6 +25,7 @@ import com.vitorpamplona.amethyst.commons.model.preferences.AccountIdentityStore import com.vitorpamplona.amethyst.commons.model.preferences.AccountSecrets import com.vitorpamplona.amethyst.commons.model.preferences.DialogDismissalStore import com.vitorpamplona.amethyst.commons.model.preferences.FeedVisibilityStore +import com.vitorpamplona.amethyst.commons.model.preferences.GeohashIdentitySecrets import com.vitorpamplona.amethyst.commons.model.preferences.LatestEventCacheStore import com.vitorpamplona.amethyst.commons.model.preferences.LegacyAccountSecretNames import com.vitorpamplona.amethyst.commons.model.preferences.LegacyKeyTable @@ -33,6 +34,7 @@ import com.vitorpamplona.amethyst.commons.model.preferences.NotificationPrefsSto import com.vitorpamplona.amethyst.commons.model.preferences.RelayAuthStore import com.vitorpamplona.amethyst.commons.model.preferences.TopNavFollowListStore import com.vitorpamplona.amethyst.commons.model.preferences.UploadSettingsStore +import com.vitorpamplona.amethyst.commons.model.preferences.readLegacyGeohashIdentity import com.vitorpamplona.quartz.utils.Log /** @@ -100,6 +102,17 @@ sealed interface LegacyCleanupResult { interface LegacyAccountFiles { fun source(npub: String): LegacyPreferenceSource + /** + * The account's OTHER legacy file: the location-chat identity, which lives + * in `secret_keeper_` rather than `secret_keeper_` + * because that is the key its writer passed. + * + * Separate from [source] because [delete] removes both, and a check that + * read the npub file for these keys would never find them — they are not + * in it. That mistake was made once already. + */ + fun geohashSource(npub: String): LegacyPreferenceSource + fun exists(npub: String): Boolean /** Returns false when there was nothing to delete. */ @@ -113,6 +126,14 @@ interface MigratedSecrets { /** Null only when the account genuinely has no private key. Throws when the store is unreadable. */ suspend fun privateKey(npub: String): String? + + /** + * The location-chat identity, or null when it has not been copied across. + * + * Its own question because it migrates out of its own file: [AccountSecrets] + * being present says nothing about whether this was carried over. + */ + suspend fun geohashIdentity(npub: String): GeohashIdentitySecrets? } /** @@ -255,9 +276,42 @@ class LegacyPreferenceCleanup( } } + reasons += geohashMismatches(npub) + return reasons } + /** + * Whether deleting this account's `secret_keeper_` file would + * lose its location-chat identity. + * + * Read from [LegacyAccountFiles.geohashSource], not from the npub file the + * rest of [verify] walks: these two keys were never in that one. An account + * that never opened a location chat holds neither, and needs no copy. + */ + private suspend fun geohashMismatches(npub: String): List { + val legacy = + try { + readLegacyGeohashIdentity(files.geohashSource(npub)) + } catch (e: Exception) { + Log.w(TAG, "Could not read the location-chat identity file for $npub", e) + return listOf("the location-chat identity file could not be read") + } + + if (legacy == GeohashIdentitySecrets()) return emptyList() + + return try { + if (secrets.geohashIdentity(npub) == null) { + listOf("the location-chat identity has not been copied across") + } else { + emptyList() + } + } catch (e: Exception) { + Log.w(TAG, "Could not read the location-chat identity store for $npub", e) + listOf("the location-chat identity store could not be read") + } + } + /** * Deletes the account's legacy file if — and only if — [verify] comes back * empty and the app has stopped writing to it. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt index 430f20be37..c797fcd434 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt @@ -57,6 +57,7 @@ import com.vitorpamplona.amethyst.commons.model.preferences.UploadSettings import com.vitorpamplona.amethyst.commons.model.preferences.UploadSettingsStore import com.vitorpamplona.amethyst.commons.model.preferences.orIfUnusable import com.vitorpamplona.amethyst.commons.model.preferences.readLegacyAccountSecrets +import com.vitorpamplona.amethyst.commons.model.preferences.readLegacyGeohashIdentity import com.vitorpamplona.amethyst.commons.model.topNavFeeds.TopFilter import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy import com.vitorpamplona.amethyst.model.AccountSettings @@ -78,6 +79,7 @@ import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent import com.vitorpamplona.quartz.nip02FollowList.ContactListEvent import com.vitorpamplona.quartz.nip17Dm.settings.ChatMessageRelayListEvent +import com.vitorpamplona.quartz.nip19Bech32.bech32.bechToBytes import com.vitorpamplona.quartz.nip19Bech32.toNpub import com.vitorpamplona.quartz.nip28PublicChat.list.ChannelListEvent import com.vitorpamplona.quartz.nip37Drafts.privateOutbox.PrivateOutboxRelayListEvent @@ -325,8 +327,14 @@ object LocalPreferences { * Flipping this is a release of its own, and it ends the rollback window. * It waits on the device pass in * `amethyst/plans/2026-09-23-encrypted-storage-retirement.md`. + * + * `internal` rather than private because the mirror is not all in this + * file: [com.vitorpamplona.amethyst.model.GeohashChatIdentityState] writes + * the location-chat identity into its own legacy file and reads this to + * know when to stop. Private, it would have kept writing after the flip + * and the switch would only half work. */ - private const val LEGACY_WRITES_RETIRED = false + internal const val LEGACY_WRITES_RETIRED = false private val legacyCleanup: LegacyPreferenceCleanup by lazy { LegacyPreferenceCleanup( @@ -338,12 +346,25 @@ object LocalPreferences { override fun exists(npub: String) = legacyAccountFile(npub).exists() + override fun geohashSource(npub: String) = LegacySharedPreferences(encryptedPreferences(geohashLegacyKey(npub))) + override suspend fun delete(npub: String): Boolean { // Clear before unlinking, as deleteAccount does: the live // SharedPreferences still holds the values in memory and // would write them straight back out. encryptedPreferences(npub).edit(commit = true) { clear() } - return legacyAccountFile(npub).delete() + val removedAccountFile = legacyAccountFile(npub).delete() + + // The location-chat identity is in a SECOND file, keyed by the + // pubkey hex rather than the npub, because that is the key its + // writer passed. Nothing else would ever remove it, so it is + // deleted here with the account's own file rather than left as + // an orphan holding a seed forever. + val hex = geohashLegacyKey(npub) + encryptedPreferences(hex).edit(commit = true) { clear() } + legacyAccountFile(hex).delete() + + return removedAccountFile } }, currentStore = { npub -> accountStores.getDataStore(npub).data.first() }, @@ -352,6 +373,8 @@ object LocalPreferences { override suspend fun secrets(npub: String) = accountSecretsStore.stored(npub) override suspend fun privateKey(npub: String) = accountKeyStore.stored(npub) + + override suspend fun geohashIdentity(npub: String) = accountSecretsStore.storedGeohashIdentity(npub) }, legacyWritesRetired = LEGACY_WRITES_RETIRED, ) @@ -368,6 +391,37 @@ object LocalPreferences { return File(prefsDirPath, "$name.xml") } + /** + * The key the location-chat identity's legacy file is named by. + * + * [GeohashChatIdentityState] passed `signer.pubKey` — hex — where every + * other caller of [encryptedPreferences] passes an npub, so that material + * sits in `secret_keeper_`, a different file from the account's own + * `secret_keeper_`. Converting here keeps that quirk in one place. + */ + private fun geohashLegacyKey(npub: String): String = npub.bechToBytes("npub").toHexKey() + + /** + * Copies the location-chat identity out of `secret_keeper_` on + * the first load after the upgrade. + * + * Eager, not lazy. [GeohashChatIdentityState] also copies on first use, but + * only a user who opens a location chat ever reaches it — and the cleanup + * refuses to delete an account's legacy files while that file still holds + * an identity the current store does not. Left to the lazy path alone, a + * user who never opens another location chat would keep both files + * forever, which is the opposite of what the migration is for. + * + * Idempotent: the store's marker makes every run after the first a no-op, + * so re-running it on each load cannot overwrite a later edit. + */ + private suspend fun copyGeohashIdentity(npub: String) { + accountSecretsStore.readGeohashIdentity( + npub = npub, + legacy = readLegacyGeohashIdentity(LegacySharedPreferences(encryptedPreferences(geohashLegacyKey(npub)))), + ) + } + /** * Everything the account's DataStore holds, read in one hop. * @@ -1031,7 +1085,15 @@ object LocalPreferences { // while `mutex` serialises every account load — under the lock, each // account on a multi-account cold start would wait for the previous // one's full cleanup pass. Nothing here feeds the load. - if (loadedHere) legacyCleanup.deleteIfVerified(npub) + if (loadedHere) { + // Before the cleanup, which refuses to delete this account's files + // while the location-chat identity has not been copied. Here rather + // than inside the loader for the reason [AccountStoreData] gives: + // that method is at the JVM's 64KB limit and one more suspend call + // inside it does not fit. + copyGeohashIdentity(npub) + legacyCleanup.deleteIfVerified(npub) + } accountSettings } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/GeohashChatIdentityState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/GeohashChatIdentityState.kt index 699fbb9f37..21c011e8d6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/GeohashChatIdentityState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/GeohashChatIdentityState.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.model import androidx.core.content.edit import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.LegacySharedPreferences +import com.vitorpamplona.amethyst.LocalPreferences import com.vitorpamplona.amethyst.accountSecretsStore import com.vitorpamplona.amethyst.commons.model.preferences.GeohashIdentitySecrets import com.vitorpamplona.amethyst.commons.model.preferences.readLegacyGeohashIdentity @@ -121,7 +122,11 @@ class GeohashChatIdentityState( persist(current().copy(nickname = trimmed)) // Mirrored, not moved: the legacy file stays readable until the // legacy writes are retired app-wide, so a rollback keeps the handle. - Amethyst.instance.encryptedStorage(signer.pubKey).edit { putString(PREF_NICKNAME, trimmed) } + // Gated on the same switch as every other mirror — otherwise flipping + // it would retire the documented four and leave this one writing. + if (!LocalPreferences.LEGACY_WRITES_RETIRED) { + Amethyst.instance.encryptedStorage(signer.pubKey).edit { putString(PREF_NICKNAME, trimmed) } + } } } @@ -151,7 +156,9 @@ class GeohashChatIdentityState( val fresh = RandomInstance.bytes(GeohashKeyDerivation.SEED_SIZE) persist(current().copy(deviceSeed = fresh.toHexKey())) - Amethyst.instance.encryptedStorage(signer.pubKey).edit { putString(PREF_KEY, fresh.toHexKey()) } + if (!LocalPreferences.LEGACY_WRITES_RETIRED) { + Amethyst.instance.encryptedStorage(signer.pubKey).edit { putString(PREF_KEY, fresh.toHexKey()) } + } return fresh } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanupTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanupTest.kt index 18bad7f959..e4cc8f3ebc 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanupTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/LegacyPreferenceCleanupTest.kt @@ -26,6 +26,7 @@ import androidx.datastore.preferences.core.emptyPreferences import androidx.datastore.preferences.core.mutablePreferencesOf import androidx.datastore.preferences.core.stringPreferencesKey import com.vitorpamplona.amethyst.commons.model.preferences.AccountSecrets +import com.vitorpamplona.amethyst.commons.model.preferences.GeohashIdentitySecrets import com.vitorpamplona.amethyst.commons.model.preferences.LegacyBooleanKey import com.vitorpamplona.amethyst.commons.model.preferences.LegacyKeyTable import com.vitorpamplona.amethyst.commons.model.preferences.LegacyPreferenceSource @@ -52,18 +53,26 @@ private class MapSource( private class FakeFiles( private val values: Map, + private val geohashValues: Map = emptyMap(), ) : LegacyAccountFiles { var deleted = false private set + /** The `secret_keeper_` file goes with the account's own. */ + var deletedGeohash = false + private set + var present = true override fun source(npub: String) = MapSource(values) + override fun geohashSource(npub: String) = MapSource(geohashValues) + override fun exists(npub: String) = present override suspend fun delete(npub: String): Boolean { deleted = true + deletedGeohash = true present = false return true } @@ -73,6 +82,7 @@ private class FakeSecrets( private val stored: AccountSecrets? = AccountSecrets(), private val key: String? = null, private val throws: Boolean = false, + private val geohash: GeohashIdentitySecrets? = GeohashIdentitySecrets(), ) : MigratedSecrets { override suspend fun secrets(npub: String): AccountSecrets? { if (throws) throw IllegalStateException("keystore unavailable") @@ -83,6 +93,11 @@ private class FakeSecrets( if (throws) throw IllegalStateException("keystore unavailable") return key } + + override suspend fun geohashIdentity(npub: String): GeohashIdentitySecrets? { + if (throws) throw IllegalStateException("keystore unavailable") + return geohash + } } /** @@ -307,4 +322,77 @@ class LegacyPreferenceCleanupTest { subject.verify(NPUB), ) } + // ── the location-chat identity's own legacy file ────────────────── + + /** + * The seed is in `secret_keeper_`, and [delete] removes that + * file too. So the gate has to refuse while it holds something the current + * store does not — otherwise every geohash identity the account has would + * change on the next launch. + */ + @Test + fun anUncopiedLocationChatIdentityBlocksDeletion() = + runTest { + val (files, subject) = + cleanup( + values = emptyMap(), + files = FakeFiles(emptyMap(), mapOf("geohash_chat_device_seed" to "a".repeat(64))), + secrets = FakeSecrets(geohash = null), + ) + + val result = subject.deleteIfVerified(NPUB) + + assertTrue(result is LegacyCleanupResult.Kept) + assertTrue( + "was ${(result as LegacyCleanupResult.Kept).reasons}", + result.reasons.any { it.contains("location-chat identity") }, + ) + assertTrue(!files.deleted) + } + + /** Copied across: nothing to lose, so it must not block. */ + @Test + fun aCopiedLocationChatIdentityDoesNotBlockDeletion() = + runTest { + val (files, subject) = + cleanup( + values = emptyMap(), + files = FakeFiles(emptyMap(), mapOf("geohash_chat_device_seed" to "a".repeat(64))), + secrets = FakeSecrets(geohash = GeohashIdentitySecrets(deviceSeed = "a".repeat(64))), + ) + + assertEquals(LegacyCleanupResult.Deleted, subject.deleteIfVerified(NPUB)) + assertTrue(files.deleted) + } + + /** + * An account that never opened a location chat holds neither key. That is a + * real answer, not "not migrated", and must not hold the file hostage. + */ + @Test + fun anAccountWithNoLocationChatIdentityIsNotBlocked() = + runTest { + val (files, subject) = + cleanup( + values = emptyMap(), + files = FakeFiles(emptyMap(), emptyMap()), + secrets = FakeSecrets(geohash = null), + ) + + assertEquals(LegacyCleanupResult.Deleted, subject.deleteIfVerified(NPUB)) + assertTrue(files.deleted) + } + + /** + * Both files go, or the hex one is an orphan nothing will ever remove — + * the whole reason it is wired into this gate. + */ + @Test + fun deletingTheAccountFileAlsoRemovesTheLocationChatFile() = + runTest { + val (files, subject) = cleanup(values = emptyMap()) + + assertEquals(LegacyCleanupResult.Deleted, subject.deleteIfVerified(NPUB)) + assertTrue("the hex-keyed file must be deleted with the account's own", files.deletedGeohash) + } } diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/AccountSecretsStoreTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/AccountSecretsStoreTest.kt index fc22b515a1..5685c6c603 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/AccountSecretsStoreTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/AccountSecretsStoreTest.kt @@ -296,4 +296,25 @@ class AccountSecretsStoreTest { assertEquals(identity, subject.loadGeohashIdentity(npub)) } + + /** + * The copy has to be idempotent, because the loader now runs it on every + * account load rather than only when a location chat is opened. A second + * run must not overwrite what the user has changed since the first. + */ + @Test + fun recopyingDoesNotClobberALaterEdit() = + runTest { + val subject = stores() + subject.saveGeohashIdentity(npub, GeohashIdentitySecrets(deviceSeed = "a".repeat(64), nickname = "old")) + + // What a fresh load would find in the legacy file: the pre-migration value. + val stored = subject.loadGeohashIdentity(npub) + assertEquals("old", stored?.nickname) + + subject.saveGeohashIdentity(npub, GeohashIdentitySecrets(deviceSeed = "a".repeat(64), nickname = "new")) + + assertEquals("new", subject.loadGeohashIdentity(npub)?.nickname) + assertEquals("a".repeat(64), subject.loadGeohashIdentity(npub)?.deviceSeed) + } }