Public Access
Ask about a host key where the connection was made, not on the host list
The trust prompt was two banners at the top of the desktop's hosts screen, so the shell navigated there before letting a handshake raise one: Screen = Hosts, Surface = Page, in OnVaultConnectionFailed and again in the palette's own connect. The reason was sound — a connection can be started from Ctrl+K on any screen, and a question behind whatever somebody is looking at is a question nobody can answer — and it was answered the wrong way round. Rather than making the decision reachable from where the user is, it moved the user to where the decision was, and charged every screen for it. It is worst for the one connection that has no host at all. A machine typed into the phone's connect box is deliberately in no keychain, so a first contact from there judged it on a list it does not appear on, after taking the box that dialled it away. So both heads now draw the decision over the surface. HostKeyCard is the desktop's, and is the counterpart of the phone's HostKeySheet: a scrim with no press handler, because a question with two named answers must not be answerable by missing; the unknown key offering TRUST AND CONNECT, because judging a fingerprint against what an operator published is a decision a person is entitled to make and the only moment they can make it; and the changed key offering no way forward at all, because a button beside that warning is "continue anyway" with two clicks instead of one. The phone needed no new markup — its sheet was already a shell-level overlay, so deleting the navigation is what puts it over the Connections screen. IsHostKeyDecisionShowing is on the shell rather than on a screen because the answer decides an occlusion. A second connection can be refused while a first one is open, so this card is routinely raised over a live terminal, and that rectangle is a native child window: layered over it the card would be sliced at its left edge with TRUST AND CONNECT taking no clicks, which for the most safety-critical question in the product is the worst place for that class of bug to land. IsTerminalShowing gives the rectangle up instead. The banners are gone rather than copied. One prompt in two markups is two copies of the most safety-critical wording here, and the second is the one that goes stale. TWO DEFECTS FOUND BEHIND IT. VaultViewModel.RejectHostKey cleared only the pending key and never the mismatch, so the changed-key refusal had no working exit. That was invisible for as long as it was a banner nothing was drawn over — nothing was trapped, and the next attempt cleared it — and it was already live on the phone, where that refusal is an opaque full-screen panel whose one button runs this command: pressing it left the panel up over every screen the user went to next, including the host editor the panel tells them to open. TransfersViewModel.RejectHostKey has always cleared both; the vault's was the outlier. Its button said BACK TO HOSTS, which was wrong twice over, and now says BACK. And an assertion written for this change could not fail: the palette test asserted the renderer was collapsed in a scenario whose only tab had just been removed, so it was collapsed for want of a session whatever the occlusion rule said. It is gone, with a note pointing at the test that can fail on it. VERIFIED. 1580 tests, build clean, no new warnings, format clean. Three mutations each seen to fail and then seen green again: dropping !IsHostKeyDecisionShowing from IsTerminalShowing, caught by AChangedHostKey_CollapsesTheTerminalItIsRefusedOver; reverting RejectHostKey to clear one flag, caught by RefusingAHostKeyDecision_TakesItOffTheScreen(false) and by that same test; and dropping the two host-key arms from OnVaultPropertyChanged, caught by TheHostKeyDecision_IsAnnouncedToTheWindowWhenItArrivesAndWhenItGoes. That last one is the first test in this repository to watch PropertyChanged, and it is worth being the first: every other assertion about the flag reads it directly, and a direct read passes with the subscription deleted — while the card would never go away. The two layout tests moved with the prompts, from the hosts screen to the card. manual-checks gains 7.4a for the occlusion, 7.4b for getting out of a refusal and 11.7a for the hand-typed case, none of which a test can see; 1.5 and 7.4 are corrected rather than left describing a window that no longer moves. ONE ROUGH EDGE, DELIBERATELY LEFT. On the desktop, refusing a first contact whose tab was the only one leaves the terminal surface with no tabs — a blank rectangle under the strip's "no terminals open · press + or Ctrl+K", which is the one sentence near that rectangle Avalonia can draw. The alternative was falling back to the page, and on the phone that means the host list, which is the bug this commit is about. A desktop connect page would close it properly.
This commit is contained in:
@@ -1213,9 +1213,9 @@ public sealed class ShellFlowTests : IAsyncLifetime
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// The other kind of not-connecting. An unknown host key is a question drawn on the hosts screen rather
|
||||
/// than a failure, so the tab goes and the window is put back where the question is — a tab saying the
|
||||
/// connection failed would be competing with the prompt that is about to resume it.
|
||||
/// The other kind of not-connecting. An unknown host key is a question rather than a failure, so the tab
|
||||
/// goes — a tab saying the connection failed would be competing with the decision that is about to resume
|
||||
/// it — and the window stays where it was, because the decision is drawn over whatever that is.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task AnUnknownHostKey_TakesTheTabAwayAndShowsTheQuestion()
|
||||
@@ -1234,13 +1234,109 @@ public sealed class ShellFlowTests : IAsyncLifetime
|
||||
shell.Tabs.ShouldBeEmpty();
|
||||
vault.HasPendingHostKey.ShouldBeTrue();
|
||||
|
||||
shell.IsHostsShowing.ShouldBeTrue("the prompt is drawn there, and it has to be reachable");
|
||||
// It used to assert IsHostsShowing here, because the prompt was a banner on that screen and this
|
||||
// handler navigated to it. The screen is untouched now and the card is over it instead.
|
||||
shell.Screen.ShouldBe(ShellScreen.Transfers);
|
||||
shell.IsHostKeyDecisionShowing.ShouldBeTrue("and it has to be reachable from wherever the user is");
|
||||
|
||||
// And answering it connects, which is the whole reason the tab was not left saying it had failed.
|
||||
ssh.Failure = null;
|
||||
await vault.TrustHostKeyCommand.ExecuteAsync(null);
|
||||
|
||||
shell.Tabs.ShouldHaveSingleItem().HasSession.ShouldBeTrue();
|
||||
shell.IsHostKeyDecisionShowing.ShouldBeFalse("the decision goes with the answer");
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// Refusing, and the state it leaves behind. Both halves have to clear: the changed-key refusal is drawn
|
||||
/// over the surface on both heads and the phone's is an opaque full-screen panel, so a dismissal that left
|
||||
/// the flag set would leave that panel up over every screen the user went to next — including the host
|
||||
/// editor it tells them to open. See VaultViewModel.RejectHostKey, which used to clear only the other one.
|
||||
/// </remarks>
|
||||
[Theory]
|
||||
[InlineData(true)]
|
||||
[InlineData(false)]
|
||||
public async Task RefusingAHostKeyDecision_TakesItOffTheScreen(bool firstContact)
|
||||
{
|
||||
var vault = await ReadyToConnectAsync();
|
||||
|
||||
await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
|
||||
|
||||
ssh.Failure = firstContact
|
||||
? new SshHostKeyUnknownException(
|
||||
new HostKeyPresentation("db.internal", 22, "ssh-ed25519", "SHA256:unknown"))
|
||||
: new SshHostKeyMismatchException(
|
||||
new HostKeyPresentation("db.internal", 22, "ssh-ed25519", "SHA256:the-new-key"),
|
||||
"SHA256:the-pinned-key");
|
||||
|
||||
await vault.ConnectCommand.ExecuteAsync(null);
|
||||
|
||||
shell.IsHostKeyDecisionShowing.ShouldBeTrue(shell.StatusMessage);
|
||||
|
||||
vault.RejectHostKeyCommand.Execute(null);
|
||||
|
||||
vault.HasPendingHostKey.ShouldBeFalse();
|
||||
vault.HasHostKeyMismatch.ShouldBeFalse();
|
||||
shell.IsHostKeyDecisionShowing.ShouldBeFalse();
|
||||
|
||||
// Nothing was pinned either way, so the machine is still a first contact next time.
|
||||
(await knownHosts.FindAsync("db.internal", 22, "ssh-ed25519", Token)).ShouldBeNull();
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// The one thing about the card that nothing else here can see, and the whole arrangement rests on it: the
|
||||
/// question belongs to the vault and the flag that draws the card — and collapses the terminal under it —
|
||||
/// belongs to the shell, so the shell has to re-raise it. Without that the card would never appear and,
|
||||
/// worse, never go away.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Nothing else in this suite watches <c>PropertyChanged</c>, and this is why it is worth being the first:
|
||||
/// every other assertion about the flag reads it directly, and a direct read passes with the subscription
|
||||
/// deleted. The departure is the half that matters — <c>OnVaultConnectionFailed</c> raises the state
|
||||
/// itself as the tab goes, so an arrival is announced twice over, and answering the question is announced
|
||||
/// only from the vault's own notification.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task TheHostKeyDecision_IsAnnouncedToTheWindowWhenItArrivesAndWhenItGoes()
|
||||
{
|
||||
var vault = await ReadyToConnectAsync();
|
||||
|
||||
await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
|
||||
|
||||
var announcements = 0;
|
||||
shell.PropertyChanged += OnShellPropertyChanged;
|
||||
|
||||
try
|
||||
{
|
||||
ssh.Failure = new SshHostKeyUnknownException(
|
||||
new HostKeyPresentation("db.internal", 22, "ssh-ed25519", "SHA256:unknown"));
|
||||
|
||||
await vault.ConnectCommand.ExecuteAsync(null);
|
||||
|
||||
announcements.ShouldBeGreaterThan(0, "the card has to be told to appear");
|
||||
|
||||
announcements = 0;
|
||||
vault.RejectHostKeyCommand.Execute(null);
|
||||
|
||||
announcements.ShouldBeGreaterThan(0, "and to go, which is the half a stale flag would trap");
|
||||
}
|
||||
finally
|
||||
{
|
||||
shell.PropertyChanged -= OnShellPropertyChanged;
|
||||
}
|
||||
|
||||
void OnShellPropertyChanged(object? sender, System.ComponentModel.PropertyChangedEventArgs e)
|
||||
{
|
||||
if (string.Equals(
|
||||
e.PropertyName,
|
||||
nameof(MainWindowViewModel.IsHostKeyDecisionShowing),
|
||||
StringComparison.Ordinal))
|
||||
{
|
||||
announcements++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
@@ -1459,12 +1555,21 @@ public sealed class ShellFlowTests : IAsyncLifetime
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// The palette can be opened from any screen, and an unknown host key is answered by a prompt drawn on
|
||||
/// the hosts screen. Without this the connection would block on a question sitting behind whatever screen
|
||||
/// the user happened to be on.
|
||||
/// <para>
|
||||
/// The palette can be opened from any screen, and this used to be why an unknown host key moved the
|
||||
/// window: the prompt was a banner on the hosts screen, so the shell navigated there before letting the
|
||||
/// vault raise it, or the connection would have blocked on a question behind whatever the user was
|
||||
/// looking at.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The decision is drawn over the surface now, on both heads, so nothing moves — and this is the same
|
||||
/// test inverted, kept rather than deleted because the requirement it was written for still holds. What
|
||||
/// changed is how it is met: the question has to be answerable from where the user is, not the user
|
||||
/// taken to where the question is.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task ConnectingFromThePalette_LandsOnTheHostsPageBeforeItCanBeRefused()
|
||||
public async Task ConnectingFromThePalette_AsksAboutTheHostKeyWithoutMovingTheWindow()
|
||||
{
|
||||
var vault = await ReadyToConnectAsync();
|
||||
|
||||
@@ -1482,8 +1587,77 @@ public sealed class ShellFlowTests : IAsyncLifetime
|
||||
|
||||
vault.HasPendingHostKey.ShouldBeTrue();
|
||||
|
||||
shell.IsHostsShowing.ShouldBeTrue("the prompt is drawn on the hosts screen");
|
||||
shell.IsTerminalShowing.ShouldBeFalse();
|
||||
shell.Screen.ShouldBe(
|
||||
ShellScreen.Transfers, "the transfer the user was looking at is still what is underneath");
|
||||
|
||||
shell.IsHostKeyDecisionShowing.ShouldBeTrue("and the card is over it");
|
||||
|
||||
// No assertion about the renderer here, deliberately: this attempt's tab was the only one, so it is
|
||||
// collapsed for want of a session whatever the occlusion rule says, and a test that cannot fail is
|
||||
// worse than no test. That claim belongs where a live terminal is behind the card — see
|
||||
// AChangedHostKey_CollapsesTheTerminalItIsRefusedOver.
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// The connection with no host, which is the case the old navigation was worst for: it took the box that
|
||||
/// was typed into away and asked about the machine on a list the machine is deliberately not on. The
|
||||
/// phone's connect box is on the terminal surface, so staying there is what keeps it behind the sheet —
|
||||
/// and what the user comes back to whichever way they answer.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task ConnectingByHand_AsksAboutTheHostKeyOverTheBoxItWasTypedInto()
|
||||
{
|
||||
var vault = await ReadyToConnectAsync();
|
||||
|
||||
await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
|
||||
|
||||
shell.ShowTerminalCommand.Execute(null);
|
||||
|
||||
ssh.Failure = new SshHostKeyUnknownException(
|
||||
new HostKeyPresentation("build.internal", 2222, "ssh-ed25519", "SHA256:unknown"));
|
||||
|
||||
vault.ManualTarget = "deploy@build.internal:2222";
|
||||
vault.ManualPassword = "hunter2";
|
||||
|
||||
await vault.ConnectManuallyCommand.ExecuteAsync(null);
|
||||
|
||||
vault.HasPendingHostKey.ShouldBeTrue();
|
||||
|
||||
shell.IsTerminalSurface.ShouldBeTrue("the connect box is on this surface and has not been taken away");
|
||||
shell.IsHostKeyDecisionShowing.ShouldBeTrue();
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// A changed key is the other half of the same control and the reason it collapses the renderer: a second
|
||||
/// connection can be refused while a first one is open, and the refusal has to be readable over it.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task AChangedHostKey_CollapsesTheTerminalItIsRefusedOver()
|
||||
{
|
||||
var vault = await ReadyToConnectAsync();
|
||||
|
||||
await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
|
||||
|
||||
// One live session first, so there is something in the rectangle for the refusal to be drawn over.
|
||||
await vault.ConnectCommand.ExecuteAsync(null);
|
||||
|
||||
shell.IsTerminalShowing.ShouldBeTrue(shell.StatusMessage);
|
||||
|
||||
ssh.Failure = new SshHostKeyMismatchException(
|
||||
new HostKeyPresentation("db.internal", 22, "ssh-ed25519", "SHA256:the-new-key"),
|
||||
"SHA256:the-pinned-key");
|
||||
|
||||
await vault.ConnectCommand.ExecuteAsync(null);
|
||||
|
||||
vault.HasHostKeyMismatch.ShouldBeTrue();
|
||||
shell.IsHostKeyDecisionShowing.ShouldBeTrue();
|
||||
shell.IsTerminalShowing.ShouldBeFalse("the card would otherwise be sliced at the WebView's edge");
|
||||
|
||||
// And it comes back when the refusal is put away, rather than needing a tab click to restore it.
|
||||
vault.RejectHostKeyCommand.Execute(null);
|
||||
|
||||
shell.IsHostKeyDecisionShowing.ShouldBeFalse();
|
||||
shell.IsTerminalShowing.ShouldBeTrue();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
Reference in New Issue
Block a user