Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<GitProviderOwner>(),
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()
{
Expand Down
20 changes: 20 additions & 0 deletions GitIntegration.Test/Hosting/GitHubProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<GitProviderOwner>(),
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()
{
Expand Down
95 changes: 95 additions & 0 deletions GitIntegration.Test/Hosting/GitProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<GitProviderOwner>(),
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<CredentialToken>() });
TestProvider provider = new()
{
Owner = "octocat".As<GitProviderOwner>(),
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<GitProviderOwner>(),
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<GitProviderOwner>(),
CredentialSource = () => HostingCredential.None,
};

Assert.AreEqual(HostingCredentialKind.None, provider.CallResolveCredential().Kind);
Assert.IsFalse(provider.IsAuthenticated);
}

[TestMethod]
public void ReportsAuthenticatedForACredentialSourceSupplyingAToken()
{
TestProvider provider = new()
{
Owner = "octocat".As<GitProviderOwner>(),
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<GitProviderOwner>(),
CredentialSource = () => null!,
};

_ = Assert.ThrowsExactly<InvalidOperationException>(provider.CallResolveCredential);
}

[TestMethod]
public void ProceedsUnauthenticatedWhenNoCredentialIsResolved()
{
Expand Down
4 changes: 4 additions & 0 deletions GitIntegration/GitHubProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <value>". GitHub requires "Bearer <value>" 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,
};
Expand Down
117 changes: 107 additions & 10 deletions GitIntegration/GitProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,37 @@ public abstract class GitProvider : IGitHostingProvider
/// <value>A GUID identifying the authentication persona.</value>
public PersonaGUID PersonaGUID { get; init; } = CredentialCache.CreatePersonaGUID();

/// <summary>
/// Gets or initializes a callback supplying this provider's credential directly, bypassing the
/// credential cache, or <see langword="null"/> to resolve through <see cref="PersonaGUID"/> as
/// usual.
/// </summary>
/// <remarks>
/// <para>
/// The seam for a credential the credential cache cannot hold. <see cref="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 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.
/// </para>
/// <para>
/// A callback rather than a stored <see cref="HostingCredential"/>, 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
/// <c>TokenCredential.GetToken</c> and its equivalents already do.
/// </para>
/// <para>
/// 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
/// <see cref="HostingCredential.None"/> is how it says "proceed unauthenticated"; returning
/// <see langword="null"/> is a caller bug and makes <see cref="ResolveCredential"/> throw.
/// </para>
/// </remarks>
/// <value>The callback, or <see langword="null"/> to use the credential cache.</value>
public Func<HostingCredential>? CredentialSource { get; init; }

/// <inheritdoc/>
/// <remarks>
/// Reports whether a request this provider issues would actually carry a credential, which is a
Expand Down Expand Up @@ -236,9 +267,21 @@ public bool TryGetCredential(out Credential? credential)
/// <exception cref="InvalidOperationException">
/// A credential was resolved whose runtime type is not one this method recognises.
/// </exception>
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.");
}

/// <summary>
/// Resolves this provider's credential, reporting an unrecognised <see cref="Credential"/>
Expand All @@ -258,6 +301,20 @@ internal HostingCredential ResolveCredential() =>
/// <returns>The resolved credential, or <see langword="null"/> for an unrecognised subtype.</returns>
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;
Expand Down Expand Up @@ -498,17 +555,28 @@ internal readonly record struct GitRepositoryAddress(string Value, bool IsHostRe
/// A record with a <see cref="HostingCredentialKind"/> discriminator rather than a type hierarchy:
/// a provider applying this result switches on <see cref="Kind"/> 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 <see cref="Credential"/> for what is, here, an internal implementation detail.
/// alongside <see cref="Credential"/>.
/// <para>
/// Public because <see cref="GitProvider.CredentialSource"/> is the vocabulary a caller supplies a
/// credential in.
/// </para>
/// </remarks>
internal sealed record HostingCredential
public sealed record HostingCredential
{
/// <summary>The singleton result for "no credential to apply".</summary>
public static readonly HostingCredential None = new() { Kind = HostingCredentialKind.None };

/// <summary>Gets which of the credential's fields are populated.</summary>
public required HostingCredentialKind Kind { get; init; }

/// <summary>Gets the bearer token, when <see cref="Kind"/> is <see cref="HostingCredentialKind.Token"/>.</summary>
/// <summary>
/// Gets the token, when <see cref="Kind"/> is <see cref="HostingCredentialKind.Token"/> or
/// <see cref="HostingCredentialKind.BearerToken"/>.
/// </summary>
/// <remarks>
/// One field for both kinds, because both carry exactly one opaque string and it is
/// <see cref="Kind"/>, not the value, that decides which scheme a provider sends it under.
/// </remarks>
public string? Token { get; init; }

/// <summary>Gets the username, when <see cref="Kind"/> is <see cref="HostingCredentialKind.UsernamePassword"/>.</summary>
Expand All @@ -517,11 +585,30 @@ internal sealed record HostingCredential
/// <summary>Gets the password, when <see cref="Kind"/> is <see cref="HostingCredentialKind.UsernamePassword"/>.</summary>
public string? Password { get; init; }

/// <summary>Creates a result carrying a bearer token.</summary>
/// <summary>Creates a result carrying a host-native token, such as a personal access token.</summary>
/// <remarks>
/// 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 <c>Token</c> scheme. Use
/// <see cref="FromBearerToken(string)"/> for a credential that must travel as
/// <c>Authorization: Bearer</c>.
/// </remarks>
/// <param name="token">The token.</param>
/// <returns>The resolved credential.</returns>
public static HostingCredential FromToken(string token) => new() { Kind = HostingCredentialKind.Token, Token = token };

/// <summary>Creates a result carrying a token to send as <c>Authorization: Bearer</c>.</summary>
/// <remarks>
/// Distinct from <see cref="FromToken(string)"/> 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 <c>Bearer</c> authentication type, which is what a
/// GitHub App installation token requires.
/// </remarks>
/// <param name="token">The token.</param>
/// <returns>The resolved credential.</returns>
public static HostingCredential FromBearerToken(string token) =>
new() { Kind = HostingCredentialKind.BearerToken, Token = token };

/// <summary>Creates a result carrying a username and password.</summary>
/// <param name="username">The username.</param>
/// <param name="password">The password.</param>
Expand All @@ -530,15 +617,25 @@ public static HostingCredential FromUsernamePassword(string username, string pas
new() { Kind = HostingCredentialKind.UsernamePassword, Username = username, Password = password };
}

/// <summary>Which fields a <see cref="HostingCredential"/> carries.</summary>
internal enum HostingCredentialKind
/// <summary>Which fields a <see cref="HostingCredential"/> carries, and under which scheme it travels.</summary>
public enum HostingCredentialKind
{
/// <summary>No credential — proceed unauthenticated.</summary>
None,

/// <summary>A bearer token, in <see cref="HostingCredential.Token"/>.</summary>
/// <summary>
/// A host-native token, in <see cref="HostingCredential.Token"/>, such as a personal access
/// token. Azure DevOps sends it as Basic with an empty username; GitHub sends it as Octokit's
/// <c>Token</c> scheme.
/// </summary>
Token,

/// <summary>
/// A token, in <see cref="HostingCredential.Token"/>, to send as <c>Authorization: Bearer</c> —
/// an Entra ID access token against Azure DevOps, or a GitHub App token against GitHub.
/// </summary>
BearerToken,

/// <summary>A username and password, in <see cref="HostingCredential.Username"/> and <see cref="HostingCredential.Password"/>.</summary>
UsernamePassword,
}
6 changes: 6 additions & 0 deletions GitIntegration/Hosting/AzureDevOpsProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading