fix(accounts): a change writes one file, a workspace's token is the one of the place it was opened for, and a name two accounts share is refused
From the review of 8d855cd (#205, second round).
- A change could write both account files (one to put a missing file back or
to move an account that was in the wrong file, one for the change), and a
failure between the two was not undone: a token put back for an account
that was gone from disk, or an account on disk whose token was taken back.
Whatever else has to be written is now written before the change, before
the vault is touched; when that fails the change fails with nothing
changed. The change itself writes one file, as it always did.
- A workspace made its listers from the record it was opened with and took
its token from the record as the store has it now. A file changed in
between could bring the token of one place to the listers of another. The
token is now resolved for the place the workspace was opened for.
- What a later version put on an account is kept whatever record is saved:
the wizard builds its record anew and dropped it.
- GitHub is the account that names no forge, and only that one. A record in
the file for other forges that names none is not read as a GitHub account
(it would get GitHub's listers and the default token): it stays there and
cannot be used.
- Two accounts of one name (an older gclo can add a GitHub account with the
name of one it does not see): sync --account, accounts edit and accounts
remove refuse the name instead of acting on the first of the two.
- Tests for each, and for the order of the two writes. docs/CLI.md, ADR 0010,
T13.
Refs #204
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
This commit is contained in:
1 parent
8d855cd9f2
commit
9b82c15eb4
10 files changed
+284
-33
No files matched your search
+5
-1
@@ -314,7 +314,11 @@ Accounts on a Gitea server are kept in a file of their own beside it,
|
||||
`accounts-forges.json`. A version of gclo from before 1.2 knows only GitHub: it
|
||||
reads every account in `accounts.json` as a GitHub account and writes the file
|
||||
back with what it knows. It neither reads nor writes the other file, so it does
|
||||
not list such accounts, and using it beside 1.2 leaves them as they are.
|
||||
not list such accounts, and using it beside 1.2 leaves them as they are. One
|
||||
thing it can do: add a GitHub account with the name of an account it does not
|
||||
see. gclo then does not choose between the two: `--account <name>`,
|
||||
`accounts edit` and `accounts remove` fail (exit 1) and say that two accounts
|
||||
have the name, until one of them is renamed in the desktop app.
|
||||
|
||||
The token of such an account is kept under an entry name made from the account's
|
||||
id, its forge and its server address as the file states them, not under `<id>`.
|
||||
|
||||
@@ -81,16 +81,25 @@ were the two lister implementations and the failure type they throw.
|
||||
GitHub account in every version: its token would be asked for again and sent to
|
||||
GitHub. Two versions on one data folder are ordinary (the app updates itself,
|
||||
the command line is a separate download), so such accounts are kept where the
|
||||
older version neither reads nor writes. The cost is two files where there was
|
||||
one: a change writes only the file of the account it changes; a name is unique
|
||||
across both, but an older version does not see the second file and can add a
|
||||
GitHub account with the name of a Gitea account, which the command line then
|
||||
finds first.
|
||||
older version neither reads nor writes. A GitHub account is the one that names
|
||||
no forge, and only that one; a record in the second file that names none is not
|
||||
read as a GitHub account. The cost is two files where there was one. A change
|
||||
itself writes only the file of the account it changes; a file that has to be put
|
||||
back, or an account that is in the wrong file, is written before the change, so
|
||||
that when it cannot be written the change fails with nothing changed and the
|
||||
vault untouched. A name is unique across both files, but an older version does
|
||||
not see the second and can add a GitHub account with the name of a Gitea
|
||||
account: the command line then refuses the name instead of choosing one of the
|
||||
two, until one is renamed in the app.
|
||||
- **What this version does not know is kept.** The kind of forge is stored as
|
||||
text; a name this version does not know (a later version's forge) makes that
|
||||
one account unusable here, not the file unreadable, and is written back as it
|
||||
was. Fields this version does not know are written back as they were read. So
|
||||
this version does not do to a later one what an older one would do to it.
|
||||
was. Fields this version does not know are written back as they were read,
|
||||
whoever built the record that is saved. So this version does not do to a later
|
||||
one what an older one would do to it. The limits a later version has to keep
|
||||
to: a field this version knows keeps its JSON type (text where text is, as for
|
||||
`Forge`; another type makes the file unreadable here), and a new field is not
|
||||
named like one of this version's computed properties.
|
||||
- **A token stays with the place it was saved for.** The forge and the server of
|
||||
an account are set when it is added and cannot be changed by an edit, in the
|
||||
app or on the command line: to move an account, it is removed and added again,
|
||||
|
||||
@@ -101,7 +101,7 @@ flowchart LR
|
||||
| T14 | PAT handling | Information disclosure | The token is offered to a host other than the one the repository was listed on: an existing folder whose `origin` points elsewhere, or a server named by the repository's own configuration during the WSL clone (for example an LFS endpoint) | Low | Critical | The git credential callback answers only a remote whose URL is HTTPS on the listed URL's host and port, and refuses every other remote with a clear error (a redirect that the listed host itself issues is followed by libgit2; that host already holds the token); a folder whose origin names another repository fails before any request; the WSL credential helper is scoped to the listed host (`credential.https://<host>.helper`) and an existing WSL destination is pulled only when its stored origin is the listed URL (#139) | Mitigated |
|
||||
| T15 | Logs and terminal | Spoofing / Tampering | Text from a repository or a server (a file path may contain a newline, an escape sequence, or a character that reorders or hides text) forges an entry in the activity log or drives the terminal the CLI prints to | Low | Medium | `DisplayText` shows control, separator and format characters (direction marks and overrides, invisible characters, tag characters) and unpaired surrogates as visible `\uXXXX` text: every log message is one line, and an exception's lines are indented under their entry so none can start like one. The CLI writes every line through one place, `Terminal`, which also shows any character the console's code page cannot represent in that form, because a legacy code page turns some printable characters into control bytes; a test fails if any other CLI file writes to the console. The JSON modes rely on the JSON encoder (#143) | Mitigated for the log and the CLI. Not covered: the desktop app's own surfaces (row error text, the path-recovery dialog), where a newline is harmless but a direction override in a path the user is asked to approve is not; and text that merely looks like something else (look-alike letters, an escape written out literally) |
|
||||
| T16 | Forge server (Gitea) | Information disclosure / Spoofing | The address of a server is typed by a user. A wrong address, a server that is hostile or misconfigured, or something between gclo and the server gets the account's token sent elsewhere, or answers in a way that sends clones (and the token with them) to another host, or makes gclo read without end | Low | Critical | `GiteaServerUrl`: HTTPS only, no credentials or query in the address, no setting for `http://`. Certificates are validated by the platform and that cannot be switched off. Every request goes to the configured address with the token in a header; redirects are not followed; the address of a next page is never taken from an answer; the client keeps no cookies. An address that is refused is not repeated in a message (it may hold a password or a token), and neither is a token. Clone addresses are built from the configured address, the owner and the name, and the server's `clone_url` is not read, so the git client's rule (the token only to the host of the clone address, T14) binds the token to the account's server. Answers are bounded in size, lists in pages and items, requests in time (reading the answer included). A server's own text reaches a message only as the reason of a refusal, on one line and cut to a length (T15). Accounts (#204): the forge and server of an account cannot be changed after it is added, so a saved token is not sent to an address somebody edited; `gclo sync --account` takes no other place from the command line; the default token (a GitHub token) is never handed to an account on another forge, and each forge has its own default token variable; accounts that are not on GitHub are kept in a file of their own, which a gclo from before forges neither reads nor rewrites, so that version cannot turn one into a GitHub account by writing back what it knows; the token of such an account is found only by that account at the address it was saved for (T13); an account whose stored forge or server cannot be used, or whose forge this version does not know, asks no server anything. | Mitigated. The server the user names sees the token: that is the purpose. A machine that trusts an intercepting certificate authority is outside this (as for GitHub) |
|
||||
| T13 | CLI / data directory | Tampering | A malicious `accounts.json`, `accounts-forges.json` or `settings.json` redirects targets, or names another place for an account | Low | Medium | Files live under the user profile; sanitized on load; target paths resolved and contained. The token of an account on a Gitea server is stored under a name made from the account's id, forge and server address as the file states them, so an account whose address was changed in the file finds no token for the new address, and a record that names another account's id finds none of that account's. What remains for someone who can write these files: a record made to look like a GitHub account can name the entry of a Gitea token (the name can be computed from the file), and that token is then offered to github.com, not to a host of their choosing | Accepted (same-user boundary: whoever can write these files can read the tokens) |
|
||||
| T13 | CLI / data directory | Tampering | A malicious `accounts.json`, `accounts-forges.json` or `settings.json` redirects targets, or names another place for an account | Low | Medium | Files live under the user profile; sanitized on load; target paths resolved and contained. The token of an account on a Gitea server is stored under a name made from the account's id, forge and server address as the file states them, so an account whose address was changed in the file finds no token for the new address, and a record that names another account's id finds none of that account's. What remains for someone who can write these files: a record made to look like a GitHub account can name the entry of a Gitea token (the name can be computed from the file), and that token is then offered to github.com, not to a host of their choosing. The app resolves a token for the place a workspace was opened for, so a file that is changed while the app runs does not bring the token of one place to the listers of another | Accepted (same-user boundary: whoever can write these files can read the tokens) |
|
||||
|
||||
## 7. Accepted risks
|
||||
|
||||
|
||||
@@ -495,4 +495,30 @@ public sealed class ForgeCliTests : IDisposable
|
||||
Assert.Null(_repos.LastToken);
|
||||
Assert.Contains(" unknown (not usable) ", console.Out, StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
|
||||
|
||||
[Fact]
|
||||
public async Task TwoAccountsOfOneName_AreNotChosenBetween()
|
||||
{
|
||||
// An older gclo does not see the accounts on another forge: it can add a GitHub account with the name of one.
|
||||
Account forge = AddGiteaAccount("platform");
|
||||
File.WriteAllText(
|
||||
Path.Combine(_dir, "accounts.json"),
|
||||
$$"""[{ "Id": "{{Guid.NewGuid()}}", "Name": "Platform", "Organization": "octo", "TargetRoot": "C:\\repos" }]""");
|
||||
_store = null; // read the files again
|
||||
using var console = new ConsoleCapture();
|
||||
const string Said = "Two accounts are named 'Platform', so the name does not say which one is meant. "
|
||||
+ "Give one of them another name in the gclo desktop app (Edit account), then try again.";
|
||||
|
||||
CliErrorException sync = await Assert.ThrowsAsync<CliErrorException>(() => Sync("--account", "platform", "--dry-run"));
|
||||
CliErrorException edit = Assert.Throws<CliErrorException>(() => Accounts("edit", "--name", "platform", "--parallel", "2"));
|
||||
CliErrorException remove = Assert.Throws<CliErrorException>(() => Accounts("remove", "--name", "platform"));
|
||||
|
||||
Assert.All([sync, edit, remove], error => Assert.Equal(Said, error.Message));
|
||||
Assert.Empty(_asked);
|
||||
Assert.Null(_repos.LastToken);
|
||||
Assert.Equal(2, Store.GetAll().Count);
|
||||
Assert.Equal("gitea-token", _vault.TryRetrieve(forge.TokenEntryId));
|
||||
}
|
||||
}
|
||||
@@ -405,6 +405,7 @@ internal static class AccountsCommand
|
||||
{
|
||||
(AccountsStore store, ITokenVault vault) = open(log);
|
||||
Account existing = store.FindByName(name.Trim()) ?? throw UnknownAccount(name.Trim(), store);
|
||||
RefuseASharedName(store, existing.Name);
|
||||
if(tokenOptions.UseDefault && existing.Forge != ForgeKind.GitHub)
|
||||
{
|
||||
throw new CliErrorException(
|
||||
@@ -482,6 +483,7 @@ internal static class AccountsCommand
|
||||
{
|
||||
(AccountsStore store, _) = open(log);
|
||||
Account existing = store.FindByName(name.Trim()) ?? throw UnknownAccount(name.Trim(), store);
|
||||
RefuseASharedName(store, existing.Name);
|
||||
store.Delete(existing.Id); // the store logs the deletion
|
||||
Terminal.Out($"Account '{existing.Name}' removed.");
|
||||
return ExitCodes.Success;
|
||||
@@ -622,6 +624,24 @@ internal static class AccountsCommand
|
||||
|
||||
|
||||
|
||||
/// <summary>
|
||||
/// Two accounts of one name: the command does not choose one of them. It can
|
||||
/// only come about through a version of gclo from before forges, which does not
|
||||
/// see the accounts on another forge and so does not know the name is taken.
|
||||
/// </summary>
|
||||
/// <exception cref="CliErrorException">More than one account is named <paramref name="name"/>.</exception>
|
||||
internal static void RefuseASharedName(AccountsStore store, string name)
|
||||
{
|
||||
if(store.IsNameShared(name))
|
||||
{
|
||||
throw new CliErrorException(
|
||||
$"Two accounts are named '{name}', so the name does not say which one is meant. "
|
||||
+ "Give one of them another name in the gclo desktop app (Edit account), then try again.");
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
||||
private static CliErrorException UnknownAccount(string name, AccountsStore store)
|
||||
{
|
||||
IReadOnlyList<Account> all = store.GetAll();
|
||||
|
||||
@@ -545,6 +545,7 @@ internal static class SyncCommand
|
||||
{
|
||||
(AccountsStore store, ITokenVault vault) = openAccounts(log);
|
||||
Account account = store.FindByName(accountName) ?? throw UnknownAccount(accountName, store);
|
||||
AccountsCommand.RefuseASharedName(store, account.Name);
|
||||
return (account, store, vault);
|
||||
}
|
||||
catch(Exception ex) when(CliErrorException.IsStorageFailure(ex))
|
||||
|
||||
@@ -361,10 +361,9 @@ public sealed class AccountFilesTests : IDisposable
|
||||
|
||||
|
||||
[Theory]
|
||||
[InlineData(null, ForgeKind.GitHub)]
|
||||
[InlineData("GitHub", ForgeKind.GitHub)]
|
||||
[InlineData("github", ForgeKind.GitHub)]
|
||||
[InlineData(null, ForgeKind.GitHub)] // GitHub is the account that names no forge
|
||||
[InlineData("Gitea", ForgeKind.Gitea)]
|
||||
[InlineData("gitea", ForgeKind.Gitea)]
|
||||
[InlineData("GITEA", ForgeKind.Gitea)]
|
||||
public void TheNameOfAForge_IsReadWhateverItsCase(string? stored, ForgeKind kind)
|
||||
{
|
||||
@@ -380,12 +379,43 @@ public sealed class AccountFilesTests : IDisposable
|
||||
[InlineData("")]
|
||||
[InlineData("1")] // not a number either: a kind is named
|
||||
[InlineData("Forgejo")]
|
||||
[InlineData("GitHub")] // a GitHub account names no forge: one that says it is on GitHub was not written by gclo
|
||||
public void ANameThatIsNoForgeThisVersionKnows_IsNoKindAtAll(string stored)
|
||||
{
|
||||
Account account = GitHubAccount() with { ForgeName = stored };
|
||||
|
||||
Assert.False(account.IsOnAKnownForge);
|
||||
Assert.False(Enum.IsDefined(account.Forge));
|
||||
Assert.NotEqual(account.Id, account.TokenEntryId);
|
||||
Assert.IsType<UnusableAccountLister>(AccountListers.RepositoryListerFor(account));
|
||||
}
|
||||
|
||||
|
||||
|
||||
[Fact]
|
||||
public void ARecordInTheFileForOtherForges_ThatNamesNoForge_IsNotAGitHubAccount_AndStaysThere()
|
||||
{
|
||||
// Written by hand: gclo writes no such record. Read as a GitHub account it would get GitHub's listers and the default token.
|
||||
Account odd = GitHubAccount("Odd");
|
||||
Account work = GitHubAccount();
|
||||
WriteFile(ForgesFile, Record(odd, $"\"forge\": \"Gitea\", \"Server\": \"{Server}\"")); // the key in another case: not the field
|
||||
_vault.Store(AppSettings.DefaultTokenVaultId, "ghp_default");
|
||||
_vault.Store(odd.Id, "a token under its id");
|
||||
AccountsStore store = NewStore();
|
||||
|
||||
Account read = Assert.Single(store.GetAll());
|
||||
|
||||
Assert.False(read.IsOnAKnownForge);
|
||||
Assert.IsType<UnusableAccountLister>(AccountListers.OrganizationListerFor(read));
|
||||
var resolver = new AccountTokenResolver(_vault);
|
||||
Assert.Null(resolver.TryResolve(read));
|
||||
Assert.Null(resolver.TryResolve(read with { TokenSource = TokenSource.Default }));
|
||||
|
||||
store.Save(work, "ghp_token"); // a change to another account
|
||||
|
||||
Assert.DoesNotContain("Odd", File.ReadAllText(GitHubFile), StringComparison.Ordinal);
|
||||
Assert.Contains("\"Name\": \"Odd\"", File.ReadAllText(ForgesFile), StringComparison.Ordinal);
|
||||
Assert.False(NewStore().FindByName("Odd")!.IsOnAKnownForge);
|
||||
}
|
||||
|
||||
|
||||
@@ -420,6 +450,30 @@ public sealed class AccountFilesTests : IDisposable
|
||||
|
||||
|
||||
|
||||
[Fact]
|
||||
public async Task WhatALaterVersionSaysAboutAnAccount_SurvivesAnEditInTheWizard_WhichBuildsTheRecordAnew()
|
||||
{
|
||||
Account work = GitHubAccount();
|
||||
WriteFile(GitHubFile, Record(work, "\"Colour\": \"blue\""));
|
||||
_vault.Store(work.Id, "ghp_token");
|
||||
AccountsStore store = NewStore();
|
||||
Account existing = store.FindByName("Work")!;
|
||||
var wizard = new AccountWizardViewModel(store, new FakeOrganizationLister(), new AppSettings(), existing, () => "ghp_token");
|
||||
|
||||
wizard.Description = "edited in the wizard";
|
||||
while(wizard.Step < 4)
|
||||
{
|
||||
Assert.True(await wizard.TryAdvanceAsync(), $"stopped at step {wizard.Step}");
|
||||
}
|
||||
await wizard.SaveAsync();
|
||||
|
||||
JsonNode written = JsonNode.Parse(File.ReadAllText(GitHubFile))![0]!;
|
||||
Assert.Equal("edited in the wizard", (string?)written["Description"]);
|
||||
Assert.Equal("blue", (string?)written["Colour"]);
|
||||
}
|
||||
|
||||
|
||||
|
||||
// ---------------------------------------------------------------- a token belongs to one account at one place
|
||||
|
||||
[Fact]
|
||||
@@ -553,6 +607,101 @@ public sealed class AccountFilesTests : IDisposable
|
||||
|
||||
|
||||
|
||||
// ---------------------------------------------------------------- what has to be written first
|
||||
|
||||
[Fact]
|
||||
public void AChangeThatFirstHasToPutAFileBack_AndCannot_ChangesNothing_NotTheVaultEither()
|
||||
{
|
||||
Account work = GitHubAccount();
|
||||
Account forge = GiteaAccount();
|
||||
Account another = GiteaAccount("Another", "https://other.example.test/");
|
||||
AccountsStore store = NewStore();
|
||||
store.Save(work, "ghp_token");
|
||||
store.Save(forge, "gitea_token");
|
||||
string forgesBefore = File.ReadAllText(ForgesFile);
|
||||
File.Delete(GitHubFile);
|
||||
Directory.CreateDirectory(GitHubFile + ".tmp"); // accounts.json cannot be written back
|
||||
|
||||
// Neither change concerns accounts.json. Both would have written their own file and then failed on the other.
|
||||
Assert.Throws<UnauthorizedAccessException>(() => store.Delete(forge.Id));
|
||||
Assert.Throws<UnauthorizedAccessException>(() => store.Save(another, "another_token"));
|
||||
|
||||
Assert.Equal(forgesBefore, File.ReadAllText(ForgesFile));
|
||||
Assert.Equal("gitea_token", _vault.TryRetrieve(forge.TokenEntryId));
|
||||
Assert.Null(_vault.TryRetrieve(another.TokenEntryId));
|
||||
Assert.Equal(["Forge", "Work"], store.GetAll().Select(a => a.Name), StringComparer.Ordinal);
|
||||
|
||||
// Once the file can be written, the same change goes through and the accounts are back.
|
||||
Directory.Delete(GitHubFile + ".tmp");
|
||||
store.Delete(forge.Id);
|
||||
|
||||
Assert.Null(_vault.TryRetrieve(forge.TokenEntryId));
|
||||
Assert.Equal("Work", Assert.Single(NewStore().GetAll()).Name);
|
||||
}
|
||||
|
||||
|
||||
|
||||
[Fact]
|
||||
public void AnAccountInTheWrongFile_ThatCannotBeMoved_StaysWhereItIs()
|
||||
{
|
||||
// The file for other forges is written first: an account is in its own file before it leaves the other.
|
||||
Account work = GitHubAccount();
|
||||
Account forge = GiteaAccount();
|
||||
WriteFile(GitHubFile, Record(work), Record(forge, $"\"Forge\": \"Gitea\", \"Server\": \"{Server}\""));
|
||||
string before = File.ReadAllText(GitHubFile);
|
||||
AccountsStore store = NewStore();
|
||||
Directory.CreateDirectory(ForgesFile + ".tmp"); // its own file cannot be written
|
||||
|
||||
Assert.Throws<UnauthorizedAccessException>(() => store.RecordSyncResult(work.Id, DateTimeOffset.UtcNow, "5 synced"));
|
||||
|
||||
Assert.Equal(before, File.ReadAllText(GitHubFile));
|
||||
Assert.Equal(["Forge", "Work"], NewStore().GetAll().Select(a => a.Name), StringComparer.Ordinal);
|
||||
}
|
||||
|
||||
|
||||
|
||||
[Fact]
|
||||
public void TwoAccountsOfOneName_AreSaidToShareIt()
|
||||
{
|
||||
// An older version does not see the accounts on another forge, and can give a GitHub account the name of one.
|
||||
Account forge = GiteaAccount("Platform");
|
||||
NewStore().Save(forge, "gitea_token");
|
||||
WriteFile(GitHubFile, Record(GitHubAccount("platform")), Record(GitHubAccount("Work")));
|
||||
|
||||
AccountsStore store = NewStore();
|
||||
|
||||
Assert.True(store.IsNameShared("PLATFORM"));
|
||||
Assert.False(store.IsNameShared("Work"));
|
||||
Assert.False(store.IsNameShared("nobody"));
|
||||
}
|
||||
|
||||
|
||||
|
||||
// ---------------------------------------------------------------- a workspace's token is the one of the place it was opened for
|
||||
|
||||
[Fact]
|
||||
public void AWorkspace_TakesNoTokenThatWasSavedForAnotherPlaceThanTheOneItWasOpenedFor()
|
||||
{
|
||||
// The file named another server when the workspace was opened; it names the account's own again by now.
|
||||
Account forge = GiteaAccount();
|
||||
AccountsStore store = NewStore();
|
||||
store.Save(forge, "gitea_token");
|
||||
Account asItReadAtOpen = forge with { Server = "https://elsewhere.example.test/" };
|
||||
var listers = new FakeRepositoryLister();
|
||||
|
||||
using var workspace = new WorkspaceViewModel(
|
||||
listers, new FakeGitClient(), new FakeOrganizationLister(), orgLookupDebounce: TimeSpan.FromMilliseconds(1),
|
||||
account: asItReadAtOpen, tokenVault: _vault, accountsStore: store);
|
||||
Assert.Equal("", workspace.Token); // nothing was saved for that place
|
||||
|
||||
workspace.RefreshAccountToken(); // the store now has the record that names the place the token was saved for
|
||||
|
||||
Assert.Equal("", workspace.Token);
|
||||
Assert.Equal("No token is saved for this account. Choose Edit account… and enter one.", workspace.MissingTokenAdvice);
|
||||
}
|
||||
|
||||
|
||||
|
||||
// ---------------------------------------------------------------- a workspace without listers of its own
|
||||
|
||||
[Fact]
|
||||
|
||||
@@ -50,16 +50,17 @@ public sealed record Account
|
||||
public string? ForgeName { get; init; }
|
||||
|
||||
/// <summary>
|
||||
/// The kind of forge the account is on; GitHub when <see cref="ForgeName"/> says
|
||||
/// nothing. For a name this version does not know it is none of the kinds
|
||||
/// The kind of forge the account is on. GitHub is the account that names no
|
||||
/// forge, and only that one: it is how every version has written a GitHub
|
||||
/// account. For a name this version does not know it is none of the kinds
|
||||
/// (<see cref="IsOnAKnownForge"/> is false): never GitHub, so nothing takes such
|
||||
/// an account for a GitHub account.
|
||||
/// </summary>
|
||||
[JsonIgnore]
|
||||
public ForgeKind Forge
|
||||
{
|
||||
get => ForgeName is null || Names(ForgeKind.GitHub) ? ForgeKind.GitHub
|
||||
: Names(ForgeKind.Gitea) ? ForgeKind.Gitea
|
||||
get => ForgeName is null ? ForgeKind.GitHub
|
||||
: string.Equals(ForgeName, nameof(ForgeKind.Gitea), StringComparison.OrdinalIgnoreCase) ? ForgeKind.Gitea
|
||||
: s_noKnownForge;
|
||||
init => ForgeName = value == ForgeKind.GitHub ? null : value.ToString();
|
||||
}
|
||||
@@ -90,7 +91,8 @@ public sealed record Account
|
||||
/// What the file says about the account that this version of gclo does not know
|
||||
/// (a field a later version added). Kept, and written back as it was read, so
|
||||
/// that changing an account here does not take away what a later version put
|
||||
/// there. Null when there is nothing of the kind.
|
||||
/// there. Null when there is nothing of the kind. <see cref="AccountsStore.Save"/>
|
||||
/// keeps what the file has, whatever the record it is given says here.
|
||||
/// </summary>
|
||||
[JsonExtensionData]
|
||||
public IDictionary<string, JsonElement>? FieldsOfALaterVersion { get; set; }
|
||||
@@ -149,10 +151,6 @@ public sealed record Account
|
||||
|
||||
|
||||
|
||||
private bool Names(ForgeKind kind) => string.Equals(ForgeName, kind.ToString(), StringComparison.OrdinalIgnoreCase);
|
||||
|
||||
|
||||
|
||||
/// <summary>
|
||||
/// Where the account lives, or the reason its forge and server cannot be used as
|
||||
/// they are stored (a file edited by hand, or a forge this version does not know).
|
||||
|
||||
@@ -66,8 +66,16 @@ namespace gclo.ViewModels;
|
||||
/// forge are therefore kept in a file of their own, which such a version neither
|
||||
/// reads nor writes. Both files are changed under the one lock. An account on
|
||||
/// another forge that is found in accounts.json (gclo does not put one there) is
|
||||
/// moved to its own file by the next change, and one that is in both files is read
|
||||
/// from its own.
|
||||
/// moved to its own file before the next change is applied, and one that is in
|
||||
/// both files is read from its own. A record in the file for other forges that
|
||||
/// names no forge is not a GitHub account: it stays there, and cannot be used.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// A change itself writes one file, the one of the account it changes. Whatever
|
||||
/// else has to be written (a file that went missing or was damaged, an account
|
||||
/// that is in the wrong file) is written first, before the vault is touched: when
|
||||
/// that fails the change fails with nothing changed, and the order and the
|
||||
/// compensating steps above hold for one file as they always did.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// A file that cannot be read is not a corrupt file. Only content that is not valid
|
||||
@@ -152,7 +160,7 @@ public sealed class AccountsStore
|
||||
|
||||
|
||||
|
||||
/// <summary>The account whose name matches case-insensitively, or null.</summary>
|
||||
/// <summary>The account whose name matches case-insensitively, or null. See <see cref="IsNameShared"/>.</summary>
|
||||
public Account? FindByName(string name)
|
||||
{
|
||||
lock(_gate)
|
||||
@@ -164,6 +172,23 @@ public sealed class AccountsStore
|
||||
|
||||
|
||||
|
||||
/// <summary>
|
||||
/// True when more than one account has the name. <see cref="Save"/> does not let
|
||||
/// that happen, but a version of gclo from before forges does not see the accounts
|
||||
/// on another forge and can give a GitHub account the name of one of them. A
|
||||
/// caller that is given a name to act on asks here first, so it does not act on
|
||||
/// the wrong one of the two.
|
||||
/// </summary>
|
||||
public bool IsNameShared(string name)
|
||||
{
|
||||
lock(_gate)
|
||||
{
|
||||
return _accounts.Count(a => string.Equals(a.Name, name, StringComparison.OrdinalIgnoreCase)) > 1;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// <summary>
|
||||
/// Inserts or updates (matching by <see cref="Account.Id"/>) and persists
|
||||
/// immediately, on top of what the file holds at that moment. Throws <see cref="ArgumentException"/> when another account
|
||||
@@ -471,10 +496,12 @@ public sealed class AccountsStore
|
||||
|
||||
|
||||
/// <summary>
|
||||
/// Brings what this store holds up to what the file holds now, before a change is
|
||||
/// Brings what this store holds up to what the files hold now, before a change is
|
||||
/// applied. Called with the file lock held. A file that is gone or damaged is not
|
||||
/// taken for "no accounts": the change is then applied to what this store last
|
||||
/// read, and writing it puts the accounts back.
|
||||
/// read, and the accounts are put back here, before the change. So is an account
|
||||
/// that is in the wrong file. When that cannot be written the change fails
|
||||
/// before it touched the vault, and the change itself then has one file to write.
|
||||
/// </summary>
|
||||
private void RefreshForAChange()
|
||||
{
|
||||
@@ -489,6 +516,7 @@ public sealed class AccountsStore
|
||||
}
|
||||
}
|
||||
_accounts = Union();
|
||||
Persist(_accounts);
|
||||
}
|
||||
|
||||
|
||||
@@ -497,12 +525,15 @@ public sealed class AccountsStore
|
||||
/// The accounts of both files as one list. An account that is in both (by its
|
||||
/// id) is the one of the file for other forges: that is the file gclo writes such
|
||||
/// an account to, and the copy in accounts.json is what an older version may
|
||||
/// have rewritten.
|
||||
/// have rewritten. A record of the file for other forges that names no forge
|
||||
/// (gclo writes none) is not read as a GitHub account, which would give it
|
||||
/// GitHub's listers and the default token: it names a forge nobody knows.
|
||||
/// </summary>
|
||||
private List<Account> Union()
|
||||
{
|
||||
HashSet<Guid> apart = [.. _forgesFile.Accounts.Select(a => a.Id)];
|
||||
return [.. _gitHubFile.Accounts.Where(a => !apart.Contains(a.Id)), .. _forgesFile.Accounts];
|
||||
List<Account> apart = [.. _forgesFile.Accounts.Select(a => a.ForgeName is null ? a with { ForgeName = "" } : a)];
|
||||
HashSet<Guid> theirIds = [.. apart.Select(a => a.Id)];
|
||||
return [.. _gitHubFile.Accounts.Where(a => !theirIds.Contains(a.Id)), .. apart];
|
||||
}
|
||||
|
||||
|
||||
@@ -518,9 +549,9 @@ public sealed class AccountsStore
|
||||
|
||||
/// <summary>
|
||||
/// Writes each account to the file it belongs in. Only a file whose accounts
|
||||
/// changed is written, so one change is one file put in place. (The exception is
|
||||
/// an account on another forge that was found in accounts.json: it is written to
|
||||
/// its own file first, and then leaves the other.)
|
||||
/// changed is written. The file for other forges goes first: an account that was
|
||||
/// found in accounts.json is in its own file before it leaves the other, so a
|
||||
/// failure in between leaves it in both, never in neither.
|
||||
/// </summary>
|
||||
private void Persist(List<Account> accounts)
|
||||
{
|
||||
@@ -549,6 +580,11 @@ public sealed class AccountsStore
|
||||
: account;
|
||||
}
|
||||
|
||||
///////////////////////////////////////////////////////////////////////////////
|
||||
// What a later version of gclo put on the account stays, whatever record the //
|
||||
// caller built: one that was made anew (the wizard's) knows nothing of it. //
|
||||
///////////////////////////////////////////////////////////////////////////////
|
||||
account = account with { FieldsOfALaterVersion = onDisk.FieldsOfALaterVersion };
|
||||
return onDisk.LastSyncUtc > (account.LastSyncUtc ?? DateTimeOffset.MinValue)
|
||||
? account with { LastSyncUtc = onDisk.LastSyncUtc, LastSyncSummary = onDisk.LastSyncSummary }
|
||||
: account;
|
||||
|
||||
@@ -588,8 +588,16 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable
|
||||
/// was opened. The record the workspace was opened with when the store no longer
|
||||
/// has the account, or when there is no store.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The place is always the one the workspace was opened for. Its listers were
|
||||
/// made for that place and stay; a record that names another place by now (the
|
||||
/// file was changed) must not bring the token of that other place to them.
|
||||
/// </remarks>
|
||||
private Account AccountAsItIsNow()
|
||||
=> _accountsStore?.GetAll().FirstOrDefault(a => a.Id == _account!.Id) ?? _account!;
|
||||
{
|
||||
Account now = _accountsStore?.GetAll().FirstOrDefault(a => a.Id == _account!.Id) ?? _account!;
|
||||
return now with { ForgeName = _account!.ForgeName, Server = _account.Server };
|
||||
}
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user