mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-10 16:33:27 +00:00
Merge pull request #3594 from nrobi144/fix/desktop-auth-tier1-coldboot-race
fix(desktop): auto-approve pending relay AUTH once the DM-inbox (kind:10050) set loads
This commit is contained in:
+107
@@ -0,0 +1,107 @@
|
||||
/*
|
||||
* 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.commons.relayClient.auth
|
||||
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
|
||||
import kotlinx.collections.immutable.PersistentMap
|
||||
import kotlinx.collections.immutable.persistentMapOf
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.update
|
||||
|
||||
/**
|
||||
* Holds the set of tier-2 NIP-42 AUTH challenges awaiting a decision and the
|
||||
* logic that settles them. Platform-agnostic (lives in `commons`) so the state
|
||||
* and, crucially, the cold-boot race fix are unit-testable without any desktop
|
||||
* wiring (relay client, LocalCache, Compose).
|
||||
*
|
||||
* A [PendingAuthApproval] is produced by [AuthApprovalPolicy.classify] when a
|
||||
* relay is neither pre-approved nor blocked; the suspended signer awaits its
|
||||
* [PendingAuthApproval.decision]. This class owns the live set the banner
|
||||
* renders and the three ways an entry leaves it:
|
||||
*
|
||||
* - [resolve] — the user picked `[Once] [Always] [Never]`.
|
||||
* - [autoApproveNowTrusted] — the relay became tier-1 after the challenge was
|
||||
* surfaced (the cold-boot race, see that method).
|
||||
* - [cancelAll] — logout / account switch tears everything down.
|
||||
*/
|
||||
class AuthApprovalRequests {
|
||||
private val _pending = MutableStateFlow<PersistentMap<NormalizedRelayUrl, PendingAuthApproval>>(persistentMapOf())
|
||||
|
||||
/** The live set of pending tier-2 challenges the AUTH banner renders. */
|
||||
val pending: StateFlow<PersistentMap<NormalizedRelayUrl, PendingAuthApproval>> = _pending.asStateFlow()
|
||||
|
||||
/** Track a newly surfaced tier-2 challenge. Last write per relay wins. */
|
||||
fun add(approval: PendingAuthApproval) {
|
||||
_pending.update { it.put(approval.relayUrl, approval) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Settle a pending challenge with the user's pick. Removes the entry
|
||||
* BEFORE completing the deferred so the suspended signer wakes exactly
|
||||
* once. Returns false if there was nothing pending for [relayUrl].
|
||||
*/
|
||||
fun resolve(
|
||||
relayUrl: NormalizedRelayUrl,
|
||||
scope: AuthApprovalScope,
|
||||
): Boolean {
|
||||
val approval = _pending.value[relayUrl] ?: return false
|
||||
_pending.update { it.remove(relayUrl) }
|
||||
return approval.decision.complete(scope)
|
||||
}
|
||||
|
||||
/**
|
||||
* Auto-approve any pending challenge whose relay is now tier-1 (present in
|
||||
* [trusted]).
|
||||
*
|
||||
* Fixes the cold-boot race: on startup an AUTH-required DM-inbox relay
|
||||
* (kind:10050) often sends its challenge before the account's own kind:10050
|
||||
* list has been fetched, so [AuthApprovalPolicy.classify] — which reads the
|
||||
* trusted set exactly once — sees an empty set and surfaces a tier-2 prompt
|
||||
* for a relay that should have auto-signed. Nothing re-evaluates that
|
||||
* pending decision when the list finally loads, leaving a spurious banner.
|
||||
*
|
||||
* When the DM-inbox set updates, call this: every pending relay now in
|
||||
* [trusted] is settled with [AuthApprovalScope.ONCE] — sign this session but
|
||||
* do NOT persist, since it is trusted by identity (it is the user's own
|
||||
* inbox), not by an explicit user grant.
|
||||
*/
|
||||
fun autoApproveNowTrusted(trusted: Set<NormalizedRelayUrl>) {
|
||||
// Snapshot the relays to settle so we don't mutate while iterating the
|
||||
// live map. ONCE = sign this session, never persist (trusted by
|
||||
// identity, not by an explicit user grant).
|
||||
_pending.value.keys
|
||||
.filter { it in trusted }
|
||||
.forEach { relayUrl -> resolve(relayUrl, AuthApprovalScope.ONCE) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Cancel every pending challenge, completing each with
|
||||
* [AuthApprovalScope.BLOCKED] so suspended signers unblock (and drop the
|
||||
* AUTH) rather than hang. Used on logout / account switch.
|
||||
*/
|
||||
fun cancelAll() {
|
||||
val snapshot = _pending.value
|
||||
_pending.value = persistentMapOf()
|
||||
snapshot.values.forEach { it.decision.complete(AuthApprovalScope.BLOCKED) }
|
||||
}
|
||||
}
|
||||
+121
@@ -0,0 +1,121 @@
|
||||
/*
|
||||
* 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.commons.relayClient.auth
|
||||
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
|
||||
import kotlinx.coroutines.CompletableDeferred
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
class AuthApprovalRequestsTest {
|
||||
private val ownInbox = NormalizedRelayUrl("wss://my.inbox.relay/")
|
||||
private val someOtherRelay = NormalizedRelayUrl("wss://random.relay/")
|
||||
|
||||
private fun pendingFor(relay: NormalizedRelayUrl): CompletableDeferred<AuthApprovalScope> {
|
||||
val deferred = CompletableDeferred<AuthApprovalScope>()
|
||||
return deferred
|
||||
}
|
||||
|
||||
/**
|
||||
* Reproduces the cold-boot tier-1 race.
|
||||
*
|
||||
* A relay challenges for AUTH before the account's kind:10050 has loaded,
|
||||
* so it is surfaced as a tier-2 pending prompt. When kind:10050 finally
|
||||
* loads and the relay turns out to be the user's own DM inbox (tier-1), the
|
||||
* spurious prompt must clear and the suspended signer must be released to
|
||||
* sign — without persisting anything (ONCE), because it is trusted by
|
||||
* identity, not by an explicit user grant.
|
||||
*
|
||||
* RED until [AuthApprovalRequests.autoApproveNowTrusted] is implemented.
|
||||
*/
|
||||
@Test
|
||||
fun pendingChallengeForNowTrustedRelayIsAutoApproved() =
|
||||
runTest {
|
||||
val requests = AuthApprovalRequests()
|
||||
val deferred = pendingFor(ownInbox)
|
||||
requests.add(PendingAuthApproval(ownInbox, deferred))
|
||||
assertTrue(requests.pending.value.containsKey(ownInbox))
|
||||
|
||||
// kind:10050 arrives: the challenging relay is now tier-1.
|
||||
requests.autoApproveNowTrusted(setOf(ownInbox))
|
||||
|
||||
assertFalse(
|
||||
requests.pending.value.containsKey(ownInbox),
|
||||
"spurious banner must clear once the relay is known to be trusted",
|
||||
)
|
||||
assertTrue(deferred.isCompleted, "the suspended signer must be released")
|
||||
assertEquals(
|
||||
AuthApprovalScope.ONCE,
|
||||
deferred.await(),
|
||||
"tier-1 auto-approval must sign but NOT persist (ONCE)",
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun pendingChallengeForUntrustedRelayIsLeftForTheUser() =
|
||||
runTest {
|
||||
val requests = AuthApprovalRequests()
|
||||
val deferred = pendingFor(someOtherRelay)
|
||||
requests.add(PendingAuthApproval(someOtherRelay, deferred))
|
||||
|
||||
// A different relay became trusted; this one is still unknown.
|
||||
requests.autoApproveNowTrusted(setOf(ownInbox))
|
||||
|
||||
assertTrue(
|
||||
requests.pending.value.containsKey(someOtherRelay),
|
||||
"an untrusted relay's prompt must remain for the user to decide",
|
||||
)
|
||||
assertFalse(deferred.isCompleted, "the untrusted signer must stay suspended")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun resolveRemovesEntryAndCompletesDeferred() =
|
||||
runTest {
|
||||
val requests = AuthApprovalRequests()
|
||||
val deferred = pendingFor(someOtherRelay)
|
||||
requests.add(PendingAuthApproval(someOtherRelay, deferred))
|
||||
|
||||
assertTrue(requests.resolve(someOtherRelay, AuthApprovalScope.BLOCKED))
|
||||
|
||||
assertFalse(requests.pending.value.containsKey(someOtherRelay))
|
||||
assertEquals(AuthApprovalScope.BLOCKED, deferred.await())
|
||||
assertFalse(requests.resolve(someOtherRelay, AuthApprovalScope.ONCE), "second resolve is a no-op")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun cancelAllReleasesEverySignerWithBlocked() =
|
||||
runTest {
|
||||
val requests = AuthApprovalRequests()
|
||||
val a = pendingFor(ownInbox)
|
||||
val b = pendingFor(someOtherRelay)
|
||||
requests.add(PendingAuthApproval(ownInbox, a))
|
||||
requests.add(PendingAuthApproval(someOtherRelay, b))
|
||||
|
||||
requests.cancelAll()
|
||||
|
||||
assertTrue(requests.pending.value.isEmpty())
|
||||
assertEquals(AuthApprovalScope.BLOCKED, a.await())
|
||||
assertEquals(AuthApprovalScope.BLOCKED, b.await())
|
||||
}
|
||||
}
|
||||
@@ -1367,6 +1367,15 @@ private fun AppInner(
|
||||
remember(account, relayManager, scope) {
|
||||
DesktopAccountRelays(account.pubKeyHex, relayManager, scope)
|
||||
}
|
||||
// Cold-boot AUTH race fix: when the account's own kind:10050 DM-inbox
|
||||
// set loads (often AFTER an inbox relay has already challenged for
|
||||
// AUTH), retroactively auto-approve any pending tier-2 prompt for a
|
||||
// relay that is actually tier-1, instead of leaving a spurious banner.
|
||||
LaunchedEffect(authCoordinator, accountRelays) {
|
||||
accountRelays.dmRelayList.collect { dmInbox ->
|
||||
authCoordinator.onSelfApprovedRelaysChanged(dmInbox)
|
||||
}
|
||||
}
|
||||
val iAccount =
|
||||
remember(account, localCache, relayManager, dmSendTracker, accountRelays, dmInboxResolver) {
|
||||
DesktopIAccount(account, localCache, relayManager, dmSendTracker, scope, accountRelays, dmInboxResolver)
|
||||
|
||||
+22
-18
@@ -22,6 +22,7 @@ package com.vitorpamplona.amethyst.desktop.auth
|
||||
|
||||
import com.vitorpamplona.amethyst.commons.relayClient.auth.AuthApprovalDecision
|
||||
import com.vitorpamplona.amethyst.commons.relayClient.auth.AuthApprovalPolicy
|
||||
import com.vitorpamplona.amethyst.commons.relayClient.auth.AuthApprovalRequests
|
||||
import com.vitorpamplona.amethyst.commons.relayClient.auth.AuthApprovalScope
|
||||
import com.vitorpamplona.amethyst.commons.relayClient.auth.AuthApprovalStore
|
||||
import com.vitorpamplona.amethyst.commons.relayClient.auth.PendingAuthApproval
|
||||
@@ -34,12 +35,8 @@ import com.vitorpamplona.quartz.nip01Core.signers.EventTemplate
|
||||
import com.vitorpamplona.quartz.nip42RelayAuth.RelayAuthEvent
|
||||
import com.vitorpamplona.quartz.utils.Log
|
||||
import kotlinx.collections.immutable.PersistentMap
|
||||
import kotlinx.collections.immutable.persistentMapOf
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.update
|
||||
|
||||
/**
|
||||
* Desktop NIP-42 AUTH wiring.
|
||||
@@ -79,13 +76,13 @@ class DesktopAuthCoordinator(
|
||||
@Volatile
|
||||
private var active: ActiveAuth? = null
|
||||
|
||||
private val _pendingApprovals = MutableStateFlow<PersistentMap<NormalizedRelayUrl, PendingAuthApproval>>(persistentMapOf())
|
||||
private val requests = AuthApprovalRequests()
|
||||
|
||||
/**
|
||||
* Tier-2 AUTH challenges awaiting the user's `[Once] [Always] [Never]`
|
||||
* decision. The banner UI subscribes and calls [resolve] to settle each.
|
||||
*/
|
||||
val pendingApprovals: StateFlow<PersistentMap<NormalizedRelayUrl, PendingAuthApproval>> = _pendingApprovals.asStateFlow()
|
||||
val pendingApprovals: StateFlow<PersistentMap<NormalizedRelayUrl, PendingAuthApproval>> = requests.pending
|
||||
|
||||
/** Wire AUTH for a newly logged-in account. Idempotent. */
|
||||
fun onLogin(account: AccountState.LoggedIn) {
|
||||
@@ -97,9 +94,7 @@ class DesktopAuthCoordinator(
|
||||
AuthApprovalPolicy(
|
||||
selfApprovedRelays = { selfApprovedRelaysFor(account.pubKeyHex) },
|
||||
store = store,
|
||||
onPromptRequired = { pending ->
|
||||
_pendingApprovals.update { it.put(pending.relayUrl, pending) }
|
||||
},
|
||||
onPromptRequired = { pending -> requests.add(pending) },
|
||||
)
|
||||
val authenticator =
|
||||
RelayAuthenticator(
|
||||
@@ -121,26 +116,35 @@ class DesktopAuthCoordinator(
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a tier-2 [PendingAuthApproval] from the banner UI.
|
||||
*
|
||||
* Removes the entry from [pendingApprovals] before completing the
|
||||
* deferred, so the suspended signer wakes up exactly once.
|
||||
* Resolve a tier-2 [PendingAuthApproval] from the banner UI. The suspended
|
||||
* signer wakes up exactly once with the user's pick.
|
||||
*/
|
||||
fun resolve(
|
||||
relayUrl: NormalizedRelayUrl,
|
||||
scope: AuthApprovalScope,
|
||||
) {
|
||||
val pending = _pendingApprovals.value[relayUrl] ?: return
|
||||
_pendingApprovals.update { it.remove(relayUrl) }
|
||||
pending.decision.complete(scope)
|
||||
requests.resolve(relayUrl, scope)
|
||||
}
|
||||
|
||||
/**
|
||||
* Called when the active account's NIP-17 DM-inbox (kind:10050) set loads
|
||||
* or changes. Auto-approves any pending tier-2 prompt whose relay is now in
|
||||
* that set, fixing the cold-boot race where an own inbox relay challenges
|
||||
* for AUTH before kind:10050 has been fetched and gets a spurious prompt
|
||||
* that nothing would otherwise re-evaluate.
|
||||
*
|
||||
* [trusted] must be the strict kind:10050 inbox set (same tier-1 source as
|
||||
* [selfApprovedRelaysFor]) — never the lenient NIP-65 fallback.
|
||||
*/
|
||||
fun onSelfApprovedRelaysChanged(trusted: Set<NormalizedRelayUrl>) {
|
||||
requests.autoApproveNowTrusted(trusted)
|
||||
}
|
||||
|
||||
private fun tearDownLocked() {
|
||||
val prev = active ?: return
|
||||
prev.authenticator.destroy()
|
||||
// Cancel any in-flight tier-2 prompts so suspended signers wake up.
|
||||
_pendingApprovals.value.values.forEach { it.decision.complete(AuthApprovalScope.BLOCKED) }
|
||||
_pendingApprovals.value = persistentMapOf()
|
||||
requests.cancelAll()
|
||||
active = null
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user