diff --git a/README.md b/README.md index bfeef22..287cf3d 100644 --- a/README.md +++ b/README.md @@ -132,8 +132,9 @@ dotnet run --project src/DodoSSH.Client.App In the app, enter `http://localhost:5233` as the server. Your browser opens for sign-in — the realm ships `alice` / `alice` — then choose a vault passphrase and **write down the recovery code**, which cannot be -skipped and cannot be recovered from the server. You can then add a host and open a shell on it. Keycloak's -admin console is at `http://localhost:18080` (`admin` / `admin`). +skipped and cannot be recovered from the server. You can then add a host and open a shell on it — double-click +it in the sidebar, or select it and press **CONNECT**, which is the same command with the password box beside +it. Keycloak's admin console is at `http://localhost:18080` (`admin` / `admin`). You can also add an SSH key, which is stored in the vault like a host and synced the same way: paste the private key, then edit a host and pick that key from its **key** dropdown. From then on that host @@ -146,6 +147,29 @@ the next sync. If a server is legitimately rebuilt and offers a new key, the con with no way to continue from the warning — edit the host and choose **Forget host key**, which is deliberately somewhere you have to go on purpose. +**Deleting asks first, and the question is worth reading.** DELETE on a host, an SSH key or a stored password +puts a question where the buttons were, and what it says is counted rather than generic: how many hosts +authenticate with the key about to go — they refuse to connect afterwards rather than falling back to a typed +password — whether a terminal is open on the host about to go, and whether this machine can push the deletion +yet or is queuing it. There is no undo, which is the other thing it says. Withdrawing host key trust is the +deliberate exception: it costs one fingerprint check on the next connection, and the dangerous button there is +the one that *adds* trust. + +**Signing in once is enough.** The refresh token is kept in the local cache, sealed under the vault's own +key, so a later launch resumes the session itself and no browser opens — and because it is sealed under that +key, resuming can only happen *after* the vault is unlocked. A machine that unlocks with no network keeps +trying: every synchronisation pass asks for a connection, so a laptop opened on a train is online again +within a minute of finding a network, with nothing pressed. Unlock takes **Enter** in the passphrase box, +and nothing about unlocking ever waits on the network. + +**Signing out** is under Preferences → *Account*, and again on the unlock screen, where it is the only +answer to a forgotten passphrase — nothing can recover one. It asks first, and says what it costs: it +empties this machine's cache (the profile, the cached items, and anything still queued to be sent) and +withdraws this machine's device key from the account. The vault itself is on the server and is untouched, so +signing in again brings it all back; the count in the confirmation is the one thing that exists nowhere +else. Your session at the identity provider is *not* ended — DodoSSH has no way to end it — so on a machine +that is not yours, sign out there too. + Two of M1's known gaps are visible immediately, so they are worth expecting rather than diagnosing: password authentication asks for the password every time, because nothing in the interface can create a vault credential yet (they do sync — there is just no editor for one); and unlock asks for the passphrase on every @@ -171,6 +195,9 @@ beside its destination and is renamed into place at the end, so an interrupted t mistaken for a finished one — which matters most for what people actually use this for, which is copying a build artefact onto a server and then running it. A destination that already exists is refused outright rather than overwritten; the remote pane has **DELETE** and **MKDIR** so that refusal is not a dead end. +DELETE asks first and names the full path, and it carries the strongest warning in the application on +purpose: everything else DodoSSH deletes is a tombstone against a copy the server still holds, and a file on +somebody's host is bytes with nothing behind them. **RESUME** on a stopped transfer carries on from what the part file already holds. Resume works within a run of the application and not across a restart, and that limit is deliberate: nothing diff --git a/docs/crypto.md b/docs/crypto.md index 4ab7f77..e7f16e5 100644 --- a/docs/crypto.md +++ b/docs/crypto.md @@ -193,6 +193,16 @@ moment to discard it. The label is versioned, so a client holding a v1 cache fails to open it and re-pulls rather than decrypting to nonsense. That is the whole reason for bumping rather than reusing the label. +**What it seals, and the one entry that is not vault content.** Three kinds of record: the plaintext +columns the server needs, the values a merge overrode, and — added 2026-07-31 — the OIDC **refresh +token** this machine may resume its sign-in with, bound as `LocalCache(User, userId)`. The third is +different in kind from the other two: it is a credential for the *account*, not for the vault, and +sealing it here is a deliberate choice about what a stolen cache file is worth. A refresh token kept in +the clear beside the ciphertext would let a copied profile reach the server as its owner without the +passphrase ever being guessed; under this key it can only be read by a process that has already opened +the vault. The cost is stated rather than worked around: **a locked client cannot reach the server at +all**, because the token it would present is behind the same lock as everything else. + ### Why the bundle is wrapped many ways This is the load-bearing structural choice. Because every wrap protects the *same* bundle: diff --git a/docs/design-import-gaps.md b/docs/design-import-gaps.md index 9a9c378..2172d8d 100644 --- a/docs/design-import-gaps.md +++ b/docs/design-import-gaps.md @@ -193,8 +193,14 @@ What the row did not anticipate is that the interesting half is not the endpoint ## Preferences -The screen ships with what is real — this machine's device key, locking, and syncing — and lists the rest -as absent rather than omitting it silently. +The screen ships with what is real — this machine's device key, locking, syncing, and signing out — and +lists the rest as absent rather than omitting it silently. + +> **Signing out is not on the design and is here anyway.** The design has no way to leave a machine, and +> without one there is no way to hand a laptop on, to enrol a second account, or to get past a forgotten +> passphrase — which is unrecoverable by construction, so the unlock screen would otherwise be a dead end. +> It empties the local cache and withdraws the device key from the account; it cannot end the session at the +> identity provider, and says so. | Design element | Layer | What it would take | | --- | --- | --- | diff --git a/src/DodoSSH.Client.App/App.axaml.cs b/src/DodoSSH.Client.App/App.axaml.cs index 814305a..cee0488 100644 --- a/src/DodoSSH.Client.App/App.axaml.cs +++ b/src/DodoSSH.Client.App/App.axaml.cs @@ -86,7 +86,14 @@ internal sealed partial class DodoSshApp : Application .SignInAsync(url, browser, TimeProvider.System, cancellationToken) .ConfigureAwait(false), TimeProvider.System, - connections); + connections, + passphraseProfile: null, + + // The other half of signing in: a refresh grant, no browser, and nobody present. It is what + // makes a launch after the first one arrive online rather than merely enrolled. + resume: async (url, refreshToken, cancellationToken) => await ServerConnection + .ResumeAsync(url, refreshToken, TimeProvider.System, cancellationToken) + .ConfigureAwait(false)); desktop.MainWindow = new MainWindow { DataContext = viewModel }; diff --git a/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs b/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs index 5a1122c..a7354af 100644 --- a/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs @@ -115,6 +115,14 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp private readonly IDeviceKeyStore deviceKeys; private readonly SignInHandler signIn; + + /// + /// Optional, and null is not merely "not configured": a shell with no resume handler is one that can + /// only be online because somebody signed in during this run, which is what every test that asserts + /// offline behaviour relies on. + /// + private readonly ResumeHandler? resume; + private readonly TimeProvider clock; private readonly Argon2Profile? passphraseProfile; @@ -129,6 +137,18 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp private readonly TeamsViewModel teams; private IVaultServer? connection; + + /// The refresh token last written to the cache, so a rotation is noticed without reading it back. + private string? rememberedToken; + + /// Guards against two resume attempts overlapping. + /// + /// A plain flag rather than a semaphore because every caller is on the UI thread — the sync loop and + /// the Sync button — and what has to be prevented is a second attempt starting while the first is + /// waiting on a token endpoint, not a data race. + /// + private bool resuming; + private bool disposed; /// @@ -142,6 +162,19 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp /// internal delegate Task SignInHandler(Uri serverUrl, CancellationToken cancellationToken); + /// + /// Re-establishes a connection from a remembered sign-in, without a browser. + /// + /// + /// A delegate for the same reason is one: a real resume needs discovery + /// and a token endpoint, and making that the only way to reach this state machine would put the whole + /// "comes back online by itself" behaviour out of reach of a test. + /// + internal delegate Task ResumeHandler( + Uri serverUrl, + string refreshToken, + CancellationToken cancellationToken); + /// /// How file-transfer sessions are opened. The same object as the connection factory in the composed /// application — one type implements both — and a separate parameter because it is a separate capability @@ -156,7 +189,8 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp SignInHandler signIn, TimeProvider clock, ISftpSessionFactory sftpSessions, - Argon2Profile? passphraseProfile = null) + Argon2Profile? passphraseProfile = null, + ResumeHandler? resume = null) { this.paths = paths; this.caches = caches; @@ -164,6 +198,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp this.knownHosts = knownHosts; this.deviceKeys = deviceKeys; this.signIn = signIn; + this.resume = resume; this.clock = clock; this.passphraseProfile = passphraseProfile; @@ -305,6 +340,15 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp internal bool IsLocked => State == ShellState.Locked; + /// Whether the unlock card itself is showing, rather than the confirmation over it. + /// + /// Its own property because the markup cannot express IsLocked && !IsConfirmingSignOut, + /// and the two cards genuinely swap rather than stack: the unlock card is already near the height the + /// window guarantees at its minimum size, so putting a second question underneath it would push + /// buttons off a screen with nothing to scroll. + /// + internal bool IsAskingForThePassphrase => IsLocked && !IsConfirmingSignOut; + internal bool IsUnlocked => State == ShellState.Unlocked; /// Whether a connection to the server is currently held. @@ -685,9 +729,23 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp AccountName = outcome.Me.DisplayName ?? outcome.Me.Email ?? outcome.Me.Subject; StatusMessage = outcome.Message; - State = outcome.Status == ProvisionStatus.EnrollmentRequired - ? ShellState.NeedsEnrollment - : ShellState.Locked; + if (outcome.Status == ProvisionStatus.EnrollmentRequired) + { + State = ShellState.NeedsEnrollment; + return; + } + + // An unlocked vault stays unlocked. This command is reachable from the preferences screen + // of a running application — it is how somebody whose sign-in expired gets back online — + // and moving the state machine to Locked there would throw an unlock screen over an open + // vault whose keys are still in memory, which is neither locked nor honest. + if (IsUnlocked) + { + await RememberSignInAsync(cancellationToken).ConfigureAwait(true); + return; + } + + State = ShellState.Locked; }).ConfigureAwait(true); } @@ -927,7 +985,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp throw; } - Vault = new VaultViewModel(session, workspace, knownHosts, () => connection); + Vault = new VaultViewModel(session, workspace, knownHosts, () => connection, ReconnectAsync); State = ShellState.Unlocked; // Offered only where it can actually be honoured: a machine that can keep a key, and a profile that @@ -955,9 +1013,190 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp // After the first load, so the list is on screen before anything talks to a server. The loop is // started from the UI thread deliberately: every pass resumes here, which is what keeps the // observable collections single-threaded. + // + // Its first pass is also what brings this machine online: the pass asks ReconnectAsync for a + // server, and that is where a remembered sign-in is resumed. Nothing here has to know whether + // this unlock followed a sign-in or a cold launch on a train. + // + // Deliberately not awaited here, and not done before this point either. Resuming is a discovery + // call and a token exchange — a network round trip, and on an unreachable network a slow one — + // and unlocking must never wait on one. Everything the unlock screen promises about working + // offline stops being true the moment the passphrase leads to a socket. So the vault opens, and + // the titlebar says OFFLINE until the round trip this starts has an answer. Vault.StartAutoSync(); } + /// + /// Gets this machine online if it is not, and keeps the remembered sign-in current if it is. + /// + /// + /// + /// Handed to the vault, which asks once per synchronisation pass. That cadence is the whole design: + /// there is no connectivity monitor and no reconnect backoff, because a pass a minute already is one, + /// and a machine that comes back from a closed lid is online again within a minute of having a + /// network — with nothing pressed and no browser opened. + /// + /// + /// Resuming needs an unlocked vault, and that is deliberate rather than incidental. The + /// remembered refresh token is sealed under the vault's own cache key, so this can only succeed after + /// somebody has opened the vault — a stolen laptop yields a cache file that cannot reach the account + /// any more than it can read the hosts. + /// + /// + /// Every failure returns null and stays quiet, with one exception: a provider that refuses the + /// token is not a transient condition and will refuse it again once a minute forever, so that one is + /// said out loud and the token is dropped. + /// + /// + private async Task ReconnectAsync(CancellationToken cancellationToken) + { + if (connection is { } held) + { + await RememberSignInAsync(cancellationToken).ConfigureAwait(true); + return held; + } + + if (resume is not { } handler || resuming || Vault is not { } vault) + { + return null; + } + + resuming = true; + + try + { + return await ResumeAsync(vault, handler, cancellationToken).ConfigureAwait(true); + } + finally + { + resuming = false; + } + } + + /// + /// Split from only so the guard, the flag and the attempt are three short + /// things rather than one long one. Everything about why this behaves as it does is up there. + /// + private async Task ResumeAsync( + VaultViewModel vault, + ResumeHandler handler, + CancellationToken cancellationToken) + { + try + { + var token = await vault.Session + .ReadRememberedSignInAsync(cancellationToken) + .ConfigureAwait(true); + + if (token is null + || !Uri.TryCreate(vault.Session.Profile.ServerUrl, UriKind.Absolute, out var server)) + { + return null; + } + + var resumed = await handler(server, token, cancellationToken).ConfigureAwait(true); + + connection = resumed; + rememberedToken = token; + + OnPropertyChanged(nameof(IsOnline)); + RaiseSyncState(); + + // The refresh that just happened may have rotated the token, and the rotated one is the only + // one the next launch can use. + await RememberSignInAsync(cancellationToken).ConfigureAwait(true); + + return resumed; + } + catch (OidcException exception) + { + // The provider answered and said no: the session was revoked, or the token was rotated and + // this machine kept the old one. Retrying costs a round trip a minute and can only ever get + // the same answer, so the token goes and the user is told the one thing that fixes it. + await ForgetSignInAsync(cancellationToken).ConfigureAwait(true); + + Announce($"Your sign-in has expired, so this machine is offline: {exception.Message} " + + "Sign in again from Preferences to start syncing."); + + return null; + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + // Everything else is a machine with no network, a server that is down, or a vault that was + // locked mid-attempt — all of which are ordinary and all of which resolve themselves. The + // titlebar already says OFFLINE; a socket error once a minute would say nothing more. + return null; + } + } + + /// + /// Writes the connection's current refresh token into the vault, if it has changed. + /// + /// + /// Called on every pass rather than driven by an event, because providers rotate the token inside a + /// refresh that happens on whatever thread an API call was made from — and a value read once a minute + /// is current enough for something only a relaunch reads. A failure here costs one browser sign-in on + /// the next launch and nothing else, which is not worth interrupting anybody over. + /// + private async Task RememberSignInAsync(CancellationToken cancellationToken) + { + if (connection?.RefreshToken is not { } token + || Vault is not { } vault + || string.Equals(token, rememberedToken, StringComparison.Ordinal)) + { + return; + } + + try + { + await vault.Session.RememberSignInAsync(token, cancellationToken).ConfigureAwait(true); + rememberedToken = token; + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + // Left unremembered. The application is signed in for this run either way. + } + } + + /// Drops the remembered sign-in, so nothing tries to resume it again. + private async Task ForgetSignInAsync(CancellationToken cancellationToken) + { + rememberedToken = null; + + if (Vault is not { } vault) + { + return; + } + + try + { + await vault.Session.ForgetSignInAsync(cancellationToken).ConfigureAwait(true); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + // A cache that will not take the deletion is one the next launch will fail to resume from and + // then delete itself. Nothing here is worth a message. + } + } + + /// + /// Says something wherever the user is looking. + /// + /// + /// The shell's own message is on the setup and unlock cards, and the status bar shows the vault's — so + /// a message about the connection, which is the shell's business but only interesting while somebody + /// is using an open vault, has to go to both or it is invisible half the time. + /// + private void Announce(string message) + { + StatusMessage = message; + + if (Vault is { } vault) + { + vault.Status = message; + } + } + /// /// Closes the vault and forgets every key it held. Open shells keep running. /// @@ -1007,10 +1246,177 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp LiveSessionCount = workspace.LiveSessionCount; + // A confirmation armed on the preferences screen must not survive onto the unlock screen, where + // the same card is offered with a warning it can no longer count. + IsConfirmingSignOut = false; + State = ShellState.Locked; StatusMessage = "Locked."; } + // ---- Signing out ---- + + /// Whether the sign-out confirmation is showing. + /// + /// A state rather than a dialog, for the same reason the recovery code is a screen: this is the one + /// action in the application that destroys something a user cannot get back from here — an unpushed + /// change — and it has to be able to say what is about to go before it goes. + /// + [ObservableProperty] + private bool isConfirmingSignOut; + + /// + /// What signing out costs, on this machine, right now. + /// + /// + /// + /// The count is the part worth having. Everything else in the vault is on the server and comes back + /// with the next sign-in; an operation still in the outbox exists nowhere else in the world, and + /// "your changes will be lost" without a number leaves somebody guessing whether it means theirs. + /// + /// + /// A locked vault cannot be counted — the outbox is sealed under the key the vault holds — so it gets + /// the honest form of the same warning rather than a zero it has not earned. + /// + /// + internal string SignOutWarning => (IsUnlocked, Vault?.PendingChanges ?? 0) switch + { + (false, _) => + "Anything this machine changed and has not sent to the server yet will be lost. It cannot be " + + "counted from here, because the vault is locked.", + (true, 0) => + "Everything this machine has changed has reached the server, so nothing will be lost.", + (true, 1) => + "1 change has not reached the server yet and will be lost. Sync first to keep it.", + (true, var pending) => + $"{pending} changes have not reached the server yet and will be lost. Sync first to keep them.", + }; + + /// Asks whether the user means it. + [RelayCommand] + private void SignOut() + { + // Taken now so the card can disclose it, on the same reasoning as the lock screen's: signing out + // does not close a shell any more than locking does, and a screen that sends somebody back to + // "connect to your server" while their upgrade is still running should say so. + LiveSessionCount = workspace.LiveSessionCount; + + OnPropertyChanged(nameof(SignOutWarning)); + + IsConfirmingSignOut = true; + } + + /// Thinks better of it. + [RelayCommand] + private void CancelSignOut() => IsConfirmingSignOut = false; + + /// + /// Signs out: closes the vault, withdraws this machine, and deletes its copy of everything. + /// + /// + /// + /// What this does and does not destroy. It empties the local cache — the profile, the wrapped + /// bundle, the item mirror, the outbox and the conflict log — and forgets this machine's device key + /// here and on the account. The vault itself is on the server and is untouched, which is what makes + /// this safe to offer beside a passphrase box: somebody who has forgotten their passphrase can reset + /// this machine and sign in again, and the only thing they lose is what this machine had not yet sent. + /// + /// + /// Ordered so that a failure cannot leave a half-signed-out machine. The device is withdrawn + /// while there is still a session and a connection to withdraw it through; the vault is closed before + /// the cache under it is emptied; and the cache is emptied last, because it is the step that makes + /// this machine unenrolled and everything before it is a courtesy that a wiped profile makes moot. + /// + /// + /// It does not end the session at the identity provider — there is no back channel to it from here, + /// and pretending otherwise would be the sort of claim this project writes down instead of implying. + /// The refresh token this machine held is dropped and never used again; the provider's own session + /// outlives it, which is what the preferences screen says out loud. + /// + /// + [RelayCommand] + private async Task ConfirmSignOutAsync(CancellationToken cancellationToken) + { + await RunAsync( + "Signing out…", + async () => + { + IsConfirmingSignOut = false; + + await WithdrawThisMachineAsync(cancellationToken).ConfigureAwait(true); + + // As Lock does, and before the session it reads from goes. + knownHosts.Close(); + + // The same detach locking does, and the same reasoning carried one step further: the host + // rows go because the vault behind them is about to be disposed, and the session and its + // queue stay because a transfer in flight is somebody's work. Signing out is the strongest + // thing this application does to itself and it still does not destroy that, for exactly the + // reason it does not close a shell — quitting DodoSSH is what ends both. + transfers.Detach(); + + if (Vault is { } open) + { + Vault = null; + await open.DisposeAsync().ConfigureAwait(true); + } + + connection?.Dispose(); + connection = null; + rememberedToken = null; + + await caches.ResetAsync(cancellationToken).ConfigureAwait(true); + + LiveSessionCount = workspace.LiveSessionCount; + + AccountName = null; + Passphrase = string.Empty; + ConfirmPassphrase = string.Empty; + RecoveryCode = null; + RecoveryCodeWrittenDown = false; + CanUnlockWithDevice = false; + CanRegisterDevice = false; + CanForgetDevice = false; + + State = ShellState.NeedsServer; + + OnPropertyChanged(nameof(IsOnline)); + RaiseSyncState(); + + StatusMessage = "Signed out. This machine's copy of the vault has been deleted; the vault " + + "itself is untouched. Sign in to set this machine up again."; + }).ConfigureAwait(true); + } + + /// + /// Best effort, and swallowed on purpose. Withdrawing the device is the tidy half of signing out — the + /// half that stops the account listing a machine whose key is about to be deleted — and a server that + /// cannot be reached, or a keystore that declines, must not be able to strand somebody on a screen + /// they asked to leave. The half that decides whether this machine can let itself in happens anyway, + /// because the profile holding the wrap is emptied a moment later. + /// + private async Task WithdrawThisMachineAsync(CancellationToken cancellationToken) + { + try + { + if (Vault is { } vault) + { + await vault.Session + .ForgetDeviceAsync(connection?.Account, deviceKeys, cancellationToken) + .ConfigureAwait(true); + + await vault.Session.ForgetSignInAsync(cancellationToken).ConfigureAwait(true); + return; + } + + await deviceKeys.ForgetAsync(cancellationToken).ConfigureAwait(true); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + // Nothing to report: the wipe below is what signing out actually is. + } + } + /// public async ValueTask DisposeAsync() { @@ -1184,6 +1590,11 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp { OnPropertyChanged(nameof(IsFullySynced)); OnPropertyChanged(nameof(SyncLabel)); + + // The same fact from a third direction: what signing out would cost is the outbox depth, and a + // confirmation card left showing a count from before the last pass would be quoting a number that + // has since been sent. + OnPropertyChanged(nameof(SignOutWarning)); } /// @@ -1290,8 +1701,10 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp OnPropertyChanged(nameof(IsNeedingEnrollment)); OnPropertyChanged(nameof(IsShowingRecoveryCode)); OnPropertyChanged(nameof(IsLocked)); + OnPropertyChanged(nameof(IsAskingForThePassphrase)); OnPropertyChanged(nameof(IsUnlocked)); OnPropertyChanged(nameof(IsTerminalShowing)); + OnPropertyChanged(nameof(SignOutWarning)); RaiseSyncState(); // Locking leaves the rail wherever it was, and unlocking should not resume on the vault's key list. @@ -1328,6 +1741,13 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp partial void OnIsSearchingChanged(bool value) => OnPropertyChanged(nameof(IsTerminalShowing)); + /// + /// The unlock card and the confirmation swap, so arming one has to hide the other — see + /// . + /// + partial void OnIsConfirmingSignOutChanged(bool value) => + OnPropertyChanged(nameof(IsAskingForThePassphrase)); + partial void OnCanRegisterDeviceChanged(bool value) => OnPropertyChanged(nameof(HasNoDeviceKeyOption)); diff --git a/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs b/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs index d3a2bbf..a275d13 100644 --- a/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs @@ -37,6 +37,12 @@ internal sealed class RemoteEntryRowViewModel(SftpEntry entry) /// The mode as drwxr-xr-x, which is the design's PERMS column. internal string Permissions => entry.Permissions; + + /// Whether the row is a file with an execute bit, which the NAME column colours for. + internal bool IsExecutable => entry.IsExecutable; + + /// Whether the row is a file anyone may write to, which the PERMS column colours for. + internal bool IsWorldWritable => entry.IsWorldWritable; } /// One local file or directory, as a row. @@ -174,6 +180,40 @@ internal sealed partial class TransferRowViewModel(TransferSnapshot snapshot) : } } +/// +/// Something on the host that has been asked about and not yet agreed to. +/// +/// +/// +/// The one deletion in this application that nothing can walk back. A vault item is a tombstone against a +/// copy the server still holds until the pass lands; a file on somebody's host is bytes, and this screen +/// has no wastebasket to put them in. +/// +/// +/// It carries the full path rather than only the name, because the name is the half that does not identify +/// anything: config in the directory that was showing a moment ago and config in the one +/// showing now look identical in a confirmation, and only one of them is the file somebody meant. +/// +/// +/// What the row was called. +/// Where it is, which is what the question actually promises to delete. +/// Whether it is a directory, which the host treats differently. +internal sealed record RemoteDeletionRequest(string Name, string FullPath, bool IsDirectory) +{ + /// The question, naming the kind because the two behave differently. + internal string Question => IsDirectory + ? $"Delete the directory '{Name}' on the host?" + : $"Delete '{Name}' on the host?"; + + /// What it costs, which is everything: there is no copy here and no undo there. + internal string Consequence => IsDirectory + ? "It is removed on the host itself. The host refuses a directory that still has anything in it, so " + + "this either removes an empty one or fails — and if it goes, it is gone: nothing here keeps a " + + "copy and there is no undo." + : "It is removed on the host itself. Nothing here keeps a copy, the folder on this machine is not " + + "touched, and there is no undo."; +} + /// /// The transfers screen: a host, two directory panes, and the queue between them. /// @@ -284,6 +324,20 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo internal bool HasRemoteEntries => RemoteEntries.Count > 0; + /// The deletion on the host that has been asked about, or null when none has. + [ObservableProperty] + private RemoteDeletionRequest? pendingRemoteDeletion; + + internal bool IsConfirmingRemoteDeletion => PendingRemoteDeletion is not null; + + /// Whether the pane's DELETE is live. + /// + /// Off while its own question is up, so a second press cannot arm a second one behind the card — and + /// disabled rather than hidden, because this button sits in a row of three and a gap where it was would + /// move UP and REFRESH out from under the pointer. + /// + internal bool CanDeleteRemote => IsConnected && !IsConfirmingRemoteDeletion; + // ---- The local pane ---- [ObservableProperty] @@ -672,15 +726,16 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo } /// - /// Deletes the chosen remote file, or an empty directory. + /// Asks whether the chosen remote file, or empty directory, should go. /// /// - /// Not recursive, and the refusal comes from the server rather than from a check here — see - /// ISftpSession.DeleteAsync. It is offered because the queue refuses to overwrite: without a way - /// to remove what is in the way, "that file is already there" would be a dead end. + /// Deleting on the host is offered because the queue refuses to overwrite: without a way to remove what + /// is in the way, "that file is already there" would be a dead end. It is asked about first because of + /// what it is — the only thing this application destroys that neither the server nor this machine has a + /// copy of. /// [RelayCommand] - private async Task DeleteRemoteAsync(CancellationToken cancellationToken) + private void DeleteRemote() { if (SelectedRemoteEntry is not { } row) { @@ -688,18 +743,44 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo return; } + PendingRemoteDeletion = new RemoteDeletionRequest(row.Name, row.FullPath, !row.IsFile); + } + + /// + /// Deletes what was agreed to. + /// + /// + /// Not recursive, and the refusal comes from the server rather than from a check here — see + /// ISftpSession.DeleteAsync. It acts on the path the question named rather than on the selection, + /// which is what makes the question a promise: nothing between asking and answering can point it + /// somewhere else. + /// + [RelayCommand] + private async Task ConfirmDeleteRemoteAsync(CancellationToken cancellationToken) + { + if (PendingRemoteDeletion is not { } request) + { + return; + } + + PendingRemoteDeletion = null; + await RunAsync( - $"Deleting {row.Name}…", + $"Deleting {request.Name}…", async () => { - await RequireSession().DeleteAsync(row.FullPath, cancellationToken).ConfigureAwait(true); + await RequireSession().DeleteAsync(request.FullPath, cancellationToken).ConfigureAwait(true); await ListRemoteAsync(RemotePath, cancellationToken).ConfigureAwait(true); - Status = $"Deleted {row.Name}."; + Status = $"Deleted {request.Name}."; }).ConfigureAwait(true); } + /// Thinks better of it. + [RelayCommand] + private void CancelDeleteRemote() => PendingRemoteDeletion = null; + /// public async ValueTask DisposeAsync() { @@ -919,11 +1000,28 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo { OnPropertyChanged(nameof(CanDownload)); OnPropertyChanged(nameof(CanUpload)); + OnPropertyChanged(nameof(CanDeleteRemote)); } - partial void OnSelectedRemoteEntryChanged(RemoteEntryRowViewModel? value) => + /// + /// Any change to the selection takes the question away, which is stricter than the vault's rule and can + /// afford to be: this list is refilled only by a navigation or a refresh somebody asked for, so there is + /// no background pass to pull a card out from under a reader. Listing and disconnecting both null the + /// selection, so this one hook covers all three ways the answer could stop being about what was asked. + /// + partial void OnSelectedRemoteEntryChanged(RemoteEntryRowViewModel? value) + { OnPropertyChanged(nameof(CanDownload)); + PendingRemoteDeletion = null; + } + + partial void OnPendingRemoteDeletionChanged(RemoteDeletionRequest? value) + { + OnPropertyChanged(nameof(IsConfirmingRemoteDeletion)); + OnPropertyChanged(nameof(CanDeleteRemote)); + } + partial void OnSelectedLocalEntryChanged(LocalEntryRowViewModel? value) => OnPropertyChanged(nameof(CanUpload)); diff --git a/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs index 3685664..1a2ee37 100644 --- a/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs @@ -494,6 +494,73 @@ internal sealed record VaultItemRowViewModel( internal bool HasBadge => Badge.Length > 0; } +/// Which list a deletion that has been asked for is aimed at. +internal enum DeletionTarget +{ + /// A host, from the sidebar beside the terminal. + Host, + + /// An SSH key, from the vault screen. + Key, + + /// A stored password, from the vault screen. + Credential, +} + +/// +/// A deletion that has been asked for and not yet agreed to. +/// +/// +/// +/// A state rather than a dialog, on the same reasoning as the sign-out confirmation — see +/// MainWindowViewModel.IsConfirmingSignOut. What makes it worth having at all is that the sentences +/// below are computed: how many hosts authenticate with the key about to go, whether a terminal is +/// open on the host about to go, and whether this machine can push the tombstone yet. A confirmation that +/// only said "are you sure?" would be a click to train people out of. +/// +/// +/// It carries the item's id rather than pointing at the selection, so that whatever moves the selection +/// between the question and the answer — a background sync, a filter, a click in the list — cannot turn an +/// agreement about one item into the deletion of another. +/// +/// +/// Which list to delete from. +/// The item the question is about. +/// The question itself, naming the item. +/// Where it goes, and how far. +/// +/// What is riding on this particular item — hosts that authenticate with it, a terminal open on it — or +/// empty when nothing is. The line that changes the answer, as opposed to the one every deletion shares. +/// +internal sealed record DeletionRequest( + DeletionTarget Target, + Guid EntityId, + string Question, + string Consequence, + string Usage) +{ + /// Whether anything depends on the item, which is the line worth reading twice. + internal bool HasUsage => Usage.Length > 0; +} + +/// +/// Gets this machine online, if it can be. +/// +/// +/// +/// Supplied by the shell, which owns the connection and the remembered sign-in behind it. It is asked +/// once per synchronisation pass rather than once per vault, and that is what makes coming back from a +/// closed lid automatic: a laptop that unlocks on a train has no connection and gets one within a minute +/// of reaching a network, with nothing pressed. +/// +/// +/// Returns null for every reason a machine may be offline — no remembered sign-in, no network, a token +/// the provider has stopped accepting — because the vault's answer to all of them is the same: work +/// locally and queue. +/// +/// +internal delegate Task ServerReconnectHandler(CancellationToken cancellationToken); + /// /// An open vault: the host list, the editor, syncing, and connecting a terminal. /// @@ -510,6 +577,12 @@ internal sealed record VaultItemRowViewModel( /// A background pass is deliberately quieter than the button: see . /// /// +/// Being offline is a state a pass tries to leave, not one it gives up on. Every pass asks the +/// shell for a connection rather than reading one it was handed at unlock — see +/// — so a machine that unlocked with no network comes online by +/// itself once it has one, and a sign-in survives a restart without a browser opening. +/// +/// /// Everything a connection needs is in the vault. Keys, passwords and host key trust are all synced /// items, so each is stored once and available on every machine — approving a fingerprint here approves it on /// every device and survives a restart. A typed password is what is left when a host is bound to nothing, and @@ -528,7 +601,8 @@ internal sealed partial class VaultViewModel( VaultSession session, TerminalWorkspace workspace, VaultKnownHostStore knownHosts, - Func connection) : ObservableObject, IAsyncDisposable + Func connection, + ServerReconnectHandler? reconnect = null) : ObservableObject, IAsyncDisposable { /// /// A minute. The pull is a delta keyed on a cursor, so an idle pass is one small request and costs the @@ -958,6 +1032,32 @@ internal sealed partial class VaultViewModel( /// The credential being edited, or null when creating. private Guid? editingCredentialId; + // ---- Deleting ---- + + /// The deletion that has been asked for, or null when nothing has been. + /// + /// One at a time, and one for all three kinds. Two armed deletions cannot be told apart by a user + /// looking at two cards, and this application only ever has one selected item per screen to aim a + /// question at. + /// + [ObservableProperty] + private DeletionRequest? pendingDeletion; + + internal bool IsConfirmingDeletion => PendingDeletion is not null; + + /// Whether the sidebar's row of host buttons is showing. + /// + /// Its own property because the markup cannot express !IsEditing && !IsConfirmingDeletion, + /// and because both halves are the same rule: the question about deleting a host takes the place of the + /// buttons that asked it, so that DELETE cannot be pressed a second time while its own confirmation is + /// on screen. + /// + internal bool ShowsHostActions => !IsEditing && !IsConfirmingDeletion; + + /// Whether the vault screen's Edit and Delete are showing. + /// + internal bool ShowsItemActions => SelectedItemIsEditable && !IsConfirmingDeletion; + // ---- Connecting ---- /// @@ -1227,7 +1327,7 @@ internal sealed partial class VaultViewModel( /// How many keys would not decrypt. /// /// Unlike the host list, the selection is not defaulted to the first row: it is what - /// acts on, and a list that picked a row on every background sync would aim + /// aims at, and a list that picked a row on every background sync would point /// that button at a key nobody chose. /// private async Task ReloadKeysAsync(CancellationToken cancellationToken) @@ -1266,9 +1366,9 @@ internal sealed partial class VaultViewModel( /// How many credentials would not decrypt. /// /// An existing selection survives a reload and a reload never invents one, which is the same pair of rules - /// as the key list and matters more here. acts on the selection, so a - /// list that fell back to its first row would put a one-click deletion of somebody's password behind a - /// button they never aimed. + /// as the key list and matters more here. reads the selection, so a list + /// that fell back to its first row would point the deletion — and the question in front of it — at a + /// password nobody chose. /// private async Task ReloadCredentialsAsync(CancellationToken cancellationToken) { @@ -1364,21 +1464,27 @@ internal sealed partial class VaultViewModel( private static string Endpoint(string host, int port) => string.Create(CultureInfo.InvariantCulture, $"{host}:{port}"); - /// Runs a synchronisation pass, if there is a server to talk to. + /// Runs a synchronisation pass, if this machine can reach a server. + /// + /// The offline branch is inside rather than in front of it, because getting + /// online is now part of what this button does: resuming a remembered sign-in is a network round trip + /// and belongs under the same busy flag as the pass it leads to. + /// [RelayCommand] private async Task SyncAsync(CancellationToken cancellationToken) { - if (connection() is not { } server) - { - LastSyncFailed = true; - Status = "Offline. Changes are queued and will be sent after you sign in."; - return; - } - await RunAsync( "Synchronising…", async () => { + if (await ResolveServerAsync(cancellationToken).ConfigureAwait(true) is not { } server) + { + LastSyncFailed = true; + Status = "Offline. Changes are queued and will be sent as soon as this machine " + + "can reach the server again."; + return; + } + var report = await SyncOnceAsync(server.Sync, cancellationToken).ConfigureAwait(true); // Null means a background pass held the gate. Saying so beats reporting a sync that this @@ -1389,6 +1495,18 @@ internal sealed partial class VaultViewModel( }).ConfigureAwait(true); } + /// + /// Finds a server to sync against, getting this machine online if it is not. + /// + /// + /// The handler is asked even when a connection is already held, which looks redundant and is not: the + /// shell is the thing that persists the refresh token so a later launch can resume, and identity + /// providers rotate that token on every refresh. Asking once per pass is what keeps the remembered + /// sign-in current without an event and without this view model knowing what a token is. + /// + private Task ResolveServerAsync(CancellationToken cancellationToken) => + reconnect is null ? Task.FromResult(connection()) : reconnect(cancellationToken); + /// /// Starts syncing in the background until the vault is disposed. /// @@ -1425,13 +1543,38 @@ internal sealed partial class VaultViewModel( /// internal async Task AutoSyncAsync(CancellationToken cancellationToken) { - if (IsBusy || connection() is not { } server) + if (IsBusy) { return; } + await SyncOnOpenAsync(cancellationToken).ConfigureAwait(true); + } + + /// + /// One background synchronisation pass, run whether or not a command is in flight. + /// + /// + /// + /// The same quiet pass as without the one thing that made it useless at + /// the moment it matters most. The loop is started from inside the unlock command, so the busy flag a + /// timed pass yields to is raised by the very command that opened the vault — and the pass on open + /// therefore never ran, silently, putting the first synchronisation a full minute after unlock. + /// + /// + /// Yielding is right for every later pass, because by then a busy flag means a person is doing + /// something. It is wrong for this one, because the thing it would be yielding to is the unlock. + /// + /// + internal async Task SyncOnOpenAsync(CancellationToken cancellationToken) + { try { + if (await ResolveServerAsync(cancellationToken).ConfigureAwait(true) is not { } server) + { + return; + } + var report = await SyncOnceAsync(server.Sync, cancellationToken).ConfigureAwait(true); if (report is null) @@ -1439,16 +1582,11 @@ internal sealed partial class VaultViewModel( return; } - // A vault that failed now arrives as a report rather than as an exception, because one - // unreachable team vault must not stop the others syncing. It still has to be treated the way - // the catch below treats a total failure: the fact recorded, the message swallowed. Otherwise - // a laptop with a lid shut all afternoon replaces whatever the user was reading, once a - // minute, with the name of a vault it could not reach. - if (report.Any(vault => !vault.Succeeded)) - { - LastSyncFailed = true; - } - + // A vault that failed is recorded by SyncOnceAsync and deliberately not announced here: it + // gets the treatment the catch below gives a total failure, the fact kept and the message + // swallowed. Otherwise a laptop with a lid shut all afternoon replaces whatever the user was + // reading, once a minute, with the name of a vault it could not reach. Pressing Sync still + // names the vault and the reason, because somebody who pressed it is waiting for an answer. if (IsWorthReporting(report)) { Status = Describe(report); @@ -1491,7 +1629,11 @@ internal sealed partial class VaultViewModel( // pulled it — and then quietly stop, which reads as the feature not working. var report = await session.SyncAllAsync(api, cancellationToken).ConfigureAwait(true); - LastSyncFailed = false; + // Not unconditionally false, which it was while a pass was one vault and a failure was an + // exception. A failure is now a report — one unreachable team vault must not stop the others + // syncing — so clearing the flag here regardless would light the titlebar green over a vault + // that had just failed to sync, which is exactly the lie that flag exists to prevent. + LastSyncFailed = report.Any(vault => !vault.Succeeded); await ReloadAsync(cancellationToken).ConfigureAwait(true); @@ -1523,7 +1665,14 @@ internal sealed partial class VaultViewModel( { // A pass on open, before the first tick. A vault edited on another machine should be current by // the time the user has finished reading the list, not a minute afterwards. - await AutoSyncAsync(cancellationToken).ConfigureAwait(true); + // + // Deliberately not through AutoSyncAsync, and this is not a shortcut. This loop is started from + // inside the unlock command, so the busy flag that pass yields to is raised by the very command + // that opened the vault — and the pass on open therefore never ran at all. It was a silent + // no-op that put the first synchronisation a full minute after unlock, on the launch where + // being current matters most. The later passes keep the check: by then, a busy flag means a + // user is doing something. + await SyncOnOpenAsync(cancellationToken).ConfigureAwait(true); while (await timer.WaitForNextTickAsync(cancellationToken).ConfigureAwait(true)) { @@ -1643,26 +1792,23 @@ internal sealed partial class VaultViewModel( } } - /// Deletes whatever the selected row is. + /// Asks about deleting whatever the selected row is. /// /// Pins are not deleted from here even though they can be. Withdrawing trust applies to an endpoint /// rather than to a row — every pin for the address goes — and calling that "delete" beside two buttons /// that remove exactly one item would misdescribe it. It has its own button, named for what it does. /// [RelayCommand] - private async Task DeleteSelectedItemAsync() + private void DeleteSelectedItem() { switch (SelectedVaultItem?.Kind) { - // Null rather than a token, and deliberately: a [RelayCommand] over a method whose only - // parameter is a CancellationToken generates ExecuteAsync(object? parameter) that ignores the - // argument and supplies a token from its own source. Passing one would read as plumbing. case VaultItemKind.Key: - await DeleteKeyCommand.ExecuteAsync(null).ConfigureAwait(true); + DeleteKeyCommand.Execute(null); break; case VaultItemKind.Credential: - await DeleteCredentialCommand.ExecuteAsync(null).ConfigureAwait(true); + DeleteCredentialCommand.Execute(null); break; default: @@ -1728,15 +1874,42 @@ internal sealed partial class VaultViewModel( await AutoSyncAsync(cancellationToken).ConfigureAwait(true); } - /// Queues a tombstone for the selected host. + /// Asks whether the selected host should go. + /// + /// A terminal already open on the host is disclosed rather than prevented, because deleting a host does + /// not close one — a session outlives the row that opened it, exactly as it outlives a lock. Somebody + /// deleting a machine they are still working on should know that is what they have done. + /// [RelayCommand] - private async Task DeleteHostAsync(CancellationToken cancellationToken) + private void DeleteHost() { if (SelectedHost is not { } row) { return; } + PendingDeletion = new DeletionRequest( + DeletionTarget.Host, + row.EntityId, + $"Delete the host '{row.Label}'?", + HowFarADeletionGoes("The host and everything saved about it"), + row.IsConnected + ? "A terminal is open on this host. It stays open — deleting the host does not close it, and " + + "nothing will reopen it afterwards." + : string.Empty); + } + + /// Queues a tombstone for the host that was agreed to. + private async Task DeleteHostNowAsync(Guid entityId, CancellationToken cancellationToken) + { + if (Hosts.FirstOrDefault(row => row.EntityId == entityId) is not { } row) + { + // Gone between the question and the answer — a sync that pulled somebody else's deletion is the + // realistic way. Saying so beats a silent no-op under a card that has just been agreed to. + Status = "That host is no longer here, so nothing was deleted."; + return; + } + await RunAsync( "Deleting…", async () => @@ -1857,15 +2030,39 @@ internal sealed partial class VaultViewModel( await AutoSyncAsync(cancellationToken).ConfigureAwait(true); } - /// Queues a tombstone for the selected key. + /// Asks whether the selected key should go. + /// + /// The private key is the thing this vault holds that is least likely to exist anywhere else, which is + /// why the question says so. What it does not say is that the key is gone from the machines it was + /// installed on: deleting it here removes this vault's copy, and the authorized_keys file on a + /// server is not something this application has ever written to. + /// [RelayCommand] - private async Task DeleteKeyAsync(CancellationToken cancellationToken) + private void DeleteKey() { if (SelectedKey is not { } row) { return; } + PendingDeletion = new DeletionRequest( + DeletionTarget.Key, + row.EntityId, + $"Delete the SSH key '{row.Label}'?", + HowFarADeletionGoes("The private key, its passphrase and everything saved with them") + + " If this key is not on disk anywhere else, this is the only copy.", + HostsBoundTo(host => host.SshKeyId, row.EntityId)); + } + + /// Queues a tombstone for the key that was agreed to. + private async Task DeleteKeyNowAsync(Guid entityId, CancellationToken cancellationToken) + { + if (Keys.FirstOrDefault(row => row.EntityId == entityId) is not { } row) + { + Status = "That key is no longer here, so nothing was deleted."; + return; + } + await RunAsync( "Deleting…", async () => @@ -1984,15 +2181,32 @@ internal sealed partial class VaultViewModel( await AutoSyncAsync(cancellationToken).ConfigureAwait(true); } - /// Queues a tombstone for the selected credential. + /// Asks whether the selected credential should go. [RelayCommand] - private async Task DeleteCredentialAsync(CancellationToken cancellationToken) + private void DeleteCredential() { if (SelectedCredential is not { } row) { return; } + PendingDeletion = new DeletionRequest( + DeletionTarget.Credential, + row.EntityId, + $"Delete the password '{row.Label}'?", + HowFarADeletionGoes("The password and the account saved with it"), + HostsBoundTo(host => host.CredentialId, row.EntityId)); + } + + /// Queues a tombstone for the credential that was agreed to. + private async Task DeleteCredentialNowAsync(Guid entityId, CancellationToken cancellationToken) + { + if (Credentials.FirstOrDefault(row => row.EntityId == entityId) is not { } row) + { + Status = "That password is no longer here, so nothing was deleted."; + return; + } + await RunAsync( "Deleting…", async () => @@ -2008,6 +2222,92 @@ internal sealed partial class VaultViewModel( await AutoSyncAsync(cancellationToken).ConfigureAwait(true); } + /// Carries out the deletion that was asked about. + /// + /// Disarmed before the work rather than after it, so that the card goes the moment it is answered and a + /// second press during a slow round trip has nothing left to agree to. + /// + [RelayCommand] + private async Task ConfirmDeleteAsync(CancellationToken cancellationToken) + { + if (PendingDeletion is not { } request) + { + return; + } + + PendingDeletion = null; + + switch (request.Target) + { + case DeletionTarget.Host: + await DeleteHostNowAsync(request.EntityId, cancellationToken).ConfigureAwait(true); + break; + + case DeletionTarget.Key: + await DeleteKeyNowAsync(request.EntityId, cancellationToken).ConfigureAwait(true); + break; + + case DeletionTarget.Credential: + await DeleteCredentialNowAsync(request.EntityId, cancellationToken).ConfigureAwait(true); + break; + + default: + break; + } + } + + /// Thinks better of it. + [RelayCommand] + private void CancelDelete() => PendingDeletion = null; + + /// + /// Where a deleted item goes, and how far. + /// + /// + /// The offline branch is the same distinction saving makes, and it matters more here: a tombstone that + /// has not been pushed is a deletion the other machines have not heard about, and somebody deleting a + /// credential because it leaked should be told which of those two they have just done. + /// + private string HowFarADeletionGoes(string what) => connection() is null + ? $"{what} goes from this machine now, and from your other machines once this one is online again. " + + "There is no undo." + : $"{what} goes from this machine now, and from your other machines at the next synchronisation. " + + "There is no undo."; + + /// + /// What the hosts that authenticate with an item would be left with, or nothing when none do. + /// + /// + /// Counted rather than warned about in general terms. The number is the difference between a sentence + /// somebody reads and one they click past, and what happens next is worth stating exactly: a host bound + /// to something the vault no longer has is refused at connect time rather than quietly falling back to a + /// typed password — see . + /// + private string HostsBoundTo(Func binding, Guid entityId) + { + var bound = Hosts + .Where(row => binding(row.Host) == entityId) + .Select(row => row.Label) + .ToArray(); + + if (bound.Length == 0) + { + return string.Empty; + } + + // Three names and a count past that, because this is read in a 244-pixel column and a vault with + // twenty hosts on one key would otherwise put a paragraph of names where a warning should be. + var named = bound.Length <= 3 + ? string.Join(", ", bound) + : $"{string.Join(", ", bound.Take(3))} and {bound.Length - 3} more"; + + return bound.Length == 1 + ? $"{named} authenticates with it, and will refuse to connect rather than fall back to a typed " + + "password." + : $"{bound.Length} hosts authenticate with it — {named} — and will refuse to connect rather than " + + "fall back to a typed password."; + } + /// /// Withdraws trust from the selected pin's endpoint. /// @@ -2677,7 +2977,14 @@ internal sealed partial class VaultViewModel( /// private static bool IsWorthReporting(IReadOnlyList reports) => reports.Any(vault => vault.Succeeded - && (vault.Report!.Pulled > 0 || vault.Report.Pushed > 0 || vault.Report.NeedsAttention)); + && (vault.Report!.Pulled > 0 + || vault.Report.Pushed > 0 + || vault.Report.NeedsAttention + + // A pass that had to start over says so even when it pulled nothing, which is the one + // place this rule is broken deliberately. A machine that silently re-read a whole vault + // has had something happen to it, and the alternative is that nobody ever finds out. + || vault.Report.ResyncedFromStart)); /// /// Counts are summed across vaults, and a failure is named with its reason. Both halves @@ -2728,11 +3035,19 @@ internal sealed partial class VaultViewModel( private static string Describe(SyncReport report) { + // Said first, and in both branches, because it is the explanation for the numbers after it. A pass + // reporting "214 in" on a vault nobody has touched all week reads as something having gone wrong; + // this is what actually happened, and it needs nothing from the reader. + var replayed = report.ResyncedFromStart + ? "The server no longer recognised this machine's position, so the vault was read again from " + + "the beginning. " + : string.Empty; + if (!report.NeedsAttention) { - return report.Pulled == 0 && report.Pushed == 0 + return replayed + (report.Pulled == 0 && report.Pushed == 0 ? "Already up to date." - : $"Synchronised: {report.Pulled} in, {report.Pushed} out."; + : $"Synchronised: {report.Pulled} in, {report.Pushed} out."); } var notes = new List(); @@ -2764,7 +3079,7 @@ internal sealed partial class VaultViewModel( notes.Add("this vault was rekeyed and your access needs re-issuing"); } - return "Synchronised, but: " + string.Join("; ", notes) + "."; + return replayed + "Synchronised, but: " + string.Join("; ", notes) + "."; } private async Task RunAsync(string busyMessage, Func work) @@ -2799,6 +3114,33 @@ internal sealed partial class VaultViewModel( { OnPropertyChanged(nameof(SelectedHostAsksForAPassword)); OnPropertyChanged(nameof(SelectedHostAuthenticationNote)); + + DisarmIfAimedElsewhere(DeletionTarget.Host, value?.EntityId); + } + + partial void OnPendingDeletionChanged(DeletionRequest? value) + { + OnPropertyChanged(nameof(IsConfirmingDeletion)); + OnPropertyChanged(nameof(ShowsHostActions)); + OnPropertyChanged(nameof(ShowsItemActions)); + } + + /// + /// Takes the question away when the selection it was asked about has moved on. + /// + /// + /// Compared by entity id rather than by row, and that is the whole point of the method. A reload + /// replaces every row object in the list, so a background pass a minute after the question would + /// otherwise take the card away from under somebody still reading it — while a click onto a different + /// item, which is the case that actually needs handling, leaves an armed deletion pointing at something + /// nobody is looking at any more. + /// + private void DisarmIfAimedElsewhere(DeletionTarget target, Guid? entityId) + { + if (PendingDeletion is { } request && request.Target == target && request.EntityId != entityId) + { + PendingDeletion = null; + } } /// @@ -2886,6 +3228,11 @@ internal sealed partial class VaultViewModel( OnPropertyChanged(nameof(SelectedItemIsEditable)); OnPropertyChanged(nameof(SelectedItemIsPin)); OnPropertyChanged(nameof(SelectedDetailHeading)); + OnPropertyChanged(nameof(ShowsItemActions)); + + // Both kinds this table can delete, because one selection covers both lists. + DisarmIfAimedElsewhere(DeletionTarget.Key, value?.EntityId); + DisarmIfAimedElsewhere(DeletionTarget.Credential, value?.EntityId); switch (value?.Kind) { @@ -2936,8 +3283,34 @@ internal sealed partial class VaultViewModel( /// flips back in every path that closes one, so this notification always observes the pair in a /// consistent state. /// - partial void OnIsEditingChanged(bool value) => + partial void OnIsEditingChanged(bool value) + { OnPropertyChanged(nameof(CanForgetHostKey)); + OnPropertyChanged(nameof(ShowsHostActions)); + + DisarmOnceAnEditorIsOpen(value); + } + + partial void OnIsEditingKeyChanged(bool value) => DisarmOnceAnEditorIsOpen(value); + + partial void OnIsEditingCredentialChanged(bool value) => DisarmOnceAnEditorIsOpen(value); + + /// + /// Takes the question away when an editor opens over the pane it was asked in. + /// + /// + /// The sidebar's confirmation replaces the buttons that could open the host editor, so that half cannot + /// happen; the vault screen's Add buttons stay on screen beside the detail pane, so that half can. One + /// rule for both, rather than a guard on the three commands that would have to be remembered by the + /// fourth. + /// + private void DisarmOnceAnEditorIsOpen(bool opened) + { + if (opened) + { + PendingDeletion = null; + } + } partial void OnPendingHostKeyChanged(HostKeyPresentation? value) => OnPropertyChanged(nameof(HasPendingHostKey)); diff --git a/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml b/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml new file mode 100644 index 0000000..8b40903 --- /dev/null +++ b/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml @@ -0,0 +1,52 @@ + + + + + + + + + + + + + + + + +