mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-09 16:14:40 +00:00
fix(nip46): make forgetting a client complete and immediate
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FHr2mu5SiHwYNR7evYUuF
This commit is contained in:
+28
-14
@@ -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)
|
||||
|
||||
+9
-1
@@ -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()
|
||||
|
||||
+17
-3
@@ -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 {
|
||||
|
||||
+39
@@ -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))
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user