diff --git a/.gitignore b/.gitignore index dc0470a..e043c9f 100644 --- a/.gitignore +++ b/.gitignore @@ -203,6 +203,11 @@ PublishScripts/ **/[Pp]ackages/* # except build/, which is used as an MSBuild target. !**/[Pp]ackages/build/ +# and except a Unity project's Packages/, which is source: Unity's package manifest and its +# resolved lock file are both meant to be committed, and a NuGet restore folder never contains +# a file by either name. +!**/[Pp]ackages/manifest.json +!**/[Pp]ackages/packages-lock.json # Uncomment if necessary however generally it will be regenerated when needed #!**/[Pp]ackages/repositories.config # NuGet v3's project.json files produces more ignorable files @@ -651,3 +656,16 @@ Temporary Items # ImGui.ini files imgui.ini + +# Game engine projects +# +# Godot: the import cache, and the mono/temp bin+obj a C# build writes. +.godot/ + +# Unity: .meta files are source, not the Visual Studio C++ build artifact that the `*.meta` rule +# further up targets. Unity generates one per asset and it carries the GUID that scenes, prefabs +# and serialized references point at, so ignoring them gives every clone fresh GUIDs and silently +# breaks those references - including for a plug-in whose .dll is itself a build output. This +# negation has to come after that rule to win, and is scoped to the asset tree so the Visual +# Studio artifact stays ignored everywhere else. +!**/[Aa]ssets/**/*.meta diff --git a/CLAUDE.md b/CLAUDE.md index 465e273..a4a95a7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,7 +41,13 @@ dotnet test --configuration Release **[ProjectDirectorOptions.cs](ProjectDirector/ProjectDirectorOptions.cs)** - Application state - Extends `AppData` from ktsu.AppDataStorage for automatic JSON persistence -- Stores: dev directory path, GitHub credentials, repo cache, UI state (divider positions, panel states) +- Stores: dev directory path, which GitHub owners are configured, repo cache, UI state (divider + positions, panel states) +- **Not** GitHub tokens. Those live in the OS secret store via + [TokenStorage.cs](ProjectDirector/TokenStorage.cs), because this file sits next to UI preferences + and is rewritten on every debounced save +- `LegacyGitHubToken` and `LegacyGitHubOwners` keep the old JSON names purely for the one-time + migration run at startup: tokens are written to the secret store first, then emptied here - Semantic string types for type-safe paths and identifiers **[GitRepository.cs](ProjectDirector/GitRepository.cs)** - Repository abstraction @@ -62,7 +68,7 @@ dotnet test --configuration Release Git LFS is a pair of filters plus a set of hooks, and all of them belong to the git command. A library that reads and writes the object database directly bypasses them: a commit stores raw bytes where a pointer belongs, and a clone or checkout lands the pointer text on disk where the file belongs. This application clones, fetches and pulls, so it is the checkout side that matters here. `ProjectDirector.Test` pins both halves down. -Authentication follows from the same decision. There are no credentials in this code, because git uses the platform credential helper, which is also what makes SSH remotes work. +Authentication follows from the same decision. There are no credentials in this code for git operations, because git uses the platform credential helper, which is also what makes SSH remotes work. The GitHub API tokens the app does hold follow the same principle: `TokenStorage` keeps them in the platform secret store (Windows Credential Manager, macOS Keychain, libsecret on Linux), derives a persona per token from a versioned namespace plus the login or owner name, and never falls back to a plain file. With no usable store, tokens read as empty and the reason reaches the log once — a throw out of a token read would take down the render loop. ### Key Dependencies diff --git a/Directory.Packages.props b/Directory.Packages.props index f5c1892..20129b9 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -7,6 +7,7 @@ + diff --git a/ProjectDirector.Test/TokenStorageTests.cs b/ProjectDirector.Test/TokenStorageTests.cs new file mode 100644 index 0000000..c798fee --- /dev/null +++ b/ProjectDirector.Test/TokenStorageTests.cs @@ -0,0 +1,448 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector.Test; + +using System.Text.Json; +using System.Text.Json.Serialization; + +using ktsu.CredentialCache; +using ktsu.CredentialCache.Storage; +using ktsu.RoundTripStringJsonConverter; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Octokit; + +using CredentialCache = ktsu.CredentialCache.CredentialCache; +using ICredentialStore = ktsu.CredentialCache.Storage.ICredentialStore; + +/// +/// Covers where GitHub personal access tokens live. The settings file sits next to window state and +/// UI preferences and is written on every debounced save, so the contract under test is that a PAT +/// reaches the OS secret store and does not survive in that file. +/// +/// +/// Follows the repository's own rule for this codebase: the part with a decision in it is a plain +/// class, so it can be driven without a live ImGui context or a display. +/// +[TestClass] +public sealed class TokenStorageTests +{ + private CredentialCache Cache { get; set; } = null!; + + [TestInitialize] + public void SetUp() + { + Cache = new CredentialCache(new InMemoryCredentialStore()); + TokenStorage.UseCache(Cache); + } + + [TestCleanup] + public void TearDown() + { + TokenStorage.UseCache(null); + Cache.Dispose(); + } + + /// + /// A store standing in for a machine whose native secret library will not load — a Linux host + /// with no Secret Service provider, which is what a container or an SSH session usually is. + /// + private sealed class UnavailableCredentialStore : ICredentialStore + { + public string Name => "Unavailable"; + + public bool TryLoad(PersonaGUID persona, out Credential? credential) => + throw new DllNotFoundException("libsecret-1.so.0"); + + public void Save(PersonaGUID persona, Credential credential) => + throw new DllNotFoundException("libsecret-1.so.0"); + + public bool Remove(PersonaGUID persona) => throw new DllNotFoundException("libsecret-1.so.0"); + } + + /// + /// Mirrors how writes the settings file, so a token would show + /// as a plain string rather than as the char array a bare serializer produces for a semantic + /// string. + /// + private static readonly JsonSerializerOptions AppDataLike = BuildAppDataLikeOptions(); + + private static JsonSerializerOptions BuildAppDataLikeOptions() + { + JsonSerializerOptions options = new() + { + ReferenceHandler = ReferenceHandler.Preserve, + }; + options.Converters.Add(new RoundTripStringJsonConverterFactory()); + return options; + } + + private static GitHubOwnerName Owner(string name) => GitHubOwnerName.Create(name); + + private static GitHubToken Token(string value) => GitHubToken.Create(value); + + /// + /// Two owners never share a persona. + /// + [TestMethod] + public void OwnerPersonasAreDistinct() => + Assert.AreNotEqual(TokenStorage.OwnerPersona(Owner("ktsu-dev")), TokenStorage.OwnerPersona(Owner("ktsu-io"))); + + /// + /// The account-level token and an owner token of the same name are separate entries, so one + /// cannot overwrite the other. + /// + [TestMethod] + public void LoginPersonaIsNotAnOwnerPersona() => + Assert.AreNotEqual( + TokenStorage.LoginPersona(GitHubLogin.Create("ktsu-dev")), + TokenStorage.OwnerPersona(Owner("ktsu-dev"))); + + /// + /// Derivation is deterministic and GUID-shaped. Changing it would orphan every token already in + /// the store. + /// + [TestMethod] + public void PersonaDerivationIsStable() + { + Assert.AreEqual( + TokenStorage.OwnerPersona(Owner("ktsu-dev")).ToString(), + TokenStorage.OwnerPersona(Owner("ktsu-dev")).ToString()); + Assert.IsTrue(Guid.TryParse(TokenStorage.OwnerPersona(Owner("ktsu-dev")).ToString(), out _)); + } + + /// + /// An owner token written through the normal path is readable again. + /// + [TestMethod] + public void OwnerTokenRoundTripsThroughTheStore() + { + Assert.IsTrue(TokenStorage.WriteOwnerToken(Owner("ktsu-dev"), Token("ghp_owner"))); + + Assert.AreEqual("ghp_owner", TokenStorage.ReadOwnerToken(Owner("ktsu-dev")).ToString()); + Assert.IsTrue(TokenStorage.ReadOwnerToken(Owner("ktsu-io")).IsEmpty()); + } + + /// + /// The account-level token set through the options property goes to the store, keyed by the + /// login it belongs to. + /// + [TestMethod] + public void AccountTokenRoundTripsThroughTheStore() + { + using ProjectDirectorOptions options = new() + { + GitHubLogin = GitHubLogin.Create("someone"), + GitHubToken = Token("ghp_account"), + }; + + Assert.AreEqual("ghp_account", options.GitHubToken.ToString()); + Assert.IsTrue(options.LegacyGitHubToken.IsEmpty()); + } + + /// + /// Clearing a token removes the entry rather than leaving an empty one behind. + /// + [TestMethod] + public void ClearingATokenRemovesItFromTheStore() + { + _ = TokenStorage.WriteOwnerToken(Owner("ktsu-dev"), Token("ghp_owner")); + + Assert.IsTrue(TokenStorage.WriteOwnerToken(Owner("ktsu-dev"), new())); + + Assert.IsFalse(Cache.TryGet(TokenStorage.OwnerPersona(Owner("ktsu-dev")), out _)); + } + + /// + /// The settings file an earlier version wrote carries its tokens into the store, and the owners + /// it listed become the owner registry. + /// + [TestMethod] + public void MigrationMovesAccountAndOwnerTokens() + { + using ProjectDirectorOptions options = new() + { + GitHubLogin = GitHubLogin.Create("someone"), + LegacyGitHubToken = Token("ghp_account"), + LegacyGitHubOwners = + { + [Owner("ktsu-dev")] = Token("ghp_owner"), + [Owner("ktsu-io")] = new(), + }, + }; + + int migrated = TokenStorage.MigrateLegacyTokens(options); + + Assert.AreEqual(2, migrated); + Assert.AreEqual("ghp_account", options.GitHubToken.ToString()); + Assert.AreEqual("ghp_owner", TokenStorage.ReadOwnerToken(Owner("ktsu-dev")).ToString()); + Assert.IsTrue(options.GitHubOwners.Contains(Owner("ktsu-dev"))); + Assert.IsTrue(options.GitHubOwners.Contains(Owner("ktsu-io"))); + } + + /// + /// Migration also empties the old fields. Ceasing to write a secret does not remove the one + /// already on disk, so this is the half that does the security work. + /// + [TestMethod] + public void MigrationEmptiesThePlaintextFields() + { + using ProjectDirectorOptions options = new() + { + LegacyGitHubToken = Token("ghp_account"), + LegacyGitHubOwners = { [Owner("ktsu-dev")] = Token("ghp_owner") }, + }; + + _ = TokenStorage.MigrateLegacyTokens(options); + + Assert.IsTrue(options.LegacyGitHubToken.IsEmpty()); + Assert.AreEqual(0, options.LegacyGitHubOwners.Count); + } + + /// + /// The settings file written after migration carries no token. + /// + [TestMethod] + public void SerializedOptionsCarryNoToken() + { + using ProjectDirectorOptions options = new() + { + LegacyGitHubToken = Token("ghp_account_secret"), + LegacyGitHubOwners = { [Owner("ktsu-dev")] = Token("ghp_owner_secret") }, + }; + + _ = TokenStorage.MigrateLegacyTokens(options); + string json = JsonSerializer.Serialize(options, AppDataLike); + + Assert.DoesNotContain("ghp_account_secret", json, StringComparison.Ordinal); + Assert.DoesNotContain("ghp_owner_secret", json, StringComparison.Ordinal); + Assert.Contains("ktsu-dev", json, StringComparison.Ordinal); + } + + /// + /// A second start has nothing left to move. + /// + [TestMethod] + public void MigrationIsIdempotent() + { + using ProjectDirectorOptions options = new() + { + LegacyGitHubToken = Token("ghp_account"), + }; + + _ = TokenStorage.MigrateLegacyTokens(options); + + Assert.AreEqual(0, TokenStorage.MigrateLegacyTokens(options)); + Assert.AreEqual("ghp_account", options.GitHubToken.ToString()); + } + + /// + /// A stale token in the settings file must not overwrite the one currently in the store — but it + /// is still cleared. + /// + [TestMethod] + public void MigrationKeepsTheStoredTokenAndStillClearsTheStaleOne() + { + _ = TokenStorage.WriteOwnerToken(Owner("ktsu-dev"), Token("ghp_current")); + using ProjectDirectorOptions options = new() + { + LegacyGitHubOwners = { [Owner("ktsu-dev")] = Token("ghp_stale") }, + }; + + Assert.AreEqual(1, TokenStorage.MigrateLegacyTokens(options)); + Assert.AreEqual("ghp_current", TokenStorage.ReadOwnerToken(Owner("ktsu-dev")).ToString()); + Assert.AreEqual(0, options.LegacyGitHubOwners.Count); + } + + /// + /// If the secret store will not take the token, the old copy stays where it is. Clearing it would + /// destroy the only copy the user has. + /// + [TestMethod] + public void MigrationKeepsTheLegacyTokenWhenTheStoreRefuses() + { + using CredentialCache unavailable = new(new UnavailableCredentialStore()); + TokenStorage.UseCache(unavailable); + + using ProjectDirectorOptions options = new() + { + LegacyGitHubOwners = { [Owner("ktsu-dev")] = Token("ghp_owner") }, + }; + + Assert.AreEqual(0, TokenStorage.MigrateLegacyTokens(options)); + Assert.AreEqual("ghp_owner", options.LegacyGitHubOwners[Owner("ktsu-dev")].ToString()); + Assert.IsTrue(options.GitHubOwners.Contains(Owner("ktsu-dev"))); + } + + /// + /// A machine with no secret store reads as "no token" rather than throwing, and says why once. + /// ProjectDirector is a desktop application, and an exception out of a token read would take down + /// the render loop. What it must never do is fall back to a plain file. + /// + [TestMethod] + public void ReadingWithoutASecretStoreIsEmptyAndReportedOnce() + { + using CredentialCache unavailable = new(new UnavailableCredentialStore()); + TokenStorage.UseCache(unavailable); + + Assert.IsTrue(TokenStorage.ReadOwnerToken(Owner("ktsu-dev")).IsEmpty()); + Assert.IsTrue(TokenStorage.StoreUnavailable); + + string report = TokenStorage.DrainUnavailableReport(); + Assert.Contains("secret store", report, StringComparison.Ordinal); + Assert.AreEqual(string.Empty, TokenStorage.DrainUnavailableReport()); + } + + /// + /// A write to an unusable store reports failure rather than pretending it saved. + /// + [TestMethod] + public void WritingWithoutASecretStoreReportsFailure() + { + using CredentialCache unavailable = new(new UnavailableCredentialStore()); + TokenStorage.UseCache(unavailable); + + Assert.IsFalse(TokenStorage.WriteOwnerToken(Owner("ktsu-dev"), Token("ghp_owner"))); + } + + /// + /// An owner with its own token authenticates as that owner, which is what makes a private + /// repository in another organization reachable. + /// + [TestMethod] + public void OwnerTokenShadowsTheAccountToken() + { + Credentials? credentials = ProjectDirector.ResolveGitHubCredentials( + Owner("ktsu-dev"), + Token("ghp_owner"), + GitHubLogin.Create("someone"), + Token("ghp_account")); + + Assert.IsNotNull(credentials); + Assert.AreEqual("ktsu-dev", credentials.Login); + Assert.AreEqual("ghp_owner", credentials.Password); + } + + /// + /// Without an owner token the account-level login and token are used. + /// + [TestMethod] + public void AccountCredentialsAreUsedWhenTheOwnerHasNoToken() + { + Credentials? credentials = ProjectDirector.ResolveGitHubCredentials( + Owner("ktsu-dev"), + new(), + GitHubLogin.Create("someone"), + Token("ghp_account")); + + Assert.IsNotNull(credentials); + Assert.AreEqual("someone", credentials.Login); + Assert.AreEqual("ghp_account", credentials.Password); + } + + /// + /// With nothing usable the answer is no credentials, rather than credentials carrying a blank + /// secret — an anonymous request is a better failure than one that authenticates as nobody. + /// + [TestMethod] + public void NoCredentialsWhenNothingIsConfigured() + { + Assert.IsNull(ProjectDirector.ResolveGitHubCredentials(Owner("ktsu-dev"), new(), new(), new())); + Assert.IsNull(ProjectDirector.ResolveGitHubCredentials( + Owner("ktsu-dev"), new(), GitHubLogin.Create("someone"), new())); + Assert.IsNull(ProjectDirector.ResolveGitHubCredentials( + Owner("ktsu-dev"), new(), new(), Token("ghp_account"))); + } + + /// + /// A token typed into the popup reaches the secret store, and nothing is logged. + /// + [TestMethod] + public void ApplyOwnerTokenStoresTheTokenSilently() + { + string message = ProjectDirector.ApplyOwnerToken(Owner("ktsu-dev"), "ghp_typed"); + + Assert.AreEqual(string.Empty, message); + Assert.AreEqual("ghp_typed", TokenStorage.ReadOwnerToken(Owner("ktsu-dev")).ToString()); + } + + /// + /// When the store refuses the token the user is told. The popup closes either way, so silence + /// here would look exactly like success. + /// + [TestMethod] + public void ApplyOwnerTokenReportsARefusal() + { + using CredentialCache unavailable = new(new UnavailableCredentialStore()); + TokenStorage.UseCache(unavailable); + + string message = ProjectDirector.ApplyOwnerToken(Owner("ktsu-dev"), "ghp_typed"); + + Assert.Contains("secret store", message, StringComparison.Ordinal); + } + + /// + /// Migrating nothing says nothing; migrating something says how much. + /// + [TestMethod] + public void MigrationIsDescribedOnlyWhenSomethingMoved() + { + Assert.AreEqual(string.Empty, ProjectDirector.DescribeTokenMigration(0)); + Assert.Contains("2", ProjectDirector.DescribeTokenMigration(2), StringComparison.Ordinal); + Assert.Contains("secret store", ProjectDirector.DescribeTokenMigration(2), StringComparison.Ordinal); + } + + /// + /// Owners list in a stable order, so the owner panels and the token menu agree with each other + /// and with the previous run. The registry they come from is a set, which does not promise one. + /// + [TestMethod] + public void OwnersListInAStableOrder() + { + HashSet owners = [Owner("ktsu-io"), Owner("acme"), Owner("ktsu-dev")]; + + string ordered = string.Join(",", ProjectDirector.OwnersInDisplayOrder(owners)); + + Assert.AreEqual("acme,ktsu-dev,ktsu-io", ordered); + } + + /// + /// A user upgrading from a version that kept the token in the settings file is authenticated on + /// that same launch, not the next one. The token only exists in the secret store once migration + /// has put it there, so resolving credentials before migrating would start the session + /// unauthenticated — this pins the order. + /// + [TestMethod] + public void StartupMigratesBeforeResolvingCredentials() + { + using ProjectDirectorOptions options = new() + { + GitHubLogin = GitHubLogin.Create("someone"), + LegacyGitHubToken = Token("ghp_from_settings_file"), + }; + + ProjectDirector.TokenStartup startup = ProjectDirector.PrepareTokens(options); + + Assert.AreEqual(1, startup.Migrated); + Assert.IsNotNull(startup.Credentials); + Assert.AreEqual("someone", startup.Credentials.Login); + Assert.AreEqual("ghp_from_settings_file", startup.Credentials.Password); + Assert.Contains("secret store", startup.Log, StringComparison.Ordinal); + } + + /// + /// A clean start has nothing to migrate, nothing to say, and no credentials to offer. + /// + [TestMethod] + public void StartupWithNoTokensIsSilentAndUnauthenticated() + { + using ProjectDirectorOptions options = new(); + + ProjectDirector.TokenStartup startup = ProjectDirector.PrepareTokens(options); + + Assert.AreEqual(0, startup.Migrated); + Assert.IsNull(startup.Credentials); + Assert.AreEqual(string.Empty, startup.Log); + } +} diff --git a/ProjectDirector/ProjectDirector.cs b/ProjectDirector/ProjectDirector.cs index 405e120..2702c42 100644 --- a/ProjectDirector/ProjectDirector.cs +++ b/ProjectDirector/ProjectDirector.cs @@ -43,12 +43,19 @@ internal sealed class ProjectDirector private ConcurrentQueue LogQueue { get; } = new(); private ImGuiPopups.InputString PopupSetDevDirectory { get; } = new(); private ImGuiPopups.InputString PopupAddNewGitHubOwner { get; } = new(); + private ImGuiPopups.InputString PopupSetGitHubOwnerToken { get; } = new(); private ImGuiPopups.Prompt PopupConfirmPull { get; } = new(); private ImGuiPopups.InputString PopupCommitMessage { get; } = new(); private Collection BrowserContentsBase { get; set; } = []; private Collection BrowserContentsCompare { get; set; } = []; private PopupPropagateFile PopupPropagateFile { get; } = new(); + /// + /// The owner whose token popup should open on the next tick. Opening a popup from inside + /// BeginMenu does not work, so the menu records the owner and the tick opens it. + /// + private GitHubOwnerName? OwnerPendingTokenPopup { get; set; } + // private ChatClient ChatClient { get; init; } private static void Main(string[] _) @@ -97,11 +104,20 @@ public ProjectDirector() GitHubClient = new(new ProductHeaderValue("ktsu.ProjectDirector")); - if (!string.IsNullOrEmpty(Options.GitHubLogin) && !string.IsNullOrEmpty(Options.GitHubToken)) + TokenStartup startup = PrepareTokens(Options); + if (startup.Migrated > 0) { - GitHubClient.Credentials = new(Options.GitHubLogin, Options.GitHubToken); + QueueSaveOptions(); } + if (startup.Credentials is not null) + { + GitHubClient.Credentials = startup.Credentials; + } + + QueueLogIfAny(startup.Log); + DrainSecretStoreReport(); + RefreshPage(); } @@ -268,6 +284,108 @@ private void RestoreDividerStates() private void QueueSaveOptions() => SaveOptionsQueuedTime = DateTime.UtcNow; + /// + /// Writes the secret store's complaint to the log, if it has one. + /// records the message rather than logging it, because the log is an instance member here. + /// + private void DrainSecretStoreReport() => QueueLogIfAny(TokenStorage.DrainUnavailableReport()); + + private void QueueLogIfAny(string message) + { + if (message.Length > 0) + { + QueueLog(message); + } + } + + /// + /// Picks the credentials for a request against . + /// + /// + /// The owner's own token when it has one, otherwise the account-level login and token, or + /// when neither is usable — which leaves the client unauthenticated + /// rather than authenticating with a blank secret. + /// + /// + /// The owner token shadowing the account token is the rule that makes a private repository in + /// another organization reachable, so it is here as a plain method rather than inline in the + /// scan loop. + /// + internal static Credentials? ResolveGitHubCredentials( + GitHubOwnerName owner, + GitHubToken ownerToken, + GitHubLogin login, + GitHubToken accountToken) + { + if (!string.IsNullOrEmpty(ownerToken)) + { + return new Credentials(owner, ownerToken); + } + + return !string.IsNullOrEmpty(login) && !string.IsNullOrEmpty(accountToken) + ? new Credentials(login, accountToken) + : null; + } + + /// + /// Stores the token a user typed for . + /// + /// + /// The message to log when the secret store refused the token, or empty when it took it. A + /// refusal has to reach the user: the popup closes either way, so silence would look like + /// success. + /// + internal static string ApplyOwnerToken(GitHubOwnerName owner, string typed) => + TokenStorage.WriteOwnerToken(owner, GitHubToken.Create(typed)) + ? string.Empty + : TokenStorage.DrainUnavailableReport(); + + /// + /// What the startup token pass decided: how many tokens moved out of the settings file, the + /// credentials to start with, and the line to log. + /// + internal sealed record TokenStartup(int Migrated, Credentials? Credentials, string Log); + + /// + /// Migrates any tokens left in the settings file, then works out the credentials to start with. + /// + /// + /// The order is the point. Migration has to run before the account token is read, because for a + /// user upgrading from a version that kept the token in the settings file, the token only exists + /// in the secret store once migration has put it there. Resolving first would start the session + /// unauthenticated and only pick the credentials up on the next launch. + /// + internal static TokenStartup PrepareTokens(ProjectDirectorOptions options) + { + Ensure.NotNull(options); + + int migrated = TokenStorage.MigrateLegacyTokens(options); + Credentials? credentials = ResolveGitHubCredentials(new(), new(), options.GitHubLogin, options.GitHubToken); + return new(migrated, credentials, DescribeTokenMigration(migrated)); + } + + /// + /// The configured owners in a stable display order. + /// + /// + /// The owner registry used to be a dictionary, whose enumeration order is not guaranteed, so the + /// owner panels and the token menu could disagree between runs. Ordering them once, here, is what + /// keeps both lists the same and predictable. + /// + internal static IEnumerable OwnersInDisplayOrder(IEnumerable owners) + { + Ensure.NotNull(owners); + return owners.OrderBy(owner => owner.ToString(), StringComparer.Ordinal); + } + + /// + /// The line to log after a startup migration, or empty when nothing moved. + /// + internal static string DescribeTokenMigration(int migratedTokens) => + migratedTokens > 0 + ? $"Moved {migratedTokens} GitHub token(s) out of the settings file into the OS secret store" + : string.Empty; + private void SaveOptionsIfRequired() { //debounce the save requests and avoid saving multiple times per frame or multiple frames in a row @@ -552,8 +670,20 @@ private void Tick(float dt) { DividerContainerCols.Tick(dt); + if (OwnerPendingTokenPopup is not null) + { + GitHubOwnerName owner = OwnerPendingTokenPopup; + OwnerPendingTokenPopup = null; + PopupSetGitHubOwnerToken.Open( + $"Set Token for {owner}", + "Personal Access Token", + string.Empty, + result => QueueLogIfAny(ApplyOwnerToken(owner, result))); + } + _ = PopupSetDevDirectory.ShowIfOpen(); _ = PopupAddNewGitHubOwner.ShowIfOpen(); + _ = PopupSetGitHubOwnerToken.ShowIfOpen(); _ = PopupConfirmPull.ShowIfOpen(); _ = PopupCommitMessage.ShowIfOpen(); @@ -777,12 +907,25 @@ private void ShowMenu() if (!string.IsNullOrEmpty(result)) { GitHubOwnerName newName = GitHubOwnerName.Create(result); - _ = Options.GitHubOwners.TryAdd(newName, GitHubToken.Create(string.Empty)); + _ = Options.GitHubOwners.Add(newName); SyncGitHubOwnerInfo(newName); } }); } + if (Options.GitHubOwners.Count > 0 && ImGui.BeginMenu("Set GitHub Owner Token")) + { + foreach (GitHubOwnerName owner in OwnersInDisplayOrder(Options.GitHubOwners)) + { + if (ImGui.MenuItem(owner)) + { + OwnerPendingTokenPopup = owner; + } + } + + ImGui.EndMenu(); + } + ImGui.Separator(); if (ImGui.MenuItem("Exit")) @@ -811,7 +954,7 @@ private void ShowMenu() private void ShowOwners() { - foreach ((GitHubOwnerName owner, GitHubToken pat) in Options.GitHubOwners) + foreach (GitHubOwnerName owner in OwnersInDisplayOrder(Options.GitHubOwners)) { ShowCollapsiblePanel(owner, () => ShowRepos(owner)); } @@ -1047,7 +1190,7 @@ private void ScanDevDirectoryForOwnersAndRepos() Options.Repos[repoFullName] = gitHubRepo; gitHubRepo.OwnerName = ownerName; gitHubRepo.RepoName = repoName; - _ = Options.GitHubOwners.TryAdd(gitHubRepo.OwnerName, GitHubToken.Create(string.Empty)); + _ = Options.GitHubOwners.Add(gitHubRepo.OwnerName); } } } @@ -1114,14 +1257,18 @@ internal static void ApplyCredentials(GitHubClient client, GitHubOwnerName owner private void ScanRemoteAccountsForRepos() { - Dictionary knownOwners = Options.GitHubOwners; - foreach ((GitHubOwnerName owner, GitHubToken pat) in knownOwners) + foreach (GitHubOwnerName owner in Options.GitHubOwners.ToArray()) { - ApplyCredentials(GitHubClient, owner, pat, Options.GitHubLogin, Options.GitHubToken); + // The owner's token now comes from the secret store rather than the options file, but the + // assignment still has to happen for every owner — including one with no token of its own — + // or the previous owner's identity carries into this scan. + ApplyCredentials(GitHubClient, owner, TokenStorage.ReadOwnerToken(owner), Options.GitHubLogin, Options.GitHubToken); SyncGitHubOwnerInfo(owner); } + DrainSecretStoreReport(); + UpdateClonedStatus(); } diff --git a/ProjectDirector/ProjectDirector.csproj b/ProjectDirector/ProjectDirector.csproj index 6d2ec1b..ed7d5ea 100644 --- a/ProjectDirector/ProjectDirector.csproj +++ b/ProjectDirector/ProjectDirector.csproj @@ -14,6 +14,7 @@ + diff --git a/ProjectDirector/ProjectDirectorOptions.cs b/ProjectDirector/ProjectDirectorOptions.cs index 7a11160..6bdcf34 100644 --- a/ProjectDirector/ProjectDirectorOptions.cs +++ b/ProjectDirector/ProjectDirectorOptions.cs @@ -34,14 +34,58 @@ private static AbsoluteDirectoryPath DefaultDevDirectory() => AbsoluteDirectoryPath.Create(OperatingSystem.IsWindows() ? @"C:\dev" : Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), "dev")); + public ImGuiAppWindowState WindowState { get; set; } = new(); public GitHubLogin GitHubLogin { get; set; } = new(); - public GitHubToken GitHubToken { get; set; } = new(); + + /// + /// The account-level token as earlier versions persisted it: plaintext, in the settings file. + /// + /// + /// Retained under its original JSON name only so + /// can move it into the OS secret store and blank it here. + /// + [JsonInclude] + [JsonPropertyName("GitHubToken")] + internal GitHubToken LegacyGitHubToken { get; set; } = new(); + + /// + /// The account-level token that goes with , held in the OS secret store. + /// + [JsonIgnore] + public GitHubToken GitHubToken + { + get => TokenStorage.Read(TokenStorage.LoginPersona(GitHubLogin)); + set + { + Ensure.NotNull(value); + _ = TokenStorage.Write(TokenStorage.LoginPersona(GitHubLogin), value); + } + } + public OpenAIToken OpenAIToken { get; set; } = new(); public Dictionary PanelStates { get; init; } = []; public Dictionary> DividerStates { get; init; } = []; - public Dictionary GitHubOwners { get; init; } = []; + + /// + /// The owner-to-token map as earlier versions persisted it: plaintext, in the settings file. + /// + /// + /// Retained under its original JSON name only so + /// can move the tokens into the OS secret store, register the owner names in + /// , and empty this. + /// + [JsonInclude] + [JsonPropertyName("GitHubOwners")] + internal Dictionary LegacyGitHubOwners { get; init; } = []; + + /// + /// Which GitHub owners are configured. Their tokens live in the OS secret store, so this is a + /// plain set of names rather than a map to secrets. + /// + [JsonPropertyName("GitHubOwnerNames")] + public HashSet GitHubOwners { get; init; } = []; public Dictionary GitHubOwnerInfo { get; init; } = []; public Dictionary ClonedRepos { get; init; } = []; public FullyQualifiedGitHubRepoName BaseRepo { get; set; } = new(); diff --git a/ProjectDirector/TokenStorage.cs b/ProjectDirector/TokenStorage.cs new file mode 100644 index 0000000..1b99062 --- /dev/null +++ b/ProjectDirector/TokenStorage.cs @@ -0,0 +1,262 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector; + +using System.Security.Cryptography; +using System.Text; + +using ktsu.CredentialCache; +using ktsu.CredentialCache.Storage; + +using Semantics.Strings; + +using CredentialCache = ktsu.CredentialCache.CredentialCache; + +/// +/// Holds GitHub personal access tokens in the operating system's secret store — Windows Credential +/// Manager, macOS Keychain, or libsecret (Secret Service) on Linux — rather than in +/// 's settings file. +/// +/// +/// serializes the whole options object to an unencrypted file +/// alongside window state and UI preferences. A PAT is not something to keep there. +/// +internal static class TokenStorage +{ + /// + /// Scopes ProjectDirector's entries within the OS secret store so they cannot collide with + /// another ktsu tool's credentials on a shared host. + /// + internal const string CredentialServiceName = "ktsu.ProjectDirector"; + + /// + /// Prefixed to every persona seed. Versioned because changing the derivation would orphan every + /// token already in the store, so a future change has to be a deliberate, visible one. + /// + private const string PersonaNamespace = "ktsu.ProjectDirector/v1/"; + + private const string UnavailableMessage = + "No usable OS secret store was found, so GitHub tokens cannot be read or saved. On Linux this " + + "usually means no Secret Service provider (libsecret with GNOME Keyring or KWallet) is installed " + + "and unlocked. ProjectDirector will not fall back to writing tokens to a plain file."; + + private static readonly Lazy LazyDefaultCache = + new(() => new CredentialCache(CredentialStoreFactory.CreateDefault(CredentialServiceName))); + + private static int _unavailableReported; + + /// + /// The cache substituted by , or for the default. + /// + private static CredentialCache? InjectedCache { get; set; } + + /// + /// Gets the cache backing this storage. + /// + /// + /// Built directly rather than taken from so the store + /// carries ProjectDirector's own service name; the singleton can only ever use the library + /// default. + /// + internal static CredentialCache Cache => InjectedCache ?? LazyDefaultCache.Value; + + /// + /// Reports whether the last read or write found the secret store unusable. + /// + internal static bool StoreUnavailable => Volatile.Read(ref _unavailableReported) != 0; + + /// + /// Substitutes the backing cache. Test seam; pass to restore the default. + /// + internal static void UseCache(CredentialCache? cache) + { + InjectedCache = cache; + _ = Interlocked.Exchange(ref _unavailableReported, 0); + } + + /// + /// Derives the persona holding the account-level token that goes with + /// . + /// + internal static PersonaGUID LoginPersona(GitHubLogin login) => DerivePersona($"github/login/{login}"); + + /// + /// Derives the persona holding an owner's token. + /// + internal static PersonaGUID OwnerPersona(GitHubOwnerName owner) => DerivePersona($"github/owner/{owner}"); + + /// + /// Reads the token stored under , or an empty token when there is none. + /// + internal static GitHubToken Read(PersonaGUID persona) + { + try + { + return Cache.TryGet(persona, out Credential? credential) && credential is CredentialWithToken token + ? GitHubToken.Create(token.Token.ToString()) + : new(); + } + catch (Exception ex) when (IsUnavailable(ex)) + { + ReportUnavailable(ex); + return new(); + } + } + + /// + /// Stores under , removing the entry when the + /// token is empty. + /// + /// when the secret store refused the write. + internal static bool Write(PersonaGUID persona, GitHubToken token) + { + try + { + if (token.IsEmpty()) + { + _ = Cache.Remove(persona); + return true; + } + + Cache.AddOrReplace(persona, new CredentialWithToken + { + Token = SemanticString.Create(token.ToString()), + }); + return true; + } + catch (Exception ex) when (IsUnavailable(ex)) + { + ReportUnavailable(ex); + return false; + } + } + + /// + /// Reads an owner's token. + /// + internal static GitHubToken ReadOwnerToken(GitHubOwnerName owner) => Read(OwnerPersona(owner)); + + /// + /// Stores an owner's token. + /// + internal static bool WriteOwnerToken(GitHubOwnerName owner, GitHubToken token) => + Write(OwnerPersona(owner), token); + + /// + /// Moves the tokens earlier versions kept in the settings file into the secret store, registers + /// the owners they were keyed by, and blanks them where they were. + /// + /// How many tokens were moved. + /// + /// The legacy owner map did double duty: it was both the registry of configured owners and the + /// place their tokens lived. The names move to + /// — which is not credential-shaped, so it stays in the settings file — while the tokens go to + /// the secret store. + /// + internal static int MigrateLegacyTokens(ProjectDirectorOptions options) + { + Ensure.NotNull(options); + + int migrated = 0; + + if (MigrateToken( + LoginPersona(options.GitHubLogin), + options.LegacyGitHubToken, + () => options.LegacyGitHubToken = new())) + { + migrated++; + } + + foreach ((GitHubOwnerName owner, GitHubToken legacy) in options.LegacyGitHubOwners.ToArray()) + { + _ = options.GitHubOwners.Add(owner); + + if (MigrateToken(OwnerPersona(owner), legacy, () => options.LegacyGitHubOwners[owner] = new())) + { + migrated++; + } + } + + // Owners whose token moved, or who never had one, no longer need an entry here at all. + foreach ((GitHubOwnerName owner, GitHubToken remaining) in options.LegacyGitHubOwners.ToArray()) + { + if (remaining.IsEmpty()) + { + _ = options.LegacyGitHubOwners.Remove(owner); + } + } + + return migrated; + } + + /// + /// Moves one token, leaving the old copy in place if the secret store will not take it. + /// + /// + /// A token already in the store wins over a legacy one, so a stale copy in the settings file + /// cannot overwrite a current credential — but the stale copy is still cleared, because ceasing + /// to write a secret does not remove the one already on disk. + /// + private static bool MigrateToken(PersonaGUID persona, GitHubToken legacy, Action clearLegacy) + { + if (legacy.IsEmpty()) + { + return false; + } + + if (Read(persona).IsEmpty() && !Write(persona, legacy)) + { + return false; + } + + clearLegacy(); + return true; + } + + private static PersonaGUID DerivePersona(string seed) + { + byte[] hash = SHA256.HashData(Encoding.UTF8.GetBytes(PersonaNamespace + seed)); + return SemanticString.Create(new Guid(hash.AsSpan(0, 16)).ToString()); + } + + /// + /// Recognises a machine with no usable secret store: the factory refusing the platform, the + /// native library failing to resolve, or the store itself reporting a failure. + /// + private static bool IsUnavailable(Exception exception) => + exception is PlatformNotSupportedException + or DllNotFoundException + or EntryPointNotFoundException + or CredentialStoreException; + + /// + /// Records the unavailable store once per process. Tokens are read on paths that run per owner + /// on every scan, so reporting each failure would bury the log. + /// + private static void ReportUnavailable(Exception exception) + { + if (Interlocked.Exchange(ref _unavailableReported, 1) == 0) + { + UnavailableReport = $"{UnavailableMessage} ({exception.Message})"; + } + } + + /// + /// The message describing why the secret store is unusable, or empty while it is fine. + /// + /// + /// Held rather than logged directly because the log in this application is an instance member of + /// ; the caller drains this into it. + /// + internal static string UnavailableReport { get; private set; } = string.Empty; + + /// + /// Returns the pending unavailable-store message and clears it, so it is reported once. + /// + internal static string DrainUnavailableReport() + { + string report = UnavailableReport; + UnavailableReport = string.Empty; + return report; + } +} diff --git a/README.md b/README.md index b457c22..98d72c2 100644 --- a/README.md +++ b/README.md @@ -57,9 +57,14 @@ dotnet run dotnet publish --configuration Release --output ./staging ``` -On first run, set the development directory that ProjectDirector should scan and supply GitHub -credentials if you want remote repositories listed. Both are persisted to the application data -folder, so subsequent runs start where you left off. +On first run, set the development directory that ProjectDirector should scan and add the GitHub +owners you want listed (**File > Add New GitHub Owner**). Give an owner a personal access token with +**File > Set GitHub Owner Token** if its repositories are private or you want a higher rate limit. + +The development directory, the owner list and the repository cache are persisted to the application +data folder. Tokens are not: they go to the operating system's secret store — Windows Credential +Manager, macOS Keychain, or libsecret (Secret Service) on Linux. A token saved by an earlier version +is moved there on the next start and emptied where it was, so nothing needs re-entering. ### Typical Workflow @@ -75,7 +80,8 @@ folder, so subsequent runs start where you left off. | Component | Responsibility | | --- | --- | | `ProjectDirector` | Main application class: ImGui loop, three-panel layout, and the fetch/pull/diff/propagate operations. | -| `ProjectDirectorOptions` | Application state persisted as JSON — dev directory, credentials, repository cache, and UI state. | +| `ProjectDirectorOptions` | Application state persisted as JSON — dev directory, configured GitHub owners, repository cache, and UI state. Deliberately holds no tokens. | +| `TokenStorage` | GitHub tokens in the OS secret store, keyed by a persona derived from the login or owner name, plus the one-time migration off the old plaintext fields. | | `GitRepository` | Abstract repository model with polymorphic JSON serialization. | | `GitHubRepository` / `AzureDevOpsRepository` | Provider-specific repository implementations. | | `PopupPropagateFile` | Modal that drives the file propagation flow. |