From 206334cae01d8bc0f1aa13205b385c1369b4e255 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Oct 2020 12:25:29 +0000 Subject: [PATCH 1/9] Add tests for IsGitHubApiException --- .../ApiExceptionExtensionsTests.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs diff --git a/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs b/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs new file mode 100644 index 0000000000..6b20304314 --- /dev/null +++ b/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs @@ -0,0 +1,31 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using Octokit; +using NSubstitute; +using NUnit.Framework; +using GitHub.Extensions; + +public class ApiExceptionExtensionsTests +{ + public class TheIsGitHubApiExceptionMethod + { + [TestCase("Not-GitHub-Request-Id", false)] + [TestCase("X-GitHub-Request-Id", true)] + public void NoGitHubRequestId(string key, bool expect) + { + var ex = CreateApiException(new Dictionary { { key, "ANYTHING" } }); + + var result = ApiExceptionExtensions.IsGitHubApiException(ex); + + Assert.That(result, Is.EqualTo(expect)); + } + + static ApiException CreateApiException(Dictionary headers) + { + var response = Substitute.For(); + response.Headers.Returns(headers.ToImmutableDictionary()); + var ex = new ApiException(response); + return ex; + } + } +} From 21b916f43617ec2c711428c06ca0c285e5ba50cc Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Oct 2020 12:26:22 +0000 Subject: [PATCH 2/9] Add test for x-github-request-id --- test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs b/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs index 6b20304314..c8899e2818 100644 --- a/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs +++ b/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs @@ -11,6 +11,7 @@ public class TheIsGitHubApiExceptionMethod { [TestCase("Not-GitHub-Request-Id", false)] [TestCase("X-GitHub-Request-Id", true)] + [TestCase("x-github-request-id", true)] public void NoGitHubRequestId(string key, bool expect) { var ex = CreateApiException(new Dictionary { { key, "ANYTHING" } }); From 7ec64ff5d66e5030c6e4e54a999b81041bd67e97 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Oct 2020 12:52:45 +0000 Subject: [PATCH 3/9] Ignore case of X-GitHub-Request-Id --- src/GitHub.Exports/ExceptionExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/GitHub.Exports/ExceptionExtensions.cs b/src/GitHub.Exports/ExceptionExtensions.cs index a9f54c6eff..4afbb3011a 100644 --- a/src/GitHub.Exports/ExceptionExtensions.cs +++ b/src/GitHub.Exports/ExceptionExtensions.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using Octokit; namespace GitHub.Extensions @@ -9,8 +10,7 @@ public static class ApiExceptionExtensions public static bool IsGitHubApiException(this Exception ex) { var apiex = ex as ApiException; - return apiex?.HttpResponse?.Headers.ContainsKey(GithubHeader) ?? false; + return apiex?.HttpResponse?.Headers.Keys.Contains(GithubHeader, StringComparer.OrdinalIgnoreCase) ?? false; } } - } From 67fc879a972441132893f743a825c7a5f0e99306 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Oct 2020 14:17:25 +0000 Subject: [PATCH 4/9] Add test when IResponse not set --- .../ApiExceptionExtensionsTests.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs b/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs index c8899e2818..90133a3673 100644 --- a/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs +++ b/test/GitHub.Exports.UnitTests/ApiExceptionExtensionsTests.cs @@ -20,6 +20,16 @@ public void NoGitHubRequestId(string key, bool expect) Assert.That(result, Is.EqualTo(expect)); } + + [Test] + public void NoResponse() + { + var ex = new ApiException(); + + var result = ApiExceptionExtensions.IsGitHubApiException(ex); + + Assert.That(result, Is.EqualTo(false)); + } static ApiException CreateApiException(Dictionary headers) { From 87b5ac837d62200c997ad843560ef0da6408d007 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 27 Oct 2020 17:47:30 +0000 Subject: [PATCH 5/9] Case insensitive X-GitHub-Request-Id --- submodules/octokit.net | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submodules/octokit.net b/submodules/octokit.net index a51818a7e0..a25a399ab7 160000 --- a/submodules/octokit.net +++ b/submodules/octokit.net @@ -1 +1 @@ -Subproject commit a51818a7e0879ae21aacbbfd48e62dcb172d68b4 +Subproject commit a25a399ab7994ff1cc230dac0cc07e31f3fafd1f From acf6a88e5a8a1d58fff52b7e52d7ed02407d1bcd Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 2 Nov 2020 15:54:49 +0000 Subject: [PATCH 6/9] Add test for lowercase "X-OAuth-Scopes" header --- .../GitHub.Api.UnitTests/LoginManagerTests.cs | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/test/GitHub.Api.UnitTests/LoginManagerTests.cs b/test/GitHub.Api.UnitTests/LoginManagerTests.cs index 6e7ca4c31c..2a2fc732be 100644 --- a/test/GitHub.Api.UnitTests/LoginManagerTests.cs +++ b/test/GitHub.Api.UnitTests/LoginManagerTests.cs @@ -298,13 +298,30 @@ public void InvalidResponseScopesCauseException() Assert.ThrowsAsync(() => target.Login(host, client, "foo", "bar")); } - IGitHubClient CreateClient(User user = null, string[] responseScopes = null) + [TestCase("X-OAuth-Scopes")] + [TestCase("x-oauth-scopes")] + public void ValidResponseScopesDoesNotThrow(string scopesHeader) + { + var client = CreateClient(responseScopes: scopes, scopesHeader: scopesHeader); + client.Authorization.GetOrCreateApplicationAuthentication("id", "secret", Arg.Any()) + .Returns(CreateApplicationAuthorization("123abc")); + + var keychain = Substitute.For(); + var tfa = new Lazy(() => Substitute.For()); + var oauthListener = Substitute.For(); + + var target = new LoginManager(keychain, tfa, oauthListener, "id", "secret", scopes, scopes); + + Assert.DoesNotThrowAsync(() => target.Login(host, client, "foo", "bar")); + } + + IGitHubClient CreateClient(User user = null, string[] responseScopes = null, string scopesHeader = "X-OAuth-Scopes") { var result = Substitute.For(); var userResponse = Substitute.For>(); userResponse.HttpResponse.Headers.Returns(new Dictionary { - { "X-OAuth-Scopes", string.Join(",", responseScopes ?? scopes) } + { scopesHeader, string.Join(",", responseScopes ?? scopes) } }); userResponse.Body.Returns(user ?? new User()); result.Connection.Get(new Uri("user", UriKind.Relative), null, null).Returns(userResponse); From bc911f11aca992f778aa3d90f2c7b8e65dcf1bcc Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 2 Nov 2020 16:11:53 +0000 Subject: [PATCH 7/9] Run tests in GitHub.Api.UnitTests --- .github/workflows/main.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d6c2b968bb..ee623a7979 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -67,7 +67,8 @@ jobs: - name: Run unit tests shell: bash run: | - vstest.console /TestAdapterPath:test /Settings:test/test.runsettings \ + vstest.console /TestAdapterPath:test /Settings:test/test.runsettings \ + test/GitHub.Api.UnitTests/bin/${{ env.config }}/net46/GitHub.Api.UnitTests.dll \ test/GitHub.App.UnitTests/bin/${{ env.config }}/net46/GitHub.App.UnitTests.dll \ test/GitHub.Exports.Reactive.UnitTests/bin/${{ env.config }}/net46/GitHub.Exports.Reactive.UnitTests.dll \ test/GitHub.Exports.UnitTests/bin/${{ env.config }}/net46/GitHub.Exports.UnitTests.dll \ From 807528a32b80e6c8bffe5a7583396483cce889ce Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 2 Nov 2020 16:22:36 +0000 Subject: [PATCH 8/9] Find X-OAuth-Scopes header of any case --- src/GitHub.Api/LoginManager.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/GitHub.Api/LoginManager.cs b/src/GitHub.Api/LoginManager.cs index 228daa65ef..bdfe84c2c9 100644 --- a/src/GitHub.Api/LoginManager.cs +++ b/src/GitHub.Api/LoginManager.cs @@ -346,9 +346,14 @@ async Task GetUserAndCheckScopes(IGitHubClient client) var response = await client.Connection.Get( UserEndpoint, null, null).ConfigureAwait(false); - if (response.HttpResponse.Headers.ContainsKey(ScopesHeader)) + var scopes = response.HttpResponse.Headers + .Where(h => string.Equals(h.Key, ScopesHeader, StringComparison.OrdinalIgnoreCase)) + .Select(h => h.Value) + .FirstOrDefault(); + + if (scopes != null) { - var returnedScopes = new ScopesCollection(response.HttpResponse.Headers[ScopesHeader] + var returnedScopes = new ScopesCollection(scopes .Split(',') .Select(x => x.Trim()) .ToArray()); From bc6eb31809d4627ecd1ff948c9beb80e7e95fee6 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 2 Nov 2020 16:31:05 +0000 Subject: [PATCH 9/9] Remove trailing whitespace --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ee623a7979..c1f87134f9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -67,7 +67,7 @@ jobs: - name: Run unit tests shell: bash run: | - vstest.console /TestAdapterPath:test /Settings:test/test.runsettings \ + vstest.console /TestAdapterPath:test /Settings:test/test.runsettings \ test/GitHub.Api.UnitTests/bin/${{ env.config }}/net46/GitHub.Api.UnitTests.dll \ test/GitHub.App.UnitTests/bin/${{ env.config }}/net46/GitHub.App.UnitTests.dll \ test/GitHub.Exports.Reactive.UnitTests/bin/${{ env.config }}/net46/GitHub.Exports.Reactive.UnitTests.dll \