From 58ca467ff0ef5536f5c55476d17c7994e7e923a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 15:50:45 +0000 Subject: [PATCH 1/5] feat: let the relay auth prompt turn on "Always log in" for every relay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prompt's bottom row had one standing answer, "Never allow", and a link out to the settings screen. The opposite standing answer — "just log in everywhere, stop asking" — was only reachable by finding Settings ▸ Relay login, so the fast way to stop a run of prompts was to block relays one at a time. "How Amethyst decides" is replaced by "Always, all relays", which switches the asking account to RelayAuthPolicy.ALWAYS and answers the pending challenge. It is the one action here that writes an account-wide setting, so it confirms first: the label cannot carry the fact that it applies to every relay that ever asks, and a mis-tap would reveal that npub to all of them. The write lands in AuthCoordinator, not the dialog, because the policy belongs to the account the prompt named — one socket serves every logged-in account, so the screen's account is not necessarily that one. No per-relay exception is stored alongside it: the policy already answers this relay, and an exception would outlive a later switch back to "decide per relay". Blocked relays and existing "never" exceptions still outrank it, which is what the confirmation promises. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013NTYFnqWcwLVPNNSr5kusd --- .../compose/RelayAuthPromptHost.kt | 66 ++++++++---- .../authCommand/model/AuthCoordinator.kt | 11 ++ .../authCommand/model/RelayAuthPromptBus.kt | 8 ++ .../amethyst/ui/navigation/AppNavigation.kt | 6 +- amethyst/src/main/res/values/strings.xml | 4 + .../model/RelayAuthAlwaysEverywhereTest.kt | 102 ++++++++++++++++++ 6 files changed, 176 insertions(+), 21 deletions(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthAlwaysEverywhereTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt index 2b03b66010..78816fd716 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt @@ -67,6 +67,7 @@ import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.model.Channel import com.vitorpamplona.amethyst.commons.relayauth.AuthPurpose import com.vitorpamplona.amethyst.commons.relayauth.AuthPurposeKind +import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.nip11RelayInfo.loadRelayInfo import com.vitorpamplona.amethyst.service.relayClient.authCommand.model.RelayAuthPrompt @@ -74,8 +75,6 @@ import com.vitorpamplona.amethyst.service.relayClient.authCommand.model.UserAuth import com.vitorpamplona.amethyst.service.relayClient.reqCommand.channel.observeChannel import com.vitorpamplona.amethyst.service.relayClient.reqCommand.user.observeUserInfo import com.vitorpamplona.amethyst.ui.components.RobohashFallbackAsyncImage -import com.vitorpamplona.amethyst.ui.navigation.navs.INav -import com.vitorpamplona.amethyst.ui.navigation.routes.Route import com.vitorpamplona.amethyst.ui.note.ClickableUserPicture import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.stringRes @@ -95,14 +94,12 @@ private const val AVATAR_SLOT = "avatar" /** * App-wide host for NIP-42 auth prompts. Collects [RelayAuthPromptBus.prompts] and shows one * dialog at a time explaining *why* a relay wants the user to log in (who it serves), letting the - * user allow once, always allow, or block the relay. Dismissing answers [UserAuthChoice.DISMISS], + * user allow once, always allow (this relay or every relay), or block the relay. Dismissing answers + * [UserAuthChoice.DISMISS], * which the bus also falls back to on timeout, so a relay connection never blocks on the UI. */ @Composable -fun RelayAuthPromptHost( - accountViewModel: AccountViewModel, - nav: INav, -) { +fun RelayAuthPromptHost(accountViewModel: AccountViewModel) { val bus = remember { Amethyst.instance.authCoordinator.promptBus } val queue = remember { mutableStateListOf() } @@ -119,7 +116,7 @@ fun RelayAuthPromptHost( // its answer window starts from the moment it is visible rather than from the challenge. queue.firstOrNull { !it.isResolved }?.let { prompt -> LaunchedEffect(prompt) { prompt.markShown() } - RelayAuthPromptDialog(prompt, accountViewModel, nav) { choice -> + RelayAuthPromptDialog(prompt, accountViewModel) { choice -> prompt.respond(choice) queue.remove(prompt) } @@ -136,18 +133,31 @@ fun RelayAuthPromptHost( * separate blocks: a purpose-specific title, a purpose label, an avatar row, and a red "if you don't" * consequence line, each of which repeated the same name. * - * Two buttons and a switch replace four stacked buttons. Nothing here writes a global setting: the - * old "always deliver my messages" silently flipped the policy to CUSTOM plus two account-wide - * toggles, so it is now a link to the screen where those toggles are visible. + * Two buttons and a switch replace four stacked buttons. The two links under them are the standing + * answers, one per direction: "Never allow" writes a DENY for this relay, "Always, all relays" + * switches the account to [RelayAuthPolicy.ALWAYS] so nothing asks again. Only the second one is + * account-wide, and it confirms before it writes — see [AlwaysAllowEverywhereConfirmation]. The old + * "always deliver my messages" is still gone: it flipped the policy to CUSTOM plus two account-wide + * toggles *silently*, which is the part that was wrong, not the writing itself. */ @Composable private fun RelayAuthPromptDialog( prompt: RelayAuthPrompt, accountViewModel: AccountViewModel, - nav: INav, onChoice: (UserAuthChoice) -> Unit, ) { var rememberRelay by remember(prompt) { mutableStateOf(false) } + var confirmEverywhere by remember(prompt) { mutableStateOf(false) } + val accountName = rememberDisplayName(prompt.askingAccount, accountViewModel) + + if (confirmEverywhere) { + AlwaysAllowEverywhereConfirmation( + accountName = accountName, + onDismiss = { confirmEverywhere = false }, + onConfirm = { onChoice(UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE) }, + ) + return + } // The purpose the user is most likely to recognize as "what I was just doing". val primary = remember(prompt) { prompt.purposes.primary() } @@ -189,7 +199,7 @@ private fun RelayAuthPromptDialog( Column(verticalArrangement = Arrangement.spacedBy(14.dp)) { RelayHeader(prompt, accountViewModel) Text( - text = stringRes(R.string.relay_auth_login_as, rememberDisplayName(prompt.askingAccount, accountViewModel)), + text = stringRes(R.string.relay_auth_login_as, accountName), style = MaterialTheme.typography.headlineSmall, ) } @@ -242,17 +252,37 @@ private fun RelayAuthPromptDialog( colors = ButtonDefaults.textButtonColors(contentColor = MaterialTheme.colorScheme.error), ) { Text(stringRes(R.string.relay_auth_never_allow), style = MaterialTheme.typography.labelMedium) } TextButton( - onClick = { - onChoice(UserAuthChoice.DISMISS) - nav.nav(Route.RelayAuthSettings) - }, - ) { Text(stringRes(R.string.relay_auth_how_we_decide), style = MaterialTheme.typography.labelMedium) } + onClick = { confirmEverywhere = true }, + ) { Text(stringRes(R.string.relay_auth_always_allow_everywhere), style = MaterialTheme.typography.labelMedium) } } } }, ) } +/** + * The one action in this flow that writes an account-wide setting, so it asks first. + * + * "Never allow" next to it is per-relay and reversible from the settings screen's exception list; + * this one changes what happens on every relay that ever asks, and a mis-tap would reveal the npub + * named above to all of them. The confirmation is what makes that scope visible before it is + * chosen — the label alone can't carry it. + */ +@Composable +private fun AlwaysAllowEverywhereConfirmation( + accountName: String, + onDismiss: () -> Unit, + onConfirm: () -> Unit, +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text(stringRes(R.string.relay_auth_always_everywhere_title)) }, + text = { Text(stringRes(R.string.relay_auth_always_everywhere_body, accountName)) }, + confirmButton = { Button(onClick = onConfirm) { Text(stringRes(R.string.relay_auth_policy_always)) } }, + dismissButton = { TextButton(onClick = onDismiss) { Text(stringRes(R.string.cancel)) } }, + ) +} + /** The relay leads the dialog, because the relay is the thing being trusted. */ @Composable private fun RelayHeader( 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 cd005c4e41..a4462eeab0 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 @@ -24,6 +24,7 @@ import androidx.compose.runtime.Stable import com.vitorpamplona.amethyst.commons.model.buzz.BuzzRelayDialect import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthContext import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthDecision +import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthVerdict import com.vitorpamplona.amethyst.isDebug import com.vitorpamplona.amethyst.model.Account @@ -155,6 +156,16 @@ class AuthCoordinator( account.relayAuthLedger.setDecision(relayUrl.url, RelayAuthDecision.ALLOW) true } + UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE -> { + // Written here rather than in the dialog because the policy belongs to + // the account the prompt named — one socket serves every logged-in + // account, so the screen's account is not necessarily this one. + // No per-relay decision is stored: the policy already answers this + // relay, and an exception on top of it would survive a later switch + // back to "decide per relay". + account.changeDefaultRelayAuthPolicy(RelayAuthPolicy.ALWAYS) + true + } UserAuthChoice.BLOCK -> { account.relayAuthLedger.setDecision(relayUrl.url, RelayAuthDecision.DENY) false diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt index a7c000ad88..e9398e72b0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt @@ -40,6 +40,14 @@ enum class UserAuthChoice { /** Authenticate now and remember ALLOW for this relay. */ ALWAYS_ALLOW, + /** + * Authenticate now and switch the asking account's top-level policy to + * [com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy.ALWAYS], so every relay that + * asks is answered without a prompt. The account-wide counterpart of [ALWAYS_ALLOW], and the + * one choice here that writes a global setting — the dialog confirms it before sending it. + */ + ALWAYS_ALLOW_EVERYWHERE, + /** Do not authenticate and remember DENY for this relay. */ BLOCK, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt index 3c15bb2fdb..085e7a8fdd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/AppNavigation.kt @@ -335,9 +335,9 @@ fun AppNavigation( val nav = rememberNav() // Shows the "log in to this relay?" dialog when a NIP-42 challenge needs the user to decide. - // Hosted here rather than in LoggedInPage because the dialog links out to the relay-login - // settings screen, which needs the nav created just above. - RelayAuthPromptHost(accountViewModel, nav) + // Hosted here rather than in LoggedInPage so one dialog serves the whole shell: challenges + // arrive off the shared relay socket, not from whatever screen happens to be on top. + RelayAuthPromptHost(accountViewModel) // One layout decision per window size for the whole shell: bottom bar vs rail vs // permanent drawer, plus the docked notification panel. Every screen, bar and panel diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 5afa83b120..efddaddbd7 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -1186,6 +1186,10 @@ Log in Not now Never allow + Always, all relays + Log in to every relay? + Amethyst will log in as %1$s to every relay that asks, and stop asking. Relays you blocked, and the ones you set to never log in, stay that way. You can change this under Relay login. + How Amethyst decides diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthAlwaysEverywhereTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthAlwaysEverywhereTest.kt new file mode 100644 index 0000000000..c555ed1558 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthAlwaysEverywhereTest.kt @@ -0,0 +1,102 @@ +/* + * 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.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.Assert.assertNull +import org.junit.Test + +/** + * The prompt's "Always, all relays" link ([UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE]) does exactly one + * thing: flip the account's policy to [RelayAuthPolicy.ALWAYS]. These tests pin what that flip has to + * buy — the prompt never comes back for *any* relay — and what it must not quietly do: write a + * per-relay exception that would outlive a later switch back to "decide per relay", or override the + * two rules that rank above the policy. + */ +class RelayAuthAlwaysEverywhereTest { + private val relay = "wss://auth.example.com/" + private val other = "wss://elsewhere.example.com/" + private val blockedRelay = "wss://blocked.example.com/" + + private var policy = RelayAuthPolicy.CUSTOM + private val store: RelayAuthPermissionStore = InMemoryRelayAuthPermissionStore() + + private val ledger = + RelayAuthPermissionLedger( + store = store, + globalPolicy = { policy }, + sessionGrants = RelayAuthSessionGrants(), + isBlocked = { it == blockedRelay }, + ) + + /** A challenge we can explain but have no automatic rule for: the ASK case the prompt is shown for. */ + private fun askable(relayUrl: String) = RelayAuthContext(relayUrl, listOf(AuthPurpose(AuthPurposeKind.MY_INBOX))) + + @Test + fun theFlipAnswersThisRelayAndEveryOtherOne() = + runTest { + assertEquals(RelayAuthVerdict.ASK, ledger.decide(askable(relay))) + assertEquals(RelayAuthVerdict.ASK, ledger.decide(askable(other))) + + policy = RelayAuthPolicy.ALWAYS + + assertEquals(RelayAuthVerdict.ALLOW, ledger.decide(askable(relay))) + assertEquals(RelayAuthVerdict.ALLOW, ledger.decide(askable(other))) + } + + /** + * The relay that happened to be asking gets no exception of its own. An ALLOW written here would + * survive a later switch back to CUSTOM/NEVER and keep authenticating a relay the user thought + * they had stopped — the policy is the whole answer, so it is the only thing that changes. + */ + @Test + fun theFlipWritesNoPerRelayException() = + runTest { + policy = RelayAuthPolicy.ALWAYS + assertEquals(RelayAuthVerdict.ALLOW, ledger.decide(askable(relay))) + + assertNull(store.loadDecision(relay)) + assertEquals(emptyMap(), store.allDecisions()) + + policy = RelayAuthPolicy.CUSTOM + assertEquals(RelayAuthVerdict.ASK, ledger.decide(askable(relay))) + } + + /** Both rules that outrank the policy keep outranking it, which is what the confirmation promises. */ + @Test + fun blockedRelaysAndNeverExceptionsStillWin() = + runTest { + ledger.setDecision(other, RelayAuthDecision.DENY) + policy = RelayAuthPolicy.ALWAYS + + assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(blockedRelay))) + assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(other))) + assertEquals(RelayAuthVerdict.ALLOW, ledger.decide(askable(relay))) + } +} From 8d2c50ae5c183362476dd2c2c7cdd082c938c6bd Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 16:23:55 +0000 Subject: [PATCH 2/5] fix: make the prompt's remember switch mean the same thing for both answers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Remember for this relay" was read only by the Log in button. Pressing "Not now" with the switch on wrote nothing at all — no exception, not even a session-scoped no — so the same dialog came back on the relay's next reconnect, while the switch sat there claiming otherwise. The one way to say "stop asking about this relay" was the red "Never allow" button beside it. The switch is now the scope of whichever answer is given, so the two buttons times the switch are the four per-relay UserAuthChoice values: log in once or always, refuse once or for good. That makes "Never allow" exactly "Not now" with the switch on, written twice, so it goes. Its slot becomes the missing half of the account-wide pair: "Never, all relays" sets RelayAuthPolicy.NEVER opposite "Always, all relays". Both confirm first, sharing one confirmation that names the consequence of each direction — the never side warns that relays will refuse to serve, which is the part a link label cannot carry. It routes through Account.changeDefaultRelayAuthPolicy, which drops this run's session grants along with the flip; a grant left behind outranks the policy, so "never log in" would have gone on authenticating the relays just answered "log in". Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013NTYFnqWcwLVPNNSr5kusd --- .../compose/RelayAuthPromptHost.kt | 88 +++++++++++++------ .../authCommand/model/AuthCoordinator.kt | 8 ++ .../authCommand/model/RelayAuthPromptBus.kt | 13 ++- amethyst/src/main/res/values/strings.xml | 5 +- ...st.kt => RelayAuthPolicyEverywhereTest.kt} | 61 +++++++++++-- 5 files changed, 140 insertions(+), 35 deletions(-) rename amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/{RelayAuthAlwaysEverywhereTest.kt => RelayAuthPolicyEverywhereTest.kt} (64%) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt index 78816fd716..5fe7a81a8f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt @@ -94,8 +94,8 @@ private const val AVATAR_SLOT = "avatar" /** * App-wide host for NIP-42 auth prompts. Collects [RelayAuthPromptBus.prompts] and shows one * dialog at a time explaining *why* a relay wants the user to log in (who it serves), letting the - * user allow once, always allow (this relay or every relay), or block the relay. Dismissing answers - * [UserAuthChoice.DISMISS], + * user answer for this relay (once or for good, either way) or for every relay at once. Dismissing + * answers [UserAuthChoice.DISMISS], * which the bus also falls back to on timeout, so a relay connection never blocks on the UI. */ @Composable @@ -133,12 +133,18 @@ fun RelayAuthPromptHost(accountViewModel: AccountViewModel) { * separate blocks: a purpose-specific title, a purpose label, an avatar row, and a red "if you don't" * consequence line, each of which repeated the same name. * - * Two buttons and a switch replace four stacked buttons. The two links under them are the standing - * answers, one per direction: "Never allow" writes a DENY for this relay, "Always, all relays" - * switches the account to [RelayAuthPolicy.ALWAYS] so nothing asks again. Only the second one is - * account-wide, and it confirms before it writes — see [AlwaysAllowEverywhereConfirmation]. The old - * "always deliver my messages" is still gone: it flipped the policy to CUSTOM plus two account-wide - * toggles *silently*, which is the part that was wrong, not the writing itself. + * Two buttons and a switch replace four stacked buttons, and the switch means what it says for + * *both* of them: it is the answer's scope, not a modifier on "Log in". The four combinations are + * the four [UserAuthChoice] values for this relay — log in once or always, refuse once or for good — + * which is why there is no separate "Never allow" button any more: it was "Not now" with the switch + * on, written twice. + * + * That frees the row underneath for the two answers this relay's buttons cannot give, one per + * direction: "Always, all relays" and "Never, all relays" set the account's [RelayAuthPolicy] so + * nothing is asked again, either way. Both are account-wide, so both confirm before they write — + * see [PolicyEverywhereConfirmation]. The old "always deliver my messages" is still gone: it + * flipped the policy to CUSTOM plus two account-wide toggles *silently*, which is the part that was + * wrong, not the writing itself. */ @Composable private fun RelayAuthPromptDialog( @@ -147,14 +153,16 @@ private fun RelayAuthPromptDialog( onChoice: (UserAuthChoice) -> Unit, ) { var rememberRelay by remember(prompt) { mutableStateOf(false) } - var confirmEverywhere by remember(prompt) { mutableStateOf(false) } + // The account-wide answer waiting on its confirmation, or null while the prompt itself is up. + var confirming by remember(prompt) { mutableStateOf(null) } val accountName = rememberDisplayName(prompt.askingAccount, accountViewModel) - if (confirmEverywhere) { - AlwaysAllowEverywhereConfirmation( + confirming?.let { choice -> + PolicyEverywhereConfirmation( + choice = choice, accountName = accountName, - onDismiss = { confirmEverywhere = false }, - onConfirm = { onChoice(UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE) }, + onDismiss = { confirming = null }, + onConfirm = { onChoice(choice) }, ) return } @@ -234,8 +242,11 @@ private fun RelayAuthPromptDialog( verticalArrangement = Arrangement.spacedBy(4.dp), ) { Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + // Both buttons read the switch, which is what makes it the scope of the answer + // rather than a modifier on one of them. Refusing *and* remembering is the DENY + // the red "Never allow" button used to write on its own. OutlinedButton( - onClick = { onChoice(UserAuthChoice.DISMISS) }, + onClick = { onChoice(if (rememberRelay) UserAuthChoice.BLOCK else UserAuthChoice.DISMISS) }, modifier = Modifier.weight(1f), ) { Text(stringRes(R.string.relay_auth_not_now)) } Button( @@ -248,11 +259,11 @@ private fun RelayAuthPromptDialog( horizontalArrangement = Arrangement.SpaceBetween, ) { TextButton( - onClick = { onChoice(UserAuthChoice.BLOCK) }, + onClick = { confirming = UserAuthChoice.NEVER_ALLOW_EVERYWHERE }, colors = ButtonDefaults.textButtonColors(contentColor = MaterialTheme.colorScheme.error), - ) { Text(stringRes(R.string.relay_auth_never_allow), style = MaterialTheme.typography.labelMedium) } + ) { Text(stringRes(R.string.relay_auth_never_allow_everywhere), style = MaterialTheme.typography.labelMedium) } TextButton( - onClick = { confirmEverywhere = true }, + onClick = { confirming = UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE }, ) { Text(stringRes(R.string.relay_auth_always_allow_everywhere), style = MaterialTheme.typography.labelMedium) } } } @@ -261,24 +272,49 @@ private fun RelayAuthPromptDialog( } /** - * The one action in this flow that writes an account-wide setting, so it asks first. + * The two actions in this flow that write an account-wide setting, so they ask first. * - * "Never allow" next to it is per-relay and reversible from the settings screen's exception list; - * this one changes what happens on every relay that ever asks, and a mis-tap would reveal the npub - * named above to all of them. The confirmation is what makes that scope visible before it is - * chosen — the label alone can't carry it. + * The buttons above are about the one relay in the title, and both of their outcomes are listed and + * reversible on the settings screen. These two are not about this relay at all: they decide every + * relay that ever asks — revealing the npub named above to all of them, or cutting it off from all of + * them — and a link label cannot carry that. The confirmation is where the scope becomes visible, + * and it names the consequence each direction actually has. */ @Composable -private fun AlwaysAllowEverywhereConfirmation( +private fun PolicyEverywhereConfirmation( + choice: UserAuthChoice, accountName: String, onDismiss: () -> Unit, onConfirm: () -> Unit, ) { + val always = choice == UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE AlertDialog( onDismissRequest = onDismiss, - title = { Text(stringRes(R.string.relay_auth_always_everywhere_title)) }, - text = { Text(stringRes(R.string.relay_auth_always_everywhere_body, accountName)) }, - confirmButton = { Button(onClick = onConfirm) { Text(stringRes(R.string.relay_auth_policy_always)) } }, + title = { + Text(stringRes(if (always) R.string.relay_auth_always_everywhere_title else R.string.relay_auth_never_everywhere_title)) + }, + text = { + Text( + stringRes( + if (always) R.string.relay_auth_always_everywhere_body else R.string.relay_auth_never_everywhere_body, + accountName, + ), + ) + }, + confirmButton = { + Button( + onClick = onConfirm, + colors = + if (always) { + ButtonDefaults.buttonColors() + } else { + ButtonDefaults.buttonColors( + containerColor = MaterialTheme.colorScheme.error, + contentColor = MaterialTheme.colorScheme.onError, + ) + }, + ) { Text(stringRes(if (always) R.string.relay_auth_policy_always else R.string.relay_auth_policy_never)) } + }, dismissButton = { TextButton(onClick = onDismiss) { Text(stringRes(R.string.cancel)) } }, ) } 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 a4462eeab0..3887b40b77 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 @@ -170,6 +170,14 @@ class AuthCoordinator( account.relayAuthLedger.setDecision(relayUrl.url, RelayAuthDecision.DENY) false } + UserAuthChoice.NEVER_ALLOW_EVERYWHERE -> { + // Same reasoning as ALWAYS_ALLOW_EVERYWHERE above, and through the + // account rather than its settings for one more reason: this is the + // policy that session grants outrank, so the grants have to go with + // it. Account.changeDefaultRelayAuthPolicy is what pairs them. + account.changeDefaultRelayAuthPolicy(RelayAuthPolicy.NEVER) + false + } UserAuthChoice.DISMISS -> false } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt index e9398e72b0..d68e749ccd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt @@ -43,14 +43,23 @@ enum class UserAuthChoice { /** * Authenticate now and switch the asking account's top-level policy to * [com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy.ALWAYS], so every relay that - * asks is answered without a prompt. The account-wide counterpart of [ALWAYS_ALLOW], and the - * one choice here that writes a global setting — the dialog confirms it before sending it. + * asks is answered without a prompt. The account-wide counterpart of [ALWAYS_ALLOW]; the dialog + * confirms it before sending it, because it is a global setting. */ ALWAYS_ALLOW_EVERYWHERE, /** Do not authenticate and remember DENY for this relay. */ BLOCK, + /** + * Do not authenticate, and switch the asking account's top-level policy to + * [com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy.NEVER], so no relay is ever + * answered again. The account-wide counterpart of [BLOCK], confirmed the same way — and, like + * every route through [com.vitorpamplona.amethyst.model.Account.changeDefaultRelayAuthPolicy], + * it drops this run's session grants, which would otherwise outrank the policy it just set. + */ + NEVER_ALLOW_EVERYWHERE, + /** No decision (dismissed or timed out) — do not authenticate, don't remember. */ DISMISS, } diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index efddaddbd7..cd8c1ea285 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -1185,11 +1185,14 @@ Remember for this relay Log in Not now - Never allow Always, all relays + Never, all relays Log in to every relay? Amethyst will log in as %1$s to every relay that asks, and stop asking. Relays you blocked, and the ones you set to never log in, stay that way. You can change this under Relay login. + Never log in to any relay? + Amethyst will stop telling relays that you are %1$s, and stop asking. Some relays will refuse to serve you: messages, replies and notifications may not go through. Relays you set to always log in stay that way. You can change this under Relay login. + Never allow How Amethyst decides diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthAlwaysEverywhereTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPolicyEverywhereTest.kt similarity index 64% rename from amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthAlwaysEverywhereTest.kt rename to amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPolicyEverywhereTest.kt index c555ed1558..82c463b3d0 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthAlwaysEverywhereTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPolicyEverywhereTest.kt @@ -33,13 +33,14 @@ import org.junit.Assert.assertNull import org.junit.Test /** - * The prompt's "Always, all relays" link ([UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE]) does exactly one - * thing: flip the account's policy to [RelayAuthPolicy.ALWAYS]. These tests pin what that flip has to - * buy — the prompt never comes back for *any* relay — and what it must not quietly do: write a - * per-relay exception that would outlive a later switch back to "decide per relay", or override the - * two rules that rank above the policy. + * The prompt's two account-wide links — "Always, all relays" + * ([UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE]) and "Never, all relays" + * ([UserAuthChoice.NEVER_ALLOW_EVERYWHERE]) — each do exactly one thing: flip the account's policy. + * These tests pin what a flip has to buy — the prompt never comes back for *any* relay, in either + * direction — and what it must not quietly do: write a per-relay exception that would outlive a + * later switch back to "decide per relay", or override the rules that rank above the policy. */ -class RelayAuthAlwaysEverywhereTest { +class RelayAuthPolicyEverywhereTest { private val relay = "wss://auth.example.com/" private val other = "wss://elsewhere.example.com/" private val blockedRelay = "wss://blocked.example.com/" @@ -99,4 +100,52 @@ class RelayAuthAlwaysEverywhereTest { assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(other))) assertEquals(RelayAuthVerdict.ALLOW, ledger.decide(askable(relay))) } + + @Test + fun theNeverFlipSilencesThisRelayAndEveryOtherOne() = + runTest { + assertEquals(RelayAuthVerdict.ASK, ledger.decide(askable(relay))) + + policy = RelayAuthPolicy.NEVER + + assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(relay))) + assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(other))) + assertNull(store.loadDecision(relay)) + } + + /** The "Always, set by you" exceptions the settings screen lists are standing answers, not casual ones. */ + @Test + fun theNeverFlipLeavesAlwaysExceptionsAlone() = + runTest { + ledger.setDecision(other, RelayAuthDecision.ALLOW) + policy = RelayAuthPolicy.NEVER + + assertEquals(RelayAuthVerdict.ALLOW, ledger.decide(askable(other))) + assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(relay))) + } + + /** + * Why the coordinator routes this through [com.vitorpamplona.amethyst.model.Account], which drops + * the session grants with the flip: a grant left behind outranks the policy, so "never log in" + * would keep authenticating exactly the relays the user had just answered "log in" for. + */ + @Test + fun aSessionGrantWouldOutrankTheNeverFlipIfItSurvived() = + runTest { + val grants = RelayAuthSessionGrants() + val ledger = + RelayAuthPermissionLedger( + store = InMemoryRelayAuthPermissionStore(), + globalPolicy = { policy }, + sessionGrants = grants, + ) + grants.grant(relay) + policy = RelayAuthPolicy.NEVER + + assertEquals(RelayAuthVerdict.ALLOW, ledger.decide(askable(relay))) + + grants.clear() + + assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(relay))) + } } From e61d30dfb7dc372faa1f418d9c61753a3d133982 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 16:35:21 +0000 Subject: [PATCH 3/5] fix: don't let the answer window swallow an account-wide relay auth answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit of the two commits before this one turned up two ways the new "Always/Never, all relays" answers could be given and not take effect. A prompt's answer window is 60s from the dialog appearing, and the confirmation dialog spends it: a user who reads the warning, thinks, and confirms past the minute hits a resolved deferred, where complete() is a no-op. The AUTH was already lost at that point — fine, the socket cannot wait — but the *setting* was lost with it, silently, which is not. The policy write moves to AuthCoordinator.applyPolicyEverywhere, called by the prompt the moment the user confirms; the answer path calls the same function, so there is still one writer and it is idempotent. A confirmation that lands late now still sets the policy, and the relay's next challenge is answered by it. The other one: prompts queued behind the dialog were decided before the policy existed, so "all relays" was immediately followed by a question about relay B. They are now answered with the same choice. That needs markShown() as well as respond() — an unshown prompt is parked in the five-minute queue-wait window and does not read an answer dropped into its deferred until that window ends, which would have left a relay unauthenticated for five minutes after the user answered for it. RelayAuthPromptBusTest pins the timing; it fails at 300000ms without the markShown. Also retires the comments in the ledger, Account and the resolver that still explained a DENY as the "never allow" button, which no longer exists. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013NTYFnqWcwLVPNNSr5kusd --- .../vitorpamplona/amethyst/model/Account.kt | 2 +- .../compose/RelayAuthPromptHost.kt | 25 +++++++++++- .../authCommand/model/AuthCoordinator.kt | 40 ++++++++++++++----- .../model/RelayAuthPermissionLedger.kt | 5 ++- .../authCommand/model/RelayAuthPromptBus.kt | 19 ++++++++- .../model/RelayAuthPolicyEverywhereTest.kt | 20 ++++++++++ .../model/RelayAuthPromptBusTest.kt | 34 ++++++++++++++++ .../commons/relayauth/RelayAuthResolver.kt | 2 +- 8 files changed, 130 insertions(+), 17 deletions(-) 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 9264598659..3dfb0d82e7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -3662,7 +3662,7 @@ class Account( // Blocking a relay has to forget any "just for now" login to it, or unblocking later would // silently resume authenticating off an answer given before the block. Blocking is the - // strongest signal available here — the weaker "never allow" already drops the grant via + // strongest signal available here — the weaker per-relay "never" answer already drops the grant via // RelayAuthPermissionLedger.setDecision, so it would be odd for the stronger one not to. // // Observed rather than hooked onto the local block action because the kind-10006 list is diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt index 5fe7a81a8f..6b5b961627 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt @@ -100,7 +100,8 @@ private const val AVATAR_SLOT = "avatar" */ @Composable fun RelayAuthPromptHost(accountViewModel: AccountViewModel) { - val bus = remember { Amethyst.instance.authCoordinator.promptBus } + val coordinator = remember { Amethyst.instance.authCoordinator } + val bus = remember { coordinator.promptBus } val queue = remember { mutableStateListOf() } LaunchedEffect(bus) { @@ -117,6 +118,28 @@ fun RelayAuthPromptHost(accountViewModel: AccountViewModel) { queue.firstOrNull { !it.isResolved }?.let { prompt -> LaunchedEffect(prompt) { prompt.markShown() } RelayAuthPromptDialog(prompt, accountViewModel) { choice -> + choice.policyEverywhere?.let { policy -> + // Applied here, not left to the answer below, so the setting survives an expired + // prompt — the answer window runs while the user reads the confirmation. See + // AuthCoordinator.applyPolicyEverywhere. + coordinator.applyPolicyEverywhere(prompt.askingAccount, policy) + + // "all relays" has to mean the ones already queued behind this dialog too. They were + // decided before the policy existed, so nothing else resolves them, and asking again + // about relay B right after being told "always/never, all relays" reads as the answer + // not having taken. Same account only: the policy is that account's. + // + // markShown() first even though these are never shown: a prompt still waiting its + // turn is parked in the bus's queue-wait window, and an answer dropped into it does + // not land until that window ends — five minutes of an unauthenticated relay the + // user already answered for. Marking it shown opens its answer window immediately. + queue.toList().forEach { + if (it.askingAccount == prompt.askingAccount) { + it.markShown() + it.respond(choice) + } + } + } prompt.respond(choice) queue.remove(prompt) } 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 3887b40b77..e90669ae04 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 @@ -157,13 +157,11 @@ class AuthCoordinator( true } UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE -> { - // Written here rather than in the dialog because the policy belongs to - // the account the prompt named — one socket serves every logged-in - // account, so the screen's account is not necessarily this one. // No per-relay decision is stored: the policy already answers this // relay, and an exception on top of it would survive a later switch - // back to "decide per relay". - account.changeDefaultRelayAuthPolicy(RelayAuthPolicy.ALWAYS) + // back to "decide per relay". The UI has normally applied this + // already (see [applyPolicyEverywhere]); repeating it is free. + applyPolicyEverywhere(account.pubKey, RelayAuthPolicy.ALWAYS) true } UserAuthChoice.BLOCK -> { @@ -171,11 +169,7 @@ class AuthCoordinator( false } UserAuthChoice.NEVER_ALLOW_EVERYWHERE -> { - // Same reasoning as ALWAYS_ALLOW_EVERYWHERE above, and through the - // account rather than its settings for one more reason: this is the - // policy that session grants outrank, so the grants have to go with - // it. Account.changeDefaultRelayAuthPolicy is what pairs them. - account.changeDefaultRelayAuthPolicy(RelayAuthPolicy.NEVER) + applyPolicyEverywhere(account.pubKey, RelayAuthPolicy.NEVER) false } UserAuthChoice.DISMISS -> false @@ -198,6 +192,32 @@ class AuthCoordinator( }, ) + /** + * Sets [askingAccount]'s top-level NIP-42 policy — the "Always, all relays" / "Never, all relays" + * answers. Public because the *prompt* calls it the moment the user confirms, instead of relying + * on the answer reaching the suspended challenge above: that answer window is 60s from the dialog + * appearing (see [RelayAuthPromptBus]), and a user who spends it reading the confirmation would + * otherwise have their setting silently dropped along with the expired prompt. The relay's own + * AUTH is the only thing worth losing to a timeout; a setting is not, and the next challenge — + * seconds later, on reconnect — is answered by the policy this wrote. + * + * Goes through [Account.changeDefaultRelayAuthPolicy] rather than the settings object because + * that is what pairs [RelayAuthPolicy.NEVER] with dropping this run's session grants, which + * outrank the policy and would otherwise keep authenticating the relays just answered "log in". + * + * Takes a pubkey rather than an [Account] because the prompt names the account whose npub is at + * stake, which on a multi-account device is not the one the screen is showing. Unknown pubkeys + * (an account logged out while its prompt was up) are a no-op. + */ + fun applyPolicyEverywhere( + askingAccount: HexKey, + policy: RelayAuthPolicy, + ) { + authWithAccounts.distinctValues().forEach { screen -> + if (screen.account.pubKey == askingAccount) screen.account.changeDefaultRelayAuthPolicy(policy) + } + } + /** * The joined Concord community whose plane [planeAddress] is, across every watched account, or * null when the pubkey isn't a plane of ours. Feeds [RelayAuthPurposeDeriver] so a pending plane diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPermissionLedger.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPermissionLedger.kt index ec6df47d4e..4ab03c9592 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPermissionLedger.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPermissionLedger.kt @@ -182,7 +182,7 @@ class RelayAuthPermissionLedger( * * Blocking already denies while it is in force, so this is about what happens *after* it is * lifted: without it, unblocking would resume authenticating off an answer given before the - * block. The weaker "never allow" drops the grant too (see [setDecision]), so the stronger + * block. The weaker "not now, and remember it" drops the grant too (see [setDecision]), so the stronger * signal has to as well. */ fun revokeSessionGrantsFor(blockedRelayUrls: Collection) = blockedRelayUrls.forEach(sessionGrants::revoke) @@ -198,7 +198,8 @@ class RelayAuthPermissionLedger( * *after* its disk write returns — so a challenge arriving between them must never see neither. * Which side to fail on depends on the decision: * - **DENY** revokes first. The window then asks or denies, never signs: a user who just pressed - * "never allow" must not get one more AUTH out of the grant they are replacing. + * "not now" with the remember switch on must not get one more AUTH out of the grant they are + * replacing. * - **ALLOW** revokes last, so the grant still covers the window. Revoking first left the relay * momentarily undecided, which re-prompted the user who had just pressed "always" — the very * dialog this whole feature exists to stop. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt index d68e749ccd..1f314cb380 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBus.kt @@ -21,6 +21,7 @@ package com.vitorpamplona.amethyst.service.relayClient.authCommand.model import com.vitorpamplona.amethyst.commons.relayauth.AuthPurpose +import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import kotlinx.coroutines.CompletableDeferred @@ -42,7 +43,7 @@ enum class UserAuthChoice { /** * Authenticate now and switch the asking account's top-level policy to - * [com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy.ALWAYS], so every relay that + * [RelayAuthPolicy.ALWAYS], so every relay that * asks is answered without a prompt. The account-wide counterpart of [ALWAYS_ALLOW]; the dialog * confirms it before sending it, because it is a global setting. */ @@ -53,7 +54,7 @@ enum class UserAuthChoice { /** * Do not authenticate, and switch the asking account's top-level policy to - * [com.vitorpamplona.amethyst.commons.relayauth.RelayAuthPolicy.NEVER], so no relay is ever + * [RelayAuthPolicy.NEVER], so no relay is ever * answered again. The account-wide counterpart of [BLOCK], confirmed the same way — and, like * every route through [com.vitorpamplona.amethyst.model.Account.changeDefaultRelayAuthPolicy], * it drops this run's session grants, which would otherwise outrank the policy it just set. @@ -62,6 +63,20 @@ enum class UserAuthChoice { /** No decision (dismissed or timed out) — do not authenticate, don't remember. */ DISMISS, + ; + + /** + * The account-wide policy this choice sets, or null for the four answers that are only about the + * relay being asked about. Lets a caller apply the setting without re-deriving which choices are + * account-wide. + */ + val policyEverywhere: RelayAuthPolicy? + get() = + when (this) { + ALWAYS_ALLOW_EVERYWHERE -> RelayAuthPolicy.ALWAYS + NEVER_ALLOW_EVERYWHERE -> RelayAuthPolicy.NEVER + ALLOW_ONCE, ALWAYS_ALLOW, BLOCK, DISMISS -> null + } } /** diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPolicyEverywhereTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPolicyEverywhereTest.kt index 82c463b3d0..c8c805469f 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPolicyEverywhereTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPolicyEverywhereTest.kt @@ -30,6 +30,7 @@ import com.vitorpamplona.amethyst.commons.relayauth.RelayAuthVerdict import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test /** @@ -124,6 +125,25 @@ class RelayAuthPolicyEverywhereTest { assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(relay))) } + /** + * The two account-wide answers are the only ones the host may apply to prompts other than the one + * on screen, and only to the account that was asked. Pinning the mapping keeps that fan-out — and + * the setting write that survives an expired prompt — from ever reaching a per-relay answer. + */ + @Test + fun onlyTheAccountWideAnswersCarryAPolicy() { + assertEquals(RelayAuthPolicy.ALWAYS, UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE.policyEverywhere) + assertEquals(RelayAuthPolicy.NEVER, UserAuthChoice.NEVER_ALLOW_EVERYWHERE.policyEverywhere) + assertTrue( + listOf( + UserAuthChoice.ALLOW_ONCE, + UserAuthChoice.ALWAYS_ALLOW, + UserAuthChoice.BLOCK, + UserAuthChoice.DISMISS, + ).all { it.policyEverywhere == null }, + ) + } + /** * Why the coordinator routes this through [com.vitorpamplona.amethyst.model.Account], which drops * the session grants with the flip: a grant left behind outranks the policy, so "never log in" diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt index d1931883bf..e375d2c071 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthPromptBusTest.kt @@ -30,6 +30,7 @@ import kotlinx.coroutines.flow.toList import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Test class RelayAuthPromptBusTest { @@ -143,6 +144,39 @@ class RelayAuthPromptBusTest { assertEquals(UserAuthChoice.ALWAYS_ALLOW, callerB.await()) } + /** + * "Always/Never, all relays" is answered on one dialog and applies to the prompts still queued + * behind it, which the host resolves without ever showing them. A prompt that is answered but not + * marked shown sits in the queue-wait window — up to five minutes — before its caller reads the + * answer already sitting in the deferred, so the relay it belongs to goes unauthenticated for that + * long despite the user having answered. Marking it shown is what makes the answer land now. + */ + @Test + fun anAnswerFannedOutToAQueuedPromptLandsWithoutWaitingOutTheQueueWindow() = + runTest { + val clock = testScheduler + val bus = RelayAuthPromptBus(timeoutMs = 1_000L, queueWaitMs = 300_000L) + val relayA = NormalizedRelayUrl("wss://a.relay.test") + val relayB = NormalizedRelayUrl("wss://b.relay.test") + + val surfaced = async { bus.prompts.take(2).toList() } + val callerA = async { bus.requestDecision(relayA, emptyList(), alice, isMyOwnRelay = false) } + val callerB = async { bus.requestDecision(relayB, emptyList(), alice, isMyOwnRelay = false) } + val prompts = surfaced.await() + + // Only A is on screen. The user's account-wide answer resolves B too, sight unseen. + prompts[0].markShown() + val start = clock.currentTime + prompts.forEach { + it.markShown() + it.respond(UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE) + } + + assertEquals(UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE, callerA.await()) + assertEquals(UserAuthChoice.ALWAYS_ALLOW_EVERYWHERE, callerB.await()) + assertTrue("the queued relay waited ${clock.currentTime - start}ms for an answer it already had", clock.currentTime - start < 1_000) + } + @Test fun aPromptNoHostCanEverShowStillTimesOut() = runTest { 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 aee001ad97..32b91cb571 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 @@ -89,7 +89,7 @@ data class RelayAuthInputs( * 1. Blocked-relay list → [RelayAuthVerdict.DENY] (never reveal identity to a blocked relay). * 2. Explicit per-relay override → honor it. * 3. [RelayAuthInputs.hasSessionGrant] → [RelayAuthVerdict.ALLOW]. Ranked *below* the stored override - * so a later "never allow" — the only way a DENY can be written for a relay already granted this + * so a later "never" answer — the only way a DENY can be written for a relay already granted this * session — takes effect immediately instead of losing to the in-memory grant. * 4. Top-level [RelayAuthPolicy]: * - [RelayAuthPolicy.NEVER] → DENY From d3aa91f8564b45491aac248da3d427a31424d068 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 17:50:49 +0000 Subject: [PATCH 4/5] feat: relabel the prompt's buttons to the answer the remember switch gives With the switch on, "Not now" wrote a permanent DENY and "Log in" wrote a permanent ALLOW while both still read as one-off answers. The switch is the scope of the answer, so the buttons now state the answer they actually give: "Never" and "Always log in". The refusal takes the error colour with it while the switch is on, which is the weight the removed red "Never allow" button used to carry. This closes the mis-tap the switch's new binding opened: flipping it for "log in", then changing your mind and pressing what still said "Not now", blocked the relay for good with nothing on screen saying so. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013NTYFnqWcwLVPNNSr5kusd --- .../authCommand/compose/RelayAuthPromptHost.kt | 18 +++++++++++++++--- amethyst/src/main/res/values/strings.xml | 3 +++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt index 6b5b961627..f91608feb3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt @@ -160,7 +160,8 @@ fun RelayAuthPromptHost(accountViewModel: AccountViewModel) { * *both* of them: it is the answer's scope, not a modifier on "Log in". The four combinations are * the four [UserAuthChoice] values for this relay — log in once or always, refuse once or for good — * which is why there is no separate "Never allow" button any more: it was "Not now" with the switch - * on, written twice. + * on, written twice. Flipping the switch relabels the buttons to the answer they now give + * ("Always log in" / "Never"), so the standing answer is never given under a one-off label. * * That frees the row underneath for the two answers this relay's buttons cannot give, one per * direction: "Always, all relays" and "Never, all relays" set the account's [RelayAuthPolicy] so @@ -268,14 +269,25 @@ private fun RelayAuthPromptDialog( // Both buttons read the switch, which is what makes it the scope of the answer // rather than a modifier on one of them. Refusing *and* remembering is the DENY // the red "Never allow" button used to write on its own. + // + // And both say so: with the switch on they relabel to the standing answer they + // now give, so nothing turns a one-off refusal into a permanent one behind a + // label that still reads "Not now". The refusal takes the error colour with it, + // which is the weight the removed red button carried. OutlinedButton( onClick = { onChoice(if (rememberRelay) UserAuthChoice.BLOCK else UserAuthChoice.DISMISS) }, modifier = Modifier.weight(1f), - ) { Text(stringRes(R.string.relay_auth_not_now)) } + colors = + if (rememberRelay) { + ButtonDefaults.outlinedButtonColors(contentColor = MaterialTheme.colorScheme.error) + } else { + ButtonDefaults.outlinedButtonColors() + }, + ) { Text(stringRes(if (rememberRelay) R.string.relay_auth_never else R.string.relay_auth_not_now)) } Button( onClick = { onChoice(if (rememberRelay) UserAuthChoice.ALWAYS_ALLOW else UserAuthChoice.ALLOW_ONCE) }, modifier = Modifier.weight(1f), - ) { Text(stringRes(R.string.relay_auth_log_in)) } + ) { Text(stringRes(if (rememberRelay) R.string.relay_auth_always_log_in else R.string.relay_auth_log_in)) } } Row( modifier = Modifier.fillMaxWidth(), diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index cd8c1ea285..1c2b7e3e82 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -1185,6 +1185,9 @@ Remember for this relay Log in Not now + + Always log in + Never Always, all relays Never, all relays Log in to every relay? From b948a0941a46c008c3bf0e262701b0f0c92f1658 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 18:22:20 +0000 Subject: [PATCH 5/5] fix: name the scope on the account-wide confirmation buttons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relabelling the prompt's buttons collided with the confirmation behind them: with the remember switch on, the prompt says "Always log in" for one relay while the confirmation for "Always, all relays" said "Always log in" too, one tap apart and meaning every relay. Same for "Never" against "Never log in". The confirmation now echoes the link that opened it — "Always, all relays" / "Never, all relays" — so the scope is stated exactly where the account-wide answer is committed. No new strings. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013NTYFnqWcwLVPNNSr5kusd --- .../authCommand/compose/RelayAuthPromptHost.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt index f91608feb3..5f7a581607 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/compose/RelayAuthPromptHost.kt @@ -337,6 +337,10 @@ private fun PolicyEverywhereConfirmation( ) }, confirmButton = { + // The label echoes the link that opened this, not the buttons behind it: with the switch + // on those now read "Always log in" / "Never" for *this relay*, so confirming an + // account-wide answer under the same words would make the scope ambiguous exactly where + // it matters most. Button( onClick = onConfirm, colors = @@ -348,7 +352,9 @@ private fun PolicyEverywhereConfirmation( contentColor = MaterialTheme.colorScheme.onError, ) }, - ) { Text(stringRes(if (always) R.string.relay_auth_policy_always else R.string.relay_auth_policy_never)) } + ) { + Text(stringRes(if (always) R.string.relay_auth_always_allow_everywhere else R.string.relay_auth_never_allow_everywhere)) + } }, dismissButton = { TextButton(onClick = onDismiss) { Text(stringRes(R.string.cancel)) } }, )