Let the host editor make the credential it is about to bind
ci / build and test (pull_request) Successful in 2m12s
ci / desktop nightly (pull_request) Skipped
ci / android head (pull_request) Successful in 3m18s
ci / api image (pull_request) Successful in 4s

The authentication picker has listed saved credentials since they existed, but
making one meant leaving a half-typed host for the keychain screen and coming
back to find it gone. On the phone it was worse than a detour: that head has no
credential editor at all, so it could bind a host to a credential and never
produce one. + NEW CREDENTIAL opens a card under the picker — name, optional
username, password, notes — and ADD writes it and binds the host in one step.

A button beside the picker rather than an entry inside it. Every row of that
list is a binding a host can have, and "make a new one" is an action: as an
entry it would sit in the box afterwards describing a state no host can be in,
and cancelling the form would leave the picker showing it.

It carries its own five fields rather than reusing the keychain editor's, and
that is the load-bearing part. IsEditingCredential is what AVaultEditorIsInTheWay
asks about, so sharing it would have made the whole Vault screen refuse to open
an editor while this card sat open on the Hosts screen, with a status line
naming a form the user cannot see on a screen they are not looking at — the
exact failure that guard was split in two to end. A test pins it.

It writes to the keychain immediately, unlike every other field in this editor,
because a credential is a shared item with an id and a host can only name an id
that exists. The consequence is honest rather than hidden and the hint says so:
a credential added this way outlives a cancelled host edit. What was still being
typed does not — every path that closes the host editor clears the form, and one
of those fields is a password.

The binding is written before the reload rather than after it. RefreshOpenEditors
rebuilds this picker and then restores it from the editor's own selection, so
setting it first is what survives the pass, and by the time it is read
ReloadCredentialsAsync has put the matching entry in the list to land on.

A name already taken is duplicated, not reused, and that is a deliberate parting
from the new-tag box six lines further down which offers the existing tag
instead. Two tags called "staging" are one intention spelled twice; two
credentials called "root" are two different passwords, and quietly binding the
host to whichever was there already would authenticate it as an account nobody
chose. A duplicate label in the picker is the smaller problem.

Into editingHostVaultId, so the credential lands wherever the host is being
sealed and everybody who can read the host can read what it authenticates with.
Stricter than the tag path — which files into the active vault and is recorded
as a gap in docs/design-import-gaps.md — and it can be, because this picker
lists credentials from every readable vault rather than one.

Five flow tests cover the bind-through-reload path, the cancel semantics on both
the saved credential and the abandoned one, the cross-screen guard, the
duplicate name and the empty-password refusal. The layout test is separate and
necessary: the card is collapsed until somebody presses the button, so a harness
driven by the default state draws none of it, and TheHostDrawerFitsWithTheHostEditorOpen
would have gone on passing over a card that blew the column. 1,854 tests, none
failing.
This commit is contained in:
2026-08-10 11:23:10 +02:00
parent 8c67fce32c
commit e96d01aab9
5 changed files with 426 additions and 0 deletions
@@ -189,6 +189,25 @@ public sealed class ScreenLayoutTests : IAsyncLifetime
await MeasureDrawerAsync(faults => faults.ShouldBeEmpty());
}
/// <remarks>
/// The editor with its new-credential card showing, which the test above never draws: the card is
/// collapsed until somebody presses + NEW CREDENTIAL, so nothing else in this suite measures the three
/// boxes, the paragraph of hint text and the two buttons it adds inside the section that already holds
/// the authentication picker. A card that only appears on a click is exactly the shape that escapes a
/// harness driven by the default state.
/// </remarks>
[Fact]
public async Task TheHostDrawerFitsWithTheNewCredentialFormOpen()
{
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.IsAddingEditorCredential.ShouldBeTrue("there is nothing to measure otherwise");
await MeasureDrawerAsync(faults => faults.ShouldBeEmpty());
}
/// <remarks>
/// The other editor, and it is in this control for the first time: the desktop's group editor used to be
/// a bar across the foot of the hosts screen, where it competed with the grid for the same column. Its
@@ -2980,6 +2980,161 @@ public sealed class ShellFlowTests : IAsyncLifetime
vault.Hosts[0].Host.CredentialId.ShouldBeNull();
}
/// <remarks>
/// The moment a credential is wanted is the moment somebody is choosing how a host authenticates and
/// finds it is not in the keychain yet, so the host editor makes one. Selecting it has to survive the
/// reload the write triggers, which is the part that needs a test: the refill rebuilds the picker from
/// the vault and restores it from the editor's own selection, so the binding is written before the
/// reload rather than after it.
/// </remarks>
[Fact]
public async Task ACredentialMadeInTheHostEditor_BindsTheHostToIt()
{
var vault = await ReadyToConnectAsync();
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.IsAddingEditorCredential.ShouldBeTrue();
vault.EditorNewCredentialLabel = "pg-primary";
vault.EditorNewCredentialUsername = "postgres";
vault.EditorNewCredentialPassword = "s3cret";
vault.EditorNewCredentialNotes = "rotated quarterly";
await vault.AddEditorCredentialCommand.ExecuteAsync(null);
var credential = vault.Credentials.ShouldHaveSingleItem();
credential.Credential.Username.ShouldBe("postgres");
credential.Credential.Notes.ShouldBe("rotated quarterly");
vault.IsAddingEditorCredential.ShouldBeFalse("the form closes once the credential is in the keychain");
vault.EditorNewCredentialPassword.ShouldBeEmpty("the form must not go on holding the password");
vault.EditorSelectedAuthentication.ShouldNotBeNull().EntityId.ShouldBe(
credential.EntityId,
"the picker has to land on the credential that was just made, through the reload");
await vault.SaveHostCommand.ExecuteAsync(null);
vault.Hosts.ShouldHaveSingleItem().Host.CredentialId.ShouldBe(credential.EntityId);
vault.Hosts[0].Host.SshKeyId.ShouldBeNull();
}
/// <remarks>
/// The honest consequence of writing immediately, and the same one the new-tag box already carries: a
/// credential is a shared item with an id, the host can only name an id that exists, so the credential
/// was never part of the host to begin with. What was still being typed is a different matter — that
/// includes a password, and it goes with the editor it was typed into.
/// </remarks>
[Fact]
public async Task CancellingTheHostEditor_KeepsTheCredentialItMade_AndDropsWhatWasStillBeingTyped()
{
var vault = await ReadyToConnectAsync();
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.EditorNewCredentialLabel = "pg-primary";
vault.EditorNewCredentialPassword = "s3cret";
await vault.AddEditorCredentialCommand.ExecuteAsync(null);
// A second one, opened and left half-typed.
vault.BeginEditorCredentialCommand.Execute(null);
vault.EditorNewCredentialLabel = "half";
vault.EditorNewCredentialPassword = "typed-but-never-added";
vault.EditorNewCredentialNotes = "half a thought";
vault.CancelEditCommand.Execute(null);
vault.Credentials.ShouldHaveSingleItem().Label.ShouldBe("pg-primary");
vault.IsAddingEditorCredential.ShouldBeFalse();
vault.EditorNewCredentialLabel.ShouldBeEmpty();
vault.EditorNewCredentialNotes.ShouldBeEmpty();
vault.EditorNewCredentialPassword.ShouldBeEmpty(
"a password typed into an abandoned form must not survive behind the next host");
vault.Hosts.ShouldHaveSingleItem().Host.CredentialId.ShouldBeNull(
"the binding itself was never saved");
}
/// <remarks>
/// Why this form has fields of its own rather than reusing the keychain screen's four.
/// <c>IsEditingCredential</c> is what <c>AVaultEditorIsInTheWay</c> asks about, so sharing it would make
/// the whole Vault screen refuse to open an editor, with a sentence naming a form the user cannot see
/// on a screen they are not looking at. That is the exact failure the guard was split in two to end.
/// </remarks>
[Fact]
public async Task TheHostEditorsCredentialForm_DoesNotBlockTheKeychainsOwnEditors()
{
var vault = await ReadyToConnectAsync();
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.NewCredentialCommand.Execute(null);
vault.IsEditingCredential.ShouldBeTrue(
"the keychain's editor lives on another screen and opens regardless");
}
/// <remarks>
/// Where this deliberately parts from the new-tag box beside it, which offers an existing tag rather
/// than repeating it. Two tags called "staging" are one intention spelled twice; two credentials called
/// "root" are two different passwords, and quietly binding the host to whichever was there already
/// would authenticate it as an account nobody chose.
/// </remarks>
[Fact]
public async Task ACredentialMadeInTheHostEditor_UnderANameAlreadyTaken_IsASecondCredential()
{
var vault = await ReadyToConnectAsync();
await AddCredentialAsync(vault, "root", password: "first");
var first = vault.Credentials.ShouldHaveSingleItem().EntityId;
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.EditorNewCredentialLabel = "root";
vault.EditorNewCredentialPassword = "second";
await vault.AddEditorCredentialCommand.ExecuteAsync(null);
vault.Credentials.Count.ShouldBe(2);
vault.EditorSelectedAuthentication.ShouldNotBeNull().EntityId.ShouldNotBe(
first,
"binding to the credential that happened to share the name would be the wrong password");
}
/// <remarks>
/// The same refusal <c>CredentialSecret.TryValidate</c> gives the keychain's editor, reaching the user
/// here rather than producing an item that looks usable and fails at the handshake.
/// </remarks>
[Fact]
public async Task ACredentialMadeInTheHostEditor_WithNoPassword_IsRefused()
{
var vault = await ReadyToConnectAsync();
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.EditorNewCredentialLabel = "pg-primary";
await vault.AddEditorCredentialCommand.ExecuteAsync(null);
vault.Credentials.ShouldBeEmpty();
vault.IsAddingEditorCredential.ShouldBeTrue("the form stays open on what it refused");
vault.Status.ShouldContain("password");
}
/// <remarks>
/// Tags reach the same editor by a different route — the keychain screen rather than the box under the
/// chips — and a chip that only appeared on the next open would send the user round the same detour.