mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 11:48:24 +00:00
fix(napplet): serialize consent prompts so concurrent requests don't drop
When a napplet issued several consent-gated calls at once (the common case: it reads relays + storage + identity on load), each launched a NappletConsentActivity concurrently. The host can only show one, so the rest were delivered to the single-top activity and silently dropped — their broker calls hung forever (storage stuck pending; a subscription's consent lost, yielding 0 events). Gate the consent-prompt path behind a Mutex on the (per-account, reused) broker so prompts queue one at a time. After taking the lock, re-read the ledger so a sibling request for the same capability honors the just-recorded grant instead of prompting again. Only the prompt is serialized — execute() and already-granted paths stay parallel. Per-use capabilities (payments) still re-prompt every time. Adds a regression test asserting 5 concurrent same-capability requests yield exactly one prompt and never two dialogs at once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
44deeb1ee0
commit
07d25c02db
+36
-5
@@ -29,6 +29,8 @@ import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal
|
||||
import com.vitorpamplona.quartz.utils.TimeUtils
|
||||
import kotlinx.coroutines.sync.Mutex
|
||||
import kotlinx.coroutines.sync.withLock
|
||||
import kotlin.coroutines.cancellation.CancellationException
|
||||
|
||||
/**
|
||||
@@ -68,6 +70,11 @@ class NappletBroker(
|
||||
private val upload: NappletUploadGateway? = null,
|
||||
private val identityReads: NappletIdentityGateway? = null,
|
||||
) {
|
||||
// Serializes the consent-prompt path so concurrent requests queue into one dialog at a time
|
||||
// (see [authorizeWithConsent]). Only the prompt is held here; non-prompting paths and execute()
|
||||
// run unserialized.
|
||||
private val consentLock = Mutex()
|
||||
|
||||
/**
|
||||
* Authorizes and runs [request] on behalf of [identity]. [declared] is the capability set the
|
||||
* manifest's `requires` resolved to; a request outside it is refused before any prompt.
|
||||
@@ -105,11 +112,7 @@ class NappletBroker(
|
||||
signerSelfGates(request) -> true
|
||||
// A standing allow short-circuits, except for per-use capabilities (e.g. payments).
|
||||
ledger.decide(identity, capability) == PermissionDecision.ALLOW && !capability.requiresPerUseConsent -> true
|
||||
else -> {
|
||||
val grant = consentPrompt.request(identity, capability, request)
|
||||
ledger.record(identity, capability, effectiveGrant(capability, grant))
|
||||
grant.allowsExecution
|
||||
}
|
||||
else -> authorizeWithConsent(identity, capability, request)
|
||||
}
|
||||
|
||||
if (!authorized) return NappletResponse.Denied(capability, "The user declined.")
|
||||
@@ -123,6 +126,34 @@ class NappletBroker(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prompts for consent under [consentLock] so several requests arriving at once — the common case,
|
||||
* a napplet reading relays + storage + identity on load — queue into one dialog at a time instead
|
||||
* of racing. Without this each would launch a consent prompt concurrently; the host can only show
|
||||
* one, so the rest are dropped and their calls hang forever.
|
||||
*
|
||||
* After taking the lock we re-read the standing decision: a sibling request for the same capability
|
||||
* may have recorded an Allow/Deny while we waited, so we honor that instead of prompting again.
|
||||
* Per-use capabilities (payments) always re-prompt, so they skip the re-check.
|
||||
*/
|
||||
private suspend fun authorizeWithConsent(
|
||||
identity: NappletIdentity,
|
||||
capability: NappletCapability,
|
||||
request: NappletRequest,
|
||||
): Boolean =
|
||||
consentLock.withLock {
|
||||
if (!capability.requiresPerUseConsent) {
|
||||
when (ledger.decide(identity, capability)) {
|
||||
PermissionDecision.DENY -> return@withLock false
|
||||
PermissionDecision.ALLOW -> return@withLock true
|
||||
PermissionDecision.ASK -> {}
|
||||
}
|
||||
}
|
||||
val grant = consentPrompt.request(identity, capability, request)
|
||||
ledger.record(identity, capability, effectiveGrant(capability, grant))
|
||||
grant.allowsExecution
|
||||
}
|
||||
|
||||
/** Identity reads and sign-as-user ops are gated by us only when we hold the key; remote/external signers gate themselves. */
|
||||
private fun signerSelfGates(request: NappletRequest): Boolean = (request.capability == NappletCapability.IDENTITY || request.signsAsUser) && signer !is NostrSignerInternal
|
||||
|
||||
|
||||
+55
@@ -36,6 +36,10 @@ import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal
|
||||
import com.vitorpamplona.quartz.nip57Zaps.LnZapPrivateEvent
|
||||
import com.vitorpamplona.quartz.nip57Zaps.LnZapRequestEvent
|
||||
import kotlinx.coroutines.CompletableDeferred
|
||||
import kotlinx.coroutines.async
|
||||
import kotlinx.coroutines.awaitAll
|
||||
import kotlinx.coroutines.test.advanceUntilIdle
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
@@ -226,6 +230,57 @@ class NappletBrokerTest {
|
||||
assertEquals(1, prompt.calls) // only the first request prompted
|
||||
}
|
||||
|
||||
/** A prompt that blocks inside [request] until [gate] is released, tracking peak concurrency. */
|
||||
private class GatedPrompt(
|
||||
private val answer: GrantState,
|
||||
) : NappletConsentPrompt {
|
||||
val gate = CompletableDeferred<Unit>()
|
||||
var calls = 0
|
||||
private set
|
||||
var maxActive = 0
|
||||
private set
|
||||
private var active = 0
|
||||
|
||||
override suspend fun request(
|
||||
identity: NappletIdentity,
|
||||
capability: NappletCapability,
|
||||
request: NappletRequest,
|
||||
): GrantState {
|
||||
calls++
|
||||
active++
|
||||
if (active > maxActive) maxActive = active
|
||||
try {
|
||||
gate.await()
|
||||
return answer
|
||||
} finally {
|
||||
active--
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class)
|
||||
@Test
|
||||
fun concurrentRequestsForOneCapabilitySerializeIntoASinglePrompt() =
|
||||
runTest {
|
||||
// Five capability calls land at once (a napplet hitting relays/storage/identity on load).
|
||||
// Without serialization each would launch its own consent prompt concurrently; the host
|
||||
// can only show one, so the rest are dropped and hang. The broker must queue them through
|
||||
// a single prompt, and siblings must honor the grant the first one records.
|
||||
val prompt = GatedPrompt(GrantState.ALLOW_ALWAYS)
|
||||
val broker = broker(prompt)
|
||||
|
||||
val jobs = List(5) { async { broker.handle(applet, NappletRequest.GetPublicKey, allDeclared) } }
|
||||
advanceUntilIdle() // everyone reaches the gate/lock; only the lock holder is inside request()
|
||||
|
||||
assertEquals(1, prompt.maxActive) // never two prompts at once (the bug would show 5)
|
||||
|
||||
prompt.gate.complete(Unit)
|
||||
val responses = jobs.awaitAll()
|
||||
|
||||
assertEquals(1, prompt.calls) // siblings honored the recorded ALLOW_ALWAYS instead of re-prompting
|
||||
responses.forEach { assertEquals(NappletResponse.PublicKey(signer.pubKey), it) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun publishSignsTheTemplateAsTheUserAndSends() =
|
||||
runTest {
|
||||
|
||||
Reference in New Issue
Block a user