sessionmanager: move browserutils.openbrowser

Given that the ISessionManager has the ability to determine if a web
browser is available (via `IsWebBrowserAvailable`), it's only natural
that this is the component that should also be responsible for
actually launching the browser.

We remove the `BrowserUtils` class and transpose its only method
(`OpenDefaultBrowser`) to the session manager component. This means we
can keep the Linux-specific behaviour in a scoped component.

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

Author: mjcheetham

src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs

@@ -259,7 +259,7 @@ public async Task<OAuth2TokenResult> CreateOAuthCredentialsAsync(InputArguments
                 FailureResponseHtmlFormat = BitbucketResources.AuthenticationResponseFailureHtmlFormat
             };
 
-            var browser = new OAuth2SystemWebBrowser(Context.Environment, browserOptions);
+            var browser = new OAuth2SystemWebBrowser(Context.SessionManager, browserOptions);
             var oauth2Client = _oauth2ClientRegistry.Get(input);
 
             var authCodeResult = await oauth2Client.GetAuthorizationCodeAsync(browser, CancellationToken.None);

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

@@ -10,9 +10,9 @@ public class OAuth2SystemWebBrowserTests
     [Fact]
     public void OAuth2SystemWebBrowser_UpdateRedirectUri_NonLoopback_ThrowsError()
     {
-        var env = new TestEnvironment();
+        var sm = new TestSessionManager();
         var options = new OAuth2WebBrowserOptions();
-        var browser = new OAuth2SystemWebBrowser(env, options);
+        var browser = new OAuth2SystemWebBrowser(sm, options);
 
         Assert.Throws<ArgumentException>(() => browser.UpdateRedirectUri(new Uri("http://example.com")));
     }
@@ -28,9 +28,9 @@ public void OAuth2SystemWebBrowser_UpdateRedirectUri_NonLoopback_ThrowsError()
     [InlineData("http://127.0.0.7:1234/oauth-callback/", "http://127.0.0.7:1234/oauth-callback/")]
     public void OAuth2SystemWebBrowser_UpdateRedirectUri_SpecificPort(string input, string expected)
     {
-        var env = new TestEnvironment();
+        var sm = new TestSessionManager();
         var options = new OAuth2WebBrowserOptions();
-        var browser = new OAuth2SystemWebBrowser(env, options);
+        var browser = new OAuth2SystemWebBrowser(sm, options);
 
         Uri actualUri = browser.UpdateRedirectUri(new Uri(input));
 
@@ -48,9 +48,9 @@ public void OAuth2SystemWebBrowser_UpdateRedirectUri_SpecificPort(string input,
     [InlineData("http://127.0.0.7/oauth-callback/")]
     public void OAuth2SystemWebBrowser_UpdateRedirectUri_AnyPort(string input)
     {
-        var env = new TestEnvironment();
+        var sm = new TestSessionManager();
         var options = new OAuth2WebBrowserOptions();
-        var browser = new OAuth2SystemWebBrowser(env, options);
+        var browser = new OAuth2SystemWebBrowser(sm, options);
 
         var inputUri = new Uri(input);
         Uri actualUri = browser.UpdateRedirectUri(inputUri);

src/shared/Core/Authentication/MicrosoftAuthentication.cs

@@ -336,7 +336,7 @@ private async Task<bool> UseDefaultAccountAsync(string userName)
                     }
                 }
 
-                var viewModel = new DefaultAccountViewModel(Context.Environment)
+                var viewModel = new DefaultAccountViewModel(Context.SessionManager)
                 {
                     UserName = userName
                 };

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

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Net;
 using System.Net.Sockets;
 using System.Threading;
@@ -33,15 +32,15 @@ public class OAuth2WebBrowserOptions
 
     public class OAuth2SystemWebBrowser : IOAuth2WebBrowser
     {
-        private readonly IEnvironment _environment;
+        private readonly ISessionManager _sessionManager;
         private readonly OAuth2WebBrowserOptions _options;
 
-        public OAuth2SystemWebBrowser(IEnvironment environment, OAuth2WebBrowserOptions options)
+        public OAuth2SystemWebBrowser(ISessionManager sessionManager, OAuth2WebBrowserOptions options)
         {
-            EnsureArgument.NotNull(environment, nameof(environment));
+            EnsureArgument.NotNull(sessionManager, nameof(sessionManager));
             EnsureArgument.NotNull(options, nameof(options));
 
-            _environment = environment;
+            _sessionManager = sessionManager;
             _options = options;
         }
 
@@ -71,7 +70,7 @@ public async Task<Uri> GetAuthenticationCodeAsync(Uri authorizationUri, Uri redi
 
             Task<Uri> interceptTask = InterceptRequestsAsync(redirectUri, ct);
 
-            BrowserUtils.OpenDefaultBrowser(_environment, authorizationUri);
+            _sessionManager.OpenBrowser(authorizationUri);
 
             return await interceptTask;
         }

src/shared/Core/Authentication/OAuthAuthentication.cs

@@ -187,7 +187,7 @@ public async Task<OAuth2TokenResult> GetTokenByBrowserAsync(OAuth2Client client,
             }
 
             var browserOptions = new OAuth2WebBrowserOptions();
-            var browser = new OAuth2SystemWebBrowser(Context.Environment, browserOptions);
+            var browser = new OAuth2SystemWebBrowser(Context.SessionManager, browserOptions);
             var authCode = await client.GetAuthorizationCodeAsync(scopes, browser, CancellationToken.None);
             return await client.GetTokenByAuthorizationCodeAsync(authCode, CancellationToken.None);
         }
@@ -241,7 +241,7 @@ public async Task<OAuth2TokenResult> GetTokenByDeviceCodeAsync(OAuth2Client clie
 
         private Task ShowDeviceCodeViaUiAsync(OAuth2DeviceCodeResult dcr, CancellationToken ct)
         {
-            var viewModel = new DeviceCodeViewModel(Context.Environment)
+            var viewModel = new DeviceCodeViewModel(Context.SessionManager)
             {
                 UserCode = dcr.UserCode,
                 VerificationUrl = dcr.VerificationUri.ToString(),

src/shared/Core/BrowserUtils.cs

@@ -1,3 +1,6 @@
+using System;
+using System.Diagnostics;
+
 namespace GitCredentialManager
 {
     public interface ISessionManager
@@ -13,6 +16,26 @@ public interface ISessionManager
         /// </summary>
         /// <returns>True if the session can display a web browser, false otherwise.</returns>
         bool IsWebBrowserAvailable { get; }
+
+        /// <summary>
+        /// Open the system web browser to the specified URL.
+        /// </summary>
+        /// <param name="uri"><see cref="Uri"/> to open the browser at.</param>
+        /// <exception cref="InvalidOperationException">Thrown if <see cref="IsWebBrowserAvailable"/> is false.</exception>
+        void OpenBrowser(Uri uri);
+    }
+
+    public static class SessionManagerExtensions
+    {
+        public static void OpenBrowser(this ISessionManager sm, string url)
+        {
+            if (!Uri.TryCreate(url, UriKind.Absolute, out Uri uri))
+            {
+                throw new ArgumentException($"Not a valid URI: '{url}'");
+            }
+
+            sm.OpenBrowser(uri);
+        }
     }
 
     public abstract class SessionManager : ISessionManager
@@ -32,5 +55,22 @@ protected SessionManager(IEnvironment env, IFileSystem fs)
         public abstract bool IsDesktopSession { get; }
 
         public virtual bool IsWebBrowserAvailable => IsDesktopSession;
+
+        public void OpenBrowser(Uri uri)
+        {
+            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));
+            }
+
+            OpenBrowserInternal(uri.ToString());
+        }
+
+        protected virtual void OpenBrowserInternal(string url)
+        {
+            var psi = new ProcessStartInfo(url) { UseShellExecute = true };
+            Process.Start(psi);
+        }
     }
 }

src/shared/Core/ISessionManager.cs

@@ -1,3 +1,5 @@
+using System;
+using System.Diagnostics;
 using GitCredentialManager.Interop.Posix;
 
 namespace GitCredentialManager.Interop.Linux;
@@ -18,15 +20,45 @@ public override bool IsWebBrowserAvailable
             return _isWebBrowserAvailable ??= GetWebBrowserAvailable();
         }
     }
-    
+
+    protected override void OpenBrowserInternal(string url)
+    {
+        //
+        // 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.
+        //
+        if (!TryGetShellExecuteHandler(Environment, out string shellExecPath))
+        {
+            throw new Exception("Failed to locate a utility to launch the default web browser.");
+        }
+
+        var psi = new ProcessStartInfo(shellExecPath, url)
+        {
+            RedirectStandardOutput = true,
+            // Ok to redirect stderr for non-git-related processes
+            RedirectStandardError = true
+        };
+
+        Process.Start(psi);
+    }
+
     private bool GetWebBrowserAvailable()
     {
         // If this is a Windows Subsystem for Linux distribution we may
         // be able to launch the web browser of the host Windows OS.
         if (WslUtils.IsWslDistribution(Environment, FileSystem, out _))
         {
             // We need a shell execute handler to be able to launch to browser
-            if (!BrowserUtils.TryGetLinuxShellExecuteHandler(Environment, out _))
+            if (!TryGetShellExecuteHandler(Environment, out _))
             {
                 return false;
             }
@@ -61,4 +93,22 @@ private bool GetWebBrowserAvailable()
         // We require an interactive desktop session to be able to launch a browser
         return IsDesktopSession;
     }
+
+    private static bool TryGetShellExecuteHandler(IEnvironment env, out string shellExecPath)
+    {
+        // One additional 'shell execute' utility we also attempt to use over the Framework
+        // is `wslview` that is commonly found on WSL (Windows Subsystem for Linux) distributions
+        // that opens the browser on the Windows host.
+        string[] shellHandlers = { "xdg-open", "gnome-open", "kfmclient", WslUtils.WslViewShellHandlerName };
+        foreach (string shellExec in shellHandlers)
+        {
+            if (env.TryLocateExecutable(shellExec, out shellExecPath))
+            {
+                return true;
+            }
+        }
+
+        shellExecPath = null;
+        return false;
+    }
 }

src/shared/Core/Interop/Linux/LinuxSessionManager.cs

@@ -28,7 +28,7 @@ protected DefaultAccountCommand(ICommandContext context)
 
     private async Task<int> ExecuteAsync(string title, string userName, bool noLogo)
     {
-        var viewModel = new DefaultAccountViewModel(Context.Environment)
+        var viewModel = new DefaultAccountViewModel(Context.SessionManager)
         {
             Title = !string.IsNullOrWhiteSpace(title)
                 ? title

src/shared/Core/UI/Commands/DefaultAccountCommand.cs

@@ -24,7 +24,7 @@ protected DeviceCodeCommand(ICommandContext context)
 
         private async Task<int> ExecuteAsync(string code, string url, bool noLogo)
         {
-            var viewModel = new DeviceCodeViewModel(Context.Environment)
+            var viewModel = new DeviceCodeViewModel(Context.SessionManager)
             {
                 UserCode = code,
                 VerificationUrl = url,