Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix OAuthClient to work correctly with GitHub Enterprise instances #1726

Merged
merged 6 commits into from
Jan 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 38 additions & 3 deletions Octokit.Tests/Clients/OauthClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -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()
{
Expand Down
17 changes: 17 additions & 0 deletions Octokit.Tests/Helpers/UriExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
6 changes: 4 additions & 2 deletions Octokit/Clients/OAuthClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
8 changes: 3 additions & 5 deletions Octokit/Helpers/ApiUrls.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -2164,7 +2162,7 @@ public static Uri User(string login)
/// <returns></returns>
public static Uri OauthAuthorize()
{
return _oauthAuthorize;
return "login/oauth/authorize".FormatUri();
}

/// <summary>
Expand All @@ -2173,7 +2171,7 @@ public static Uri OauthAuthorize()
/// <returns></returns>
public static Uri OauthAccessToken()
{
return _oauthAccessToken;
return "login/oauth/access_token".FormatUri();
}

/// <summary>
Expand Down
24 changes: 24 additions & 0 deletions Octokit/Helpers/UriExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down