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

Add OAuth support to generic host provider #1062

Merged
merged 10 commits into from
Feb 1, 2023

Conversation

mjcheetham
Copy link
Collaborator

@mjcheetham mjcheetham commented Jan 27, 2023

Add ability to provide OAuth-based authentication for generic hosts by way of simple Git configuration.

When a remote URL does not match any known host provider plugin, the generic provider will now first check for OAuth configuration in the Git config or environment variables. If such config is available then we try and perform OAuth authentication. Support for device code flow is optional, and refresh tokens will be used if the service supports and returns them.

Users can make use of existing Git config include to easily organise and share custom OAuth configurations:

~/.gitconfig

[include]
    path = ~/.gcm/oauth/example.ini

example.ini

[credential "https://example.com"]
	provider = generic
	oauthClientId = <client-id>
	oauthClientSecret = <client-secret>
	oauthRedirectUri = <redirect-uri>
	oauthAuthorizeEndpoint = <auth-endpoint>
	oauthTokenEndpoint = <token-endpoint>
	oauthScopes = <scopes>
	oauthDeviceEndpoint = <device-endpoint>
	oauthDefaultUserName = <username>
	oauthUseClientAuthHeader = <true|false>

image

image

Fixes #1047

Fix a bug in a view model property for the basic credentials prompt; we
should be updating the backing field and also raising the
PropertyChanged event.
@mjcheetham mjcheetham marked this pull request as ready for review January 28, 2023 00:04
@mjcheetham mjcheetham force-pushed the generic-oauth branch 2 times, most recently from 567a0a8 to 36ad480 Compare January 28, 2023 00:55
Context.Trace,
config.UseAuthHeader);

string refreshService = remoteUri.AbsoluteUri.TrimEnd('/') + "/refresh_token";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know there are some concerns about how we generate a service name for the refresh tokens with collisions with other credentials.

@hickford, you had suggested in the past using a subdomain to differentiate ATs and RTs:

https://foo.example.com/path/repo
https://refresh_token.foo.example.com/path/repo

Did you or @ldennington have any other suggestions? I was going to change to the refresh_token subdomain example above if there was no objections.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with the subdomain implementation.

@mjcheetham
Copy link
Collaborator Author

Right now there's no facility to 'test' an existing OAuth credential is valid, so we rely on the less-than-optimal flow of Git trying, failing, telling us to erase, and then expect the user to retry and then we'll use the refresh token.

In the future we may wish to allow a 'validity' endpoint to be configured where the generic implementation can make a GET request with the bearer token, and use the HTTP status code as a signal for validity (200 = OK, 401/403 => invalid).

Copy link
Contributor

@ldennington ldennington left a comment

Choose a reason for hiding this comment

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

This is awesome!! 🚀

src/shared/Core/GenericOAuthConfig.cs Show resolved Hide resolved
@@ -128,6 +128,33 @@ private async Task<ICredential> GetOAuthAccessToken(Uri remoteUri, string userNa
Context.Trace,
config.UseAuthHeader);

//
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Is this line needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've used this pattern in other places in the code base to visually make a block of explanatory comments standout more.

docs/generic-oauth.md Outdated Show resolved Hide resolved
docs/generic-oauth.md Outdated Show resolved Hide resolved
Teach the Generic host provider to read configuration for OAuth-based
authentication. These are largely parameters required for the
OAuth2Client to be constructed including Client ID/Secret, Redirect URI
and Scopes.
Add OAuth support for the generic provider offering browser (authcode
grant) and device code (device auth grant) support. Device code
and mode selection is initially only offered for TTY users.
Add support for storing and using OAuth refresh tokens.
Prepend "refresh_token" as a subdomain to give better chances of
avoiding a name clash compared with appending "/refresh_token" to the
path component.
Add shared view models and commands for the OAuth GUI prompts.

Two new commands and VMs are added, one for the initial 'mode'
selection, and another to display the device code.
Add AvaloniaUI based implementations of the OAuth and Device Code
generic UI prompts.
Add the WPF based GUI implementation of the OAuth and device code
generic prompts.
Add detection and use of GUI prompts for OAuth authentication.
@ANGGBELPAS
Copy link

Duplicate of #789

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.

Support any OAuth server with custom config
3 participants