mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
fix(relay-auth): ask instead of silently denying a non-first-party relay
On device, no AUTH prompt ever appeared. Instrumenting the coordinator showed why: 56 of 56 challenges that arrived with an account registered were dropped at the `isFirstParty` gate, and none reached the ledger. Zero AUTH events were sent under *either* "Decide per relay" or "Always log in". The gate returned early, so a relay this account has no reason of its own to be on was denied without a word — including `purposes=[READ_OUTBOX]`, a purpose that names a counterparty and renders a perfectly good sentence. That made "Decide per relay" mean "deny, and don't mention it" for every purpose about someone else, which is the case the prompt was built to explain. Not auto-authing there is right and stays: that is what keeps a bystander account off a relay only another account uses. So `isFirstParty` stops being a gate and becomes an input to the pure resolver, where it now guards only the *automatic* grants — both the CUSTOM toggle categories and the ALWAYS policy. A challenge we cannot explain is still denied silently; one we can now falls through to ASK. Nothing the user already closed reopens: blocked relays, stored per-relay overrides and the NEVER policy are all evaluated ahead of this and unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
de247c3b5a
commit
0bddb64abc
+9
-2
@@ -92,10 +92,17 @@ class AuthCoordinator(
|
||||
authWithAccounts.distinctValues().forEach forEachAccount@{ screen ->
|
||||
val account = screen.account
|
||||
if (!account.signer.isWriteable()) return@forEachAccount
|
||||
if (!isFirstParty(account, relayUrl)) return@forEachAccount
|
||||
|
||||
// Not a gate any more, an input. Failing it only rules out the *automatic* grants
|
||||
// (see RelayAuthResolver): a relay this account has no first-party reason to be on
|
||||
// is never silently authenticated, but a challenge we can explain still becomes a
|
||||
// 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.
|
||||
val firstParty = isFirstParty(account, relayUrl)
|
||||
|
||||
val approve =
|
||||
when (account.relayAuthLedger.decide(context)) {
|
||||
when (account.relayAuthLedger.decide(context, firstParty)) {
|
||||
RelayAuthVerdict.ALLOW -> true
|
||||
RelayAuthVerdict.DENY -> false
|
||||
RelayAuthVerdict.ASK -> {
|
||||
|
||||
+12
-2
@@ -47,8 +47,17 @@ class RelayAuthPermissionLedger(
|
||||
val isFollowed: (String) -> Boolean = { false },
|
||||
val isTrustedVenue: (String) -> Boolean = { false },
|
||||
) {
|
||||
/** The authorization verdict for [ctx], taking the challenge's purpose into account. */
|
||||
suspend fun decide(ctx: RelayAuthContext): RelayAuthVerdict {
|
||||
/**
|
||||
* The authorization verdict for [ctx], taking the challenge's purpose into account.
|
||||
*
|
||||
* [isFirstParty] says whether this account has a reason of its own to be on the relay. It gates
|
||||
* the automatic grants only — a non-first-party challenge is never auto-allowed, but one we can
|
||||
* explain still reaches the user as a prompt.
|
||||
*/
|
||||
suspend fun decide(
|
||||
ctx: RelayAuthContext,
|
||||
isFirstParty: Boolean = true,
|
||||
): RelayAuthVerdict {
|
||||
fun isWrite(kind: AuthPurposeKind) = kind == AuthPurposeKind.SEND_DM || kind == AuthPurposeKind.NOTIFY_INBOX
|
||||
val inputs =
|
||||
RelayAuthInputs(
|
||||
@@ -85,6 +94,7 @@ class RelayAuthPermissionLedger(
|
||||
it.counterparties.isNotEmpty() ||
|
||||
it.venues.isNotEmpty()
|
||||
},
|
||||
isFirstParty = isFirstParty,
|
||||
)
|
||||
return RelayAuthResolver.resolve(inputs)
|
||||
}
|
||||
|
||||
+15
-2
@@ -56,6 +56,12 @@ data class RelayAuthCustomToggles(
|
||||
* @param servesStrangerWriteCounterparty a non-followed user's inbox is served here (messaging them).
|
||||
* @param hasAttributablePurpose we know *why* this relay wants auth (so a prompt can explain it).
|
||||
* When false, an unresolved challenge is denied silently rather than prompting.
|
||||
* @param isFirstParty this account has a reason of its *own* to be on this relay — it is publishing
|
||||
* 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
|
||||
* prompt rather than a silent denial.
|
||||
*/
|
||||
data class RelayAuthInputs(
|
||||
val storedOverride: RelayAuthDecision?,
|
||||
@@ -68,6 +74,7 @@ data class RelayAuthInputs(
|
||||
val servesFollowedWriteCounterparty: Boolean,
|
||||
val servesStrangerWriteCounterparty: Boolean,
|
||||
val hasAttributablePurpose: Boolean,
|
||||
val isFirstParty: Boolean = true,
|
||||
)
|
||||
|
||||
/**
|
||||
@@ -82,6 +89,12 @@ data class RelayAuthInputs(
|
||||
* this relay (own relays/venues, reading follows, messaging follows, messaging strangers);
|
||||
* else fall through
|
||||
* 4. Fall-through → [RelayAuthVerdict.ASK] when the purpose is known, otherwise DENY.
|
||||
*
|
||||
* Both automatic grants in step 3 additionally require [RelayAuthInputs.isFirstParty]: an account
|
||||
* never reveals its identity *without being asked* on a relay it has no reason of its own to be on.
|
||||
* That 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
|
||||
* "decide per relay" means the user decides rather than a silent denial they never see.
|
||||
*/
|
||||
object RelayAuthResolver {
|
||||
fun resolve(inputs: RelayAuthInputs): RelayAuthVerdict {
|
||||
@@ -96,9 +109,9 @@ object RelayAuthResolver {
|
||||
|
||||
return when (inputs.policy) {
|
||||
RelayAuthPolicy.NEVER -> RelayAuthVerdict.DENY
|
||||
RelayAuthPolicy.ALWAYS -> RelayAuthVerdict.ALLOW
|
||||
RelayAuthPolicy.ALWAYS -> if (inputs.isFirstParty) RelayAuthVerdict.ALLOW else fallThrough(inputs)
|
||||
RelayAuthPolicy.CUSTOM ->
|
||||
if (customAllows(inputs)) RelayAuthVerdict.ALLOW else fallThrough(inputs)
|
||||
if (inputs.isFirstParty && customAllows(inputs)) RelayAuthVerdict.ALLOW else fallThrough(inputs)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+33
@@ -35,6 +35,7 @@ class RelayAuthResolverTest {
|
||||
servesFollowedWriteCounterparty: Boolean = false,
|
||||
servesStrangerWriteCounterparty: Boolean = false,
|
||||
hasAttributablePurpose: Boolean = true,
|
||||
isFirstParty: Boolean = true,
|
||||
) = RelayAuthInputs(
|
||||
storedOverride = storedOverride,
|
||||
isBlocked = isBlocked,
|
||||
@@ -46,6 +47,7 @@ class RelayAuthResolverTest {
|
||||
servesFollowedWriteCounterparty = servesFollowedWriteCounterparty,
|
||||
servesStrangerWriteCounterparty = servesStrangerWriteCounterparty,
|
||||
hasAttributablePurpose = hasAttributablePurpose,
|
||||
isFirstParty = isFirstParty,
|
||||
)
|
||||
|
||||
private fun resolve(inputs: RelayAuthInputs) = RelayAuthResolver.resolve(inputs)
|
||||
@@ -130,4 +132,35 @@ class RelayAuthResolverTest {
|
||||
assertEquals(RelayAuthVerdict.ASK, resolve(inputs(hasAttributablePurpose = true)))
|
||||
assertEquals(RelayAuthVerdict.DENY, resolve(inputs(hasAttributablePurpose = false)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun nonFirstPartyAsksInsteadOfAutoAllowing() {
|
||||
// Every category that would auto-auth on our own relay becomes a question on a relay we have
|
||||
// no first-party reason to be on. Nothing here is *denied* — the user still gets to decide.
|
||||
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)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun nonFirstPartyNeverAutoAuthsUnderAlwaysPolicy() {
|
||||
// "Always log in" is a statement about the relays this account uses. A relay it is only
|
||||
// touching because of somebody else's traffic still has to be asked about.
|
||||
assertEquals(RelayAuthVerdict.ALLOW, resolve(inputs(policy = RelayAuthPolicy.ALWAYS, isFirstParty = true)))
|
||||
assertEquals(RelayAuthVerdict.ASK, resolve(inputs(policy = RelayAuthPolicy.ALWAYS, isFirstParty = false)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun nonFirstPartyStillHonoursBlocksOverridesAndNever() {
|
||||
// Restoring the question must not reopen anything the user already closed.
|
||||
assertEquals(RelayAuthVerdict.DENY, resolve(inputs(isBlocked = true, isFirstParty = false)))
|
||||
assertEquals(RelayAuthVerdict.DENY, resolve(inputs(storedOverride = RelayAuthDecision.DENY, isFirstParty = false)))
|
||||
assertEquals(RelayAuthVerdict.ALLOW, resolve(inputs(storedOverride = RelayAuthDecision.ALLOW, isFirstParty = false)))
|
||||
assertEquals(RelayAuthVerdict.DENY, resolve(inputs(policy = RelayAuthPolicy.NEVER, isFirstParty = false)))
|
||||
// Still no prompt when we cannot explain the challenge.
|
||||
assertEquals(RelayAuthVerdict.DENY, resolve(inputs(hasAttributablePurpose = false, isFirstParty = false)))
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user