diff --git a/README.md b/README.md index eea832e..1a4d0cd 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ gclo authenticates to GitHub with a personal access token — it is used for bot | Classic PAT | `repo` scope (read access to the org's repositories); optionally `read:org` so the organization dropdown / `gclo orgs` can list your orgs | | Fine-grained PAT | Repository read access for the organization | -**Token storage.** In **Quick Sync**, the token is held in memory for the session only and never written anywhere. A **saved account's** token and the optional **default token** (Settings) are kept in the **Windows Credential Manager** (per-user, DPAPI-protected) — never in `settings.json`, `accounts.json`, or any log. See [SECURITY.md](SECURITY.md). +**Token storage.** In **Quick Sync**, the token is held in memory for the session only and never written anywhere. A **saved account's** token and the optional **default token** (Settings) are kept in the **Windows Credential Manager** (per-user, DPAPI-protected) — never in `settings.json`, `accounts.json`, or any log. See [SECURITY.md](SECURITY.md). A saved token is read at the moment it is used: replacing or removing the default token in Settings, or an account's token in its wizard, takes effect for the workspaces that are already open, without a restart. Quick Sync follows a changed default token only while its token box still holds the one Settings put there. ## Using the GUI diff --git a/docs/CLI.md b/docs/CLI.md index c32d146..01c67c1 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -263,7 +263,8 @@ throttle is reported as such (exit code 4) rather than shrinking the list. An *account* is a saved sync profile: a name, an organization, a target root folder, a parallelism setting, an org-subfolder preference, an optional description, and a token stored in **Windows Credential Manager** (under -`gclo:account:`; the metadata file, +`gclo:account:`, or `gclo::account:` when `GCLO_DATA_DIR` points +the data elsewhere; the metadata file, `%LOCALAPPDATA%\KofTwentyTwo\gclo\accounts.json`, never contains the token). Accounts are shared between the CLI and the desktop app: create them with `gclo accounts add` on a headless mirror server, or in the app's account diff --git a/gclo.Cli.Tests/AccountsCommandTests.cs b/gclo.Cli.Tests/AccountsCommandTests.cs index 23adf18..35f01c8 100644 --- a/gclo.Cli.Tests/AccountsCommandTests.cs +++ b/gclo.Cli.Tests/AccountsCommandTests.cs @@ -764,6 +764,8 @@ public sealed class AccountsCommandTests : IDisposable public void Delete(Guid accountId) { } + + public string DescribeEntry(Guid accountId) => $"a broken vault (account {accountId:N})"; } diff --git a/gclo.Cli.Tests/SyncCommandTests.cs b/gclo.Cli.Tests/SyncCommandTests.cs index e1c8415..8f76845 100644 --- a/gclo.Cli.Tests/SyncCommandTests.cs +++ b/gclo.Cli.Tests/SyncCommandTests.cs @@ -818,6 +818,8 @@ public sealed class SyncCommandTests : IDisposable public void Delete(Guid accountId) { } + + public string DescribeEntry(Guid accountId) => $"an unreadable vault (account {accountId:N})"; } diff --git a/gclo.Cli/SyncCommand.cs b/gclo.Cli/SyncCommand.cs index c1fd604..14f7129 100644 --- a/gclo.Cli/SyncCommand.cs +++ b/gclo.Cli/SyncCommand.cs @@ -515,10 +515,11 @@ internal static class SyncCommand } // vault is non-null whenever account is: both come from AccountsCommand.Open. + var resolver = new AccountTokenResolver(vault!); string? token; try { - token = new AccountTokenResolver(vault!).TryResolve(account); + token = resolver.TryResolve(account); } catch(Exception ex) when(CliErrorException.IsStorageFailure(ex)) { @@ -535,7 +536,7 @@ internal static class SyncCommand : "Re-enter the token with 'gclo accounts edit --name --token-stdin' or in the desktop app's " + "account wizard, restore the credential entry manually,"; throw new CliErrorException( - $"Account '{account.Name}' {AccountTokenResolver.MissingDescription(account)}. {fix} " + $"Account '{account.Name}' {resolver.MissingDescription(account)}. {fix} " + "or pass a token with --token-env, --token-file, or --token-stdin."); } return token; diff --git a/gclo.Engine.Tests/AccountTokenResolverTests.cs b/gclo.Engine.Tests/AccountTokenResolverTests.cs index 91ec697..948ee47 100644 --- a/gclo.Engine.Tests/AccountTokenResolverTests.cs +++ b/gclo.Engine.Tests/AccountTokenResolverTests.cs @@ -43,7 +43,7 @@ public sealed class AccountTokenResolverTests var resolver = new AccountTokenResolver(_vault); Assert.Throws(() => resolver.TryResolve(null!)); - Assert.Throws(() => AccountTokenResolver.MissingDescription(null!)); + Assert.Throws(() => resolver.MissingDescription(null!)); } @@ -129,11 +129,38 @@ public sealed class AccountTokenResolverTests Account own = MakeAccount(TokenSource.Own); Account byDefault = MakeAccount(TokenSource.Default); - string ownText = AccountTokenResolver.MissingDescription(own); - string defaultText = AccountTokenResolver.MissingDescription(byDefault); + var resolver = new AccountTokenResolver(_vault); + string ownText = resolver.MissingDescription(own); + string defaultText = resolver.MissingDescription(byDefault); - Assert.Contains($"gclo:account:{own.Id:N}", ownText); + Assert.Contains($"{own.Id:N}", ownText); Assert.Contains("Settings", defaultText); Assert.DoesNotContain("ghp", ownText + defaultText); } + + + + [Fact] + public void MissingDescription_NamesTheEntry_TheWayTheVaultInUseNamesIt() + { + if(!OperatingSystem.IsWindows()) + { + return; // Credential Manager is a Windows store + } + // The data folder scopes the entry names in Credential Manager (#157): the + // message must name the entry a person will actually find there. + Account own = MakeAccount(TokenSource.Own); + var unscoped = new AccountTokenResolver(new CredentialManagerVault(isWindows: true)); + var scoped = new AccountTokenResolver(new CredentialManagerVault(isWindows: true, scope: "0123456789abcdef")); + + Assert.Equal( + $"has no token in Windows Credential Manager (entry 'gclo:account:{own.Id:N}')", + unscoped.MissingDescription(own)); + Assert.Equal( + $"has no token in Windows Credential Manager (entry 'gclo:0123456789abcdef:account:{own.Id:N}')", + scoped.MissingDescription(own)); + Assert.Equal( + $"has no token in this session's memory (account {own.Id:N})", + new AccountTokenResolver(new InMemoryVault()).MissingDescription(own)); + } } diff --git a/gclo.Engine.Tests/AccountWizardViewModelTests.cs b/gclo.Engine.Tests/AccountWizardViewModelTests.cs index a335793..1578b6e 100644 --- a/gclo.Engine.Tests/AccountWizardViewModelTests.cs +++ b/gclo.Engine.Tests/AccountWizardViewModelTests.cs @@ -743,4 +743,46 @@ public sealed class AccountWizardViewModelTests : IDisposable Assert.Equal(account.Id, Assert.Single(_store.GetAll()).Id); Assert.Equal("ghp_rotated", _vault.TryRetrieve(account.Id)); } + + + + // ---------------------------------------------------------------- an account on the default token while none is saved (#157) + + [Fact] + public async Task Edit_OfAnAccountOnTheDefaultToken_WhileNoneIsSaved_OffersTheChoice_SoItCanGetATokenOfItsOwn() + { + Account account = MakeAccount("Shared") with { TokenSource = TokenSource.Default }; + _store.Save(account, token: null); + AccountWizardViewModel wizard = NewWizardWithDefault(defaultToken: null, existing: account); + + Assert.False(wizard.HasDefaultToken); + Assert.True(wizard.UseDefaultToken); + Assert.True(wizard.OffersTokenChoice); + Assert.StartsWith("No default token is saved right now.", wizard.DefaultTokenCaption); + + // Step 2 as it is: nothing to validate with, and the message says what to do... + await AdvanceToStepAsync(wizard, 2); + Assert.False(await wizard.TryAdvanceAsync()); + Assert.Contains("use a different token for this account", wizard.TokenError); + + // ...which is now possible. + wizard.TokenChoiceIndex = 1; + wizard.Token = "ghp_a_token_of_its_own"; + Assert.True(await wizard.TryAdvanceAsync()); + Assert.Equal(3, wizard.Step); + } + + + + [Fact] + public void TheTokenChoice_IsOffered_WhenADefaultTokenIsSaved_AndNotForAnAccountThatNeverUsedOne() + { + Account own = MakeAccount("Own"); + _store.Save(own, "ghp_own"); + + Assert.True(NewWizardWithDefault("ghp_default").OffersTokenChoice); + Assert.True(NewWizardWithDefault("ghp_default", existing: own, existingToken: "ghp_own").OffersTokenChoice); + Assert.False(NewWizardWithDefault(defaultToken: null).OffersTokenChoice); + Assert.False(NewWizardWithDefault(defaultToken: null, existing: own, existingToken: "ghp_own").OffersTokenChoice); + } } diff --git a/gclo.Engine.Tests/AccountsStoreConsistencyTests.cs b/gclo.Engine.Tests/AccountsStoreConsistencyTests.cs index 6ddf8a1..c0e43d1 100644 --- a/gclo.Engine.Tests/AccountsStoreConsistencyTests.cs +++ b/gclo.Engine.Tests/AccountsStoreConsistencyTests.cs @@ -491,6 +491,10 @@ public sealed class AccountsStoreConsistencyTests : IDisposable + public string DescribeEntry(Guid accountId) => _inner.DescribeEntry(accountId); + + + public void Delete(Guid accountId) { DeleteCalls++; diff --git a/gclo.Engine.Tests/SyncAllCoordinatorTests.cs b/gclo.Engine.Tests/SyncAllCoordinatorTests.cs index 57c6dd3..bd3f2d7 100644 --- a/gclo.Engine.Tests/SyncAllCoordinatorTests.cs +++ b/gclo.Engine.Tests/SyncAllCoordinatorTests.cs @@ -462,4 +462,77 @@ public sealed class SyncAllCoordinatorTests : IDisposable }, states); } + + + + // ---------------------------------------------------------------- a token saved after the workspace opened (#157) + + [Fact] + public async Task RunAsync_DefaultTokenAccount_WhoseTokenWasSavedAfterItOpened_Runs() + { + WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: false); + Assert.Equal("", shared.Token); + _vault.Store(AppSettings.DefaultTokenVaultId, "tok-saved-later"); // saved in Settings meanwhile + + SyncAllResult result = await _coordinator.RunAsync([shared], CancellationToken.None); + + Assert.Equal(1, result.Ran); + Assert.Equal(0, result.Skipped); + Assert.Equal("tok-saved-later", shared.Token); + } + + + + [Fact] + public async Task RunAsync_DefaultTokenAccount_WhoseTokenWasRemovedAfterItOpened_IsSkipped() + { + WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: true); + _vault.Delete(AppSettings.DefaultTokenVaultId); // removed in Settings meanwhile + + SyncAllResult result = await _coordinator.RunAsync([shared], CancellationToken.None); + + Assert.Equal(0, result.Ran); + Assert.Equal(1, result.Skipped); + Assert.Empty(_lister.Organizations); // GitHub was never asked with the old token + } + + + + // ---------------------------------------------------------------- review of #157: no success for a sync that never ran + + [Fact] + public async Task RunAsync_AnAccountWhoseOwnTokenIsGone_IsSkipped_NotReportedAsSucceeded() + { + WorkspaceViewModel workspace = CreateWorkspace("own"); + await workspace.LoadReposCommand.ExecuteAsync(null); // a loaded table: the sync command can run without a token + _vault.Delete(workspace.AccountId!.Value); + var states = new List(); + _coordinator.AccountStateChanged = (_, state) => states.Add(state); + + SyncAllResult result = await _coordinator.RunAsync([workspace], CancellationToken.None); + + Assert.Equal(0, result.Ran); + Assert.Equal(1, result.Skipped); + Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Skipped], states); + } + + + + [Fact] + public async Task RunAsync_ASyncThatCouldNotStart_IsFailed_NotSucceeded_AndIsNotCountedAsRun() + { + // The target folder cannot be used: a file sits where it should be. + WorkspaceViewModel workspace = CreateWorkspace("blocked"); + Directory.CreateDirectory(_root); + File.WriteAllText(Path.Combine(_root, "blocked"), "a file, not a folder"); + var states = new List(); + _coordinator.AccountStateChanged = (_, state) => states.Add(state); + + SyncAllResult result = await _coordinator.RunAsync([workspace], CancellationToken.None); + + Assert.Equal(RunResultKind.Error, workspace.ResultKind); + Assert.Equal(0, result.Ran); + Assert.Equal(1, result.Skipped); + Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Running, SyncAllAccountState.Failed], states); + } } diff --git a/gclo.Engine.Tests/WorkspaceViewModelTests.cs b/gclo.Engine.Tests/WorkspaceViewModelTests.cs index a713423..c1b0710 100644 --- a/gclo.Engine.Tests/WorkspaceViewModelTests.cs +++ b/gclo.Engine.Tests/WorkspaceViewModelTests.cs @@ -2392,4 +2392,389 @@ public sealed class WorkspaceViewModelTests : IDisposable Assert.DoesNotContain(vm.Repos, r => string.Equals(r.Name, "ghost-repo", StringComparison.Ordinal)); } + + + + // ---------------------------------------------------------------- a saved token is read when it is used (#157) + + private static Account MakeDefaultTokenAccount(string targetRoot) => MakeAccount(targetRoot) with { TokenSource = TokenSource.Default }; + + + + /// A folder of this test's own; the view model's target folder is removed when the test ends. + private static string NewTargetRoot() => Path.Combine(Path.GetTempPath(), "gclo-tests", Guid.NewGuid().ToString("N")); + + + + [Fact] + public async Task AccountWorkspace_UsesTheDefaultTokenAsItIsNow_NotAsItWasWhenTheWorkspaceOpened() + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + _lister.Repositories = [Repo("alpha")]; + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + await vm.LoadReposCommand.ExecuteAsync(null); + + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-new-456"); // replaced in Settings + await vm.LoadReposCommand.ExecuteAsync(null); + await vm.SyncCommand.ExecuteAsync(null); + + Assert.Equal(["default-token-old-123", "default-token-new-456"], _lister.Calls.Select(c => c.Token), StringComparer.Ordinal); + Assert.Equal("default-token-new-456", Assert.Single(_git.CloneCalls).Token); + Assert.Equal("default-token-new-456", vm.Token); + } + + + + [Fact] + public async Task AccountWorkspace_WhoseDefaultTokenWasSavedAfterItOpened_LoadsWhenShown() + { + var vault = new InMemoryVault(); + _lister.Repositories = [Repo("alpha")]; + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + Assert.False(vm.LoadRepositoriesIfSaved()); // no token yet: nothing to load with + + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-saved-789"); // first saved in Settings + bool started = vm.LoadRepositoriesIfSaved(); + await WaitUntilAsync(() => vm.HasLoadedRepos, "the load to finish"); + + Assert.True(started); + Assert.Equal("default-token-saved-789", Assert.Single(_lister.Calls).Token); + } + + + + [Fact] + public async Task AccountWorkspace_WhoseDefaultTokenWasRemoved_DoesNotSyncWithTheOldOne_AndSaysWhereToLook() + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + _lister.Repositories = [Repo("alpha")]; + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + await vm.LoadReposCommand.ExecuteAsync(null); + + vault.Delete(AppSettings.DefaultTokenVaultId); // removed in Settings + await vm.SyncCommand.ExecuteAsync(null); + + Assert.Empty(_git.CloneCalls); + Assert.Equal(RunResultKind.Error, vm.ResultKind); + Assert.Equal("Account 'Work' uses the default token, but no default token is saved (desktop app: Settings).", vm.ResultMessage); + Assert.Equal("", vm.Token); + } + + + + [Fact] + public async Task AccountWorkspace_WhoseOwnTokenIsGone_FailsTheLoad_NamingTheEntry() + { + var vault = new InMemoryVault(); + Account account = MakeAccount(NewTargetRoot()); + vault.Store(account.Id, "own-token-1234567890"); + WorkspaceViewModel vm = CreateViewModel(account: account, vault: vault); + vault.Delete(account.Id); + + await vm.LoadReposCommand.ExecuteAsync(null); + + Assert.True(vm.LoadErrorOpen); + Assert.Equal($"Account 'Work' has no token in this session's memory (account {account.Id:N}).", vm.LoadErrorMessage); + Assert.Empty(_lister.Calls); + } + + + + [Fact] + public void QuickSync_FollowsTheDefaultToken_OnlyWhileItsBoxHoldsWhatSettingsPutThere() + { + WorkspaceViewModel prefilled = CreateViewModel(); + prefilled.Token = "default-token-old-123"; // what the shell pre-fills from Settings + WorkspaceViewModel typed = CreateViewModel(); + typed.Token = "typed-by-hand-0000000"; + WorkspaceViewModel empty = CreateViewModel(); + + foreach(WorkspaceViewModel vm in new[] { prefilled, typed, empty }) + { + vm.OnDefaultTokenChanged(previous: "default-token-old-123", current: "default-token-new-456"); + } + + Assert.Equal("default-token-new-456", prefilled.Token); + Assert.Equal("typed-by-hand-0000000", typed.Token); // somebody's own token stays + Assert.Equal("", empty.Token); // the box was cleared by hand: it stays cleared + + prefilled.OnDefaultTokenChanged(previous: "default-token-new-456", current: null); + empty.OnDefaultTokenChanged(previous: null, current: "default-token-first-1"); // first saved: nothing was there before + prefilled.RefreshAccountToken(); // Quick Sync has no source to read from: nothing happens + + Assert.Equal("", prefilled.Token); + Assert.Equal("default-token-first-1", empty.Token); + } + + + + [Fact] + public void AccountWorkspace_IsToldWhenTheDefaultTokenChanged_AndTakesItFromTheVault() + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-new-456"); + + // What the caller passes is not what an account uses: its source is the vault. + vm.OnDefaultTokenChanged(previous: "something else entirely", current: "also something else"); + + Assert.Equal("default-token-new-456", vm.Token); + } + + + + [Fact] + public void ATokenReadFromItsSource_IsLoggedAsSuch_NotAsTyped_AndNeverByValue() + { + var log = new RecordingActivityLog(); + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + WorkspaceViewModel vm = Track(new WorkspaceViewModel( + _lister, _git, _orgs, handler => new SyncProgress(handler), TimeSpan.FromMilliseconds(1), log, + MakeDefaultTokenAccount(NewTargetRoot()), vault, null, _wsl)); + + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-new-456"); + vm.RefreshAccountToken(); + vm.RefreshAccountToken(); // unchanged: nothing to say + vault.Delete(AppSettings.DefaultTokenVaultId); + vm.RefreshAccountToken(); + + Assert.Equal( + ["Workspace 'Work': its saved token changed (21 characters).", "Workspace 'Work': its token is no longer saved."], + log.Messages.Where(m => m.StartsWith("Workspace 'Work'", StringComparison.Ordinal)), + StringComparer.Ordinal); + Assert.DoesNotContain(log.Messages, m => m.Contains("Token entered", StringComparison.Ordinal)); + Assert.DoesNotContain(log.Messages, m => m.Contains("default-token", StringComparison.Ordinal)); + } + + + + // ---------------------------------------------------------------- review of #157: what a token that went away must not break + + /// An account workspace with its own token in , loaded with the given repositories. + private async Task<(WorkspaceViewModel Vm, Account Account)> CreateLoadedAccountWorkspaceAsync( + InMemoryVault vault, params RepoDescriptor[] repos) + { + Account account = MakeAccount(NewTargetRoot()); + vault.Store(account.Id, "own-token-1234567890"); + _lister.Repositories = repos; + WorkspaceViewModel vm = CreateViewModel(account: account, vault: vault); + await vm.LoadReposCommand.ExecuteAsync(null); + return (vm, account); + } + + + + [Fact] + public async Task ASyncThatCannotStartForWantOfAToken_LeavesTheRowsAsTheyWere() + { + var vault = new InMemoryVault(); + MakeAlphaFailWithInvalidPaths(); + (WorkspaceViewModel vm, Account account) = await CreateLoadedAccountWorkspaceAsync(vault, Repo("alpha"), Repo("bravo")); + await vm.SyncCommand.ExecuteAsync(null); // alpha fails with its paths, bravo is done + RepoItemViewModel alpha = vm.Repos.Single(r => string.Equals(r.Name, "alpha", StringComparison.Ordinal)); + Assert.True(vm.ResolvePathsCommand.CanExecute(alpha)); + + vault.Delete(account.Id); + await vm.RetryFailedCommand.ExecuteAsync(null); + + Assert.Equal(RunResultKind.Error, vm.ResultKind); + Assert.StartsWith("Account 'Work' has no token in", vm.ResultMessage); + // The failure, its paths, and the way to act on them are all still there. + Assert.Equal(SyncStatus.Failed, alpha.Status); + Assert.NotNull(alpha.Error); + Assert.NotNull(alpha.InvalidPaths); + Assert.True(vm.HasFailedRepos); + Assert.True(vm.ResolvePathsCommand.CanExecute(alpha)); + Assert.True(vm.RetryFailedCommand.CanExecute(null)); + Assert.Single(_git.CloneCalls, c => string.Equals(Path.GetFileName(c.LocalPath), "alpha", StringComparison.Ordinal)); + } + + + + [Fact] + public async Task TheOrganization_SurvivesItsTokenGoingAway_AlsoWhenTheBoxDropsItsTextWithTheList() + { + var vault = new InMemoryVault(); + (WorkspaceViewModel vm, Account account) = await CreateLoadedAccountWorkspaceAsync(vault, Repo("alpha")); + await WaitUntilAsync(() => vm.Organizations.Count > 0 || !vm.IsLoadingOrgs, "the first lookup to settle"); + // What the editable box does: when its list changes it loses its text, and + // the two-way binding writes the empty text back. + vm.Organizations.CollectionChanged += (_, _) => vm.Organization = ""; + + vault.Delete(account.Id); + vm.RefreshAccountToken(); + + Assert.Equal("acme", vm.Organization); + Assert.False(vm.IsTableStale); + } + + + + [Fact] + public async Task TheLookupStartedByReadingTheTokenAgain_SaysNothing_AndLeavesTheLoadsErrorOpen() + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + var lookup = new TaskCompletionSource>(TaskCreationOptions.RunContinuationsAsynchronously); + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + await WaitUntilAsync(() => vm.StatusText.Length > 0, "the first lookup to settle"); + + // The token is replaced; the load that reads it fails; the lookup it started lands afterwards. + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-new-456"); + _orgs.Handler = (_, _) => lookup.Task; + _lister.ExceptionToThrow = new InvalidOperationException("organization not found"); + await vm.LoadReposCommand.ExecuteAsync(null); + Assert.True(vm.LoadErrorOpen); + string statusAfterTheLoad = vm.StatusText; + lookup.SetResult(["me", "acme"]); + await WaitUntilAsync(() => vm.Organizations.Count == 2, "the lookup to land"); + + Assert.True(vm.LoadErrorOpen, "the error of the load was closed by a lookup nobody asked for"); + Assert.Equal("organization not found", vm.LoadErrorMessage); + Assert.Equal(statusAfterTheLoad, vm.StatusText); + } + + + + [Fact] + public async Task ALookupThatFails_AfterTheTokenWasReadAgain_IsLogged_NotShown() + { + var log = new RecordingActivityLog(); + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + WorkspaceViewModel vm = Track(new WorkspaceViewModel( + _lister, _git, _orgs, handler => new SyncProgress(handler), TimeSpan.FromMilliseconds(1), log, + MakeDefaultTokenAccount(NewTargetRoot()), vault, null, _wsl)); + await WaitUntilAsync(() => vm.StatusText.Length > 0, "the first lookup to settle"); + + _orgs.Handler = (_, _) => Task.FromException>(new InvalidOperationException("rate limited")); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-new-456"); + vm.RefreshAccountToken(); + await WaitUntilAsync( + () => log.Messages.Any(m => m.StartsWith("Organization lookup failed", StringComparison.Ordinal)), "the lookup to fail"); + + Assert.False(vm.LoadErrorOpen); + } + + + + [Fact] + public async Task CloneInWsl_UsesTheTokenAsItIsNow() + { + var vault = new InMemoryVault(); + MakeAlphaFailWithInvalidPaths(); + _wsl.Availability = new WslAvailability(true, "git version 2.53.0"); + (WorkspaceViewModel vm, Account account) = await CreateLoadedAccountWorkspaceAsync(vault, Repo("alpha")); + await vm.SyncCommand.ExecuteAsync(null); + vm.RecoveryInteraction = _ => Task.FromResult(new PathRecoveryDecision.CloneInWsl()); + + vault.Store(account.Id, "own-token-replaced-99"); // replaced from another gclo process + await vm.ResolvePathsCommand.ExecuteAsync(vm.Repos[0]); + + Assert.Equal("own-token-replaced-99", Assert.Single(_wsl.CloneCalls).Token); + } + + + + [Fact] + public void AnAccountSwitchedToATokenOfItsOwn_IsReadFromItsNewSource() + { + string root = NewTargetRoot(); + try + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + var store = new AccountsStore(vault, Path.Combine(root, "store"), new NullActivityLog()); + Account account = MakeDefaultTokenAccount(Path.Combine(root, "repos")); + store.Save(account, token: null); + WorkspaceViewModel vm = CreateViewModel(account: account, vault: vault, store: store); + Assert.Equal("default-token-old-123", vm.Token); + + // The source changes in the store (an edit made elsewhere); the workspace + // still holds the record it was opened with. + store.Save(account with { TokenSource = TokenSource.Own }, "own-token-1234567890"); + vm.RefreshAccountToken(); + + Assert.Equal("own-token-1234567890", vm.Token); + } + finally + { + TryDeleteDirectory(root); + } + } + + + + /// A vault whose reads fail, as Credential Manager's do when it does not answer. + private sealed class UnreadableVault(InMemoryVault inner) : ITokenVault + { + public bool Fails { get; set; } + + public void Store(Guid accountId, string token) => inner.Store(accountId, token); + + public string? TryRetrieve(Guid accountId) + => Fails ? throw new System.ComponentModel.Win32Exception(1312, "A specified logon session does not exist.") : inner.TryRetrieve(accountId); + + public void Delete(Guid accountId) => inner.Delete(accountId); + + public string DescribeEntry(Guid accountId) => inner.DescribeEntry(accountId); + } + + + + [Fact] + public void ACredentialStoreThatDoesNotAnswer_KeepsTheTokenReadEarlier_AndSaysSoInTheLog() + { + var log = new RecordingActivityLog(); + var inner = new InMemoryVault(); + inner.Store(AppSettings.DefaultTokenVaultId, "default-token-old-123"); + var vault = new UnreadableVault(inner); + WorkspaceViewModel vm = Track(new WorkspaceViewModel( + _lister, _git, _orgs, handler => new SyncProgress(handler), TimeSpan.FromMilliseconds(1), log, + MakeDefaultTokenAccount(NewTargetRoot()), vault, null, _wsl)); + + vault.Fails = true; + vm.RefreshAccountToken(); // must not throw: the shell and Sync all call this + vm.OnDefaultTokenChanged(previous: null, current: null); + + Assert.Equal("default-token-old-123", vm.Token); + Assert.Contains(log.Messages, m => m.Contains("its saved token could not be read", StringComparison.Ordinal)); + Assert.DoesNotContain(log.Messages, m => m.Contains("default-token", StringComparison.Ordinal)); + } + + + + [Fact] + public async Task ASavedTokenOfNothingButSpaces_IsNoToken() + { + var vault = new InMemoryVault(); + vault.Store(AppSettings.DefaultTokenVaultId, " "); + WorkspaceViewModel vm = CreateViewModel(account: MakeDefaultTokenAccount(NewTargetRoot()), vault: vault); + vm.Token = "typed-over-it-000000"; // so the command can be run at all + + await vm.LoadReposCommand.ExecuteAsync(null); + + Assert.True(vm.LoadErrorOpen); + Assert.Equal("Account 'Work' uses the default token, but no default token is saved (desktop app: Settings).", vm.LoadErrorMessage); + Assert.Empty(_lister.Calls); + } + + + + [Fact] + public async Task QuickSync_WhoseTokenBoxWasEmptied_FailsTheSyncInItsOwnWords() + { + WorkspaceViewModel vm = await CreateLoadedViewModelAsync(Repo("alpha")); + vm.OnDefaultTokenChanged(previous: "token-1234567890", current: null); // the pre-filled default was removed + + await vm.SyncCommand.ExecuteAsync(null); + + Assert.Equal(RunResultKind.Error, vm.ResultKind); + Assert.Equal("There is no token: enter one in the token box.", vm.ResultMessage); + Assert.Empty(_git.CloneCalls); + } } diff --git a/gclo.ViewModels/AccountTokenResolver.cs b/gclo.ViewModels/AccountTokenResolver.cs index 6deaa70..23b56b3 100644 --- a/gclo.ViewModels/AccountTokenResolver.cs +++ b/gclo.ViewModels/AccountTokenResolver.cs @@ -50,14 +50,15 @@ public sealed class AccountTokenResolver /// /// Why returned null for , worded - /// for the user and naming the source, so every "no token" message says what to fix. - /// Never contains a token. + /// for the user and naming the source, so every "no token" message says what to fix: + /// for a token of the account's own, the entry as this vault names it. Never + /// contains a token. /// - public static string MissingDescription(Account account) + public string MissingDescription(Account account) { ArgumentNullException.ThrowIfNull(account); return account.TokenSource == TokenSource.Default ? "uses the default token, but no default token is saved (desktop app: Settings)" - : $"has no token in Windows Credential Manager (entry 'gclo:account:{account.Id:N}')"; + : $"has no token in {_vault.DescribeEntry(account.Id)}"; } } diff --git a/gclo.ViewModels/AccountWizardViewModel.cs b/gclo.ViewModels/AccountWizardViewModel.cs index 8dff12e..719802d 100644 --- a/gclo.ViewModels/AccountWizardViewModel.cs +++ b/gclo.ViewModels/AccountWizardViewModel.cs @@ -210,6 +210,13 @@ public sealed partial class AccountWizardViewModel : ObservableObject /// The token box is enabled only for a token of the account's own. public bool IsOwnTokenSelected => !UseDefaultToken; + /// + /// Whether the choice between the default token and a token of the account's own + /// is shown: when a default token is saved, and also for an account that uses the + /// default token while none is saved, which otherwise could not be given another. + /// + public bool OffersTokenChoice => HasDefaultToken || _existing?.UsesDefaultToken == true; + /// /// as the selected index of a two-item radio group: /// 0 = the default token, 1 = a token of this account's own. @@ -228,6 +235,11 @@ public sealed partial class AccountWizardViewModel : ObservableObject { get { + if(!HasDefaultToken) + { + return "No default token is saved right now. Save one in Settings, or give this account a token of its own."; + } + int others = _store.CountUsingDefaultToken() - (_existing?.UsesDefaultToken == true ? 1 : 0); string shared = others switch { diff --git a/gclo.ViewModels/CredentialManagerVault.cs b/gclo.ViewModels/CredentialManagerVault.cs index f35850c..9b42d86 100644 --- a/gclo.ViewModels/CredentialManagerVault.cs +++ b/gclo.ViewModels/CredentialManagerVault.cs @@ -88,6 +88,13 @@ public sealed class CredentialManagerVault : ITokenVault + /// + /// Names the entry as this process's data root scopes it, which is what Credential Manager lists. + public string DescribeEntry(Guid accountId) + => $"Windows Credential Manager (entry '{TargetName(accountId, _scope)}')"; + + + /// The Credential Manager target name for an account under a scope. internal static string TargetName(Guid accountId, string? scope) => scope is null ? $"gclo:account:{accountId:N}" : $"gclo:{scope}:account:{accountId:N}"; diff --git a/gclo.ViewModels/ITokenVault.cs b/gclo.ViewModels/ITokenVault.cs index 3c54616..5c618b1 100644 --- a/gclo.ViewModels/ITokenVault.cs +++ b/gclo.ViewModels/ITokenVault.cs @@ -32,4 +32,12 @@ public interface ITokenVault /// Removes the account's token. Deleting an absent entry is a no-op. void Delete(Guid accountId); + + + + /// + /// Where this vault keeps the account's token, in words a person can follow to + /// look for it ("Windows Credential Manager (entry '...')"). Never a token. + /// + string DescribeEntry(Guid accountId); } diff --git a/gclo.ViewModels/InMemoryVault.cs b/gclo.ViewModels/InMemoryVault.cs index b343196..0934796 100644 --- a/gclo.ViewModels/InMemoryVault.cs +++ b/gclo.ViewModels/InMemoryVault.cs @@ -37,4 +37,9 @@ public sealed class InMemoryVault : ITokenVault /// public void Delete(Guid accountId) => _tokens.TryRemove(accountId, out _); + + + + /// + public string DescribeEntry(Guid accountId) => $"this session's memory (account {accountId:N})"; } diff --git a/gclo.ViewModels/SyncAllCoordinator.cs b/gclo.ViewModels/SyncAllCoordinator.cs index 6808152..4851797 100644 --- a/gclo.ViewModels/SyncAllCoordinator.cs +++ b/gclo.ViewModels/SyncAllCoordinator.cs @@ -78,13 +78,16 @@ public sealed class SyncAllCoordinator(IActivityLog log) continue; } - if(workspace.UsesDefaultToken && string.IsNullOrEmpty(workspace.Token)) + // A token saved, replaced, or removed since the workspace was opened counts. + workspace.RefreshAccountToken(); + if(workspace.MissingTokenDescription is { } missingToken) { - // The account resolves to the default token and none is saved: a - // generic auth failure would send the user to the wrong place. + // Whichever source the account's token comes from, it holds none: say + // which, instead of a sync that cannot start or a generic auth failure + // that sends the user to the wrong place. skipped++; Announce(workspace, SyncAllAccountState.Skipped); - _log.Info($"Sync all: skipped '{workspace.DisplayName}' — no default token is saved (Settings)."); + _log.Info($"Sync all: skipped '{workspace.DisplayName}' — {missingToken}"); continue; } @@ -114,6 +117,17 @@ public sealed class SyncAllCoordinator(IActivityLog log) _log.Info($"Sync all: syncing '{workspace.DisplayName}'."); await workspace.SyncCommand.ExecuteAsync(null); + if(workspace.ResultKind == RunResultKind.Error) + { + // The sync never ran its repositories (its target folder could not be + // used, its token went away at that moment): that is not a success with + // nothing to report. + skipped++; + Announce(workspace, SyncAllAccountState.Failed); + _log.Info($"Sync all: '{workspace.DisplayName}' could not be synced — {workspace.ResultMessage}"); + continue; + } + ran++; Announce( workspace, diff --git a/gclo.ViewModels/WorkspaceViewModel.cs b/gclo.ViewModels/WorkspaceViewModel.cs index e3838a4..94d46ef 100644 --- a/gclo.ViewModels/WorkspaceViewModel.cs +++ b/gclo.ViewModels/WorkspaceViewModel.cs @@ -42,6 +42,12 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable private readonly Account? _account; + /// Where an account workspace's token comes from; null for Quick Sync. + private readonly AccountTokenResolver? _tokenResolver; + + /// True while the token is being taken from its source again, which is not the user typing one. + private bool _readingTokenFromItsSource; + private readonly AccountsStore? _accountsStore; private readonly Dictionary _itemsByName = new(StringComparer.OrdinalIgnoreCase); @@ -99,6 +105,10 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable /// organization, target folder, subfolder preference, and concurrency — and pulls the /// token from (the Token setter's org lookup fires /// naturally); finished syncs are then recorded on . + /// The token is read from the vault again whenever it is about to be used + /// (), so one that was replaced or removed since + /// (the default token in Settings, the account's own in the wizard) takes effect + /// without a restart. /// public WorkspaceViewModel( IRepositoryLister? lister = null, @@ -142,7 +152,8 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable MaxConcurrency = account.MaxConcurrency; // The Token setter's existing org lookup fires naturally with the resolved // token: the account's own entry, or the default token from Settings. - Token = tokenVault is null ? "" : new AccountTokenResolver(tokenVault).TryResolve(account) ?? ""; + _tokenResolver = tokenVault is null ? null : new AccountTokenResolver(tokenVault); + Token = _tokenResolver?.TryResolve(account) ?? ""; } _constructed = true; } @@ -175,6 +186,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable /// public bool LoadRepositoriesIfSaved() { + RefreshAccountToken(); if(_account is null || HasLoadedRepos || !LoadReposCommand.CanExecute(null)) { return false; @@ -513,7 +525,13 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable // The value itself never reaches the log; its length is enough to tell a // paste from a cleared box when reading back a session (#40). Construction // seeds the property (empty, or the vault token) and is not a user action. - if(_constructed) + if(_readingTokenFromItsSource) + { + _log.Info(value.Length == 0 + ? $"Workspace '{DisplayName}': its token is no longer saved." + : $"Workspace '{DisplayName}': its saved token changed ({value.Length} characters)."); + } + else if(_constructed) { _log.Info(value.Length == 0 ? $"Token cleared in workspace '{DisplayName}'." @@ -524,6 +542,122 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable + /// + /// Takes an account workspace's token from its source again (its own vault entry, + /// or the default token from Settings) and uses it from here on. Called before the + /// token is used and whenever the shell knows a saved token changed. Nothing + /// happens for Quick Sync, whose token is whatever is in its box, or when the + /// source still holds the same token. + /// + public void RefreshAccountToken() + { + if(_account is null || _tokenResolver is null) + { + return; + } + + string token; + try + { + token = _tokenResolver.TryResolve(AccountAsItIsNow()) ?? ""; + } + catch(Exception ex) when(ex is System.ComponentModel.Win32Exception or IOException or InvalidOperationException) + { + /////////////////////////////////////////////////////////////////////////// + // The credential store did not answer. That says nothing about the // + // token: the one read earlier stays in use, and whatever needs it next // + // succeeds or fails on its own terms. // + /////////////////////////////////////////////////////////////////////////// + _log.Error($"Workspace '{DisplayName}': its saved token could not be read ({ex.Message}); the one read earlier stays in use.", ex); + return; + } + + SetTokenFromItsSource(token); + } + + + + /// + /// The account as the store has it now: where its token comes from can have been + /// changed (to the default token, or to a token of its own) since this workspace + /// was opened. The record the workspace was opened with when the store no longer + /// has the account, or when there is no store. + /// + private Account AccountAsItIsNow() + => _accountsStore?.GetAll().FirstOrDefault(a => a.Id == _account!.Id) ?? _account!; + + + + /// + /// Why this workspace has no token to work with, in words that name where to look; + /// null when it has one. For callers that decide before starting anything (Sync + /// all) and want to say why they did not. + /// + public string? MissingTokenDescription + => Token.Trim().Length > 0 + ? null + : _account is not null && _tokenResolver is not null + ? $"Account '{_account.Name}' {_tokenResolver.MissingDescription(AccountAsItIsNow())}." + : "There is no token: enter one in the token box."; + + + + /// + /// Tells the workspace that the default token in Settings was saved, replaced, or + /// removed. An account workspace takes its token from its source again. Quick + /// Sync follows only when its box still holds what Settings put there + /// (, or nothing): a token somebody typed stays. + /// + /// The default token before the change; null when there was none. + /// The default token now; null when there is none. + public void OnDefaultTokenChanged(string? previous, string? current) + { + if(_account is not null) + { + RefreshAccountToken(); + } + else if(string.Equals(Token, previous ?? "", StringComparison.Ordinal)) + { + SetTokenFromItsSource(current ?? ""); + } + } + + + + private void SetTokenFromItsSource(string token) + { + if(string.Equals(token, Token, StringComparison.Ordinal)) + { + return; + } + + _readingTokenFromItsSource = true; + try + { + Token = token; + } + finally + { + _readingTokenFromItsSource = false; + } + } + + + + /// + /// The token to use right now: for an account, read from its source this moment, + /// and a failure that names the source when it holds none. + /// + private string TokenToUseNow() + { + RefreshAccountToken(); + return MissingTokenDescription is { } missing + ? throw new InvalidOperationException(missing) + : Token.Trim(); + } + + + /// Records that the user opened the target folder from the results bar (#40). public void NoteFolderOpened() => _log.Info($"Opened folder '{EffectiveTargetRoot}'."); @@ -574,10 +708,24 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable // Captured before any successor can dispose cts; used everywhere below. CancellationToken lookupToken = cts.Token; + /////////////////////////////////////////////////////////////////////////////// + // A lookup started because the saved token was read again is nobody's // + // action: it refreshes the list of organizations and says nothing. The load // + // or sync that read the token owns the status line and the error bar, and a // + // lookup that lands in the middle of it must not write over either. // + /////////////////////////////////////////////////////////////////////////////// + bool quiet = _readingTokenFromItsSource; + string token = Token.Trim(); if(token.Length < 10) { + // The same capture and restore as below: the editable box loses its text + // when its list changes, and an account's organization must survive its + // token going away (it is what the account syncs). + string organizationBeforeClear = Organization; Organizations.Clear(); + Organization = organizationBeforeClear; + OnPropertyChanged(nameof(Organization)); // The newest invocation owns the flag: a canceled predecessor deliberately // leaves it alone, so an early return must clear any spinner it left behind. IsLoadingOrgs = false; @@ -606,12 +754,15 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable } Organization = organizationBeforeRefresh; OnPropertyChanged(nameof(Organization)); - LoadErrorOpen = false; // a lookup that works clears a stale lookup failure - // The production lister always lists the token's own account first, so a - // single entry means no organizations were visible. - StatusText = orgs.Count <= 1 - ? "Only your personal account is visible — add read:org (classic) for organizations, or type an org name manually." - : $"Found {orgs.Count} organizations and accounts."; + if(!quiet) + { + LoadErrorOpen = false; // a lookup that works clears a stale lookup failure + // The production lister always lists the token's own account first, so a + // single entry means no organizations were visible. + StatusText = orgs.Count <= 1 + ? "Only your personal account is visible — add read:org (classic) for organizations, or type an org name manually." + : $"Found {orgs.Count} organizations and accounts."; + } } catch(OperationCanceledException) { @@ -622,7 +773,10 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable if(_orgLoadCts == cts) { _log.Error($"Organization lookup failed: {ex.Message}", ex); - ShowLoadError("Could not list organizations", ex.Message); + if(!quiet) + { + ShowLoadError("Could not list organizations", ex.Message); + } } } finally @@ -764,11 +918,12 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable try { string organization = Organization.Trim(); + string token = TokenToUseNow(); StatusText = "Loading repositories..."; _log.Info($"Loading repositories for organization '{organization}'."); var descriptors = (await _lister - .ListOrganizationRepositoriesAsync(organization, Token.Trim())) + .ListOrganizationRepositoriesAsync(organization, token)) // Names key the row dictionary; a duplicate (possible from pagination // shifts) must not produce two rows fighting over one folder. .DistinctBy(r => r.Name, StringComparer.OrdinalIgnoreCase) @@ -844,6 +999,10 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable ActiveRepos.Clear(); try { + // First, before any row is touched: a sync that cannot start for want of a + // token leaves the table (failures, their paths, the retry) as it was. + string token = TokenToUseNow(); + var selected = Repos.Where(r => r.IsSelected).ToList(); foreach(RepoItemViewModel item in selected) { @@ -870,7 +1029,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable var progress = new FailureLoggingProgress(_progressFactory(HandleProgress), _log); var request = new SyncRequest( - Organization.Trim(), Token.Trim(), targetRoot, MaxConcurrency); + Organization.Trim(), token, targetRoot, MaxConcurrency); IReadOnlyList descriptors = selected.Select(r => r.Descriptor).ToList(); Func, IProgress, CancellationToken, Task> run = @@ -1105,7 +1264,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable // The organization the table was loaded for, not whatever the box says now: // the row belongs to that organization, and its folder under ~/gclo should too. WslCloneResult result = await _wsl.CloneAsync( - item.Descriptor.CloneUrl, LoadedOrganization ?? Organization.Trim(), item.Name, Token.Trim(), _lifetimeCts.Token); + item.Descriptor.CloneUrl, LoadedOrganization ?? Organization.Trim(), item.Name, TokenToUseNow(), _lifetimeCts.Token); item.Status = SyncStatus.Done; item.Error = null; diff --git a/gclo/AccountWizardDialog.xaml b/gclo/AccountWizardDialog.xaml index 9319f20..b924a4a 100644 --- a/gclo/AccountWizardDialog.xaml +++ b/gclo/AccountWizardDialog.xaml @@ -65,7 +65,7 @@ AutomationProperties.AutomationId="WizardTokenChoice" Header="Which token should this account use?" SelectedIndex="{x:Bind ViewModel.TokenChoiceIndex, Mode=TwoWay}" - Visibility="{x:Bind local:StatusFormat.VisibleIf(ViewModel.HasDefaultToken), Mode=OneWay}"> + Visibility="{x:Bind local:StatusFormat.VisibleIf(ViewModel.OffersTokenChoice), Mode=OneWay}"> diff --git a/gclo/MainWindow.xaml.cs b/gclo/MainWindow.xaml.cs index 21d58ef..54451ba 100644 --- a/gclo/MainWindow.xaml.cs +++ b/gclo/MainWindow.xaml.cs @@ -1105,8 +1105,15 @@ public sealed partial class MainWindow : Window int concurrencyBefore = _settings.DefaultMaxConcurrency; if(await DialogGuard.ShowAsync(dialog) == ContentDialogResult.Primary) { + // Held only to tell the open workspaces what changed; never logged or stored. + string? defaultTokenBefore = _tokenVault.TryRetrieve(AppSettings.DefaultTokenVaultId); dialog.ApplyAndSave(); // logs what changed ApplySettings(seedQuickSyncConcurrency: _settings.DefaultMaxConcurrency != concurrencyBefore); + string? defaultTokenNow = _tokenVault.TryRetrieve(AppSettings.DefaultTokenVaultId); + foreach(WorkspaceEntry workspace in _workspaces.Values) + { + workspace.ViewModel.OnDefaultTokenChanged(defaultTokenBefore, defaultTokenNow); + } } else {