From 80bb4c4e58a2a2d0772c2e93d5813fd810cfdfd3 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Sun, 20 Sep 2026 11:21:41 -0400 Subject: [PATCH] Say which server changed its identity, instead of shrugging at the network The relay pin exists to catch one thing: the server signing with an issuer key this app never agreed to. It caught it and threw an exception carrying a warning written for a person to read, naming the host and giving both readings the app cannot tell apart -- a rotation done on purpose, and something else answering in its place. describeFailure had a branch for a forked key log and one for a contact's changed key, and none for this, so it fell through to "Something went wrong reaching that server." The alarm arrived as a shrug about the network, and the carefully written warning was discarded on the way. Found looking for the Android half of the iOS relay-identity banner added earlier today. Android does raise it; nothing said so. Co-Authored-By: Claude Opus 5 --- .../messenger/app/MessengerViewModel.kt | 13 ++++ .../AChangedServerIdentityIsExplainedTest.kt | 70 +++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 app/src/test/kotlin/io/flamenet/messenger/app/AChangedServerIdentityIsExplainedTest.kt diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index 8678a80..8117b1b 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -16,6 +16,8 @@ import io.flamenet.client.MessengerApi import io.flamenet.client.RelayAccounts import io.flamenet.client.RelayClient import io.flamenet.client.RelayException +import io.flamenet.client.RelayIdentityChangedException +import io.flamenet.client.RelayIdentityPin import io.flamenet.e2e.AccountKdf import io.flamenet.e2e.Backup import io.flamenet.e2e.BackupPayload @@ -4283,6 +4285,17 @@ internal fun describeFailure(failure: Throwable): String = "security code with them another way before continuing." } + // The one case the relay pin exists to catch, and it reached the + // screen as "Something went wrong reaching that server." + // + // `RelayIdentityPin.warning` is written for a person to read and says + // both things the app cannot tell apart: a key rotated on purpose and + // something answering in the server's place. Falling through to the + // generic line threw that away and turned the alarm into a shrug. + is RelayIdentityChangedException -> { + failure.message ?: RelayIdentityPin.warning(failure.host) + } + is Vault.Corrupt -> { "This phone's stored keys could not be read." } diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/AChangedServerIdentityIsExplainedTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/AChangedServerIdentityIsExplainedTest.kt new file mode 100644 index 0000000..33fec03 --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/AChangedServerIdentityIsExplainedTest.kt @@ -0,0 +1,70 @@ +package io.flamenet.messenger.app + +import io.flamenet.client.RelayIdentityChangedException +import kotlin.test.Test +import kotlin.test.assertTrue + +/** + * The relay pin exists to catch one thing: the server signing with an issuer + * key this app did not agree to. It caught it, threw an exception carrying a + * warning written for a person to read, and the screen said "Something went + * wrong reaching that server." + * + * `describeFailure` named the key-log fork and the changed contact key and had + * no branch for this, so the alarm arrived as a shrug about the network. + */ +class AChangedServerIdentityIsExplainedTest { + private val message = describeFailure(RelayIdentityChangedException("relay.example.com")) + + @Test + fun `it names the server whose identity changed`() { + assertTrue( + message.contains("relay.example.com"), + "\"$message\" does not say which server, and a person with two " + + "accounts cannot act on that", + ) + } + + /** + * Both readings, because the app genuinely cannot tell them apart and + * saying only the frightening one is as wrong as saying only the dull one. + */ + @Test + fun `it gives both readings rather than picking the comfortable one`() { + assertTrue( + message.contains("identity", ignoreCase = true), + "\"$message\" does not say what changed", + ) + assertTrue( + message.contains("on purpose", ignoreCase = true) || + message.contains("rotated", ignoreCase = true), + "\"$message\" does not allow that this can be an ordinary rotation", + ) + assertTrue( + message.contains("in its place", ignoreCase = true) || + message.contains("something else", ignoreCase = true), + "\"$message\" does not allow that this can be somebody else answering", + ) + } + + /** And says what to do, which is not "try again". */ + @Test + fun `it says to check with whoever runs the server`() { + assertTrue( + message.contains("whoever runs it", ignoreCase = true) || + message.contains("check with", ignoreCase = true), + "\"$message\" leaves the person with no next step", + ) + } + + /** Not the generic fallback, which is what this used to produce. */ + @Test + fun `it is not the generic something-went-wrong line`() { + assertTrue( + !message.contains("Something went wrong", ignoreCase = true) && + !message.contains("reaching that server", ignoreCase = true), + "a changed server identity still falls through to the generic " + + "network line: \"$message\"", + ) + } +}