diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt index 09864df91f..9b6a2abe3f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt @@ -355,7 +355,27 @@ object LocalPreferences { } } - suspend fun setDefaultAccount(accountSettings: AccountSettings) { + /** + * Make [accountSettings] the current account, persisting + caching it. Returns the settings that + * actually became current — normally [accountSettings] itself, but see the downgrade guard below. + */ + suspend fun setDefaultAccount(accountSettings: AccountSettings): AccountSettings { + val npub = accountSettings.keyPair.pubKey.toNpub() + + // Downgrade guard: adding a read-only npub for a pubkey we already hold a SIGNING account for + // must not clobber that account. Accounts dedup by npub, so saving fresh read-only settings + // here would overwrite the signing account's per-npub file — wiping its cached follow/relay/ + // mute lists and flipping hasPrivKey off, which silently disables its push notifications. A + // signing account already does everything the read-only one would, so keep it and just make + // it current instead of degrading it. + if (!accountSettings.isWriteable()) { + val existing = loadAccountConfigFromEncryptedStorage(npub) + if (existing != null && existing.isWriteable()) { + setCurrentAccount(existing) + return existing + } + } + // Save the per-npub file before emitting onto the savedAccounts flow. // Otherwise a collector (e.g. AlwaysOnNotificationServiceManager) can race in // and call loadAccountConfigFromEncryptedStorage(npub) before NOSTR_PUBKEY is @@ -363,9 +383,9 @@ object LocalPreferences { // rest of the session — making every later switch to this account land on // LoggedOff instead of LoggedIn. saveToEncryptedStorage(accountSettings) - val npub = accountSettings.keyPair.pubKey.toNpub() mutex.withLock { cachedAccounts.put(npub, accountSettings) } setCurrentAccount(accountSettings) + return accountSettings } suspend fun allSavedAccounts(): List = savedAccounts() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/AccountSessionManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/AccountSessionManager.kt index 0c85dc7081..bf63ace422 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/AccountSessionManager.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/AccountSessionManager.kt @@ -175,9 +175,11 @@ class AccountSessionManager( } } - localPreferences.setDefaultAccount(accountSettings) + // setDefaultAccount may keep an existing signing account instead of this one when a + // read-only npub is added for a pubkey we already sign for — show whichever became current. + val current = localPreferences.setDefaultAccount(accountSettings) - startUI(accountSettings) + startUI(current) } fun startUI( diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManager.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManager.kt index 114a4cc885..b5c4e5598c 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManager.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManager.kt @@ -508,6 +508,18 @@ class AccountManager internal constructor( suspend fun saveCurrentAccount(): Result { val current = currentAccount() ?: return Result.failure(Exception("No account logged in")) + // Downgrade guard (mirrors Android LocalPreferences.setDefaultAccount): never persist a + // read-only account over an existing SIGNING account for the same pubkey. Accounts key by + // npub, so this would overwrite signerType to ViewOnly and orphan the stored key, routing + // every later switch through read-only. A signing account already subsumes a read-only one, + // so keep it and switch to it instead of degrading it. + if (current.isReadOnly) { + val existing = accountStorage.loadAccounts().firstOrNull { it.npub == current.npub } + if (existing != null && existing.signerType !is SignerType.ViewOnly) { + return switchAccount(current.npub).map { } + } + } + // Bunker accounts: private key saved during loginWithBunker if (current.signerType is SignerType.Remote) { // Still ensure multi-account storage is updated diff --git a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt index 05129a4c26..4d511c57f7 100644 --- a/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt +++ b/desktopApp/src/jvmTest/kotlin/com/vitorpamplona/amethyst/desktop/account/AccountManagerKeyLoginTest.kt @@ -159,4 +159,45 @@ class AccountManagerKeyLoginTest { val accounts = manager.accountStorage.loadAccounts() assertTrue(accounts.any { it.npub == keyPair.pubKey.toNpub() }) } + + /** + * Regression: adding the read-only npub for a pubkey we already hold a SIGNING account for must + * NOT downgrade it. Accounts key by npub, so persisting the ViewOnly entry would overwrite the + * Internal signerType and orphan the stored key, routing every later switch through read-only. + * The signing account must survive and become current instead. + */ + @Test + fun addingReadOnlyNpubDoesNotDowngradeExistingSigningAccount() = + runTest { + val keyPair = KeyPair() + val npub = keyPair.pubKey.toNpub() + + // 1. Sign in with the nsec and persist it as a full signing account. + manager.loginWithKey(keyPair.privKey!!.toNsec()) + manager.saveCurrentAccount() + assertEquals( + SignerType.Internal, + manager.accountStorage + .loadAccounts() + .first { it.npub == npub } + .signerType, + ) + assertTrue(keyStore.containsKey(npub)) + + // 2. Add the read-only npub for the SAME pubkey and try to save it. + manager.loginWithKey(npub) + assertTrue((manager.accountState.value as AccountState.LoggedIn).isReadOnly) + val result = manager.saveCurrentAccount() + assertTrue(result.isSuccess) + + // 3. The signing account survives: storage stays Internal, the key is kept, + // and the current account was switched back to signing (not downgraded). + val stored = manager.accountStorage.loadAccounts().first { it.npub == npub } + assertEquals(SignerType.Internal, stored.signerType) + assertTrue(keyStore.containsKey(npub)) + val finalState = manager.accountState.value + assertIs(finalState) + assertFalse(finalState.isReadOnly) + assertEquals(SignerType.Internal, finalState.signerType) + } }