Better browser/interactive session detection inside of WSL

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);
     }
 }