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

Support third-party OAuth hosts #7

Merged
merged 2 commits into from Oct 15, 2021
Merged

Support third-party OAuth hosts #7

merged 2 commits into from Oct 15, 2021

Conversation

jamierocks
Copy link
Contributor

@jamierocks jamierocks commented Apr 3, 2021

This pull request should make some headway to supporting further OAuth providers than just GitHub. This pull request doesn't introduce specific support for anything, but provides an API to allow third-parties to their flavour endpoints.


Not all OAuth hosts use the same routes as GitHub, for example:

  • Microsoft use /oauth2/v2.0/devicecode
  • Google use /device/code
  • Auth0 use /oauth/device/code

Similar differences are present for the authorise and access token
routes too.

This commit introduces a concept of a Server, which is a container for
the endpoints that the library uses. This is a replacement for Flow's
Hostname and as such is a breaking change.

Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this flexibility; thanks for proposing! But let's keep it backwards-compatible.

Would just modifying the URLs allow this library to work with Microsoft without additional changes? See #3

server.go Outdated Show resolved Hide resolved
examples_test.go Outdated Show resolved Hide resolved
@jamierocks
Copy link
Contributor Author

My original intention wasn't for this to be backwards-compatible, but looking back now its absolutely achievable - so I'll make an update to handle that.

Would just modifying the URLs allow this library to work with Microsoft without additional changes?

No. GitHub's server implementation is non-standard and responds with application/x-www-form-urlencoded responses which no other provider I'm aware of does. Never-the-less supporting Microsoft's endpoints is my end-goal here too, and it's not too dificult to support application/json responses.

I did make a start on this effort locally, with my original intent being to submit a second pr - those changes make sense under this pr, so I may just finish that up and push it here :)

@jamierocks
Copy link
Contributor Author

I will look at the tests in a bit, but I've verified that the changes now support Microsoft's endpoints :)

Would you be interested in adding something like

// ServerAzure gets a Server for the given Azure Active Directory tenant. If tenant is
// empty, it will default to "common".
//
// See https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-device-code
func ServerAzure(tenant string) *Server {
	if tenant == "" {
		tenant = "common"
	}

	return &Server{
		DeviceCodeURL: fmt.Sprintf("https://login.microsoftonline.com/%s/oauth2/v2.0/devicecode", tenant),
		AuthorizeURL:  fmt.Sprintf("https://login.microsoftonline.com/%s/oauth2/v2.0/authorize", tenant),
		TokenURL:      fmt.Sprintf("https://login.microsoftonline.com/%s/oauth2/v2.0/token", tenant),
	}
}

@jamierocks jamierocks force-pushed the server branch 2 times, most recently from 7d51c27 to ec550ca Compare April 7, 2021 11:53
@jamierocks jamierocks changed the title Allow for arbitrary OAuth hosts Support third-party OAuth hosts Apr 7, 2021
@jamierocks jamierocks requested a review from mislav April 14, 2021 17:30
values: map[string]string{
"access_token": "ATOKEN",
"token_type": "bearer",
"scope": "repo gist",
},
},
want: &AccessToken{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gh pr checkout 7

api/form_test.go Outdated
@@ -173,7 +173,7 @@ func TestPostForm(t *testing.T) {
want: &FormResponse{
StatusCode: 502,
requestURI: "https://github.com/oauth",
values: url.Values(nil),
values: make(map[string]string),
},
wantErr: false,
},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

api/form.go

server.go Outdated
AuthorizeURL: fmt.Sprintf("https://%s/login/oauth/authorize", host),
TokenURL: fmt.Sprintf("https://%s/login/oauth/access_token", host),
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server.go

jamierocks and others added 2 commits October 15, 2021 19:18
Not all OAuth hosts use the same routes as GitHub, for example:
- Microsoft use /oauth2/v2.0/devicecode
- Google use /device/code
- Auth0 use /oauth/device/code

Similar differences are present for the authorise and access token
routes too.

This commit introduces a concept of a Host, which is a container for
the endpoints that the library uses. After this, the Hostname field is
deprecated.

Co-authored-by: Mislav Marohnić <mislav@github.com>
Co-authored-by: Mislav Marohnić <mislav@github.com>
Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience! I've pushed some cleanups/renames.

@mislav mislav merged commit ce77fe6 into cli:main Oct 15, 2021
tonymreyes537 pushed a commit to tonymreyes537/oauth that referenced this pull request Dec 31, 2021
Support third-party OAuth hosts

Signed-off-by: antonio de jesus medina <tonymreyes537@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants