Merge pull request #236 from mjcheetham/githubpatmode

Add explicit PAT authentication mode for the GitHub Provider

Author: mjcheetham

src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs

@@ -50,7 +50,7 @@ public async Task<ICredential> GetBasicCredentialsAsync(Uri targetUri, string us
                 var cmdArgs = new StringBuilder("userpass");
                 if (!string.IsNullOrWhiteSpace(userName))
                 {
-                    cmdArgs.AppendFormat(" --username {0}", userName);
+                    cmdArgs.AppendFormat(" --username {0}", QuoteCmdArg(userName));
                 }
 
                 IDictionary<string, string> output = await InvokeHelperAsync(helperPath, cmdArgs.ToString());

src/shared/GitHub.Tests/GitHubHostProviderTests.cs

@@ -221,7 +221,8 @@ public async Task GitHubHostProvider_GenerateCredentialAsync_Basic_1FAOnly_Retur
 
             var ghAuthMock = new Mock<IGitHubAuthentication>(MockBehavior.Strict);
             ghAuthMock.Setup(x => x.GetAuthenticationAsync(expectedTargetUri, null, It.IsAny<AuthenticationModes>()))
-                      .ReturnsAsync(new AuthenticationPromptResult(new GitCredential(expectedUserName, expectedPassword)));
+                      .ReturnsAsync(new AuthenticationPromptResult(
+                          AuthenticationModes.Basic, new GitCredential(expectedUserName, expectedPassword)));
 
             var ghApiMock = new Mock<IGitHubRestApi>(MockBehavior.Strict);
             ghApiMock.Setup(x => x.CreatePersonalAccessTokenAsync(expectedTargetUri, expectedUserName, expectedPassword, null, It.IsAny<IEnumerable<string>>()))
@@ -270,7 +271,8 @@ public async Task GitHubHostProvider_GenerateCredentialAsync_Basic_2FARequired_R
 
             var ghAuthMock = new Mock<IGitHubAuthentication>(MockBehavior.Strict);
             ghAuthMock.Setup(x => x.GetAuthenticationAsync(expectedTargetUri, null, It.IsAny<AuthenticationModes>()))
-                      .ReturnsAsync(new AuthenticationPromptResult(new GitCredential(expectedUserName, expectedPassword)));
+                      .ReturnsAsync(new AuthenticationPromptResult(
+                          AuthenticationModes.Basic, new GitCredential(expectedUserName, expectedPassword)));
             ghAuthMock.Setup(x => x.GetTwoFactorCodeAsync(expectedTargetUri, false))
                       .ReturnsAsync(expectedAuthCode);
 

src/shared/GitHub/GitHubAuthentication.cs

@@ -28,15 +28,15 @@ public AuthenticationPromptResult(AuthenticationModes mode)
             AuthenticationMode = mode;
         }
 
-        public AuthenticationPromptResult(ICredential basicCredential)
-            : this(AuthenticationModes.Basic)
+        public AuthenticationPromptResult(AuthenticationModes mode, ICredential credential)
+            : this(mode)
         {
-            BasicCredential = basicCredential;
+            Credential = credential;
         }
 
         public AuthenticationModes AuthenticationMode { get; }
 
-        public ICredential BasicCredential { get; set; }
+        public ICredential Credential { get; set; }
     }
 
     [Flags]
@@ -45,6 +45,9 @@ public enum AuthenticationModes
         None  = 0,
         Basic = 1,
         OAuth = 1 << 1,
+        Pat   = 1 << 2,
+
+        All   = Basic | OAuth | Pat
     }
 
     public class GitHubAuthentication : AuthenticationBase, IGitHubAuthentication
@@ -63,16 +66,24 @@ public async Task<AuthenticationPromptResult> GetAuthenticationAsync(Uri targetU
 
             if (modes == AuthenticationModes.None)
             {
-                throw new ArgumentException($"Must specify at least one {nameof(AuthenticationModes)}", nameof(modes));
+                throw new ArgumentException(@$"Must specify at least one {nameof(AuthenticationModes)}", nameof(modes));
             }
 
             if (TryFindHelperExecutablePath(out string helperPath))
             {
                 var promptArgs = new StringBuilder("prompt");
-                if ((modes & AuthenticationModes.Basic) != 0) promptArgs.Append(" --basic");
-                if ((modes & AuthenticationModes.OAuth) != 0) promptArgs.Append(" --oauth");
-                if (!GitHubHostProvider.IsGitHubDotCom(targetUri)) promptArgs.AppendFormat(" --enterprise-url {0}", targetUri);
-                if (!string.IsNullOrWhiteSpace(userName)) promptArgs.AppendFormat(" --username {0}", userName);
+                if (modes == AuthenticationModes.All)
+                {
+                    promptArgs.Append(" --all");
+                }
+                else
+                {
+                    if ((modes & AuthenticationModes.Basic) != 0) promptArgs.Append(" --basic");
+                    if ((modes & AuthenticationModes.OAuth) != 0) promptArgs.Append(" --oauth");
+                    if ((modes & AuthenticationModes.Pat)   != 0) promptArgs.Append(" --pat");
+                }
+                if (!GitHubHostProvider.IsGitHubDotCom(targetUri)) promptArgs.AppendFormat(" --enterprise-url {0}", QuoteCmdArg(targetUri.ToString()));
+                if (!string.IsNullOrWhiteSpace(userName)) promptArgs.AppendFormat(" --username {0}", QuoteCmdArg(userName));
 
                 IDictionary<string, string> resultDict = await InvokeHelperAsync(helperPath, promptArgs.ToString(), null);
 
@@ -83,6 +94,15 @@ public async Task<AuthenticationPromptResult> GetAuthenticationAsync(Uri targetU
 
                 switch (responseMode.ToLowerInvariant())
                 {
+                    case "pat":
+                        if (!resultDict.TryGetValue("pat", out string pat))
+                        {
+                            throw new Exception("Missing 'pat' in response");
+                        }
+
+                        return new AuthenticationPromptResult(
+                            AuthenticationModes.Pat, new GitCredential(userName, pat));
+
                     case "oauth":
                         return new AuthenticationPromptResult(AuthenticationModes.OAuth);
 
@@ -97,7 +117,8 @@ public async Task<AuthenticationPromptResult> GetAuthenticationAsync(Uri targetU
                             throw new Exception("Missing 'password' in response");
                         }
 
-                        return new AuthenticationPromptResult(new GitCredential(userName, password));
+                        return new AuthenticationPromptResult(
+                            AuthenticationModes.Basic, new GitCredential(userName, password));
 
                     default:
                         throw new Exception($"Unknown mode value in response '{responseMode}'");
@@ -109,21 +130,6 @@ public async Task<AuthenticationPromptResult> GetAuthenticationAsync(Uri targetU
 
                 switch (modes)
                 {
-                    case AuthenticationModes.Basic | AuthenticationModes.OAuth:
-                        var menuTitle = $"Select an authentication method for '{targetUri}'";
-                        var menu = new TerminalMenu(Context.Terminal, menuTitle)
-                        {
-                            new TerminalMenuItem(1, "Web browser", isDefault: true),
-                            new TerminalMenuItem(2, "Username/password")
-                        };
-
-                        int option = menu.Show();
-
-                        if (option == 1) goto case AuthenticationModes.OAuth;
-                        if (option == 2) goto case AuthenticationModes.Basic;
-
-                        throw new Exception();
-
                     case AuthenticationModes.Basic:
                         Context.Terminal.WriteLine("Enter GitHub credentials for '{0}'...", targetUri);
 
@@ -138,13 +144,41 @@ public async Task<AuthenticationPromptResult> GetAuthenticationAsync(Uri targetU
 
                         string password = Context.Terminal.PromptSecret("Password");
 
-                        return new AuthenticationPromptResult(new GitCredential(userName, password));
+                        return new AuthenticationPromptResult(
+                            AuthenticationModes.Basic, new GitCredential(userName, password));
 
                     case AuthenticationModes.OAuth:
                         return new AuthenticationPromptResult(AuthenticationModes.OAuth);
 
+                    case AuthenticationModes.Pat:
+                        Context.Terminal.WriteLine("Enter GitHub personal access token for '{0}'...", targetUri);
+                        string pat = Context.Terminal.PromptSecret("Token");
+                        return new AuthenticationPromptResult(
+                            AuthenticationModes.Pat, new GitCredential(userName, pat));
+
+                    case AuthenticationModes.None:
+                        throw new ArgumentOutOfRangeException(nameof(modes), @$"At least one {nameof(AuthenticationModes)} must be supplied");
+
                     default:
-                        throw new ArgumentOutOfRangeException(nameof(modes), $"Unknown {nameof(AuthenticationModes)} value");
+                        var menuTitle = $"Select an authentication method for '{targetUri}'";
+                        var menu = new TerminalMenu(Context.Terminal, menuTitle);
+
+                        TerminalMenuItem oauthItem = null;
+                        TerminalMenuItem basicItem = null;
+                        TerminalMenuItem patItem = null;
+
+                        if ((modes & AuthenticationModes.OAuth) != 0) oauthItem = menu.Add("Web browser");
+                        if ((modes & AuthenticationModes.Pat)   != 0) patItem   = menu.Add("Personal access token");
+                        if ((modes & AuthenticationModes.Basic) != 0) basicItem = menu.Add("Username/password");
+
+                        // Default to the 'first' choice in the menu
+                        TerminalMenuItem choice = menu.Show(0);
+
+                        if (choice == oauthItem) goto case AuthenticationModes.OAuth;
+                        if (choice == basicItem) goto case AuthenticationModes.Basic;
+                        if (choice == patItem)   goto case AuthenticationModes.Pat;
+
+                        throw new Exception();
                 }
             }
         }

src/shared/GitHub/GitHubConstants.cs

@@ -35,7 +35,11 @@ public static class GitHubConstants
         /// <summary>
         /// Supported authentication modes for github.com.
         /// </summary>
-        public const AuthenticationModes DotComAuthenticationModes = AuthenticationModes.OAuth;
+        /// <remarks>
+        /// As of 13th November 2020, github.com does not support username/password (basic) authentication to the APIs.
+        /// See https://developer.github.com/changes/2020-02-14-deprecating-oauth-auth-endpoint for more information.
+        /// </remarks>
+        public const AuthenticationModes DotComAuthenticationModes = AuthenticationModes.OAuth | AuthenticationModes.Pat;
 
         public static class TokenScopes
         {

src/shared/GitHub/GitHubHostProvider.cs

@@ -96,7 +96,7 @@ public override async Task<ICredential> GenerateCredentialAsync(InputArguments i
             switch (promptResult.AuthenticationMode)
             {
                 case AuthenticationModes.Basic:
-                    GitCredential patCredential = await GeneratePersonalAccessTokenAsync(remoteUri, promptResult.BasicCredential);
+                    GitCredential patCredential = await GeneratePersonalAccessTokenAsync(remoteUri, promptResult.Credential);
 
                     // HACK: Store the PAT immediately in case this PAT is not valid for SSO.
                     // We don't know if this PAT is valid for SAML SSO and if it's not Git will fail
@@ -111,6 +111,22 @@ public override async Task<ICredential> GenerateCredentialAsync(InputArguments i
                 case AuthenticationModes.OAuth:
                     return await GenerateOAuthCredentialAsync(remoteUri);
 
+                case AuthenticationModes.Pat:
+                    // The token returned by the user should be good to use directly as the password for Git
+                    string token = promptResult.Credential.Password;
+
+                    // Resolve the GitHub user handle if we don't have a specific username already from the
+                    // initial request. The reason for this is GitHub requires a (any?) value for the username
+                    // when Git makes calls to GitHub.
+                    string userName = promptResult.Credential.Account;
+                    if (userName is null)
+                    {
+                        GitHubUserInfo userInfo = await _gitHubApi.GetUserInfoAsync(remoteUri, token);
+                        userName = userInfo.Login;
+                    }
+
+                    return new GitCredential(userName, token);
+
                 default:
                     throw new ArgumentOutOfRangeException(nameof(promptResult));
             }
@@ -186,15 +202,16 @@ internal async Task<AuthenticationModes> GetSupportedAuthenticationModesAsync(Ur
                 }
             }
 
-            // github.com should use OAuth authentication only
+            // github.com should use OAuth or manual PAT based authentication only, never basic auth as of 13th November 2020
+            // https://developer.github.com/changes/2020-02-14-deprecating-oauth-auth-endpoint
             if (IsGitHubDotCom(targetUri))
             {
                 Context.Trace.WriteLine($"{targetUri} is github.com - authentication schemes: '{GitHubConstants.DotComAuthenticationModes}'");
                 return GitHubConstants.DotComAuthenticationModes;
             }
 
             // For GitHub Enterprise we must do some detection of supported modes
-            Context.Trace.WriteLine($"{targetUri} is GitHub Enterprise - checking for supporting authentication schemes...");
+            Context.Trace.WriteLine($"{targetUri} is GitHub Enterprise - checking for supported authentication schemes...");
 
             try
             {
@@ -219,7 +236,9 @@ internal async Task<AuthenticationModes> GetSupportedAuthenticationModesAsync(Ur
                 Context.Trace.WriteException(ex);
 
                 Context.Terminal.WriteLine($"warning: failed to query '{targetUri}' for supported authentication schemes.");
-                return AuthenticationModes.Basic | AuthenticationModes.OAuth;
+
+                // Fall-back to offering all modes so the user is never blocked from authenticating by at least one mode
+                return AuthenticationModes.All;
             }
         }
 

src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/AuthenticationBaseTests.cs

@@ -0,0 +1,27 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license.
+using Microsoft.Git.CredentialManager.Authentication;
+using Xunit;
+
+namespace Microsoft.Git.CredentialManager.Tests.Authentication
+{
+    public class AuthenticationBaseTests
+    {
+        [Theory]
+        [InlineData("foo", "foo")]
+        [InlineData("foo bar", "\"foo bar\"")]
+        [InlineData("foo\nbar", "\"foo\nbar\"")]
+        [InlineData("foo\rbar", "\"foo\rbar\"")]
+        [InlineData("foo\tbar", "\"foo\tbar\"")]
+        [InlineData("foo\" bar", "\"foo\\\" bar\"")]
+        [InlineData("foo\"", "\"foo\\\"\"")]
+        [InlineData("\"foo", "\"\\\"foo\"")]
+        [InlineData("foo\\", "\"foo\\\\\"")]
+        [InlineData("foo\\\"", "\"foo\\\\\\\"\"")]
+        public void AuthenticationBase_QuoteCmdArg(string input, string expected)
+        {
+            string actual = AuthenticationBase.QuoteCmdArg(input);
+            Assert.Equal(expected, actual);
+        }
+    }
+}

src/shared/Microsoft.Git.CredentialManager.Tests/EnsureArgumentTests.cs

@@ -0,0 +1,33 @@
+using System;
+using Xunit;
+
+namespace Microsoft.Git.CredentialManager.Tests
+{
+    public class EnsureArgumentTests
+    {
+        [Theory]
+        [InlineData(5, 0, 10, true, true)]
+        [InlineData(0, 0, 10, true, true)]
+        [InlineData(10, 0, 10, true, true)]
+        [InlineData(0, -10, 0, true, true)]
+        [InlineData(-10, -10, 0, true, true)]
+        public void EnsureArgument_InRange_DoesNotThrow(int x, int lower, int upper, bool lowerInc, bool upperInc)
+        {
+            EnsureArgument.InRange(x, nameof(x), lower, upper, lowerInc, upperInc);
+        }
+
+        [Theory]
+        [InlineData(-3, 0, 10, true, true)]
+        [InlineData(13, 0, 10, true, true)]
+        [InlineData(10, 0, 10, true, false)]
+        [InlineData(0, 0, 10, false, true)]
+        [InlineData(-10, -10, 0, false, true)]
+        [InlineData(0, -10, 0, true, false)]
+        public void EnsureArgument_InRange_ThrowsException(int x, int lower, int upper, bool lowerInc, bool upperInc)
+        {
+            Assert.Throws<ArgumentOutOfRangeException>(
+                () => EnsureArgument.InRange(x, nameof(x), lower, upper, lowerInc, upperInc)
+            );
+        }
+    }
+}

src/shared/Microsoft.Git.CredentialManager/Authentication/AuthenticationBase.cs

@@ -4,7 +4,9 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
+using System.Linq;
 using System.Reflection;
+using System.Text;
 using System.Threading.Tasks;
 
 namespace Microsoft.Git.CredentialManager.Authentication
@@ -137,5 +139,22 @@ protected bool TryFindHelperExecutablePath(string envar, string configName, stri
 
             return true;
         }
+
+        public static string QuoteCmdArg(string str)
+        {
+            char[] needsQuoteChars = {'"', ' ', '\\', '\n', '\r', '\t'};
+            bool needsQuotes = str.Any(x => needsQuoteChars.Contains(x));
+
+            if (!needsQuotes)
+            {
+                return str;
+            }
+
+            // Replace all '\' characters with an escaped '\\', and all '"' with '\"'
+            string escapedStr = str.Replace("\\", "\\\\").Replace("\"", "\\\"");
+
+            // Bookend the escaped string with double-quotes '"'
+            return $"\"{escapedStr}\"";
+        }
     }
 }

src/shared/Microsoft.Git.CredentialManager/EnsureArgument.cs

@@ -75,5 +75,28 @@ public static void Negative(int arg, string name)
                 throw new ArgumentOutOfRangeException(name, "Argument must be negative.");
             }
         }
+
+        public static void InRange(int arg, string name, int lower, int upper, bool lowerInclusive = true, bool upperInclusive = true)
+        {
+            if (lowerInclusive && arg < lower)
+            {
+                throw new ArgumentOutOfRangeException(name, $"Argument must be greater than or equal to {lower}.");
+            }
+
+            if (!lowerInclusive && arg <= lower)
+            {
+                throw new ArgumentOutOfRangeException(name, $"Argument must be strictly greater than {lower}.");
+            }
+
+            if (upperInclusive && arg > upper)
+            {
+                throw new ArgumentOutOfRangeException(name, $"Argument must be less than or equal to {upper}.");
+            }
+
+            if (!upperInclusive && arg >= upper)
+            {
+                throw new ArgumentOutOfRangeException(name, $"Argument must be strictly less than {upper}.");
+            }
+        }
     }
 }