From 06b49acf06674e3e7f25090e405720212570ba45 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 15:16:12 +0000 Subject: [PATCH 1/6] fix: make the "reading someone I follow" relay-auth toggle reachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under the CUSTOM ("decide per relay") policy, RelayAuthResolver AND-gated every toggle behind isFirstParty: if (inputs.isFirstParty && customAllows(inputs)) ALLOW else fallThrough isFirstParty (RelayAuthFirstParty.hasReason) is true only when we publish to the relay, the relay is on our own list, or it hosts a room we joined. A follow's outbox relay is none of those — it is theirs — so the readFollows branch of customAllows could never be reached. With "…I'm reading someone I follow" explicitly on, every follow's outbox relay still fell through to ASK, producing one login prompt per follow. The only challenges the category ever granted were ones myRelaysAndVenues already covered. customAllows now checks readFollows ahead of the gate. Exempting just that category keeps what the gate is for: the follow graph it consults is this account's, so another account's traffic cannot conjure a match, and the other three categories still require first-party — which is what stops a bystander account being auto-authenticated (and billed) on a paid inbox relay because another logged-in account's outgoing DM happened to name someone we follow. Those three lose nothing by keeping it: our own relay list and our joined rooms' hosts are first-party by definition, and a pending event of ours makes its destination first-party too. RelayAuthResolverTest pinned the old behaviour as intended (nonFirstPartyAsksInsteadOfAutoAllowing), which is why this went unnoticed; that assertion is replaced by readFollowsGrantsOnTheFollowsOwnOutboxRelay plus readFollowsExemptionDoesNotLeakIntoTheOtherCategories, and a new RelayAuthReadFollowsTest covers the same case end-to-end through the ledger. Verified: 80 relay-auth tests green across :commons:jvmTest and :amethyst:testFdroidDebugUnitTest; spotlessApply clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014UmCeSWetuKmHdrWcZkWDR --- .../2026-08-03-auth-permissions-redesign.md | 6 + .../authCommand/model/AuthCoordinator.kt | 18 ++- .../model/RelayAuthReadFollowsTest.kt | 128 ++++++++++++++++++ .../commons/relayauth/RelayAuthResolver.kt | 37 ++++- .../relayauth/RelayAuthResolverTest.kt | 43 +++++- 5 files changed, 222 insertions(+), 10 deletions(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthReadFollowsTest.kt diff --git a/amethyst/plans/2026-08-03-auth-permissions-redesign.md b/amethyst/plans/2026-08-03-auth-permissions-redesign.md index ae69a30b65..295de64b98 100644 --- a/amethyst/plans/2026-08-03-auth-permissions-redesign.md +++ b/amethyst/plans/2026-08-03-auth-permissions-redesign.md @@ -51,6 +51,12 @@ Shipped as designed. Where it diverged or went further: connection forever. A second challenge for the same (relay, account) rides along on the owner's answer with no deadline of its own — running one would let it resolve the shared deferred and tear down a dialog mid-read. +- **Corrected later:** this plan left the decision model alone, including the + blanket `isFirstParty` gate on `CUSTOM`. That gate turned out to make + `readFollows` ("…I'm reading someone I follow") unreachable — a follow's outbox + relay is theirs, so it is never first-party for us, and every follow produced a + prompt with the toggle explicitly on. `RelayAuthResolver.customAllows` now + checks that one category ahead of the gate; the other three still require it. - **Still not done:** what a timeout should *look like*. It is now an honest 60s of visible time rather than a clock the user never saw, but it is still a dialog that vanishes and an event left pending in the outbox with no feedback. That diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt index 7ff6533dc3..91c3e0a791 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt @@ -103,6 +103,10 @@ class AuthCoordinator( // question. Returning early here instead made "decide per relay" mean "deny, and // don't mention it" for every purpose that names someone else — the exact case the // prompt was built to explain. + // + // It does not reach the "…I'm reading someone I follow" toggle at all: a follow's + // outbox relay can never be first-party for us, so applying it there emptied the + // category instead of narrowing it. RelayAuthResolver.customAllows has the detail. val firstParty = isFirstParty(account, relayUrl) val approve = @@ -115,8 +119,9 @@ class AuthCoordinator( // reveal @b — an answer is only about the identity it was shown for. // The bus still collapses concurrent challenges for the same // (relay, account) pair, which is the case the shared prompt was for. - // In practice this rarely means two dialogs: isFirstParty already - // drops every account without its own reason to be on this relay. + // In practice this rarely means two dialogs: for everything except + // reading a follow, isFirstParty already drops every account without + // its own reason to be on this relay. // // But never block the derived stream-key AUTH behind that dialog: on a // relay that hosts our Concord planes we DISMISS the user-auth ASK @@ -231,8 +236,13 @@ class AuthCoordinator( * Merely *following* the counterparty of someone else's traffic is deliberately NOT first-party: * that is exactly how a bystander account got dragged into a paid inbox relay's AUTH (the shared * auth context carries the OTHER account's counterparties, evaluated against this account's - * follow graph). Reads of a followed author's outbox on an auth-gated relay this account doesn't - * use are therefore no longer auto-authed — a deliberate privacy-positive trade-off. + * follow graph). + * + * Reading a followed author's outbox is the one case this cannot speak to. That relay is the + * author's, so nothing here can ever return true for it, which is why + * [com.vitorpamplona.amethyst.commons.relayauth.RelayAuthResolver] applies the + * [com.vitorpamplona.amethyst.commons.relayauth.RelayAuthCustomToggles.readFollows] category + * without consulting this — otherwise the toggle would be permanently off. */ private fun isFirstParty( account: Account, diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthReadFollowsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthReadFollowsTest.kt new file mode 100644 index 0000000000..4d41a8b9f3 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthReadFollowsTest.kt @@ -0,0 +1,128 @@ +/* + * 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.service.relayClient.authCommand.model + +import com.vitorpamplona.amethyst.commons.relayauth.AuthPurpose +import com.vitorpamplona.amethyst.commons.relayauth.AuthPurposeKind +import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthContext +import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthCustomToggles +import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthDecision +import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPermissionStore +import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy +import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthVerdict +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * "…I'm reading someone I follow" has to actually cover the relays it is about. + * + * The whole point of the toggle is the outbox relay of somebody else — a relay we do not publish to, + * do not read our own inbox from, and do not list. That is exactly the shape `isFirstParty` reports + * false for, so requiring it emptied the category: with the toggle explicitly on, every one of the + * user's follows still produced a login prompt for its outbox relay. + */ +class RelayAuthReadFollowsTest { + private val followsRelay = "wss://outbox.someone-i-follow.example/" + private val followed = "a".repeat(64) + private val stranger = "b".repeat(64) + + private class NoStore : RelayAuthPermissionStore { + override suspend fun loadDecision(relayUrl: String): RelayAuthDecision? = null + + override suspend fun storeDecision( + relayUrl: String, + decision: RelayAuthDecision, + ) = Unit + + override suspend fun clearDecision(relayUrl: String) = Unit + + override suspend fun allDecisions(): Map = emptyMap() + } + + private fun ledger(toggles: RelayAuthCustomToggles = RelayAuthCustomToggles()) = + RelayAuthPermissionLedger( + store = NoStore(), + globalPolicy = { RelayAuthPolicy.CUSTOM }, + customToggles = { toggles }, + isFollowed = { it == followed }, + ) + + private fun readOutbox(vararg authors: String) = RelayAuthContext(followsRelay, listOf(AuthPurpose(AuthPurposeKind.READ_OUTBOX, authors.toSet()))) + + @Test + fun readingAFollowAutoAuthenticatesOnTheirOwnOutboxRelay() = + runTest { + // isFirstParty = false is not an edge case here, it is *the* case: the relay belongs to the + // author we are reading. Before the fix this returned ASK, so a user on "decide per relay" + // with this toggle on was prompted once per follow. + assertEquals( + RelayAuthVerdict.ALLOW, + ledger().decide(readOutbox(followed), isFirstParty = false), + ) + } + + @Test + fun readingAFollowStillAsksWhenTheToggleIsOff() = + runTest { + val off = RelayAuthCustomToggles(readFollows = false) + assertEquals( + RelayAuthVerdict.ASK, + ledger(off).decide(readOutbox(followed), isFirstParty = false), + ) + } + + @Test + fun readingAStrangerStillAsks() = + runTest { + // There is deliberately no "read strangers" category — browsing a profile we don't follow + // on a relay of theirs is still a question. + assertEquals( + RelayAuthVerdict.ASK, + ledger().decide(readOutbox(stranger), isFirstParty = false), + ) + } + + @Test + fun oneFollowInABatchedReadIsEnough() = + runTest { + // Outbox reads are batched per relay, so a single filter routinely names a mix. One + // followed author in it is the reason we are on this relay at all. + assertEquals( + RelayAuthVerdict.ALLOW, + ledger().decide(readOutbox(stranger, followed), isFirstParty = false), + ) + } + + @Test + fun messagingIsNotCoveredByTheReadExemption() = + runTest { + // Delivering to a followed user's *inbox* keeps the first-party gate: the pending event + // would be ours, and when it isn't, the traffic belongs to another logged-in account. + val ctx = + RelayAuthContext( + followsRelay, + listOf(AuthPurpose(AuthPurposeKind.SEND_DM, setOf(followed))), + ) + assertEquals(RelayAuthVerdict.ASK, ledger().decide(ctx, isFirstParty = false)) + assertEquals(RelayAuthVerdict.ALLOW, ledger().decide(ctx, isFirstParty = true)) + } +} diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayauth/RelayAuthResolver.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayauth/RelayAuthResolver.kt index 3d095ca38b..f3dd99dae5 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayauth/RelayAuthResolver.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayauth/RelayAuthResolver.kt @@ -62,7 +62,8 @@ data class RelayAuthCustomToggles( * there, a subscription there reads its own inbox/outbox, or the relay is in its own relay list. * False means the only reason we are here belongs to somebody else (another logged-in account's * traffic, or a followed author whose outbox happens to live here). Gates the *automatic* grants - * only: a non-first-party challenge is never auto-allowed, but it still reaches the user as a + * only, and only for the categories it can gate without emptying them (see [RelayAuthResolver]): + * a non-first-party challenge is never auto-allowed there, but it still reaches the user as a * prompt rather than a silent denial. */ data class RelayAuthInputs( @@ -92,12 +93,17 @@ data class RelayAuthInputs( * else fall through * 4. Fall-through → [RelayAuthVerdict.ASK] when the purpose is known, otherwise DENY. * - * The [RelayAuthPolicy.CUSTOM] grant additionally requires [RelayAuthInputs.isFirstParty]: under + * Most [RelayAuthPolicy.CUSTOM] grants additionally require [RelayAuthInputs.isFirstParty]: under * "decide per relay" an account never reveals its identity *without being asked* on a relay it has no * reason of its own to be on, which is what keeps a bystander account off a relay only another account * uses. It deliberately does not suppress the question — a non-first-party challenge we can explain * falls through to ASK, so the user decides rather than getting a silent denial they never see. * + * [RelayAuthCustomToggles.readFollows] is the one category exempt from that gate, because the gate is + * unsatisfiable there rather than merely strict: reading a followed author means talking to *their* + * outbox relay, which is by definition not one we publish to, subscribe to for our own inbox, or list. + * See [customAllows]. + * * [RelayAuthPolicy.ALWAYS] is NOT gated this way: it means what it says, every relay that asks. Users * who want the narrower "only the relays I actually use" behaviour choose CUSTOM. */ @@ -120,14 +126,37 @@ object RelayAuthResolver { // a large follow list, produced a prompt for each of the 250+ third-party outbox relays. RelayAuthPolicy.ALWAYS -> RelayAuthVerdict.ALLOW RelayAuthPolicy.CUSTOM -> - if (inputs.isFirstParty && customAllows(inputs)) RelayAuthVerdict.ALLOW else fallThrough(inputs) + if (customAllows(inputs)) RelayAuthVerdict.ALLOW else fallThrough(inputs) } } + /** + * Whether an enabled [RelayAuthCustomToggles] category covers this relay. + * + * [RelayAuthCustomToggles.readFollows] is checked *before* the [RelayAuthInputs.isFirstParty] + * gate because that gate is unsatisfiable for it, not merely strict. "I'm reading someone I + * follow" describes their outbox relay: not one we publish to, not one serving our own + * inbox/outbox, not one on our list — so `isFirstParty` is false by construction and gating the + * category made it unreachable. Every follow's outbox relay prompted even with the toggle on, and + * the only challenges it ever granted were ones `myRelaysAndVenues` already covered. + * + * Exempting it is safe in the way the gate is meant to be: the follow graph consulted is *this* + * account's, so no other account's traffic can conjure a match. What it can match is another + * logged-in account reading an author we follow too — and the cost of that is an AUTH on a relay + * we would be reading that same author from anyway, which is what the toggle asks for. + * + * Every other category keeps the gate, where it costs them nothing: our own relay list and our + * joined rooms' hosts are first-party by definition, and a pending event of ours makes its + * destination first-party too. That is precisely what stops a bystander account being + * auto-authenticated — and billed — on a paid inbox relay because *another* account's outgoing + * DM happens to name someone we follow. + */ private fun customAllows(inputs: RelayAuthInputs): Boolean { val t = inputs.toggles + if (t.readFollows && inputs.servesFollowedReadCounterparty) return true + if (!inputs.isFirstParty) return false + return (t.myRelaysAndVenues && (inputs.isInMyRelayList || inputs.servesTrustedVenue)) || - (t.readFollows && inputs.servesFollowedReadCounterparty) || (t.messageFollows && inputs.servesFollowedWriteCounterparty) || (t.messageStrangers && inputs.servesStrangerWriteCounterparty) } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/relayauth/RelayAuthResolverTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/relayauth/RelayAuthResolverTest.kt index 9b97ddeea4..485344e630 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/relayauth/RelayAuthResolverTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/relayauth/RelayAuthResolverTest.kt @@ -98,6 +98,31 @@ class RelayAuthResolverTest { ) } + @Test + fun readFollowsGrantsOnTheFollowsOwnOutboxRelay() { + // The situation the toggle is *named for*: someone we follow publishes to a relay of theirs + // that we do not use. `isFirstParty` is false by construction there — the relay is theirs, we + // have no traffic of our own on it — so gating this category on it made "…I'm reading someone + // I follow" unreachable: every follow's outbox relay prompted, on an account with the toggle + // explicitly on. The only time it ever granted was when the relay was also on our own list, + // where `myRelaysAndVenues` already covered it. + assertEquals( + RelayAuthVerdict.ALLOW, + resolve(inputs(servesFollowedReadCounterparty = true, isFirstParty = false)), + ) + // Still off when the toggle is off. + assertEquals( + RelayAuthVerdict.ASK, + resolve( + inputs( + servesFollowedReadCounterparty = true, + isFirstParty = false, + toggles = RelayAuthCustomToggles(readFollows = false), + ), + ), + ) + } + @Test fun customMessageFollowsToggleGatesMessagingFollows() { assertEquals(RelayAuthVerdict.ALLOW, resolve(inputs(servesFollowedWriteCounterparty = true))) @@ -140,9 +165,22 @@ class RelayAuthResolverTest { val allOn = RelayAuthCustomToggles(myRelaysAndVenues = true, readFollows = true, messageFollows = true, messageStrangers = true) assertEquals(RelayAuthVerdict.ASK, resolve(inputs(toggles = allOn, isInMyRelayList = true, isFirstParty = false))) assertEquals(RelayAuthVerdict.ASK, resolve(inputs(toggles = allOn, servesTrustedVenue = true, isFirstParty = false))) - assertEquals(RelayAuthVerdict.ASK, resolve(inputs(toggles = allOn, servesFollowedReadCounterparty = true, isFirstParty = false))) assertEquals(RelayAuthVerdict.ASK, resolve(inputs(toggles = allOn, servesFollowedWriteCounterparty = true, isFirstParty = false))) assertEquals(RelayAuthVerdict.ASK, resolve(inputs(toggles = allOn, servesStrangerWriteCounterparty = true, isFirstParty = false))) + // readFollows is deliberately absent: see readFollowsGrantsOnTheFollowsOwnOutboxRelay. Its + // relay is the *follow's*, never ours, so the gate could only ever empty the category. + } + + @Test + fun readFollowsExemptionDoesNotLeakIntoTheOtherCategories() { + // Only the read category is exempt. With readFollows on but nothing being read from a follow, + // a non-first-party relay still asks for every other reason it might want us. + val allOn = RelayAuthCustomToggles(myRelaysAndVenues = true, readFollows = true, messageFollows = true, messageStrangers = true) + assertEquals(RelayAuthVerdict.ASK, resolve(inputs(toggles = allOn, isInMyRelayList = true, isFirstParty = false))) + assertEquals(RelayAuthVerdict.ASK, resolve(inputs(toggles = allOn, servesFollowedWriteCounterparty = true, isFirstParty = false))) + // The bystander case the gate exists for: another account's outgoing DM names someone we + // follow. Ours is not the traffic, so we do not sign for it without being asked. + assertEquals(RelayAuthVerdict.ASK, resolve(inputs(toggles = allOn, servesStrangerWriteCounterparty = true, isFirstParty = false))) } @Test @@ -158,7 +196,8 @@ class RelayAuthResolverTest { @Test fun customPolicyStillRequiresFirstParty() { // The first-party gate belongs to CUSTOM: a toggle that matches is not enough if the only reason - // we are on this relay belongs to somebody else. + // we are on this relay belongs to somebody else. (Except readFollows, whose relay always + // belongs to the follow — see readFollowsGrantsOnTheFollowsOwnOutboxRelay.) val allOn = RelayAuthCustomToggles(myRelaysAndVenues = true, readFollows = true, messageFollows = true, messageStrangers = true) assertEquals(RelayAuthVerdict.ALLOW, resolve(inputs(toggles = allOn, isInMyRelayList = true, isFirstParty = true))) assertEquals(RelayAuthVerdict.ASK, resolve(inputs(toggles = allOn, isInMyRelayList = true, isFirstParty = false))) From ff71e3c674f3acc4ce6cac8fd49397931368efbd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 16:05:01 +0000 Subject: [PATCH 2/6] fix: scope joined Buzz workspaces per account, correct the venue toggle label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups from auditing the rest of the isFirstParty path against what the relay-auth screen options actually promise. **Joined Buzz workspaces are now per account.** BuzzWorkspaces was a process-wide singleton persisted to one device-global preference key. Everything else feeding AuthCoordinator.isFirstParty is read off the account, so one account redeeming a workspace invite made *every* other logged-in account first-party on that relay — the bystander-account AUTH leak the per-account gate exists to prevent, reintroduced on the line above the call to RelayAuthFirstParty.hasReason (which is why RelayAuthFirstPartyTest could not catch it: the Buzz clause sits outside the pure function it tests). Joining is a per-user act — the invite was redeemed by one key and the relay grants membership to that key alone. BuzzWorkspaces becomes a class held as Account.buzzWorkspaces; the dialect mark stays global, since which protocol a relay speaks is a property of the relay and not of who is asking. BuzzWorkspacePreferences namespaces its key by pubkey and is constructed per account, mirroring the same move the relay-auth overrides made from an app-wide file to a per-account one. AccountCacheState takes no Context, so it gets a startBuzzWorkspacePersistence lambda the way it already takes rootFilesDir and geolocationFlow. Restore falls back to the pre-namespacing key once per account so an upgrade doesn't empty the workspaces hub — that set is what every account already saw, and the first join after the upgrade writes to the account's own key and takes over. **The venue toggle's label was wrong, not its code.** "…it's my relay, or a room I joined" undersold isTrustedVenue, which also covers venues reached through the follow graph — the intent is joined, subscribed to, or favorited. Reworded to "…it's my relay, or a room I joined or follow". Renamed the key rather than reusing it (relay_auth_auto_my_relays → relay_auth_auto_my_relays_and_venues) so a stale Crowdin translation cannot bind to the changed copy, and dropped the 7 now-orphaned translations. Not addressed here: the write categories ("…I'm messaging …") are derived from pending events with the author discarded, so they read as "somebody is messaging" and lean on isFirstParty as an approximate stand-in. Fixing that needs purpose attribution by event.pubKey and is left for a separate change. Verified: 2797 tests green across :commons:jvmTest and :amethyst:testFdroidDebugUnitTest, incl. 2 new BuzzWorkspaces isolation tests; spotlessApply clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014UmCeSWetuKmHdrWcZkWDR --- .../com/vitorpamplona/amethyst/AppModules.kt | 12 ++-- .../vitorpamplona/amethyst/model/Account.kt | 8 +++ .../model/accountsCache/AccountCacheState.kt | 11 ++++ .../preferences/BuzzWorkspacePreferences.kt | 52 ++++++++++++----- .../authCommand/model/AuthCoordinator.kt | 10 ++-- .../loggedIn/buzz/AgentConsoleViewModel.kt | 3 +- .../screen/loggedIn/buzz/BuzzDmDiscovery.kt | 3 +- .../loggedIn/buzz/BuzzDmListViewModel.kt | 14 +++-- .../screen/loggedIn/buzz/BuzzInviteScreen.kt | 3 +- .../loggedIn/buzz/BuzzRelayImportViewModel.kt | 3 +- .../relayauth/RelayAuthSettingsScreen.kt | 2 +- amethyst/src/main/res/values-cs/strings.xml | 1 - .../src/main/res/values-de-rDE/strings.xml | 1 - .../src/main/res/values-hi-rIN/strings.xml | 1 - .../src/main/res/values-hu-rHU/strings.xml | 1 - .../src/main/res/values-pl-rPL/strings.xml | 1 - .../src/main/res/values-pt-rBR/strings.xml | 1 - .../src/main/res/values-sv-rSE/strings.xml | 1 - amethyst/src/main/res/values/strings.xml | 2 +- .../commons/model/buzz/BuzzWorkspaces.kt | 21 ++++--- .../commons/model/buzz/BuzzWorkspacesTest.kt | 58 ++++++++++++++----- 21 files changed, 141 insertions(+), 68 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index ed83fcf45c..44fd6526ae 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -291,11 +291,6 @@ class AppModules( // lazy) so it loads before the first Buzz-relay AUTH and mirrors later changes to disk. val buzzAttestationPrefs = BuzzAttestationPreferences(appContext, applicationIOScope) - // Restore + persist the joined Buzz workspace relays across restarts (device-global). Eager so - // the app knows which relays to sync as workspaces on cold start (Buzz membership is - // server-side; there is no join event to rebuild the set from). - val buzzWorkspacePrefs = BuzzWorkspacePreferences(appContext, applicationIOScope) - // Restore + persist the user's starred Buzz workspace channels across restarts (device-global). val buzzChannelStarPrefs = BuzzChannelStarPreferences(appContext, applicationIOScope) @@ -905,6 +900,13 @@ class AppModules( meterSigner = { MeteringNostrSigner(it, resourceUsage) }, signerPermissionStore = signerPermissionStore, nip46ClientStore = nip46ClientStore, + // Restore + persist each account's joined Buzz workspace relays across restarts, so the + // app knows which relays to sync as workspaces on cold start (Buzz membership is + // server-side; there is no join event to rebuild the set from). Per account: the set + // also makes a relay first-party for NIP-42. + startBuzzWorkspacePersistence = { pubKey, workspaces, accountScope -> + BuzzWorkspacePreferences(appContext, accountScope, pubKey, workspaces) + }, ) val sessionManager = diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index ee50121463..605bb05fdc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -35,6 +35,7 @@ import com.vitorpamplona.amethyst.commons.defaults.DefaultIndexerRelayList import com.vitorpamplona.amethyst.commons.marmot.MarmotManager import com.vitorpamplona.amethyst.commons.model.IAccount import com.vitorpamplona.amethyst.commons.model.buzz.BuzzRelayDialect +import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.model.concord.ConcordChannel import com.vitorpamplona.amethyst.commons.model.concord.ConcordChannelListState import com.vitorpamplona.amethyst.commons.model.concord.ConcordSessionManager @@ -406,6 +407,13 @@ class Account( // answered without a disk read. Backed by a per-account file (see AccountCacheState). val relayAuthPermissions = RelayAuthPermissionCache(relayAuthPermissionStore, scope) + // The `block/buzz` workspaces THIS account joined. Per account, not per device: the invite was + // redeemed by this key and the relay grants membership to it alone — and this set makes the + // relay first-party for NIP-42 (see AuthCoordinator.isFirstParty), so a device-global set would + // hand every other logged-in account an automatic login on a workspace it never joined. + // Restored/persisted per account by BuzzWorkspacePreferences (see AccountCacheState). + val buzzWorkspaces = BuzzWorkspaces() + // Per-account NIP-42 policy evaluator (blocked → per-relay override → global policy → prompt), // reading THIS account's own toggles, relay lists and follow graph. Cached here so every AUTH // path (foreground screen + background notification consumer) shares one instance, and so an diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/accountsCache/AccountCacheState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/accountsCache/AccountCacheState.kt index 49b9004e68..2881c4f254 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/accountsCache/AccountCacheState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/accountsCache/AccountCacheState.kt @@ -26,6 +26,7 @@ import com.vitorpamplona.amethyst.commons.connectedApps.nip46.InMemoryNip46Clien import com.vitorpamplona.amethyst.commons.connectedApps.nip46.Nip46ClientStore import com.vitorpamplona.amethyst.commons.connectedApps.signers.InMemoryNostrSignerPermissionStore import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerPermissionStore +import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.service.pow.PoWPublishQueue import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.AccountSettings @@ -73,6 +74,12 @@ class AccountCacheState( val signerPermissionStore: NostrSignerPermissionStore = InMemoryNostrSignerPermissionStore(), /** App-global store of connected NIP-46 client display + relay info. */ val nip46ClientStore: Nip46ClientStore = InMemoryNip46ClientStore(), + /** + * Starts per-account persistence of the joined Buzz workspaces (restore now, mirror later + * changes). A lambda because the store needs an Android `Context` and this class deliberately + * takes none; no-op by default so tests and non-Android hosts build an Account without it. + */ + val startBuzzWorkspacePersistence: (HexKey, BuzzWorkspaces, CoroutineScope) -> Unit = { _, _, _ -> }, ) { val accounts = MutableStateFlow>(emptyMap()) @@ -286,6 +293,10 @@ class AccountCacheState( signerPermissionStore = signerPermissionStore, nip46ClientStore = nip46ClientStore, ).also { newAccount -> + // Per account, not per device: this set makes a relay first-party for NIP-42, so a + // shared one hands every other logged-in account an automatic login on a workspace it + // never joined. See BuzzWorkspacePreferences. + startBuzzWorkspacePersistence(signer.pubKey, newAccount.buzzWorkspaces, newAccount.scope) accounts.update { existingAccounts -> existingAccounts.plus(Pair(signer.pubKey, newAccount)) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzWorkspacePreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzWorkspacePreferences.kt index cdd9a7855d..9b15b361b1 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzWorkspacePreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzWorkspacePreferences.kt @@ -25,6 +25,7 @@ import androidx.compose.runtime.Stable import androidx.datastore.preferences.core.edit import androidx.datastore.preferences.core.stringSetPreferencesKey import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces +import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer import com.vitorpamplona.quartz.utils.Log @@ -35,36 +36,56 @@ import kotlinx.coroutines.launch import kotlin.coroutines.cancellation.CancellationException /** - * Device-global persistence for the set of joined `block/buzz` workspaces ([BuzzWorkspaces]), - * so the app knows which relays to connect + NIP-42-authenticate + run member-channel discovery - * against on a cold start — Buzz membership is server-side (granted by the HTTP invite claim), - * with no NIP-51/kind-10009 join event to rebuild the set from. Uses the app-wide - * [sharedPreferencesDataStore] like [BuzzAttestationPreferences] (not per-account: a joined - * relay is workspace-wide, and restoring only marks relays to sync — the relay still gates every - * read/write by the authenticated key). + * Per-account persistence for the set of joined `block/buzz` workspaces ([BuzzWorkspaces]), so the + * app knows which relays to connect + NIP-42-authenticate + run member-channel discovery against on + * a cold start — Buzz membership is server-side (granted by the HTTP invite claim), with no + * NIP-51/kind-10009 join event to rebuild the set from. * - * On construction it loads the saved relay URLs into the singleton (re-normalizing each, dropping - * any that no longer parse), then mirrors every later change back to disk. Construct once, eagerly. + * **Per account, not per device.** The set used to be one device-global key shared by every logged-in + * account, on the reasoning that restoring only marks relays to sync and the relay gates each + * read/write by the authenticated key anyway. That missed one consumer: the joined set also makes a + * relay first-party in `AuthCoordinator.isFirstParty`, so one account joining a workspace silently + * gave *every* other logged-in account an automatic NIP-42 login there — the bystander-account leak + * the per-account gate exists to prevent. The key is namespaced by pubkey for the same reason the + * relay-auth overrides moved to a per-account file. + * + * Still on the app-wide [sharedPreferencesDataStore] file — the namespacing, not the file, is what + * separates accounts, and one file avoids a second DataStore per logged-in account. + * + * On construction it loads this account's saved relay URLs into [workspaces] (re-normalizing each, + * dropping any that no longer parse), then mirrors every later change back to disk. Construct once + * per account, eagerly. */ @Stable class BuzzWorkspacePreferences( private val context: Context, private val scope: CoroutineScope, + private val pubKeyHex: HexKey, + private val workspaces: BuzzWorkspaces, ) { + private val key = stringSetPreferencesKey("$KEY_PREFIX$pubKeyHex") + init { scope.launch { restoreFromDisk() // Persist on every change AFTER the initial restore (drop(1) skips the value present // at collection start, which restoreFromDisk already wrote). - BuzzWorkspaces.flow.drop(1).collect { persist(it) } + workspaces.flow.drop(1).collect { persist(it) } } } private suspend fun restoreFromDisk() { try { - val raw = context.sharedPreferencesDataStore.data.first()[KEY] ?: return + val prefs = context.sharedPreferencesDataStore.data.first() + // Fall back to the pre-namespacing device-global key so an upgrade doesn't empty the + // workspaces hub. That set is whatever any account joined, which is exactly what every + // account already saw before this became per-account — so seeding from it changes + // nothing that was true yesterday, and the first join after the upgrade writes to this + // account's own key and takes over. The legacy key is left in place for the other + // accounts to seed from; nothing writes it again. + val raw = prefs[key] ?: prefs[LEGACY_KEY] ?: return val relays = raw.mapNotNull { RelayUrlNormalizer.normalizeOrNull(it) }.toSet() - if (relays.isNotEmpty()) BuzzWorkspaces.restore(relays) + if (relays.isNotEmpty()) workspaces.restore(relays) } catch (e: Exception) { if (e is CancellationException) throw e Log.e("BuzzWorkspacePrefs") { "Error reading joined workspaces: ${e.message}" } @@ -74,7 +95,7 @@ class BuzzWorkspacePreferences( private suspend fun persist(relays: Set) { try { context.sharedPreferencesDataStore.edit { prefs -> - prefs[KEY] = relays.map { it.url }.toSet() + prefs[key] = relays.map { it.url }.toSet() } } catch (e: Exception) { if (e is CancellationException) throw e @@ -83,6 +104,9 @@ class BuzzWorkspacePreferences( } companion object { - private val KEY = stringSetPreferencesKey("buzz.joinedWorkspaces") + private const val KEY_PREFIX = "buzz.joinedWorkspaces." + + /** The device-global key written before the set became per-account; read-only now. */ + private val LEGACY_KEY = stringSetPreferencesKey("buzz.joinedWorkspaces") } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt index 91c3e0a791..cb5a0c3d37 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt @@ -23,7 +23,6 @@ package com.vitorpamplona.amethyst.service.relayClient.authCommand.model import androidx.compose.runtime.Stable import com.vitorpamplona.amethyst.commons.model.buzz.BuzzHeldAttestations import com.vitorpamplona.amethyst.commons.model.buzz.BuzzRelayDialect -import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthContext import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthDecision import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthVerdict @@ -248,11 +247,12 @@ class AuthCoordinator( account: Account, relayUrl: NormalizedRelayUrl, ): Boolean = - // A Buzz workspace the user explicitly joined is a first-party reason to authenticate: its - // channel/DM discovery is read-only (`#p` = me), which is otherwise deliberately NOT + // A Buzz workspace THIS account explicitly joined is a first-party reason to authenticate: + // its channel/DM discovery is read-only (`#p` = me), which is otherwise deliberately NOT // first-party, so without this the p-gated 44100/30622 reads would never be served and the - // workspace would stay empty. - BuzzWorkspaces.isJoined(relayUrl) || + // workspace would stay empty. Read off the account, never a device-global set: the invite was + // redeemed by one key, and a shared set made every other logged-in account first-party here. + account.buzzWorkspaces.isJoined(relayUrl) || RelayAuthFirstParty.hasReason( me = account.pubKey, relayUrl = relayUrl, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentConsoleViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentConsoleViewModel.kt index 2433459063..9cd906ebcb 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentConsoleViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentConsoleViewModel.kt @@ -26,7 +26,6 @@ import androidx.lifecycle.viewModelScope import com.vitorpamplona.amethyst.commons.model.buzz.AgentFleetAggregator import com.vitorpamplona.amethyst.commons.model.buzz.AgentFleetMetrics import com.vitorpamplona.amethyst.commons.model.buzz.BuzzRelayDialect -import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthDecision import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.LocalCache @@ -107,7 +106,7 @@ class AgentConsoleViewModel : ViewModel() { this.scopeRelay = relay this.account = account relay?.let { - val newlyJoined = BuzzWorkspaces.join(it) + val newlyJoined = account.buzzWorkspaces.join(it) viewModelScope.launch { account.relayAuthLedger.setDecision(it.url, RelayAuthDecision.ALLOW) } if (newlyJoined) reconnectPoolAfterJoin(account.client) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmDiscovery.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmDiscovery.kt index f1fd9926d7..b9b8e9a7e0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmDiscovery.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmDiscovery.kt @@ -27,7 +27,6 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.vitorpamplona.amethyst.commons.model.buzz.BuzzChannelInvite import com.vitorpamplona.amethyst.commons.model.buzz.BuzzChannelInvites import com.vitorpamplona.amethyst.commons.model.buzz.BuzzDmChannels -import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthDecision import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.LocalCache @@ -62,7 +61,7 @@ import kotlinx.coroutines.launch @Composable fun BuzzDmDiscoveryPreload(accountViewModel: AccountViewModel) { val account = accountViewModel.account - val joined by BuzzWorkspaces.flow.collectAsStateWithLifecycle() + val joined by account.buzzWorkspaces.flow.collectAsStateWithLifecycle() // Restart the whole discovery (initial warm-auth fetch + live 44100 subs) whenever the joined // workspace set changes; the LaunchedEffect scope owns the live subscriptions and cancels them on diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmListViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmListViewModel.kt index dccf0cb851..e9316522a3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmListViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzDmListViewModel.kt @@ -26,7 +26,6 @@ import androidx.lifecycle.viewModelScope import com.vitorpamplona.amethyst.commons.model.buzz.BuzzDmChannels import com.vitorpamplona.amethyst.commons.model.buzz.BuzzDmRegistry import com.vitorpamplona.amethyst.commons.model.buzz.BuzzRelayDialect -import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthDecision import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.LocalCache @@ -111,7 +110,14 @@ class BuzzDmListViewModel : ViewModel() { val lastActivity: Long, ) - private fun relays(): Set = scopeRelay?.let { setOf(it) } ?: (BuzzWorkspaces.flow.value + BuzzRelayDialect.flow.value) + private fun relays(): Set = + scopeRelay?.let { setOf(it) } ?: ( + account + ?.buzzWorkspaces + ?.flow + ?.value + .orEmpty() + BuzzRelayDialect.flow.value + ) /** * Binds to [account] scoped to the community [relayUrl]. Marks that relay a joined workspace and @@ -128,7 +134,7 @@ class BuzzDmListViewModel : ViewModel() { val relay = RelayUrlNormalizer.normalizeOrNull(relayUrl) ?: return this.scopeRelay = relay - val newlyJoined = BuzzWorkspaces.join(relay) + val newlyJoined = account.buzzWorkspaces.join(relay) viewModelScope.launch { account.relayAuthLedger.setDecision(relay.url, RelayAuthDecision.ALLOW) } if (newlyJoined) reconnectPoolAfterJoin(account.client) @@ -315,7 +321,7 @@ class BuzzDmListViewModel : ViewModel() { } // Re-project when my hidden set (30622) or the joined-relay set changes. launch { - combine(BuzzDmRegistry.hidden, BuzzWorkspaces.flow, BuzzRelayDialect.flow) { _, _, _ -> } + combine(BuzzDmRegistry.hidden, account.buzzWorkspaces.flow, BuzzRelayDialect.flow) { _, _, _ -> } .collect { rebuildRows(account) } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzInviteScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzInviteScreen.kt index be61d00fe7..d5b3e7a101 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzInviteScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzInviteScreen.kt @@ -55,7 +55,6 @@ import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols -import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthDecision import com.vitorpamplona.amethyst.favorites.FavoriteAppLauncher import com.vitorpamplona.amethyst.ui.navigation.navs.INav @@ -189,7 +188,7 @@ fun BuzzInviteScreen( // authenticates without a prompt, then hand off to the in-app window.nostr // browser to accept terms + sign the claim. RelayUrlNormalizer.normalizeOrNull(invite.relayUrl())?.let { relay -> - BuzzWorkspaces.join(relay) + accountViewModel.account.buzzWorkspaces.join(relay) scope.launch { accountViewModel.account.relayAuthLedger.setDecision(relay.url, RelayAuthDecision.ALLOW) } } FavoriteAppLauncher.launchUrl(context, link) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzRelayImportViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzRelayImportViewModel.kt index a3976d203f..2b4cf9a15f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzRelayImportViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/BuzzRelayImportViewModel.kt @@ -22,7 +22,6 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.buzz import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.model.nip29RelayGroups.RelayGroupDeletions import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthDecision import com.vitorpamplona.amethyst.model.Account @@ -104,7 +103,7 @@ class BuzzRelayImportViewModel : ViewModel() { // The user came here to import from THIS relay: remember it as a joined workspace (persisted, // marks the Buzz dialect) and pre-approve NIP-42 auth so the `#p=me` read below is served. - val newlyJoined = BuzzWorkspaces.join(normalized) + val newlyJoined = account.buzzWorkspaces.join(normalized) viewModelScope.launch { account.relayAuthLedger.setDecision(normalized.url, RelayAuthDecision.ALLOW) } // Unlocks the persistent group-roster (39002) subscription — see [reconnectPoolAfterJoin]. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relayauth/RelayAuthSettingsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relayauth/RelayAuthSettingsScreen.kt index 209329fa47..463c4d34a0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relayauth/RelayAuthSettingsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/relayauth/RelayAuthSettingsScreen.kt @@ -232,7 +232,7 @@ fun RelayAuthSettingsScreen( ) { SettingsSwitchTile( icon = MaterialSymbols.Dns, - title = R.string.relay_auth_auto_my_relays, + title = R.string.relay_auth_auto_my_relays_and_venues, checked = myRelays, onCheckedChange = { account.settings.changeRelayAuthTrustMyRelaysAndVenues(it) }, ) diff --git a/amethyst/src/main/res/values-cs/strings.xml b/amethyst/src/main/res/values-cs/strings.xml index 7f82c4ef79..3619e47f67 100644 --- a/amethyst/src/main/res/values-cs/strings.xml +++ b/amethyst/src/main/res/values-cs/strings.xml @@ -1170,7 +1170,6 @@ keys are new rather than reused - a stale translation of the old standalone titles would read as a non-sequitur under this header. --> Přihlásit se bez ptaní, když… - …jde o mé relé nebo o místnost, do které jsem vstoupil …čtu někoho, koho sleduji …píšu někomu, koho sleduji …píšu komukoli jinému diff --git a/amethyst/src/main/res/values-de-rDE/strings.xml b/amethyst/src/main/res/values-de-rDE/strings.xml index 9b6f63b595..0609204e38 100644 --- a/amethyst/src/main/res/values-de-rDE/strings.xml +++ b/amethyst/src/main/res/values-de-rDE/strings.xml @@ -1110,7 +1110,6 @@ keys are new rather than reused - a stale translation of the old standalone titles would read as a non-sequitur under this header. --> Ohne Nachfrage anmelden, wenn… - …es mein Relay ist oder ein Raum, dem ich beigetreten bin …ich jemanden lese, dem ich folge …ich jemandem schreibe, dem ich folge …ich jemand anderem schreibe diff --git a/amethyst/src/main/res/values-hi-rIN/strings.xml b/amethyst/src/main/res/values-hi-rIN/strings.xml index 396d7b7ac9..891c1738b8 100644 --- a/amethyst/src/main/res/values-hi-rIN/strings.xml +++ b/amethyst/src/main/res/values-hi-rIN/strings.xml @@ -1110,7 +1110,6 @@ keys are new rather than reused - a stale translation of the old standalone titles would read as a non-sequitur under this header. --> कब पूछे बिना प्रवेशांकन करें\u2026 - \u2026यह मेरा पुनःप्रसारक है। अथवा एक शाला जिससे मैं जुड चुका \u2026मैं पढ रहा हूँ किसी को जिसका मैं अनुगमन करता हूँ \u2026मैं सन्देश भेज रहा हूँ किसी को जिसका मैं अनुगमन करता हूँ \u2026मैं किसी अन्य को सन्देश भेज रहा हूँ diff --git a/amethyst/src/main/res/values-hu-rHU/strings.xml b/amethyst/src/main/res/values-hu-rHU/strings.xml index 3260f909e9..26955e7ec9 100644 --- a/amethyst/src/main/res/values-hu-rHU/strings.xml +++ b/amethyst/src/main/res/values-hu-rHU/strings.xml @@ -1111,7 +1111,6 @@ keys are new rather than reused - a stale translation of the old standalone titles would read as a non-sequitur under this header. --> Jelentkezzen be kérdés nélkül, ha\u2026 - \u2026ez a saját átjátszóm, vagy egy szoba, amihez csatlakozott \u2026olyan valakit olvasok, akit követek \u2026olyan valakinek írok, akit követek \u2026bárki másnak írok diff --git a/amethyst/src/main/res/values-pl-rPL/strings.xml b/amethyst/src/main/res/values-pl-rPL/strings.xml index 999b439ab8..9a431ac71e 100644 --- a/amethyst/src/main/res/values-pl-rPL/strings.xml +++ b/amethyst/src/main/res/values-pl-rPL/strings.xml @@ -1171,7 +1171,6 @@ Zaplanowane posty z innych kont nie zostaną opublikowane, dopóki to konto jest keys are new rather than reused - a stale translation of the old standalone titles would read as a non-sequitur under this header. --> Zaloguj się bez pytania, kiedy\u2026 - \u2026to mój transmiter lub pokój, do którego dołączyłem Czytam wpis osoby, którą obserwuję \u2026wysyłam wiadomość do osoby, którą obserwuję \u2026piszę do wszystkich pozostałych diff --git a/amethyst/src/main/res/values-pt-rBR/strings.xml b/amethyst/src/main/res/values-pt-rBR/strings.xml index c540be9ad5..8ad1958b4a 100644 --- a/amethyst/src/main/res/values-pt-rBR/strings.xml +++ b/amethyst/src/main/res/values-pt-rBR/strings.xml @@ -1108,7 +1108,6 @@ keys are new rather than reused - a stale translation of the old standalone titles would read as a non-sequitur under this header. --> Entrar sem perguntar quando… - …for o meu relay, ou uma sala em que entrei …eu estiver lendo alguém que sigo …eu estiver enviando mensagem para alguém que sigo …eu estiver enviando mensagem para qualquer outra pessoa diff --git a/amethyst/src/main/res/values-sv-rSE/strings.xml b/amethyst/src/main/res/values-sv-rSE/strings.xml index a743b77c10..b08b32c9a6 100644 --- a/amethyst/src/main/res/values-sv-rSE/strings.xml +++ b/amethyst/src/main/res/values-sv-rSE/strings.xml @@ -1108,7 +1108,6 @@ keys are new rather than reused - a stale translation of the old standalone titles would read as a non-sequitur under this header. --> Logga in utan att fråga när… - …det är mitt relä, eller ett rum jag gått med i …jag läser någon jag följer …jag skriver till någon jag följer …jag skriver till någon annan diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 97e1ef306e..8c36a5e5b0 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -1149,7 +1149,7 @@ keys are new rather than reused - a stale translation of the old standalone titles would read as a non-sequitur under this header. --> Log in without asking when\u2026 - \u2026it\'s my relay, or a room I joined + \u2026it\'s my relay, or a room I joined or follow \u2026I\'m reading someone I follow \u2026I\'m messaging someone I follow \u2026I\'m messaging anyone else diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzWorkspaces.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzWorkspaces.kt index f5a6e42574..2b599f7404 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzWorkspaces.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzWorkspaces.kt @@ -35,11 +35,19 @@ import kotlinx.coroutines.flow.StateFlow * join event to key off — so this joined set is the client's own bookkeeping of which relays * to sync as workspaces. * - * Persisted across launches by the platform (`BuzzWorkspacePreferences` on Android mirrors it - * to a device-global store and restores it at startup). Like [BuzzRelayDialect] it is a - * process-wide singleton; joining also marks the relay as a Buzz dialect. + * **One instance per account** (`Account.buzzWorkspaces`) — deliberately NOT a process-wide + * singleton like [BuzzRelayDialect]. Joining is a per-user act: the invite was redeemed by one + * key and the relay grants membership to that key alone. While this was device-global it also + * fed `AuthCoordinator.isFirstParty`, so one account joining a workspace made *every* logged-in + * account first-party there — the bystander-account AUTH leak the per-account gate exists to + * prevent. The dialect mark stays global: which protocol a relay speaks is a property of the + * relay, not of who is asking. + * + * Persisted across launches by the platform (`BuzzWorkspacePreferences` on Android mirrors each + * account's set to that account's own store and restores it at startup). Joining also marks the + * relay as a Buzz dialect. */ -object BuzzWorkspaces { +class BuzzWorkspaces { private val joined = MutableStateFlow>(emptySet()) /** The joined workspace relays; discovery subscriptions and the workspaces hub collect this. */ @@ -74,9 +82,4 @@ object BuzzWorkspaces { relays.forEach { BuzzRelayDialect.mark(it) } joined.value = relays } - - /** Test-only: clears the joined set so unit tests don't leak state into each other. */ - fun clearForTesting() { - joined.value = emptySet() - } } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzWorkspacesTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzWorkspacesTest.kt index 9e72bbcd3b..8a8195eac6 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzWorkspacesTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzWorkspacesTest.kt @@ -29,44 +29,74 @@ import kotlin.test.assertFalse import kotlin.test.assertTrue class BuzzWorkspacesTest { + // A fresh instance per test: the joined set is per-account state now, not a process-wide + // singleton, so there is nothing to reset between tests. The dialect mark stays global. + private val workspaces = BuzzWorkspaces() + private val a = RelayUrlNormalizer.normalize("wss://a.buzz.example") private val b = RelayUrlNormalizer.normalize("wss://b.buzz.example") @BeforeTest fun setup() { - BuzzWorkspaces.clearForTesting() BuzzRelayDialect.clearForTesting() } @AfterTest fun teardown() { - BuzzWorkspaces.clearForTesting() BuzzRelayDialect.clearForTesting() } @Test fun joiningRecordsAndMarksDialect() { - assertTrue(BuzzWorkspaces.join(a)) - assertTrue(BuzzWorkspaces.isJoined(a)) - assertEquals(setOf(a), BuzzWorkspaces.flow.value) + assertTrue(workspaces.join(a)) + assertTrue(workspaces.isJoined(a)) + assertEquals(setOf(a), workspaces.flow.value) // Joining also marks the relay a Buzz dialect so its events render as workspace channels. assertTrue(BuzzRelayDialect.isBuzz(a)) // Re-joining is a no-op (returns false). - assertFalse(BuzzWorkspaces.join(a)) + assertFalse(workspaces.join(a)) } @Test fun leaveRemoves() { - BuzzWorkspaces.join(a) - BuzzWorkspaces.join(b) - BuzzWorkspaces.leave(a) - assertEquals(setOf(b), BuzzWorkspaces.flow.value) - assertFalse(BuzzWorkspaces.isJoined(a)) + workspaces.join(a) + workspaces.join(b) + workspaces.leave(a) + assertEquals(setOf(b), workspaces.flow.value) + assertFalse(workspaces.isJoined(a)) } @Test fun restoreReplacesAndMarksAll() { - BuzzWorkspaces.join(a) - BuzzWorkspaces.restore(setOf(b)) - assertEquals(setOf(b), BuzzWorkspaces.flow.value) + workspaces.join(a) + workspaces.restore(setOf(b)) + assertEquals(setOf(b), workspaces.flow.value) assertTrue(BuzzRelayDialect.isBuzz(b)) } + + @Test + fun oneAccountsJoinDoesNotJoinForAnother() { + // The bystander-AUTH leak this became per-account for: while the joined set was a + // process-wide singleton it fed AuthCoordinator.isFirstParty, so an account that never + // redeemed the invite was auto-authenticated (and identified) on someone else's workspace. + val mine = BuzzWorkspaces() + val theirs = BuzzWorkspaces() + + mine.join(a) + + assertTrue(mine.isJoined(a)) + assertFalse(theirs.isJoined(a)) + assertEquals(emptySet(), theirs.flow.value) + } + + @Test + fun leavingOnOneAccountLeavesTheOtherJoined() { + val mine = BuzzWorkspaces() + val theirs = BuzzWorkspaces() + mine.join(a) + theirs.join(a) + + mine.leave(a) + + assertFalse(mine.isJoined(a)) + assertTrue(theirs.isJoined(a)) + } } From 64cb6484473d43c1b4aa498c335d2d23b89098b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 16:25:42 +0000 Subject: [PATCH 3/6] fix: scope starred Buzz channels per account MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same class of bug as the joined workspaces, spotted by reading the neighbour: BuzzChannelStars was a process-wide singleton on one device-global preference key. Its own KDoc calls a star "personal" and "the client's own bookkeeping", and the only justification offered for sharing it was "like BuzzWorkspaces, this is a process-wide singleton" — which stopped being true one commit ago. A star reorders and badges the community channel list, so while the set was shared, one account pinning a channel reordered every other logged-in account's list, and switching accounts silently rewrote the set they had in common. No AUTH exposure here — this one is cosmetic — but it is the same mistake and the plumbing was already in place. BuzzChannelStars becomes a class held as Account.buzzChannelStars, and BuzzChannelStarPreferences namespaces its key by pubkey with the same one-time fallback to the pre-namespacing key, so upgrading doesn't unpin everything. BuzzPinDropdownItem takes an AccountViewModel to read and toggle the right set. AccountCacheState's per-account Buzz hook collapses from startBuzzWorkspacePersistence(pubKey, workspaces, scope) to startBuzzPersistence(account): two features needing the same wiring is the point at which passing the account beats threading each piece of state through. Verified: 2800 tests green across :commons:jvmTest and :amethyst:testFdroidDebugUnitTest, incl. 3 new BuzzChannelStars tests (one pinning the cross-account isolation); spotlessApply clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014UmCeSWetuKmHdrWcZkWDR --- .../com/vitorpamplona/amethyst/AppModules.kt | 16 ++--- .../vitorpamplona/amethyst/model/Account.kt | 6 ++ .../model/accountsCache/AccountCacheState.kt | 14 ++-- .../preferences/BuzzChannelStarPreferences.kt | 33 ++++++--- .../relayGroup/BuzzChannelMenuItems.kt | 12 ++-- .../relayGroup/RelayGroupChannelListScreen.kt | 4 +- .../relayGroup/RelayGroupThreadsScreen.kt | 2 +- .../relayGroup/RelayGroupTopBar.kt | 2 +- .../commons/model/buzz/BuzzChannelStars.kt | 16 ++--- .../model/buzz/BuzzChannelStarsTest.kt | 69 +++++++++++++++++++ 10 files changed, 132 insertions(+), 42 deletions(-) create mode 100644 commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzChannelStarsTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index 44fd6526ae..18c9289c08 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -291,9 +291,6 @@ class AppModules( // lazy) so it loads before the first Buzz-relay AUTH and mirrors later changes to disk. val buzzAttestationPrefs = BuzzAttestationPreferences(appContext, applicationIOScope) - // Restore + persist the user's starred Buzz workspace channels across restarts (device-global). - val buzzChannelStarPrefs = BuzzChannelStarPreferences(appContext, applicationIOScope) - // Restore + persist the set of relay-group channels deleted (kind-9008) on this device, so a // deleted channel stays hidden across a restart even if the host relay re-announces a stale // kind-44100 for it (device-global; a delete is authoritative and terminal for everyone). @@ -900,12 +897,13 @@ class AppModules( meterSigner = { MeteringNostrSigner(it, resourceUsage) }, signerPermissionStore = signerPermissionStore, nip46ClientStore = nip46ClientStore, - // Restore + persist each account's joined Buzz workspace relays across restarts, so the - // app knows which relays to sync as workspaces on cold start (Buzz membership is - // server-side; there is no join event to rebuild the set from). Per account: the set - // also makes a relay first-party for NIP-42. - startBuzzWorkspacePersistence = { pubKey, workspaces, accountScope -> - BuzzWorkspacePreferences(appContext, accountScope, pubKey, workspaces) + // Restore + persist the Buzz bookkeeping that has no Nostr event to rebuild from: the + // joined workspace relays (so the app knows which relays to sync as workspaces on cold + // start — Buzz membership is server-side) and the starred channels. Per account: the + // joined set makes a relay first-party for NIP-42, and a star is personal. + startBuzzPersistence = { account -> + BuzzWorkspacePreferences(appContext, account.scope, account.pubKey, account.buzzWorkspaces) + BuzzChannelStarPreferences(appContext, account.scope, account.pubKey, account.buzzChannelStars) }, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index 605bb05fdc..a7ada72abc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -34,6 +34,7 @@ import com.vitorpamplona.amethyst.commons.defaults.Constants import com.vitorpamplona.amethyst.commons.defaults.DefaultIndexerRelayList import com.vitorpamplona.amethyst.commons.marmot.MarmotManager import com.vitorpamplona.amethyst.commons.model.IAccount +import com.vitorpamplona.amethyst.commons.model.buzz.BuzzChannelStars import com.vitorpamplona.amethyst.commons.model.buzz.BuzzRelayDialect import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.model.concord.ConcordChannel @@ -414,6 +415,11 @@ class Account( // Restored/persisted per account by BuzzWorkspacePreferences (see AccountCacheState). val buzzWorkspaces = BuzzWorkspaces() + // The Buzz channels THIS account pinned. A star says which channels this user wants at the top + // of the community view, so a shared set let one account reorder and badge every other one's + // channel list. Restored/persisted per account by BuzzChannelStarPreferences. + val buzzChannelStars = BuzzChannelStars() + // Per-account NIP-42 policy evaluator (blocked → per-relay override → global policy → prompt), // reading THIS account's own toggles, relay lists and follow graph. Cached here so every AUTH // path (foreground screen + background notification consumer) shares one instance, and so an diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/accountsCache/AccountCacheState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/accountsCache/AccountCacheState.kt index 2881c4f254..cc875f2b0d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/accountsCache/AccountCacheState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/accountsCache/AccountCacheState.kt @@ -26,7 +26,6 @@ import com.vitorpamplona.amethyst.commons.connectedApps.nip46.InMemoryNip46Clien import com.vitorpamplona.amethyst.commons.connectedApps.nip46.Nip46ClientStore import com.vitorpamplona.amethyst.commons.connectedApps.signers.InMemoryNostrSignerPermissionStore import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerPermissionStore -import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.service.pow.PoWPublishQueue import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.model.AccountSettings @@ -75,11 +74,12 @@ class AccountCacheState( /** App-global store of connected NIP-46 client display + relay info. */ val nip46ClientStore: Nip46ClientStore = InMemoryNip46ClientStore(), /** - * Starts per-account persistence of the joined Buzz workspaces (restore now, mirror later - * changes). A lambda because the store needs an Android `Context` and this class deliberately + * Starts per-account persistence of the Buzz client-side bookkeeping that has no Nostr event to + * rebuild from — the joined workspaces and the starred channels (restore now, mirror later + * changes). A lambda because those stores need an Android `Context` and this class deliberately * takes none; no-op by default so tests and non-Android hosts build an Account without it. */ - val startBuzzWorkspacePersistence: (HexKey, BuzzWorkspaces, CoroutineScope) -> Unit = { _, _, _ -> }, + val startBuzzPersistence: (Account) -> Unit = { }, ) { val accounts = MutableStateFlow>(emptyMap()) @@ -293,10 +293,10 @@ class AccountCacheState( signerPermissionStore = signerPermissionStore, nip46ClientStore = nip46ClientStore, ).also { newAccount -> - // Per account, not per device: this set makes a relay first-party for NIP-42, so a + // Per account, not per device: the joined set makes a relay first-party for NIP-42, so a // shared one hands every other logged-in account an automatic login on a workspace it - // never joined. See BuzzWorkspacePreferences. - startBuzzWorkspacePersistence(signer.pubKey, newAccount.buzzWorkspaces, newAccount.scope) + // never joined, and a shared star set reorders everyone's channel list at once. + startBuzzPersistence(newAccount) accounts.update { existingAccounts -> existingAccounts.plus(Pair(signer.pubKey, newAccount)) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzChannelStarPreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzChannelStarPreferences.kt index ba694304b6..e0997047b5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzChannelStarPreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzChannelStarPreferences.kt @@ -25,6 +25,7 @@ import androidx.compose.runtime.Stable import androidx.datastore.preferences.core.edit import androidx.datastore.preferences.core.stringSetPreferencesKey import com.vitorpamplona.amethyst.commons.model.buzz.BuzzChannelStars +import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.drop @@ -33,28 +34,39 @@ import kotlinx.coroutines.launch import kotlin.coroutines.cancellation.CancellationException /** - * Device-global persistence for the set of starred Buzz workspace channels ([BuzzChannelStars]), - * so favorites survive a restart. Mirrors [BuzzWorkspacePreferences]: app-wide (not per-account), - * loads the saved ids into the singleton on construction, then writes every later change back. - * Construct once, eagerly. + * Per-account persistence for the set of starred Buzz workspace channels ([BuzzChannelStars]), so + * favorites survive a restart. Mirrors [BuzzWorkspacePreferences] in both shape and reasoning: the + * key is namespaced by pubkey because a star is personal — it says which channels *this* user wants + * pinned — and one device-global set meant one account's favorites reordered and badged every other + * logged-in account's channel list. Loads this account's saved ids into [stars] on construction, + * then writes every later change back. Construct once per account, eagerly. */ @Stable class BuzzChannelStarPreferences( private val context: Context, private val scope: CoroutineScope, + private val pubKeyHex: HexKey, + private val stars: BuzzChannelStars, ) { + private val key = stringSetPreferencesKey("$KEY_PREFIX$pubKeyHex") + init { scope.launch { restoreFromDisk() // drop(1) skips the value present at collection start, which restoreFromDisk already wrote. - BuzzChannelStars.flow.drop(1).collect { persist(it) } + stars.flow.drop(1).collect { persist(it) } } } private suspend fun restoreFromDisk() { try { - val raw = context.sharedPreferencesDataStore.data.first()[KEY] ?: return - if (raw.isNotEmpty()) BuzzChannelStars.restore(raw) + val prefs = context.sharedPreferencesDataStore.data.first() + // Fall back to the pre-namespacing device-global key so an upgrade doesn't unpin + // everything. That set is what every account already saw; the next toggle writes to this + // account's own key and takes over. The legacy key is left for other accounts to seed + // from and is never written again. + val raw = prefs[key] ?: prefs[LEGACY_KEY] ?: return + if (raw.isNotEmpty()) stars.restore(raw) } catch (e: Exception) { if (e is CancellationException) throw e Log.e("BuzzChannelStarPrefs") { "Error reading starred channels: ${e.message}" } @@ -63,7 +75,7 @@ class BuzzChannelStarPreferences( private suspend fun persist(ids: Set) { try { - context.sharedPreferencesDataStore.edit { prefs -> prefs[KEY] = ids } + context.sharedPreferencesDataStore.edit { prefs -> prefs[key] = ids } } catch (e: Exception) { if (e is CancellationException) throw e Log.e("BuzzChannelStarPrefs") { "Error writing starred channels: ${e.message}" } @@ -71,6 +83,9 @@ class BuzzChannelStarPreferences( } companion object { - private val KEY = stringSetPreferencesKey("buzz.starredChannels") + private const val KEY_PREFIX = "buzz.starredChannels." + + /** The device-global key written before the set became per-account; read-only now. */ + private val LEGACY_KEY = stringSetPreferencesKey("buzz.starredChannels") } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/BuzzChannelMenuItems.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/BuzzChannelMenuItems.kt index 6240aca33a..e6f762607d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/BuzzChannelMenuItems.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/BuzzChannelMenuItems.kt @@ -39,16 +39,18 @@ import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.quartz.nip29RelayGroups.GroupId /** - * Pin/Unpin a Buzz channel (a device-local favorite — [BuzzChannelStars]). Moved off the per-channel - * list row into the opened channel's/forum's top-bar overflow, so the list row stays a clean - * tap-to-open target. Reads the live starred set so the label + icon reflect the current state. + * Pin/Unpin a Buzz channel (a local favorite of this account — [BuzzChannelStars]). Moved off the + * per-channel list row into the opened channel's/forum's top-bar overflow, so the list row stays a + * clean tap-to-open target. Reads the live starred set so the label + icon reflect the current state. */ @Composable fun BuzzPinDropdownItem( groupId: GroupId, + accountViewModel: AccountViewModel, closeMenu: () -> Unit, ) { - val starred by BuzzChannelStars.flow.collectAsStateWithLifecycle() + val stars = accountViewModel.account.buzzChannelStars + val starred by stars.flow.collectAsStateWithLifecycle() val isStarred = groupId.id in starred DropdownMenuItem( leadingIcon = { @@ -62,7 +64,7 @@ fun BuzzPinDropdownItem( text = { Text(stringRes(if (isStarred) R.string.buzz_unpin else R.string.buzz_pin)) }, onClick = { closeMenu() - BuzzChannelStars.toggle(groupId.id) + stars.toggle(groupId.id) }, ) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt index 6e0ae2815f..571e328c9a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt @@ -68,7 +68,6 @@ import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.model.Note -import com.vitorpamplona.amethyst.commons.model.buzz.BuzzChannelStars import com.vitorpamplona.amethyst.commons.model.buzz.BuzzCommunityMembership import com.vitorpamplona.amethyst.commons.model.buzz.BuzzRelayDialect import com.vitorpamplona.amethyst.commons.model.nip29RelayGroups.RelayGroupChannel @@ -289,7 +288,8 @@ fun RelayGroupChannelListScreen( // visibly reshuffled in the second after opening, and came back differently each visit. Ordering // by a property of the channel instead makes the first frame the final order; a channel whose // 39000 hasn't arrived sorts by its id until the name lands. - val starred by BuzzChannelStars.flow.collectAsStateWithLifecycle() + val starred by accountViewModel.account.buzzChannelStars.flow + .collectAsStateWithLifecycle() fun buzzSortKey(groupId: GroupId): String = channelsById[groupId.id]?.toBestDisplayName()?.lowercase() ?: groupId.id diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupThreadsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupThreadsScreen.kt index 33354badb9..afca51af34 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupThreadsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupThreadsScreen.kt @@ -188,7 +188,7 @@ private fun RelayGroupThreads( ) } DropdownMenu(expanded = menuOpen, onDismissRequest = { menuOpen = false }) { - BuzzPinDropdownItem(channel.groupId) { menuOpen = false } + BuzzPinDropdownItem(channel.groupId, accountViewModel) { menuOpen = false } RelayGroupMessagesDropdownItem(channel, accountViewModel) { menuOpen = false } if (isAdmin) { val archived = channel.isArchived() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupTopBar.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupTopBar.kt index 4d725b781b..0fdb50c1f4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupTopBar.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupTopBar.kt @@ -255,7 +255,7 @@ fun RelayGroupTopBar( // Pin/Unpin moved here off the community-list row. A local favorite, so it's offered // for any Buzz channel/forum regardless of membership; DMs are never pinned. if (isBuzzRelay && !isDm) { - BuzzPinDropdownItem(channel.groupId) { menuOpen = false } + BuzzPinDropdownItem(channel.groupId, accountViewModel) { menuOpen = false } } // A DM's Add/Remove-from-Messages, moved off the DM list row. It rides the per-viewer // 30622 hide snapshot (kind-41012 hide / re-open), not the kind-10009 list, and is diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzChannelStars.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzChannelStars.kt index c5237ed38c..c7241132a5 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzChannelStars.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzChannelStars.kt @@ -28,10 +28,15 @@ import kotlinx.coroutines.flow.StateFlow * NIP-29 `h`/UUID, globally unique on Buzz). Starred channels float to the top of the community view. * * There is no Nostr event for a personal star — it's the client's own bookkeeping — so, like - * [BuzzWorkspaces], this is a process-wide singleton mirrored to a device-global store by the - * platform ([com.vitorpamplona.amethyst] `BuzzChannelStarPreferences`) and restored at startup. + * [BuzzWorkspaces], it is mirrored to disk by the platform + * ([com.vitorpamplona.amethyst] `BuzzChannelStarPreferences`) and restored at startup. + * + * **One instance per account** (`Account.buzzChannelStars`). A star is by definition personal: it + * says which channels *this user* wants pinned to the top of the community view. While it was a + * process-wide singleton, one account's favorites reordered and badged every other logged-in + * account's channel list — and switching accounts silently rewrote the set they shared. */ -object BuzzChannelStars { +class BuzzChannelStars { private val starred = MutableStateFlow>(emptySet()) /** The starred channel ids; the community view collects this to pin + badge them. */ @@ -52,9 +57,4 @@ object BuzzChannelStars { fun restore(ids: Set) { starred.value = ids } - - /** Test-only: clears the set so unit tests don't leak state into each other. */ - fun clearForTesting() { - starred.value = emptySet() - } } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzChannelStarsTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzChannelStarsTest.kt new file mode 100644 index 0000000000..feae2dcb2a --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzChannelStarsTest.kt @@ -0,0 +1,69 @@ +/* + * 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.model.buzz + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class BuzzChannelStarsTest { + // A fresh instance per test: stars are per-account state now, not a process-wide singleton. + private val stars = BuzzChannelStars() + + private val a = "channel-a" + private val b = "channel-b" + + @Test + fun toggleStarsAndUnstars() { + assertFalse(stars.isStarred(a)) + assertTrue(stars.toggle(a)) + assertTrue(stars.isStarred(a)) + assertEquals(setOf(a), stars.flow.value) + + assertFalse(stars.toggle(a)) + assertFalse(stars.isStarred(a)) + assertEquals(emptySet(), stars.flow.value) + } + + @Test + fun restoreReplacesTheWholeSet() { + stars.toggle(a) + stars.restore(setOf(b)) + assertEquals(setOf(b), stars.flow.value) + assertFalse(stars.isStarred(a)) + } + + @Test + fun oneAccountsStarsDoNotPinForAnother() { + // Why this is per account: a star reorders and badges the community channel list. While the + // set was a process-wide singleton, one account pinning a channel reordered every other + // logged-in account's list, and switching accounts rewrote the set they shared. + val mine = BuzzChannelStars() + val theirs = BuzzChannelStars() + + mine.toggle(a) + + assertTrue(mine.isStarred(a)) + assertFalse(theirs.isStarred(a)) + assertEquals(emptySet(), theirs.flow.value) + } +} From 6e8dd621fea56671ad4c7ea10ff865031cb9996b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 22:45:30 +0000 Subject: [PATCH 4/6] refactor: move the held NIP-OA attestation onto Account and collapse its map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not a leak — unlike the joined workspaces and the starred channels, this store was already keyed by the agent pubkey each attestation authorizes, so no account could ever read another's credential. It was a per-account store with extra steps, and the steps were hiding a real gap. Every caller only ever touched the entry for the account doing the AUTH: AgentAttestationScreen put/removed `myPubkey`, AuthCoordinator read `authTagFor(accountPubKey)`. So `Map` was a single-entry map behind a lookup that could not miss. BuzzHeldAttestations becomes a class holding one nullable attestation for the key it is constructed with, held as Account.buzzAttestation. The two CAS loops go with it — they guarded concurrent writers to a shared map, and a single slot is last-write-wins either way. Owning the agent key lets put() do the verification its KDoc used to delegate ("The caller must have already confirmed attestation.verify(agentPubKey)"). That obligation was discharged in two places and is now discharged in one, on the only door into the store, so the paste path and the on-disk restore are gated identically. BuzzAttestationPreferences drops its own re-verify loop as a result. That gap is worth naming: the old tests stored `sig = "c".repeat(128)` and asserted it came back out as an auth tag — an assertion that the store would hold a credential no relay would accept, which is exactly what the store promises not to do. They now sign real attestations with OwnerAttestation.sign and cover both rejection paths (issued to another key, tampered conditions), including that a rejected put leaves the held one intact. Persistence is per account with the key namespaced by pubkey. The migration is exact rather than best-effort: the legacy device-global list was already agent-keyed, so this account picks out its own entry and no other can match. Verified: 2801 tests green across :commons:jvmTest and :amethyst:testFdroidDebugUnitTest; spotlessApply clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014UmCeSWetuKmHdrWcZkWDR --- .../com/vitorpamplona/amethyst/AppModules.kt | 7 +- .../vitorpamplona/amethyst/model/Account.kt | 6 ++ .../preferences/BuzzAttestationPreferences.kt | 73 ++++++++++----- .../authCommand/model/AuthCoordinator.kt | 23 +++-- .../loggedIn/buzz/AgentAttestationScreen.kt | 22 +++-- .../model/buzz/BuzzHeldAttestations.kt | 92 +++++++------------ .../model/buzz/BuzzHeldAttestationsTest.kt | 64 +++++++++---- 7 files changed, 160 insertions(+), 127 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index 18c9289c08..930b118476 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -287,10 +287,6 @@ class AppModules( } } - // Restore + persist held NIP-OA attestations across restarts (device-global). Eager (not - // lazy) so it loads before the first Buzz-relay AUTH and mirrors later changes to disk. - val buzzAttestationPrefs = BuzzAttestationPreferences(appContext, applicationIOScope) - // Restore + persist the set of relay-group channels deleted (kind-9008) on this device, so a // deleted channel stays hidden across a restart even if the host relay re-announces a stale // kind-44100 for it (device-global; a delete is authoritative and terminal for everyone). @@ -904,6 +900,9 @@ class AppModules( startBuzzPersistence = { account -> BuzzWorkspacePreferences(appContext, account.scope, account.pubKey, account.buzzWorkspaces) BuzzChannelStarPreferences(appContext, account.scope, account.pubKey, account.buzzChannelStars) + // Eager like the rest, so a held NIP-OA attestation is loaded before this account's + // first Buzz-relay AUTH rather than after it. + BuzzAttestationPreferences(appContext, account.scope, account.pubKey, account.buzzAttestation) }, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index a7ada72abc..2f461497fc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -35,6 +35,7 @@ import com.vitorpamplona.amethyst.commons.defaults.DefaultIndexerRelayList import com.vitorpamplona.amethyst.commons.marmot.MarmotManager import com.vitorpamplona.amethyst.commons.model.IAccount import com.vitorpamplona.amethyst.commons.model.buzz.BuzzChannelStars +import com.vitorpamplona.amethyst.commons.model.buzz.BuzzHeldAttestations import com.vitorpamplona.amethyst.commons.model.buzz.BuzzRelayDialect import com.vitorpamplona.amethyst.commons.model.buzz.BuzzWorkspaces import com.vitorpamplona.amethyst.commons.model.concord.ConcordChannel @@ -420,6 +421,11 @@ class Account( // channel list. Restored/persisted per account by BuzzChannelStarPreferences. val buzzChannelStars = BuzzChannelStars() + // The NIP-OA attestation an owner issued to THIS account's key, attached to its Buzz-relay + // AUTH so the relay grants virtual membership. Restored/persisted per account by + // BuzzAttestationPreferences. + val buzzAttestation = BuzzHeldAttestations(pubKey) + // Per-account NIP-42 policy evaluator (blocked → per-relay override → global policy → prompt), // reading THIS account's own toggles, relay lists and follow graph. Cached here so every AUTH // path (foreground screen + background notification consumer) shares one instance, and so an diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationPreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationPreferences.kt index 54d604318f..63e5fac2a9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationPreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationPreferences.kt @@ -37,25 +37,37 @@ import kotlinx.serialization.json.Json import kotlin.coroutines.cancellation.CancellationException /** - * Device-global persistence for the NIP-OA attestations this device holds - * ([BuzzHeldAttestations]), so a held credential survives an app restart instead of - * needing to be re-pasted. Uses the app-wide [sharedPreferencesDataStore] like - * [NamecoinSharedPreferences] (not per-account — the store is already keyed by the agent - * pubkey each attestation authorizes). + * Per-account persistence for the NIP-OA attestation this account holds + * ([BuzzHeldAttestations]), so a held credential survives an app restart instead of needing to be + * re-pasted. The key is namespaced by pubkey; the store used to be one device-global list because + * each entry carried the agent key it authorized, which made the file a per-account store with + * extra steps. * - * On construction it loads the saved entries into the singleton — **re-verifying each - * against its agent key**, so a tampered on-disk credential is dropped rather than trusted - * — then mirrors every later change back to disk. Construct once, eagerly, at startup. + * On construction it loads this account's saved attestation and mirrors every later change back to + * disk. Re-verification on restore is no longer done here: [BuzzHeldAttestations.put] verifies + * against the agent key itself and rejects what fails, so a tampered on-disk credential is dropped + * by the same gate that rejects a mistyped one. Construct once per account, eagerly. */ @Stable class BuzzAttestationPreferences( private val context: Context, private val scope: CoroutineScope, + private val pubKeyHex: HexKey, + private val attestation: BuzzHeldAttestations, ) { private val json = Json { ignoreUnknownKeys = true } + private val key = stringPreferencesKey("$KEY_PREFIX$pubKeyHex") @Serializable private data class Entry( + val owner: HexKey, + val conditions: String, + val sig: HexKey, + ) + + /** The pre-namespacing on-disk shape: one list for the whole device, each entry agent-keyed. */ + @Serializable + private data class LegacyEntry( val agent: HexKey, val owner: HexKey, val conditions: String, @@ -67,41 +79,52 @@ class BuzzAttestationPreferences( restoreFromDisk() // Persist on every change AFTER the initial restore (drop(1) skips the value // present at collection start, which restoreFromDisk already wrote). - BuzzHeldAttestations.flow.drop(1).collect { persist(it) } + attestation.flow.drop(1).collect { persist(it) } } } private suspend fun restoreFromDisk() { try { - val raw = context.sharedPreferencesDataStore.data.first()[KEY] ?: return - val verified = - json - .decodeFromString>(raw) - .mapNotNull { e -> - val attestation = OwnerAttestation(e.owner, e.conditions, e.sig) - // Only reinstate a credential that still verifies for its agent key. - if (attestation.verify(e.agent)) e.agent to attestation else null - }.toMap() - if (verified.isNotEmpty()) BuzzHeldAttestations.restore(verified) + val prefs = context.sharedPreferencesDataStore.data.first() + // put() verifies, so a credential that no longer checks out is dropped either way. + val saved = prefs[key]?.let { json.decodeFromString(it) } + if (saved != null) { + attestation.put(OwnerAttestation(saved.owner, saved.conditions, saved.sig)) + return + } + // Nothing under this account's key: pick our entry out of the pre-namespacing list. That + // list was already agent-keyed, so this migration is exact — no other account's + // credential can match, and one that fails put()'s check is simply not reinstated. + val legacy = prefs[LEGACY_KEY] ?: return + json + .decodeFromString>(legacy) + .firstOrNull { it.agent == pubKeyHex } + ?.let { attestation.put(OwnerAttestation(it.owner, it.conditions, it.sig)) } } catch (e: Exception) { if (e is CancellationException) throw e - Log.e("BuzzAttestationPrefs") { "Error reading held attestations: ${e.message}" } + Log.e("BuzzAttestationPrefs") { "Error reading held attestation: ${e.message}" } } } - private suspend fun persist(entries: Map) { + private suspend fun persist(held: OwnerAttestation?) { try { - val list = entries.map { (agent, a) -> Entry(agent, a.ownerPubKey, a.conditions, a.sig) } context.sharedPreferencesDataStore.edit { prefs -> - prefs[KEY] = json.encodeToString(list) + if (held == null) { + prefs.remove(key) + } else { + prefs[key] = json.encodeToString(Entry(held.ownerPubKey, held.conditions, held.sig)) + } } } catch (e: Exception) { if (e is CancellationException) throw e - Log.e("BuzzAttestationPrefs") { "Error writing held attestations: ${e.message}" } + Log.e("BuzzAttestationPrefs") { "Error writing held attestation: ${e.message}" } } } companion object { - private val KEY = stringPreferencesKey("buzz.heldAttestations") + private const val KEY_PREFIX = "buzz.heldAttestation." + + /** The device-global key written before the store became per-account; read-only now. */ + private val LEGACY_KEY = stringPreferencesKey("buzz.heldAttestations") } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt index cb5a0c3d37..ac9afc8d2e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/AuthCoordinator.kt @@ -21,7 +21,6 @@ package com.vitorpamplona.amethyst.service.relayClient.authCommand.model import androidx.compose.runtime.Stable -import com.vitorpamplona.amethyst.commons.model.buzz.BuzzHeldAttestations import com.vitorpamplona.amethyst.commons.model.buzz.BuzzRelayDialect import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthContext import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthDecision @@ -162,7 +161,7 @@ class AuthCoordinator( // Remember why we granted this relay so the settings screen can explain it. account.relayAuthLedger.recordGrant(context) try { - signed.add(account.signer.sign(buzzAugmented(authTemplate, account.pubKey, relayUrl))) + signed.add(account.signer.sign(buzzAugmented(authTemplate, account, relayUrl))) } catch (e: Exception) { Log.e("AuthCoordinator", "Failed trying to authenticate a writeable account", e) } @@ -204,23 +203,23 @@ class AuthCoordinator( } /** - * If [relayUrl] speaks the Buzz dialect and this device holds a NIP-OA attestation - * authorizing [accountPubKey], returns [template] with the owner-signed `auth` tag - * appended — so the relay grants virtual membership to an un-enrolled agent key while - * its owner stays a member. Otherwise returns [template] unchanged. + * If [relayUrl] speaks the Buzz dialect and [account] holds a NIP-OA attestation authorizing + * its key, returns [template] with the owner-signed `auth` tag appended — so the relay grants + * virtual membership to an un-enrolled agent key while its owner stays a member. Otherwise + * returns [template] unchanged. * - * Applied ONLY to an account's own AUTH (the caller passes the account pubkey), never - * to the Concord stream-key AUTHs that share the same [template] object, and it is a - * no-op on non-Buzz relays and for accounts with no held attestation — so it can never - * add an `auth` tag where one isn't wanted. + * Applied ONLY to an account's own AUTH (the caller passes the account being signed for), never + * to the Concord stream-key AUTHs that share the same [template] object, and it is a no-op on + * non-Buzz relays and for accounts with no held attestation — so it can never add an `auth` tag + * where one isn't wanted. */ private fun buzzAugmented( template: EventTemplate, - accountPubKey: HexKey, + account: Account, relayUrl: NormalizedRelayUrl, ): EventTemplate { if (!BuzzRelayDialect.isBuzz(relayUrl)) return template - val authTag = BuzzHeldAttestations.authTagFor(accountPubKey) ?: return template + val authTag = account.buzzAttestation.authTag() ?: return template return EventTemplate(template.createdAt, template.kind, template.tags + arrayOf(authTag), template.content) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentAttestationScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentAttestationScreen.kt index 55fe5a3a12..de47959816 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentAttestationScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentAttestationScreen.kt @@ -139,7 +139,7 @@ fun AgentAttestationScreen( // Agent side: hold an attestation an owner gave you, so this account // authenticates to the owner's Buzz relays as a virtual member. Available to // any signer — holding a credential doesn't require the raw key. - HoldAttestationSection(myPubkey = myPubkey) + HoldAttestationSection(myPubkey = myPubkey, attestation = accountViewModel.account.buzzAttestation) // Owner side: issue an attestation for an agent key. Needs the raw private key. val privKey = keyPair.privKey @@ -153,15 +153,17 @@ fun AgentAttestationScreen( } /** - * Agent-side: paste an `auth` tag an owner issued to this account's key. It is verified - * against [myPubkey] and, on success, stored in [BuzzHeldAttestations] so the auth - * coordinator attaches it when this account AUTHs to a Buzz relay. Persisted across - * restarts (device-global) by `BuzzAttestationPreferences`. + * Agent-side: paste an `auth` tag an owner issued to this account's key. [parseHeldAttestation] + * turns it into a typed failure the field can show, and [BuzzHeldAttestations.put] re-checks the + * signature before storing, so the auth coordinator attaches it when this account AUTHs to a Buzz + * relay. Persisted across restarts, per account, by `BuzzAttestationPreferences`. */ @Composable -private fun HoldAttestationSection(myPubkey: String) { - val held by BuzzHeldAttestations.flow.collectAsState() - val mine = held[myPubkey] +private fun HoldAttestationSection( + myPubkey: String, + attestation: BuzzHeldAttestations, +) { + val mine = attestation.flow.collectAsState().value var input by remember { mutableStateOf("") } var error by remember { mutableStateOf(null) } @@ -183,7 +185,7 @@ private fun HoldAttestationSection(myPubkey: String) { style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, ) - OutlinedButton(onClick = { BuzzHeldAttestations.remove(myPubkey) }) { + OutlinedButton(onClick = { attestation.clear() }) { Text(stringRes(R.string.buzz_attest_remove)) } } else { @@ -210,7 +212,7 @@ private fun HoldAttestationSection(myPubkey: String) { when (val outcome = parseHeldAttestation(input, myPubkey)) { is HoldOutcome.Failure -> error = outcome.message is HoldOutcome.Success -> { - BuzzHeldAttestations.put(myPubkey, outcome.attestation) + attestation.put(outcome.attestation) input = "" error = null } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzHeldAttestations.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzHeldAttestations.kt index 2680dede13..5ec6667c26 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzHeldAttestations.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzHeldAttestations.kt @@ -26,75 +26,51 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow /** - * Holds the NIP-OA [OwnerAttestation]s this device has *received* — one owner-signed - * authorization per agent key that lets that key publish in the owner's Buzz workspace - * without being enrolled as a relay member. + * The NIP-OA [OwnerAttestation] this account holds — an owner-signed authorization letting its key + * publish in the owner's Buzz workspace without being enrolled as a relay member. * - * The counterpart of issuance ([OwnerAttestation] is signed by an owner and handed to an - * agent operator out-of-band): when the account whose pubkey equals a stored key - * authenticates (NIP-42) to a Buzz-dialect relay, the auth coordinator attaches the - * matching [OwnerAttestation.toTag] to the AUTH event, and the relay grants virtual - * membership while the owner stays a member. + * The counterpart of issuance ([OwnerAttestation] is signed by an owner and handed to an agent + * operator out-of-band): when this account authenticates (NIP-42) to a Buzz-dialect relay, the auth + * coordinator attaches [authTag] to the AUTH event, and the relay grants virtual membership while + * the owner stays a member. * - * Keyed by the **agent** pubkey (the key the attestation authorizes). Only a - * [OwnerAttestation.verify]-passing attestation for that key should be stored, so the - * store never carries a credential the relay would reject. - * - * Like [BuzzRelayDialect] this is a process-wide singleton, and — for now — in-memory - * only: a held attestation is re-pasted after a process restart. Persisting it across - * launches (per-account, encrypted) is a follow-up. + * **One instance per account** (`Account.buzzAttestation`), holding at most one attestation — the + * one issued to [agentPubKey]. It was a process-wide `Map`, but every + * caller only ever read or wrote the entry for the account doing the AUTH, so the map was a + * single-entry map with a lookup that could not miss. Owning the agent key here also lets [put] + * enforce the verification its callers used to be told to perform, which is the property that + * matters: the store never carries a credential the relay would reject. */ -object BuzzHeldAttestations { - private val heldByAgent = MutableStateFlow>(emptyMap()) +class BuzzHeldAttestations( + private val agentPubKey: HexKey, +) { + private val held = MutableStateFlow(null) - /** All held attestations, keyed by agent pubkey; UI can collect this. */ - val flow: StateFlow> = heldByAgent - - /** The attestation held for [agentPubKey], or null. */ - fun attestationFor(agentPubKey: HexKey): OwnerAttestation? = heldByAgent.value[agentPubKey] + /** The attestation held for this account, or null. UI collects this. */ + val flow: StateFlow = held /** - * The `auth` tag to attach to [agentPubKey]'s NIP-42 AUTH event, or null when no - * verified attestation is held for that key. + * The `auth` tag to attach to this account's NIP-42 AUTH event, or null when no verified + * attestation is held. */ - fun authTagFor(agentPubKey: HexKey): Array? = attestationFor(agentPubKey)?.toTag() + fun authTag(): Array? = held.value?.toTag() /** - * Stores [attestation] as authorizing [agentPubKey]. The caller must have already - * confirmed `attestation.verify(agentPubKey)`; this is a CAS-loop put so concurrent - * writers don't clobber each other. + * Stores [attestation] as authorizing this account, if it verifies for [agentPubKey]. Returns + * false — storing nothing — when it does not. + * + * The check lives here rather than in the caller so it cannot be skipped: this is the single + * door into the store, used by the paste flow and by the on-disk restore alike, so a tampered + * saved credential is dropped by the same gate that rejects a mistyped one. */ - fun put( - agentPubKey: HexKey, - attestation: OwnerAttestation, - ) { - while (true) { - val current = heldByAgent.value - if (current[agentPubKey] == attestation) return - if (heldByAgent.compareAndSet(current, current + (agentPubKey to attestation))) return - } + fun put(attestation: OwnerAttestation): Boolean { + if (!attestation.verify(agentPubKey)) return false + held.value = attestation + return true } - /** Removes any attestation held for [agentPubKey]. */ - fun remove(agentPubKey: HexKey) { - while (true) { - val current = heldByAgent.value - if (agentPubKey !in current) return - if (heldByAgent.compareAndSet(current, current - agentPubKey)) return - } - } - - /** - * Replaces the whole store with [entries] — used to restore from disk at startup. The - * caller must have re-verified each attestation against its agent key (the same gate - * [put] documents), so a tampered on-disk credential can't be reinstated. - */ - fun restore(entries: Map) { - heldByAgent.value = entries - } - - /** Test-only: clears all held attestations so unit tests don't leak state. */ - fun clearForTesting() { - heldByAgent.value = emptyMap() + /** Drops the held attestation. */ + fun clear() { + held.value = null } } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzHeldAttestationsTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzHeldAttestationsTest.kt index 0e9376b956..fde7288201 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzHeldAttestationsTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/buzz/BuzzHeldAttestationsTest.kt @@ -21,44 +21,72 @@ package com.vitorpamplona.amethyst.commons.model.buzz import com.vitorpamplona.quartz.buzz.oaOwnerAttestation.OwnerAttestation -import kotlin.test.AfterTest +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import kotlin.test.Test import kotlin.test.assertContentEquals import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNull +import kotlin.test.assertTrue class BuzzHeldAttestationsTest { - private val agent = "a".repeat(64) - private val owner = "b".repeat(64) - private val attestation = OwnerAttestation(ownerPubKey = owner, conditions = "kind=40002", sig = "c".repeat(128)) + private val agentKey = KeyPair() + private val otherKey = KeyPair() + private val ownerKey = KeyPair() - @AfterTest - fun tearDown() = BuzzHeldAttestations.clearForTesting() + private val agent = agentKey.pubKey.toHexKey() + private val other = otherKey.pubKey.toHexKey() + + private val attestation = OwnerAttestation.sign(agent, CONDITIONS, ownerKey.privKey!!) + + // One store per account, holding the attestation issued to that account's key. + private val held = BuzzHeldAttestations(agent) @Test fun emptyStoreYieldsNoTag() { - assertNull(BuzzHeldAttestations.attestationFor(agent)) - assertNull(BuzzHeldAttestations.authTagFor(agent)) + assertNull(held.flow.value) + assertNull(held.authTag()) } @Test fun heldAttestationSurfacesAsItsAuthTag() { - BuzzHeldAttestations.put(agent, attestation) - assertEquals(attestation, BuzzHeldAttestations.attestationFor(agent)) + assertTrue(held.put(attestation)) + assertEquals(attestation, held.flow.value) // The tag attached to the agent's AUTH is exactly the attestation's ["auth", …] tag. - assertContentEquals(attestation.toTag(), BuzzHeldAttestations.authTagFor(agent)) + assertContentEquals(attestation.toTag(), held.authTag()) } @Test - fun removeClearsTheHeldAttestation() { - BuzzHeldAttestations.put(agent, attestation) - BuzzHeldAttestations.remove(agent) - assertNull(BuzzHeldAttestations.authTagFor(agent)) + fun clearDropsTheHeldAttestation() { + held.put(attestation) + held.clear() + assertNull(held.authTag()) + assertNull(held.flow.value) } @Test - fun oneAgentsAttestationDoesNotLeakToAnother() { - BuzzHeldAttestations.put(agent, attestation) - assertNull(BuzzHeldAttestations.authTagFor("d".repeat(64))) + fun anAttestationIssuedToAnotherKeyIsRejected() { + // The check that used to be the caller's job: this credential is real and verifies — for + // somebody else's key. Storing it would attach an `auth` tag the relay rejects, and the + // store's whole contract is that it never holds one. + val theirs = BuzzHeldAttestations(other) + + assertFalse(theirs.put(attestation)) + assertNull(theirs.authTag()) + } + + @Test + fun aTamperedAttestationIsRejectedAndLeavesTheHeldOneIntact() { + held.put(attestation) + + val forged = attestation.copy(conditions = "kind=1") + + assertFalse(held.put(forged)) + assertEquals(attestation, held.flow.value) + } + + companion object { + private const val CONDITIONS = "kind=40002" } } From adca6e96661c5879d24d9c74d549f21902ad6590 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 23:24:22 +0000 Subject: [PATCH 5/6] fix: don't resurrect a removed NIP-OA attestation from the legacy store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit of this branch. The per-account migration added in the previous commit made "removed" indistinguishable from "never migrated", so removing a held attestation lasted exactly until the next launch. persist(null) removed the account's key. restoreFromDisk treats an absent key as "never migrated" and falls back to the pre-namespacing device-global list — which nothing ever clears — so the credential the user just deleted was put back. It survives every restart, because every restart repeats the same seeding. The joined-workspace and starred-channel stores are not affected, but only by luck of type: they persist a Set, and an empty Set reads back present, so their cleared state suppresses the fallback on its own. Verified both halves of that against a real PreferenceDataStore before fixing — a removed key reads back null, an empty set does not. Comments now say so at both persist() sites, since the correctness is entirely implicit and a later "cleanup" to remove() would be silent. The attestation store has no empty value to lean on, so it writes an explicit tombstone. The restore decision moves into a pure internal restoreFrom(saved, legacy, agent) — the store needs a Context and cannot be unit-tested on the JVM, and this is the part with the sharp edge. Five tests cover the precedence, including the regression (verified failing against the pre-fix logic). Also from the audit: AgentAttestationScreen ignored put()'s new boolean. It is unreachable today — parseHeldAttestation verifies against the same key the store does — but a rejected paste would have cleared the field and shown success while storing nothing. It now surfaces the shared failure message. Verified: 2806 tests green across :commons:jvmTest and :amethyst:testFdroidDebugUnitTest; spotlessApply clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014UmCeSWetuKmHdrWcZkWDR --- .../preferences/BuzzAttestationPreferences.kt | 61 +++++++++----- .../preferences/BuzzChannelStarPreferences.kt | 3 + .../preferences/BuzzWorkspacePreferences.kt | 3 + .../loggedIn/buzz/AgentAttestationScreen.kt | 17 +++- .../preferences/BuzzAttestationRestoreTest.kt | 79 +++++++++++++++++++ 5 files changed, 140 insertions(+), 23 deletions(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationRestoreTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationPreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationPreferences.kt index 63e5fac2a9..618518e919 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationPreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationPreferences.kt @@ -55,7 +55,6 @@ class BuzzAttestationPreferences( private val pubKeyHex: HexKey, private val attestation: BuzzHeldAttestations, ) { - private val json = Json { ignoreUnknownKeys = true } private val key = stringPreferencesKey("$KEY_PREFIX$pubKeyHex") @Serializable @@ -87,19 +86,7 @@ class BuzzAttestationPreferences( try { val prefs = context.sharedPreferencesDataStore.data.first() // put() verifies, so a credential that no longer checks out is dropped either way. - val saved = prefs[key]?.let { json.decodeFromString(it) } - if (saved != null) { - attestation.put(OwnerAttestation(saved.owner, saved.conditions, saved.sig)) - return - } - // Nothing under this account's key: pick our entry out of the pre-namespacing list. That - // list was already agent-keyed, so this migration is exact — no other account's - // credential can match, and one that fails put()'s check is simply not reinstated. - val legacy = prefs[LEGACY_KEY] ?: return - json - .decodeFromString>(legacy) - .firstOrNull { it.agent == pubKeyHex } - ?.let { attestation.put(OwnerAttestation(it.owner, it.conditions, it.sig)) } + restoreFrom(prefs[key], prefs[LEGACY_KEY], pubKeyHex)?.let(attestation::put) } catch (e: Exception) { if (e is CancellationException) throw e Log.e("BuzzAttestationPrefs") { "Error reading held attestation: ${e.message}" } @@ -109,11 +96,10 @@ class BuzzAttestationPreferences( private suspend fun persist(held: OwnerAttestation?) { try { context.sharedPreferencesDataStore.edit { prefs -> - if (held == null) { - prefs.remove(key) - } else { - prefs[key] = json.encodeToString(Entry(held.ownerPubKey, held.conditions, held.sig)) - } + // Write [NONE] rather than removing the key: removing it is indistinguishable from + // never having migrated, which would let the legacy list re-seed a credential the + // user just deleted. See [restoreFrom]. + prefs[key] = if (held == null) NONE else json.encodeToString(Entry(held.ownerPubKey, held.conditions, held.sig)) } } catch (e: Exception) { if (e is CancellationException) throw e @@ -126,5 +112,42 @@ class BuzzAttestationPreferences( /** The device-global key written before the store became per-account; read-only now. */ private val LEGACY_KEY = stringPreferencesKey("buzz.heldAttestations") + + /** + * Tombstone for "this account has been migrated and holds nothing", which an *absent* key + * cannot express — absent still means "never migrated" and is allowed to seed from + * [LEGACY_KEY]. Without it, removing a held attestation lasted only until the next launch, + * because nothing ever clears the legacy list. (The starred-channel and joined-workspace + * stores get this for free: they persist an empty *set*, which reads back present.) + */ + private const val NONE = "" + + private val json = Json { ignoreUnknownKeys = true } + + /** + * Which attestation to reinstate, given this account's saved value and the pre-namespacing + * device-global list. Pure, so the migration precedence is testable without a `Context`. + * + * [saved] wins whenever it is present, [NONE] included. Only a never-migrated account falls + * back to [legacy], and it takes just the entry issued to its own key — that list was + * already agent-keyed, so no other account's credential can match. Nothing is verified here; + * [BuzzHeldAttestations.put] is the gate that rejects a tampered credential. + */ + internal fun restoreFrom( + saved: String?, + legacy: String?, + agentPubKey: HexKey, + ): OwnerAttestation? { + if (saved != null) { + if (saved == NONE) return null + val entry = json.decodeFromString(saved) + return OwnerAttestation(entry.owner, entry.conditions, entry.sig) + } + val list = legacy ?: return null + return json + .decodeFromString>(list) + .firstOrNull { it.agent == agentPubKey } + ?.let { OwnerAttestation(it.owner, it.conditions, it.sig) } + } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzChannelStarPreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzChannelStarPreferences.kt index e0997047b5..96c62f23eb 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzChannelStarPreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzChannelStarPreferences.kt @@ -75,6 +75,9 @@ class BuzzChannelStarPreferences( private suspend fun persist(ids: Set) { try { + // Always write the starred set, empty included — never remove the key. An absent key + // means "never migrated" and re-seeds from the legacy one above, so removing it + // would undo the user's last removal on the next launch. context.sharedPreferencesDataStore.edit { prefs -> prefs[key] = ids } } catch (e: Exception) { if (e is CancellationException) throw e diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzWorkspacePreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzWorkspacePreferences.kt index 9b15b361b1..10cc89faa1 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzWorkspacePreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/BuzzWorkspacePreferences.kt @@ -94,6 +94,9 @@ class BuzzWorkspacePreferences( private suspend fun persist(relays: Set) { try { + // Always write the joined set, empty included — never remove the key. An absent key + // means "never migrated" and re-seeds from the legacy one above, so removing it + // would undo the user's last removal on the next launch. context.sharedPreferencesDataStore.edit { prefs -> prefs[key] = relays.map { it.url }.toSet() } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentAttestationScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentAttestationScreen.kt index de47959816..f926a47f72 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentAttestationScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/buzz/AgentAttestationScreen.kt @@ -212,9 +212,15 @@ private fun HoldAttestationSection( when (val outcome = parseHeldAttestation(input, myPubkey)) { is HoldOutcome.Failure -> error = outcome.message is HoldOutcome.Success -> { - attestation.put(outcome.attestation) - input = "" - error = null + // put() re-checks the signature, so honour its answer instead of + // assuming it stored: clearing the field on a rejected paste would + // read as success and leave the account holding nothing. + if (attestation.put(outcome.attestation)) { + input = "" + error = null + } else { + error = NOT_FOR_THIS_ACCOUNT + } } } }, @@ -238,6 +244,9 @@ private sealed interface HoldOutcome { ) : HoldOutcome } +/** Shown for both rejection paths — the parse-time check and [BuzzHeldAttestations.put]'s. */ +private const val NOT_FOR_THIS_ACCOUNT = "This attestation does not authorize the current account, or its signature is invalid." + /** * Parses a pasted `["auth", owner, conditions, sig]` JSON array and verifies it * authorizes [myPubkey]. Returns a human-readable failure on malformed JSON, a @@ -261,7 +270,7 @@ private fun parseHeldAttestation( OwnerAttestation.parse(tag) ?: return HoldOutcome.Failure("Not a NIP-OA auth tag.") if (!attestation.verify(myPubkey)) { - return HoldOutcome.Failure("This attestation does not authorize the current account, or its signature is invalid.") + return HoldOutcome.Failure(NOT_FOR_THIS_ACCOUNT) } return HoldOutcome.Success(attestation) } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationRestoreTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationRestoreTest.kt new file mode 100644 index 0000000000..a14c8cdac8 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/preferences/BuzzAttestationRestoreTest.kt @@ -0,0 +1,79 @@ +/* + * 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.preferences + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * The migration precedence in [BuzzAttestationPreferences.restoreFrom]: which of the two on-disk + * shapes wins when the held attestation moved from one device-global list to a per-account key. + * + * The store itself needs a `Context`, so the decision is pulled out as a pure function — this is + * the part with the sharp edge, and it is the part the DataStore round-trip cannot express. + */ +class BuzzAttestationRestoreTest { + private val me = "a".repeat(64) + private val someoneElse = "b".repeat(64) + private val owner = "c".repeat(64) + private val sig = "d".repeat(128) + + private fun saved( + owner: String = this.owner, + conditions: String = "kind=40002", + ) = """{"owner":"$owner","conditions":"$conditions","sig":"$sig"}""" + + private fun legacyList(vararg agents: String) = agents.joinToString(",", "[", "]") { """{"agent":"$it","owner":"$owner","conditions":"kind=40002","sig":"$sig"}""" } + + @Test + fun nothingSavedAnywhereRestoresNothing() { + assertNull(BuzzAttestationPreferences.restoreFrom(null, null, me)) + } + + @Test + fun thisAccountsOwnKeyWins() { + val restored = BuzzAttestationPreferences.restoreFrom(saved(), legacyList(me), me) + assertEquals(owner, restored?.ownerPubKey) + } + + @Test + fun aRemovedAttestationIsNotResurrectedFromTheLegacyList() { + // The regression this test exists for. Removing the held credential used to delete the + // per-account key, which is indistinguishable from "never migrated" — so the next launch + // seeded it straight back out of the legacy list, which nothing ever clears. An explicit + // tombstone is the only thing that can say "migrated, and holding nothing". + assertNull(BuzzAttestationPreferences.restoreFrom("", legacyList(me), me)) + } + + @Test + fun aNeverMigratedAccountTakesItsOwnEntryFromTheLegacyList() { + val restored = BuzzAttestationPreferences.restoreFrom(null, legacyList(someoneElse, me), me) + assertEquals(owner, restored?.ownerPubKey) + } + + @Test + fun anotherAgentsLegacyEntryIsNeverPickedUp() { + // The legacy list was already agent-keyed, so the migration is exact rather than + // best-effort: there is no shared blob to accidentally inherit. + assertNull(BuzzAttestationPreferences.restoreFrom(null, legacyList(someoneElse), me)) + } +} From 41fa4538c20eb959ba58c7774b6924e47ef87eed Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Tue, 18 Aug 2026 23:37:27 -0400 Subject: [PATCH 6/6] fix(buzz): open v2 invite links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The invite server now mints `v2.` tokens, and the client could not open them at all. BuzzInviteLink.parse requires `.` and reads the community out of the payload; for a v2 token the payload segment is the literal `v2`, which decodes to one byte and fails the JSON parse, so parse returned null. Every entry point is gated on that one call, so the failure was total and silent: the deep link fell through to the external browser (where nothing can sign the claim with the user's key — the reason the in-app flow exists), a pasted link in search did nothing at all, and a link inside a note rendered as a plain url instead of an invite. `amy buzz join` refused it too. Nothing is lost by admitting the shape. The join needs the host and the code, both carried by the url itself: relayUrl() is `wss://$host`, httpBase() is `https://$host`, and the claim response returns community_id and role — which is why the screen never reads communityId. Expiry is the relay's call for a token it alone can interpret. Matched on the literal `v2` prefix rather than by relaxing the decode, so `…/invite/anything.else` still fails to parse and a Concord naddr invite (no dot) is still rejected. Tests cover the real v2 token end to end plus both guards; all three fail against the unpatched parser. Verified on device against a live workspace: the link now opens the join screen, hands off to the window.nostr browser, and the claim enrolls the key. Co-Authored-By: Claude Opus 5 (1M context) --- .../quartz/buzz/invite/BuzzInviteLink.kt | 36 +++++++++++++++--- .../quartz/buzz/invite/BuzzInviteLinkTest.kt | 37 +++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/buzz/invite/BuzzInviteLink.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/buzz/invite/BuzzInviteLink.kt index 044ca61fde..1f1b0fbc37 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/buzz/invite/BuzzInviteLink.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/buzz/invite/BuzzInviteLink.kt @@ -27,8 +27,12 @@ import kotlin.io.encoding.ExperimentalEncodingApi /** * A parsed Buzz workspace invite link: `https:///invite/`, where `` is a - * relay-signed token `.` (base64url, JWT-style but not a JWT). The - * payload names the community, the granted role, an expiry and a nonce. + * relay-signed token in one of two shapes: + * + * - `.` (base64url, JWT-style but not a JWT) — the payload names the + * community, the granted role, an expiry and a nonce. + * - `v2.` — a bare server-side handle. Nothing about the invite is readable here; + * the relay resolves it on claim. * * A Buzz invite is **not** a NIP-29 invite code (kind 9009) — it is redeemed over HTTP against * the relay's tenant host: `POST /api/invites/claim`, NIP-98-signed by the joining key, after @@ -43,11 +47,15 @@ data class BuzzInvite( val host: String, /** The full opaque token (`payload.sig`) to hand back to the relay's claim endpoint. */ val code: String, - /** The community (workspace/tenant) UUID the invite admits into — the payload's `c`. */ + /** + * The community (workspace/tenant) UUID the invite admits into — the payload's `c`. Empty for + * a `v2.` token, whose code is opaque: the relay resolves the community on claim and returns + * it, so nothing client-side needs it (the join hands off to the tenant host, not the id). + */ val communityId: String, - /** The role granted on claim (e.g. `member`) — the payload's `r`. */ + /** The role granted on claim (e.g. `member`) — the payload's `r`, or [DEFAULT_ROLE] for `v2.`. */ val role: String, - /** Unix-seconds expiry, or null when the payload omits it — the payload's `e`. */ + /** Unix-seconds expiry, or null when the payload omits it (always for `v2.`) — the payload's `e`. */ val expiresAt: Long?, ) { /** The tenant's relay websocket URL. */ @@ -63,6 +71,12 @@ data class BuzzInvite( object BuzzInviteLink { private const val MARKER = "/invite/" + /** The payload segment of an opaque, server-resolved token. */ + private const val V2_PREFIX = "v2" + + /** What the relay grants when the token doesn't say — and it never says for `v2.`. */ + private const val DEFAULT_ROLE = "member" + private val JSON = Json { ignoreUnknownKeys = true } @Serializable @@ -100,6 +114,16 @@ object BuzzInviteLink { val payloadB64 = code.substringBefore('.') if (payloadB64 == code || payloadB64.isEmpty()) return null + // `v2.` carries no client-readable payload — the community, role and expiry live + // only on the relay, which resolves the code on claim and returns them. There is nothing + // to decode and nothing to lose by admitting it: the join flow needs the host (for the + // relay url and the REST base) and the code, both of which the url itself carries, and the + // claim response supplies the rest. Matched on the literal prefix rather than by relaxing + // the decode below, so `…/invite/anything.else` still fails to parse. + if (payloadB64 == V2_PREFIX) { + return BuzzInvite(host = host, code = code, communityId = "", role = DEFAULT_ROLE, expiresAt = null) + } + val payload = try { val bytes = Base64.UrlSafe.decode(padBase64(payloadB64)) @@ -113,7 +137,7 @@ object BuzzInviteLink { host = host, code = code, communityId = community, - role = payload.r?.takeIf { it.isNotBlank() } ?: "member", + role = payload.r?.takeIf { it.isNotBlank() } ?: DEFAULT_ROLE, expiresAt = payload.e, ) } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/buzz/invite/BuzzInviteLinkTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/buzz/invite/BuzzInviteLinkTest.kt index 2bbf4beb78..4583b09b99 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/buzz/invite/BuzzInviteLinkTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/buzz/invite/BuzzInviteLinkTest.kt @@ -59,6 +59,37 @@ class BuzzInviteLinkTest { assertEquals("c03abaa9-65e4-43b1-b9b3-f502a2812d0b", BuzzInviteLink.parse("$realUrl?ref=1")!!.communityId) } + // A real v2 token minted by amethyst.communities.buzz.xyz: `v2.` plus an opaque handle. Nothing + // about the invite is encoded in it — the relay resolves the code when the claim arrives. + private val v2Token = "v2.WWsMv33mYH8o04ZGdcoZKmIImGOEMW7auc5cZ0UdH24" + private val v2Url = "https://amethyst.communities.buzz.xyz/invite/$v2Token" + + @Test + fun parsesAnOpaqueV2Invite() { + val invite = BuzzInviteLink.parse(v2Url)!! + assertEquals("amethyst.communities.buzz.xyz", invite.host) + assertEquals(v2Token, invite.code) + // Unknowable client-side: the claim response carries the community and the granted role. + assertEquals("", invite.communityId) + assertEquals("member", invite.role) + assertNull(invite.expiresAt) + // What the join actually needs, both derived from the host. + assertEquals("wss://amethyst.communities.buzz.xyz", invite.relayUrl()) + assertEquals("https://amethyst.communities.buzz.xyz", invite.httpBase()) + } + + @Test + fun aV2InviteNeverExpiresClientSide() { + // No expiry to check, so the courtesy check must not block the claim — the relay decides. + assertTrue(!BuzzInviteLink.parse(v2Url)!!.isExpired(Long.MAX_VALUE)) + } + + @Test + fun toleratesTrailingFragmentAndQueryOnV2() { + assertEquals(v2Token, BuzzInviteLink.parse("$v2Url#x")!!.code) + assertEquals(v2Token, BuzzInviteLink.parse("$v2Url?ref=1")!!.code) + } + @Test fun rejectsNonInviteAndConcordShapes() { assertNull(BuzzInviteLink.parse("https://amethyst.communities.buzz.xyz/")) @@ -67,5 +98,11 @@ class BuzzInviteLinkTest { assertNull(BuzzInviteLink.parse("https://amethyst.social/invite/naddr1abcdef#deadbeef")) // Dotless token → not a Buzz invite. assertNull(BuzzInviteLink.parse("https://host.example/invite/justsometext")) + // The v2 exemption is the literal prefix, not "give up on decoding": an undecodable + // payload with any other prefix is still not an invite. + assertNull(BuzzInviteLink.parse("https://host.example/invite/v3.WWsMv33mYH8o04ZGdcoZKmI")) + assertNull(BuzzInviteLink.parse("https://host.example/invite/notbase64json.sig")) + // `v2` without the dot is a dotless token like any other. + assertNull(BuzzInviteLink.parse("https://host.example/invite/v2")) } }