fix(app): a saved token is read when it is used, not once when a workspace opens #158

Merged
james.maes merged 3 commits from fix/157-token-at-use into main 2026-11-10 22:59:25 +00:00
19 changed files with 779 additions and 29 deletions

No files matched your search

+1 -1
View File
@@ -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
+2 -1
View File
@@ -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:<id>`; the metadata file,
`gclo:account:<id>`, or `gclo:<scope>:account:<id>` 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
+2
View File
@@ -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})";
}
+2
View File
@@ -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})";
}
+3 -2
View File
@@ -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 <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;
+31 -4
View File
@@ -43,7 +43,7 @@ public sealed class AccountTokenResolverTests
var resolver = new AccountTokenResolver(_vault);
Assert.Throws<ArgumentNullException>(() => resolver.TryResolve(null!));
Assert.Throws<ArgumentNullException>(() => AccountTokenResolver.MissingDescription(null!));
Assert.Throws<ArgumentNullException>(() => 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));
}
}
@@ -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);
}
}
@@ -491,6 +491,10 @@ public sealed class AccountsStoreConsistencyTests : IDisposable
public string DescribeEntry(Guid accountId) => _inner.DescribeEntry(accountId);
public void Delete(Guid accountId)
{
DeleteCalls++;
@@ -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<SyncAllAccountState>();
_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<SyncAllAccountState>();
_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);
}
}
@@ -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 };
/// <summary>A folder of this test's own; the view model's target folder is removed when the test ends.</summary>
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
/// <summary>An account workspace with its own token in <paramref name="vault"/>, loaded with the given repositories.</summary>
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<IReadOnlyList<string>>(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<IReadOnlyList<string>>(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<PathRecoveryDecision?>(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);
}
}
/// <summary>A vault whose reads fail, as Credential Manager's do when it does not answer.</summary>
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);
}
}
+5 -4
View File
@@ -50,14 +50,15 @@ public sealed class AccountTokenResolver
/// <summary>
/// Why <see cref="TryResolve"/> returned null for <paramref name="account"/>, 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.
/// </summary>
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)}";
}
}
+12
View File
@@ -210,6 +210,13 @@ public sealed partial class AccountWizardViewModel : ObservableObject
/// <summary>The token box is enabled only for a token of the account's own.</summary>
public bool IsOwnTokenSelected => !UseDefaultToken;
/// <summary>
/// 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.
/// </summary>
public bool OffersTokenChoice => HasDefaultToken || _existing?.UsesDefaultToken == true;
/// <summary>
/// <see cref="UseDefaultToken"/> 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
{
@@ -88,6 +88,13 @@ public sealed class CredentialManagerVault : ITokenVault
/// <inheritdoc/>
/// <remarks>Names the entry as this process's data root scopes it, which is what Credential Manager lists.</remarks>
public string DescribeEntry(Guid accountId)
=> $"Windows Credential Manager (entry '{TargetName(accountId, _scope)}')";
/// <summary>The Credential Manager target name for an account under a scope.</summary>
internal static string TargetName(Guid accountId, string? scope)
=> scope is null ? $"gclo:account:{accountId:N}" : $"gclo:{scope}:account:{accountId:N}";
+8
View File
@@ -32,4 +32,12 @@ public interface ITokenVault
/// <summary>Removes the account's token. Deleting an absent entry is a no-op.</summary>
void Delete(Guid accountId);
/// <summary>
/// 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.
/// </summary>
string DescribeEntry(Guid accountId);
}
+5
View File
@@ -37,4 +37,9 @@ public sealed class InMemoryVault : ITokenVault
/// <inheritdoc/>
public void Delete(Guid accountId)
=> _tokens.TryRemove(accountId, out _);
/// <inheritdoc/>
public string DescribeEntry(Guid accountId) => $"this session's memory (account {accountId:N})";
}
+18 -4
View File
@@ -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,
+171 -12
View File
@@ -42,6 +42,12 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable
private readonly Account? _account;
/// <summary>Where an account workspace's token comes from; null for Quick Sync.</summary>
private readonly AccountTokenResolver? _tokenResolver;
/// <summary>True while the token is being taken from its source again, which is not the user typing one.</summary>
private bool _readingTokenFromItsSource;
private readonly AccountsStore? _accountsStore;
private readonly Dictionary<string, RepoItemViewModel> _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 <paramref name="tokenVault"/> (the Token setter's org lookup fires
/// naturally); finished syncs are then recorded on <paramref name="accountsStore"/>.
/// The token is read from the vault again whenever it is about to be used
/// (<see cref="RefreshAccountToken"/>), 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.
/// </summary>
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
/// </summary>
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
/// <summary>
/// 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.
/// </summary>
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);
}
/// <summary>
/// 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.
/// </summary>
private Account AccountAsItIsNow()
=> _accountsStore?.GetAll().FirstOrDefault(a => a.Id == _account!.Id) ?? _account!;
/// <summary>
/// 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.
/// </summary>
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.";
/// <summary>
/// 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
/// (<paramref name="previous"/>, or nothing): a token somebody typed stays.
/// </summary>
/// <param name="previous">The default token before the change; null when there was none.</param>
/// <param name="current">The default token now; null when there is none.</param>
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;
}
}
/// <summary>
/// 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.
/// </summary>
private string TokenToUseNow()
{
RefreshAccountToken();
return MissingTokenDescription is { } missing
? throw new InvalidOperationException(missing)
: Token.Trim();
}
/// <summary>Records that the user opened the target folder from the results bar (#40).</summary>
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<RepoDescriptor> descriptors = selected.Select(r => r.Descriptor).ToList();
Func<SyncRequest, IReadOnlyList<RepoDescriptor>, IProgress<RepoProgress>, CancellationToken, Task<SyncSummary>> 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;
+1 -1
View File
@@ -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}">
<RadioButton AutomationProperties.AutomationId="WizardUseDefaultToken" Content="Use the default token from Settings" />
<RadioButton AutomationProperties.AutomationId="WizardUseOwnToken" Content="Use a different token for this account" />
</RadioButtons>
+7
View File
@@ -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
{