Public Access
`Tag` has been a full item kind for three commits — a table, a migration, a codec, a merge, a cipher — and `HostSecret.TagIds` has merged per tag so two people tagging one host both keep theirs. Nothing drew a chip. The tags a client could store were ones nothing here could see. Chips on host rows, both heads, from names resolved through the tag list rather than ids: a tag that does not resolve is left out rather than drawn, because it means the tag was deleted elsewhere or belongs to a vault this session cannot read, and a host with one chip fewer is the honest answer where a host wearing a GUID is not. The id stays on the host, so the chip comes back if the tag does. The picker is chips that toggle, matching the chips on the row behind it. A list of names to tick would make the user match an entry to a chip they can see two inches away. The box under it creates a tag and puts it on straight away, because that is when a tag is usually wanted — while tagging a host and finding it does not exist yet. Unlike every other field in that editor it writes to the keychain immediately, since a host can only name an id that exists; cancelling therefore leaves the tag behind, which is honest rather than hidden. A name that already exists is used rather than repeated: two tags called "staging" are storable and must stay storable, because two people creating one offline is how it happens, but typing it into a box beside a chip of the same name is a slip. Renaming and deleting needed a home, or the picker fills with names nobody uses and never empties. That home is a TAGS category on the keychain screen, where every other item kind is managed — and renaming is the whole reason a tag is an item rather than a string repeated inside twenty payloads: it is one write, and no host is touched. The delete confirmation counts the hosts wearing it, which is the difference between a tidy-up and losing a filter somebody relies on. The desktop host editor now scrolls, and that is not a tidy-up. A picker's height is a chip per tag in the keychain, wrapped, so somebody with fifteen tags has an editor half again as tall as somebody with three; no fixed height holds that, and trimming other fields to buy room only moves the failure to whoever has sixteen. The layout suite caught it the moment its seeder grew tags — which is why the seeder now creates ten rather than three, enough to drive the pane onto its cap so the capped shape is what gets measured rather than one no real keychain produces. The cost is named where it is paid: the harness skips anything inside a ScrollViewer, so from here it certifies that pane fits the column rather than that every field in it does. Two smaller things fell out. Five buttons overflowed the keychain header by a few pixels, so GENERATE lost the word KEY — its tooltip carries what the word did. And TotalItemCount had been counting keys and credentials while ALL showed four kinds; it counts all five now, because a number under a chip that disagrees with the rows it opens is worse than no number. An adversarial review of this change found two defects it had introduced, both green against the full suite. NewTag filed into the "new items go to" picker while the tag list only ever holds the active vault's — so with a team vault selected a tag would be created, queued for push, reported as added, and then invisible, with no row, no count, no picker entry and nothing able to rename or delete it, because there is no active-vault switcher to go and find it with. The comment on the host editor's own create path states that exact rule; this was the one place that broke it, and NewObjectStore, whose list is likewise active-vault-only, already ignored the picker. And the tag editor was the only one of five that did not disarm a pending deletion when it opened, so arming a key's deletion and then pressing + TAG left a live DELETE for an item the user was no longer looking at, directly above the boxes they were typing into. Both are fixed, both have a test, and the first was checked against the broken version before being kept. The same review caught a doc comment that had been inserted between SnippetRowViewModel's summary and its declaration, silently taking it over. Verified by the whole suite on a clean build: 1413 tests over nineteen projects, none failing. Both heads build. The rectangles the layout suite cannot reach are phase 9 of docs/manual-checks.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
312 lines
12 KiB
C#
312 lines
12 KiB
C#
using DodoSSH.Client.Session;
|
|
// FakeDeviceKeyStore is compiled into this assembly from a source link and keeps its original namespace;
|
|
// see the csproj for why it is shared rather than reimplemented.
|
|
using DodoSSH.Client.Session.Tests;
|
|
using DodoSSH.Client.Shell.ViewModels;
|
|
using DodoSSH.Client.Ssh;
|
|
using DodoSSH.Client.Storage;
|
|
using DodoSSH.Client.Terminal;
|
|
using DodoSSH.Crypto;
|
|
|
|
namespace DodoSSH.Client.App.Tests;
|
|
|
|
/// <summary>
|
|
/// Teams, from the side that holds the keys: create one, add somebody, and wrap a vault key to them.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// <para>
|
|
/// The reason this suite exists rather than leaving teams to the server's own tests is that the
|
|
/// interesting half is not on the server. Adding a member is a row; <b>sharing is a decision the client
|
|
/// makes about whether to trust a public key the server just handed it</b>, and that decision is what
|
|
/// stands between an end-to-end encrypted vault and one the operator can read by answering a directory
|
|
/// lookup with a key of their own.
|
|
/// </para>
|
|
/// <para>
|
|
/// So the fake server keeps a real key log — chained with the same <c>KeyLogChain</c> the server uses —
|
|
/// and can be told to corrupt it. A test that only ever saw a well-formed log would be checking that
|
|
/// sharing works, not that verification does.
|
|
/// </para>
|
|
/// </remarks>
|
|
public sealed class TeamSharingTests : IAsyncLifetime
|
|
{
|
|
private const string Passphrase = "a sufficiently long passphrase";
|
|
|
|
private static readonly Argon2Profile CheapProfile =
|
|
Argon2Profile.FromStoredParameters(memoryKibibytes: 8 * 1024, passes: 1, parallelism: 1);
|
|
|
|
private readonly FakeVaultServer server = new();
|
|
private readonly FakeSshConnectionFactory ssh = new();
|
|
|
|
private string directory = null!;
|
|
private ClientCacheFactory caches = null!;
|
|
private TerminalWorkspace workspace = null!;
|
|
private VaultKnownHostStore knownHosts = null!;
|
|
private FakeDeviceKeyStore deviceKeys = null!;
|
|
private MainWindowViewModel shell = null!;
|
|
|
|
private static CancellationToken Token => TestContext.Current.CancellationToken;
|
|
|
|
/// <inheritdoc />
|
|
public ValueTask InitializeAsync()
|
|
{
|
|
directory = Path.Combine(Path.GetTempPath(), $"dodossh-teams-{Guid.CreateVersion7():N}");
|
|
|
|
var paths = new ClientPaths(directory);
|
|
|
|
caches = ClientCacheFactory.ForFile(paths.CacheFile);
|
|
knownHosts = new VaultKnownHostStore();
|
|
deviceKeys = new FakeDeviceKeyStore();
|
|
|
|
workspace = new TerminalWorkspace(
|
|
new InMemoryTerminalAssetProvider(
|
|
new Dictionary<string, TerminalAsset>(StringComparer.Ordinal)),
|
|
ssh,
|
|
TimeProvider.System);
|
|
|
|
shell = new MainWindowViewModel(
|
|
paths,
|
|
caches,
|
|
workspace,
|
|
knownHosts,
|
|
deviceKeys,
|
|
(_, _) => Task.FromResult<IVaultServer>(server),
|
|
TimeProvider.System,
|
|
NSubstitute.Substitute.For<ISftpSessionFactory>(),
|
|
CheapProfile);
|
|
|
|
return ValueTask.CompletedTask;
|
|
}
|
|
|
|
/// <inheritdoc />
|
|
public async ValueTask DisposeAsync()
|
|
{
|
|
await shell.DisposeAsync();
|
|
knownHosts.Close();
|
|
await workspace.DisposeAsync();
|
|
caches.Dispose();
|
|
|
|
try
|
|
{
|
|
Directory.Delete(directory, recursive: true);
|
|
}
|
|
catch (IOException)
|
|
{
|
|
// A cache file the process has not finished releasing. The directory is under the temp path
|
|
// and named per run, so leaving it costs a few kilobytes and never collides.
|
|
}
|
|
}
|
|
|
|
/// <remarks>
|
|
/// The whole point of a team, in one test. Note what the status line says after the add and before
|
|
/// the share: adding somebody grants them nothing readable, and the interface has to say so rather
|
|
/// than let a user believe the credential is already with their colleague.
|
|
/// </remarks>
|
|
[Fact]
|
|
public async Task CreatingATeamAndSharingItsVault_WrapsTheKeyToTheOtherMember()
|
|
{
|
|
await UnlockedAsync();
|
|
|
|
var teams = shell.Teams;
|
|
var colleague = server.AddAccount("bob@example.com", "Bob Example");
|
|
|
|
await CreateTeamAsync(teams, "Platform", "platform");
|
|
|
|
await teams.CreateVaultCommand.ExecuteAsync(null);
|
|
teams.Vaults.Count.ShouldBe(1, teams.Status);
|
|
|
|
teams.InviteEmail = "bob@example.com";
|
|
await teams.AddMemberCommand.ExecuteAsync(null);
|
|
|
|
teams.Members.Count.ShouldBe(2, teams.Status);
|
|
teams.Status.ShouldContain("cannot read anything yet");
|
|
|
|
teams.SelectedMember = teams.Members.Single(member => member.UserId == colleague);
|
|
teams.SelectedVault = teams.Vaults[0];
|
|
|
|
await teams.ShareVaultCommand.ExecuteAsync(null);
|
|
|
|
var vaultId = teams.Vaults[0].VaultId;
|
|
|
|
server.IssuedGrants.ShouldContainKey((vaultId, colleague));
|
|
teams.Status.ShouldContain("Shared");
|
|
|
|
// The one thing verification cannot promise, said in the same breath as the success.
|
|
teams.Status.ShouldContain("fingerprint", Case.Insensitive);
|
|
}
|
|
|
|
/// <remarks>
|
|
/// <para>
|
|
/// The test this whole design exists for. A server that wants to read a team's vault only has to
|
|
/// answer one directory lookup with a key it holds the private half of — so the client reads the
|
|
/// append-only key log, verifies its chain, and refuses to wrap anything unless the key it was
|
|
/// offered is in there unchanged.
|
|
/// </para>
|
|
/// <para>
|
|
/// Nothing may be sent. A refusal that still issued the grant, or that issued it on a retry, would be
|
|
/// worse than no check at all, because the interface would have said it was verified.
|
|
/// </para>
|
|
/// </remarks>
|
|
[Fact]
|
|
public async Task ATamperedKeyLog_StopsTheShareRatherThanWarningAboutIt()
|
|
{
|
|
await UnlockedAsync();
|
|
|
|
var teams = shell.Teams;
|
|
var colleague = server.AddAccount("mallory@example.com", "Mallory Example");
|
|
|
|
await CreateTeamAsync(teams, "Platform", "platform");
|
|
await teams.CreateVaultCommand.ExecuteAsync(null);
|
|
|
|
teams.InviteEmail = "mallory@example.com";
|
|
await teams.AddMemberCommand.ExecuteAsync(null);
|
|
|
|
teams.SelectedMember = teams.Members.Single(member => member.UserId == colleague);
|
|
teams.SelectedVault = teams.Vaults[0];
|
|
|
|
server.CorruptKeyLog = true;
|
|
|
|
await teams.ShareVaultCommand.ExecuteAsync(null);
|
|
|
|
server.IssuedGrants.ShouldBeEmpty();
|
|
teams.Status.ShouldContain("Did not share");
|
|
teams.Status.ShouldContain("key log");
|
|
}
|
|
|
|
/// <remarks>
|
|
/// A vault created here is usable here, without a relock. The key was generated in this process, so
|
|
/// making the user lock and unlock to reach the vault they just made would be asking them to work
|
|
/// around bookkeeping.
|
|
/// </remarks>
|
|
[Fact]
|
|
public async Task ATeamVaultCreatedHere_IsImmediatelyReadableAndWritable()
|
|
{
|
|
await UnlockedAsync();
|
|
|
|
var teams = shell.Teams;
|
|
|
|
await CreateTeamAsync(teams, "Platform", "platform");
|
|
await teams.CreateVaultCommand.ExecuteAsync(null);
|
|
|
|
var vaultId = teams.Vaults[0].VaultId;
|
|
var session = shell.Vault!.Session;
|
|
|
|
session.ReadableVaults.Select(vault => vault.VaultId).ShouldContain(vaultId);
|
|
|
|
// And it is offered as somewhere to file a new item, which is what makes it worth having.
|
|
await shell.Vault.LoadAsync(Token);
|
|
|
|
shell.Vault.TargetVaults.Select(choice => choice.VaultId).ShouldContain(vaultId);
|
|
shell.Vault.HasVaultChoice.ShouldBeTrue();
|
|
}
|
|
|
|
/// <remarks>
|
|
/// Filing into a team vault has to be chosen and has to stick. The bug this guards is the obvious
|
|
/// one: an editor that read the picker at save time rather than at open time, so changing the picker
|
|
/// with a half-typed host on screen would move it.
|
|
/// </remarks>
|
|
[Fact]
|
|
public async Task AHostFiledIntoATeamVault_StaysThere()
|
|
{
|
|
await UnlockedAsync();
|
|
|
|
var teams = shell.Teams;
|
|
|
|
await CreateTeamAsync(teams, "Platform", "platform");
|
|
await teams.CreateVaultCommand.ExecuteAsync(null);
|
|
|
|
var vault = shell.Vault!;
|
|
var teamVaultId = teams.Vaults[0].VaultId;
|
|
|
|
await vault.LoadAsync(Token);
|
|
|
|
vault.SelectedTargetVault =
|
|
vault.TargetVaults.Single(choice => choice.VaultId == teamVaultId);
|
|
|
|
vault.NewHostCommand.Execute(null);
|
|
vault.EditorLabel = "prod-db";
|
|
vault.EditorHostname = "db.internal";
|
|
vault.EditorUsername = "deploy";
|
|
|
|
// Moved back after the editor opened. The host must still land in the team's vault.
|
|
vault.SelectedTargetVault =
|
|
vault.TargetVaults.First(choice => choice.VaultId != teamVaultId);
|
|
|
|
await vault.SaveHostCommand.ExecuteAsync(null);
|
|
|
|
var row = vault.Hosts.Single(
|
|
host => string.Equals(host.Label, "prod-db", StringComparison.Ordinal));
|
|
row.VaultId.ShouldBe(teamVaultId);
|
|
}
|
|
|
|
/// <remarks>
|
|
/// The mirror image of the host test above, and it goes the other way on purpose. A host filed into a
|
|
/// team vault has to stay there, because hosts are read across every readable vault and so come back.
|
|
/// Tags are not — the editable list is the active vault's alone, like groups and buckets — so a tag
|
|
/// filed anywhere else would be created, pushed, reported as added and then invisible, with nothing on
|
|
/// the keychain screen able to rename or delete it and no active-vault switcher to go and find it with.
|
|
/// </remarks>
|
|
[Fact]
|
|
public async Task ATagIgnoresTheTargetPicker_BecauseItsListOnlyEverShowsOneVault()
|
|
{
|
|
await UnlockedAsync();
|
|
|
|
var teams = shell.Teams;
|
|
|
|
await CreateTeamAsync(teams, "Platform", "platform");
|
|
await teams.CreateVaultCommand.ExecuteAsync(null);
|
|
|
|
var teamVaultId = teams.Vaults[0].VaultId;
|
|
var vault = shell.Vault!;
|
|
|
|
await vault.LoadAsync(Token);
|
|
|
|
vault.HasVaultChoice.ShouldBeTrue("this test is meaningless with one vault");
|
|
|
|
vault.SelectedTargetVault = vault.TargetVaults.Single(
|
|
choice => choice.VaultId == teamVaultId);
|
|
|
|
vault.NewTagCommand.Execute(null);
|
|
vault.TagEditorLabel = "eu-west-1";
|
|
await vault.SaveTagCommand.ExecuteAsync(null);
|
|
|
|
vault.Tags.ShouldHaveSingleItem().Label
|
|
.ShouldBe("eu-west-1", "a tag that is not in the list is a tag nothing can reach");
|
|
}
|
|
|
|
private async Task CreateTeamAsync(TeamsViewModel teams, string name, string slug)
|
|
{
|
|
await teams.LoadAsync(Token);
|
|
|
|
teams.NewTeamCommand.Execute(null);
|
|
teams.NewTeamName = name;
|
|
teams.NewTeamSlug = slug;
|
|
|
|
await teams.CreateTeamCommand.ExecuteAsync(null);
|
|
|
|
teams.SelectedTeam.ShouldNotBeNull(teams.Status);
|
|
}
|
|
|
|
/// <remarks>
|
|
/// The whole path rather than a shortcut into the unlocked state, because sharing needs an identity
|
|
/// key that was really enrolled: the fake server publishes it into its key log during enrollment, and
|
|
/// that entry is what the client verifies its own directory answer against.
|
|
/// </remarks>
|
|
private async Task UnlockedAsync()
|
|
{
|
|
await shell.StartAsync(Token);
|
|
await shell.SignInCommand.ExecuteAsync(null);
|
|
|
|
shell.Passphrase = Passphrase;
|
|
shell.ConfirmPassphrase = Passphrase;
|
|
await shell.EnrollCommand.ExecuteAsync(null);
|
|
|
|
shell.RecoveryCodeWrittenDown = true;
|
|
shell.ConfirmRecoveryCodeCommand.Execute(null);
|
|
|
|
shell.Passphrase = Passphrase;
|
|
await shell.UnlockCommand.ExecuteAsync(null);
|
|
|
|
shell.State.ShouldBe(ShellState.Unlocked, shell.StatusMessage);
|
|
}
|
|
}
|