diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/connectedApps/consent/SignerConsentActivity.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/connectedApps/consent/SignerConsentActivity.kt index e9528737f2..0e9f7edc40 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/connectedApps/consent/SignerConsentActivity.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/connectedApps/consent/SignerConsentActivity.kt @@ -193,6 +193,16 @@ private fun SignerConsentDialog( if (info.accountName != null) { ConnectedAccountRow(info.accountName, info.accountPicture, info.accountPubKey) } + // For a decrypt request, WHOSE conversation is being read is the decision. Show + // that person as an avatar + name, never as nothing. + if (info.counterpartyName != null) { + Text( + stringResource(R.string.nip46_signer_messages_with), + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + ConnectedAccountRow(info.counterpartyName, info.counterpartyPicture, info.counterpartyPubKey) + } } Spacer(Modifier.height(12.dp)) @@ -204,12 +214,31 @@ private fun SignerConsentDialog( HorizontalDivider() Spacer(Modifier.height(8.dp)) - // Primary: always allow this op - Button( - onClick = { onGrant(SignerOpGrant.AllowForOp(info.op)) }, - modifier = Modifier.fillMaxWidth().padding(horizontal = 24.dp), - ) { - Text(stringResource(R.string.napplet_consent_allow_always)) + // Primary: the NARROWEST "remember" available. For decrypt that is "always allow for + // Alice" — one broad decrypt grant would otherwise hand over every conversation + // forever, and scoping the op itself would mean a prompt per conversation. + val narrowOp = info.narrowOp + if (narrowOp != null && info.narrowOpLabel != null) { + Button( + onClick = { onGrant(SignerOpGrant.AllowForOp(narrowOp)) }, + modifier = Modifier.fillMaxWidth().padding(horizontal = 24.dp), + ) { + Text(info.narrowOpLabel) + } + // The broad grant stays available, but demoted below the scoped one. + OutlinedButton( + onClick = { onGrant(SignerOpGrant.AllowForOp(info.op)) }, + modifier = Modifier.fillMaxWidth().padding(horizontal = 24.dp), + ) { + Text(stringResource(R.string.napplet_consent_allow_always)) + } + } else { + Button( + onClick = { onGrant(SignerOpGrant.AllowForOp(info.op)) }, + modifier = Modifier.fillMaxWidth().padding(horizontal = 24.dp), + ) { + Text(stringResource(R.string.napplet_consent_allow_always)) + } } // Secondary: allow just once diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/connectedApps/consent/SignerConsentCoordinator.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/connectedApps/consent/SignerConsentCoordinator.kt index 92730ea5b7..ac3f5247fe 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/connectedApps/consent/SignerConsentCoordinator.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/connectedApps/consent/SignerConsentCoordinator.kt @@ -65,6 +65,23 @@ data class SignerConsentInfo( * non-event ops. */ val previewTemplate: EventTemplate? = null, + /** + * The OTHER party of a decrypt request — whose conversation the app is asking to read — shown as + * an avatar + name. "X wants to read your messages with Alice" is a categorically different + * decision from "X wants to read your private messages", so this must reach the dialog. + * Null for every op that has no counterparty (signing, and the napplet/browser paths). + */ + val counterpartyName: String? = null, + val counterpartyPicture: String? = null, + val counterpartyPubKey: String? = null, + /** + * A NARROWER op the dialog may offer to remember instead of [op] — today only + * [com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerOp.DecryptFrom], i.e. + * "always allow, but only for this counterparty". Offered ALONGSIDE the broad "Always allow" so + * the user gets granularity without a prompt per conversation. [narrowOpLabel] is its button text. + */ + val narrowOp: NostrSignerOp? = null, + val narrowOpLabel: String? = null, ) /** One pending per-operation consent request, as the batched sheet renders it. */ diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46ConsentBridge.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46ConsentBridge.kt index caf6de75ec..104166e77a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46ConsentBridge.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46ConsentBridge.kt @@ -29,14 +29,14 @@ import com.vitorpamplona.amethyst.commons.connectedApps.signers.SignerOpGrant import com.vitorpamplona.amethyst.connectedApps.consent.SignerConnectCoordinator import com.vitorpamplona.amethyst.connectedApps.consent.SignerConnectInfo import com.vitorpamplona.amethyst.connectedApps.consent.SignerConsentCoordinator -import com.vitorpamplona.amethyst.connectedApps.consent.SignerConsentInfo import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.napplet.label import com.vitorpamplona.quartz.nip01Core.core.HexKey -import com.vitorpamplona.quartz.nip01Core.jackson.JacksonMapper +import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestConnect -import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestSign +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestNip04Decrypt +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestNip44Decrypt import kotlinx.coroutines.withTimeoutOrNull /** @@ -119,39 +119,43 @@ object Nip46ConsentBridge { } ?: AppConnectResult.Cancelled } - /** Per-operation consent: describe the request (op + event preview) and await the user's grant. */ + /** + * Per-operation consent: describe the request and await the user's grant. + * + * For a decrypt request this DECRYPTS FIRST and shows the resulting plaintext, together with the + * counterparty the conversation is with. That is what makes the decision reviewable: without it + * the dialog said only "wants to read your private messages" with no way to tell one request from + * another. Decryption is local — [signer] runs on this device and nothing leaves it unless the + * user approves — and it is bounded by [Nip46ConsentInfoBuilder.DECRYPT_PREVIEW_TIMEOUT_MS] so a slow or failing signer + * degrades to an explanatory message instead of hanging or blanking the prompt. + */ suspend fun requestOp( coordinate: String, clientPubKey: HexKey, op: NostrSignerOp, request: BunkerRequest, + signer: NostrSigner, ): SignerOpGrant { val context = Amethyst.instance.appContext val info = runCatching { Amethyst.instance.nip46ClientStore.load(coordinate) }.getOrNull() val title = info?.name?.ifBlank { null } ?: context.getString(R.string.nip46_signer_remote_app) - val preview = - if (request is BunkerRequestSign) { - request.event.content - .take(160) - .trim() - } else { - "" - } - val rawData = if (request is BunkerRequestSign) JacksonMapper.toJsonPretty(request.event) else "" - val face = accountFace(coordinate) + val consentInfo = - SignerConsentInfo( - appletTitle = title, + Nip46ConsentInfoBuilder.build( coordinate = coordinate, - op = op, - operationSummary = op.label(context), - contentPreview = preview, - rawData = rawData, + title = title, iconUrl = info?.image, - accountName = face.name, - accountPicture = face.picture, - accountPubKey = face.pubKey, - previewTemplate = (request as? BunkerRequestSign)?.event, + op = op, + request = request, + account = accountFace(coordinate), + faceOf = ::userFace, + strings = + Nip46ConsentStrings( + opLabel = { it.label(context) }, + allowAlwaysFor = { context.getString(R.string.nip46_signer_allow_always_for, it) }, + decryptFailed = context.getString(R.string.nip46_signer_decrypt_failed), + ), + decrypt = { decryptWithAccountSigner(signer, it) }, ) // Fail closed if the prompt is never answered so a stuck dialog can't hold the signer hostage. return withTimeoutOrNull(CONSENT_TIMEOUT_MS) { @@ -159,16 +163,30 @@ object Nip46ConsentBridge { } ?: SignerOpGrant.DenyOnce } + /** + * Performs the local decryption behind the decrypt preview with the account's own signer. Errors + * and timeouts are handled by [Nip46ConsentInfoBuilder]; this only maps the request to a call. + */ + private suspend fun decryptWithAccountSigner( + signer: NostrSigner, + request: BunkerRequest, + ): String? = + when (request) { + is BunkerRequestNip04Decrypt -> signer.nip04Decrypt(request.ciphertext, request.pubKey) + is BunkerRequestNip44Decrypt -> signer.nip44Decrypt(request.ciphertext, request.pubKey) + else -> null + } + /** The account being signed for (avatar + name), resolved from the coordinate's signer pubkey. */ - private fun accountFace(coordinate: String): AccountFace { + private fun accountFace(coordinate: String): SignerFace { val pubKey = Nip46PermissionAuthorizer.signerPubKeyOf(coordinate) val user = pubKey?.let { LocalCache.getUserIfExists(it) } - return AccountFace(name = user?.toBestDisplayName(), picture = user?.profilePicture(), pubKey = pubKey) + return SignerFace(name = user?.toBestDisplayName(), picture = user?.profilePicture(), pubKey = pubKey) } - private data class AccountFace( - val name: String?, - val picture: String?, - val pubKey: String?, - ) + /** Cached profile for a counterparty; the builder supplies the shortened-npub fallback. */ + private fun userFace(pubKey: HexKey): SignerFace { + val user = LocalCache.getUserIfExists(pubKey) + return SignerFace(name = user?.toBestDisplayName(), picture = user?.profilePicture(), pubKey = pubKey) + } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46ConsentInfoBuilder.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46ConsentInfoBuilder.kt new file mode 100644 index 0000000000..c31b73dc4f --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46ConsentInfoBuilder.kt @@ -0,0 +1,176 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.model.nip46Signer + +import com.vitorpamplona.amethyst.commons.connectedApps.nip46.Nip46PermissionAuthorizer.Companion.decryptCounterparty +import com.vitorpamplona.amethyst.commons.connectedApps.nip46.Nip46PermissionAuthorizer.Companion.toNarrowSignerOp +import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerOp +import com.vitorpamplona.amethyst.connectedApps.consent.SignerConsentInfo +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.jackson.JacksonMapper +import com.vitorpamplona.quartz.nip19Bech32.entities.NPub +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestSign +import com.vitorpamplona.quartz.utils.Log +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.withTimeoutOrNull + +/** Avatar + display name for one pubkey, as the consent dialogs render it. */ +data class SignerFace( + val name: String?, + val picture: String?, + val pubKey: String?, +) + +/** + * The user-visible strings the builder needs, injected rather than read from `R.string` so the + * builder itself carries no Android dependency and can be unit-tested. + */ +class Nip46ConsentStrings( + /** Human-readable label for an op, e.g. "read your private messages with Alice". */ + val opLabel: (NostrSignerOp) -> String, + /** Button text for the counterparty-scoped grant; the argument is the counterparty's name. */ + val allowAlwaysFor: (String) -> String, + /** Shown as the preview when Amethyst itself could not decrypt the message. */ + val decryptFailed: String, +) + +/** + * Builds the [SignerConsentInfo] for one NIP-46 per-operation prompt. + * + * Split out of [Nip46ConsentBridge] (which owns the Android `Context`/`LocalCache` lookups) so the + * decisions that matter for safety are testable without an emulator: + * - a decrypt request is DECRYPTED FIRST and the plaintext becomes the preview, honouring the + * contract the dialog documented but never implemented; + * - a decrypt that cannot be decrypted still produces a populated dialog, never a blank one; + * - the counterparty label is never empty — it degrades to a shortened npub, never to nothing. + */ +object Nip46ConsentInfoBuilder { + /** Characters of plaintext/content shown inline before the "show more" toggle takes over. */ + const val PREVIEW_MAX_CHARS = 160 + + /** + * Upper bound on the pre-consent decryption. Short on purpose: the preview is a nicety, the + * prompt is not, so a signer that stalls (e.g. an external NIP-55 app that is not responding) + * must not delay the dialog. + */ + const val DECRYPT_PREVIEW_TIMEOUT_MS = 8_000L + + suspend fun build( + coordinate: String, + title: String, + iconUrl: String?, + op: NostrSignerOp, + request: BunkerRequest, + account: SignerFace, + /** Resolves a pubkey to a cached profile; the builder supplies its own npub fallback. */ + faceOf: (HexKey) -> SignerFace, + strings: Nip46ConsentStrings, + /** Performs the local decryption. May fail, return null, or hang — all are handled. */ + decrypt: suspend (BunkerRequest) -> String?, + ): SignerConsentInfo { + val counterparty = request.decryptCounterparty() + val plaintext = if (counterparty != null) decryptPreview(request, decrypt, strings.decryptFailed) else null + + val preview = + when { + request is BunkerRequestSign -> + request.event.content + .take(PREVIEW_MAX_CHARS) + .trim() + plaintext != null -> plaintext.take(PREVIEW_MAX_CHARS).trim() + else -> "" + } + val rawData = + when { + request is BunkerRequestSign -> JacksonMapper.toJsonPretty(request.event) + // Only worth a "show more" toggle when the preview actually truncated it. + plaintext != null && plaintext.length > PREVIEW_MAX_CHARS -> plaintext + else -> "" + } + + // A decrypt grant can be scoped to one conversation: offer "always allow for Alice" next to + // the broad "always allow", instead of only the all-conversations-forever choice. + val narrowOp = request.toNarrowSignerOp() + val counterpartyFace = counterparty?.let { face(it, faceOf) } + + return SignerConsentInfo( + appletTitle = title, + coordinate = coordinate, + op = op, + // For decrypt this names the counterparty ("read your private messages with Alice"). + operationSummary = strings.opLabel(narrowOp ?: op), + contentPreview = preview, + rawData = rawData, + iconUrl = iconUrl, + accountName = account.name, + accountPicture = account.picture, + accountPubKey = account.pubKey, + previewTemplate = (request as? BunkerRequestSign)?.event, + counterpartyName = counterpartyFace?.name, + counterpartyPicture = counterpartyFace?.picture, + counterpartyPubKey = counterparty, + narrowOp = narrowOp, + narrowOpLabel = counterpartyFace?.name?.let { strings.allowAlwaysFor(it) }, + ) + } + + /** + * Decrypts the message the app asked to read. Never throws and never hangs: a signer that fails, + * refuses, returns nothing, or takes too long yields [failureText], because a request whose + * ciphertext we cannot even read is itself worth showing — a blank dialog is not. + */ + private suspend fun decryptPreview( + request: BunkerRequest, + decrypt: suspend (BunkerRequest) -> String?, + failureText: String, + ): String = + withTimeoutOrNull(DECRYPT_PREVIEW_TIMEOUT_MS) { + try { + decrypt(request)?.ifBlank { null } + } catch (e: CancellationException) { + // Includes this block's own timeout — must propagate so withTimeoutOrNull sees it. + throw e + } catch (e: Exception) { + Log.w("NIP46Signer") { "decrypt preview failed: ${e.message}" } + null + } + } ?: failureText + + /** [faceOf], but with a guaranteed non-blank name (shortened npub when the user isn't cached). */ + private fun face( + pubKey: HexKey, + faceOf: (HexKey) -> SignerFace, + ): SignerFace { + val resolved = runCatching { faceOf(pubKey) }.getOrNull() + return SignerFace( + name = resolved?.name?.ifBlank { null } ?: shortIdentifier(pubKey), + picture = resolved?.picture, + pubKey = pubKey, + ) + } + + /** A shortened npub for an uncached pubkey; falls back to the hex prefix if it isn't valid hex. */ + fun shortIdentifier(pubKey: HexKey): String { + val npub = runCatching { NPub.create(pubKey) }.getOrNull() + return if (!npub.isNullOrBlank()) npub.take(12) + "…" else pubKey.take(12) + "…" + } +} 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 5466080f30..4fd726c943 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 @@ -170,7 +170,12 @@ class Nip46SignerState( // connect, and an allow/deny prompt whenever the ledger says ASK (dangerous kinds, // decryption, DMs, or a PARANOID app). Same surface + ledger as napplet/browser signing. connectConsent = Nip46ConsentBridge::requestConnect, - opConsent = Nip46ConsentBridge::requestOp, + // The account's own signer goes to the bridge so a decrypt request can be decrypted + // BEFORE the prompt — the dialog shows the actual plaintext instead of an opaque + // "wants to read your private messages". Local only; nothing is disclosed until approval. + opConsent = { coordinate, clientPubKey, op, request -> + Nip46ConsentBridge.requestOp(coordinate, clientPubKey, op, request, signer) + }, ) init { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NostrSignerOpLabels.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NostrSignerOpLabels.kt index b7fab80348..3472835691 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NostrSignerOpLabels.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NostrSignerOpLabels.kt @@ -30,10 +30,13 @@ import com.vitorpamplona.amethyst.commons.napplet.protocol.NappletRequest import com.vitorpamplona.amethyst.connectedApps.consent.SignerConnectInfo import com.vitorpamplona.amethyst.connectedApps.consent.SignerConsentInfo import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry +import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.ui.screen.loggedIn.relays.kindNameFor import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.jackson.JacksonMapper import com.vitorpamplona.quartz.nip01Core.signers.EventTemplate +import com.vitorpamplona.quartz.nip19Bech32.entities.NPub import com.vitorpamplona.quartz.utils.TimeUtils /** Human-readable label for a [NostrSignerOp]. */ @@ -42,8 +45,24 @@ fun NostrSignerOp.label(context: Context): String = is NostrSignerOp.SignKind -> context.getString(R.string.napplet_op_sign_kind_named, kindNameFor(context, kind), kind) NostrSignerOp.Encrypt -> context.getString(R.string.napplet_op_encrypt) NostrSignerOp.Decrypt -> context.getString(R.string.napplet_op_decrypt) + is NostrSignerOp.DecryptFrom -> context.getString(R.string.napplet_op_decrypt_from, counterpartyLabel(counterparty)) } +/** + * A person's display name for a consent prompt: their profile name when we have it cached, otherwise + * a shortened npub. Never empty — "read your private messages with " would be worse than the + * broad wording it replaces. + */ +fun counterpartyLabel(pubKeyHex: HexKey): String { + LocalCache + .getUserIfExists(pubKeyHex) + ?.toBestDisplayName() + ?.ifBlank { null } + ?.let { return it } + val npub = runCatching { NPub.create(pubKeyHex) }.getOrNull() + return if (npub != null) npub.take(12) + "…" else pubKeyHex.take(12) + "…" +} + /** Builds the [SignerConsentInfo] needed by the per-op consent dialog. */ fun buildSignerConsentInfo( context: Context, 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 d7abf620c2..ea09d12af3 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 @@ -82,6 +82,7 @@ import com.vitorpamplona.amethyst.commons.napplet.permissions.NappletPermissionL import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry import com.vitorpamplona.amethyst.favorites.rememberManifestIconModel import com.vitorpamplona.amethyst.favorites.rememberWebAppIconModel +import com.vitorpamplona.amethyst.napplet.counterpartyLabel import com.vitorpamplona.amethyst.napplet.descriptionRes import com.vitorpamplona.amethyst.napplet.labelRes import com.vitorpamplona.amethyst.napplet.resolveNappletMeta @@ -703,6 +704,7 @@ private fun NostrSignerOp.opLabel(): String = is NostrSignerOp.SignKind -> stringResource(R.string.napplet_op_sign_kind, kind) NostrSignerOp.Encrypt -> stringResource(R.string.napplet_op_encrypt) NostrSignerOp.Decrypt -> stringResource(R.string.napplet_op_decrypt) + is NostrSignerOp.DecryptFrom -> stringResource(R.string.napplet_op_decrypt_from, counterpartyLabel(counterparty)) } @Composable diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index e74fab5127..ae9eb3ad26 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -947,6 +947,14 @@ encrypt a message read your private messages + + read your private messages with %1$s + + Always allow for %1$s + + Amethyst could not decrypt this message. It may not be addressed to this account. + + Messages with Connected Apps diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46ConsentInfoBuilderTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46ConsentInfoBuilderTest.kt new file mode 100644 index 0000000000..bb5344e8c6 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46ConsentInfoBuilderTest.kt @@ -0,0 +1,177 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.model.nip46Signer + +import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerOp +import com.vitorpamplona.amethyst.connectedApps.consent.SignerConsentInfo +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.signers.EventTemplate +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestNip04Decrypt +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestNip44Decrypt +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestSign +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The decrypt-consent contract: a user asked to expose a private message must be able to see WHOSE + * conversation it is and WHAT it says. Before this, both were empty for every decrypt request, so the + * dialog skipped its whole preview block and offered "Allow always" over a blank body. + */ +class Nip46ConsentInfoBuilderTest { + private val alice = "1".repeat(64) + private val coordinate = "nip46:${"a".repeat(64)}:${"c".repeat(64)}" + private val account = SignerFace(name = "Me", picture = null, pubKey = "a".repeat(64)) + + private val decryptFailedText = "Amethyst could not decrypt this message." + + private fun strings() = + Nip46ConsentStrings( + opLabel = { op -> + when (op) { + is NostrSignerOp.DecryptFrom -> "read your private messages with ${op.counterparty.take(6)}" + NostrSignerOp.Decrypt -> "read your private messages" + else -> "do something" + } + }, + allowAlwaysFor = { "Always allow for $it" }, + decryptFailed = decryptFailedText, + ) + + private suspend fun build( + request: BunkerRequest, + op: NostrSignerOp = NostrSignerOp.Decrypt, + faceOf: (String) -> SignerFace = { SignerFace(name = null, picture = null, pubKey = it) }, + decrypt: suspend (BunkerRequest) -> String? = { "the plaintext" }, + ): SignerConsentInfo = + Nip46ConsentInfoBuilder.build( + coordinate = coordinate, + title = "Some App", + iconUrl = null, + op = op, + request = request, + account = account, + faceOf = faceOf, + strings = strings(), + decrypt = decrypt, + ) + + @Test + fun decryptConsentCarriesTheCounterpartyAndThePlaintext() = + runTest { + val info = build(BunkerRequestNip44Decrypt("1", alice, "ciphertext")) + + assertEquals(alice, info.counterpartyPubKey) + assertTrue("counterparty label must not be empty", !info.counterpartyName.isNullOrBlank()) + assertEquals("the plaintext", info.contentPreview) + // The dialog renders its preview block only when one of these is non-blank. + assertTrue("the dialog must have content to show", info.contentPreview.isNotBlank() || info.rawData.isNotBlank()) + } + + @Test + fun anUncachedCounterpartyFallsBackToAShortenedNpubNeverToNothing() = + runTest { + val info = build(BunkerRequestNip44Decrypt("1", alice, "ct"), faceOf = { SignerFace(null, null, it) }) + + val name = info.counterpartyName + assertNotNull(name) + assertTrue("expected an npub fallback, got '$name'", name!!.startsWith("npub")) + } + + @Test + fun aBlankCachedNameStillFallsBackRatherThanShowingAnEmptyLabel() = + runTest { + val info = build(BunkerRequestNip44Decrypt("1", alice, "ct"), faceOf = { SignerFace(" ", null, it) }) + + assertTrue(!info.counterpartyName.isNullOrBlank()) + } + + @Test + fun anUndecryptableMessageStillProducesAPopulatedDialog() = + runTest { + val info = build(BunkerRequestNip04Decrypt("1", alice, "garbage"), decrypt = { error("bad ciphertext") }) + + assertEquals(decryptFailedText, info.contentPreview) + assertTrue("the counterparty is still shown", !info.counterpartyName.isNullOrBlank()) + assertTrue("the dialog must not be blank", info.contentPreview.isNotBlank()) + } + + @Test + fun aSignerThatReturnsNothingIsTreatedAsAFailureNotAsAnEmptyDialog() = + runTest { + val blank = build(BunkerRequestNip44Decrypt("1", alice, "ct"), decrypt = { " " }) + assertEquals(decryptFailedText, blank.contentPreview) + + val none = build(BunkerRequestNip44Decrypt("1", alice, "ct"), decrypt = { null }) + assertEquals(decryptFailedText, none.contentPreview) + } + + /** A signer that never answers must not wedge the prompt — runTest fast-forwards the timeout. */ + @Test + fun aHangingSignerTimesOutIntoTheFailureTextInsteadOfBlockingThePrompt() = + runTest { + val info = build(BunkerRequestNip44Decrypt("1", alice, "ct"), decrypt = { awaitCancellation() }) + + assertEquals(decryptFailedText, info.contentPreview) + } + + @Test + fun aLongPlaintextIsTruncatedInlineAndOfferedInFullBehindTheToggle() = + runTest { + val long = "x".repeat(500) + val info = build(BunkerRequestNip44Decrypt("1", alice, "ct"), decrypt = { long }) + + assertEquals(Nip46ConsentInfoBuilder.PREVIEW_MAX_CHARS, info.contentPreview.length) + assertEquals(long, info.rawData) + } + + @Test + fun decryptOffersANarrowerPerCounterpartyGrantAlongsideTheBroadOne() = + runTest { + val info = build(BunkerRequestNip44Decrypt("1", alice, "ct")) + + assertEquals(NostrSignerOp.DecryptFrom(alice), info.narrowOp) + assertTrue("the narrow button needs a label", !info.narrowOpLabel.isNullOrBlank()) + // The broad op is still what the dialog's "Always allow" grants. + assertEquals(NostrSignerOp.Decrypt, info.op) + // The headline names the counterparty rather than saying "your private messages". + assertTrue(info.operationSummary.contains("with")) + } + + @Test + fun signRequestsAreUnchangedAndCarryNoCounterparty() = + runTest { + val request = BunkerRequestSign("1", EventTemplate(createdAt = 1L, kind = 1, tags = emptyArray(), content = "hello")) + val info = build(request, op = NostrSignerOp.SignKind(1)) + + assertEquals("hello", info.contentPreview) + assertTrue("sign still shows its JSON", info.rawData.contains("\"kind\"")) + assertNull(info.counterpartyName) + assertNull(info.counterpartyPubKey) + assertNull(info.narrowOp) + assertNotNull(info.previewTemplate) + } +} diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/BunkerCommand.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/BunkerCommand.kt index 130414c7c0..db81032a02 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/BunkerCommand.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/BunkerCommand.kt @@ -47,6 +47,7 @@ import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeoutOrNull +import java.util.concurrent.ConcurrentHashMap /** * `amy bunker [--relay URL[,URL…]] [--secret S] [--perms P] [--interactive] [--timeout SECS]` @@ -94,11 +95,25 @@ object BunkerCommand { ) : Nip46RequestAuthorizer { private val promptLock = Mutex() + /** + * Clients that presented the right secret in this process's lifetime. The CLI bunker keeps no + * state on disk, so pairing is in-memory: a client must `connect` once per `amy bunker` run + * before it can read the hosted identity. + */ + private val paired = ConcurrentHashMap.newKeySet() + + /** + * Ungated (the headless default hosting the operator's own key) everything is open, matching + * [authorize]. Gated, the identity reads require a `connect` first. + */ + override suspend fun isPaired(clientPubKey: HexKey): Boolean = !gated || clientPubKey in paired + override suspend fun onConnect( clientPubKey: HexKey, request: BunkerRequestConnect, ): Nip46ConnectDecision = if (request.secret == secret) { + paired.add(clientPubKey) Nip46ConnectDecision.Accept(BunkerRequestProcessor.ACK) } else { Nip46ConnectDecision.Reject("invalid secret") 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 1eb84daa24..ff7d41797e 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 @@ -136,6 +136,14 @@ class Nip46PermissionAuthorizer( if (shouldWrite) ledger.updateLastUsed(coordinate, now) } + /** + * A client is paired exactly when the ledger holds a trust level for its coordinate — which is + * what a successful `connect` writes (and what "Forget" removes). Because it is persisted, a + * client stays paired across restarts, so a returning app that does not re-`connect` still reads + * the identity it was already granted. + */ + override suspend fun isPaired(clientPubKey: HexKey): Boolean = ledger.hasPolicy(coordinateFor(clientPubKey)) + override suspend fun onConnect( clientPubKey: HexKey, request: BunkerRequestConnect, @@ -187,16 +195,23 @@ class Nip46PermissionAuthorizer( // sign/encrypt/decrypt, so this branch is a safety net). val op = request.toSignerOp() ?: return true val coordinate = coordinateFor(clientPubKey) + // A decrypt request also carries a narrower op ("decrypt messages from THIS counterparty"). + // A standing narrow grant satisfies the request without widening the broad one. + val narrowOp = request.toNarrowSignerOp() val allowed = when (ledger.decide(coordinate, op)) { NostrOpDecision.ALLOW -> true + // An explicit DENY on the broad op is final — a narrow grant never overrides it. NostrOpDecision.DENY -> false - // ASK: honor a live session grant first, otherwise prompt the user (if wired). No - // prompt → deny, so a headless signer only ever performs pre-granted operations. + // ASK: honor a live session grant first, then any narrower standing/session grant, + // otherwise prompt the user (if wired). No prompt → deny, so a headless signer only + // ever performs pre-granted operations. NostrOpDecision.ASK -> { if (isSessionAllowed(coordinate, op)) { true + } else if (narrowOp != null && isNarrowAllowed(coordinate, narrowOp)) { + true } else { askOpConsent(coordinate, clientPubKey, op, request) } @@ -215,11 +230,23 @@ class Nip46PermissionAuthorizer( val grant = opConsent?.invoke(coordinate, clientPubKey, op, request) ?: return false ledger.record(coordinate, grant) if (grant is SignerOpGrant.AllowForSession) { - throttleLock.withLock { sessionAllows.add(sessionKey(coordinate, op)) } + // Use the GRANT's op, not the requested one: the dialog may have returned a narrower op + // ("only from this counterparty"), and a session grant must be no wider than what was given. + throttleLock.withLock { sessionAllows.add(sessionKey(coordinate, grant.op)) } } return grant.isAllowed } + /** True when a standing or session grant exists for the narrower [narrowOp] (e.g. decrypt-from-X). */ + private suspend fun isNarrowAllowed( + coordinate: String, + narrowOp: NostrSignerOp, + ): Boolean = + isSessionAllowed(coordinate, narrowOp) || + // Only an explicit per-op override counts. decide() would otherwise fall through to the + // app's policy, and FULL_TRUST/REASONABLE would answer for an op nobody ever granted. + ledger.store.loadOpDecision(coordinate, narrowOp)?.let { ledger.decide(coordinate, narrowOp) == NostrOpDecision.ALLOW } ?: false + private suspend fun isSessionAllowed( coordinate: String, op: NostrSignerOp, @@ -344,5 +371,28 @@ class Nip46PermissionAuthorizer( is BunkerRequestNip44Decrypt -> NostrSignerOp.Decrypt else -> null } + + /** + * The NARROWER op a request could be granted, or `null` when it has no narrower form. + * + * Only decryption has one today: `decrypt` reveals private conversations, and one broad + * "always allow" hands over every conversation forever — so the consent dialog can also offer + * "always allow for THIS counterparty" ([NostrSignerOp.DecryptFrom]). Encryption and signing + * have no equivalent: their counterparty/kind is already the thing being granted. + */ + fun BunkerRequest.toNarrowSignerOp(): NostrSignerOp? = + when (this) { + is BunkerRequestNip04Decrypt -> NostrSignerOp.DecryptFrom(pubKey) + is BunkerRequestNip44Decrypt -> NostrSignerOp.DecryptFrom(pubKey) + else -> null + } + + /** The counterparty a decrypt request names, or `null` for any other request. */ + fun BunkerRequest.decryptCounterparty(): HexKey? = + when (this) { + is BunkerRequestNip04Decrypt -> pubKey + is BunkerRequestNip44Decrypt -> pubKey + else -> null + } } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerOp.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerOp.kt index 5987dc46ef..2187b69f07 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerOp.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerOp.kt @@ -33,9 +33,22 @@ sealed interface NostrSignerOp { /** Encrypt a message (NIP-04 or NIP-44). */ data object Encrypt : NostrSignerOp - /** Decrypt a message (NIP-04 or NIP-44). */ + /** Decrypt a message (NIP-04 or NIP-44) from ANYONE — the broad grant. */ data object Decrypt : NostrSignerOp + /** + * Decrypt messages from one [counterparty] only — the narrow alternative to [Decrypt]. + * + * A single "always allow decrypt" grant lets an app read every private conversation the user has, + * forever. Replacing [Decrypt] with this would be worse, though: a DM client would prompt once per + * conversation, training users to approve everything. So this exists *alongside* [Decrypt]: the + * consent dialog offers both, and the authorizer honours a narrow grant when the broad one is + * still ASK. Never auto-granted — only a deliberate "always allow for X" writes one. + */ + data class DecryptFrom( + val counterparty: String, + ) : NostrSignerOp + /** Stable storage key for this operation, used as a DataStore key fragment. */ val key: String get() = @@ -43,6 +56,7 @@ sealed interface NostrSignerOp { is SignKind -> "sign:$kind" Encrypt -> "encrypt" Decrypt -> "decrypt" + is DecryptFrom -> "decrypt:$counterparty" } companion object { @@ -50,6 +64,8 @@ sealed interface NostrSignerOp { when { key == "encrypt" -> Encrypt key == "decrypt" -> Decrypt + // Additive: the broad grant keeps its bare "decrypt" key, so no stored key migrates. + key.startsWith("decrypt:") -> key.removePrefix("decrypt:").ifBlank { null }?.let { DecryptFrom(it) } key.startsWith("sign:") -> key.removePrefix("sign:").toIntOrNull()?.let { SignKind(it) } else -> null } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedger.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedger.kt index 9688631c8b..1c7807a640 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedger.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedger.kt @@ -168,7 +168,9 @@ class NostrSignerPermissionLedger( is NostrSignerOp.SignKind -> if (op.kind in REASONABLE_SIGN_KINDS) NostrOpDecision.ALLOW else NostrOpDecision.ASK NostrSignerOp.Encrypt -> NostrOpDecision.ALLOW - NostrSignerOp.Decrypt -> NostrOpDecision.ASK + // Decryption always asks under REASONABLE — both the broad grant and the per-counterparty + // one, which is only ever created by an explicit "always allow for X" in the dialog. + NostrSignerOp.Decrypt, is NostrSignerOp.DecryptFrom -> NostrOpDecision.ASK } 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 96a51803c7..3ddbafa2ba 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 @@ -97,6 +97,156 @@ class Nip46PermissionAuthorizerTest { assertFalse(authorizer.authorize(client, BunkerRequestNip44Decrypt("2", client, "ct"))) } + // --------------------------------------------------------------------- + // Pairing gate (identity reads) + // --------------------------------------------------------------------- + + @Test + fun isPairedIsFalseBeforeConnectAndTrueAfter() = + runTest { + val ledger = ledger() + val authorizer = Nip46PermissionAuthorizer(ledger, signerPubKey = signer, validateSecret = { _, s -> s == "good" }) + + assertFalse(authorizer.isPaired(client), "a client that never connected must not read the identity") + + authorizer.onConnect(client, BunkerRequestConnect(id = "1", remoteKey = client, secret = "good")) + + assertTrue(authorizer.isPaired(client), "a successful connect pairs the client") + } + + @Test + fun isPairedStaysFalseAfterARejectedConnect() = + runTest { + val ledger = ledger() + val authorizer = Nip46PermissionAuthorizer(ledger, signerPubKey = signer, validateSecret = { _, s -> s == "good" }) + + authorizer.onConnect(client, BunkerRequestConnect(id = "1", remoteKey = client, secret = "wrong")) + + assertFalse(authorizer.isPaired(client)) + } + + @Test + fun forgetUnpairsTheClient() = + runTest { + val ledger = ledger() + val authorizer = Nip46PermissionAuthorizer(ledger, signerPubKey = signer, validateSecret = { _, _ -> true }) + authorizer.onConnect(client, BunkerRequestConnect(id = "1", remoteKey = client, secret = "x")) + + authorizer.forget(client) + + assertFalse(authorizer.isPaired(client)) + } + + // --------------------------------------------------------------------- + // Per-counterparty decrypt grants + // --------------------------------------------------------------------- + + private val alice = "1".repeat(64) + private val bob = "2".repeat(64) + + @Test + fun aPerCounterpartyDecryptGrantAllowsThatConversationWithoutPrompting() = + runTest { + val ledger = ledger() + ledger.setPolicy(coordinate, AppSignerPolicy.REASONABLE) + var prompts = 0 + val authorizer = + Nip46PermissionAuthorizer( + ledger, + signerPubKey = signer, + validateSecret = { _, _ -> true }, + opConsent = { _, _, _, _ -> + prompts++ + SignerOpGrant.DenyOnce + }, + ) + + ledger.setOpDecision(coordinate, NostrSignerOp.DecryptFrom(alice), NostrOpDecision.ALLOW) + + assertTrue(authorizer.authorize(client, BunkerRequestNip44Decrypt("1", alice, "ct"))) + assertEquals(0, prompts, "a standing narrow grant must not re-prompt") + } + + @Test + fun aPerCounterpartyDecryptGrantDoesNotLeakToOtherCounterparties() = + runTest { + val ledger = ledger() + ledger.setPolicy(coordinate, AppSignerPolicy.REASONABLE) + val authorizer = Nip46PermissionAuthorizer(ledger, signerPubKey = signer, validateSecret = { _, _ -> true }) + + ledger.setOpDecision(coordinate, NostrSignerOp.DecryptFrom(alice), NostrOpDecision.ALLOW) + + assertFalse(authorizer.authorize(client, BunkerRequestNip44Decrypt("1", bob, "ct")), "Bob's messages were never granted") + } + + @Test + fun aBroadDecryptDenyIsNotOverriddenByANarrowGrant() = + runTest { + val ledger = ledger() + ledger.setPolicy(coordinate, AppSignerPolicy.REASONABLE) + val authorizer = Nip46PermissionAuthorizer(ledger, signerPubKey = signer, validateSecret = { _, _ -> true }) + + ledger.setOpDecision(coordinate, NostrSignerOp.Decrypt, NostrOpDecision.DENY) + ledger.setOpDecision(coordinate, NostrSignerOp.DecryptFrom(alice), NostrOpDecision.ALLOW) + + assertFalse(authorizer.authorize(client, BunkerRequestNip44Decrypt("1", alice, "ct")), "an explicit broad DENY is final") + } + + @Test + fun aNarrowRememberFromTheDialogIsPersistedAndReusedForThatCounterpartyOnly() = + runTest { + val ledger = ledger() + ledger.setPolicy(coordinate, AppSignerPolicy.REASONABLE) + var prompts = 0 + val authorizer = + Nip46PermissionAuthorizer( + ledger, + signerPubKey = signer, + validateSecret = { _, _ -> true }, + // The dialog's "Always allow for Alice" button returns the NARROW op. + opConsent = { _, _, _, _ -> + prompts++ + SignerOpGrant.AllowForOp(NostrSignerOp.DecryptFrom(alice)) + }, + ) + + assertTrue(authorizer.authorize(client, BunkerRequestNip44Decrypt("1", alice, "ct"))) + assertEquals(1, prompts) + + // Second request from Alice reuses the stored narrow grant. + assertTrue(authorizer.authorize(client, BunkerRequestNip44Decrypt("2", alice, "ct2"))) + assertEquals(1, prompts, "the narrow grant must be remembered") + + // Bob still prompts (and this prompt would grant Alice again, so it is denied). + authorizer.authorize(client, BunkerRequestNip44Decrypt("3", bob, "ct3")) + assertEquals(2, prompts, "a different counterparty is a different decision") + } + + @Test + fun aNarrowSessionGrantIsScopedToItsOwnCounterparty() = + runTest { + val ledger = ledger() + ledger.setPolicy(coordinate, AppSignerPolicy.REASONABLE) + var prompts = 0 + val authorizer = + Nip46PermissionAuthorizer( + ledger, + signerPubKey = signer, + validateSecret = { _, _ -> true }, + opConsent = { _, _, _, _ -> + prompts++ + SignerOpGrant.AllowForSession(NostrSignerOp.DecryptFrom(alice)) + }, + ) + + assertTrue(authorizer.authorize(client, BunkerRequestNip44Decrypt("1", alice, "ct"))) + assertTrue(authorizer.authorize(client, BunkerRequestNip44Decrypt("2", alice, "ct2"))) + assertEquals(1, prompts, "the session grant covers Alice for the rest of the session") + + authorizer.authorize(client, BunkerRequestNip44Decrypt("3", bob, "ct3")) + assertEquals(2, prompts, "and must NOT cover Bob") + } + @Test fun paranoidAppRefusesEverythingUntilPerOpGrant() = runTest { diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedgerTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedgerTest.kt index c6a0ed1fc0..87b08de6e1 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedgerTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedgerTest.kt @@ -106,4 +106,32 @@ class NostrSignerPermissionLedgerTest { val ledger = NostrSignerPermissionLedger(InMemoryNostrSignerPermissionStore()) assertEquals(NostrOpDecision.ASK, ledger.decide(coordinate, NostrSignerOp.SignKind(1))) } + + // --------------------------------------------------------------------- + // Per-counterparty decrypt op + // --------------------------------------------------------------------- + + @Test + fun decryptFromRoundTripsThroughItsStorageKeyWithoutColliding() { + val alice = "1".repeat(64) + val op = NostrSignerOp.DecryptFrom(alice) + + assertEquals("decrypt:$alice", op.key) + // The broad grant keeps its historical bare key, so no stored decision migrates. + assertEquals("decrypt", NostrSignerOp.Decrypt.key) + assertEquals(op, NostrSignerOp.fromKey(op.key)) + assertEquals(NostrSignerOp.Decrypt, NostrSignerOp.fromKey("decrypt")) + assertEquals(null, NostrSignerOp.fromKey("decrypt:")) + } + + @Test + fun decryptFromAlwaysAsksUnderReasonableSoItIsOnlyEverGrantedExplicitly() = + runTest { + val store = InMemoryNostrSignerPermissionStore() + val ledger = NostrSignerPermissionLedger(store) + val coordinate = "nip46:a:b" + ledger.setPolicy(coordinate, AppSignerPolicy.REASONABLE) + + assertEquals(NostrOpDecision.ASK, ledger.decide(coordinate, NostrSignerOp.DecryptFrom("1".repeat(64)))) + } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessor.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessor.kt index 973f288b45..2328aa3d5c 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessor.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessor.kt @@ -59,12 +59,23 @@ import kotlinx.coroutines.sync.withLock * [NostrSigner] surface (`sign`, `nip04/44Encrypt/Decrypt`). Whichever signer * the user logged in with is the one that ultimately performs the work. * - * Authorization is delegated to [authorizer]: signing/encryption/decryption are - * gated, while public/harmless reads (`get_public_key`, `ping`, `get_relays`) - * always succeed. All failures — decryption, authorization, an unsupported - * method, or an exception from the signer — are turned into a - * [BunkerResponseError] carrying the request id, so the client always gets a - * reply it can correlate. + * Authorization is delegated to [authorizer] in two layers: + * - **pairing** ([Nip46RequestAuthorizer.isPaired]) gates the identity reads + * `get_public_key` and `get_relays`. Anyone who obtains the `bunker://` URI + * holds the transport pubkey and the NIP-44 conversation key, so without this + * gate they could ask an unpaired signer *which Nostr account it signs for* + * (and its inbox relay set) without ever knowing the pairing secret — which + * would defeat the transport/identity split the rest of the design maintains. + * `connect` is deliberately NOT gated (it is how pairing happens), and neither + * is `ping`: it only confirms that a signer is alive at a pubkey the caller + * already has, leaks no identity, and clients use it as a pre-flight liveness + * check, so gating it would risk breaking legitimate handshakes for no gain. + * - **per-operation consent** ([Nip46RequestAuthorizer.authorize]) gates + * signing/encryption/decryption. + * + * All failures — decryption, authorization, an unsupported method, or an + * exception from the signer — are turned into a [BunkerResponseError] carrying + * the request id, so the client always gets a reply it can correlate. * * Pairs with [NostrConnectSignerService], which subscribes to the relays, * decrypts each kind-24133 request, calls [process], and publishes the reply. @@ -100,12 +111,20 @@ class BunkerRequestProcessor( is Nip46ConnectDecision.Reject -> BunkerResponseError(request.id, decision.reason) } - is BunkerRequestGetPublicKey -> BunkerResponsePublicKey(request.id, signer.pubKey) + // Identity reads: only for a client that has already paired. See the class doc — an + // unpaired holder of the bunker URI must not be able to learn WHICH account this is. + is BunkerRequestGetPublicKey -> + ifPaired(clientPubKey, request) { + BunkerResponsePublicKey(request.id, signer.pubKey) + } + // Liveness only; answers with no identity at all, so it stays open (see class doc). is BunkerRequestPing -> BunkerResponsePong(request.id) is BunkerRequestGetRelays -> - BunkerResponseGetRelays(request.id, relays().associate { it.url to ReadWrite(read = true, write = true) }) + ifPaired(clientPubKey, request) { + BunkerResponseGetRelays(request.id, relays().associate { it.url to ReadWrite(read = true, write = true) }) + } is BunkerRequestSign -> ifAuthorized(clientPubKey, request) { @@ -148,6 +167,22 @@ class BunkerRequestProcessor( BunkerResponseError(request.id, "${e::class.simpleName}: ${e.message}") } + /** + * Runs [block] only when [clientPubKey] has already paired with this signer (a successful + * `connect`). Used for the identity reads, which need no per-op consent but must not answer a + * stranger who merely holds the bunker URI. + */ + private suspend inline fun ifPaired( + clientPubKey: HexKey, + request: BunkerRequest, + block: () -> BunkerResponse, + ): BunkerResponse = + if (authorizer.isPaired(clientPubKey)) { + block() + } else { + BunkerResponseError(request.id, ERROR_NOT_CONNECTED) + } + private suspend inline fun ifAuthorized( clientPubKey: HexKey, request: BunkerRequest, @@ -172,6 +207,13 @@ class BunkerRequestProcessor( /** Error result returned when [Nip46RequestAuthorizer.authorize] denies a request. */ const val ERROR_UNAUTHORIZED: String = "unauthorized" + /** + * Error result returned to a client that has not paired ([Nip46RequestAuthorizer.isPaired]) + * when it asks for the signer's identity (`get_public_key`, `get_relays`). It must not reveal + * whether the account exists, so it says only that this client is not connected. + */ + const val ERROR_NOT_CONNECTED: String = "not connected" + /** Error result returned when the account can no longer sign (logged out / read-only / no signer). */ const val ERROR_ACCOUNT_UNAVAILABLE: String = "account unavailable" diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/Nip46RequestAuthorizer.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/Nip46RequestAuthorizer.kt index 670208581b..1ee44400b9 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/Nip46RequestAuthorizer.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/Nip46RequestAuthorizer.kt @@ -35,10 +35,26 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestConnect * client do this?". Everything here is `suspend` so an implementation may block * on disk, a live user prompt, or IPC without changing the processor. * - * Public, harmless requests (`get_public_key`, `ping`, `get_relays`) are NOT - * routed through [authorize]; only signing, encryption and decryption are. + * Two gates, not one: + * - [isPaired] answers "has this client completed a `connect`?" and guards the + * identity reads (`get_public_key`, `get_relays`) — they need no per-op + * consent, but they must not answer a stranger who merely obtained the + * `bunker://` URI, since the answer names the user's account. `ping` is not + * guarded (pure liveness), and `connect` obviously cannot be. + * - [authorize] gates signing, encryption and decryption. */ interface Nip46RequestAuthorizer { + /** + * `true` when [clientPubKey] has already paired with this signer — i.e. a `connect` succeeded + * and the client holds a standing grant. Guards the identity reads (`get_public_key`, + * `get_relays`), which would otherwise tell any holder of the bunker URI *which Nostr account* + * this signer belongs to without ever presenting the pairing secret. + * + * Deliberately has no default: every implementation must state its own pairing rule, so a new + * authorizer cannot silently inherit "everyone is paired". + */ + suspend fun isPaired(clientPubKey: HexKey): Boolean + /** * Called when a client sends a `connect` request. The implementation * validates the offered secret (the `bunker://…?secret=…` pairing token), diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessorConcurrencyTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessorConcurrencyTest.kt index f64a4fc7a6..7ee9fdc303 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessorConcurrencyTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessorConcurrencyTest.kt @@ -114,6 +114,8 @@ class BunkerRequestProcessorConcurrencyTest { private class GatedAuthorizer( val gateFor: (BunkerRequest) -> CompletableDeferred?, ) : Nip46RequestAuthorizer { + override suspend fun isPaired(clientPubKey: HexKey) = true + override suspend fun onConnect( clientPubKey: HexKey, request: BunkerRequestConnect, diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessorTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessorTest.kt index fcfb8a711b..3533bb2959 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessorTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/BunkerRequestProcessorTest.kt @@ -45,6 +45,7 @@ import com.vitorpamplona.quartz.nip57Zaps.LnZapRequestEvent import kotlinx.coroutines.test.runTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNull import kotlin.test.assertTrue /** @@ -128,9 +129,16 @@ class BunkerRequestProcessorTest { private class FakeAuthorizer( val connectDecision: Nip46ConnectDecision, val allow: Boolean, + val paired: Boolean = true, ) : Nip46RequestAuthorizer { var connectCalls = 0 var authorizeCalls = 0 + var isPairedCalls = 0 + + override suspend fun isPaired(clientPubKey: HexKey): Boolean { + isPairedCalls++ + return paired + } override suspend fun onConnect( clientPubKey: HexKey, @@ -157,34 +165,86 @@ class BunkerRequestProcessorTest { ) = BunkerRequestProcessor(signer, { setOf(relay) }, authorizer) @Test - fun getPublicKeyReturnsUserPubKeyWithoutAuthorization() = + fun getPublicKeyReturnsUserPubKeyForAPairedClientWithoutPerOpAuthorization() = runTest { - val authorizer = FakeAuthorizer(Nip46ConnectDecision.Accept("ack"), allow = false) + // Paired but every op denied: the identity read still answers — it needs pairing, not consent. + val authorizer = FakeAuthorizer(Nip46ConnectDecision.Accept("ack"), allow = false, paired = true) val res = processor(authorizer = authorizer).process(clientPubKey, BunkerRequestGetPublicKey("1")) assertTrue(res is BunkerResponsePublicKey) assertEquals(userPubKey, res.pubkey) assertEquals("1", res.id) - // public reads are never gated + // gated on pairing only, never on per-op consent assertEquals(0, authorizer.authorizeCalls) + assertEquals(1, authorizer.isPairedCalls) } + /** + * The identity leak: anyone who obtains the `bunker://` URI can encrypt a well-formed request + * without knowing the secret. `get_public_key` must not tell them WHICH account this signer is. + */ @Test - fun pingReturnsPong() = + fun getPublicKeyIsRefusedForAnUnpairedClient() = runTest { - val res = processor().process(clientPubKey, BunkerRequestPing("2")) + val authorizer = FakeAuthorizer(Nip46ConnectDecision.Accept("ack"), allow = true, paired = false) + val res = processor(authorizer = authorizer).process(clientPubKey, BunkerRequestGetPublicKey("1")) + + assertTrue(res is BunkerResponseError, "unpaired get_public_key must error, got $res") + assertEquals(BunkerRequestProcessor.ERROR_NOT_CONNECTED, res.error) + assertEquals("1", res.id) + assertNull(res.result, "the error reply must carry no identity result") + } + + /** + * `ping` stays open on purpose: it only confirms a signer is alive at a pubkey the caller already + * has (it is in the URI they hold) and leaks no identity, so gating it would risk breaking a + * legitimate pre-connect liveness check for no privacy gain. Pinned so the choice is deliberate. + */ + @Test + fun pingReturnsPongEvenForAnUnpairedClient() = + runTest { + val authorizer = FakeAuthorizer(Nip46ConnectDecision.Accept("ack"), allow = false, paired = false) + val res = processor(authorizer = authorizer).process(clientPubKey, BunkerRequestPing("2")) assertTrue(res is BunkerResponsePong) assertEquals("2", res.id) + assertEquals(0, authorizer.isPairedCalls, "ping is deliberately not pairing-gated") } @Test - fun getRelaysReturnsConfiguredRelays() = + fun getRelaysReturnsConfiguredRelaysForAPairedClient() = runTest { val res = processor().process(clientPubKey, BunkerRequestGetRelays("3")) assertTrue(res is BunkerResponseGetRelays) assertTrue(res.relays.containsKey(relay.url)) } + /** `get_relays` additionally yields the user's inbox relay set — same pairing gate. */ + @Test + fun getRelaysIsRefusedForAnUnpairedClient() = + runTest { + val authorizer = FakeAuthorizer(Nip46ConnectDecision.Accept("ack"), allow = true, paired = false) + val res = processor(authorizer = authorizer).process(clientPubKey, BunkerRequestGetRelays("3")) + + assertTrue(res is BunkerResponseError, "unpaired get_relays must error, got $res") + assertEquals(BunkerRequestProcessor.ERROR_NOT_CONNECTED, res.error) + assertNull(res.result, "the error reply must carry no relay set") + } + + /** + * The regression that would break ALL pairing: `connect` is how a client becomes paired, so it + * must stay reachable to a client that is not paired yet. + */ + @Test + fun connectStillWorksForAnUnpairedClient() = + runTest { + val authorizer = FakeAuthorizer(Nip46ConnectDecision.Accept("s3cr3t"), allow = false, paired = false) + val res = processor(authorizer = authorizer).process(clientPubKey, BunkerRequestConnect(id = "9", remoteKey = userPubKey, secret = "s3cr3t")) + + assertEquals(1, authorizer.connectCalls) + assertTrue(res !is BunkerResponseError, "connect must not be pairing-gated, got $res") + assertEquals("s3cr3t", res.result) + } + @Test fun connectAcceptEchoesSecret() = runTest { diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerServiceTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerServiceTest.kt index 98a7dcff66..9f3ac113f1 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerServiceTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerServiceTest.kt @@ -147,6 +147,8 @@ class NostrConnectSignerServiceTest { private class AllowAuthorizer : Nip46RequestAuthorizer { var logoutCalls = 0 + override suspend fun isPaired(clientPubKey: HexKey) = true + override suspend fun onConnect( clientPubKey: HexKey, request: BunkerRequestConnect,