Merge pull request #587 from hickford/usability

Skip helper when no choice necessary

Author: mjcheetham

src/shared/Core/Authentication/AuthenticationBase.cs

@@ -27,7 +27,7 @@ protected Task<IDictionary<string, string>> InvokeHelperAsync(string path, strin
             return InvokeHelperAsync(path, args, null, CancellationToken.None);
         }
 
-        protected async Task<IDictionary<string, string>> InvokeHelperAsync(string path, string args,
+        internal protected virtual async Task<IDictionary<string, string>> InvokeHelperAsync(string path, string args,
             IDictionary<string, string> standardInput, CancellationToken ct)
         {
             var procStartInfo = new ProcessStartInfo(path)

src/shared/Core/InternalsVisibleTo.cs

@@ -1,4 +1,4 @@
 using System.Runtime.CompilerServices;
 
 [assembly: InternalsVisibleTo("Core.Tests")]
-
+[assembly: InternalsVisibleTo("GitHub.Tests")]

src/shared/GitHub.Tests/GitHubAuthenticationTests.cs

@@ -0,0 +1,99 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Threading.Tasks;
+using GitCredentialManager.Tests.Objects;
+using Moq;
+using Moq.Protected;
+using Xunit;
+
+namespace GitHub.Tests
+{
+    public class GitHubAuthenticationTests
+    {
+        [Fact]
+        public async Task GitHubAuthentication_GetAuthenticationAsync_AuthenticationModesNone_ThrowsException()
+        {
+            var context = new TestCommandContext();
+            var auth = new GitHubAuthentication(context);
+            await Assert.ThrowsAsync<ArgumentException>("modes",
+                () => auth.GetAuthenticationAsync(null, null, AuthenticationModes.None)
+            );
+        }
+
+        [Theory]
+        [InlineData(AuthenticationModes.Browser)]
+        [InlineData(AuthenticationModes.Device)]
+        public async Task GitHubAuthentication_GetAuthenticationAsync_SingleChoice_TerminalAndInteractionNotRequired(GitHub.AuthenticationModes modes)
+        {
+            var context = new TestCommandContext();
+            context.Settings.IsTerminalPromptsEnabled = false;
+            context.Settings.IsInteractionAllowed = false;
+            context.SessionManager.IsDesktopSession = true; // necessary for browser
+            context.FileSystem.Files["/usr/local/bin/GitHub.UI"] = new byte[0];
+            context.FileSystem.Files[@"C:\Program Files\Git Credential Manager Core\GitHub.UI.exe"] = new byte[0];
+            var auth = new GitHubAuthentication(context);
+            var result = await auth.GetAuthenticationAsync(null, null, modes);
+            Assert.Equal(modes, result.AuthenticationMode);
+        }
+
+        [Fact]
+        public async Task GitHubAuthentication_GetAuthenticationAsync_TerminalPromptsDisabled_Throws()
+        {
+            var context = new TestCommandContext();
+            context.Settings.IsTerminalPromptsEnabled = false;
+            var auth = new GitHubAuthentication(context);
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => auth.GetAuthenticationAsync(null, null, AuthenticationModes.All)
+            );
+            Assert.Equal("Cannot prompt because terminal prompts have been disabled.", exception.Message);
+        }
+
+        // reproduces https://github.com/GitCredentialManager/git-credential-manager/issues/453
+        [Fact]
+        public async Task GitHubAuthentication_GetAuthenticationAsync_Terminal()
+        {
+            var context = new TestCommandContext();
+            context.FileSystem.Files["/usr/local/bin/GitHub.UI"] = new byte[0];
+            context.FileSystem.Files[@"C:\Program Files\Git Credential Manager Core\GitHub.UI.exe"] = new byte[0];
+            var auth = new GitHubAuthentication(context);
+            context.Terminal.Prompts["option (enter for default)"] = "";
+            var result = await auth.GetAuthenticationAsync(null, null, AuthenticationModes.All);
+            Assert.Equal(AuthenticationModes.Device, result.AuthenticationMode);
+        }
+
+        [Fact]
+        public async Task GitHubAuthentication_GetAuthenticationAsync_AuthenticationModesAll_RequiresInteraction()
+        {
+            var context = new TestCommandContext();
+            context.Settings.IsInteractionAllowed = false;
+            var auth = new GitHubAuthentication(context);
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(
+                () => auth.GetAuthenticationAsync(new Uri("https://github.com"), null, AuthenticationModes.All)
+            );
+            Assert.Equal("Cannot prompt because user interactivity has been disabled.", exception.Message);
+        }
+
+        [Fact]
+        public async Task GitHubAuthentication_GetAuthenticationAsync_Helper_Basic()
+        {
+            var context = new TestCommandContext();
+            context.FileSystem.Files["/usr/local/bin/GitHub.UI"] = new byte[0];
+            context.FileSystem.Files[@"C:\Program Files\Git Credential Manager Core\GitHub.UI.exe"] = new byte[0];
+            context.SessionManager.IsDesktopSession = true;
+            var auth = new Mock<GitHubAuthentication>(MockBehavior.Strict, context);
+            auth.Setup(x => x.InvokeHelperAsync(It.IsAny<string>(), "prompt --all", It.IsAny<IDictionary<string, string>>(), It.IsAny<System.Threading.CancellationToken>()))
+            .Returns(Task.FromResult<IDictionary<string, string>>(
+                new Dictionary<string, string>
+                {
+                    ["mode"] = "basic",
+                    ["username"] = "tim",
+                    ["password"] = "hunter2"
+                }));
+            var result = await auth.Object.GetAuthenticationAsync(new Uri("https://github.com"), null, AuthenticationModes.All);
+            Assert.Equal(AuthenticationModes.Basic, result.AuthenticationMode);
+            Assert.Equal("tim", result.Credential.Account);
+            Assert.Equal("hunter2", result.Credential.Password);
+        }
+    }
+}

src/shared/GitHub/GitHubAuthentication.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Threading.Tasks;
 using System.Collections.Generic;
-using System.Globalization;
 using System.Net.Http;
 using System.Text;
 using System.Threading;
@@ -65,8 +64,6 @@ public GitHubAuthentication(ICommandContext context)
 
         public async Task<AuthenticationPromptResult> GetAuthenticationAsync(Uri targetUri, string userName, AuthenticationModes modes)
         {
-            ThrowIfUserInteractionDisabled();
-
             // If we don't have a desktop session/GUI then we cannot offer browser
             if (!Context.SessionManager.IsDesktopSession)
             {
@@ -79,6 +76,15 @@ public async Task<AuthenticationPromptResult> GetAuthenticationAsync(Uri targetU
                 throw new ArgumentException(@$"Must specify at least one {nameof(AuthenticationModes)}", nameof(modes));
             }
 
+            // If there is no mode choice to be made and no interaction required,
+            // just return that result.
+            if (modes == AuthenticationModes.Browser ||
+                modes == AuthenticationModes.Device)
+            {
+                    return new AuthenticationPromptResult(modes);
+            }
+
+            ThrowIfUserInteractionDisabled();
             if (Context.SessionManager.IsDesktopSession && TryFindHelperExecutablePath(out string helperPath))
             {
                 var promptArgs = new StringBuilder("prompt");

src/shared/GitHub/GitHubHostProvider.cs

@@ -316,6 +316,8 @@ public static bool IsGitHubDotCom(string targetUrl)
 
         public static bool IsGitHubDotCom(Uri targetUri)
         {
+            EnsureArgument.AbsoluteUri(targetUri, nameof(targetUri));
+
             return StringComparer.OrdinalIgnoreCase.Equals(targetUri.Host, GitHubConstants.GitHubBaseUrlHost);
         }