diff --git a/ProjectDirector.Test/ScanCredentialTests.cs b/ProjectDirector.Test/ScanCredentialTests.cs new file mode 100644 index 0000000..5d9a1cd --- /dev/null +++ b/ProjectDirector.Test/ScanCredentialTests.cs @@ -0,0 +1,126 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector.Test; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Octokit; + +/// +/// Tests the rule that decides which credentials one owner is scanned with. +/// +/// +/// is one mutable property on a client shared by +/// every owner in the scan, so the rule that sets it has to answer for every owner rather than only +/// for the ones that have credentials. It used to be a condition wrapped around the assignment, +/// which meant an owner with no credentials left the previous owner's in place and was scanned as +/// them. exists separately so that rule can be +/// driven without a live ImGui context or a GitHub account, the way +/// drives the pull rule. +/// +/// Getting it wrong is quiet: the owner's own private repositories go missing, and anything that +/// genuinely needed its auth answers with an ApiException the caller swallows, so the user is given +/// no reason for the gap. +/// +[TestClass] +public sealed class ScanCredentialTests +{ + private static GitHubOwnerName Owner(string value) => GitHubOwnerName.Create(value); + private static GitHubToken Token(string value) => GitHubToken.Create(value); + private static GitHubLogin Login(string value) => GitHubLogin.Create(value); + + [TestMethod] + public void AnOwnerWithItsOwnTokenIsScannedAsItself() + { + Credentials chosen = ProjectDirector.ChooseCredentials(Owner("alpha"), Token("alpha-pat"), Login("global-login"), Token("global-token")); + + Assert.AreEqual(AuthenticationType.Basic, chosen.AuthenticationType); + Assert.AreEqual("alpha", chosen.Login, "An owner's own token should win over the global login."); + Assert.AreEqual("alpha-pat", chosen.Password); + } + + [TestMethod] + public void AnOwnerWithoutATokenFallsBackToTheGlobalLogin() + { + Credentials chosen = ProjectDirector.ChooseCredentials(Owner("beta"), Token(string.Empty), Login("global-login"), Token("global-token")); + + Assert.AreEqual(AuthenticationType.Basic, chosen.AuthenticationType); + Assert.AreEqual("global-login", chosen.Login); + Assert.AreEqual("global-token", chosen.Password); + } + + /// + /// The regression this file exists for. + /// + /// + /// Owner A has a PAT and owner B has nothing. B must come back anonymous, because the caller + /// assigns whatever this returns to the shared client, and anything short of an answer for B + /// leaves A's identity in place. + /// + [TestMethod] + public void AnOwnerWithNoCredentialsAnywhereIsScannedAnonymously() + { + Credentials forOwnerWithPat = ProjectDirector.ChooseCredentials(Owner("alpha"), Token("alpha-pat"), Login(string.Empty), Token(string.Empty)); + Credentials forOwnerWithout = ProjectDirector.ChooseCredentials(Owner("beta"), Token(string.Empty), Login(string.Empty), Token(string.Empty)); + + Assert.AreEqual("alpha", forOwnerWithPat.Login, "The first owner should still be scanned as itself."); + + Assert.AreEqual(AuthenticationType.Anonymous, forOwnerWithout.AuthenticationType, + "An owner with no credentials of its own and no global login must be scanned anonymously, " + + "not as whichever owner was scanned before it."); + Assert.AreNotEqual("alpha", forOwnerWithout.Login, "The previous owner's login must not carry over."); + } + + /// + /// The defect in the shape it actually had: one client, reused across owners. + /// + /// + /// pins the rule, but the rule + /// only helps if the caller applies it for every owner. Scanning A and then B against a single + /// client is what the loop does, so this is what catches a caller that skips the assignment. + /// + [TestMethod] + public void ScanningASecondOwnerDoesNotInheritTheFirstOwnersCredentials() + { + GitHubClient client = new(new ProductHeaderValue("ktsu.ProjectDirector.Test")); + + ProjectDirector.ApplyCredentials(client, Owner("alpha"), Token("alpha-pat"), Login(string.Empty), Token(string.Empty)); + + Assert.AreEqual("alpha", client.Credentials.Login, "The first owner should be scanned as itself."); + + ProjectDirector.ApplyCredentials(client, Owner("beta"), Token(string.Empty), Login(string.Empty), Token(string.Empty)); + + Assert.AreEqual(AuthenticationType.Anonymous, client.Credentials.AuthenticationType, + "The client must not still be authenticated as the previous owner when scanning one with no credentials."); + Assert.AreNotEqual("alpha", client.Credentials.Login); + } + + [TestMethod] + public void ApplyingAGlobalLoginPointsTheClientAtIt() + { + GitHubClient client = new(new ProductHeaderValue("ktsu.ProjectDirector.Test")); + + ProjectDirector.ApplyCredentials(client, Owner("beta"), Token(string.Empty), Login("global-login"), Token("global-token")); + + Assert.AreEqual("global-login", client.Credentials.Login); + Assert.AreEqual("global-token", client.Credentials.Password); + } + + [TestMethod] + public void AGlobalLoginMissingItsTokenIsNotUsed() + { + // Octokit rejects an empty password, so a half-configured global login has to be treated as + // no login rather than passed through. + Credentials chosen = ProjectDirector.ChooseCredentials(Owner("beta"), Token(string.Empty), Login("global-login"), Token(string.Empty)); + + Assert.AreEqual(AuthenticationType.Anonymous, chosen.AuthenticationType); + } + + [TestMethod] + public void AGlobalTokenMissingItsLoginIsNotUsed() + { + Credentials chosen = ProjectDirector.ChooseCredentials(Owner("beta"), Token(string.Empty), Login(string.Empty), Token("global-token")); + + Assert.AreEqual(AuthenticationType.Anonymous, chosen.AuthenticationType); + } +} diff --git a/ProjectDirector/ProjectDirector.cs b/ProjectDirector/ProjectDirector.cs index 79d2af9..d46138c 100644 --- a/ProjectDirector/ProjectDirector.cs +++ b/ProjectDirector/ProjectDirector.cs @@ -881,15 +881,64 @@ private void ScanDevDirectoryForOwnersAndRepos() UpdateClonedStatus(); } + /// + /// Chooses the credentials one owner is scanned with. + /// + /// The owner about to be scanned. + /// That owner's own personal access token, empty if it has none. + /// The globally configured login, empty if there is none. + /// The globally configured token, empty if there is none. + /// + /// The owner's own token where it has one, otherwise the global login where there is one, + /// otherwise . + /// + /// + /// The answer has to be total. is one mutable + /// property on a client shared by every owner in the scan, so an owner that leaves it alone is + /// not scanned anonymously -- it is scanned as whoever was set last. A PAT configured for one + /// owner therefore carried into the next owner that had none, which misses that owner's own + /// private repositories and answers anything needing its auth with an + /// that the caller swallows, leaving the repositories missing with + /// no indication why. + /// + internal static Credentials ChooseCredentials(GitHubOwnerName owner, GitHubToken pat, GitHubLogin login, GitHubToken token) + { + if (!string.IsNullOrEmpty(pat)) + { + return new Credentials(owner, pat); + } + + return !string.IsNullOrEmpty(login) && !string.IsNullOrEmpty(token) + ? new Credentials(login, token) + : Credentials.Anonymous; + } + + /// + /// Points the shared client at the credentials one owner is scanned with. + /// + /// The client every owner in the scan shares. + /// The owner about to be scanned. + /// That owner's own personal access token, empty if it has none. + /// The globally configured login, empty if there is none. + /// The globally configured token, empty if there is none. + /// + /// Assigned for every owner, including one with no credentials of its own, so that the previous + /// owner's identity cannot carry into this one. That is the whole of the rule, and it is here + /// rather than inline in the loop so a test can watch one client across two owners, which is the + /// shape the defect actually had. + /// + internal static void ApplyCredentials(GitHubClient client, GitHubOwnerName owner, GitHubToken pat, GitHubLogin login, GitHubToken token) + { + Ensure.NotNull(client); + client.Credentials = ChooseCredentials(owner, pat, login, token); + } + private void ScanRemoteAccountsForRepos() { Dictionary knownOwners = Options.GitHubOwners; foreach ((GitHubOwnerName owner, GitHubToken pat) in knownOwners) { - if (!string.IsNullOrEmpty(pat) || (!string.IsNullOrEmpty(Options.GitHubLogin) && !string.IsNullOrEmpty(Options.GitHubToken))) - { - GitHubClient.Credentials = !string.IsNullOrEmpty(pat) ? new(owner, pat) : new(Options.GitHubLogin, Options.GitHubToken); - } + ApplyCredentials(GitHubClient, owner, pat, Options.GitHubLogin, Options.GitHubToken); SyncGitHubOwnerInfo(owner); }