Fix OAuthClient to work correctly with GitHub Enterprise instances (#…

…1726)

* Add more test cases for GetGitHubLoginUrl tests

* Fix enterprise authorize Urls by replacing the existing relative Uri segment (/api/v3) instead of appending to it via an extension method

* Add tests for new ReplaceRelativeUri extension method

* add test for GHE URL on oauth CreateAccessToken

* move away from class level consts

* Oauth client can store stripped base URL so it works with GHE

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<IConnection>();
-            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"));
@@ -93,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<IApiResponse<OauthToken>>();
+            response.Body.Returns(responseToken);
+            var connection = Substitute.For<IConnection>();
+            connection.BaseAddress.Returns(new Uri("https://example.com/api/v3"));
+            Uri calledUri = null;
+            FormUrlEncodedContent calledBody = null;
+            Uri calledHostAddress = null;
+            connection.Post<OauthToken>(
+                Arg.Do<Uri>(uri => calledUri = uri),
+                Arg.Do<object>(body => calledBody = body as FormUrlEncodedContent),
+                "application/json",
+                null,
+                Arg.Do<Uri>(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()
         {

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]

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();
         }
 
         /// <summary>

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);
-
+
         /// <summary>
         /// Returns the <see cref="Uri"/> that returns all public repositories in
         /// response to a GET request.
@@ -2164,7 +2162,7 @@ public static Uri User(string login)
         /// <returns></returns>
         public static Uri OauthAuthorize()
         {
-            return _oauthAuthorize;
+            return "login/oauth/authorize".FormatUri();
         }
 
         /// <summary>
@@ -2173,7 +2171,7 @@ public static Uri OauthAuthorize()
         /// <returns></returns>
         public static Uri OauthAccessToken()
         {
-            return _oauthAccessToken;
+            return "login/oauth/access_token".FormatUri();
         }
 
         /// <summary>

Octokit/Helpers/UriExtensions.cs

@@ -9,6 +9,30 @@ namespace Octokit
     /// </summary>
     public static class UriExtensions
     {
+        /// <summary>
+        /// Returns a Uri where any existing relative Uri component is stripped
+        /// eg https://example.com/some/path becomes https://example.com
+        /// </summary>
+        /// <param name="uri">Base Uri</param>
+        /// <returns></returns>
+        public static Uri StripRelativeUri(this Uri uri)
+        {
+            return new Uri(uri, "/");
+        }
+
+        /// <summary>
+        /// 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
+        /// </summary>
+        /// <param name="uri">Base Uri</param>
+        /// <param name="relativeUri">Relative Uri to add to the base Uri, replacing any existing relative Uri component</param>
+        /// <returns></returns>
+        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(StripRelativeUri(uri), relativeUri);
+        }
+
         /// <summary>
         /// Merge a dictionary of values with an existing <see cref="Uri"/>
         /// </summary>