Better browser/interactive session detection inside of WSL (#1148)

Today we're gating browser-based authentication mechanisms behind a
GUI-possible session, which isn't 100% correct. When inside of a WSL
session, without WSLg enabled, it is still possible to launch a web
browser in the hosting Windows session.

Split the determination between "GUI session" and "browser possible",
and add the special case for WSL inside an interactive Windows session.

Things are complicated here because it is possible to SSH in to the WSL
instance from outside of the Windows host, meaning the Windows session
also doesn't have UI. This case we still need to prevent browser-based
options.

In order to detect these cases we need to 'punch out' to the Windows
host and run a script/cmdlet to determine the session type.

Fixes #878

Author: mjcheetham

src/shared/Core.Tests/IniFileTests.cs

@@ -0,0 +1,135 @@
+using System.Collections.Generic;
+using System.Text;
+using GitCredentialManager.Tests.Objects;
+using Xunit;
+
+namespace GitCredentialManager.Tests
+{
+    public class IniFileTests
+    {
+        [Fact]
+        public void IniSectionName_Equality()
+        {
+            var a1 = new IniSectionName("foo");
+            var b1 = new IniSectionName("foo");
+            Assert.Equal(a1,b1);
+            Assert.Equal(a1.GetHashCode(),b1.GetHashCode());
+
+            var a2 = new IniSectionName("foo");
+            var b2 = new IniSectionName("FOO");
+            Assert.Equal(a2,b2);
+            Assert.Equal(a2.GetHashCode(),b2.GetHashCode());
+
+            var a3 = new IniSectionName("foo", "bar");
+            var b3 = new IniSectionName("foo", "BAR");
+            Assert.NotEqual(a3,b3);
+            Assert.NotEqual(a3.GetHashCode(),b3.GetHashCode());
+
+            var a4 = new IniSectionName("foo", "bar");
+            var b4 = new IniSectionName("FOO", "bar");
+            Assert.Equal(a4,b4);
+            Assert.Equal(a4.GetHashCode(),b4.GetHashCode());
+        }
+
+        [Fact]
+        public void IniSerializer_Deserialize()
+        {
+            const string path = "/tmp/test.ini";
+            string iniText = @"
+[one]
+    foo = 123
+  [two]
+    foo   =   abc
+# comment
+[two ""subsection name""] # comment [section]
+    foo = this is different # comment prop = val
+
+#[notasection]
+
+    [
+[bad #section]
+recovery tests]
+[]
+    ]
+
+    [three]
+    bar = a
+    bar = b
+    # comment
+    bar = c
+    empty =
+[TWO]
+    foo = hello
+    widget = ""Hello, World!""
+[four]
+[five]
+    prop1 = ""this hash # is inside quotes""
+    prop2 = ""this hash # is inside quotes"" # this line has two hashes
+    prop3 = ""   this dquoted string has three spaces around   ""
+    #prop4 = this property has been commented-out
+";
+
+            var fs = new TestFileSystem
+            {
+                Files = { [path] = Encoding.UTF8.GetBytes(iniText) }
+            };
+
+            IniFile ini = IniSerializer.Deserialize(fs, path);
+
+            Assert.Equal(6, ini.Sections.Count);
+
+            AssertSection(ini, "one", out IniSection one);
+            Assert.Equal(1, one.Properties.Count);
+            AssertProperty(one, "foo", "123");
+
+            AssertSection(ini, "two", out IniSection twoA);
+            Assert.Equal(3, twoA.Properties.Count);
+            AssertProperty(twoA, "foo", "hello");
+            AssertProperty(twoA, "widget", "Hello, World!");
+
+            AssertSection(ini, "two", "subsection name", out IniSection twoB);
+            Assert.Equal(1, twoB.Properties.Count);
+            AssertProperty(twoB, "foo", "this is different");
+
+            AssertSection(ini, "three", out IniSection three);
+            Assert.Equal(4, three.Properties.Count);
+            AssertMultiProperty(three, "bar", "a", "b", "c");
+            AssertProperty(three, "empty", "");
+
+            AssertSection(ini, "four", out IniSection four);
+            Assert.Equal(0, four.Properties.Count);
+
+            AssertSection(ini, "five", out IniSection five);
+            Assert.Equal(3, five.Properties.Count);
+            AssertProperty(five, "prop1", "this hash # is inside quotes");
+            AssertProperty(five, "prop2", "this hash # is inside quotes");
+            AssertProperty(five, "prop3", "   this dquoted string has three spaces around   ");
+        }
+
+        private static void AssertSection(IniFile file, string name, out IniSection section)
+        {
+            Assert.True(file.TryGetSection(name, out section));
+            Assert.Equal(name, section.Name.Name);
+            Assert.Null(section.Name.SubName);
+        }
+
+        private static void AssertSection(IniFile file, string name, string subName, out IniSection section)
+        {
+            Assert.True(file.TryGetSection(name, subName, out section));
+            Assert.Equal(name, section.Name.Name);
+            Assert.Equal(subName, section.Name.SubName);
+        }
+
+        private static void AssertProperty(IniSection section, string name, string value)
+        {
+            Assert.True(section.TryGetProperty(name, out var actualValue));
+            Assert.Equal(value, actualValue);
+        }
+
+        private static void AssertMultiProperty(IniSection section, string name, params string[] values)
+        {
+            Assert.True(section.TryGetMultiProperty(name, out IEnumerable<string> actualValues));
+            Assert.Equal(values, actualValues);
+        }
+    }
+}

src/shared/Core/Authentication/MicrosoftAuthentication.cs

@@ -508,14 +508,14 @@ private void EnsureCanUseEmbeddedWebView()
         private bool CanUseSystemWebView(IPublicClientApplication app, Uri redirectUri)
         {
             // MSAL requires the application redirect URI is a loopback address to use the System WebView
-            return Context.SessionManager.IsDesktopSession && app.IsSystemWebViewAvailable && redirectUri.IsLoopback;
+            return Context.SessionManager.IsWebBrowserAvailable && app.IsSystemWebViewAvailable && redirectUri.IsLoopback;
         }
 
         private void EnsureCanUseSystemWebView(IPublicClientApplication app, Uri redirectUri)
         {
-            if (!Context.SessionManager.IsDesktopSession)
+            if (!Context.SessionManager.IsWebBrowserAvailable)
             {
-                throw new InvalidOperationException("System web view is not available without a desktop session.");
+                throw new InvalidOperationException("System web view is not available without a way to start a browser.");
             }
 
             if (!app.IsSystemWebViewAvailable)

src/shared/Core/BrowserUtils.cs

@@ -25,9 +25,10 @@ public static void OpenDefaultBrowser(IEnvironment environment, Uri uri)
 
             string url = uri.ToString();
 
-            ProcessStartInfo psi = null;
+            ProcessStartInfo psi;
             if (PlatformUtils.IsLinux())
             {
+                //
                 // 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
@@ -40,29 +41,17 @@ public static void OpenDefaultBrowser(IEnvironment environment, Uri uri)
                 // We try and use the same 'shell execute' utilities as the Framework does,
                 // searching for them in the same order until we find one.
                 //
-                // One additional 'shell execute' utility we also attempt to use is `wslview`
-                // that is commonly found on WSL (Windows Subsystem for Linux) distributions that
-                // opens the browser on the Windows host.
-                foreach (string shellExec in new[] {"xdg-open", "gnome-open", "kfmclient", "wslview"})
+                if (!TryGetLinuxShellExecuteHandler(environment, out string shellExecPath))
                 {
-                    if (environment.TryLocateExecutable(shellExec, out string shellExecPath))
-                    {
-                        psi = new ProcessStartInfo(shellExecPath, url)
-                        {
-                            RedirectStandardOutput = true,
-                            // Ok to redirect stderr for non-git-related processes
-                            RedirectStandardError = true
-                        };
-
-                        // We found a way to open the URI; stop searching!
-                        break;
-                    }
+                    throw new Exception("Failed to locate a utility to launch the default web browser.");
                 }
 
-                if (psi is null)
+                psi = new ProcessStartInfo(shellExecPath, url)
                 {
-                    throw new Exception("Failed to locate a utility to launch the default web browser.");
-                }
+                    RedirectStandardOutput = true,
+                    // Ok to redirect stderr for non-git-related processes
+                    RedirectStandardError = true
+                };
             }
             else
             {
@@ -77,5 +66,23 @@ public static void OpenDefaultBrowser(IEnvironment environment, Uri uri)
             // is no need to add the extra overhead associated with ChildProcess here.
             Process.Start(psi);
         }
+
+        public static bool TryGetLinuxShellExecuteHandler(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/CommandContext.cs

@@ -102,8 +102,8 @@ public CommandContext()
             if (PlatformUtils.IsWindows())
             {
                 FileSystem        = new WindowsFileSystem();
-                SessionManager    = new WindowsSessionManager();
                 Environment       = new WindowsEnvironment(FileSystem);
+                SessionManager    = new WindowsSessionManager(Environment, FileSystem);
                 ProcessManager    = new WindowsProcessManager(Trace2);
                 Terminal          = new WindowsTerminal(Trace);
                 string gitPath    = GetGitPath(Environment, FileSystem, Trace);
@@ -118,7 +118,7 @@ public CommandContext()
             else if (PlatformUtils.IsMacOS())
             {
                 FileSystem        = new MacOSFileSystem();
-                SessionManager    = new MacOSSessionManager();
+                SessionManager    = new MacOSSessionManager(Environment, FileSystem);
                 Environment       = new MacOSEnvironment(FileSystem);
                 ProcessManager    = new ProcessManager(Trace2);
                 Terminal          = new MacOSTerminal(Trace);
@@ -134,8 +134,7 @@ public CommandContext()
             else if (PlatformUtils.IsLinux())
             {
                 FileSystem        = new LinuxFileSystem();
-                // TODO: support more than just 'Posix' or X11
-                SessionManager    = new PosixSessionManager();
+                SessionManager    = new LinuxSessionManager(Environment, FileSystem);
                 Environment       = new PosixEnvironment(FileSystem);
                 ProcessManager    = new ProcessManager(Trace2);
                 Terminal          = new LinuxTerminal(Trace);

src/shared/Core/EnvironmentBase.cs

@@ -53,18 +53,45 @@ public interface IEnvironment
         /// <param name="target">Target level of environment variable to set (Machine, Process, or User).</param>
         void SetEnvironmentVariable(string variable, string value,
             EnvironmentVariableTarget target = EnvironmentVariableTarget.Process);
+
+        /// <summary>
+        /// Refresh the current process environment variables. See <see cref="Variables"/>.
+        /// </summary>
+        /// <remarks>This is automatically called after <see cref="SetEnvironmentVariable"/>.</remarks>
+        void Refresh();
     }
 
     public abstract class EnvironmentBase : IEnvironment
     {
+        private IReadOnlyDictionary<string, string> _variables;
+
         protected EnvironmentBase(IFileSystem fileSystem)
         {
             EnsureArgument.NotNull(fileSystem, nameof(fileSystem));
-
             FileSystem = fileSystem;
         }
 
-        public IReadOnlyDictionary<string, string> Variables { get; protected set; }
+        internal EnvironmentBase(IFileSystem fileSystem, IReadOnlyDictionary<string, string> variables)
+            : this(fileSystem)
+        {
+            EnsureArgument.NotNull(variables, nameof(variables));
+            _variables = variables;
+        }
+
+        public IReadOnlyDictionary<string, string> Variables
+        {
+            get
+            {
+                // Variables are lazily loaded
+                if (_variables is null)
+                {
+                    Refresh();
+                }
+
+                Debug.Assert(_variables != null);
+                return _variables;
+            }
+        }
 
         protected IFileSystem FileSystem { get; }
 
@@ -126,9 +153,22 @@ internal virtual bool TryLocateExecutable(string program, ICollection<string> pa
         public void SetEnvironmentVariable(string variable, string value,
             EnvironmentVariableTarget target = EnvironmentVariableTarget.Process)
         {
-            if (Variables.Keys.Contains(variable)) return;
+            // Don't bother setting the variable if it already has the same value
+            if (Variables.TryGetValue(variable, out var currentValue) &&
+                StringComparer.Ordinal.Equals(currentValue, value))
+            {
+                return;
+            }
+
             Environment.SetEnvironmentVariable(variable, value, target);
-            Variables = GetCurrentVariables();
+
+            // Immediately refresh the variables so that the new value is available to callers using IEnvironment
+            Refresh();
+        }
+
+        public void Refresh()
+        {
+            _variables = GetCurrentVariables();
         }
 
         protected abstract IReadOnlyDictionary<string, string> GetCurrentVariables();
@@ -151,5 +191,18 @@ public static string LocateExecutable(this IEnvironment environment, string prog
 
             throw new Exception($"Failed to locate '{program}' executable on the path.");
         }
+
+        /// <summary>
+        /// Retrieves the value of an environment variable from the current process.
+        /// </summary>
+        /// <param name="environment">The <see cref="IEnvironment"/>.</param>
+        /// <param name="variable">The name of the environment variable.</param>
+        /// <returns>
+        /// The value of the environment variable specified by variable, or null if the environment variable is not found.
+        /// </returns>
+        public static string GetEnvironmentVariable(this IEnvironment environment, string variable)
+        {
+            return environment.Variables.TryGetValue(variable, out string value) ? value : null;
+        }
     }
 }

src/shared/Core/FileSystem.cs

@@ -84,6 +84,29 @@ public interface IFileSystem
         /// </returns>
         IEnumerable<string> EnumerateFiles(string path, string searchPattern);
 
+        /// <summary>
+        /// Returns an enumerable collection of directory full names in a specified path.
+        /// </summary>
+        /// <param name="path">The relative or absolute path to the directory to search. This string is not case-sensitive.</param>
+        /// <returns>
+        /// An enumerable collection of the full names (including paths) for the directories
+        /// in the directory specified by path.
+        /// </returns>
+        IEnumerable<string> EnumerateDirectories(string path);
+
+        /// <summary>
+        /// Opens a text file, reads all the text in the file, and then closes the file
+        /// </summary>
+        /// <param name="path">The file to open for reading.</param>
+        /// <returns>A string containing all the text in the file.</returns>
+        string ReadAllText(string path);
+
+        /// <summary>
+        /// Opens a text file, reads all lines of the file, and then closes the file.
+        /// </summary>
+        /// <param name="path">The file to open for reading.</param>
+        /// <returns>A string array containing all lines of the file.</returns>
+        string[] ReadAllLines(string path);
     }
 
     /// <summary>
@@ -111,5 +134,11 @@ public Stream OpenFileStream(string path, FileMode fileMode, FileAccess fileAcce
         public void DeleteFile(string path) => File.Delete(path);
 
         public IEnumerable<string> EnumerateFiles(string path, string searchPattern) => Directory.EnumerateFiles(path, searchPattern);
+
+        public IEnumerable<string> EnumerateDirectories(string path) => Directory.EnumerateDirectories(path);
+
+        public string ReadAllText(string path) => File.ReadAllText(path);
+
+        public string[] ReadAllLines(string path) => File.ReadAllLines(path);
     }
 }