Merge branch 'claude/host-password-persistence-2c2c1f'

src/DodoSSH.Client.Android/Views/HostsScreen.axaml

@@ -418,6 +418,10 @@
         <!--
           Shown only for a host that actually asks for one. A password box beside a key-authenticated host
           is an invitation to type a secret nothing will use.
+
+          The tick below it is the phone's whole answer to storing one, and on this head it is the only one:
+          the keychain lists credentials here but has no editor to create one in, so before this a password
+          typed on a phone could only ever be typed again. The host editor's picker could then bind it.
         -->
         <TextBox Classes="field secret" IsVisible="{Binding SelectedHostAsksForAPassword}"
                  Text="{Binding ConnectPassword}" PlaceholderText="password">
@@ -426,6 +430,12 @@
           </TextBox.KeyBindings>
         </TextBox>
 
+        <CheckBox IsChecked="{Binding RemembersConnectPassword}" MinHeight="44"
+                  IsVisible="{Binding SelectedHostAsksForAPassword}">
+          <TextBlock Classes="mono" FontSize="11.5" TextWrapping="Wrap"
+                     Text="Remember this password for this host" />
+        </CheckBox>
+
         <TextBlock Classes="detail" TextWrapping="Wrap" IsVisible="{Binding !SelectedHostAsksForAPassword}"
                    Text="{Binding SelectedHostAuthenticationNote}" />
 

src/DodoSSH.Client.App/Views/HostsScreen.axaml

@@ -36,14 +36,23 @@
         a key wants nothing typed here — and a sentence in its place when it does not, because "nothing
         needs typing" and "something needs typing and the box has not appeared yet" look identical and only
         one of them is fine.
+
+        REMEMBER travels with the box and hides with it. It is the two-step chore the box's tooltip used to
+        describe — add a password under Keychain, then bind the host to it — done from the one screen that
+        already has the password, and it takes effect only once the remote has accepted it.
       -->
       <Border Grid.Row="0" Padding="12,8" Background="{StaticResource Panel}"
               BorderBrush="{StaticResource BorderSubtle}" BorderThickness="0,0,0,1">
         <StackPanel Orientation="Horizontal" Spacing="8">
-          <TextBox Text="{Binding Vault.ConnectPassword}" PlaceholderText="password (not stored)"
+          <TextBox Text="{Binding Vault.ConnectPassword}" PlaceholderText="password"
                    PasswordChar="•" Width="200" VerticalAlignment="Center"
                    IsVisible="{Binding Vault.SelectedHostAsksForAPassword}"
-                   ToolTip.Tip="Typed each time and never stored. To stop typing it, add a password under Keychain and bind this host to it in the host's own editor." />
+                   ToolTip.Tip="Typed each time unless REMEMBER is ticked, in which case it is saved to your keychain and bound to this host once the connection succeeds." />
+          <CheckBox IsChecked="{Binding Vault.RemembersConnectPassword}" VerticalAlignment="Center"
+                    IsVisible="{Binding Vault.SelectedHostAsksForAPassword}"
+                    ToolTip.Tip="Saves this password to your keychain, bound to this host, so it is not asked for again. It syncs to your other machines, and only happens if the connection works.">
+            <TextBlock Text="REMEMBER" Classes="hint" FontSize="11" />
+          </CheckBox>
           <TextBlock Text="{Binding Vault.SelectedHostAuthenticationNote}" Classes="hint"
                      FontSize="11" VerticalAlignment="Center"
                      IsVisible="{Binding !Vault.SelectedHostAsksForAPassword}" />

src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs

@@ -333,7 +333,13 @@ internal sealed partial class HostRowViewModel(
 /// <summary>What a host can authenticate with.</summary>
 internal enum AuthenticationKind
 {
-    /// <summary>Typed at the moment of connecting, and never stored.</summary>
+    /// <summary>Typed at the moment of connecting.</summary>
+    /// <remarks>
+    /// Nothing is stored under this kind. Ticking the connect bar's REMEMBER does not change that — it
+    /// creates a credential and moves the host to <see cref="Credential"/>, so a stored password is always
+    /// an item somebody can find, rename and delete rather than a fourth place a secret quietly lives. See
+    /// <see cref="VaultViewModel.RemembersConnectPassword"/>.
+    /// </remarks>
     Typed,
 
     /// <summary>An SSH key in this vault.</summary>
@@ -1984,14 +1990,36 @@ internal sealed partial class VaultViewModel(
     // ---- Connecting ----
 
     /// <remarks>
-    /// Typed per connection, never persisted, and now only reached by a host bound to nothing. It stays because
-    /// not every password is worth storing — a one-off on a machine somebody will never open again, or one
-    /// they would rather this vault did not hold — and because a credential has to be created before it can be
-    /// bound, which means the first connection to a new host happens through this box.
+    /// Typed per connection, not persisted unless <see cref="RemembersConnectPassword"/> says otherwise, and
+    /// only reached by a host bound to nothing. It stays because not every password is worth storing — a
+    /// one-off on a machine somebody will never open again, or one they would rather this vault did not hold
+    /// — and because a credential has to be created before it can be bound, which means the first connection
+    /// to a new host happens through this box.
     /// </remarks>
     [ObservableProperty]
     private string connectPassword = string.Empty;
 
+    /// <summary>
+    /// Whether a password typed here should be kept, so this host stops asking for it.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// What it produces is an ordinary keychain credential bound to the host, and not a fourth place a
+    /// password can live. The two-step chore it replaces — add a password under Keychain, then open the host
+    /// and bind it — is what the box's tooltip used to instruct people to do by hand, and doing it by hand
+    /// means typing the secret into a second screen while the first one already has it.
+    /// </para>
+    /// <para>
+    /// <b>Off by default, and it stays a decision.</b> The reason a typed password exists at all is that not
+    /// every password belongs in a synchronised vault; remembering silently would move each of them there and
+    /// tell nobody. It also only takes effect once the handshake has succeeded — see
+    /// <see cref="RememberTypedPasswordAsync"/> — because a password that has just been refused is precisely
+    /// the one not worth keeping.
+    /// </para>
+    /// </remarks>
+    [ObservableProperty]
+    private bool remembersConnectPassword;
+
     /// <summary>
     /// Whether the selected host will want something typed into the password box.
     /// </summary>
@@ -5231,6 +5259,95 @@ internal sealed partial class VaultViewModel(
                 sessionId,
                 row.Label,
                 Dialled(row, authentication)));
+
+        // Last, and after the tab exists: keeping the password is a favour, and the session the user asked
+        // for must not wait on a vault write to appear.
+        await RememberTypedPasswordAsync(row, authentication, cancellationToken).ConfigureAwait(true);
+    }
+
+    /// <summary>
+    /// Turns the password that just worked into a keychain credential bound to this host.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// <b>Only after a handshake the remote accepted.</b> Storing a password the moment it is typed would
+    /// bind whatever was in the box — including the typo that is about to be refused — and the host would
+    /// then stop asking, leaving a machine that cannot be connected to until somebody works out that the
+    /// keychain is where the wrong password now lives.
+    /// </para>
+    /// <para>
+    /// <b>A credential rather than a field on the host, which is why nothing else here had to change.</b>
+    /// It syncs, merges, appears in the keychain, can be renamed, deleted and — the reason the item type
+    /// exists — bound to the other nineteen machines that share the account. See <see cref="HostSecret"/>
+    /// on why the binding is an id and not a copy.
+    /// </para>
+    /// <para>
+    /// The credential carries no username of its own, so it keeps taking the host's — which is what the
+    /// connection that just succeeded did. Copying the resolved username into it would pin whatever the
+    /// group happened to say at this moment, and quietly stop following the group afterwards.
+    /// </para>
+    /// <para>
+    /// Every failure is reported and swallowed. The caller's <c>catch</c> blocks describe a connection that
+    /// did not happen, and this one did: a vault write that fails here must not tell the user their terminal
+    /// was abandoned, and a cancellation must not report it as cancelled.
+    /// </para>
+    /// </remarks>
+    private async Task RememberTypedPasswordAsync(
+        HostRowViewModel row,
+        HostAuthentication authentication,
+        CancellationToken cancellationToken)
+    {
+        // The password as dialled, not as the box currently reads: the two can differ by now, because a
+        // handshake takes time and the box stays typeable throughout it.
+        if (!RemembersConnectPassword
+            || row.Resolved.Binding.Kind is not ResolvedBindingKind.TypedPassword
+            || authentication.Credential is not SshPasswordCredential { Password.Length: > 0 } typed)
+        {
+            return;
+        }
+
+        if (row.IsReadOnly)
+        {
+            Status = $"Connected to {row.Label}. Its password was not saved: this host was written by a "
+                + "newer version of DodoSSH, and binding a credential would re-encode it.";
+            return;
+        }
+
+        var credential = new CredentialSecret { Label = row.Label, Password = typed.Password };
+
+        try
+        {
+            var credentialId = await session.Credentials
+                .CreateAsync(row.VaultId, credential, cancellationToken)
+                .ConfigureAwait(true);
+
+            // Into the same vault as the host, deliberately: a credential in the personal vault bound to a
+            // team's host is a binding every other member can see and none of them can resolve.
+            await session.Hosts
+                .UpdateAsync(
+                    row.VaultId,
+                    row.EntityId,
+                    row.Host with { CredentialId = credentialId, AsksForPassword = null },
+                    cancellationToken)
+                .ConfigureAwait(true);
+        }
+        catch (Exception exception)
+        {
+            Status = $"Connected to {row.Label}, but its password could not be saved: {exception.Message}";
+            return;
+        }
+
+        // Cleared together. The box is about to disappear — the host answers "credential" now — and a tick
+        // left behind would apply to the next host somebody selects.
+        RemembersConnectPassword = false;
+        ConnectPassword = string.Empty;
+
+        await ReloadAsync(cancellationToken).ConfigureAwait(true);
+
+        Status = $"Connected to {row.Label}. Its password is saved in your keychain as '{row.Label}', so it "
+            + "will not be asked for again.";
+
+        await AutoSyncAsync(cancellationToken).ConfigureAwait(true);
     }
 
     /// <summary>The address as actually dialled.</summary>

tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs

@@ -2661,6 +2661,123 @@ public sealed class ShellFlowTests : IAsyncLifetime
         vault.Hosts[0].Host.CredentialId.ShouldBe(credentialId, "an unrelated edit must not drop the binding");
     }
 
+    // ---- Remembering a typed password ----
+
+    [Fact]
+    public async Task RememberingATypedPassword_BindsItToTheHostSoItIsNotAskedForAgain()
+    {
+        var vault = await ReadyToConnectAsync();
+
+        vault.RemembersConnectPassword.ShouldBeFalse("storing a password stays a decision");
+
+        vault.ConnectPassword = "s3cret";
+        vault.RemembersConnectPassword = true;
+
+        await ConnectAndRememberAsync(vault);
+
+        // An ordinary keychain credential, named after the host, and carrying no username of its own — the
+        // connection that just succeeded used the host's, and pinning a copy of it here would stop following
+        // the host.
+        var stored = vault.Credentials.ShouldHaveSingleItem();
+        stored.Label.ShouldBe("prod-db");
+        stored.Credential.Password.ShouldBe("s3cret");
+        stored.Credential.Username.ShouldBeNull();
+
+        var host = vault.Hosts.ShouldHaveSingleItem();
+        host.Host.CredentialId.ShouldBe(stored.EntityId);
+        host.Authentication.ShouldBe("credential");
+
+        // The box has nothing left to hold and nothing left to ask, and the tick does not carry over to
+        // whatever host is selected next.
+        vault.ConnectPassword.ShouldBeEmpty();
+        vault.RemembersConnectPassword.ShouldBeFalse();
+        vault.SelectedHostAsksForAPassword.ShouldBeFalse();
+    }
+
+    [Fact]
+    public async Task ARememberedPassword_SurvivesTheServerAndIsSentOnTheNextConnection()
+    {
+        // The whole point of storing it in the vault rather than on this machine: it is a property of the
+        // host that reaches the other machines, not a box this one happens to remember filling in.
+        var vault = await ReadyToConnectAsync();
+
+        // One renderer for both connections. The page's token is spent on the first attach, so a second
+        // FakeRenderer is answered with a 409 — which is the real renderer's behaviour too, and the reason
+        // nothing else in this suite connects twice.
+        await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
+
+        vault.ConnectPassword = "s3cret";
+        vault.RemembersConnectPassword = true;
+
+        await vault.ConnectCommand.ExecuteAsync(null);
+
+        await vault.SyncCommand.ExecuteAsync(null);
+        await vault.LoadAsync(Token);
+
+        vault.Credentials.ShouldHaveSingleItem().Credential.Password.ShouldBe("s3cret");
+
+        vault.SelectedHost = vault.Hosts[0];
+        vault.ConnectPassword.ShouldBeEmpty("nothing should need typing now");
+
+        await vault.ConnectCommand.ExecuteAsync(null);
+
+        ssh.Requests.Count.ShouldBe(2);
+        ssh.Requests[1].Credential.ShouldBeOfType<SshPasswordCredential>().Password.ShouldBe("s3cret");
+    }
+
+    [Fact]
+    public async Task ARefusedConnection_RemembersNothing()
+    {
+        // The failure this feature could most easily cause: a typo bound to the host, which then stops asking
+        // and cannot be connected to until somebody works out that the keychain is where the wrong password
+        // now lives. Only a handshake the remote accepted is worth keeping.
+        var vault = await ReadyToConnectAsync();
+
+        ssh.Failure = new InvalidOperationException("authentication failed");
+
+        vault.ConnectPassword = "wrong";
+        vault.RemembersConnectPassword = true;
+
+        await vault.ConnectCommand.ExecuteAsync(null);
+
+        vault.Credentials.ShouldBeEmpty();
+        vault.Hosts.ShouldHaveSingleItem().Host.CredentialId.ShouldBeNull();
+        vault.SelectedHostAsksForAPassword.ShouldBeTrue();
+    }
+
+    [Fact]
+    public async Task ConnectingWithoutTheTick_StoresNothing()
+    {
+        // The other half of the decision, and the reason the typed box still exists: a one-off password on a
+        // machine somebody will never open again must not end up synchronised to every device they own.
+        var vault = await ReadyToConnectAsync();
+
+        vault.ConnectPassword = "s3cret";
+
+        await ConnectWithRendererAsync(vault);
+
+        vault.Credentials.ShouldBeEmpty();
+        vault.Hosts.ShouldHaveSingleItem().Host.CredentialId.ShouldBeNull();
+        vault.ConnectPassword.ShouldBe("s3cret", "the box is left as it was typed");
+    }
+
+    [Fact]
+    public async Task RememberingIsIgnoredForAHostThatDoesNotAskForAPassword()
+    {
+        // A tick left over from a host that did ask must not manufacture a credential out of a stored one's
+        // password — which is what reading the dialled secret without checking the binding would do.
+        var vault = await ReadyToConnectAsync();
+        await AddCredentialAsync(vault, "prod deploy", password: "s3cret");
+        await BindCredentialAsync(vault, vault.Hosts[0], vault.Credentials[0].EntityId);
+
+        vault.SelectedHost = vault.Hosts[0];
+        vault.RemembersConnectPassword = true;
+
+        await ConnectWithRendererAsync(vault);
+
+        vault.Credentials.ShouldHaveSingleItem("nothing should have been added to the keychain");
+    }
+
     /// <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
@@ -5088,6 +5205,24 @@ public sealed class ShellFlowTests : IAsyncLifetime
         vault.Status.ShouldContain("Connected", Case.Insensitive);
     }
 
+    /// <summary>
+    /// The same, for a connection that is expected to store its password.
+    /// </summary>
+    /// <remarks>
+    /// Without the status assertion, and that is the whole reason it is separate. Remembering writes two
+    /// items and then pushes them, exactly as saving a host does, so the pass repaints the line with its own
+    /// count — leaving "Connected" true of what happened and false of what the line says. What the connection
+    /// actually did is asserted on the vault, which is where it is durable.
+    /// </remarks>
+    private async Task ConnectAndRememberAsync(VaultViewModel vault)
+    {
+        await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
+
+        await vault.ConnectCommand.ExecuteAsync(null);
+
+        ssh.Requests.ShouldNotBeEmpty("the password is only kept once a handshake has succeeded");
+    }
+
     /// <summary>An unlocked vault with one selected host and a renderer attached.</summary>
     private async Task<VaultViewModel> ReadyToConnectAsync()
     {