Add login hint support for GitHub browser-based OAuth authentication

src/shared/Atlassian.Bitbucket/BitbucketOAuth2Client.cs

@@ -10,7 +10,7 @@ namespace Atlassian.Bitbucket
 {
     public abstract class BitbucketOAuth2Client : OAuth2Client
     {
-        public BitbucketOAuth2Client(HttpClient httpClient, OAuth2ServerEndpoints endpoints, string clientId, Uri redirectUri, string clientSecret, ITrace trace) : base(httpClient, endpoints, clientId, redirectUri, clientSecret, trace, false)
+        public BitbucketOAuth2Client(HttpClient httpClient, OAuth2ServerEndpoints endpoints, string clientId, Uri redirectUri, string clientSecret, ITrace trace) : base(httpClient, endpoints, clientId, redirectUri, clientSecret, false)
         {
         }
 
@@ -27,9 +27,9 @@ public string GetRefreshTokenServiceName(InputArguments input)
             return uri.AbsoluteUri.TrimEnd('/');
         }
 
-        public Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(IOAuth2WebBrowser browser, CancellationToken ct) 
+        public Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(IOAuth2WebBrowser browser, CancellationToken ct)
         {
-            return GetAuthorizationCodeAsync(Scopes, browser, ct);
+            return this.GetAuthorizationCodeAsync(Scopes, browser, ct);
         }
 
         protected override bool TryCreateTokenEndpointResult(string json, out OAuth2TokenResult result)

src/shared/Core.Tests/Authentication/OAuth2ClientTests.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Net.Http;
 using System.Threading;
 using System.Threading.Tasks;
@@ -37,11 +38,90 @@ public async Task OAuth2Client_GetAuthorizationCodeAsync()
 
             OAuth2Client client = CreateClient(httpHandler, endpoints);
 
-            OAuth2AuthorizationCodeResult result = await client.GetAuthorizationCodeAsync(expectedScopes, browser, CancellationToken.None);
+            OAuth2AuthorizationCodeResult result = await client.GetAuthorizationCodeAsync(expectedScopes, browser, null, CancellationToken.None);
 
             Assert.Equal(expectedAuthCode, result.Code);
         }
 
+        [Fact]
+        public async Task OAuth2Client_GetAuthorizationCodeAsync_ExtraQueryParams()
+        {
+            const string expectedAuthCode = "68c39cbd8d";
+
+            var baseUri = new Uri("https://example.com");
+            OAuth2ServerEndpoints endpoints = CreateEndpoints(baseUri);
+
+            var httpHandler = new TestHttpMessageHandler {ThrowOnUnexpectedRequest = true};
+
+            string[] expectedScopes = {"read", "write", "delete"};
+
+            var extraParams = new Dictionary<string, string>
+            {
+                ["param1"] = "value1",
+                ["param2"] = "value2",
+                ["param3"] = "value3"
+            };
+
+            OAuth2Application app = CreateTestApplication();
+
+            var server = new TestOAuth2Server(endpoints);
+            server.RegisterApplication(app);
+            server.Bind(httpHandler);
+            server.TokenGenerator.AuthCodes.Add(expectedAuthCode);
+
+            server.AuthorizationEndpointInvoked += (_, request) =>
+            {
+                IDictionary<string, string> actualParams = request.RequestUri.GetQueryParameters();
+                foreach (var expected in extraParams)
+                {
+                    Assert.True(actualParams.TryGetValue(expected.Key, out string actualValue));
+                    Assert.Equal(expected.Value, actualValue);
+                }
+            };
+
+            IOAuth2WebBrowser browser = new TestOAuth2WebBrowser(httpHandler);
+
+            OAuth2Client client = CreateClient(httpHandler, endpoints);
+
+            OAuth2AuthorizationCodeResult result = await client.GetAuthorizationCodeAsync(expectedScopes, browser, extraParams, CancellationToken.None);
+
+            Assert.Equal(expectedAuthCode, result.Code);
+        }
+
+        [Fact]
+        public async Task OAuth2Client_GetAuthorizationCodeAsync_ExtraQueryParams_OverrideStandardArgs_ThrowsException()
+        {
+            const string expectedAuthCode = "68c39cbd8d";
+
+            var baseUri = new Uri("https://example.com");
+            OAuth2ServerEndpoints endpoints = CreateEndpoints(baseUri);
+
+            var httpHandler = new TestHttpMessageHandler {ThrowOnUnexpectedRequest = true};
+
+            string[] expectedScopes = {"read", "write", "delete"};
+
+            var extraParams = new Dictionary<string, string>
+            {
+                ["param1"] = "value1",
+                [OAuth2Constants.ClientIdParameter] = "value2",
+                ["param3"] = "value3"
+            };
+
+            OAuth2Application app = CreateTestApplication();
+
+            var server = new TestOAuth2Server(endpoints);
+            server.RegisterApplication(app);
+            server.Bind(httpHandler);
+            server.TokenGenerator.AuthCodes.Add(expectedAuthCode);
+
+            IOAuth2WebBrowser browser = new TestOAuth2WebBrowser(httpHandler);
+
+            OAuth2Client client = CreateClient(httpHandler, endpoints);
+
+            await Assert.ThrowsAsync<ArgumentException>(() =>
+                client.GetAuthorizationCodeAsync(expectedScopes, browser, extraParams, CancellationToken.None));
+        }
+
         [Fact]
         public async Task OAuth2Client_GetDeviceCodeAsync()
         {
@@ -217,7 +297,7 @@ public async Task OAuth2Client_E2E_InteractiveWebFlowAndRefresh()
             OAuth2Client client = CreateClient(httpHandler, endpoints);
 
             OAuth2AuthorizationCodeResult authCodeResult = await client.GetAuthorizationCodeAsync(
-                expectedScopes, browser, CancellationToken.None);
+                expectedScopes, browser, null, CancellationToken.None);
 
             OAuth2TokenResult result1 = await client.GetTokenByAuthorizationCodeAsync(authCodeResult, CancellationToken.None);
 

src/shared/Core/Authentication/OAuth/OAuth2Client.cs

@@ -20,9 +20,15 @@ public interface IOAuth2Client
         /// </summary>
         /// <param name="scopes">Scopes to request.</param>
         /// <param name="browser">User agent to use to start the authorization code grant flow.</param>
+        /// <param name="extraQueryParams">Extra parameters to add to the URL query component.</param>
         /// <param name="ct">Token to cancel the operation.</param>
         /// <returns>Authorization code.</returns>
-        Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(IEnumerable<string> scopes, IOAuth2WebBrowser browser, CancellationToken ct);
+        Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(
+            IEnumerable<string> scopes,
+            IOAuth2WebBrowser browser,
+            IDictionary<string, string> extraQueryParams,
+            CancellationToken ct
+        );
 
         /// <summary>
         /// Retrieve a device code grant.
@@ -65,19 +71,17 @@ public class OAuth2Client : IOAuth2Client
         private readonly Uri _redirectUri;
         private readonly string _clientId;
         private readonly string _clientSecret;
-        private readonly ITrace _trace;
         private readonly bool _addAuthHeader;
 
         private IOAuth2CodeGenerator _codeGenerator;
 
-        public OAuth2Client(HttpClient httpClient, OAuth2ServerEndpoints endpoints, string clientId, Uri redirectUri = null, string clientSecret = null, ITrace trace = null, bool addAuthHeader = true)
+        public OAuth2Client(HttpClient httpClient, OAuth2ServerEndpoints endpoints, string clientId, Uri redirectUri = null, string clientSecret = null, bool addAuthHeader = true)
         {
             _httpClient = httpClient;
             _endpoints = endpoints;
             _clientId = clientId;
             _redirectUri = redirectUri;
             _clientSecret = clientSecret;
-            _trace = trace;
             _addAuthHeader = addAuthHeader;
         }
 
@@ -87,21 +91,10 @@ public IOAuth2CodeGenerator CodeGenerator
             set => _codeGenerator = value;
         }
 
-        protected string ClientId => _clientId;
-
-        protected string ClientSecret => _clientSecret;
-
-        protected ITrace Trace => _trace;
-
-        protected OAuth2ServerEndpoints Endpoints => _endpoints;
-
-        protected HttpClient HttpClient => _httpClient;
-
-        protected Uri RedirectUri => _redirectUri;
-
         #region IOAuth2Client
 
-        public async Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(IEnumerable<string> scopes, IOAuth2WebBrowser browser, CancellationToken ct)
+        public async Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(IEnumerable<string> scopes,
+            IOAuth2WebBrowser browser, IDictionary<string, string> extraQueryParams, CancellationToken ct)
         {
             string state = CodeGenerator.CreateNonce();
             string codeVerifier = CodeGenerator.CreatePkceCodeVerifier();
@@ -118,6 +111,21 @@ public async Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(IEnum
                 [OAuth2Constants.AuthorizationEndpoint.PkceChallengeParameter] = codeChallenge
             };
 
+            if (extraQueryParams?.Count > 0)
+            {
+                foreach (var kvp in extraQueryParams)
+                {
+                    if (queryParams.ContainsKey(kvp.Key))
+                    {
+                        throw new ArgumentException(
+                            $"Extra query parameter '{kvp.Key}' would override required standard OAuth parameters.",
+                            nameof(extraQueryParams));
+                    }
+
+                    queryParams[kvp.Key] = kvp.Value;
+                }
+            }
+
             Uri redirectUri = null;
             if (_redirectUri != null)
             {
@@ -389,4 +397,13 @@ protected static bool TryDeserializeJson<T>(string json, out T obj)
 
         #endregion
     }
+
+    public static class OAuth2ClientExtensions
+    {
+        public static Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(
+            this IOAuth2Client client, IEnumerable<string> scopes, IOAuth2WebBrowser browser, CancellationToken ct)
+        {
+            return client.GetAuthorizationCodeAsync(scopes, browser, null, ct);
+        }
+    }
 }

src/shared/Core/GenericHostProvider.cs

@@ -125,7 +125,6 @@ private async Task<ICredential> GetOAuthAccessToken(Uri remoteUri, string userNa
                 config.ClientId,
                 config.RedirectUri,
                 config.ClientSecret,
-                Context.Trace,
                 config.UseAuthHeader);
 
             //

src/shared/GitHub.Tests/GitHubHostProviderTests.cs

@@ -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.GetOAuthTokenViaBrowserAsync(expectedTargetUri, It.IsAny<IEnumerable<string>>()))
+            ghAuthMock.Setup(x => x.GetOAuthTokenViaBrowserAsync(expectedTargetUri, It.IsAny<IEnumerable<string>>(), It.IsAny<string>()))
                       .ReturnsAsync(response);
 
             var ghApiMock = new Mock<IGitHubRestApi>(MockBehavior.Strict);
@@ -213,7 +213,56 @@ public async Task GitHubHostProvider_GenerateCredentialAsync_Browser_ReturnsCred
 
             ghAuthMock.Verify(
                 x => x.GetOAuthTokenViaBrowserAsync(
-                    expectedTargetUri, expectedOAuthScopes),
+                    expectedTargetUri, expectedOAuthScopes, null),
+                Times.Once);
+        }
+
+        [Fact]
+        public async Task GitHubHostProvider_GenerateCredentialAsync_Browser_LoginHint_IncludesHintAndReturnsCredential()
+        {
+            var input = new InputArguments(new Dictionary<string, string>
+            {
+                ["protocol"] = "https",
+                ["host"] = "github.com",
+                ["username"] = "john.doe"
+            });
+
+            var expectedTargetUri = new Uri("https://github.com/");
+            IEnumerable<string> expectedOAuthScopes = new[]
+            {
+                GitHubConstants.OAuthScopes.Repo,
+                GitHubConstants.OAuthScopes.Gist,
+                GitHubConstants.OAuthScopes.Workflow,
+            };
+
+            var expectedUserName = "john.doe";
+            var tokenValue = "OAUTH-TOKEN";
+            var response = new OAuth2TokenResult(tokenValue, "bearer");
+
+            var context = new TestCommandContext();
+
+            var ghAuthMock = new Mock<IGitHubAuthentication>(MockBehavior.Strict);
+            ghAuthMock.Setup(x => x.GetAuthenticationAsync(expectedTargetUri, expectedUserName, It.IsAny<AuthenticationModes>()))
+                      .ReturnsAsync(new AuthenticationPromptResult(AuthenticationModes.Browser));
+
+            ghAuthMock.Setup(x => x.GetOAuthTokenViaBrowserAsync(expectedTargetUri, It.IsAny<IEnumerable<string>>(), It.IsAny<string>()))
+                      .ReturnsAsync(response);
+
+            var ghApiMock = new Mock<IGitHubRestApi>(MockBehavior.Strict);
+            ghApiMock.Setup(x => x.GetUserInfoAsync(expectedTargetUri, tokenValue))
+                     .ReturnsAsync(new GitHubUserInfo{Login = expectedUserName});
+
+            var provider = new GitHubHostProvider(context, ghApiMock.Object, ghAuthMock.Object);
+
+            ICredential credential = await provider.GenerateCredentialAsync(input);
+
+            Assert.NotNull(credential);
+            Assert.Equal(expectedUserName, credential.Account);
+            Assert.Equal(tokenValue, credential.Password);
+
+            ghAuthMock.Verify(
+                x => x.GetOAuthTokenViaBrowserAsync(
+                    expectedTargetUri, expectedOAuthScopes, expectedUserName),
                 Times.Once);
         }
 

src/shared/GitHub/GitHubAuthentication.cs

@@ -16,7 +16,7 @@ public interface IGitHubAuthentication : IDisposable
 
         Task<string> GetTwoFactorCodeAsync(Uri targetUri, bool isSms);
 
-        Task<OAuth2TokenResult> GetOAuthTokenViaBrowserAsync(Uri targetUri, IEnumerable<string> scopes);
+        Task<OAuth2TokenResult> GetOAuthTokenViaBrowserAsync(Uri targetUri, IEnumerable<string> scopes, string loginHint);
 
         Task<OAuth2TokenResult> GetOAuthTokenViaDeviceCodeAsync(Uri targetUri, IEnumerable<string> scopes);
     }
@@ -251,7 +251,7 @@ public async Task<string> GetTwoFactorCodeAsync(Uri targetUri, bool isSms)
             }
         }
 
-        public async Task<OAuth2TokenResult> GetOAuthTokenViaBrowserAsync(Uri targetUri, IEnumerable<string> scopes)
+        public async Task<OAuth2TokenResult> GetOAuthTokenViaBrowserAsync(Uri targetUri, IEnumerable<string> scopes, string loginHint)
         {
             ThrowIfUserInteractionDisabled();
 
@@ -270,11 +270,21 @@ public async Task<OAuth2TokenResult> GetOAuthTokenViaBrowserAsync(Uri targetUri,
             };
             var browser = new OAuth2SystemWebBrowser(Context.Environment, browserOptions);
 
+            // If we have a login hint we should pass this to GitHub as an extra query parameter
+            IDictionary<string, string> queryParams = null;
+            if (loginHint != null)
+            {
+                queryParams = new Dictionary<string, string>
+                {
+                    ["login"] = loginHint
+                };
+            }
+
             // Write message to the terminal (if any is attached) for some feedback that we're waiting for a web response
             Context.Terminal.WriteLine("info: please complete authentication in your browser...");
 
             OAuth2AuthorizationCodeResult authCodeResult =
-                await oauthClient.GetAuthorizationCodeAsync(scopes, browser, CancellationToken.None);
+                await oauthClient.GetAuthorizationCodeAsync(scopes, browser, queryParams, CancellationToken.None);
 
             return await oauthClient.GetTokenByAuthorizationCodeAsync(authCodeResult, CancellationToken.None);
         }

src/shared/GitHub/GitHubHostProvider.cs

@@ -150,10 +150,10 @@ public override async Task<ICredential> GenerateCredentialAsync(InputArguments i
                     return patCredential;
 
                 case AuthenticationModes.Browser:
-                    return await GenerateOAuthCredentialAsync(remoteUri, useBrowser: true);
+                    return await GenerateOAuthCredentialAsync(remoteUri, loginHint: input.UserName, useBrowser: true);
 
                 case AuthenticationModes.Device:
-                    return await GenerateOAuthCredentialAsync(remoteUri, useBrowser: false);
+                    return await GenerateOAuthCredentialAsync(remoteUri, loginHint: input.UserName, useBrowser: false);
 
                 case AuthenticationModes.Pat:
                     // The token returned by the user should be good to use directly as the password for Git
@@ -176,10 +176,10 @@ public override async Task<ICredential> GenerateCredentialAsync(InputArguments i
             }
         }
 
-        private async Task<GitCredential> GenerateOAuthCredentialAsync(Uri targetUri, bool useBrowser)
+        private async Task<GitCredential> GenerateOAuthCredentialAsync(Uri targetUri, string loginHint, bool useBrowser)
         {
             OAuth2TokenResult result = useBrowser
-                ? await _gitHubAuth.GetOAuthTokenViaBrowserAsync(targetUri, GitHubOAuthScopes)
+                ? await _gitHubAuth.GetOAuthTokenViaBrowserAsync(targetUri, GitHubOAuthScopes, loginHint)
                 : await _gitHubAuth.GetOAuthTokenViaDeviceCodeAsync(targetUri, GitHubOAuthScopes);
 
             // Resolve the GitHub user handle