diff --git a/README.md b/README.md index 4ebf4c4..05b88d6 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,21 @@ 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. +**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 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 a1dd79a..dda7657 100644 --- a/docs/design-import-gaps.md +++ b/docs/design-import-gaps.md @@ -153,8 +153,14 @@ Nothing on this screen exists. It is in the nav rail and reaches a screen that s ## 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 1b8496a..7ad50bd 100644 --- a/src/DodoSSH.Client.App/App.axaml.cs +++ b/src/DodoSSH.Client.App/App.axaml.cs @@ -81,7 +81,14 @@ internal sealed partial class DodoSshApp : Application async (url, cancellationToken) => await ServerConnection .SignInAsync(url, browser, TimeProvider.System, cancellationToken) .ConfigureAwait(false), - TimeProvider.System); + TimeProvider.System, + 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 c22a633..e0d7805 100644 --- a/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs @@ -115,10 +115,30 @@ 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; 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; /// @@ -132,6 +152,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); + internal MainWindowViewModel( ClientPaths paths, ClientCacheFactory caches, @@ -140,7 +173,8 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp IDeviceKeyStore deviceKeys, SignInHandler signIn, TimeProvider clock, - Argon2Profile? passphraseProfile = null) + Argon2Profile? passphraseProfile = null, + ResumeHandler? resume = null) { this.paths = paths; this.caches = caches; @@ -148,6 +182,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; @@ -262,6 +297,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. @@ -642,9 +686,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); } @@ -884,7 +942,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 @@ -908,9 +966,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. /// @@ -955,10 +1194,170 @@ 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(); + + 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() { @@ -1127,6 +1526,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)); } /// @@ -1233,8 +1637,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. @@ -1262,6 +1668,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/VaultViewModel.cs b/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs index 978ae24..e373ebe 100644 --- a/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs @@ -422,6 +422,24 @@ internal sealed record VaultItemRowViewModel( internal bool HasBadge => Badge.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. /// @@ -438,6 +456,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 @@ -456,7 +480,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 @@ -1138,21 +1163,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 @@ -1163,6 +1194,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. /// @@ -1199,13 +1242,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 not null && (report.Pulled > 0 || report.Pushed > 0 || report.NeedsAttention)) @@ -1277,7 +1345,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)) { diff --git a/src/DodoSSH.Client.App/Views/MainWindow.axaml b/src/DodoSSH.Client.App/Views/MainWindow.axaml index 413c1ce..7fd644d 100644 --- a/src/DodoSSH.Client.App/Views/MainWindow.axaml +++ b/src/DodoSSH.Client.App/Views/MainWindow.axaml @@ -252,7 +252,13 @@ - + +