From 37777734e6e0e8f1db69e697185dfb8ae3b07fad Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Tue, 12 Dec 2017 23:07:06 +1000 Subject: [PATCH 1/6] Add more test cases for GetGitHubLoginUrl tests --- Octokit.Tests/Clients/OauthClientTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Octokit.Tests/Clients/OauthClientTests.cs b/Octokit.Tests/Clients/OauthClientTests.cs index 7f723b26e0..d79aab7106 100644 --- a/Octokit.Tests/Clients/OauthClientTests.cs +++ b/Octokit.Tests/Clients/OauthClientTests.cs @@ -24,12 +24,13 @@ public class TheGetGitHubLoginUrlMethod [Theory] [InlineData("https://api.github.com", "https://github.com/login/oauth/authorize?client_id=secret")] [InlineData("https://github.com", "https://github.com/login/oauth/authorize?client_id=secret")] - [InlineData("https://example.com", "https://example.com/login/oauth/authorize?client_id=secret")] - [InlineData("https://api.example.com", "https://api.example.com/login/oauth/authorize?client_id=secret")] + [InlineData("https://example.com/api/v3", "https://example.com/login/oauth/authorize?client_id=secret")] + [InlineData("https://api.example.com/any/path/really", "https://api.example.com/login/oauth/authorize?client_id=secret")] + [InlineData(null, "https://github.com/login/oauth/authorize?client_id=secret")] public void ReturnsProperAuthorizeUrl(string baseAddress, string expectedUrl) { var connection = Substitute.For(); - connection.BaseAddress.Returns(new Uri(baseAddress)); + connection.BaseAddress.Returns(baseAddress == null ? null : new Uri(baseAddress)); var client = new OauthClient(connection); var result = client.GetGitHubLoginUrl(new OauthLoginRequest("secret")); From 2988e6bd7fba53b4f3f68c2399df2ca0a308a2d4 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Tue, 12 Dec 2017 23:08:00 +1000 Subject: [PATCH 2/6] Fix enterprise authorize Urls by replacing the existing relative Uri segment (/api/v3) instead of appending to it via an extension method --- Octokit/Clients/OAuthClient.cs | 2 +- Octokit/Helpers/UriExtensions.cs | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Octokit/Clients/OAuthClient.cs b/Octokit/Clients/OAuthClient.cs index e6e17fb82c..84317f7e47 100644 --- a/Octokit/Clients/OAuthClient.cs +++ b/Octokit/Clients/OAuthClient.cs @@ -38,7 +38,7 @@ public Uri GetGitHubLoginUrl(OauthLoginRequest request) { Ensure.ArgumentNotNull(request, "request"); - return new Uri(hostAddress, ApiUrls.OauthAuthorize()) + return hostAddress.ReplaceRelativeUri(ApiUrls.OauthAuthorize()) .ApplyParameters(request.ToParametersDictionary()); } diff --git a/Octokit/Helpers/UriExtensions.cs b/Octokit/Helpers/UriExtensions.cs index 7317b9b90b..927241e3f0 100644 --- a/Octokit/Helpers/UriExtensions.cs +++ b/Octokit/Helpers/UriExtensions.cs @@ -9,6 +9,19 @@ namespace Octokit /// public static class UriExtensions { + /// + /// Returns a Uri where any existing relative Uri component is replaced with the respective value + /// eg https://example.com/some/path becomes https://example.com/replacement/path + /// + /// Base Uri + /// Relative Uri to add to the base Uri, replacing any existing relative Uri component + /// + public static Uri ReplaceRelativeUri(this Uri uri, Uri relativeUri) + { + // Prepending a forward slash to the relative Uri causes it to replace any that is existing + return new Uri(uri, "/" + relativeUri.ToString().TrimStart('/')); + } + /// /// Merge a dictionary of values with an existing /// From 30d4f6e885b7fbc09cf334dfc4aaa34365acc0cf Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Tue, 12 Dec 2017 23:08:21 +1000 Subject: [PATCH 3/6] Add tests for new ReplaceRelativeUri extension method --- Octokit.Tests/Helpers/UriExtensionsTests.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Octokit.Tests/Helpers/UriExtensionsTests.cs b/Octokit.Tests/Helpers/UriExtensionsTests.cs index 7effa276c5..1463ffb604 100644 --- a/Octokit.Tests/Helpers/UriExtensionsTests.cs +++ b/Octokit.Tests/Helpers/UriExtensionsTests.cs @@ -6,6 +6,23 @@ namespace Octokit.Tests.Helpers { public class UriExtensionsTests { + public class TheReplaceRelativeUriMethod + { + [Theory] + [InlineData("https://api.github.com", "my/new/path", "https://api.github.com/my/new/path")] + [InlineData("https://api.github.com", "/my/new/path", "https://api.github.com/my/new/path")] + [InlineData("https://example.com/api/v3", "my/new/path", "https://example.com/my/new/path")] + [InlineData("https://example.com/api/v3", "/my/new/path", "https://example.com/my/new/path")] + public void ReplacesRelativeUrisCorrectly(string baseUri, string relativeUri, string expectedUri) + { + var uri = new Uri(baseUri); + + var newUri = uri.ReplaceRelativeUri(relativeUri.FormatUri()); + + Assert.Equal(newUri.ToString(), expectedUri); + } + } + public class TheApplyParametersMethod { [Fact] From de3b60e5748bee10f1574330518466be868850bd Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Fri, 29 Dec 2017 22:50:31 +1000 Subject: [PATCH 4/6] add test for GHE URL on oauth CreateAccessToken --- Octokit.Tests/Clients/OauthClientTests.cs | 34 +++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Octokit.Tests/Clients/OauthClientTests.cs b/Octokit.Tests/Clients/OauthClientTests.cs index d79aab7106..b4155f9abf 100644 --- a/Octokit.Tests/Clients/OauthClientTests.cs +++ b/Octokit.Tests/Clients/OauthClientTests.cs @@ -94,6 +94,40 @@ public async Task PostsWithCorrectBodyAndContentType() await calledBody.ReadAsStringAsync()); } + [Fact] + public async Task PostsWithCorrectBodyAndContentTypeForGHE() + { + var responseToken = new OauthToken(null, null, null); + var response = Substitute.For>(); + response.Body.Returns(responseToken); + var connection = Substitute.For(); + connection.BaseAddress.Returns(new Uri("https://example.com/api/v3")); + Uri calledUri = null; + FormUrlEncodedContent calledBody = null; + Uri calledHostAddress = null; + connection.Post( + Arg.Do(uri => calledUri = uri), + Arg.Do(body => calledBody = body as FormUrlEncodedContent), + "application/json", + null, + Arg.Do(uri => calledHostAddress = uri)) + .Returns(_ => Task.FromResult(response)); + var client = new OauthClient(connection); + + var token = await client.CreateAccessToken(new OauthTokenRequest("secretid", "secretsecret", "code") + { + RedirectUri = new Uri("https://example.com/foo") + }); + + Assert.Same(responseToken, token); + Assert.Equal("login/oauth/access_token", calledUri.ToString()); + Assert.NotNull(calledBody); + Assert.Equal("https://example.com/", calledHostAddress.ToString()); + Assert.Equal( + "client_id=secretid&client_secret=secretsecret&code=code&redirect_uri=https%3A%2F%2Fexample.com%2Ffoo", + await calledBody.ReadAsStringAsync()); + } + [Fact] public async Task DeserializesOAuthScopeFormat() { From d14f559bf9f3deac6ce313d1f679b46842f0d41b Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Fri, 29 Dec 2017 22:50:53 +1000 Subject: [PATCH 5/6] move away from class level consts --- Octokit/Helpers/ApiUrls.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Octokit/Helpers/ApiUrls.cs b/Octokit/Helpers/ApiUrls.cs index 24120b3165..a9c18556ce 100644 --- a/Octokit/Helpers/ApiUrls.cs +++ b/Octokit/Helpers/ApiUrls.cs @@ -18,9 +18,7 @@ public static partial class ApiUrls static readonly Uri _currentUserNotificationsEndpoint = new Uri("notifications", UriKind.Relative); static readonly Uri _currentUserAllIssues = new Uri("issues", UriKind.Relative); static readonly Uri _currentUserOwnedAndMemberIssues = new Uri("user/issues", UriKind.Relative); - static readonly Uri _oauthAuthorize = new Uri("login/oauth/authorize", UriKind.Relative); - static readonly Uri _oauthAccessToken = new Uri("login/oauth/access_token", UriKind.Relative); - + /// /// Returns the that returns all public repositories in /// response to a GET request. @@ -2164,7 +2162,7 @@ public static Uri User(string login) /// public static Uri OauthAuthorize() { - return _oauthAuthorize; + return "login/oauth/authorize".FormatUri(); } /// @@ -2173,7 +2171,7 @@ public static Uri OauthAuthorize() /// public static Uri OauthAccessToken() { - return _oauthAccessToken; + return "login/oauth/access_token".FormatUri(); } /// From 218278408e9747d63a919d7fb713d8b2c6da83dc Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Fri, 29 Dec 2017 22:51:24 +1000 Subject: [PATCH 6/6] Oauth client can store stripped base URL so it works with GHE --- Octokit/Clients/OAuthClient.cs | 8 +++++--- Octokit/Helpers/UriExtensions.cs | 13 ++++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/Octokit/Clients/OAuthClient.cs b/Octokit/Clients/OAuthClient.cs index 84317f7e47..d163ae3bdd 100644 --- a/Octokit/Clients/OAuthClient.cs +++ b/Octokit/Clients/OAuthClient.cs @@ -23,10 +23,12 @@ public OauthClient(IConnection connection) this.connection = connection; var baseAddress = connection.BaseAddress ?? GitHubClient.GitHubDotComUrl; - // The Oauth login stuff uses https://github.com and not the https://api.github.com URLs. + // The Oauth login stuff uses the main website and not the API URLs + // For https://api.github.com we use https://github.com + // For any other address (presumably a GitHub Enterprise address) we need to strip any relative Uri such as /api/v3 hostAddress = baseAddress.Host.Equals("api.github.com") ? new Uri("https://github.com") - : baseAddress; + : baseAddress.StripRelativeUri(); } /// @@ -38,7 +40,7 @@ public Uri GetGitHubLoginUrl(OauthLoginRequest request) { Ensure.ArgumentNotNull(request, "request"); - return hostAddress.ReplaceRelativeUri(ApiUrls.OauthAuthorize()) + return new Uri(hostAddress, ApiUrls.OauthAuthorize()) .ApplyParameters(request.ToParametersDictionary()); } diff --git a/Octokit/Helpers/UriExtensions.cs b/Octokit/Helpers/UriExtensions.cs index 927241e3f0..7be15802ad 100644 --- a/Octokit/Helpers/UriExtensions.cs +++ b/Octokit/Helpers/UriExtensions.cs @@ -9,6 +9,17 @@ namespace Octokit /// public static class UriExtensions { + /// + /// Returns a Uri where any existing relative Uri component is stripped + /// eg https://example.com/some/path becomes https://example.com + /// + /// Base Uri + /// + public static Uri StripRelativeUri(this Uri uri) + { + return new Uri(uri, "/"); + } + /// /// Returns a Uri where any existing relative Uri component is replaced with the respective value /// eg https://example.com/some/path becomes https://example.com/replacement/path @@ -19,7 +30,7 @@ public static class UriExtensions public static Uri ReplaceRelativeUri(this Uri uri, Uri relativeUri) { // Prepending a forward slash to the relative Uri causes it to replace any that is existing - return new Uri(uri, "/" + relativeUri.ToString().TrimStart('/')); + return new Uri(StripRelativeUri(uri), relativeUri); } ///