From c1b5e69d2f94da2513e5ae7280908e4f42bc8a53 Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Mon, 30 Nov 2020 22:44:06 +0100 Subject: [PATCH 1/3] Detect github.my-company-server.com as GitHub --- .../GitHub.Tests/GitHubHostProviderTests.cs | 24 ++++++++++- src/shared/GitHub/GitHubHostProvider.cs | 42 ++++++++++++++----- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/shared/GitHub.Tests/GitHubHostProviderTests.cs b/src/shared/GitHub.Tests/GitHubHostProviderTests.cs index 338329024..0166e720e 100644 --- a/src/shared/GitHub.Tests/GitHubHostProviderTests.cs +++ b/src/shared/GitHub.Tests/GitHubHostProviderTests.cs @@ -30,10 +30,26 @@ public void GitHubHostProvider_IsGitHubDotCom(string input, bool expected) // show a helpful error message in the call to `GenerateCredentialAsync` instead. [InlineData("http://github.com", true)] [InlineData("http://gist.github.com", true)] - [InlineData("https://github.com", true)] - [InlineData("https://gist.github.com", true)] [InlineData("ssh://github.com", false)] [InlineData("https://example.com", false)] + + [InlineData("https://github.com", true)] + [InlineData("https://github.con", false)] // No support of phony similar tld. + [InlineData("https://gist.github.con", false)] // No support of phony similar tld. + [InlineData("https://foogithub.com", false)] // No support of non github.com domains. + [InlineData("https://api.github.com", false)] // No support of github.com subdomains. + [InlineData("https://gist.github.com", true)] // Except gists. + + [InlineData("http://github.my-company-server.com", true)] + [InlineData("http://gist.github.my-company-server.com", true)] + [InlineData("https://github.my-company-server.com", true)] + [InlineData("https://gist.github.my-company-server.com", true)] + [InlineData("https://gist.my-company-server.com", false)] + [InlineData("https://my-company-server.com", false)] + [InlineData("https://github.my.company.server.com", true)] + [InlineData("https://foogithub.my-company-server.com", false)] + [InlineData("https://api.github.my-company-server.com", false)] + [InlineData("https://gist.github.my.company.server.com", true)] public void GitHubHostProvider_IsSupported(string uriString, bool expected) { Uri uri = new Uri(uriString); @@ -55,6 +71,10 @@ public void GitHubHostProvider_IsSupported(string uriString, bool expected) [Theory] [InlineData("https://github.com", "https://github.com")] [InlineData("https://gist.github.com", "https://github.com")] + [InlineData("https://github.my-company-server.com", "https://github.my-company-server.com")] + [InlineData("https://gist.github.my-company-server.com", "https://github.my-company-server.com")] + [InlineData("https://github.my.company.server.com", "https://github.my.company.server.com")] + [InlineData("https://gist.github.my.company.server.com", "https://github.my.company.server.com")] public void GitHubHostProvider_GetCredentialServiceUrl(string uriString, string expectedService) { Uri uri = new Uri(uriString); diff --git a/src/shared/GitHub/GitHubHostProvider.cs b/src/shared/GitHub/GitHubHostProvider.cs index e1ec4a86b..16edc6854 100644 --- a/src/shared/GitHub/GitHubHostProvider.cs +++ b/src/shared/GitHub/GitHubHostProvider.cs @@ -52,16 +52,35 @@ public override bool IsSupported(InputArguments input) return false; } - // Split port number and hostname from host input argument - input.TryGetHostAndPort(out string hostName, out _); - // We do not support unencrypted HTTP communications to GitHub, // but we report `true` here for HTTP so that we can show a helpful // error message for the user in `CreateCredentialAsync`. - return (StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "http") || - StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "https")) && - (StringComparer.OrdinalIgnoreCase.Equals(hostName, GitHubConstants.GitHubBaseUrlHost) || - StringComparer.OrdinalIgnoreCase.Equals(hostName, GitHubConstants.GistBaseUrlHost)); + if (!StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "http") && + !StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "https")) + { + return false; + } + + // Split port number and hostname from host input argument + input.TryGetHostAndPort(out string hostName, out _); + + if (StringComparer.OrdinalIgnoreCase.Equals(hostName, GitHubConstants.GitHubBaseUrlHost) || + StringComparer.OrdinalIgnoreCase.Equals(hostName, GitHubConstants.GistBaseUrlHost)) + { + return true; + } + + string[] domains = hostName.Split(new char[] { '.' }); + + // github[.subdomain].domain.tld + if (domains.Length >= 3 && domains[0] == "github") + return true; + + // gist.github[.subdomain].domain.tld + if (domains.Length >= 4 && domains[0] == "gist" && domains[1] == "github") + return true; + + return false; } public override string GetServiceName(InputArguments input) @@ -264,10 +283,11 @@ 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" - if (uri.DnsSafeHost.Equals(GitHubConstants.GistBaseUrlHost, StringComparison.OrdinalIgnoreCase)) - { - return new Uri("https://" + GitHubConstants.GitHubBaseUrlHost); + // 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 (uri.DnsSafeHost.Substring(0, firstDot).Equals("gist", StringComparison.OrdinalIgnoreCase)) { + return new Uri("https://" + uri.DnsSafeHost.Substring(firstDot+1)); } return uri; From e5e4ff46bb40fb59b9a63ae1d98fd871985b7529 Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Tue, 8 Dec 2020 22:17:42 +0100 Subject: [PATCH 2/3] Make comparisons case insensitive Co-authored-by: Matthew John Cheetham --- .../GitHub.Tests/GitHubHostProviderTests.cs | 17 +++++++++++++++-- src/shared/GitHub/GitHubHostProvider.cs | 13 ++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/shared/GitHub.Tests/GitHubHostProviderTests.cs b/src/shared/GitHub.Tests/GitHubHostProviderTests.cs index 0166e720e..84161414b 100644 --- a/src/shared/GitHub.Tests/GitHubHostProviderTests.cs +++ b/src/shared/GitHub.Tests/GitHubHostProviderTests.cs @@ -39,6 +39,8 @@ public void GitHubHostProvider_IsGitHubDotCom(string input, bool expected) [InlineData("https://foogithub.com", false)] // No support of non github.com domains. [InlineData("https://api.github.com", false)] // No support of github.com subdomains. [InlineData("https://gist.github.com", true)] // Except gists. + [InlineData("https://GiST.GitHub.Com", true)] + [InlineData("https://GitHub.Com", true)] [InlineData("http://github.my-company-server.com", true)] [InlineData("http://gist.github.my-company-server.com", true)] @@ -50,6 +52,8 @@ public void GitHubHostProvider_IsGitHubDotCom(string input, bool expected) [InlineData("https://foogithub.my-company-server.com", false)] [InlineData("https://api.github.my-company-server.com", false)] [InlineData("https://gist.github.my.company.server.com", true)] + [InlineData("https://GitHub.My-Company-Server.Com", true)] + [InlineData("https://GiST.GitHub.My-Company-Server.com", true)] public void GitHubHostProvider_IsSupported(string uriString, bool expected) { Uri uri = new Uri(uriString); @@ -61,7 +65,7 @@ public void GitHubHostProvider_IsSupported(string uriString, bool expected) }); // Ensure nothing got lost during transformation - Assert.Equal(uriString, input.Protocol + "://" + input.Host); + Assert.Equal(uriString.ToLower(), input.Protocol + "://" + input.Host); var provider = new GitHubHostProvider(new TestCommandContext()); Assert.Equal(expected, provider.IsSupported(input)); @@ -70,11 +74,17 @@ public void GitHubHostProvider_IsSupported(string uriString, bool expected) [Theory] [InlineData("https://github.com", "https://github.com")] + [InlineData("https://GitHub.Com", "https://github.com")] [InlineData("https://gist.github.com", "https://github.com")] + [InlineData("https://GiST.GitHub.Com", "https://github.com")] [InlineData("https://github.my-company-server.com", "https://github.my-company-server.com")] + [InlineData("https://GitHub.My-Company-Server.Com", "https://github.my-company-server.com")] [InlineData("https://gist.github.my-company-server.com", "https://github.my-company-server.com")] + [InlineData("https://GiST.GitHub.My-Company-Server.Com", "https://github.my-company-server.com")] [InlineData("https://github.my.company.server.com", "https://github.my.company.server.com")] + [InlineData("https://GitHub.My.Company.Server.Com", "https://github.my.company.server.com")] [InlineData("https://gist.github.my.company.server.com", "https://github.my.company.server.com")] + [InlineData("https://GiST.GitHub.My.Company.Server.Com", "https://github.my.company.server.com")] public void GitHubHostProvider_GetCredentialServiceUrl(string uriString, string expectedService) { Uri uri = new Uri(uriString); @@ -86,7 +96,7 @@ public void GitHubHostProvider_GetCredentialServiceUrl(string uriString, string }); // Ensure nothing got lost during transformation - Assert.Equal(uriString, input.Protocol + "://" + input.Host); + Assert.Equal(uriString.ToLower(), input.Protocol + "://" + input.Host); var provider = new GitHubHostProvider(new TestCommandContext()); Assert.Equal(expectedService, provider.GetServiceName(input)); @@ -96,8 +106,11 @@ public void GitHubHostProvider_GetCredentialServiceUrl(string uriString, string [Theory] [InlineData("https://example.com", "oauth", AuthenticationModes.OAuth)] [InlineData("https://github.com", "NOT-A-REAL-VALUE", GitHubConstants.DotComAuthenticationModes)] + [InlineData("https://GitHub.Com", "NOT-A-REAL-VALUE", GitHubConstants.DotComAuthenticationModes)] [InlineData("https://github.com", "none", GitHubConstants.DotComAuthenticationModes)] + [InlineData("https://GitHub.Com", "none", GitHubConstants.DotComAuthenticationModes)] [InlineData("https://github.com", null, GitHubConstants.DotComAuthenticationModes)] + [InlineData("https://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/GitHubHostProvider.cs b/src/shared/GitHub/GitHubHostProvider.cs index 16edc6854..a85f6ba40 100644 --- a/src/shared/GitHub/GitHubHostProvider.cs +++ b/src/shared/GitHub/GitHubHostProvider.cs @@ -73,12 +73,19 @@ public override bool IsSupported(InputArguments input) string[] domains = hostName.Split(new char[] { '.' }); // github[.subdomain].domain.tld - if (domains.Length >= 3 && domains[0] == "github") + if (domains.Length >= 3 && + StringComparer.OrdinalIgnoreCase.Equals(domains[0], "github")) + { return true; + } // gist.github[.subdomain].domain.tld - if (domains.Length >= 4 && domains[0] == "gist" && domains[1] == "github") + if (domains.Length >= 4 && + StringComparer.OrdinalIgnoreCase.Equals(domains[0], "gist") && + StringComparer.OrdinalIgnoreCase.Equals(domains[1], "github")) + { return true; + } return false; } @@ -286,7 +293,7 @@ private static Uri NormalizeUri(Uri uri) // 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 (uri.DnsSafeHost.Substring(0, firstDot).Equals("gist", StringComparison.OrdinalIgnoreCase)) { + if (firstDot > -1 && uri.DnsSafeHost.Substring(0, firstDot).Equals("gist", StringComparison.OrdinalIgnoreCase)) { return new Uri("https://" + uri.DnsSafeHost.Substring(firstDot+1)); } From 7d6b9c58906ce499a2c9502d7e076882427b813c Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Tue, 8 Dec 2020 22:34:08 +0100 Subject: [PATCH 3/3] Make tests actually test mixed case input Uri converts to lowercase. --- .../GitHub.Tests/GitHubHostProviderTests.cs | 98 +++++++++---------- 1 file changed, 44 insertions(+), 54 deletions(-) diff --git a/src/shared/GitHub.Tests/GitHubHostProviderTests.cs b/src/shared/GitHub.Tests/GitHubHostProviderTests.cs index 84161414b..6a8cce07e 100644 --- a/src/shared/GitHub.Tests/GitHubHostProviderTests.cs +++ b/src/shared/GitHub.Tests/GitHubHostProviderTests.cs @@ -28,76 +28,66 @@ public void GitHubHostProvider_IsGitHubDotCom(string input, bool expected) [Theory] // We report that we support unencrypted HTTP here so that we can fail and // show a helpful error message in the call to `GenerateCredentialAsync` instead. - [InlineData("http://github.com", true)] - [InlineData("http://gist.github.com", true)] - [InlineData("ssh://github.com", false)] - [InlineData("https://example.com", false)] - - [InlineData("https://github.com", true)] - [InlineData("https://github.con", false)] // No support of phony similar tld. - [InlineData("https://gist.github.con", false)] // No support of phony similar tld. - [InlineData("https://foogithub.com", false)] // No support of non github.com domains. - [InlineData("https://api.github.com", false)] // No support of github.com subdomains. - [InlineData("https://gist.github.com", true)] // Except gists. - [InlineData("https://GiST.GitHub.Com", true)] - [InlineData("https://GitHub.Com", true)] - - [InlineData("http://github.my-company-server.com", true)] - [InlineData("http://gist.github.my-company-server.com", true)] - [InlineData("https://github.my-company-server.com", true)] - [InlineData("https://gist.github.my-company-server.com", true)] - [InlineData("https://gist.my-company-server.com", false)] - [InlineData("https://my-company-server.com", false)] - [InlineData("https://github.my.company.server.com", true)] - [InlineData("https://foogithub.my-company-server.com", false)] - [InlineData("https://api.github.my-company-server.com", false)] - [InlineData("https://gist.github.my.company.server.com", true)] - [InlineData("https://GitHub.My-Company-Server.Com", true)] - [InlineData("https://GiST.GitHub.My-Company-Server.com", true)] - public void GitHubHostProvider_IsSupported(string uriString, bool expected) + [InlineData("http", "github.com", true)] + [InlineData("http", "gist.github.com", true)] + [InlineData("ssh", "github.com", false)] + [InlineData("https", "example.com", false)] + + [InlineData("https", "github.com", true)] + [InlineData("https", "github.con", false)] // No support of phony similar tld. + [InlineData("https", "gist.github.con", false)] // No support of phony similar tld. + [InlineData("https", "foogithub.com", false)] // No support of non github.com domains. + [InlineData("https", "api.github.com", false)] // No support of github.com subdomains. + [InlineData("https", "gist.github.com", true)] // Except gists. + [InlineData("https", "GiST.GitHub.Com", true)] + [InlineData("https", "GitHub.Com", true)] + + [InlineData("http", "github.my-company-server.com", true)] + [InlineData("http", "gist.github.my-company-server.com", true)] + [InlineData("https", "github.my-company-server.com", true)] + [InlineData("https", "gist.github.my-company-server.com", true)] + [InlineData("https", "gist.my-company-server.com", false)] + [InlineData("https", "my-company-server.com", false)] + [InlineData("https", "github.my.company.server.com", true)] + [InlineData("https", "foogithub.my-company-server.com", false)] + [InlineData("https", "api.github.my-company-server.com", false)] + [InlineData("https", "gist.github.my.company.server.com", true)] + [InlineData("https", "GitHub.My-Company-Server.Com", true)] + [InlineData("https", "GiST.GitHub.My-Company-Server.com", true)] + public void GitHubHostProvider_IsSupported(string protocol, string host, bool expected) { - Uri uri = new Uri(uriString); - var input = new InputArguments(new Dictionary { - ["protocol"] = uri.Scheme, - ["host"] = uri.Host, + ["protocol"] = protocol, + ["host"] = host, }); - // Ensure nothing got lost during transformation - Assert.Equal(uriString.ToLower(), input.Protocol + "://" + input.Host); - var provider = new GitHubHostProvider(new TestCommandContext()); Assert.Equal(expected, provider.IsSupported(input)); } [Theory] - [InlineData("https://github.com", "https://github.com")] - [InlineData("https://GitHub.Com", "https://github.com")] - [InlineData("https://gist.github.com", "https://github.com")] - [InlineData("https://GiST.GitHub.Com", "https://github.com")] - [InlineData("https://github.my-company-server.com", "https://github.my-company-server.com")] - [InlineData("https://GitHub.My-Company-Server.Com", "https://github.my-company-server.com")] - [InlineData("https://gist.github.my-company-server.com", "https://github.my-company-server.com")] - [InlineData("https://GiST.GitHub.My-Company-Server.Com", "https://github.my-company-server.com")] - [InlineData("https://github.my.company.server.com", "https://github.my.company.server.com")] - [InlineData("https://GitHub.My.Company.Server.Com", "https://github.my.company.server.com")] - [InlineData("https://gist.github.my.company.server.com", "https://github.my.company.server.com")] - [InlineData("https://GiST.GitHub.My.Company.Server.Com", "https://github.my.company.server.com")] - public void GitHubHostProvider_GetCredentialServiceUrl(string uriString, string expectedService) + [InlineData("https", "github.com", "https://github.com")] + [InlineData("https", "GitHub.Com", "https://github.com")] + [InlineData("https", "gist.github.com", "https://github.com")] + [InlineData("https", "GiST.GitHub.Com", "https://github.com")] + [InlineData("https", "github.my-company-server.com", "https://github.my-company-server.com")] + [InlineData("https", "GitHub.My-Company-Server.Com", "https://github.my-company-server.com")] + [InlineData("https", "gist.github.my-company-server.com", "https://github.my-company-server.com")] + [InlineData("https", "GiST.GitHub.My-Company-Server.Com", "https://github.my-company-server.com")] + [InlineData("https", "github.my.company.server.com", "https://github.my.company.server.com")] + [InlineData("https", "GitHub.My.Company.Server.Com", "https://github.my.company.server.com")] + [InlineData("https", "gist.github.my.company.server.com", "https://github.my.company.server.com")] + [InlineData("https", "GiST.GitHub.My.Company.Server.Com", "https://github.my.company.server.com")] + public void GitHubHostProvider_GetCredentialServiceUrl(string protocol, string host, string expectedService) { - Uri uri = new Uri(uriString); - var input = new InputArguments(new Dictionary { - ["protocol"] = uri.Scheme, - ["host"] = uri.Host, + ["protocol"] = protocol, + ["host"] = host, }); - // Ensure nothing got lost during transformation - Assert.Equal(uriString.ToLower(), input.Protocol + "://" + input.Host); - var provider = new GitHubHostProvider(new TestCommandContext()); Assert.Equal(expectedService, provider.GetServiceName(input)); }