Merge pull request #181 from microsoft/master

New release to fix Linux web browser launching

src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs

@@ -129,7 +129,7 @@ public async Task<OAuth2TokenResult> CreateOAuthCredentialsAsync(Uri targetUri)
                 FailureResponseHtmlFormat = BitbucketResources.AuthenticationResponseFailureHtmlFormat
             };
 
-            var browser = new OAuth2SystemWebBrowser(browserOptions);
+            var browser = new OAuth2SystemWebBrowser(Context.Environment, browserOptions);
             var authCodeResult = await oauthClient.GetAuthorizationCodeAsync(Scopes, browser, CancellationToken.None);
 
             return await oauthClient.GetTokenByAuthorizationCodeAsync(authCodeResult, CancellationToken.None);

src/shared/GitHub/GitHubAuthentication.cs

@@ -197,7 +197,7 @@ public async Task<OAuth2TokenResult> GetOAuthTokenAsync(Uri targetUri, IEnumerab
                     SuccessResponseHtml = GitHubResources.AuthenticationResponseSuccessHtml,
                     FailureResponseHtmlFormat = GitHubResources.AuthenticationResponseFailureHtmlFormat
                 };
-                var browser = new OAuth2SystemWebBrowser(browserOptions);
+                var browser = new OAuth2SystemWebBrowser(Context.Environment, browserOptions);
 
                 // 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...");

src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2SystemWebBrowser.cs

@@ -35,10 +35,15 @@ public class OAuth2WebBrowserOptions
 
     public class OAuth2SystemWebBrowser : IOAuth2WebBrowser
     {
+        private readonly IEnvironment _environment;
         private readonly OAuth2WebBrowserOptions _options;
 
-        public OAuth2SystemWebBrowser(OAuth2WebBrowserOptions options)
+        public OAuth2SystemWebBrowser(IEnvironment environment, OAuth2WebBrowserOptions options)
         {
+            EnsureArgument.NotNull(environment, nameof(environment));
+            EnsureArgument.NotNull(options, nameof(options));
+
+            _environment = environment;
             _options = options;
         }
 
@@ -75,18 +80,56 @@ public async Task<Uri> GetAuthenticationCodeAsync(Uri authorizationUri, Uri redi
 
         private void OpenDefaultBrowser(Uri uri)
         {
-            if (!StringComparer.OrdinalIgnoreCase.Equals(Uri.UriSchemeHttp,  uri.Scheme) &&
-                !StringComparer.OrdinalIgnoreCase.Equals(Uri.UriSchemeHttps, uri.Scheme))
+            if (!uri.Scheme.Equals(Uri.UriSchemeHttp, StringComparison.OrdinalIgnoreCase) &&
+                !uri.Scheme.Equals(Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase))
             {
                 throw new ArgumentException("Can only open HTTP/HTTPS URIs", nameof(uri));
             }
 
-            var pci = new ProcessStartInfo(uri.ToString())
+            string url = uri.ToString();
+
+            ProcessStartInfo psi = null;
+            if (PlatformUtils.IsLinux())
             {
-                UseShellExecute = true
-            };
+                // On Linux, 'shell execute' utilities like xdg-open launch a process without
+                // detaching from the standard in/out descriptors. Some applications (like
+                // Chromium) write messages to stdout, which is currently hooked up and being
+                // consumed by Git, and cause errors.
+                //
+                // Sadly, the Framework does not allow us to redirect standard streams if we
+                // set ProcessStartInfo::UseShellExecute = true, so we must manually launch
+                // these utilities and redirect the standard streams manually.
+                //
+                // We try and use the same 'shell execute' utilities as the Framework does,
+                // searching for them in the same order until we find one.
+                foreach (string shellExec in new[] { "xdg-open", "gnome-open", "kfmclient" })
+                {
+                    if (_environment.TryLocateExecutable(shellExec, out string shellExecPath))
+                    {
+                        psi = new ProcessStartInfo(shellExecPath, url)
+                        {
+                            RedirectStandardOutput = true,
+                            RedirectStandardError = true
+                        };
+
+                        // We found a way to open the URI; stop searching!
+                        break;
+                    }
+                }
+
+                if (psi is null)
+                {
+                    throw new Exception("Failed to locate a utility to launch the default web browser.");
+                }
+            }
+            else
+            {
+                // On Windows and macOS, `ShellExecute` and `/usr/bin/open` disconnect the child process
+                // from our standard in/out streams, so we can just use the Framework to do this.
+                psi = new ProcessStartInfo(url) {UseShellExecute = true};
+            }
 
-            Process.Start(pci);
+            Process.Start(psi);
         }
 
         private async Task<Uri> InterceptRequestsAsync(Uri listenUri, CancellationToken ct)

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

@@ -41,8 +41,9 @@ public interface IEnvironment
         /// Locate an executable on the current PATH.
         /// </summary>
         /// <param name="program">Executable program name.</param>
-        /// <returns>List of all instances of the found executable program, in order of most specific to least.</returns>
-        string LocateExecutable(string program);
+        /// <param name="path">First instance of the found executable program.</param>
+        /// <returns>True if the executable was found, false otherwise.</returns>
+        bool TryLocateExecutable(string program, out string path);
     }
 
     public abstract class EnvironmentBase : IEnvironment
@@ -75,6 +76,25 @@ public bool IsDirectoryOnPath(string directoryPath)
 
         protected abstract string[] SplitPathVariable(string value);
 
-        public abstract string LocateExecutable(string program);
+        public abstract bool TryLocateExecutable(string program, out string path);
+    }
+
+    public static class EnvironmentExtensions
+    {
+        /// <summary>
+        /// Locate an executable on the current PATH.
+        /// </summary>
+        /// <param name="environment">The <see cref="IEnvironment"/>.</param>
+        /// <param name="program">Executable program name.</param>
+        /// <returns>List of all instances of the found executable program, in order of most specific to least.</returns>
+        public static string LocateExecutable(this IEnvironment environment, string program)
+        {
+            if (environment.TryLocateExecutable(program, out string path))
+            {
+                return path;
+            }
+
+            throw new Exception($"Failed to locate '{program}' executable on the path.");
+        }
     }
 }

src/shared/Microsoft.Git.CredentialManager/Interop/Posix/PosixEnvironment.cs

@@ -31,7 +31,7 @@ protected override string[] SplitPathVariable(string value)
             return value.Split(':');
         }
 
-        public override string LocateExecutable(string program)
+        public override bool TryLocateExecutable(string program, out string path)
         {
             const string whichPath = "/usr/bin/which";
             var psi = new ProcessStartInfo(whichPath, program)
@@ -45,19 +45,21 @@ public override string LocateExecutable(string program)
                 where.Start();
                 where.WaitForExit();
 
-                if (where.ExitCode != 0)
+                switch (where.ExitCode)
                 {
-                    throw new Exception($"Failed to locate '{program}' using {whichPath}. Exit code: {where.ExitCode}.");
-                }
+                    case 0: // found
+                        string stdout = where.StandardOutput.ReadToEnd();
+                        string[] results = stdout.Split(new[] {'\n'}, StringSplitOptions.RemoveEmptyEntries);
+                        path = results.First();
+                        return true;
 
-                string stdout = where.StandardOutput.ReadToEnd();
-                if (string.IsNullOrWhiteSpace(stdout))
-                {
-                    return null;
-                }
+                    case 1: // not found
+                        path = null;
+                        return false;
 
-                string[] results = stdout.Split(new[] {'\n'}, StringSplitOptions.RemoveEmptyEntries);
-                return results.FirstOrDefault();
+                    default:
+                        throw new Exception($"Unknown error locating '{program}' using {whichPath}. Exit code: {where.ExitCode}.");
+                }
             }
         }
 

src/shared/Microsoft.Git.CredentialManager/Interop/Windows/WindowsEnvironment.cs

@@ -65,7 +65,7 @@ public override void RemoveDirectoryFromPath(string directoryPath, EnvironmentVa
             }
         }
 
-        public override string LocateExecutable(string program)
+        public override bool TryLocateExecutable(string program, out string path)
         {
             string wherePath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "where.exe");
             var psi = new ProcessStartInfo(wherePath, program)
@@ -79,19 +79,21 @@ public override string LocateExecutable(string program)
                 where.Start();
                 where.WaitForExit();
 
-                if (where.ExitCode != 0)
+                switch (where.ExitCode)
                 {
-                    throw new Exception($"Failed to locate '{program}' using where.exe. Exit code: {where.ExitCode}.");
+                    case 0: // found
+                        string stdout = where.StandardOutput.ReadToEnd();
+                        string[] results = stdout.Split(new[] {'\r', '\n'}, StringSplitOptions.RemoveEmptyEntries);
+                        path = results.First();
+                        return true;
+
+                    case 1: // not found
+                        path = null;
+                        return false;
+
+                    default:
+                        throw new Exception($"Unknown error locating '{program}' using where.exe. Exit code: {where.ExitCode}.");
                 }
-
-                string stdout = where.StandardOutput.ReadToEnd();
-                if (string.IsNullOrWhiteSpace(stdout))
-                {
-                    return null;
-                }
-
-                string[] results = stdout.Split(new[] {'\r', '\n'}, StringSplitOptions.RemoveEmptyEntries);
-                return results.FirstOrDefault();
             }
         }
 

src/shared/TestInfrastructure/Objects/TestEnvironment.cs

@@ -80,21 +80,24 @@ public void RemoveDirectoryFromPath(string directoryPath, EnvironmentVariableTar
             Variables["PATH"] = string.Join(_envPathSeparator, Path);
         }
 
-        public string LocateExecutable(string program)
+        public bool TryLocateExecutable(string program, out string path)
         {
             if (WhichFiles.TryGetValue(program, out ICollection<string> paths))
             {
-                return paths.FirstOrDefault();
+                path = paths.First();
+                return true;
             }
 
             if (!System.IO.Path.HasExtension(program) && PlatformUtils.IsWindows())
             {
                 // If we're testing on a Windows platform, don't have a file extension, and were unable to locate
                 // the executable file.. try appending .exe.
-                return WhichFiles.TryGetValue($"{program}.exe", out paths) ? paths.FirstOrDefault() : null;
+                path = WhichFiles.TryGetValue($"{program}.exe", out paths) ? paths.First() : null;
+                return !(path is null);
             }
 
-            return null;
+            path = null;
+            return false;
         }
 
         #endregion

src/windows/Atlassian.Bitbucket.UI.Windows/ViewModels/CredentialsViewModel.cs

@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license.
+using System.Diagnostics;
 using System.Security;
 using System.Windows.Input;
 using Microsoft.Git.CredentialManager;
@@ -23,8 +24,8 @@ public CredentialsViewModel(string username)
         {
             LoginCommand = new RelayCommand(Accept, () => IsValid);
             CancelCommand = new RelayCommand(Cancel);
-            ForgotPasswordCommand = new RelayCommand(() => BrowserHelper.OpenDefaultBrowser(BitbucketResources.PasswordResetUrl));
-            SignUpCommand = new RelayCommand(() => BrowserHelper.OpenDefaultBrowser(BitbucketResources.SignUpLinkUrl));
+            ForgotPasswordCommand = new RelayCommand(() => OpenDefaultBrowser(BitbucketResources.PasswordResetUrl));
+            SignUpCommand = new RelayCommand(() => OpenDefaultBrowser(BitbucketResources.SignUpLinkUrl));
 
             LoginValidator = PropertyValidator.For(this, x => x.Login).Required(BitbucketResources.LoginRequired);
             PasswordValidator = PropertyValidator.For(this, x => x.Password).Required(BitbucketResources.PasswordRequired);

src/windows/Atlassian.Bitbucket.UI.Windows/ViewModels/OAuthViewModel.cs

@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license.
+using System.Diagnostics;
 using System.Windows.Input;
 using Microsoft.Git.CredentialManager;
 using Microsoft.Git.CredentialManager.UI;
@@ -16,9 +17,9 @@ public OAuthViewModel()
         {
             OkCommand = new RelayCommand(Accept);
             CancelCommand = new RelayCommand(Cancel);
-            LearnMoreCommand = new RelayCommand(() => BrowserHelper.OpenDefaultBrowser(BitbucketResources.TwoFactorLearnMoreLinkUrl));
-            ForgotPasswordCommand = new RelayCommand(() => BrowserHelper.OpenDefaultBrowser(BitbucketResources.PasswordResetUrl));
-            SignUpCommand = new RelayCommand(() => BrowserHelper.OpenDefaultBrowser(BitbucketResources.SignUpLinkUrl));
+            LearnMoreCommand = new RelayCommand(() => OpenDefaultBrowser(BitbucketResources.TwoFactorLearnMoreLinkUrl));
+            ForgotPasswordCommand = new RelayCommand(() => OpenDefaultBrowser(BitbucketResources.PasswordResetUrl));
+            SignUpCommand = new RelayCommand(() => OpenDefaultBrowser(BitbucketResources.SignUpLinkUrl));
         }
 
         public override string Title => BitbucketResources.OAuthWindowTitle;

src/windows/GitHub.UI.Windows/Login/Login2FaViewModel.cs

@@ -1,4 +1,5 @@
-using System.Windows.Input;
+using System.Diagnostics;
+using System.Windows.Input;
 using Microsoft.Git.CredentialManager;
 using Microsoft.Git.CredentialManager.UI;
 using Microsoft.Git.CredentialManager.UI.ViewModels;
@@ -87,7 +88,7 @@ private void Verify()
 
         private void NavigateLearnMore()
         {
-            BrowserHelper.OpenDefaultBrowser(NavigateLearnMoreUrl);
+            OpenDefaultBrowser(NavigateLearnMoreUrl);
         }
 
         public string NavigateLearnMoreUrl => "https://aka.ms/vs-core-github-auth-help";

src/windows/Shared.UI.Windows/ViewModels/WindowViewModel.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license.
 using System;
+using System.Diagnostics;
 
 namespace Microsoft.Git.CredentialManager.UI.ViewModels
 {
@@ -20,5 +21,21 @@ public void Cancel()
         {
             Canceled?.Invoke(this, EventArgs.Empty);
         }
+
+        public static void OpenDefaultBrowser(string url)
+        {
+            if (!url.StartsWith(Uri.UriSchemeHttp, StringComparison.OrdinalIgnoreCase) &&
+                !url.StartsWith(Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase))
+            {
+                throw new ArgumentException("Can only open HTTP/HTTPS URLs", nameof(url));
+            }
+
+            var psi = new ProcessStartInfo(url)
+            {
+                UseShellExecute = true
+            };
+
+            Process.Start(psi);
+        }
     }
 }