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..a470fc0 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,13 @@ 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 because is the vocabulary a caller supplies a +/// credential in. +/// /// -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 +569,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 +585,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. 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 +617,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