Add explicit GitHub device code authentication option #478
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rename the OAuth GitHub authentication mode to Browser to better match what interaction will happen when selecting this mode. Also prepare for the introduction of an explicit device code OAuth flow mode.
Consolidate all the UI helper options for the GitHub `prompt` view in to an `Options` class (rather than bloat the execution handler with more arguments). Also fix a bug whereby the GitHub host provider would attempt to invoke the UI helper with an `--all` option when all authentication modes are available, but no such option is available! This option was only available on the old WPF UI helper, and was never carried forward to the Avalonia UI.
mjcheetham
requested review from
vtbassmatt,
ldennington,
jeffhostetler,
derrickstolee and
vdye
Implement an explicit OAuth device code authentication mode for the GitHub host provider. Previously the 'web browser' option combined both the interactive/browser based flow (when a UI was present), and a TTY-based device code flow. This change allows users to select the device code flow even when they have a desktop/UI session present. This is useful to workaround possible problems with the browser loopback/redirect mechanism.
Add the device code option to the Avalonia and WPF helper UI applications for GitHub. This includes adding a new explicit "device code" button to the primary authentication prompt view, and a new device code display prompt.
Update documentation to include the new `browser` and `device` authentication modes for GitHub. The old `oauth` value expands to both `browser, device` for compatibility.
mjcheetham
force-pushed
the
gh-device-ui
branch
from
f3ea92b to
0fb32aa
Compare
derrickstolee
approved these changes
Oct 6, 2021
| @@ -121,7 +121,7 @@ public async Task GitHubHostProvider_GetSupportedAuthenticationModes(string uriS | |||
| [Theory] | |||
| [InlineData("https://example.com", null, "0.1", false, AuthenticationModes.Pat)] | |||
| [InlineData("https://example.com", null, "0.1", true, AuthenticationModes.Basic | AuthenticationModes.Pat)] | |||
| [InlineData("https://example.com", null, "100.0", false, AuthenticationModes.Browser | AuthenticationModes.Pat)] | |||
| [InlineData("https://example.com", null, "100.0", false, AuthenticationModes.OAuth | AuthenticationModes.Pat)] | |||
There was a problem hiding this comment.
This seems like an errant revert of a previous change.
There was a problem hiding this comment.
Oh, I see. OAuth is the combination of two. Should we have lines for Browser, Device, and OAuth on this test?
There was a problem hiding this comment.
One change is to rename the current (browser) behaviour to Browser, to better reflect what that mode does.
The next change is to add a new Device mode, and reuse the newly "freed" option of OAuth to represent the combination of Browser and Device.
This test checks that a GHES instance with version number 100.0 should support both Browser and Device(i.e, OAuth).
| @@ -196,7 +196,7 @@ public async Task GitHubHostProvider_GenerateCredentialAsync_Browser_ReturnsCred | |||
| ghAuthMock.Setup(x => x.GetAuthenticationAsync(expectedTargetUri, null, It.IsAny<AuthenticationModes>())) | |||
| .ReturnsAsync(new AuthenticationPromptResult(AuthenticationModes.Browser)); | |||
|
|
|||
| ghAuthMock.Setup(x => x.GetOAuthTokenAsync(expectedTargetUri, It.IsAny<IEnumerable<string>>())) | |||
| ghAuthMock.Setup(x => x.GetOAuthTokenViaBrowserAsync(expectedTargetUri, It.IsAny<IEnumerable<string>>())) | |||
There was a problem hiding this comment.
In the message for this commit, you say
This is useful to workaround possible
problems with the browser loopback/redirect mechanism.
Do you have an example? I'm guessing WSL has something to do with this.
There was a problem hiding this comment.
This is indeed one, but the one that jumps to mind is problems we've had with:
- AVs blocking loopback listening
- enterprises that have registered
localhostas a valid name with their DNS nameservers (yes, it has happened!!) - There are some random 'failures' that other teams have not been able to diagnose
See these issues the GitHub VS Extension has had problems with the loopback mechanism before:
Sign in with browsernot working - things to try github/VisualStudio#2568- Redirect to 127.0.0.1 instead of localhost github/VisualStudio#2554
- Unable to connect to GitHub from VS2019 github/VisualStudio#2550
- unable to connect to GitHub using visual studio 2019 github/VisualStudio#2527
- Github "Sign in with browser" open localhost in browser, login fails github/VisualStudio#2521
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement an explicit OAuth device code authentication mode for the GitHub host provider. Previously the 'web browser' option combined both the interactive/browser based flow (when a UI was present), and a TTY-based device code flow.
This change allows users to select the device code flow even when they have a desktop/UI session present. This is useful to workaround possible problems with the browser loopback/redirect mechanism.
As well as introducing new
"browser"and"device"values, we keep the existing"oauth"value for theGCM_GITHUB_AUTHMODESenvironment variable andcredential.gitHubAuthModesGit config, and let it expand to bothbrowseranddeviceto allow users who may have set the older value before.(macOS)
(Ubuntu)
(Windows)