Show one kind of vault item at a time, and let the vault hold passwords

Outstanding items #8 and #9, in one commit rather than two. They are separable
as work and were built in that order, but not as a diff: the section enum has
three members, the one-editor guard has three arms, and the picker offers keys
and credentials from the same list. Reconstructing an #8-only state would mean
hand-writing an intermediate version of VaultViewModel that never existed and
that no test has ever run. One honest commit beats two invented ones.

--- #8, the type selector ---

The column showed two lists and two editors stacked in 340 pixels, and only
just: the key list needed a MaxHeight and had to hide itself whenever its editor
opened, both to stop the host list above it pushing the buttons off the bottom
edge. Credentials would not have fitted at all. It now shows one kind at a time,
chosen by a selector at the top, and both workarounds are gone because a section
owns the whole column.

Three departures from the plan, each with a reason found while building it.

The selector is plain Buttons and a parameterised command, not a TabControl, a
TabStrip or a ListBox. All three of those hold the selection themselves, so a
click moves the highlight before the view model can refuse it — and this column
does refuse, while an editor is open. A selector lit on a section the column is
not showing is worse than the refusal it would be hiding. Buttons carry no state
and cannot disagree with the vault.

The one-editor-at-a-time rule survives with its justification replaced. That
rule was a workaround for the sizing problem above, and sections dissolved it:
the editors are in different sections and only one section is ever laid out.
BothEditorsAtOnce_DoNotFit_WhichIsWhyTheRuleExists is now
BothEditorsOpen_NowFit_BecauseOnlyOneSectionIsLaidOut — the same test, inverted,
because its own comment said that if it ever started passing the rule had become
unnecessary. It has. The rule stays for a better reason: an open key editor
holds a pasted private key in a bound string, and letting the column move on
would leave key material in a form nobody can see, with nothing on screen to say
it is there. A sizing hack became a rule about not hiding a secret from the
person holding it. KeyEditorIsInTheWay and HostEditorIsInTheWay are one
AnEditorIsInTheWay, called by the section switch and by every editor-opening
command.

And releasing the keyboard from the terminal has never worked. MainWindow takes
Win32 focus off the WebView's child window and then calls Focus() on
VaultColumn.KeyboardTarget — and a ListBox is not focusable by default in
Avalonia, which leaves focus to its items. So the call returned false, the
window ended up with nothing focused, and the keystrokes went nowhere: exactly
the state that method's own comment says its second half exists to prevent.
Found by writing the test to assert focus was taken rather than that the right
control was named — the cheap assertion was already passing. Fixed with
Focusable="True" on every list.

--- #9, credentials ---

Credentials have synced since they were added and could not be created. They can
now, and the sync layer needed no change at all: fourth item type, same result,
which is the item-kind seam working as intended.

One picker for all three ways a host authenticates, which is what makes the
illegal combination unrepresentable rather than merely invalid. SshKeyChoice
became AuthenticationChoice carrying an AuthenticationKind, and BuildHost reads
both SshKeyId and CredentialId off that single selection, so a host naming a key
and a credential — which HostSecret.TryValidate refuses — cannot be expressed.
Two pickers would have expressed it and then rejected it at save time. The kind
travels with the id in three places and none is padding: Missing takes it, the
placeholder lookup matches on kind as well as id, and Bound(kind) returns null
unless the selection is that kind. Drop any one and a dangling credential comes
back as a dangling key, which saves as a key binding to an id no key has.

A credential's username had to reach the SSH request, not just its password.
TryBuildCredential returned only the secret and the connect path read the
username off the host, so a stored credential would have gone out under the
wrong account — wrong in a way a server only reports as "authentication failed".
It is now TryBuildAuthentication returning a (Username, Credential) pair.

The no-username refusal moved, and had to. It ran before anything looked at the
binding, which made a credential's username unreachable in the one case it is
most useful: a host somebody never filled a username in for. It is now the last
thing every branch agrees on, so such a host is perfectly usable through a
credential that carries one, and a host with neither still refuses and now says
where to put one.

--- What the measurements cost ---

Ten mutations, all caught. Two are worth naming.

Removing a section's IsVisible is caught by OnlyOneSectionIsOnScreenAtOnce and
by nothing else: two visible sections overlap in the row they share rather than
clip, so every fit test still passes while the column shows one list through
another.

Defaulting the credential selection to the first row is caught by
ReloadingKeepsACredentialSelectionButNeverInventsOne, and the property is a
safety one rather than tidiness — Delete acts on the selection, so a list that
picked a row on every background sync would aim a one-click password deletion at
something nobody chose. The key list has the same property, and its comment
cited a method that has not existed for some time; both now name the delete
command they actually protect.

One test of mine could not fail, and the mutation pass is what found it.
AHostBoundToACredential_SendsItsPasswordAndItsUsername gave the credential and
the host the same username, so it passed whichever one the code read. An
override is only tested when the two values differ.

Two shipped statements went false and were corrected rather than left: the class
remark saying passwords were "not yet" in the vault, and the terminal column's
"Keys are in the vault; passwords are not yet."

That column's hint is now a tooltip on the password box rather than a sentence
in the row, which was measured the hard way — by looking. At the window's 820px
minimum the column gets 480, and a 220px box plus Connect plus any sentence does
not fit; the row has shipped clipped for as long as it has had a hint in it.
That strip is the one part of the window nothing can measure, because MainWindow
cannot be laid out headlessly at all. Extracting it into its own control, as the
vault column was extracted for exactly this reason, is what would fix that, and
is not done here.

911 tests green, 30 of them new. Zero warnings, dotnet format clean.

Seen by a person, which is how the two defects above were found. Still open from
that pass: unlocking with the device key raises its consent dialog and then
never returns, while registering one works — the difference is which thread the
CNG call lands on, and diagnosing it properly is its own change.
This commit is contained in:
2026-07-30 17:08:59 +02:00
parent 573f5d5668
commit da7462e41f
6 changed files with 1835 additions and 341 deletions
File diff suppressed because it is too large Load Diff
+22 -15
View File
@@ -90,28 +90,35 @@
<Border Grid.Row="0" Padding="10,8" Background="#171b24" IsVisible="{Binding IsUnlocked}"> <Border Grid.Row="0" Padding="10,8" Background="#171b24" IsVisible="{Binding IsUnlocked}">
<StackPanel Orientation="Horizontal" Spacing="8"> <StackPanel Orientation="Horizontal" Spacing="8">
<!-- <!--
Typed per connection. Credentials do sync, but nothing in this interface can create one, so Typed per connection, and now only for a host that asks to be. A host bound to a stored
the vault genuinely does not hold this — saying so beats a password box that looks like it credential wants nothing typed here; so does one bound to a key. "not stored" rather than "not
should have been remembered. Disabled rather than hidden when a key is being used, so it stored yet", because the vault can hold this now — a host on a typed password is a host nobody
stays visible that a password is what the other choice means. has bound a credential to, which is a choice rather than a missing feature.
The rest of that explanation is a tooltip and not a sentence in this row, which was measured the
hard way: at the window's 820px minimum this column gets 480, and a 220px box plus Connect plus
any sentence does not fit. The row shipped clipped for as long as it has had a hint in it. This
is also the one strip of the window no test can see — MainWindow cannot be laid out headlessly at
all, because WebView2 refuses the headless dispatcher's MTA thread. Extracting this header into
its own control, as the vault column was extracted for exactly this reason, is what would make
it measurable.
--> -->
<TextBox Text="{Binding Vault.ConnectPassword}" PlaceholderText="password (not stored yet)" <TextBox Text="{Binding Vault.ConnectPassword}" PlaceholderText="password (not stored)"
PasswordChar="•" Width="220" VerticalAlignment="Center" PasswordChar="•" Width="220" VerticalAlignment="Center"
IsVisible="{Binding !Vault.SelectedHostUsesAKey}" /> IsVisible="{Binding Vault.SelectedHostAsksForAPassword}"
ToolTip.Tip="Typed each time and never stored. To stop typing it, add a password under Passwords and bind this host to it in the host's own editor." />
<!-- <!--
Hidden rather than disabled for the key case, unlike most of this window. A disabled password Hidden rather than disabled, unlike most of this window. A disabled password box invites the
box invites the reading that a password is wanted and unavailable; the honest statement for a reading that a password is wanted and unavailable; the honest statement for a host that carries
key-authenticated host is that nothing needs typing, and an absent box says that better than a its own secret is that nothing needs typing, and an absent box says that better than a
greyed-out one. greyed-out one. Which secret it carries is in the sentence, because a stored password can be
wrong and re-typed here and a key cannot.
--> -->
<TextBlock Text="This host authenticates with its SSH key." Classes="hint" FontSize="11" <TextBlock Text="{Binding Vault.SelectedHostAuthenticationNote}" Classes="hint" FontSize="11"
VerticalAlignment="Center" VerticalAlignment="Center"
IsVisible="{Binding Vault.SelectedHostUsesAKey}" /> IsVisible="{Binding !Vault.SelectedHostAsksForAPassword}" />
<Button Content="Connect" Command="{Binding Vault.ConnectCommand}" <Button Content="Connect" Command="{Binding Vault.ConnectCommand}"
IsEnabled="{Binding !Vault.IsBusy}" VerticalAlignment="Center" /> IsEnabled="{Binding !Vault.IsBusy}" VerticalAlignment="Center" />
<TextBlock Classes="hint" FontSize="11" VerticalAlignment="Center"
Text="Keys are in the vault; passwords are not yet."
IsVisible="{Binding !Vault.SelectedHostUsesAKey}" />
</StackPanel> </StackPanel>
</Border> </Border>
+181 -40
View File
@@ -5,28 +5,99 @@
x:DataType="vm:VaultViewModel"> x:DataType="vm:VaultViewModel">
<!-- <!--
The vault column: hosts above, SSH keys below. The vault column: a selector at the top, and one kind of item below it.
One kind at a time rather than every list stacked. Stacking is what this column did with two of them and
it was already at its limit — the key list needed a MaxHeight and had to hide itself whenever its editor
opened, both to keep the host list above it from pushing the buttons off the bottom edge. Neither of those
workarounds is here any more, because a section owns the whole column. Credentials are the third kind and
would not have fitted at all.
Its own control rather than part of MainWindow, for two reasons that arrived together. It is the part of Its own control rather than part of MainWindow, for two reasons that arrived together. It is the part of
this window with a height budget tight enough to break — one editor open at a time is a state rule this window with a height budget tight enough to break, and it is the only part that can be laid out by a
standing in for a sizing guarantee — and it is the only part that can be laid out by a test, because test, because MainWindow cannot be shown headlessly at all: the terminal's WebView2 adapter initialises on
MainWindow cannot be shown headlessly at all: the terminal's WebView2 adapter initialises on attach and attach and refuses a non-STA thread, which a headless dispatcher is. See DodoSSH.Client.App.Layout.Tests,
refuses a non-STA thread, which a headless dispatcher is. See DodoSSH.Client.App.Layout.Tests. which measures one section at a time because that is all a user can see at once.
Two lists in one column rather than a TabControl. A TabControl is the tidier layout and it was not Buttons and a command rather than a TabControl, a TabStrip or a ListBox, and not for looks. All three hold
chosen because of what the terminal does with the keyboard: MainWindow releases focus to whichever list the selection themselves, so a click moves the highlight before the view model can refuse it — and this
is showing, and a tabbed version would put that target outside the visual tree when its tab is not column does refuse, while an editor is open. A selector lit on a section the column is not showing is
selected. This repository already has one measured finding of that shape — Focus() on a collapsed worse than the refusal it would be hiding. Buttons carry no state and cannot disagree with the vault.
control is a no-op and is not replayed when it is revealed — and whether an unselected TabItem behaves
the same way here is untested. Not worth finding out by shipping it, for a layout preference.
The host list keeps the flexible row, so it is what grows with the window; the key section takes what it It also keeps the keyboard working. MainWindow releases focus to VaultColumn.KeyboardTarget, and Focus()
needs and no more. on a collapsed control is measurably a no-op that is not replayed when the control is revealed — so the
target has to be a control that is on screen right now. A tabbed version would put the unselected list
outside the visual tree entirely; here it is one section switch away and the column knows which.
--> -->
<Grid RowDefinitions="*,Auto,Auto,Auto,Auto,Auto,Auto" Background="#131722">
<!-- Named because it is where keyboard focus lands when the user leaves the terminal. --> <UserControl.Styles>
<ListBox Grid.Row="0" x:Name="HostList" Margin="6" <!--
The selector's two states. The brushes go on the presenter rather than the button because that is where
the Fluent theme puts its own: a Background set on the button is a TemplateBinding the theme's
:pointerover rule overrides, so the accent would survive until the pointer arrived and then vanish.
-->
<Style Selector="StackPanel#SectionSelector > Button /template/ ContentPresenter#PART_ContentPresenter">
<Setter Property="Background" Value="Transparent" />
<Setter Property="Foreground" Value="#9aa4b6" />
<Setter Property="BorderBrush" Value="Transparent" />
</Style>
<Style Selector="StackPanel#SectionSelector > Button.active /template/ ContentPresenter#PART_ContentPresenter">
<Setter Property="Foreground" Value="#e6e9f0" />
<Setter Property="BorderBrush" Value="#5b7cfa" />
</Style>
<!-- Hover on the inactive one, so the row reads as clickable before anything is clicked. -->
<Style Selector="StackPanel#SectionSelector > Button:pointerover /template/ ContentPresenter#PART_ContentPresenter">
<Setter Property="Background" Value="#1c2230" />
</Style>
</UserControl.Styles>
<Grid RowDefinitions="Auto,*" Background="#131722">
<!--
Adding a kind is an enum member, one button here, and one section grid below. Deliberately not a
generated list: each section's contents are different markup, so a loop would buy nothing and cost the
compiled bindings.
-->
<Border Grid.Row="0" Background="#10141d" BorderBrush="#1c2230" BorderThickness="0,0,0,1">
<StackPanel x:Name="SectionSelector" Orientation="Horizontal">
<Button Content="Hosts" Padding="12,7" CornerRadius="0" BorderThickness="0,0,0,2"
Classes.active="{Binding ShowsHosts}"
Command="{Binding ShowSectionCommand}"
CommandParameter="{x:Static vm:VaultSection.Hosts}" />
<Button Content="SSH keys" Padding="12,7" CornerRadius="0" BorderThickness="0,0,0,2"
Classes.active="{Binding ShowsKeys}"
Command="{Binding ShowSectionCommand}"
CommandParameter="{x:Static vm:VaultSection.Keys}" />
<Button Content="Passwords" Padding="12,7" CornerRadius="0" BorderThickness="0,0,0,2"
Classes.active="{Binding ShowsCredentials}"
Command="{Binding ShowSectionCommand}"
CommandParameter="{x:Static vm:VaultSection.Credentials}" />
</StackPanel>
</Border>
<!--
The two sections share this row and take turns being visible. Same row rather than two rows with one
collapsed, so neither can reserve height from the other by accident.
The list keeps the flexible row in both, so it is what grows with the window and what shrinks when an
editor opens; the editor takes what it needs. The list stays visible while its editor is open, which the
key list did not manage before — and that is what keeps KeyboardTarget honest, since the control it
returns has to be one a Focus() call can actually reach.
-->
<!-- Hosts -->
<Grid Grid.Row="1" RowDefinitions="*,Auto,Auto" IsVisible="{Binding ShowsHosts}">
<!--
Named because it is where keyboard focus lands when the user leaves the terminal.
Focusable, which a ListBox is not by default — Avalonia leaves focus to the items and an empty list has
none. Without it MainWindow's release-the-keyboard path was a measured no-op: it took Win32 focus off
the terminal's child window and then called Focus() on something that refuses it, leaving the window
with nothing focused and the keystrokes going nowhere. Exactly the state the second half of
ReleaseKeyboardTo says it exists to prevent.
-->
<ListBox Grid.Row="0" x:Name="HostList" Margin="6" Focusable="True"
ItemsSource="{Binding Hosts}" ItemsSource="{Binding Hosts}"
SelectedItem="{Binding SelectedHost}" SelectedItem="{Binding SelectedHost}"
Background="Transparent"> Background="Transparent">
@@ -67,17 +138,27 @@
<TextBox Text="{Binding EditorNotes}" PlaceholderText="notes" AcceptsReturn="True" <TextBox Text="{Binding EditorNotes}" PlaceholderText="notes" AcceptsReturn="True"
Height="60" TextWrapping="Wrap" /> Height="60" TextWrapping="Wrap" />
<!-- <!--
Which key this host authenticates with, or a password. Part of the host rather than of the How this host authenticates: a typed password, one of the vault's keys, or one of its credentials.
connection, so it follows the host to every machine; a host bound to a key that has since been Part of the host rather than of the connection, so it follows the host to every machine; a host
deleted keeps a placeholder entry here, so that editing the port cannot quietly turn it back bound to something since deleted keeps a placeholder entry here, so that editing the port cannot
into a password host. quietly turn it back into a typed-password host.
One control for all three, which is what makes "a key or a credential, never both" impossible to
express rather than merely invalid. The qualifier beside each label is not decoration: a key called
"deploy" and the deploy account's password are the ordinary case, and bare labels would offer two
identical-looking entries that authenticate completely differently.
--> -->
<ComboBox ItemsSource="{Binding EditorKeyChoices}" <ComboBox ItemsSource="{Binding EditorAuthenticationChoices}"
SelectedItem="{Binding EditorSelectedKey}" SelectedItem="{Binding EditorSelectedAuthentication}"
HorizontalAlignment="Stretch"> HorizontalAlignment="Stretch">
<ComboBox.ItemTemplate> <ComboBox.ItemTemplate>
<DataTemplate x:DataType="vm:SshKeyChoice"> <DataTemplate x:DataType="vm:AuthenticationChoice">
<StackPanel Orientation="Horizontal" Spacing="6">
<TextBlock Text="{Binding Label}" /> <TextBlock Text="{Binding Label}" />
<TextBlock Text="{Binding Qualifier}" Classes="hint" FontSize="11"
VerticalAlignment="Center"
IsVisible="{Binding Qualifier, Converter={x:Static StringConverters.IsNotNullOrEmpty}}" />
</StackPanel>
</DataTemplate> </DataTemplate>
</ComboBox.ItemTemplate> </ComboBox.ItemTemplate>
</ComboBox> </ComboBox>
@@ -99,9 +180,6 @@
somewhere deliberate to be re-approved from, and that somewhere must not be the warning somewhere deliberate to be re-approved from, and that somewhere must not be the warning
itself. It takes effect when clicked rather than on Save, and the status line says so; it is itself. It takes effect when clicked rather than on Save, and the status line says so; it is
not a field of the host. not a field of the host.
Added to this row rather than as a row of its own on purpose: this column's editors already
only just fit at the window's minimum height, which is why only one may be open at a time.
--> -->
<Button Content="Forget host key" Command="{Binding ForgetHostKeyCommand}" <Button Content="Forget host key" Command="{Binding ForgetHostKeyCommand}"
IsVisible="{Binding CanForgetHostKey}" IsVisible="{Binding CanForgetHostKey}"
@@ -117,21 +195,16 @@
<Button Content="Delete" Command="{Binding DeleteHostCommand}" /> <Button Content="Delete" Command="{Binding DeleteHostCommand}" />
</StackPanel> </StackPanel>
<Border Grid.Row="3" Padding="8,6" Background="#10141d"> </Grid>
<TextBlock Text="SSH keys" Foreground="#9aa4b6" FontSize="11" FontWeight="SemiBold" />
</Border>
<!-- <!-- SSH keys -->
Bounded rather than flexible, and hidden while its editor is open. The key editor is the tallest <Grid Grid.Row="1" RowDefinitions="*,Auto,Auto" IsVisible="{Binding ShowsKeys}">
thing in this column — a private key needs a real text area — and at the window's minimum height
there is not room for both. Browsing the list and editing one of its rows are not things anyone <!-- The other place the keyboard can land, and focusable for the same reason; see KeyboardTarget. -->
needs to do at the same moment. <ListBox Grid.Row="0" x:Name="KeyList" Margin="6" Focusable="True"
-->
<ListBox Grid.Row="4" x:Name="KeyList" Margin="6" MaxHeight="170"
ItemsSource="{Binding Keys}" ItemsSource="{Binding Keys}"
SelectedItem="{Binding SelectedKey}" SelectedItem="{Binding SelectedKey}"
Background="Transparent" Background="Transparent">
IsVisible="{Binding !IsEditingKey}">
<ListBox.ItemTemplate> <ListBox.ItemTemplate>
<DataTemplate x:DataType="vm:SshKeyRowViewModel"> <DataTemplate x:DataType="vm:SshKeyRowViewModel">
<StackPanel Spacing="2" Margin="2,4"> <StackPanel Spacing="2" Margin="2,4">
@@ -153,7 +226,7 @@
</ListBox.ItemTemplate> </ListBox.ItemTemplate>
</ListBox> </ListBox>
<Border Grid.Row="5" Padding="10" Background="#171b24" <Border Grid.Row="1" Padding="10" Background="#171b24"
IsVisible="{Binding IsEditingKey}"> IsVisible="{Binding IsEditingKey}">
<StackPanel Spacing="6"> <StackPanel Spacing="6">
<TextBox Text="{Binding KeyEditorLabel}" PlaceholderText="name" /> <TextBox Text="{Binding KeyEditorLabel}" PlaceholderText="name" />
@@ -181,7 +254,7 @@
</StackPanel> </StackPanel>
</Border> </Border>
<StackPanel Grid.Row="6" Orientation="Horizontal" Spacing="6" Margin="8,4,8,8" <StackPanel Grid.Row="2" Orientation="Horizontal" Spacing="6" Margin="8,4,8,8"
IsVisible="{Binding !IsEditingKey}"> IsVisible="{Binding !IsEditingKey}">
<Button Content="Add key" Command="{Binding NewKeyCommand}" /> <Button Content="Add key" Command="{Binding NewKeyCommand}" />
<Button Content="Edit" Command="{Binding EditSelectedKeyCommand}" /> <Button Content="Edit" Command="{Binding EditSelectedKeyCommand}" />
@@ -190,4 +263,72 @@
</Grid> </Grid>
<!-- Stored passwords -->
<Grid Grid.Row="1" RowDefinitions="*,Auto,Auto" IsVisible="{Binding ShowsCredentials}">
<!-- The third place the keyboard can land; see KeyboardTarget. -->
<ListBox Grid.Row="0" x:Name="CredentialList" Margin="6" Focusable="True"
ItemsSource="{Binding Credentials}"
SelectedItem="{Binding SelectedCredential}"
Background="Transparent">
<ListBox.ItemTemplate>
<DataTemplate x:DataType="vm:CredentialRowViewModel">
<StackPanel Spacing="2" Margin="2,4">
<StackPanel Orientation="Horizontal" Spacing="6">
<TextBlock Text="{Binding Label}" Foreground="#e6e9f0" FontWeight="SemiBold" />
<Border Background="#2b2410" CornerRadius="3" Padding="4,0"
IsVisible="{Binding Badge, Converter={x:Static StringConverters.IsNotNullOrEmpty}}">
<TextBlock Text="{Binding Badge}" Foreground="#e8dcb0" FontSize="10"
VerticalAlignment="Center" />
</Border>
</StackPanel>
<!--
The account, never the password. The same rule as the key list, and the same reason: a
password bound into a list item's visual tree is one a tooltip or a screen reader can read out.
-->
<TextBlock Text="{Binding Description}" Classes="hint" FontSize="11" />
</StackPanel>
</DataTemplate>
</ListBox.ItemTemplate>
</ListBox>
<Border Grid.Row="1" Padding="10" Background="#171b24"
IsVisible="{Binding IsEditingCredential}">
<StackPanel Spacing="6">
<TextBox Text="{Binding CredentialEditorLabel}" PlaceholderText="name" />
<!--
Optional, and the reason a credential is worth being its own item rather than two more fields on a
host: one account on twenty machines is described once and rotated once. Left blank, each host
supplies its own username and only the password is shared.
-->
<TextBox Text="{Binding CredentialEditorUsername}"
PlaceholderText="username (blank: use each host's own)" />
<!--
Masked, unlike the private key box, and the difference is not inconsistency. A key's armour has to
be visible to be checked for truncation after a paste; a password is short, usually typed, and
shoulder-surfing is the likelier problem.
-->
<TextBox Text="{Binding CredentialEditorPassword}" PlaceholderText="password" PasswordChar="•" />
<TextBox Text="{Binding CredentialEditorNotes}" PlaceholderText="notes" AcceptsReturn="True"
Height="48" TextWrapping="Wrap" />
<TextBlock Classes="hint" FontSize="11"
Text="Encrypted here and never sent to the server in a form it can read. Bind a host to this in the host's own editor, under how it authenticates." />
<StackPanel Orientation="Horizontal" Spacing="8">
<Button Content="Save" Command="{Binding SaveCredentialCommand}" />
<Button Content="Cancel" Command="{Binding CancelCredentialEditCommand}" />
</StackPanel>
</StackPanel>
</Border>
<StackPanel Grid.Row="2" Orientation="Horizontal" Spacing="6" Margin="8,4,8,8"
IsVisible="{Binding !IsEditingCredential}">
<Button Content="Add password" Command="{Binding NewCredentialCommand}" />
<Button Content="Edit" Command="{Binding EditSelectedCredentialCommand}" />
<Button Content="Delete" Command="{Binding DeleteCredentialCommand}" />
</StackPanel>
</Grid>
</Grid>
</UserControl> </UserControl>
@@ -1,5 +1,6 @@
using Avalonia.Controls; using Avalonia.Controls;
using Avalonia.Input; using Avalonia.Input;
using DodoSSH.Client.App.ViewModels;
namespace DodoSSH.Client.App.Views; namespace DodoSSH.Client.App.Views;
@@ -18,10 +19,29 @@ internal sealed partial class VaultColumn : UserControl
/// Where the keyboard should land when the terminal hands it back. /// Where the keyboard should land when the terminal hands it back.
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// <para>
/// Exposed as a property rather than left for the window to find by name, because the name is now inside /// Exposed as a property rather than left for the window to find by name, because the name is now inside
/// this control's template and the window cannot see it. Which is the better arrangement anyway: when /// this control's template and the window cannot see it. Which is the better arrangement anyway: the
/// this column shows one list at a time, "the list the keyboard belongs to" is a question only the column /// column shows one list at a time, so "the list the keyboard belongs to" is a question only the column
/// can answer, and answering it here means the window never has to know how many lists there are. /// can answer, and answering it here means the window never has to know how many lists there are.
/// </para>
/// <para>
/// It has to be the list that is on screen, not a fixed one. <c>Focus()</c> on a collapsed control is
/// measurably a no-op and is not replayed when the control is revealed, so returning the host list while
/// the keys section is showing would swallow the keyboard: the terminal would let go and nothing would
/// take it.
/// </para>
/// <para>
/// Read from the view model rather than from the controls' own <c>IsVisible</c>, because that is the
/// direction the truth flows — the section is the state and the visibility is a binding to it. Asking the
/// controls would answer the same question one indirection later, and would answer it wrongly for the
/// moment between a section change and the next layout pass.
/// </para>
/// </remarks> /// </remarks>
internal IInputElement KeyboardTarget => HostList; internal IInputElement KeyboardTarget => DataContext switch
{
VaultViewModel { ShowsKeys: true } => KeyList,
VaultViewModel { ShowsCredentials: true } => CredentialList,
_ => HostList,
};
} }
@@ -16,10 +16,16 @@ namespace DodoSSH.Client.App.Layout.Tests;
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// <para> /// <para>
/// The column is 340 pixels wide and holds a list and an editor per item type, and the only thing keeping it /// The column is 340 pixels wide and holds a list and an editor per item type, of which it shows one type at a
/// from clipping its own Save button at the window's minimum height is a state rule — one editor open at a /// time. This suite is the measurement behind that arrangement: the column used to stack both types and keep
/// time. That rule was added on the strength of an argument, not a measurement, and this suite is the /// itself from clipping its own Save button with a state rule — one editor open at a time — and that rule was
/// measurement. /// added on the strength of an argument. The argument was right about the stacked column and is now moot,
/// which is a thing this suite found rather than assumed. See
/// <see cref="BothEditorsOpen_NowFit_BecauseOnlyOneSectionIsLaidOut" />.
/// </para>
/// <para>
/// One test per section, and one per section with its editor open, because that is the full set of shapes a
/// user can put this column into. A third section will add two more.
/// </para> /// </para>
/// <para> /// <para>
/// A real <c>VaultViewModel</c> over a real unlocked vault, rather than a stand-in. Compiled bindings resolve /// A real <c>VaultViewModel</c> over a real unlocked vault, rather than a stand-in. Compiled bindings resolve
@@ -87,13 +93,13 @@ public sealed class VaultColumnLayoutTests : IAsyncLifetime
} }
[Fact] [Fact]
public async Task TheColumnFitsWithNoEditorOpen() public async Task TheHostsSectionFitsWithNoEditorOpen()
{ {
await MeasureAsync(faults => faults.ShouldBeEmpty()); await MeasureAsync(faults => faults.ShouldBeEmpty());
} }
[Fact] [Fact]
public async Task TheColumnFitsWithTheHostEditorOpen() public async Task TheHostsSectionFitsWithItsEditorOpen()
{ {
vault.NewHostCommand.Execute(null); vault.NewHostCommand.Execute(null);
vault.IsEditing.ShouldBeTrue(); vault.IsEditing.ShouldBeTrue();
@@ -102,12 +108,23 @@ public sealed class VaultColumnLayoutTests : IAsyncLifetime
} }
[Fact] [Fact]
public async Task TheColumnFitsWithTheKeyEditorOpen() public async Task TheKeysSectionFitsWithNoEditorOpen()
{ {
// The tall one: a private key needs a real text area, and this is the editor the MaxHeight on the key vault.ShowSectionCommand.Execute(VaultSection.Keys);
// list exists to make room for. vault.ShowsKeys.ShouldBeTrue();
await MeasureAsync(faults => faults.ShouldBeEmpty());
}
[Fact]
public async Task TheKeysSectionFitsWithItsEditorOpen()
{
// The tall one: a private key needs a real text area, and this editor is what the key list used to
// hide itself and cap its own height for. Both workarounds are gone, so this measurement is now the
// only thing saying they were not needed.
vault.NewKeyCommand.Execute(null); vault.NewKeyCommand.Execute(null);
vault.IsEditingKey.ShouldBeTrue(); vault.IsEditingKey.ShouldBeTrue();
vault.ShowsKeys.ShouldBeTrue("opening an editor has to bring its own section into view");
vault.KeyEditorPrivateKey = string.Join( vault.KeyEditorPrivateKey = string.Join(
'\n', '\n',
@@ -117,35 +134,205 @@ public sealed class VaultColumnLayoutTests : IAsyncLifetime
} }
[Fact] [Fact]
public async Task BothEditorsAtOnce_DoNotFit_WhichIsWhyTheRuleExists() public async Task TheCredentialsSectionFitsWithNoEditorOpen()
{ {
// The justification for KeyEditorIsInTheWay/HostEditorIsInTheWay, turned from an argument in a comment vault.ShowSectionCommand.Execute(VaultSection.Credentials);
// into a number. The flags are set directly because the commands refuse this on purpose — the point is vault.ShowsCredentials.ShouldBeTrue();
// to measure what the refusal is protecting.
await MeasureAsync(faults => faults.ShouldBeEmpty());
}
[Fact]
public async Task TheCredentialsSectionFitsWithItsEditorOpen()
{
vault.NewCredentialCommand.Execute(null);
vault.IsEditingCredential.ShouldBeTrue();
vault.ShowsCredentials.ShouldBeTrue("opening an editor has to bring its own section into view");
await MeasureAsync(faults => faults.ShouldBeEmpty());
}
/// <remarks>
/// The host editor is the one a third item type made taller: its authentication picker is now a ComboBox
/// with a two-line-capable item template, and the section it sits in is the only one holding a
/// <c>NumericUpDown</c>, a <c>CheckBox</c> and two paragraphs of hint text. Measured with the picker
/// populated, because an empty ComboBox is shorter than one showing a qualifier beside a label.
/// </remarks>
[Fact]
public async Task TheHostEditorFitsWithTheAuthenticationPickerFull()
{
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.EditorAuthenticationChoices.Count
.ShouldBeGreaterThan(1, "the picker has to be populated for this to measure anything");
vault.EditorSelectedAuthentication = vault.EditorAuthenticationChoices
.First(choice => choice.Kind is AuthenticationKind.Credential);
await MeasureAsync(faults => faults.ShouldBeEmpty());
}
[Fact]
public async Task BothEditorsOpen_NowFit_BecauseOnlyOneSectionIsLaidOut()
{
// This test used to assert the opposite, and its own comment said that if it ever started passing the
// rule it justified had become unnecessary. That has happened, and this is the record of it: the two
// editors are in different sections now and only one section is laid out, so the sizing argument for
// one-editor-at-a-time is dead.
// //
// If this test ever starts passing, the rule has become unnecessary and the comments claiming it is // The rule itself is not, and AnEditorIsInTheWay says why — an open key editor holds a pasted private
// load-bearing have become false. That is a finding, not a flake: read it as "the column has room // key, and moving on would leave it in a form nobody can see. That is a state rule with a state
// now", check what changed, and delete the rule rather than this test. // reason, so it belongs in the shell's tests and not here. This suite's job was the sizing claim, and
// the honest thing to do with a measurement that has flipped is to keep measuring it.
vault.IsEditing = true; vault.IsEditing = true;
vault.IsEditingKey = true; vault.IsEditingKey = true;
await MeasureAsync(faults => faults.ShouldNotBeEmpty( await MeasureAsync(faults => faults.ShouldBeEmpty(
"one editor at a time is a workaround for a column that cannot hold two")); "one section at a time means two open editors are never laid out together"));
vault.Section = VaultSection.Keys;
await MeasureAsync(faults => faults.ShouldBeEmpty(
"and the same holds from the other side, where the taller editor is the visible one"));
}
/// <remarks>
/// <para>
/// The one thing a wrong answer here breaks is unrecoverable from the keyboard: <c>MainWindow</c> takes the
/// keyboard off the terminal's native child window first and then focuses this target, so a target that
/// cannot take focus leaves the user with no focused element and no way back except the mouse.
/// </para>
/// <para>
/// Which is why this asserts that focus was <i>taken</i> rather than that the right control was named.
/// Naming is the cheap half and it was already right; taking it was not — a <c>ListBox</c> is not focusable
/// by default, so this call returned false against the column as it stood and the shipped release-the-
/// keyboard path did nothing. Two ways to fail, and only the assertion that runs the call sees both: a
/// control in the section that is not showing is collapsed, and <c>Focus()</c> on a collapsed control is a
/// no-op that is not replayed when it is revealed.
/// </para>
/// </remarks>
[Fact]
public async Task TheKeyboardTargetIsTheListThatIsOnScreenAndItTakesFocus()
{
await OnTheColumnAsync((column, _) =>
{
column.KeyboardTarget.ShouldBeSameAs(column.HostList);
column.KeyboardTarget.Focus().ShouldBeTrue("the hosts section is showing");
});
vault.ShowSectionCommand.Execute(VaultSection.Keys);
await OnTheColumnAsync((column, _) =>
{
column.KeyboardTarget.ShouldBeSameAs(column.KeyList);
column.KeyboardTarget.Focus().ShouldBeTrue("the keys section is showing");
});
vault.ShowSectionCommand.Execute(VaultSection.Credentials);
await OnTheColumnAsync((column, _) =>
{
column.KeyboardTarget.ShouldBeSameAs(column.CredentialList);
column.KeyboardTarget.Focus().ShouldBeTrue("the credentials section is showing");
});
}
/// <remarks>
/// The same call in the state the section rule allows: an editor open, its own list still on screen behind
/// it. The key list used to collapse itself whenever its editor opened, so a target that followed the
/// section would have been a no-op in exactly the state a user is most likely to leave the terminal in.
/// </remarks>
[Fact]
public async Task TheKeyboardTargetStillTakesFocusWithAnEditorOpen()
{
vault.NewKeyCommand.Execute(null);
await OnTheColumnAsync((column, _) =>
{
column.KeyList.IsEffectivelyVisible.ShouldBeTrue();
column.KeyboardTarget.Focus().ShouldBeTrue();
});
}
/// <remarks>
/// The claim the whole arrangement rests on, and the one nothing else here would notice breaking: two
/// sections left visible at once would overlap in the row they share rather than clip, so every fit test
/// above would still pass while the column showed one list through another.
/// </remarks>
[Fact]
public async Task OnlyOneSectionIsOnScreenAtOnce()
{
await AssertOnlyVisibleAsync(VaultSection.Hosts);
await AssertOnlyVisibleAsync(VaultSection.Keys);
await AssertOnlyVisibleAsync(VaultSection.Credentials);
}
/// <summary>Shows one section and checks that it is the only one a user can see.</summary>
private async Task AssertOnlyVisibleAsync(VaultSection section)
{
vault.Section = section;
await OnTheColumnAsync((column, _) =>
{
var lists = new Dictionary<VaultSection, ListBox>
{
[VaultSection.Hosts] = column.HostList,
[VaultSection.Keys] = column.KeyList,
[VaultSection.Credentials] = column.CredentialList,
};
foreach (var (owner, list) in lists)
{
list.IsEffectivelyVisible.ShouldBe(
owner == section,
$"{owner} showing while {section} is selected");
}
});
}
/// <remarks>
/// The selector is the only way to reach a section, so a click that lands on nothing is a column with one
/// half of it walled off. Its buttons are covered by every fit test above — the harness treats a
/// <see cref="Button"/> as interactive — but that only proves they are inside the window. This proves they
/// are the size a pointer can find, which a zero-height row of buttons in a collapsed border would not be.
/// </remarks>
[Fact]
public async Task TheSelectorIsBigEnoughToClick()
{
await OnTheColumnAsync((column, _) =>
{
var buttons = column.SectionSelector.Children.OfType<Button>().ToList();
buttons.Count.ShouldBe(3, "one per section that exists");
foreach (var button in buttons)
{
button.Bounds.Height.ShouldBeGreaterThan(20);
button.Bounds.Width.ShouldBeGreaterThan(40);
}
});
} }
/// <summary>Lays the column out at the size the window gives it and hands the faults to an assertion.</summary> /// <summary>Lays the column out at the size the window gives it and hands the faults to an assertion.</summary>
private Task MeasureAsync(Action<IReadOnlyList<string>> assert) => private Task MeasureAsync(Action<IReadOnlyList<string>> assert) =>
OnTheColumnAsync((_, window) => assert(LayoutHarness.Unreachable(window)));
/// <summary>Shows the column at the size the window gives it and runs one body against it.</summary>
private Task OnTheColumnAsync(Action<VaultColumn, Window> body) =>
LayoutHarness.OnTheUiThreadAsync( LayoutHarness.OnTheUiThreadAsync(
() => () =>
{ {
var column = new VaultColumn { DataContext = vault };
var window = LayoutHarness.HostAtMinimumSize( var window = LayoutHarness.HostAtMinimumSize(
new VaultColumn { DataContext = vault }, column,
LayoutHarness.VaultColumnWidth, LayoutHarness.VaultColumnWidth,
LayoutHarness.VaultColumnHeight); LayoutHarness.VaultColumnHeight);
try try
{ {
assert(LayoutHarness.Unreachable(window)); body(column, window);
} }
finally finally
{ {
@@ -155,8 +342,9 @@ public sealed class VaultColumnLayoutTests : IAsyncLifetime
Token); Token);
/// <remarks> /// <remarks>
/// Enough rows that the lists are not empty, because an empty list is the easiest case and the one least /// Enough rows in both lists that neither is empty, because an empty list is the easiest case and the one
/// worth certifying. /// least worth certifying — and since the selector arrived, the keys section has a whole column of its own
/// to fill rather than a capped strip at the bottom of the hosts one.
/// </remarks> /// </remarks>
private async Task SeedAsync() private async Task SeedAsync()
{ {
@@ -169,6 +357,27 @@ public sealed class VaultColumnLayoutTests : IAsyncLifetime
await vault.SaveHostCommand.ExecuteAsync(null); await vault.SaveHostCommand.ExecuteAsync(null);
} }
for (var i = 0; i < 4; i++)
{
vault.NewKeyCommand.Execute(null);
vault.KeyEditorLabel = $"key-{i}";
vault.KeyEditorPrivateKey =
$"-----BEGIN OPENSSH PRIVATE KEY-----\nMATERIAL-{i}\n-----END OPENSSH PRIVATE KEY-----\n";
await vault.SaveKeyCommand.ExecuteAsync(null);
}
for (var i = 0; i < 3; i++)
{
vault.NewCredentialCommand.Execute(null);
vault.CredentialEditorLabel = $"credential-{i}";
vault.CredentialEditorPassword = $"password-{i}";
vault.CredentialEditorUsername = $"account-{i}";
await vault.SaveCredentialCommand.ExecuteAsync(null);
}
// Back to where the column opens, so every test starts from the state a user would see.
vault.Section = VaultSection.Hosts;
await vault.LoadAsync(Token); await vault.LoadAsync(Token);
} }
} }
+548 -11
View File
@@ -995,12 +995,125 @@ public sealed class ShellFlowTests : IAsyncLifetime
vault.PendingChanges.ShouldBe(0); vault.PendingChanges.ShouldBe(0);
} }
// ---- One kind of item at a time ----
[Fact]
public async Task TheColumnOpensOnHostsAndTheSelectorMovesBetweenSections()
{
await UnlockedAsync();
var vault = shell.Vault!;
// Hosts, because connecting is what somebody who has just unlocked a vault came to do. Keys and
// credentials exist to make that work, and neither is where the first click belongs.
vault.Section.ShouldBe(VaultSection.Hosts);
vault.ShowsHosts.ShouldBeTrue();
vault.ShowsKeys.ShouldBeFalse();
vault.ShowSectionCommand.Execute(VaultSection.Keys);
vault.ShowsKeys.ShouldBeTrue();
vault.ShowsHosts.ShouldBeFalse("both flags are one fact read two ways and cannot both be true");
vault.ShowSectionCommand.Execute(VaultSection.Hosts);
vault.ShowsHosts.ShouldBeTrue();
}
/// <remarks> /// <remarks>
/// A layout invariant expressed as a state one, because it is the only form of it this repository can /// The invariant that makes an unreachable editor impossible: an editor is only ever open in the section
/// check: nothing here loads a <c>.axaml</c>, and both editors are <c>Auto</c> rows in the same /// that is showing. Without it, adding a key from a keyboard shortcut or a future menu would open an
/// 340-pixel column whose combined height exceeds the column at the window's minimum size. Two open /// editor nobody can see, holding a private key nobody can cancel.
/// editors put the lower one's buttons past the bottom edge — the same failure this window shipped once /// </remarks>
/// already, with the setup screens sliced and unclickable. [Fact]
public async Task OpeningAnEditorBringsItsOwnSectionIntoView()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddHostAsync(vault, "prod-db");
await AddKeyAsync(vault, "deploy");
vault.Section = VaultSection.Hosts;
vault.NewKeyCommand.Execute(null);
vault.ShowsKeys.ShouldBeTrue("the key editor cannot be open in the hosts section");
vault.CancelKeyEditCommand.Execute(null);
vault.NewHostCommand.Execute(null);
vault.ShowsHosts.ShouldBeTrue();
vault.CancelEditCommand.Execute(null);
// And through the other door into each editor.
vault.SelectedKey = vault.Keys[0];
vault.EditSelectedKeyCommand.Execute(null);
vault.ShowsKeys.ShouldBeTrue();
vault.CancelKeyEditCommand.Execute(null);
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.ShowsHosts.ShouldBeTrue();
}
/// <remarks>
/// The refusal that keeps the rule above true. Leaving the section while an editor is open would hide it,
/// and in the key editor's case that means a pasted private key sitting in a form with nothing on screen
/// to say it is there.
/// </remarks>
[Fact]
public async Task SwitchingSectionIsRefusedWhileAnEditorIsOpen()
{
await UnlockedAsync();
var vault = shell.Vault!;
vault.NewKeyCommand.Execute(null);
vault.KeyEditorPrivateKey = PrivateKey("PASTED-AND-NOWHERE-ELSE");
vault.ShowSectionCommand.Execute(VaultSection.Hosts);
vault.ShowsKeys.ShouldBeTrue("the selector must not move away from an open editor");
vault.Status.ShouldContain("SSH key");
vault.KeyEditorPrivateKey.ShouldBe(PrivateKey("PASTED-AND-NOWHERE-ELSE"));
// A refusal, not a lockout: dealing with the editor releases the selector.
vault.CancelKeyEditCommand.Execute(null);
vault.ShowSectionCommand.Execute(VaultSection.Hosts);
vault.ShowsHosts.ShouldBeTrue();
// Symmetrically, and with the message naming the editor that is actually open — which matters more
// here than it used to, because the thing to go back to may not be the section on screen.
vault.NewHostCommand.Execute(null);
vault.EditorLabel = "half-typed";
vault.ShowSectionCommand.Execute(VaultSection.Keys);
vault.ShowsHosts.ShouldBeTrue();
vault.Status.ShouldContain("host");
vault.EditorLabel.ShouldBe("half-typed");
}
/// <remarks>
/// Asking for the section that is already showing is not a refusal, so a second click on the selected
/// button while an editor is open says nothing. Worth pinning because the obvious implementation — check
/// the editor, then compare — would scold somebody for clicking where they already are.
/// </remarks>
[Fact]
public async Task ReselectingTheSectionAlreadyShowingSaysNothing()
{
await UnlockedAsync();
var vault = shell.Vault!;
vault.NewHostCommand.Execute(null);
vault.Status = string.Empty;
vault.ShowSectionCommand.Execute(VaultSection.Hosts);
vault.Status.ShouldBeEmpty();
vault.IsEditing.ShouldBeTrue();
}
/// <remarks>
/// One editor at a time, still — but no longer for the reason it was introduced for. Both editors used to
/// be <c>Auto</c> rows in one 340-pixel column whose combined height exceeded it; sections ended that, and
/// the layout suite now measures two open editors fitting. What the rule buys today is that an open key
/// editor is always one somebody can see, because it is holding their private key.
/// </remarks> /// </remarks>
[Fact] [Fact]
public async Task OnlyOneEditorOpensAtATime_AndTheRefusalKeepsWhatWasTyped() public async Task OnlyOneEditorOpensAtATime_AndTheRefusalKeepsWhatWasTyped()
@@ -1084,8 +1197,9 @@ public sealed class ShellFlowTests : IAsyncLifetime
vault.SelectedHost = vault.Hosts[0]; vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null); vault.EditSelectedHostCommand.Execute(null);
vault.EditorSelectedKey.ShouldNotBeNull().EntityId.ShouldBe(keyId); vault.EditorSelectedAuthentication.ShouldNotBeNull().EntityId.ShouldBe(keyId);
vault.EditorKeyChoices[0].EntityId.ShouldBeNull("the password entry stays first"); vault.EditorAuthenticationChoices[0].Kind
.ShouldBe(AuthenticationKind.Typed, "the typed-password entry stays first");
} }
[Fact] [Fact]
@@ -1187,8 +1301,9 @@ public sealed class ShellFlowTests : IAsyncLifetime
vault.EditSelectedHostCommand.Execute(null); vault.EditSelectedHostCommand.Execute(null);
// The binding is still there, still selected, and says what is wrong with it. // The binding is still there, still selected, and says what is wrong with it.
var selected = vault.EditorSelectedKey.ShouldNotBeNull(); var selected = vault.EditorSelectedAuthentication.ShouldNotBeNull();
selected.EntityId.ShouldBe(keyId); selected.EntityId.ShouldBe(keyId);
selected.Kind.ShouldBe(AuthenticationKind.SshKey, "a missing key must not come back as a credential");
selected.Label.ShouldContain("no longer here"); selected.Label.ShouldContain("no longer here");
vault.EditorPort = 2244; vault.EditorPort = 2244;
@@ -1207,7 +1322,8 @@ public sealed class ShellFlowTests : IAsyncLifetime
vault.SelectedHost = vault.Hosts[0]; vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null); vault.EditSelectedHostCommand.Execute(null);
vault.EditorSelectedKey = vault.EditorKeyChoices.Single(choice => choice.EntityId is null); vault.EditorSelectedAuthentication = vault.EditorAuthenticationChoices
.Single(choice => choice.Kind is AuthenticationKind.Typed);
await vault.SaveHostCommand.ExecuteAsync(null); await vault.SaveHostCommand.ExecuteAsync(null);
vault.Hosts.ShouldHaveSingleItem().Host.SshKeyId.ShouldBeNull(); vault.Hosts.ShouldHaveSingleItem().Host.SshKeyId.ShouldBeNull();
@@ -1219,6 +1335,397 @@ public sealed class ShellFlowTests : IAsyncLifetime
ssh.Requests.ShouldHaveSingleItem().Credential.ShouldBeOfType<SshPasswordCredential>(); ssh.Requests.ShouldHaveSingleItem().Credential.ShouldBeOfType<SshPasswordCredential>();
} }
// ---- Stored credentials ----
[Fact]
public async Task ACredentialRoundTripsThroughTheEditorAndTheVault()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddCredentialAsync(vault, "prod deploy", password: "s3cret", username: "deploy");
var row = vault.Credentials.ShouldHaveSingleItem();
row.Label.ShouldBe("prod deploy");
row.Description.ShouldBe("deploy", "the account is what the row has to show");
// Through the server and back, which is the whole point of storing it in the vault rather than on the
// machine that typed it.
await vault.SyncCommand.ExecuteAsync(null);
await vault.LoadAsync(Token);
vault.Credentials.ShouldHaveSingleItem().Credential.Password.ShouldBe("s3cret");
vault.SelectedCredential = vault.Credentials[0];
vault.EditSelectedCredentialCommand.Execute(null);
vault.CredentialEditorPassword.ShouldBe(
"s3cret", "the editor has to load it, because saving re-encodes every field");
vault.CredentialEditorUsername.ShouldBe("deploy");
}
[Fact]
public async Task ACredentialWithNoUsername_SaysItUsesTheHosts()
{
// Blank and absent are one state — CredentialSecret normalises them — and the row has to say which of
// the two things a blank box means, because "no username" and "the host's username" are not the same
// statement and only one of them is true.
await UnlockedAsync();
var vault = shell.Vault!;
await AddCredentialAsync(vault, "shared password", username: " ");
vault.Credentials.ShouldHaveSingleItem().Credential.Username.ShouldBeNull();
vault.Credentials[0].Description.ShouldBe("uses each host's own username");
}
[Fact]
public async Task ACredentialWithNoPassword_IsRefusedAndTheEditorStaysOpen()
{
await UnlockedAsync();
var vault = shell.Vault!;
vault.NewCredentialCommand.Execute(null);
vault.CredentialEditorLabel = "prod deploy";
await vault.SaveCredentialCommand.ExecuteAsync(null);
vault.Status.ShouldContain("password");
vault.Credentials.ShouldBeEmpty();
vault.IsEditingCredential.ShouldBeTrue("the editor stays open so it can be filled in");
}
[Fact]
public async Task CancellingTheCredentialEditor_LeavesNoPasswordBehindInIt()
{
// The same rule as the key editor, for the same reason: the editor holds the secret in a bound property
// for as long as it is open, and an abandoned editor that kept it would hand it to whatever opened next.
await UnlockedAsync();
var vault = shell.Vault!;
vault.NewCredentialCommand.Execute(null);
vault.CredentialEditorLabel = "prod deploy";
vault.CredentialEditorPassword = "ABANDONED";
vault.CancelCredentialEditCommand.Execute(null);
vault.IsEditingCredential.ShouldBeFalse();
vault.CredentialEditorPassword.ShouldBeEmpty();
vault.CredentialEditorLabel.ShouldBeEmpty();
vault.Credentials.ShouldBeEmpty();
}
[Fact]
public async Task DeletingACredential_RemovesItLocallyAndPushesTheTombstone()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddCredentialAsync(vault, "prod deploy");
vault.SelectedCredential = vault.Credentials[0];
await vault.DeleteCredentialCommand.ExecuteAsync(null);
vault.Credentials.ShouldBeEmpty();
server.LiveRowCount.ShouldBe(0);
vault.PendingChanges.ShouldBe(0);
}
/// <remarks>
/// A reload must not <em>invent</em> a selection, which is a safety property rather than a tidiness one:
/// Delete acts on the selection, so a list that fell back to its first row would put a one-click deletion of
/// somebody's password behind a button they never aimed. It does keep an existing selection, exactly as the
/// host list does — losing it on every background sync would move the target out from under the user.
/// </remarks>
[Fact]
public async Task ReloadingKeepsACredentialSelectionButNeverInventsOne()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddCredentialAsync(vault, "prod deploy");
// Saving selects what was just saved, which is wanted, and a reload has to leave it alone.
vault.SelectedCredential.ShouldNotBeNull();
await vault.LoadAsync(Token);
vault.SelectedCredential.ShouldNotBeNull();
// Nothing selected is the state Delete must find nothing in.
vault.SelectedCredential = null;
await vault.LoadAsync(Token);
vault.SelectedCredential.ShouldBeNull();
await vault.DeleteCredentialCommand.ExecuteAsync(null);
vault.Credentials.ShouldHaveSingleItem();
}
[Fact]
public async Task TheVaultSummaryCountsCredentialsToo()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddHostAsync(vault, "prod-db");
await AddKeyAsync(vault, "deploy");
await AddCredentialAsync(vault, "prod deploy");
await vault.LoadAsync(Token);
vault.Status.ShouldBe("1 host(s), 1 key(s), 1 credential(s) in Personal.");
// And a kind with nothing in it is left out rather than reported as zero.
vault.SelectedCredential = vault.Credentials[0];
await vault.DeleteCredentialCommand.ExecuteAsync(null);
await vault.LoadAsync(Token);
vault.Status.ShouldBe("1 host(s), 1 key(s) in Personal.");
}
/// <remarks>
/// What decides whether the terminal column shows a password box at all. Three states and only one of them
/// wants typing, so this is the property that keeps a box from appearing on a host that has no use for one
/// — and keeps the sentence in its place from claiming the wrong reason.
/// </remarks>
[Fact]
public async Task ThePasswordBoxOnlyAppearsForAHostThatWillAskForOne()
{
var vault = await ReadyToConnectAsync();
await AddKeyAsync(vault, "deploy");
await AddCredentialAsync(vault, "prod deploy");
vault.SelectedHost = vault.Hosts[0];
vault.SelectedHostAsksForAPassword.ShouldBeTrue();
vault.SelectedHostAuthenticationNote.ShouldBeEmpty();
await BindKeyAsync(vault, vault.Hosts[0], vault.Keys[0].EntityId);
vault.SelectedHost = vault.Hosts[0];
vault.SelectedHostAsksForAPassword.ShouldBeFalse();
vault.SelectedHostAuthenticationNote.ShouldContain("SSH key");
await BindCredentialAsync(vault, vault.Hosts[0], vault.Credentials[0].EntityId);
vault.SelectedHost = vault.Hosts[0];
vault.SelectedHostAsksForAPassword.ShouldBeFalse();
vault.SelectedHostAuthenticationNote.ShouldContain("stored in your vault");
}
// ---- Authenticating with a stored credential ----
[Fact]
public async Task AHostBoundToACredential_SendsItsPasswordAndItsUsername()
{
// Both halves, and the username is the half that was easy to lose: a credential's whole reason for
// existing is that it describes an account once, and sending its password under the host's username is
// wrong in a way the server only reports as "authentication failed".
//
// The credential's username is deliberately *not* the host's — the host is on "deploy" — because the
// two being equal is what makes this assertion pass under an implementation that reads the wrong one.
var vault = await ReadyToConnectAsync();
await AddCredentialAsync(vault, "prod deploy", password: "s3cret", username: "svc-deploy");
var credentialId = vault.Credentials[0].EntityId;
await BindCredentialAsync(vault, vault.Hosts[0], credentialId);
vault.Hosts.ShouldHaveSingleItem().Host.CredentialId.ShouldBe(credentialId);
vault.Hosts[0].Authentication.ShouldBe("credential");
vault.ConnectPassword = "should-not-be-used";
await ConnectWithRendererAsync(vault);
var request = ssh.Requests.ShouldHaveSingleItem();
request.Credential.ShouldBeOfType<SshPasswordCredential>().Password.ShouldBe("s3cret");
request.Username.ShouldBe("svc-deploy", "the credential's account overrides the host's");
}
[Fact]
public async Task ACredentialWithNoUsername_LeavesTheHostsInPlace()
{
// The shared-password case: one password used under whatever account each machine knows you by. The
// fallback is what makes that expressible at all, and getting it backwards would send every connection
// to the same account.
var vault = await ReadyToConnectAsync();
await AddCredentialAsync(vault, "shared password", password: "s3cret");
await BindCredentialAsync(vault, vault.Hosts[0], vault.Credentials[0].EntityId);
await ConnectWithRendererAsync(vault);
var request = ssh.Requests.ShouldHaveSingleItem();
request.Username.ShouldBe("deploy", "the host's own username, which ReadyToConnectAsync sets");
request.Credential.ShouldBeOfType<SshPasswordCredential>().Password.ShouldBe("s3cret");
}
[Fact]
public async Task AHostWithNoUsernameOfItsOwn_IsUsableThroughACredentialThatCarriesOne()
{
// The refusal for a host with no username used to run before anything looked at the binding, which made
// a credential's username unreachable in exactly the case it was most useful: a host somebody never
// filled a username in for.
var vault = await ReadyToConnectAsync();
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.EditorUsername = string.Empty;
await vault.SaveHostCommand.ExecuteAsync(null);
vault.Hosts.ShouldHaveSingleItem().Host.Username.ShouldBeNull();
await AddCredentialAsync(vault, "prod deploy", password: "s3cret", username: "deploy");
await BindCredentialAsync(vault, vault.Hosts[0], vault.Credentials[0].EntityId);
await ConnectWithRendererAsync(vault);
ssh.Requests.ShouldHaveSingleItem().Username.ShouldBe("deploy");
}
[Fact]
public async Task AHostWithNoUsernameAndNoCredential_IsStillRefusedAndSaysWhereToPutOne()
{
var vault = await ReadyToConnectAsync();
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.EditorUsername = string.Empty;
await vault.SaveHostCommand.ExecuteAsync(null);
await vault.ConnectCommand.ExecuteAsync(null);
ssh.Requests.ShouldBeEmpty();
vault.Status.ShouldContain("no username");
vault.Status.ShouldContain("credential", Case.Insensitive);
}
[Fact]
public async Task AHostWhoseCredentialHasBeenDeleted_RefusesRatherThanFallingBackToTheTypedPassword()
{
// The key case's twin, and it has to be its own test: the two branches are separate code, and the one
// that was written second is the one nothing would have covered.
var vault = await ReadyToConnectAsync();
await AddCredentialAsync(vault, "prod deploy");
await BindCredentialAsync(vault, vault.Hosts[0], vault.Credentials[0].EntityId);
vault.SelectedCredential = vault.Credentials[0];
await vault.DeleteCredentialCommand.ExecuteAsync(null);
vault.Credentials.ShouldBeEmpty();
vault.SelectedHost = vault.Hosts[0];
vault.ConnectPassword = "must-not-be-sent";
await vault.ConnectCommand.ExecuteAsync(null);
ssh.Requests.ShouldBeEmpty("nothing should have been dialled at all");
vault.Status.ShouldContain("credential that is not in this vault");
}
[Fact]
public async Task EditingAHostWhoseCredentialHasBeenDeleted_DoesNotQuietlyUnbindIt()
{
var vault = await ReadyToConnectAsync();
await AddCredentialAsync(vault, "prod deploy");
var credentialId = vault.Credentials[0].EntityId;
await BindCredentialAsync(vault, vault.Hosts[0], credentialId);
vault.SelectedCredential = vault.Credentials[0];
await vault.DeleteCredentialCommand.ExecuteAsync(null);
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
var selected = vault.EditorSelectedAuthentication.ShouldNotBeNull();
selected.EntityId.ShouldBe(credentialId);
selected.Kind.ShouldBe(
AuthenticationKind.Credential, "a missing credential must not come back as a missing key");
selected.Label.ShouldContain("no longer here");
vault.EditorPort = 2244;
await vault.SaveHostCommand.ExecuteAsync(null);
vault.Hosts.ShouldHaveSingleItem().Host.Port.ShouldBe(2244);
vault.Hosts[0].Host.CredentialId.ShouldBe(credentialId, "an unrelated edit must not drop the binding");
}
/// <remarks>
/// The reason the picker is one control rather than two. <c>HostSecret.TryValidate</c> refuses a host naming
/// both a key and a credential, so two pickers would have been able to express the state and would have had
/// to reject it at save time; one picker cannot express it. This asserts the structural version of that —
/// that rebinding replaces rather than accumulates.
/// </remarks>
[Fact]
public async Task RebindingFromAKeyToACredential_ReplacesTheBindingRatherThanAddingToIt()
{
var vault = await ReadyToConnectAsync();
await AddKeyAsync(vault, "deploy");
await AddCredentialAsync(vault, "prod deploy", password: "s3cret", username: "deploy");
var keyId = vault.Keys[0].EntityId;
var credentialId = vault.Credentials[0].EntityId;
await BindKeyAsync(vault, vault.Hosts[0], keyId);
vault.Hosts[0].Host.SshKeyId.ShouldBe(keyId);
await BindCredentialAsync(vault, vault.Hosts[0], credentialId);
var host = vault.Hosts.ShouldHaveSingleItem().Host;
host.CredentialId.ShouldBe(credentialId);
host.SshKeyId.ShouldBeNull("one picker means one binding");
host.TryValidate(out _).ShouldBeTrue();
// And back again, which is the direction that would leave a stale credential behind.
await BindKeyAsync(vault, vault.Hosts[0], keyId);
vault.Hosts.ShouldHaveSingleItem().Host.CredentialId.ShouldBeNull();
vault.Hosts[0].Host.SshKeyId.ShouldBe(keyId);
}
/// <remarks>
/// A key and a credential the user has named the same thing is the ordinary case, not a contrived one — a
/// key called <c>deploy</c> and the deploy account's password. Without the qualifier the picker offers two
/// identical rows that authenticate completely differently.
/// </remarks>
[Fact]
public async Task ThePickerDistinguishesAKeyAndACredentialWithTheSameName()
{
var vault = await ReadyToConnectAsync();
await AddKeyAsync(vault, "deploy");
await AddCredentialAsync(vault, "deploy");
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
var named = vault.EditorAuthenticationChoices
.Where(choice => string.Equals(choice.Label, "deploy", StringComparison.Ordinal))
.ToList();
named.Count.ShouldBe(2);
named.Select(choice => choice.Qualifier).ShouldBe(["SSH key", "credential"]);
}
[Fact]
public async Task TheCredentialEditorIsAlsoOneEditorAtATime()
{
await UnlockedAsync();
var vault = shell.Vault!;
vault.NewCredentialCommand.Execute(null);
vault.CredentialEditorPassword = "half-typed";
vault.NewHostCommand.Execute(null);
vault.IsEditing.ShouldBeFalse("the host editor must not open over the credential editor");
vault.Status.ShouldContain("credential");
vault.CredentialEditorPassword.ShouldBe("half-typed");
vault.ShowSectionCommand.Execute(VaultSection.Hosts);
vault.ShowsCredentials.ShouldBeTrue("and the selector must not move away from it either");
vault.CancelCredentialEditCommand.Execute(null);
vault.ShowSectionCommand.Execute(VaultSection.Hosts);
vault.ShowsHosts.ShouldBeTrue();
}
// ---- Helpers ---- // ---- Helpers ----
private static CancellationToken Token => TestContext.Current.CancellationToken; private static CancellationToken Token => TestContext.Current.CancellationToken;
@@ -1369,17 +1876,47 @@ public sealed class ShellFlowTests : IAsyncLifetime
} }
/// <summary>Points a host at a key through the editor, the way a user would.</summary> /// <summary>Points a host at a key through the editor, the way a user would.</summary>
private static async Task BindKeyAsync(VaultViewModel vault, HostRowViewModel host, Guid keyId) private static Task BindKeyAsync(VaultViewModel vault, HostRowViewModel host, Guid keyId) =>
BindAsync(vault, host, AuthenticationKind.SshKey, keyId);
/// <summary>Points a host at a stored credential through the same picker.</summary>
private static Task BindCredentialAsync(VaultViewModel vault, HostRowViewModel host, Guid credentialId) =>
BindAsync(vault, host, AuthenticationKind.Credential, credentialId);
/// <remarks>
/// One helper for both because there is one control for both, and a test that reached for the binding a
/// different way than the interface does would stop covering the interface.
/// </remarks>
private static async Task BindAsync(
VaultViewModel vault,
HostRowViewModel host,
AuthenticationKind kind,
Guid entityId)
{ {
vault.SelectedHost = host; vault.SelectedHost = host;
vault.EditSelectedHostCommand.Execute(null); vault.EditSelectedHostCommand.Execute(null);
vault.IsEditing.ShouldBeTrue("the host editor has to be open for the picker to be populated"); vault.IsEditing.ShouldBeTrue("the host editor has to be open for the picker to be populated");
vault.EditorSelectedKey = vault.EditorKeyChoices.Single(choice => choice.EntityId == keyId); vault.EditorSelectedAuthentication = vault.EditorAuthenticationChoices
.Single(choice => choice.Kind == kind && choice.EntityId == entityId);
await vault.SaveHostCommand.ExecuteAsync(null); await vault.SaveHostCommand.ExecuteAsync(null);
} }
private static async Task AddCredentialAsync(
VaultViewModel vault,
string label,
string password = "s3cret",
string username = "")
{
vault.NewCredentialCommand.Execute(null);
vault.CredentialEditorLabel = label;
vault.CredentialEditorPassword = password;
vault.CredentialEditorUsername = username;
await vault.SaveCredentialCommand.ExecuteAsync(null);
}
/// <summary>Connects with a renderer attached, which the data plane requires before a session opens.</summary> /// <summary>Connects with a renderer attached, which the data plane requires before a session opens.</summary>
private async Task ConnectWithRendererAsync(VaultViewModel vault) private async Task ConnectWithRendererAsync(VaultViewModel vault)
{ {