Merged PR 585338: Cut new release including security fix

**Changes:**

1. Fix NTLM proxy authentication
2. Fix reading empty Git configuration entry values
3. Allow users to select the type of interactive authentication flow for the Microsoft auth stack

Author: mjcheetham

docs/configuration.md

@@ -226,3 +226,46 @@ git config --global credential.plaintextStorePath /mnt/external-drive/credential
 ```
 
 **Also see: [GCM_PLAINTEXT_STORE_PATH](environment.md#GCM_PLAINTEXT_STORE_PATH)**
+
+---
+
+### credential.msauthFlow
+
+Specify which authentication flow should be used when performing Microsoft authentication and an interactive flow is required.
+
+Defaults to the value `auto`.
+
+**Note:** This setting will be ignored if a native authentication helper is configured and available. See [`credential.msauthHelper`](#credentialmsauthhelper) for more information.
+
+Value|Credential Store
+-|-
+`auto` _(default)_|Select the best option depending on the current environment and platform.
+`embedded`|Show a window with embedded web view control.
+`system`|Open the user's default web browser.
+`devicecode`|Show a device code.
+
+#### Example
+
+```shell
+git config --global credential.msauthFlow devicecode
+```
+
+**Also see: [GCM_MSAUTH_FLOW](environment.md#GCM_MSAUTH_FLOW)**
+
+---
+
+### credential.msauthHelper
+
+Full path to an external 'helper' tool to which Microsoft authentication should be delegated.
+
+On macOS this defaults to the included native `Microsoft.Authentication.Helper` tool. On all other platforms this is not set.
+
+**Note:** If a helper is set and available then all Microsoft authentication will be delegated to this helper and the [`credential.msauthFlow`](#credentialmsauthflow) setting will be ignored. Setting the value to the empty string (`""`) will unset any default helper.
+
+#### Example
+
+```shell
+git config --global credential.msauthHelper "C:\path\to\helper.exe"
+```
+
+**Also see: [GCM_MSAUTH_HELPER](environment.md#GCM_MSAUTH_HELPER)**

docs/environment.md

@@ -373,3 +373,58 @@ export GCM_PLAINTEXT_STORE_PATH=/mnt/external-drive/credentials
 ```
 
 **Also see: [credential.plaintextStorePath](configuration.md#credentialplaintextstorepath)**
+
+---
+
+### GCM_MSAUTH_FLOW
+
+Specify which authentication flow should be used when performing Microsoft authentication and an interactive flow is required.
+
+Defaults to the value `auto`.
+
+**Note:** This setting will be ignored if a native authentication helper is configured and available. See [`GCM_MSAUTH_HELPER`](#gcm_msauth_helper) for more information.
+
+Value|Credential Store
+-|-
+`auto` _(default)_|Select the best option depending on the current environment and platform.
+`embedded`|Show a window with embedded web view control.
+`system`|Open the user's default web browser.
+`devicecode`|Show a device code.
+
+##### Windows
+
+```batch
+SET GCM_MSAUTH_FLOW="devicecode"
+```
+
+##### macOS/Linux
+
+```bash
+export GCM_MSAUTH_FLOW="devicecode"
+```
+
+**Also see: [credential.msauthFlow](configuration.md#credentialmsauthflow)**
+
+---
+
+### GCM_MSAUTH_HELPER
+
+Full path to an external 'helper' tool to which Microsoft authentication should be delegated.
+
+On macOS this defaults to the included native `Microsoft.Authentication.Helper` tool. On all other platforms this is not set.
+
+**Note:** If a helper is set and available then all Microsoft authentication will be delegated to this helper and the [`GCM_MSAUTH_FLOW`](#gcm_msauth_flow) setting will be ignored. Setting the value to the empty string (`""`) will unset any default helper.
+
+##### Windows
+
+```batch
+SET GCM_MSAUTH_HELPER="C:\path\to\helper.exe"
+```
+
+##### macOS/Linux
+
+```bash
+export GCM_MSAUTH_HELPER="/usr/local/bin/msauth-helper"
+```
+
+**Also see: [credential.msauthHelper](configuration.md#credentialmsauthhelper)**

src/shared/Microsoft.AzureRepos/AzureDevOpsConstants.cs

@@ -13,9 +13,8 @@ internal static class AzureDevOpsConstants
         // We share this to be able to consume existing access tokens from the VS caches
         public const string AadClientId = "872cd9fa-d31f-45e0-9eab-6e460a02d1f1";
 
-        // Standard redirect URI for native client 'v1 protocol' applications
-        // https://docs.microsoft.com/en-us/azure/active-directory/develop/v1-protocols-oauth-code#request-an-authorization-code
-        public static readonly Uri AadRedirectUri = new Uri("urn:ietf:wg:oauth:2.0:oob");
+        // Redirect URI specified by the Visual Studio application configuration
+        public static readonly Uri AadRedirectUri = new Uri("http://localhost");
 
         public const string VstsHostSuffix = ".visualstudio.com";
         public const string AzureDevOpsHost = "dev.azure.com";

src/shared/Microsoft.Git.CredentialManager.Tests/EnvironmentTests.cs

@@ -0,0 +1,71 @@
+using System.Collections.Generic;
+using Microsoft.Git.CredentialManager.Interop.Windows;
+using Microsoft.Git.CredentialManager.Tests.Objects;
+using Xunit;
+
+namespace Microsoft.Git.CredentialManager.Tests
+{
+    public class EnvironmentTests
+    {
+        [PlatformFact(Platforms.Windows)]
+        public void WindowsEnvironment_TryLocateExecutable_NotExists_ReturnFalse()
+        {
+            string pathVar = @"C:\Users\john.doe\bin;C:\Windows\system32;C:\Windows";
+            string execName = "foo.exe";
+            var fs = new TestFileSystem();
+            var envars = new Dictionary<string, string> {["PATH"] = pathVar};
+            var env = new WindowsEnvironment(fs, envars);
+
+            bool actualResult = env.TryLocateExecutable(execName, out string actualPath);
+
+            Assert.False(actualResult);
+            Assert.Null(actualPath);
+        }
+
+        [PlatformFact(Platforms.Windows)]
+        public void WindowsEnvironment_TryLocateExecutable_Windows_Exists_ReturnTrueAndPath()
+        {
+            string pathVar = @"C:\Users\john.doe\bin;C:\Windows\system32;C:\Windows";
+            string execName = "foo.exe";
+            string expectedPath = @"C:\Windows\system32\foo.exe";
+            var fs = new TestFileSystem
+            {
+                Files = new Dictionary<string, byte[]>
+                {
+                    [@"C:\Windows\system32\foo.exe"] = new byte[0],
+                }
+            };
+            var envars = new Dictionary<string, string> {["PATH"] = pathVar};
+            var env = new WindowsEnvironment(fs, envars);
+
+            bool actualResult = env.TryLocateExecutable(execName, out string actualPath);
+
+            Assert.True(actualResult);
+            Assert.Equal(expectedPath, actualPath);
+        }
+
+        [PlatformFact(Platforms.Windows)]
+        public void WindowsEnvironment_TryLocateExecutable_Windows_ExistsMultiple_ReturnTrueAndFirstPath()
+        {
+            string pathVar = @"C:\Users\john.doe\bin;C:\Windows\system32;C:\Windows";
+            string execName = "foo.exe";
+            string expectedPath = @"C:\Users\john.doe\bin\foo.exe";
+            var fs = new TestFileSystem
+            {
+                Files = new Dictionary<string, byte[]>
+                {
+                    [@"C:\Users\john.doe\bin\foo.exe"] = new byte[0],
+                    [@"C:\Windows\system32\foo.exe"] = new byte[0],
+                    [@"C:\Windows\foo.exe"] = new byte[0],
+                }
+            };
+            var envars = new Dictionary<string, string> {["PATH"] = pathVar};
+            var env = new WindowsEnvironment(fs, envars);
+
+            bool actualResult = env.TryLocateExecutable(execName, out string actualPath);
+
+            Assert.True(actualResult);
+            Assert.Equal(expectedPath, actualPath);
+        }
+    }
+}

src/shared/Microsoft.Git.CredentialManager.Tests/HttpClientFactoryTests.cs

@@ -118,6 +118,104 @@ public void HttpClientFactory_TryCreateProxy_ProxyWithCredentials_ReturnsTrueOut
             Assert.Equal(proxyPass, configuredCredentials.Password);
         }
 
+        [Fact]
+        public void HttpClientFactory_TryCreateProxy_ProxyWithNonEmptyUserAndEmptyPass_ReturnsTrueOutProxyWithUrlConfiguredCredentials()
+        {
+            const string proxyScheme = "https";
+            const string proxyUser   = "john.doe";
+            const string proxyHost   = "proxy.example.com/git";
+            const string repoPath    = "/tmp/repos/foo";
+            const string repoRemote  = "https://remote.example.com/foo.git";
+
+            string proxyConfigString = $"{proxyScheme}://{proxyUser}:@{proxyHost}";
+            string expectedProxyUrl  = $"{proxyScheme}://{proxyHost}";
+            var repoRemoteUri = new Uri(repoRemote);
+
+            var settings = new TestSettings
+            {
+                RemoteUri = repoRemoteUri,
+                RepositoryPath = repoPath,
+                ProxyConfiguration = new Uri(proxyConfigString)
+            };
+            var httpFactory = new HttpClientFactory(Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());
+
+            bool result = httpFactory.TryCreateProxy(out IWebProxy proxy);
+
+            Assert.True(result);
+            Assert.NotNull(proxy);
+            var configuredProxyUrl = proxy.GetProxy(repoRemoteUri);
+            Assert.Equal(expectedProxyUrl, configuredProxyUrl.ToString());
+
+            Assert.NotNull(proxy.Credentials);
+            Assert.IsType<NetworkCredential>(proxy.Credentials);
+            var configuredCredentials = (NetworkCredential) proxy.Credentials;
+            Assert.Equal(proxyUser, configuredCredentials.UserName);
+            Assert.True(string.IsNullOrWhiteSpace(configuredCredentials.Password));
+        }
+
+        [Fact]
+        public void HttpClientFactory_TryCreateProxy_ProxyWithEmptyUserAndNonEmptyPass_ReturnsTrueOutProxyWithUrlConfiguredCredentials()
+        {
+            const string proxyScheme = "https";
+            const string proxyPass   = "letmein";
+            const string proxyHost   = "proxy.example.com/git";
+            const string repoPath    = "/tmp/repos/foo";
+            const string repoRemote  = "https://remote.example.com/foo.git";
+
+            string proxyConfigString = $"{proxyScheme}://:{proxyPass}@{proxyHost}";
+            string expectedProxyUrl  = $"{proxyScheme}://{proxyHost}";
+            var repoRemoteUri = new Uri(repoRemote);
+
+            var settings = new TestSettings
+            {
+                RemoteUri = repoRemoteUri,
+                RepositoryPath = repoPath,
+                ProxyConfiguration = new Uri(proxyConfigString)
+            };
+            var httpFactory = new HttpClientFactory(Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());
+
+            bool result = httpFactory.TryCreateProxy(out IWebProxy proxy);
+
+            Assert.True(result);
+            Assert.NotNull(proxy);
+            var configuredProxyUrl = proxy.GetProxy(repoRemoteUri);
+            Assert.Equal(expectedProxyUrl, configuredProxyUrl.ToString());
+
+            Assert.NotNull(proxy.Credentials);
+            Assert.IsType<NetworkCredential>(proxy.Credentials);
+            var configuredCredentials = (NetworkCredential) proxy.Credentials;
+            Assert.True(string.IsNullOrWhiteSpace(configuredCredentials.UserName));
+            Assert.Equal(proxyPass, configuredCredentials.Password);
+        }
+
+        [Fact]
+        public void HttpClientFactory_TryCreateProxy_ProxyEmptyUserAndEmptyPass_ReturnsTrueOutProxyWithUrlDefaultCredentials()
+        {
+            const string repoPath = "/tmp/repos/foo";
+            const string repoRemote = "https://remote.example.com/foo.git";
+            var repoRemoteUri = new Uri(repoRemote);
+
+            string proxyConfigString = "https://:@proxy.example.com/git";
+            string expectedProxyUrl = "https://proxy.example.com/git";
+
+            var settings = new TestSettings
+            {
+                RemoteUri = repoRemoteUri,
+                RepositoryPath = repoPath,
+                ProxyConfiguration = new Uri(proxyConfigString)
+            };
+            var httpFactory = new HttpClientFactory(Mock.Of<ITrace>(), settings, Mock.Of<IStandardStreams>());
+
+            bool result = httpFactory.TryCreateProxy(out IWebProxy proxy);
+
+            Assert.True(result);
+            Assert.NotNull(proxy);
+            var configuredProxyUrl = proxy.GetProxy(repoRemoteUri);
+            Assert.Equal(expectedProxyUrl, configuredProxyUrl.ToString());
+
+            AssertDefaultCredentials(proxy.Credentials);
+        }
+
         private static void AssertDefaultCredentials(ICredentials credentials)
         {
             var netCred = (NetworkCredential) credentials;

src/shared/Microsoft.Git.CredentialManager/Authentication/AuthenticationBase.cs

@@ -106,6 +106,13 @@ protected bool TryFindHelperExecutablePath(string envar, string configName, stri
                 helperName = PlatformUtils.IsWindows() ? $"{defaultValue}.exe" : defaultValue;
             }
 
+            // If the user set the helper override to the empty string then they are signalling not to use a helper
+            if (string.IsNullOrEmpty(helperName))
+            {
+                path = null;
+                return false;
+            }
+
             if (Path.IsPathRooted(helperName))
             {
                 path = helperName;