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

Change redirect URI from localhost to 127.0.0.1 #594

Closed
hickford opened this issue Dec 31, 2021 · 11 comments · Fixed by #1286
Closed

Change redirect URI from localhost to 127.0.0.1 #594

hickford opened this issue Dec 31, 2021 · 11 comments · Fixed by #1286
Assignees
Labels
enhancement New feature or request external Relating to an external partner, team, or library host:github Specific to the GitHub host provider
Milestone

Comments

@hickford
Copy link
Contributor

hickford commented Dec 31, 2021

The OAuth spec https://datatracker.ietf.org/doc/html/rfc8252#section-8.3 recommends to use 127.0.0.1 instead of localhost:

While redirect URIs using localhost (i.e., "http://localhost:{port}/{path}") function similarly to loopback IP redirects described in Section 7.3, the use of localhost is NOT RECOMMENDED. Specifying a redirect URI with the loopback IP literal rather than localhost avoids inadvertently listening on network interfaces other than the loopback interface. It is also less susceptible to client-side firewalls and misconfigured host name resolution on the user's device.

Examples in code -- BitBucket, GitHub, Azure https://github.com/GitCredentialManager/git-credential-manager/search?q=localhost

@hickford hickford added the enhancement New feature or request label Dec 31, 2021
@hickford
Copy link
Contributor Author

hickford commented Jan 5, 2022

NB. This would require changing the redirect URI in the OAuth app settings on GitHub etc. https://docs.github.com/en/developers/apps/building-oauth-apps/authorizing-oauth-apps#redirect-urls

Since GitHub allows at most one URI, I can't see how to do this in a backwards compatible way.

@mjcheetham
Copy link
Collaborator

NB. This would require changing the redirect URI in the OAuth app settings on GitHub etc. https://docs.github.com/en/developers/apps/building-oauth-apps/authorizing-oauth-apps#redirect-urls

You are sadly correct here. I was made aware that we should probably be using the loopback IP address rather than localhost for our redirect URI, but we cannot make that change without breaking older clients 😢

The only other option (in lieu of GitHub adding support for multiple redirect URIs) is to create a new OAuth app with the new redirect URI.. however this just adds more complexity to the management and consent of two separate applications.

@mjcheetham mjcheetham added host:github Specific to the GitHub host provider external Relating to an external partner, team, or library labels Jan 6, 2022
@mislav
Copy link

mislav commented Feb 8, 2022

For GitHub, instead of redirecting back to either 127.0.0.1 or localhost, GCM should switch to implementing the Device Authorization Flow.

The benefits are that the completed browser flow does not have to redirect to any particular URL (and thus GCM doesn't have to spawn a web server at localhost), and the user is able to complete the browser flow on a different machine (e.g. on their smartphone) than the one where GCM is running as a process.

@hickford
Copy link
Contributor Author

hickford commented Feb 8, 2022

@mislav GCM already supports OAuth Device Authorization Grant for GitHub, This works great on my headless Rapberry Pi:

Select an authentication method for 'https://github.com/':  
  1. Web browser (default)  
  2. Personal access token  
 option (enter for default):                                                                                                                
 To complete authentication please visit https://github.com/login/device and enter the following code:  
 ABCD-1234

If you can't see the option, you could try forcing it with preference credential.gitHubAuthModes.

@mislav
Copy link

mislav commented Feb 8, 2022

@hickford Thanks for the info! I was able to opt into Device flow by doing this:

git config --global credential.gitHubAuthModes device

My next question is, why hasn't GCM auto-detected this when authorizing a git operation from https://github.com, but instead it defaulted to OAuth web app flow? I would argue that the Device flow should be the default and that it's straightforward to feature-detect.

@hickford
Copy link
Contributor Author

hickford commented Feb 8, 2022

@mislav what version are you using? Update to https://github.com/GitCredentialManager/git-credential-manager/releases/tag/v2.0.567 or later to get #478 "Add explicit GitHub device code authentication option"

My Raspberry Pi has an older version "2.0.498+7ad55fb809" hence 'device code' isn't shown as a separate option.

@mislav
Copy link

mislav commented Feb 8, 2022

@hickford I have 2.0.632+478753bbb7 and I do see that my version supports the explicit option of choosing Device flow (as per my previous comment). At the risk of derailing the original topic in this thread, my question is whether I should expect that Device flow is auto-detected as a default authentication method for GitHub.com when nothing is set as credential.gitHubAuthModes (i.e. taking precedence over web app flow). Should I open another thread to ask this in?

@hickford
Copy link
Contributor Author

hickford commented Feb 8, 2022

@mislav Good idea. With a recent version on a desktop machine I get the following choice:

Select an authentication method for 'https://github.com/':
  1. Web browser (default)
  2. Device code
  3. Personal access token

Whether you see the 'web browser' option depends on the DISPLAY environment variable https://github.com/GitCredentialManager/git-credential-manager/blob/main/src/shared/Core/Interop/Posix/PosixSessionManager.cs

@mislav
Copy link

mislav commented Feb 21, 2022

Whether you see the 'web browser' option depends on the DISPLAY environment variable

Ah, I see! DISPLAY isn't set for me on macOS, where I would guess that this variable is never set by default. Nevertheless, I think that maybe GCM could assume that macOS always has desktop capability, since it should be pretty uncommon that developers run macOS in headless mode.

Going back to the original topic: in GitHub CLI, which also used to authorize over OAuth web application flow, we've migrated from http://localhost callback to http://127.0.0.1 in a way that's backwards-compatible, using the same OAuth app. This relied on a feature that's not public. If you'd want help with that, I'm sure we could extend the feature to your OAuth app as well: just email me at mislav@github.com.

However, it's even a better idea to default to device flow whenever possible instead of the web application flow. That's why I was talking so much about device flow in this thread; sure, you could spend time improving the HTTP redirect experience, but a much better long-term solution is to avoid the local server redirect altogether.

@mjcheetham
Copy link
Collaborator

My next question is, why hasn't GCM auto-detected this when authorizing a git operation from https://github.com, but instead it defaulted to OAuth web app flow? I would argue that the Device flow should be the default and that it's straightforward to feature-detect.

GCM is detecting this is GitHub.com and should be presenting all possible auth options available (interactive via browser, manually generated PAT, OAuth device code flow).

At the risk of derailing the original topic in this thread, my question is whether I should expect that Device flow is auto-detected as a default authentication method for GitHub.com when nothing is set as credential.gitHubAuthModes (i.e. taking precedence over web app flow). Should I open another thread to ask this in?

When nothing is set for credential.gitHubAuthModes we show a prompt asking the user to pick the flow that suits them.

Whether you see the 'web browser' option depends on the DISPLAY environment variable https://github.com/GitCredentialManager/git-credential-manager/blob/main/src/shared/Core/Interop/Posix/PosixSessionManager.cs
...
Ah, I see! DISPLAY isn't set for me on macOS, where I would guess that this variable is never set by default

We actually use GetProcessWindowStation() and GetUserObjectInformation() on Windows, and SessionGetInfo() on macOS to detect the presence of a GUI environment. On other POSIX operating systems (e.g. Linux), we use the presence of the $DISPLAY variable.

Nevertheless, I think that maybe GCM could assume that macOS always has desktop capability, since it should be pretty uncommon that developers run macOS in headless mode.

That's not always the case 😉(!) Developers should be allowed to SSH in to a macOS machine, imagine SSH-ing in to a headless Mac mini build machine for example (I have done this myself in previous job!). Also think of something like VS Code SSH remote sessions.

Unlike the GitHub CLI, GCM needs to support and present GUI prompts as well as terminal-prompts. IDEs, tools and editors like SourceTree or VS invoke the git executable directly to provider source control. Git can then invoke GCM with no terminal available, and therefore no way for a user to respond to the prompt(!)

[..] (i.e. taking precedence over web app flow). Should I open another thread to ask this in?
...
However, it's even a better idea to default to device flow whenever possible instead of the web application flow.

Yes, this might be better explored in a different issue. Right now we offer all possible options to the user, and the default selection is to use the browser/local-redirect.

[..] in GitHub CLI, which also used to authorize over OAuth web application flow, we've migrated from http://localhost callback to http://127.0.0.1 in a way that's backwards-compatible, using the same OAuth app. This relied on a feature that's not public. If you'd want help with that, I'm sure we could extend the feature to your OAuth app as well: just email me at mislav@github.com.

That's an interesting development! I will reach out.

@keanenwold keanenwold added the gcm label Nov 17, 2022
@ldennington ldennington added this to the Future milestone Feb 13, 2023
@ldennington ldennington modified the milestones: Future, Git 2.42 May 22, 2023
@mjcheetham mjcheetham self-assigned this May 22, 2023
mjcheetham added a commit that referenced this issue Jun 8, 2023
The OAuth 2.0 spec requires that redirect URLs be matched _exactly_ if
specified, including matching trailing slashes.

Since the .NET `Uri` type's `.ToString()` method will append a trailing
slash to the end of path-less URLs (e.g., "http://foo" => "http://foo/")
we need to use the `.OriginalString` property instead.

Shoring up this area in anticipation for changes to support multiple
GitHub redirect URLs with #594
mjcheetham added a commit that referenced this issue Jun 12, 2023
Use an IPv4 loopback redirect URL instead of the `localhost` name. This
is in accordance with the recommendation in the OAuth spec[^1][^2] and
also GitHub's documentation[^3].

Note that this change depends on an update to the Git Credential Manager
OAuth application on GitHub to add the "http://127.0.0.1/" redirect
(with a trailing slash!). We will be strictly adding the new URL, and
keep the older localhost-based redirect URL untouched for older clients.

The change to the OAuth app registration can occur before this is
merged.

Fixes #594 

[^1]: https://datatracker.ietf.org/doc/html/rfc8252#section-7.3
[^2]: https://datatracker.ietf.org/doc/html/rfc8252#section-8.3
[^3]:
https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps#loopback-redirect-urls
@mjcheetham
Copy link
Collaborator

The main branch and OAuth app registration have both been updated with the new 127.0.0.1 redirect URI (also maintaining the existing localhost URI for older clients).

The next release of GCM will now be using the IP loopback address!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request external Relating to an external partner, team, or library host:github Specific to the GitHub host provider
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants