Apply suggestions from code review

Co-authored-by: Matthew John Cheetham <[email protected]>

Author: hickford

README.md

@@ -25,6 +25,7 @@ Secure platform credential storage|&#10003;<br/>[(see more)](docs/credstores.md)
 Multi-factor authentication support for Azure DevOps|&#10003;|&#10003;|&#10003;
 Two-factor authentication support for GitHub|&#10003;|&#10003;|&#10003;
 Two-factor authentication support for Bitbucket|&#10003;|&#10003;|&#10003;
+Two-factor authentication support for GitLab|&#10003;|&#10003;|&#10003;
 Windows Integrated Authentication (NTLM/Kerberos) support|&#10003;|_N/A_|_N/A_
 Basic HTTP authentication support|&#10003;|&#10003;|&#10003;
 Proxy support|&#10003;|&#10003;|&#10003;

src/shared/GitLab.Tests/GitLabAuthenticationTests.cs

@@ -57,6 +57,34 @@ public void GitLabAuthentication_GetAuthenticationAsync_Terminal()
             Assert.Equal(AuthenticationModes.Browser, result.AuthenticationMode);
         }
 
+        [Fact]
+        public void GitLabAuthentication_GetAuthenticationAsync_ChoosePat()
+        {
+            var context = new TestCommandContext();
+            var auth = new GitLabAuthentication(context);
+            context.Terminal.Prompts["option (enter for default)"] = "";
+            context.Terminal.Prompts["Username"] = "username";
+            context.Terminal.SecretPrompts["Personal access token"] = "token";
+            var result = auth.GetAuthentication(null, null, AuthenticationModes.All);
+            Assert.Equal(AuthenticationModes.Pat, result.AuthenticationMode);
+            Assert.Equal("username", result.Credential.Account);
+            Assert.Equal("token", result.Credential.Password);
+        }
+
+        [Fact]
+        public void GitLabAuthentication_GetAuthenticationAsync_ChooseBasic()
+        {
+            var context = new TestCommandContext();
+            var auth = new GitLabAuthentication(context);
+            context.Terminal.Prompts["option (enter for default)"] = "2";
+            context.Terminal.Prompts["Username"] = "username";
+            context.Terminal.SecretPrompts["Password"] = "password";
+            var result = auth.GetAuthentication(null, null, AuthenticationModes.All);
+            Assert.Equal(AuthenticationModes.Basic, result.AuthenticationMode);
+            Assert.Equal("username", result.Credential.Account);
+            Assert.Equal("password", result.Credential.Password);
+        }
+
         [Fact]
         public void GitLabAuthentication_GetAuthenticationAsync_AuthenticationModesAll_RequiresInteraction()
         {

src/shared/GitLab/GitLabAuthentication.cs

@@ -71,6 +71,22 @@ public AuthenticationPromptResult GetAuthentication(Uri targetUri, string userNa
             switch (modes)
             {
                 case AuthenticationModes.Basic:
+                    ThrowIfUserInteractionDisabled();
+                    ThrowIfTerminalPromptsDisabled();
+                    Context.Terminal.WriteLine("Enter GitLab credentials for '{0}'...", targetUri);
+
+                    if (string.IsNullOrWhiteSpace(userName))
+                    {
+                        userName = Context.Terminal.Prompt("Username");
+                    }
+                    else
+                    {
+                        Context.Terminal.WriteLine("Username: {0}", userName);
+                    }
+
+                    string password = Context.Terminal.PromptSecret("Password");
+                    return new AuthenticationPromptResult(AuthenticationModes.Basic, new GitCredential(userName, password));
+
                 case AuthenticationModes.Pat:
                     ThrowIfUserInteractionDisabled();
                     ThrowIfTerminalPromptsDisabled();
@@ -85,8 +101,8 @@ public AuthenticationPromptResult GetAuthentication(Uri targetUri, string userNa
                         Context.Terminal.WriteLine("Username: {0}", userName);
                     }
 
-                    string password_or_token = Context.Terminal.PromptSecret(modes == AuthenticationModes.Basic ? "Password" : "Personal access token");
-                    return new AuthenticationPromptResult(modes, new GitCredential(userName, password_or_token));
+                    string token = Context.Terminal.PromptSecret("Personal access token");
+                    return new AuthenticationPromptResult(AuthenticationModes.Pat, new GitCredential(userName, token));
 
                 case AuthenticationModes.Browser:
                     return new AuthenticationPromptResult(AuthenticationModes.Browser);

src/shared/GitLab/GitLabHostProvider.cs

@@ -160,7 +160,7 @@ public override async Task<ICredential> GetCredentialAsync(InputArguments input)
             }
 
             string refreshService = GetRefreshTokenServiceName(input);
-            string refreshToken = Context.CredentialStore.Get(service, input.UserName)?.Password;
+            string refreshToken = Context.CredentialStore.Get(refreshService, input.UserName)?.Password;
             if (refreshToken != null)
             {
                 Context.Trace.WriteLine("Refreshing OAuth token...");
@@ -183,7 +183,7 @@ public override async Task<ICredential> GetCredentialAsync(InputArguments input)
                 // store credential, since we know it to be valid (whereas Git will only store credential if git push succeeds)
                 Context.CredentialStore.AddOrUpdate(service, oAuthCredential.Account, oAuthCredential.AccessToken);
                 // store refresh token under a separate service
-                Context.CredentialStore.AddOrUpdate(GetRefreshTokenServiceName(input), oAuthCredential.Account, oAuthCredential.RefreshToken);
+                Context.CredentialStore.AddOrUpdate(refreshService, oAuthCredential.Account, oAuthCredential.RefreshToken);
             }
             return credential;
         }