From b501b8b95cff52dc19678677604a5032096fe0fc Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 5 Sep 2023 14:22:11 -0700 Subject: [PATCH] github: support gist remote URLs for GitHub We have not been consistently detecting or normalising "gist" URLs for dotcom or GHES instances. Gists are backed by a Git repository and can be cloned/pushed-to etc like a normal repository. Credentials are the same as the base site. Update our OAuth, rest API, and dotcom-detection methods that deal with the remote or target URL to correctly support gists URLs. Also add some tests around this. --- .../GitHub.Tests/GitHubHostProviderTests.cs | 5 +++++ src/shared/GitHub.Tests/GitHubRestApiTests.cs | 16 ++++++++++++++-- src/shared/GitHub/GitHubHostProvider.cs | 11 +++++++---- src/shared/GitHub/GitHubOAuth2Client.cs | 5 ++++- src/shared/GitHub/GitHubRestApi.cs | 9 +++++++-- 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/shared/GitHub.Tests/GitHubHostProviderTests.cs b/src/shared/GitHub.Tests/GitHubHostProviderTests.cs index aa085b34c..573ab4e08 100644 --- a/src/shared/GitHub.Tests/GitHubHostProviderTests.cs +++ b/src/shared/GitHub.Tests/GitHubHostProviderTests.cs @@ -15,8 +15,11 @@ public class GitHubHostProviderTests [InlineData("https://github.com", true)] [InlineData("https://gitHUB.CoM", true)] [InlineData("https://GITHUB.COM", true)] + [InlineData("https://gist.github.com", true)] [InlineData("https://foogithub.com", false)] [InlineData("https://api.github.com", false)] + [InlineData("https://api.gist.github.com", false)] + [InlineData("https://foogist.github.com", false)] public void GitHubHostProvider_IsGitHubDotCom(string input, bool expected) { Assert.Equal(expected, GitHubHostProvider.IsGitHubDotCom(new Uri(input))); @@ -98,6 +101,8 @@ public void GitHubHostProvider_GetCredentialServiceUrl(string protocol, string h [InlineData("https://GitHub.Com", "none", GitHubConstants.DotComAuthenticationModes)] [InlineData("https://github.com", null, GitHubConstants.DotComAuthenticationModes)] [InlineData("https://GitHub.Com", null, GitHubConstants.DotComAuthenticationModes)] + [InlineData("https://gist.github.com", null, GitHubConstants.DotComAuthenticationModes)] + [InlineData("https://GIST.GITHUB.COM", null, GitHubConstants.DotComAuthenticationModes)] public async Task GitHubHostProvider_GetSupportedAuthenticationModes(string uriString, string gitHubAuthModes, AuthenticationModes expectedModes) { var targetUri = new Uri(uriString); diff --git a/src/shared/GitHub.Tests/GitHubRestApiTests.cs b/src/shared/GitHub.Tests/GitHubRestApiTests.cs index 01f1cb0e8..c0c42e089 100644 --- a/src/shared/GitHub.Tests/GitHubRestApiTests.cs +++ b/src/shared/GitHub.Tests/GitHubRestApiTests.cs @@ -2,8 +2,6 @@ using System.Linq; using System.Net; using System.Net.Http; -using System.Net.Http.Headers; -using System.Text; using System.Threading.Tasks; using GitCredentialManager.Tests; using GitCredentialManager.Tests.Objects; @@ -13,6 +11,20 @@ namespace GitHub.Tests { public class GitHubRestApiTests { + [Theory] + [InlineData("https://github.com", "user", "https://api.github.com/user")] + [InlineData("https://github.com", "users/123", "https://api.github.com/users/123")] + [InlineData("https://gItHuB.cOm", "uSeRs/123", "https://api.github.com/uSeRs/123")] + [InlineData("https://gist.github.com", "user", "https://api.github.com/user")] + [InlineData("https://github.example.com", "user", "https://github.example.com/api/v3/user")] + [InlineData("https://raw.github.example.com", "user", "https://github.example.com/api/v3/user")] + [InlineData("https://gist.github.example.com", "user", "https://github.example.com/api/v3/user")] + public void GitHubRestApi_GetApiRequestUri(string targetUrl, string apiUrl, string expected) + { + Uri actualUri = GitHubRestApi.GetApiRequestUri(new Uri(targetUrl), apiUrl); + Assert.Equal(expected, actualUri.ToString()); + } + [Fact] public async Task GitHubRestApi_AcquireTokenAsync_NullUri_ThrowsException() { diff --git a/src/shared/GitHub/GitHubHostProvider.cs b/src/shared/GitHub/GitHubHostProvider.cs index 6e96e3621..918e859a0 100644 --- a/src/shared/GitHub/GitHubHostProvider.cs +++ b/src/shared/GitHub/GitHubHostProvider.cs @@ -487,10 +487,12 @@ public static bool IsGitHubDotCom(Uri targetUri) { EnsureArgument.AbsoluteUri(targetUri, nameof(targetUri)); - return StringComparer.OrdinalIgnoreCase.Equals(targetUri.Host, GitHubConstants.GitHubBaseUrlHost); + // github.com or gist.github.com are both considered dotcom + return StringComparer.OrdinalIgnoreCase.Equals(targetUri.Host, GitHubConstants.GitHubBaseUrlHost) || + StringComparer.OrdinalIgnoreCase.Equals(targetUri.Host, GitHubConstants.GistBaseUrlHost); } - private static Uri NormalizeUri(Uri uri) + internal static Uri NormalizeUri(Uri uri) { if (uri is null) { @@ -500,8 +502,9 @@ private static Uri NormalizeUri(Uri uri) // Special case for gist.github.com which are git backed repositories under the hood. // Credentials for these repositories are the same as the one stored with "github.com". // Same for gist.github[.subdomain].domain.tld. The general form was already checked via IsSupported. - int firstDot = uri.DnsSafeHost.IndexOf("."); - if (firstDot > -1 && uri.DnsSafeHost.Substring(0, firstDot).Equals("gist", StringComparison.OrdinalIgnoreCase)) { + int firstDot = uri.DnsSafeHost.IndexOf(".", StringComparison.Ordinal); + if (firstDot > -1 && uri.DnsSafeHost.Substring(0, firstDot).Equals("gist", StringComparison.OrdinalIgnoreCase)) + { return new Uri("https://" + uri.DnsSafeHost.Substring(firstDot+1)); } diff --git a/src/shared/GitHub/GitHubOAuth2Client.cs b/src/shared/GitHub/GitHubOAuth2Client.cs index 437b4c066..2eb4aae88 100644 --- a/src/shared/GitHub/GitHubOAuth2Client.cs +++ b/src/shared/GitHub/GitHubOAuth2Client.cs @@ -11,8 +11,11 @@ public GitHubOAuth2Client(HttpClient httpClient, ISettings settings, Uri baseUri : base(httpClient, CreateEndpoints(baseUri), GetClientId(settings), trace2, GetRedirectUri(settings, baseUri), GetClientSecret(settings)) { } - private static OAuth2ServerEndpoints CreateEndpoints(Uri baseUri) + private static OAuth2ServerEndpoints CreateEndpoints(Uri uri) { + // Ensure that the base URI is normalized to support Gist subdomains + Uri baseUri = GitHubHostProvider.NormalizeUri(uri); + Uri authEndpoint = new Uri(baseUri, GitHubConstants.OAuthAuthorizationEndpointRelativeUri); Uri tokenEndpoint = new Uri(baseUri, GitHubConstants.OAuthTokenEndpointRelativeUri); Uri deviceAuthEndpoint = new Uri(baseUri, GitHubConstants.OAuthDeviceEndpointRelativeUri); diff --git a/src/shared/GitHub/GitHubRestApi.cs b/src/shared/GitHub/GitHubRestApi.cs index 783812b8e..5051ed2bb 100644 --- a/src/shared/GitHub/GitHubRestApi.cs +++ b/src/shared/GitHub/GitHubRestApi.cs @@ -203,7 +203,7 @@ private async Task ParseSuccessResponseAsync(Uri targetUri } } - private Uri GetApiRequestUri(Uri targetUri, string apiUrl) + internal /* for testing */ static Uri GetApiRequestUri(Uri targetUri, string apiUrl) { if (GitHubHostProvider.IsGitHubDotCom(targetUri)) { @@ -214,8 +214,13 @@ private Uri GetApiRequestUri(Uri targetUri, string apiUrl) // If we're here, it's GitHub Enterprise via a configured authority var baseUrl = targetUri.GetLeftPart(UriPartial.Authority); + RegexOptions reOptions = RegexOptions.CultureInvariant | RegexOptions.IgnoreCase; + // Check for 'raw.' in the hostname and remove it to get the correct GHE API URL - baseUrl = Regex.Replace(baseUrl, @"^(https?://)raw\.", "$1", RegexOptions.CultureInvariant | RegexOptions.IgnoreCase); + baseUrl = Regex.Replace(baseUrl, @"^(https?://)raw\.", "$1", reOptions); + + // Likewise check for `gist.` in the hostname and remove it to get the correct GHE API URL + baseUrl = Regex.Replace(baseUrl, @"^(https?://)gist\.", "$1", reOptions); return new Uri(baseUrl + $"/api/v3/{apiUrl}"); }