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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
cc4f5d8844
commit
80bb4c4e58
@@ -16,6 +16,8 @@ import io.flamenet.client.MessengerApi
|
|||||||
import io.flamenet.client.RelayAccounts
|
import io.flamenet.client.RelayAccounts
|
||||||
import io.flamenet.client.RelayClient
|
import io.flamenet.client.RelayClient
|
||||||
import io.flamenet.client.RelayException
|
import io.flamenet.client.RelayException
|
||||||
|
import io.flamenet.client.RelayIdentityChangedException
|
||||||
|
import io.flamenet.client.RelayIdentityPin
|
||||||
import io.flamenet.e2e.AccountKdf
|
import io.flamenet.e2e.AccountKdf
|
||||||
import io.flamenet.e2e.Backup
|
import io.flamenet.e2e.Backup
|
||||||
import io.flamenet.e2e.BackupPayload
|
import io.flamenet.e2e.BackupPayload
|
||||||
@@ -4283,6 +4285,17 @@ internal fun describeFailure(failure: Throwable): String =
|
|||||||
"security code with them another way before continuing."
|
"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 -> {
|
is Vault.Corrupt -> {
|
||||||
"This phone's stored keys could not be read."
|
"This phone's stored keys could not be read."
|
||||||
}
|
}
|
||||||
|
|||||||
+70
@@ -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\"",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user