Public Access
Keep the password that just worked, so the host stops asking for it
The vault has held credentials and host bindings since they landed, and the only route from a typed password into one ran through two screens: add a password under Keychain, open the host, bind it. The password box's own tooltip instructed people to do exactly that by hand — which means typing the secret a second time, into a screen that has no idea which host it is for, while the screen that does have the password is the one being left behind. A tick beside the box does it in one step. What it produces is an ordinary CredentialSecret, named after the host, bound through CredentialId with AsksForPassword cleared beside it. That is the whole reason nothing else here had to change: it syncs, it merges, it appears in the keychain, it can be renamed and deleted, and it can be bound to the other nineteen machines that share the account. A password field on HostSecret would have been a payload schema bump, a fourth place a secret lives, and a copy per host to rotate and forget. It waits for a handshake the remote accepted, and that is not caution for its own sake. Binding on the keystroke would store whatever was in the box — including the typo about to be refused — and the host would then stop asking, leaving a machine nobody can connect to until they work out that the keychain is where the wrong password now lives. For the same reason the password is read from the credential as dialled rather than from the box, which stays typeable throughout a handshake. Off by default, because the typed box exists precisely for the passwords that should not be in a synchronised vault — a one-off on a machine somebody will never open again. The credential is written into the host's own vault rather than the active one: in the personal vault, bound to a team's host, it would be a binding every other member can see and none of them can resolve. One thing is given up knowingly. The confirmation naming where the password went is replaced a moment later by the auto-sync's own count, exactly as SaveHostAsync's "Saved 'x'" is; this follows that rather than becoming the one write that suppresses a sync line. The feedback that lasts is the row answering "credential" and the box disappearing — which is also the only way to store a password on the phone at all, since that head lists credentials but has never had an editor to create one in.
This commit is contained in:
@@ -2590,6 +2590,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
|
||||
@@ -5017,6 +5134,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()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user