From d1119d8b6c984ab7780295e752d79c49d6a42728 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Mon, 21 Sep 2026 09:58:31 +1000 Subject: [PATCH 1/2] feat: add bearer credentials and a credential injection seam Azure DevOps accepts an Entra ID access token only as `Authorization: Bearer`. Both hosting providers previously had no way to send one: HostingCredentialKind carried Token and UsernamePassword only, and AzureDevOpsProvider applies Token as Basic with an empty username, which is the personal access token scheme. An Entra token in that slot does not authenticate. Credentials also resolved solely from ktsu.CredentialCache via PersonaGUID. That suits a long-lived secret such as a personal access token and suits nothing about a short-lived one: an Entra access token is minted per session and expires within the hour, so writing it to the host's keyring would persist a secret that is stale before it is read again. Two additions: - HostingCredentialKind.BearerToken, with HostingCredential.FromBearerToken. AzureDevOpsProvider sends it as a Bearer header; GitHubProvider maps it to Octokit's AuthenticationType.Bearer, which is separately what a GitHub App installation token requires. FromToken keeps its existing per-host behaviour, and its doc comment is corrected: it was described as a bearer token while being applied as neither host's bearer scheme. - GitProvider.CredentialSource, a Func consulted on every resolution rather than cached, so a caller can return a freshly refreshed token each time. It takes precedence over the credential cache when set. Returning HostingCredential.None proceeds unauthenticated; returning null throws from ResolveCredential with a message naming the mistake, while reaching IsAuthenticated as false rather than throwing from a property getter. HostingCredential and HostingCredentialKind become public, since they are now the vocabulary a caller supplies a credential in. They were internal while the credential cache was the only way in. Verified: 603/603 tests pass, including 8 new ones. The Azure DevOps bearer test was mutation-checked by reverting the mapping to Basic, which fails it. Pre-existing and unrelated: `dotnet pack` reports CP0014 net9.0/net10.0 ApiCompat errors on Polyfill's shims, reproduced on main. Co-Authored-By: Claude Opus 5 (1M context) --- .../Hosting/AzureDevOpsProviderTests.cs | 22 ++++ .../Hosting/GitHubProviderTests.cs | 20 +++ .../Hosting/GitProviderTests.cs | 95 ++++++++++++++ GitIntegration/GitHubProvider.cs | 4 + GitIntegration/GitProvider.cs | 118 ++++++++++++++++-- GitIntegration/Hosting/AzureDevOpsProvider.cs | 6 + README.md | 43 ++++++- 7 files changed, 297 insertions(+), 11 deletions(-) diff --git a/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs b/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs index 97b32db..ebc0882 100644 --- a/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs +++ b/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs @@ -340,6 +340,28 @@ public async Task SendsBasicAuthWithAnEmptyUsernameForATokenCredentialAsync() Assert.AreEqual(":pat-abc123", decoded); } + [TestMethod] + public async Task SendsBearerAuthForABearerTokenCredentialAsync() + { + // The whole point of the BearerToken kind. Azure DevOps accepts an Entra ID access token only + // in a Bearer header; putting one in the Basic/PAT slot fails to authenticate, so a provider + // that collapsed the two kinds onto one header would silently break Entra callers. + using FakeHttpMessageHandler handler = new FakeHttpMessageHandler() + .Respond(HttpStatusCode.OK, Fixture("azure-devops-repositories.json"), ("Content-Type", "application/json")); + AzureDevOpsProvider provider = new() + { + Owner = "contoso".As(), + Handler = handler, + CredentialSource = () => HostingCredential.FromBearerToken("eyJ0eXAiOiJKV1Qi"), + }; + + _ = await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + // Asserted verbatim and un-decoded: a Bearer header carries the token as-is, and any + // base64 round-trip here would mean it had been through the Basic path by mistake. + Assert.AreEqual("Bearer eyJ0eXAiOiJKV1Qi", handler.Requests[0].Headers["Authorization"]); + } + [TestMethod] public async Task SendsNoAuthorizationHeaderWhenUnauthenticatedAsync() { diff --git a/GitIntegration.Test/Hosting/GitHubProviderTests.cs b/GitIntegration.Test/Hosting/GitHubProviderTests.cs index c1d1a3b..e27d013 100644 --- a/GitIntegration.Test/Hosting/GitHubProviderTests.cs +++ b/GitIntegration.Test/Hosting/GitHubProviderTests.cs @@ -102,6 +102,26 @@ public async Task AppliesATokenCredentialToTheRequestAsync() Assert.AreEqual("Token ghp_abc123", handler.Requests[0].Headers["Authorization"]); } + [TestMethod] + public async Task SendsBearerAuthForABearerTokenCredentialAsync() + { + // GitHub distinguishes the two as Octokit AuthenticationType values, and sends a different + // scheme for each: "Token" for a PAT, "Bearer" for a JWT such as a GitHub App token. Proves + // the BearerToken kind reaches the Octokit client as Bearer rather than collapsing to Token. + using FakeHttpMessageHandler handler = new FakeHttpMessageHandler() + .Respond(HttpStatusCode.OK, Fixture("github-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() + { + Owner = "contoso".As(), + Handler = handler, + CredentialSource = () => HostingCredential.FromBearerToken("eyJ0eXAiOiJKV1Qi"), + }; + + _ = await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual("Bearer eyJ0eXAiOiJKV1Qi", handler.Requests[0].Headers["Authorization"]); + } + [TestMethod] public async Task EnumeratesRepositoriesForTheOwnerAsync() { diff --git a/GitIntegration.Test/Hosting/GitProviderTests.cs b/GitIntegration.Test/Hosting/GitProviderTests.cs index ecfc603..cb03e83 100644 --- a/GitIntegration.Test/Hosting/GitProviderTests.cs +++ b/GitIntegration.Test/Hosting/GitProviderTests.cs @@ -61,6 +61,101 @@ public void UsesAUsernamePasswordCredential() Assert.AreEqual("hunter2", resolved.Password); } + [TestMethod] + public void UsesABearerTokenFromTheCredentialSource() + { + // The case the credential cache cannot serve: an Entra ID access token is minted per session + // by an identity library and expires, so it never belongs in the OS keyring PersonaGUID reads. + TestProvider provider = new() + { + Owner = "octocat".As(), + CredentialSource = () => HostingCredential.FromBearerToken("eyJ0eXAiOiJKV1Qi"), + }; + + HostingCredential resolved = provider.CallResolveCredential(); + + Assert.AreEqual(HostingCredentialKind.BearerToken, resolved.Kind); + Assert.AreEqual("eyJ0eXAiOiJKV1Qi", resolved.Token); + } + + [TestMethod] + public void PrefersTheCredentialSourceOverTheCredentialCache() + { + // Both are configured. The explicitly injected one wins, so a caller that supplies a source + // never has to also clear whatever the keyring happens to hold for its persona. + PersonaGUID persona = SeedCredential(new CredentialWithToken { Token = "pat-from-cache".As() }); + TestProvider provider = new() + { + Owner = "octocat".As(), + PersonaGUID = persona, + CredentialSource = () => HostingCredential.FromBearerToken("token-from-source"), + }; + + HostingCredential resolved = provider.CallResolveCredential(); + + Assert.AreEqual(HostingCredentialKind.BearerToken, resolved.Kind); + Assert.AreEqual("token-from-source", resolved.Token); + } + + [TestMethod] + public void ConsultsTheCredentialSourceOnEveryResolution() + { + // An access token expires. A caller returning a fresh one per call has to actually be asked + // each time, so the first answer must not be cached anywhere. + int calls = 0; + TestProvider provider = new() + { + Owner = "octocat".As(), + CredentialSource = () => HostingCredential.FromBearerToken($"token-{++calls}"), + }; + + Assert.AreEqual("token-1", provider.CallResolveCredential().Token); + Assert.AreEqual("token-2", provider.CallResolveCredential().Token); + Assert.AreEqual(2, calls); + } + + [TestMethod] + public void ProceedsUnauthenticatedWhenTheCredentialSourceSuppliesNone() + { + // HostingCredential.None is how a source says "proceed unauthenticated" — the same meaning a + // resolved CredentialWithNothing carries on the cache path. + TestProvider provider = new() + { + Owner = "octocat".As(), + CredentialSource = () => HostingCredential.None, + }; + + Assert.AreEqual(HostingCredentialKind.None, provider.CallResolveCredential().Kind); + Assert.IsFalse(provider.IsAuthenticated); + } + + [TestMethod] + public void ReportsAuthenticatedForACredentialSourceSupplyingAToken() + { + TestProvider provider = new() + { + Owner = "octocat".As(), + CredentialSource = () => HostingCredential.FromBearerToken("eyJ0eXAiOiJKV1Qi"), + }; + + Assert.IsTrue(provider.IsAuthenticated); + } + + [TestMethod] + public void ThrowsWhenTheCredentialSourceReturnsNull() + { + // Returning null is a caller bug, not a way to say "unauthenticated" — HostingCredential.None + // says that explicitly. Treating null as None would hide the bug, which is the same reason an + // unrecognised Credential subtype throws rather than proceeding. + TestProvider provider = new() + { + Owner = "octocat".As(), + CredentialSource = () => null!, + }; + + _ = Assert.ThrowsExactly(provider.CallResolveCredential); + } + [TestMethod] public void ProceedsUnauthenticatedWhenNoCredentialIsResolved() { diff --git a/GitIntegration/GitHubProvider.cs b/GitIntegration/GitHubProvider.cs index 53b7104..ade82c1 100644 --- a/GitIntegration/GitHubProvider.cs +++ b/GitIntegration/GitHubProvider.cs @@ -288,6 +288,10 @@ protected override void Dispose(bool disposing) private static Credentials ToOctokitCredentials(HostingCredential credential) => credential switch { { Kind: HostingCredentialKind.Token, Token: string token } => new Credentials(token), + // AuthenticationType.Bearer rather than the single-argument constructor above, which defaults + // to Oauth and sends "Token ". GitHub requires "Bearer " for a JWT such as a + // GitHub App installation token, and rejects it under the Token scheme. + { Kind: HostingCredentialKind.BearerToken, Token: string bearerToken } => new Credentials(bearerToken, AuthenticationType.Bearer), { Kind: HostingCredentialKind.UsernamePassword, Username: string username, Password: string password } => new Credentials(username, password), _ => Credentials.Anonymous, }; diff --git a/GitIntegration/GitProvider.cs b/GitIntegration/GitProvider.cs index 8bb187b..d87b6f4 100644 --- a/GitIntegration/GitProvider.cs +++ b/GitIntegration/GitProvider.cs @@ -49,6 +49,37 @@ public abstract class GitProvider : IGitHostingProvider /// A GUID identifying the authentication persona. public PersonaGUID PersonaGUID { get; init; } = CredentialCache.CreatePersonaGUID(); + /// + /// Gets or initializes a callback supplying this provider's credential directly, bypassing the + /// credential cache, or to resolve through as + /// usual. + /// + /// + /// + /// The seam for a credential the credential cache cannot hold. reads + /// the host's native keyring, which suits a long-lived secret such as a personal access token, + /// and suits nothing about a short-lived one: an Entra ID access token is minted per session by + /// an identity library, expires within the hour, and writing it to a keyring would persist a + /// secret that is stale before it is read again. + /// + /// + /// A callback rather than a stored , and consulted on every + /// resolution rather than cached, precisely so an expiring token can be refreshed: a caller + /// returns whatever its identity library hands it at the moment of the call. The cost is that + /// the callback runs inline on the request path, so it should return a cached-and-still-valid + /// token rather than block on a fresh network round trip each time — which is what + /// TokenCredential.GetToken and its equivalents already do. + /// + /// + /// Takes precedence over the credential cache when set, so a caller supplying one never has to + /// also clear whatever the keyring happens to hold for its persona. Returning + /// is how it says "proceed unauthenticated"; returning + /// is a caller bug and makes throw. + /// + /// + /// The callback, or to use the credential cache. + public Func? CredentialSource { get; init; } + /// /// /// Reports whether a request this provider issues would actually carry a credential, which is a @@ -236,9 +267,21 @@ public bool TryGetCredential(out Credential? credential) /// /// A credential was resolved whose runtime type is not one this method recognises. /// - internal HostingCredential ResolveCredential() => - ResolveRecognisedCredential(out Credential? credential) ?? throw new InvalidOperationException( - $"Provider '{Name}' resolved a credential of type '{credential!.GetType()}', which this library does not recognise."); + internal HostingCredential ResolveCredential() + { + HostingCredential? resolved = ResolveRecognisedCredential(out Credential? credential); + if (resolved is not null) + { + return resolved; + } + + // Two ways to get here, and they are different caller mistakes, so they get different + // messages. A null credential means CredentialSource returned null, because that path never + // assigns the out parameter; a non-null one means the cache held a subtype not in the table. + throw new InvalidOperationException(credential is null + ? $"Provider '{Name}' has a {nameof(CredentialSource)} that returned null. Return {nameof(HostingCredential)}.{nameof(HostingCredential.None)} to proceed unauthenticated." + : $"Provider '{Name}' resolved a credential of type '{credential.GetType()}', which this library does not recognise."); + } /// /// Resolves this provider's credential, reporting an unrecognised @@ -258,6 +301,20 @@ internal HostingCredential ResolveCredential() => /// The resolved credential, or for an unrecognised subtype. private HostingCredential? ResolveRecognisedCredential(out Credential? credential) { + credential = null; + + // Checked before the cache, not merged with it: a caller that supplied a source has said + // where its credential comes from, and falling back to the keyring when the source returns + // None would silently authenticate as somebody else. + if (CredentialSource is not null) + { + // A null return leaves `credential` null, which is what tells ResolveCredential to + // report this as a CredentialSource bug rather than an unrecognised cache subtype. It + // reaches IsAuthenticated as "no credential a request could carry", which is a property + // getter and so must not throw. + return CredentialSource(); + } + if (!TryGetCredential(out credential) || credential is null or CredentialWithNothing) { return HostingCredential.None; @@ -498,9 +555,14 @@ internal readonly record struct GitRepositoryAddress(string Value, bool IsHostRe /// A record with a discriminator rather than a type hierarchy: /// a provider applying this result switches on exactly once, so a closed set of /// fields on one type reads as clearly as a hierarchy would and avoids a second type family -/// alongside for what is, here, an internal implementation detail. +/// alongside . +/// +/// Public rather than internal, because is the vocabulary +/// a caller supplies a credential in. It was internal while the credential cache was the only way in +/// and this type was purely an implementation detail of that path. +/// /// -internal sealed record HostingCredential +public sealed record HostingCredential { /// The singleton result for "no credential to apply". public static readonly HostingCredential None = new() { Kind = HostingCredentialKind.None }; @@ -508,7 +570,14 @@ internal sealed record HostingCredential /// Gets which of the credential's fields are populated. public required HostingCredentialKind Kind { get; init; } - /// Gets the bearer token, when is . + /// + /// Gets the token, when is or + /// . + /// + /// + /// One field for both kinds, because both carry exactly one opaque string and it is + /// , not the value, that decides which scheme a provider sends it under. + /// public string? Token { get; init; } /// Gets the username, when is . @@ -517,11 +586,30 @@ internal sealed record HostingCredential /// Gets the password, when is . public string? Password { get; init; } - /// Creates a result carrying a bearer token. + /// Creates a result carrying a host-native token, such as a personal access token. + /// + /// Not a bearer token, despite what this method was once documented as: Azure DevOps sends this + /// kind as Basic with an empty username, the scheme its personal access tokens require, and + /// GitHub sends it as Octokit's Token scheme. Use + /// for a credential that must travel as Authorization: Bearer. + /// /// The token. /// The resolved credential. public static HostingCredential FromToken(string token) => new() { Kind = HostingCredentialKind.Token, Token = token }; + /// Creates a result carrying a token to send as Authorization: Bearer. + /// + /// Distinct from because the two travel under different schemes + /// and hosts do not accept them interchangeably. An Entra ID access token authenticates against + /// Azure DevOps only as Bearer, and fails outright in the Basic slot a personal access token + /// uses; on GitHub this maps to Octokit's Bearer authentication type, which is what a + /// GitHub App installation token requires. + /// + /// The token. + /// The resolved credential. + public static HostingCredential FromBearerToken(string token) => + new() { Kind = HostingCredentialKind.BearerToken, Token = token }; + /// Creates a result carrying a username and password. /// The username. /// The password. @@ -530,15 +618,25 @@ public static HostingCredential FromUsernamePassword(string username, string pas new() { Kind = HostingCredentialKind.UsernamePassword, Username = username, Password = password }; } -/// Which fields a carries. -internal enum HostingCredentialKind +/// Which fields a carries, and under which scheme it travels. +public enum HostingCredentialKind { /// No credential — proceed unauthenticated. None, - /// A bearer token, in . + /// + /// A host-native token, in , such as a personal access + /// token. Azure DevOps sends it as Basic with an empty username; GitHub sends it as Octokit's + /// Token scheme. + /// Token, + /// + /// A token, in , to send as Authorization: Bearer — + /// an Entra ID access token against Azure DevOps, or a GitHub App token against GitHub. + /// + BearerToken, + /// A username and password, in and . UsernamePassword, } diff --git a/GitIntegration/Hosting/AzureDevOpsProvider.cs b/GitIntegration/Hosting/AzureDevOpsProvider.cs index 0d02baa..640109f 100644 --- a/GitIntegration/Hosting/AzureDevOpsProvider.cs +++ b/GitIntegration/Hosting/AzureDevOpsProvider.cs @@ -427,6 +427,12 @@ private static void ApplyAuthentication(HttpRequestMessage request, HostingCrede case HostingCredentialKind.Token: request.Headers.Authorization = BasicAuthenticationHeader(string.Empty, credential.Token ?? string.Empty); break; + case HostingCredentialKind.BearerToken: + // Sent as-is under the Bearer scheme, not base64-encoded into the Basic slot above. + // Azure DevOps accepts an Entra ID access token only this way; the same token in the + // personal-access-token slot does not authenticate. + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", credential.Token ?? string.Empty); + break; case HostingCredentialKind.UsernamePassword: request.Headers.Authorization = BasicAuthenticationHeader(credential.Username ?? string.Empty, credential.Password ?? string.Empty); break; diff --git a/README.md b/README.md index 8d442a5..c2a16a8 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,10 @@ builds and parses its requests by hand instead. `GitHubProvider` implements it on top of Octokit, `AzureDevOpsProvider` on a raw `HttpClient` against Azure DevOps's REST API. - **Credential Resolution**: hosting providers integrate with `ktsu.CredentialCache`, so credentials - come from the host's native keyring rather than configuration files. + come from the host's native keyring rather than configuration files. `CredentialSource` is the + escape hatch for a credential a keyring should not hold, such as a short-lived Entra ID access + token, and `HostingCredential.FromBearerToken` sends it as `Authorization: Bearer` rather than in + the personal-access-token slot. - **Semantic Git Types**: validated wrapper types for every identifier Git tooling passes around, so mismatched arguments fail at compile time rather than at runtime. @@ -592,6 +595,43 @@ IGitHostingProvider azure = new AzureDevOpsProvider IReadOnlyList repositories = await azure.GetRepositoriesAsync(); ``` +#### Supplying a credential directly + +`PersonaGUID` reads the host's native keyring, which suits a long-lived secret such as a personal +access token and suits nothing about a short-lived one. An Entra ID access token is minted per +session, expires within the hour, and authenticates against Azure DevOps only as +`Authorization: Bearer` — the Basic slot a personal access token travels in rejects it. Set +`CredentialSource` for that case: + +```csharp +using Azure.Core; +using Azure.Identity; +using ktsu.GitIntegration; +using ktsu.Semantics.Strings; + +TokenCredential entra = new InteractiveBrowserCredential(); +TokenRequestContext context = new(["499b84ac-1321-427f-aa17-267ca6975798/user_impersonation"]); + +IGitHostingProvider azure = new AzureDevOpsProvider +{ + Owner = "my-org".As(), + Project = "my-project".As(), + CredentialSource = () => HostingCredential.FromBearerToken(entra.GetToken(context, default).Token), +}; +``` + +The callback runs on the request path and is consulted on every resolution, never cached, so a +caller can return a freshly refreshed token each time. It should return an already-valid cached +token rather than block on a network round trip — which is what `TokenCredential.GetToken` and its +equivalents already do. `CredentialSource` takes precedence over the credential cache when set, so +there is no need to also clear whatever the keyring holds for the persona. Return +`HostingCredential.None` to proceed unauthenticated. + +`HostingCredential.FromToken` remains the right choice for a host-native token: Azure DevOps sends +it as Basic with an empty username, which is what its personal access tokens require, and GitHub +sends it under Octokit's `Token` scheme. `FromBearerToken` maps to Octokit's `Bearer` type on +GitHub, which is what a GitHub App installation token needs. + `Project` is only required for pull request operations — Azure DevOps has no project-less pull request endpoint, and calling `GetPullRequestsAsync` or `CreatePullRequest` without it throws `InvalidOperationException` immediately. `GetPullRequestsAsync` returns **open** pull requests @@ -871,6 +911,7 @@ pull request creation, over whichever transport and authentication scheme the ho | `Name` | `GitProviderName` | Display name of the provider. | | `Owner` | `GitProviderOwner` | The owner of the repositories in this provider. | | `PersonaGUID` | `PersonaGUID` | The persona GUID used for authentication with the provider (from `ktsu.CredentialCache`). | +| `CredentialSource` | `Func?` | Supplies the credential directly, bypassing the credential cache. Consulted on every resolution, so an expiring token can be refreshed. `null` to resolve through `PersonaGUID`. | | `IsAuthenticated` | `bool` | Whether requests this provider issues carry a credential. A cache entry that resolves to "proceed unauthenticated", and one of a type the provider cannot apply, both report `false`. | #### Methods From d39880fe87c50e7d9c27beed7e95661d5be8120c Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Mon, 21 Sep 2026 10:17:15 +1000 Subject: [PATCH 2/2] docs: state credential rationale without narrating history Two comments described what the code used to be rather than what it is. The diff and the pull request already carry that, and a comment describing a prior state goes stale as soon as anything else moves. Co-Authored-By: Claude Opus 5 (1M context) --- GitIntegration/GitProvider.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/GitIntegration/GitProvider.cs b/GitIntegration/GitProvider.cs index d87b6f4..a470fc0 100644 --- a/GitIntegration/GitProvider.cs +++ b/GitIntegration/GitProvider.cs @@ -557,9 +557,8 @@ internal readonly record struct GitRepositoryAddress(string Value, bool IsHostRe /// fields on one type reads as clearly as a hierarchy would and avoids a second type family /// alongside . /// -/// Public rather than internal, because is the vocabulary -/// a caller supplies a credential in. It was internal while the credential cache was the only way in -/// and this type was purely an implementation detail of that path. +/// Public because is the vocabulary a caller supplies a +/// credential in. /// /// public sealed record HostingCredential @@ -588,10 +587,10 @@ public sealed record HostingCredential /// Creates a result carrying a host-native token, such as a personal access token. /// - /// Not a bearer token, despite what this method was once documented as: Azure DevOps sends this - /// kind as Basic with an empty username, the scheme its personal access tokens require, and - /// GitHub sends it as Octokit's Token scheme. Use - /// for a credential that must travel as Authorization: Bearer. + /// Not a bearer token. Azure DevOps sends this kind as Basic with an empty username, the scheme + /// its personal access tokens require, and GitHub sends it as Octokit's Token scheme. Use + /// for a credential that must travel as + /// Authorization: Bearer. /// /// The token. /// The resolved credential.