diff --git a/src/DodoSSH.Client.App/Views/HostsScreen.axaml b/src/DodoSSH.Client.App/Views/HostsScreen.axaml index d91eba9..97bdbb0 100644 --- a/src/DodoSSH.Client.App/Views/HostsScreen.axaml +++ b/src/DodoSSH.Client.App/Views/HostsScreen.axaml @@ -683,7 +683,7 @@ TextTrimming never fires and the name would run on past the card's own border instead of trimming. See the longer note this carried in v4. --> - + @@ -708,17 +708,27 @@ + - + + + + - - + - + @@ -767,12 +782,13 @@ + Foreground="{StaticResource TextFaint}" + TextTrimming="CharacterEllipsis" /> - + - @@ -290,17 +290,23 @@ Margin="8,0,8,10"> - - - + + + - - + VerticalAlignment="Center" Margin="0,0,10,0" /> + + + Foreground="{StaticResource TextGhost}" VerticalAlignment="Center" + TextTrimming="CharacterEllipsis" /> diff --git a/src/DodoSSH.Client.App/Views/MainWindow.axaml.cs b/src/DodoSSH.Client.App/Views/MainWindow.axaml.cs index e802611..cdc73c1 100644 --- a/src/DodoSSH.Client.App/Views/MainWindow.axaml.cs +++ b/src/DodoSSH.Client.App/Views/MainWindow.axaml.cs @@ -244,6 +244,13 @@ internal sealed partial class MainWindow : Window viewModel.CloseImportCommand.Execute(null); e.Handled = true; } + // The members dialog on Settings → Vaults, by the same rule: it is drawn over the page, so it goes + // first and settings stays. + else if (e.Key == Key.Escape && viewModel.IsSettingsMode && viewModel.Vaults.IsMembersPanelOpen) + { + viewModel.Vaults.CloseMembersPanelCommand.Execute(null); + e.Handled = true; + } else if (e.Key == Key.Escape && viewModel.IsSettingsMode) { viewModel.LeaveSettingsCommand.Execute(null); diff --git a/src/DodoSSH.Client.App/Views/SettingsSecurityPage.axaml b/src/DodoSSH.Client.App/Views/SettingsSecurityPage.axaml index 7550f3d..778b49b 100644 --- a/src/DodoSSH.Client.App/Views/SettingsSecurityPage.axaml +++ b/src/DodoSSH.Client.App/Views/SettingsSecurityPage.axaml @@ -46,19 +46,21 @@ - - + + + - + - - + diff --git a/src/DodoSSH.Client.Shell/ViewModels/HetznerImportViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/HetznerImportViewModel.cs index 51d08fb..0dd0a42 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/HetznerImportViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/HetznerImportViewModel.cs @@ -566,7 +566,7 @@ internal sealed partial class HetznerImportViewModel( // and visible rather than harmless and confusing. Refresh(resetSelection: true); - Status = WhatWasStored(outcome.Hosts); + Status = WhatWasStored(outcome.Hosts) + outcome.Caveats; Step = HetznerImportStep.Done; } diff --git a/src/DodoSSH.Client.Shell/ViewModels/ImportViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/ImportViewModel.cs index 75ffa46..691875e 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/ImportViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/ImportViewModel.cs @@ -22,10 +22,27 @@ namespace DodoSSH.Client.Shell.ViewModels; /// internal sealed record ImportedHostRequest(HostSecret Host, SshKeySecret? Key, string? KeyPath); -/// What an import stored. +/// What an import stored, and what it did not. /// Hosts written. /// Private keys written. Counts files read, not hosts bound to one. -internal sealed record ImportOutcome(int Hosts, int Keys); +/// Hosts skipped because they would not validate — no hostname, a bad port. +/// Why the batch stopped before the end, or null if it did not. +/// +/// The last two are what the importing screen needs to not claim a success it did not have. The vault's own +/// status line says these things too, but it is hidden in settings mode, which is where both importers live. +/// +internal sealed record ImportOutcome(int Hosts, int Keys, int Refused = 0, string? Failure = null) +{ + /// What to add after the screen's own sentence about what was stored. + internal string Caveats => + (Refused switch + { + 0 => string.Empty, + 1 => " 1 was skipped because it is not a host this keychain can connect to.", + _ => $" {Refused} were skipped because they are not hosts this keychain can connect to.", + }) + + (Failure is { } failure ? $" The import stopped early: {failure}" : string.Empty); +} /// One host an ssh_config offered, as a row somebody decides about. /// @@ -409,7 +426,7 @@ internal sealed partial class ImportViewModel( _ => $" {outcome.Keys} private keys came with them, and every host that named one is bound to it.", }; - return $"Imported {outcome.Hosts} host(s). They are on the Hosts screen.{material}"; + return $"Imported {outcome.Hosts} host(s). They are on the Hosts screen.{material}{outcome.Caveats}"; } /// diff --git a/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs index a67d7ff..e84e703 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs @@ -1795,6 +1795,10 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp ActiveSettingsPage = null; IsImportOpen = false; IsHetznerImportOpen = false; + + // Drawn over Settings → Vaults. Left open, it was still up the next time Settings opened, over + // whichever page that was. + vaults.IsMembersPanelOpen = false; Screen = settingsReturnScreen; Surface = settingsReturnSurface; } @@ -5107,6 +5111,14 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp Screen = ShellScreen.Hosts; Surface = ShellSurface.Page; } + else + { + // Settings is a mode of the unlocked window, and it brings its own title bar while the main one + // is hidden. Locking or signing out from it — LOCK NOW is on Preferences, sign-out on Account — + // left the window in settings mode under the lock card: no title bar, so nothing to drag, minimise + // or close, and an unlock that came back to Settings rather than to Hosts. + LeaveSettings(); + } } /// diff --git a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs index 97f226e..b50889b 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs @@ -8838,53 +8838,94 @@ internal sealed partial class VaultViewModel( ArgumentNullException.ThrowIfNull(requests); var target = vaultId ?? session.ActiveVaultId; - var hosts = 0; - var keys = 0; - var refused = 0; + var tally = new ImportTally(); + var ran = false; + string? failure = null; await RunAsync( requests.Count == 1 ? "Importing 1 host…" : $"Importing {requests.Count} hosts…", async () => { - // Keyed on the path rather than on the material, and filled as the batch runs rather than up - // front: the second host naming a file has to land on the id the first one produced, and - // nothing is reloaded until the batch finishes. - var bound = new Dictionary(StringComparer.OrdinalIgnoreCase); + ran = true; - foreach (var request in requests) + try { - if (!request.Host.TryValidate(out _)) - { - refused++; - continue; - } - - var (keyId, created) = await BindImportedKeyAsync( - request, bound, target, cancellationToken) - .ConfigureAwait(true); - - if (created) - { - keys++; - } - - var host = keyId is { } id ? request.Host with { SshKeyId = id } : request.Host; - - await session.Hosts - .CreateAsync(target, host, cancellationToken) - .ConfigureAwait(true); - - hosts++; + await ImportEachAsync(requests, target, tally, cancellationToken).ConfigureAwait(true); + } + catch (Exception exception) when (exception is not OutOfMemoryException + and not OperationCanceledException) + { + // Reported, and the hosts written before it are still reloaded below: they are in the + // keychain whether or not the rest followed, and the screen has to say both. + failure = exception.Message; } await ReloadAsync(cancellationToken).ConfigureAwait(true); - Status = DescribeImport(hosts, keys, refused); + Status = DescribeImport(tally.Hosts, tally.Keys, tally.Refused) + + (failure is null ? string.Empty : $" The import stopped early: {failure}"); }).ConfigureAwait(true); + if (!ran) + { + // RunAsync returns at once while anything else is running — a sync pass, a save — and the + // screen asking would otherwise have said it imported nothing as though that were the result. + failure = "the keychain was busy with something else. Nothing was imported; try again."; + } + await AutoSyncAsync(cancellationToken).ConfigureAwait(true); - return new ImportOutcome(hosts, keys); + return new ImportOutcome(tally.Hosts, tally.Keys, tally.Refused, failure); + } + + /// Writes each host of an import, and any key it brought, counting as it goes. + /// + /// Counted into rather than returned, so that when a write fails partway the + /// caller still knows how many landed before it — which is what it has to tell the person. + /// + private async Task ImportEachAsync( + IReadOnlyList batch, + Guid into, + ImportTally tally, + CancellationToken token) + { + // Keyed on the path rather than on the material, and filled as the batch runs rather than up + // front: the second host naming a file has to land on the id the first one produced, and nothing + // is reloaded until the batch finishes. + var bound = new Dictionary(StringComparer.OrdinalIgnoreCase); + + foreach (var request in batch) + { + if (!request.Host.TryValidate(out _)) + { + tally.Refused++; + continue; + } + + var (keyId, created) = await BindImportedKeyAsync(request, bound, into, token) + .ConfigureAwait(true); + + if (created) + { + tally.Keys++; + } + + var host = keyId is { } id ? request.Host with { SshKeyId = id } : request.Host; + + await session.Hosts.CreateAsync(into, host, token).ConfigureAwait(true); + + tally.Hosts++; + } + } + + /// What one import has written so far. + private sealed class ImportTally + { + internal int Hosts { get; set; } + + internal int Keys { get; set; } + + internal int Refused { get; set; } } /// Finds or stores the key an imported host should be bound to. diff --git a/src/DodoSSH.Client.Shell/ViewModels/VaultsViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/VaultsViewModel.cs index 8e0184a..27017e7 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/VaultsViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/VaultsViewModel.cs @@ -1639,11 +1639,32 @@ internal sealed partial class VaultsViewModel( } // Fire-and-forget on purpose, and the only place in this class that is: selection changes come - // from a list box, which has no cancellation token and no way to await. Wrapped here rather than - // inside the read, because the read is also awaited from inside a reload that already holds the - // busy gate — and a second RunAsync there would find the gate held by its own caller and skip the - // members list entirely. - _ = RunAsync(() => LoadSelectedAsync(CancellationToken.None)); + // from a list box, which has no cancellation token and no way to await. + // + // ◆ Not through RunAsync. Its busy gate returns at once while anything else runs — the first load, + // a share, a reload — so a vault chosen in that moment never had its people read, and the panel + // went on showing the previous vault's under the new one's name, with SHARE and REVOKE acting on + // that mixture. The read does not need the gate: LoadSelectedAsync bumps the selection generation, + // and every answer for an earlier selection is discarded by it. + _ = LoadSelectionAsync(); + } + + /// Reads the newly selected vault's people, turning a failure into a sentence. + private async Task LoadSelectionAsync() + { + try + { + await LoadSelectedAsync(CancellationToken.None).ConfigureAwait(true); + } + catch (DodoSshApiException exception) + { + Status = exception.Message; + } + catch (Exception exception) when (exception is not OutOfMemoryException + and not OperationCanceledException) + { + Status = exception.Message; + } } partial void OnPendingActionChanged(VaultActionRequest? value) diff --git a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs index b8538da..c5445f1 100644 --- a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs +++ b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs @@ -1871,6 +1871,31 @@ public sealed partial class ShellFlowTests : IAsyncLifetime /// connect and restored afterwards — and the dial itself goes through the ordinary connect path, so the /// host key gate and the log entry are the ones every other connection gets. /// + /// + /// A sync pass reloads the vault, which empties its host list and fills it again — and the picker used to + /// re-find its selection from the empty moment, landing on the first host every minute. CONNECT then + /// dialled that host with the password typed for the one chosen. + /// + [Fact] + public async Task TheFilesHostPicker_KeepsTheChosenHostAndItsPasswordAcrossAVaultReload() + { + var vault = await ReadyToConnectAsync(); + await AddHostAsync(vault, "staging-db"); + + shell.Transfers.Attach(vault, knownHosts); + + var chosen = shell.Transfers.Hosts.Single(row => row.Host.Label == "staging-db"); + shell.Transfers.SelectedHost = chosen; + shell.Transfers.TypedPassword = "typed for staging"; + + shell.Transfers.Hosts[0].ShouldNotBe(chosen, "otherwise falling back to the first proves nothing"); + + await vault.LoadAsync(Token); + + shell.Transfers.SelectedHost.ShouldNotBeNull().EntityId.ShouldBe(chosen.EntityId); + shell.Transfers.TypedPassword.ShouldBe("typed for staging"); + } + [Fact] public async Task ReconnectingALostFileSession_DialsAgainAndGoesBackToTheDirectoryItWasShowing() { @@ -5705,6 +5730,16 @@ public sealed partial class ShellFlowTests : IAsyncLifetime .Single(row => string.Equals(row.Alias, "web-01", StringComparison.Ordinal)) .IsSelected.ShouldBeTrue(); + // A keychain busy with something else — a sync pass, a save — used to skip the whole batch and let this + // screen say "Imported 0 host(s)" as though that were the result. + vault.IsBusy = true; + + await import.ImportCommand.ExecuteAsync(null); + + vault.IsBusy = false; + vault.Hosts.Count.ShouldBe(1); + import.Status.ShouldContain("Nothing was imported"); + await import.ImportCommand.ExecuteAsync(null); var imported = vault.Hosts.Single(row => string.Equals(row.Label, "web-01", StringComparison.Ordinal)); @@ -10628,6 +10663,35 @@ public sealed partial class ShellFlowTests : IAsyncLifetime shell.Screen.ShouldBe(ShellScreen.Keychain); } + /// + /// LOCK NOW is on the Preferences page. Settings mode hides the main title bar and draws its own, which + /// the lock card then covers — so a lock that stayed in settings mode left a window with nothing to drag, + /// minimise or close, and an unlock that came back to Settings rather than to Hosts. + /// + [Fact] + public async Task LockingFromSettings_LeavesSettingsModeAndUnlockingLandsOnHosts() + { + await ReadyToConnectAsync(); + shell.ShowScreenCommand.Execute(ShellScreen.Keychain); + + shell.EnterSettingsCommand.Execute(SettingsPage.Vaults); + await shell.Vaults.LoadAsync(Token); + shell.Vaults.OpenMembersPanelCommand.Execute(shell.Vaults.Vaults[0]); + shell.Vaults.IsMembersPanelOpen.ShouldBeTrue("otherwise its closing proves nothing"); + + await shell.LockCommand.ExecuteAsync(null); + + shell.State.ShouldBe(ShellState.Locked); + shell.IsSettingsMode.ShouldBeFalse("the lock card needs the main title bar above it"); + shell.Vaults.IsMembersPanelOpen.ShouldBeFalse(); + + shell.Passphrase = Passphrase; + await shell.UnlockCommand.ExecuteAsync(null); + + shell.IsSettingsMode.ShouldBeFalse(); + shell.Screen.ShouldBe(ShellScreen.Hosts); + } + /// /// Settings mode collapses the terminal the same way any other page does — /// and are exclusive by construction — and "Back to application" has diff --git a/tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs b/tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs index 43edf05..9acd93a 100644 --- a/tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs +++ b/tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs @@ -2177,10 +2177,70 @@ public sealed class VaultSharingTests : IAsyncLifetime vaults.Members.ShouldHaveSingleItem().Role.ShouldBe("OWNER"); } + /// + /// A vault chosen while anything else on the screen was running — a reload, a share — went through a busy + /// gate that answered by doing nothing, so its people were never read and the panel kept showing the + /// previous vault's under the new one's name. SHARE and REVOKE then acted on that mixture. + /// + [Fact] + public async Task AVaultChosenWhileAReloadIsRunning_ShowsItsOwnPeople() + { + await UnlockedAsync(); + + var vaults = shell.Vaults; + server.AddAccount("bob@example.com", "Bob Example"); + + await CreateVaultAsync(vaults, "Alone"); + var alone = vaults.SelectedVault!.VaultId; + + await CreateVaultAsync(vaults, "Shared with Bob"); + var shared = vaults.SelectedVault!.VaultId; + + vaults.NewMemberEmail = "bob@example.com"; + await vaults.AddMemberCommand.ExecuteAsync(null); + vaults.Members.Count.ShouldBe(2, vaults.Status); + + vaults.SelectedVault = vaults.Vaults.Single(row => row.VaultId == alone); + await EventuallyAsync(() => vaults.Members.Count == 1, "the vault on its own has only its owner"); + + var gate = new TaskCompletionSource(); + server.MemberReadGate = gate; + + var reload = vaults.LoadAsync(Token); + + vaults.IsBusy.ShouldBeTrue("the reload is what holds the screen busy while its read is held"); + + vaults.SelectedVault = vaults.Vaults.Single(row => row.VaultId == shared); + + gate.SetResult(); + server.MemberReadGate = null; + await reload; + + await EventuallyAsync(() => vaults.Members.Count == 2, "Bob is in the vault now showing"); + } + /// /// Through the form rather than straight at the command, because the name is what the form is for — /// and because the form is now the only way in: there is no separate "make a team" step behind it. /// + /// Waits for something a fire-and-forget read will make true, within a bound. + private static async Task EventuallyAsync(Func condition, string because) + { + var deadline = TimeProvider.System.GetUtcNow().AddSeconds(10); + + while (TimeProvider.System.GetUtcNow() < deadline) + { + if (condition()) + { + return; + } + + await Task.Delay(TimeSpan.FromMilliseconds(20), Token); + } + + throw new ShouldAssertException(because); + } + /// The armour a key is stored in, which this suite never parses and only round-trips. private static string PrivateKey(string body) => $"-----BEGIN OPENSSH PRIVATE KEY-----\n{body}\n-----END OPENSSH PRIVATE KEY-----\n";