Skip to content

Commit

Permalink
Assume GitHub Enterprise instances with a null version number support…
Browse files Browse the repository at this point in the history
… OAuth (#1154)

Enterprise instances that return a null version are considered to
support OAuth. Only disable OAuth as an option if we can verify we are
talking to an old Enterprise Server.

This is a less fragile option as we never block the user from using an
auth method they could use, and don't need to special case GHAE.
  • Loading branch information
mjcheetham authored Mar 15, 2023
2 parents aa53868 + a961eac commit d1b9b8b
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 15 deletions.
6 changes: 3 additions & 3 deletions src/shared/GitHub.Tests/GitHubHostProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,14 @@ public async Task GitHubHostProvider_GetSupportedAuthenticationModes(string uriS
Assert.Equal(expectedModes, actualModes);
}


[Theory]
[InlineData("https://example.com", null, "0.1", false, AuthenticationModes.Pat)]
[InlineData("https://example.com", null, "0.1", true, AuthenticationModes.Basic | AuthenticationModes.Pat)]
[InlineData("https://example.com", null, "100.0", false, AuthenticationModes.OAuth | AuthenticationModes.Pat)]
[InlineData("https://example.com", null, "100.0", true, AuthenticationModes.All)]
[InlineData("https://example.com", null, null, false, AuthenticationModes.OAuth | AuthenticationModes.Pat)]
[InlineData("https://example.com", null, "", false, AuthenticationModes.OAuth | AuthenticationModes.Pat)]
[InlineData("https://example.com", null, " ", false, AuthenticationModes.OAuth | AuthenticationModes.Pat)]
public async Task GitHubHostProvider_GetSupportedAuthenticationModes_WithMetadata(string uriString, string gitHubAuthModes,
string installedVersion, bool verifiablePasswordAuthentication, AuthenticationModes expectedModes)
{
Expand All @@ -149,8 +151,6 @@ public async Task GitHubHostProvider_GetSupportedAuthenticationModes_WithMetadat
Assert.Equal(expectedModes, actualModes);
}



[Fact]
public async Task GitHubHostProvider_GenerateCredentialAsync_UnencryptedHttp_ThrowsException()
{
Expand Down
5 changes: 0 additions & 5 deletions src/shared/GitHub/GitHubConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ public static class GitHubConstants
/// </summary>
public static readonly Version MinimumOnPremOAuthVersion = new Version("3.2");

/// <summary>
/// The version string returned from the meta API endpoint for GitHub AE instances.
/// </summary>
public const string GitHubAeVersionString = "GitHub AE";

/// <summary>
/// Supported authentication modes for GitHub.com.
/// </summary>
Expand Down
14 changes: 7 additions & 7 deletions src/shared/GitHub/GitHubHostProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -263,20 +263,20 @@ internal async Task<AuthenticationModes> GetSupportedAuthenticationModesAsync(Ur
{
GitHubMetaInfo metaInfo = await _gitHubApi.GetMetaInfoAsync(targetUri);

// All Enterprise/AE instances support PATs
var modes = AuthenticationModes.Pat;

// If the server says it supports basic auth, we can use that too!
if (metaInfo.VerifiablePasswordAuthentication)
{
modes |= AuthenticationModes.Basic;
}

if (StringComparer.OrdinalIgnoreCase.Equals(metaInfo.InstalledVersion, GitHubConstants.GitHubAeVersionString))
{
// Assume all GHAE instances have the GCM OAuth application deployed
modes |= AuthenticationModes.OAuth;
}
else if (Version.TryParse(metaInfo.InstalledVersion, out var version) && version >= GitHubConstants.MinimumOnPremOAuthVersion)
// If the version is unknown, we *assume* it supports OAuth.
// If the server version at least the minimum required, we *know* we can use OAuth.
if (!Version.TryParse(metaInfo.InstalledVersion, out var version) ||
version >= GitHubConstants.MinimumOnPremOAuthVersion)
{
// Only GHES versions beyond the minimum version have the GCM OAuth application deployed
modes |= AuthenticationModes.OAuth;
}

Expand Down

0 comments on commit d1b9b8b

Please sign in to comment.