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 aa45baf7fa..81caf47040 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 @@ -149,8 +149,24 @@ class RelayAuthPermissionLedger( /** * Remembers a "log in" answer for [relayUrl] until the app is restarted, so the relay's next * reconnect doesn't ask again. Nothing is written to disk — see [RelayAuthSessionGrants]. + * + * Refused, returning false, while [globalPolicy] is [RelayAuthPolicy.NEVER]: that is the + * switch-it-all-off answer, and a session grant outranks the policy (see [RelayAuthResolver]), + * so recording one here would quietly re-enable the very thing the user just turned off. The + * settings screen clears existing grants when the policy is set to NEVER; this stops a *new* + * one being written afterwards — which the undo on "forget this login" otherwise did, because + * its snackbar carries an action label and so sits on screen indefinitely, long enough for the + * policy to change underneath it. + * + * Only the policy needs this guard. A stored override arriving in the same window is + * self-protecting: it is ranked *above* the grant, so an ALLOW or DENY written meanwhile + * decides the relay either way. So is the block list, which outranks everything. */ - fun grantForSession(relayUrl: String) = sessionGrants.grant(relayUrl) + fun grantForSession(relayUrl: String): Boolean { + if (globalPolicy() == RelayAuthPolicy.NEVER) return false + sessionGrants.grant(relayUrl) + return true + } /** Forgets this session's grant for [relayUrl], so the next challenge is decided from scratch. */ fun revokeSessionGrant(relayUrl: String) = sessionGrants.revoke(relayUrl) 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 31b333c2cb..8f676adbcd 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 @@ -155,18 +155,29 @@ fun RelayAuthSettingsScreen( val removedLabel = stringResource(R.string.relay_auth_exception_removed_undo) val sessionForgottenLabel = stringResource(R.string.relay_auth_session_forgotten_undo) + val sessionUndoBlockedLabel = stringResource(R.string.relay_auth_session_undo_blocked) val undoLabel = stringResource(R.string.relay_auth_undo) fun forgetSessionGrant(url: String) { ledger.revokeSessionGrant(url) scope.launch { + val display = url.normalizeRelayUrlOrNull()?.displayUrl() ?: url val result = snackbarHostState.showSnackbar( - message = sessionForgottenLabel.format(url.normalizeRelayUrlOrNull()?.displayUrl() ?: url), + message = sessionForgottenLabel.format(display), actionLabel = undoLabel, withDismissAction = true, ) - if (result == SnackbarResult.ActionPerformed) ledger.grantForSession(url) + // An action label makes Material3 show this indefinitely, so the undo can be tapped long + // after the fact — including after the policy above was switched to "Never log in", which + // clears every grant. The ledger refuses to write a new one in that state; report that + // instead of leaving a tapped undo looking like it silently did nothing. + if (result == SnackbarResult.ActionPerformed && !ledger.grantForSession(url)) { + snackbarHostState.showSnackbar( + message = sessionUndoBlockedLabel.format(display), + withDismissAction = true, + ) + } } } diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index a8e4100d84..89bd5d8792 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -1202,6 +1202,7 @@ Logged in until you restart Amethyst Forget this login %1$s will ask again the next time it needs you. + Not restored. “Never log in” is on, so Amethyst won\'t log in to %1$s. Blocked by your block list Amethyst never logs in to blocked relays. Nothing blocked. Relays you block will never be logged in to, whatever you set here. diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthSessionGrantsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthSessionGrantsTest.kt index b9285192c8..c3290c9876 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthSessionGrantsTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/authCommand/model/RelayAuthSessionGrantsTest.kt @@ -53,9 +53,10 @@ class RelayAuthSessionGrantsTest { grants: RelayAuthSessionGrants = RelayAuthSessionGrants(), store: RelayAuthPermissionStore = InMemoryRelayAuthPermissionStore(), blocked: Set = emptySet(), + policy: () -> RelayAuthPolicy = { RelayAuthPolicy.CUSTOM }, ) = RelayAuthPermissionLedger( store = store, - globalPolicy = { RelayAuthPolicy.CUSTOM }, + globalPolicy = policy, sessionGrants = grants, isBlocked = { it in blocked }, ) @@ -234,4 +235,58 @@ class RelayAuthSessionGrantsTest { grants.clear() assertEquals(emptySet(), grants.grants.value) } + + @Test + fun aGrantIsRefusedWhileThePolicyIsNever() = + runTest { + val grants = RelayAuthSessionGrants() + val ledger = ledger(grants = grants, policy = { RelayAuthPolicy.NEVER }) + + assertFalse(ledger.grantForSession(relay)) + assertFalse(grants.isGranted(relay)) + assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(relay))) + } + + @Test + fun undoingAForgetAfterSwitchingToNeverDoesNotResurrectTheGrant() = + runTest { + // The settings screen's undo snackbar carries an action label, so Material3 leaves it up + // indefinitely — the user can switch the whole policy off and only then tap undo. + val grants = RelayAuthSessionGrants() + var policy = RelayAuthPolicy.CUSTOM + val ledger = ledger(grants = grants, policy = { policy }) + + assertTrue(ledger.grantForSession(relay)) + assertEquals(RelayAuthVerdict.ALLOW, ledger.decide(askable(relay))) + + // "Forget this login", then "Never log in" — which also clears what is already granted. + ledger.revokeSessionGrant(relay) + policy = RelayAuthPolicy.NEVER + grants.clear() + + // ...and only now, undo. + assertFalse(ledger.grantForSession(relay)) + assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(relay))) + } + + @Test + fun theGuardOnlyAppliesToNever() = + runTest { + assertTrue(ledger(policy = { RelayAuthPolicy.CUSTOM }).grantForSession(relay)) + assertTrue(ledger(policy = { RelayAuthPolicy.ALWAYS }).grantForSession(relay)) + } + + @Test + fun aStoredDecisionTakenDuringTheUndoWindowNeedsNoGuardBecauseItOutranksTheGrant() = + runTest { + // Why the guard is narrowed to the policy: an override written while the snackbar was up + // is ranked above the grant, so restoring the grant cannot undo the user's newer answer. + val ledger = ledger() + + ledger.revokeSessionGrant(relay) + ledger.setDecision(relay, RelayAuthDecision.DENY) + + assertTrue(ledger.grantForSession(relay)) + assertEquals(RelayAuthVerdict.DENY, ledger.decide(askable(relay))) + } }