From 922a5841d02e8aa5d209b00f0c8a8e0fdfd0e7ec Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 00:17:14 +0000 Subject: [PATCH] fix(nip46): make forgetting a client complete and immediate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clearing a connected client on logout had two gaps: - The user-facing "Forget this app" button only revoked the permission ledger; it never cleared the NIP-46 client store, so a forgotten app's metadata and relays lingered and were re-recovered on the next restart. Route NIP-46 coordinates through the host's new forgetClient() so the store is cleared too. - Neither logout path stopped the RUNNING session from listening on the app's relays — only the next restart picked up the change. extraRelays is now a live projection of the client store (recomputed on connect, on start, and on disconnect via a new onDisconnected hook), so a forgotten app's relays are dropped immediately. onLogout and the UI Forget now share one authorizer.forget() path (revoke grant + clear store + clear throttle entry + signal the host), so client-initiated and user-initiated disconnects behave identically. Adds tests for both. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015FHr2mu5SiHwYNR7evYUuF --- .../model/nip46Signer/Nip46SignerState.kt | 42 ++++++++++++------- .../napplets/ConnectedAppDetailScreen.kt | 10 ++++- .../nip46/Nip46PermissionAuthorizer.kt | 20 +++++++-- .../nip46/Nip46PermissionAuthorizerTest.kt | 39 +++++++++++++++++ 4 files changed, 93 insertions(+), 18 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt index bad7429191..83041b61f0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt @@ -25,6 +25,7 @@ import com.vitorpamplona.amethyst.commons.connectedApps.nip46.Nip46ClientStore import com.vitorpamplona.amethyst.commons.connectedApps.nip46.Nip46PermissionAuthorizer import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerPermissionLedger import com.vitorpamplona.amethyst.model.AccountSettings +import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient import com.vitorpamplona.quartz.nip01Core.relay.client.single.newSubId import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl @@ -105,22 +106,16 @@ class Nip46SignerState( } }, clientStore = clientStore, + // A forgotten client's relays are gone from the store now; recompute the listen set so we + // stop listening on them this session instead of waiting for a restart. + onDisconnected = { refreshExtraRelaysFromStore() }, ) init { - // Recover the relays of `nostrconnect://`-paired apps so they stay reachable across restarts - // (bunker-flow apps use the inbox relays, which are already in the listen set). - scope.launch(Dispatchers.IO) { - val recovered = - clientStore - .all() - .filterKeys { Nip46PermissionAuthorizer.belongsTo(it, signer.pubKey) } - .values - .flatMap { it.relays } - .mapNotNull { RelayUrlNormalizer.normalizeOrNull(it) } - .toSet() - if (recovered.isNotEmpty()) extraRelays.value = extraRelays.value + recovered - } + // extraRelays is a live projection of the persisted client store (the nostrconnect apps' own + // relays). Load it on start so paired apps stay reachable across restarts; it is refreshed + // whenever a client connects or is forgotten (bunker-flow apps use the inbox relays instead). + scope.launch(Dispatchers.IO) { refreshExtraRelaysFromStore() } scope.launch(Dispatchers.IO) { combine(settings.nip46SignerEnabled, listeningRelays) { enabled, relays -> enabled to relays } @@ -172,6 +167,25 @@ class Nip46SignerState( return fresh } + /** Recomputes [extraRelays] from the persisted client store — the source of truth for nostrconnect relays. */ + private suspend fun refreshExtraRelaysFromStore() { + extraRelays.value = + clientStore + .all() + .filterKeys { Nip46PermissionAuthorizer.belongsTo(it, signer.pubKey) } + .values + .flatMap { it.relays } + .mapNotNull { RelayUrlNormalizer.normalizeOrNull(it) } + .toSet() + } + + /** + * Forgets a connected client (the user's "Forget" action): revokes its grant, drops its stored + * metadata/relays, and stops listening on relays that only it used. Same path as a client-sent + * `logout`, so both are consistent. + */ + suspend fun forgetClient(clientPubKey: HexKey) = authorizer.forget(clientPubKey) + /** Returns the current pairing secret, generating and persisting one the first time. */ private fun ensureSecret(): String { val current = settings.nip46BunkerSecret.value @@ -209,7 +223,7 @@ class Nip46SignerState( coordinate, Nip46ClientInfo(name = offer.name, url = offer.url, image = offer.image, relays = offer.relays.map { it.url }.toSet()), ) - extraRelays.value = extraRelays.value + offer.relays + refreshExtraRelaysFromStore() setEnabled(true) ConnectResult.Connected(offer.clientPubKey, offer.name) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/ConnectedAppDetailScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/ConnectedAppDetailScreen.kt index 29ff080981..6747651bd5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/ConnectedAppDetailScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/ConnectedAppDetailScreen.kt @@ -61,6 +61,7 @@ import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.browser.OmniboxInput +import com.vitorpamplona.amethyst.commons.connectedApps.nip46.Nip46PermissionAuthorizer import com.vitorpamplona.amethyst.commons.connectedApps.signers.AppSignerPolicy import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrOpDecision import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerOp @@ -221,7 +222,14 @@ fun ConnectedAppDetailScreen( Button( onClick = { mutate { - signerLedger.revokeAll(coordinate) + val nip46Client = Nip46PermissionAuthorizer.clientPubKeyOf(coordinate) + if (nip46Client != null) { + // Route NIP-46 clients through the host so the client store + the running + // listen set are cleared too, not just the permission ledger. + accountViewModel.account.nip46Signer.forgetClient(nip46Client) + } else { + signerLedger.revokeAll(coordinate) + } capabilityLedger.revokeAll(identity) } nav.popBack() diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/nip46/Nip46PermissionAuthorizer.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/nip46/Nip46PermissionAuthorizer.kt index 2a9076d99c..9d24d758dd 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/nip46/Nip46PermissionAuthorizer.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/nip46/Nip46PermissionAuthorizer.kt @@ -77,6 +77,12 @@ class Nip46PermissionAuthorizer( val onConnected: (suspend (clientPubKey: HexKey, request: BunkerRequestConnect) -> Unit)? = null, /** Persisted client metadata/relays; cleared on logout so a disconnected app leaves nothing behind. */ val clientStore: Nip46ClientStore? = null, + /** + * Invoked after a client is fully forgotten ([onLogout]/[forget]) so the host can react in the + * running session — e.g. stop listening on relays that only that client used, instead of waiting + * for the next restart. + */ + val onDisconnected: (suspend (clientPubKey: HexKey) -> Unit)? = null, ) : Nip46RequestAuthorizer { // A high-throughput client can authorize many signs per second; last-used is display-only, // so coalesce the DataStore write to at most one per client per LAST_USED_THROTTLE_SECS @@ -136,12 +142,20 @@ class Nip46PermissionAuthorizer( return allowed } - override suspend fun onLogout(clientPubKey: HexKey) { - // The client asked to disconnect — drop its standing grant (so it must pair again) and its - // persisted metadata/relays (so we stop listening on its relays after the next restart). + override suspend fun onLogout(clientPubKey: HexKey) = forget(clientPubKey) + + /** + * Fully disconnects [clientPubKey], from either the client's `logout` request or the user's + * "Forget" action: drops its standing grant (so it must pair again), its persisted metadata/relays, + * and its in-memory throttle entry, then signals [onDisconnected] so the running session can stop + * listening on relays that only this client used. + */ + suspend fun forget(clientPubKey: HexKey) { val coordinate = coordinateFor(clientPubKey) ledger.revokeAll(coordinate) clientStore?.remove(coordinate) + throttleLock.withLock { lastUsedThrottle.remove(coordinate) } + onDisconnected?.invoke(clientPubKey) } companion object { diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/nip46/Nip46PermissionAuthorizerTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/nip46/Nip46PermissionAuthorizerTest.kt index 89196d453f..34de64bde5 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/nip46/Nip46PermissionAuthorizerTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/nip46/Nip46PermissionAuthorizerTest.kt @@ -135,4 +135,43 @@ class Nip46PermissionAuthorizerTest { assertEquals(null, ledger.store.loadPolicy(coordinate)) } + + @Test + fun forgetClearsGrantAndStoreAndSignalsDisconnect() = + runTest { + val ledger = ledger() + ledger.setPolicy(coordinate, AppSignerPolicy.FULL_TRUST) + val store = InMemoryNip46ClientStore() + store.store(coordinate, Nip46ClientInfo(name = "X", relays = setOf("wss://relay.example.com"))) + var disconnected: String? = null + val authorizer = + Nip46PermissionAuthorizer( + ledger, + signerPubKey = signer, + validateSecret = { _, _ -> true }, + clientStore = store, + onDisconnected = { disconnected = it }, + ) + + authorizer.forget(client) + + assertEquals(null, ledger.store.loadPolicy(coordinate), "grant cleared") + assertEquals(null, store.load(coordinate), "stored metadata + relays cleared") + assertEquals(client, disconnected, "host notified so it can drop the relays this session") + } + + @Test + fun logoutIsEquivalentToForget() = + runTest { + val ledger = ledger() + ledger.setPolicy(coordinate, AppSignerPolicy.REASONABLE) + val store = InMemoryNip46ClientStore() + store.store(coordinate, Nip46ClientInfo(name = "X")) + val authorizer = Nip46PermissionAuthorizer(ledger, signerPubKey = signer, validateSecret = { _, _ -> true }, clientStore = store) + + authorizer.onLogout(client) + + assertEquals(null, ledger.store.loadPolicy(coordinate)) + assertEquals(null, store.load(coordinate)) + } }