GCM release 2.3.0 (#1361)

**Changes since 2.2.2:** - Fix a GCM/Git Trace2 file locking issue - Issue: #1323 - PR: #1340 - Remove symlinks to `git-credential-manager-core` exe - Issue: #1322 - PR: #1327 - Add fallback http uri to `diagnose` command - Issue: #1215 - PR: #1339 - Workaround MSAL tenant issue with silent auth - Issue: #1297 - PR: #1321
git-ecosystem · Aug 1, 2023 · 58e34e3 · 58e34e3
2 parents 5d7e823 + 8c430c9
commit 58e34e3
Show file tree

Hide file tree

Showing 17 changed files with 204 additions and 86 deletions.
diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-2.2.2.0
+2.3.0.0
diff --git a/docs/install.md b/docs/install.md
@@ -205,7 +205,7 @@ the preferred install method for Linux because you can use it to install on any
 distribution][dotnet-supported-distributions]. You
 can also use this method on macOS if you so choose.
 
-**Note:** Make sure you have installed [version 6.0 of the .NET
+**Note:** Make sure you have installed [version 7.0 of the .NET
 SDK][dotnet-install] before attempting to run the following `dotnet tool`
 commands. After installing, you will also need to follow the output instructions
 to add the tools directory to your `PATH`.

diff --git a/docs/multiple-users.md b/docs/multiple-users.md
@@ -93,7 +93,7 @@ which you should also be aware of if you're using it.
 
 You can use the `github [list | login | logout]` commands to manage your GitHub
 accounts. These commands are documented in the [command-line usage][cli-usage]
-or by running `git-credential-manager github --help`.
+or by running `git credential-manager github --help`.
 
 ## TL;DR: Tell GCM to remember which account to use
 

diff --git a/src/linux/Packaging.Linux/build.sh b/src/linux/Packaging.Linux/build.sh
@@ -70,12 +70,6 @@ if [ $INSTALL_FROM_SOURCE = true ]; then
             "$LINK_TO/git-credential-manager" || exit 1
     fi
 
-    # Create legacy symlink with older name
-    if [ ! -f "$LINK_TO/git-credential-manager-core" ]; then
-        ln -s -r "$INSTALL_TO/git-credential-manager" \
-            "$LINK_TO/git-credential-manager-core" || exit 1
-    fi
-
     echo "Install complete."
 else
     # Pack

diff --git a/src/linux/Packaging.Linux/pack.sh b/src/linux/Packaging.Linux/pack.sh
@@ -126,12 +126,6 @@ if [ ! -f "$LINK_TO/git-credential-manager" ]; then
         "$LINK_TO/git-credential-manager" || exit 1
 fi
 
-# Create legacy symlink with older name
-if [ ! -f "$LINK_TO/git-credential-manager-core" ]; then
-    ln -s -r "$INSTALL_TO/git-credential-manager" \
-        "$LINK_TO/git-credential-manager-core" || exit 1
-fi
-
 dpkg-deb -Zxz --build "$DEBROOT" "$DEBPKG" || exit 1
 
 echo $MESSAGE
diff --git a/src/osx/Installer.Mac/scripts/postinstall b/src/osx/Installer.Mac/scripts/postinstall
@@ -30,9 +30,6 @@ fi
 mkdir -p /usr/local/bin
 /bin/ln -Fs "$INSTALL_DESTINATION/git-credential-manager" /usr/local/bin/git-credential-manager
 
-# Create legacy symlink to GCMCore in /usr/local/bin
-/bin/ln -Fs "$INSTALL_DESTINATION/git-credential-manager" /usr/local/bin/git-credential-manager-core
-
 # Configure GCM for the current user (running as the current user to avoid root
 # from taking ownership of ~/.gitconfig)
 sudo -u ${USER} "$INSTALL_DESTINATION/git-credential-manager" configure

diff --git a/src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs b/src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs
@@ -24,7 +24,7 @@ public async System.Threading.Tasks.Task MicrosoftAuthentication_GetAccessTokenA
             var msAuth = new MicrosoftAuthentication(context);
 
             await Assert.ThrowsAsync<Trace2InvalidOperationException>(
-                () => msAuth.GetTokenAsync(authority, clientId, redirectUri, scopes, userName));
+                () => msAuth.GetTokenAsync(authority, clientId, redirectUri, scopes, userName, false));
         }
     }
 }
diff --git a/src/shared/Core.Tests/Commands/DiagnoseCommandTests.cs b/src/shared/Core.Tests/Commands/DiagnoseCommandTests.cs
@@ -0,0 +1,82 @@
+using System;
+using System.Net.Http;
+using System.Security.AccessControl;
+using System.Text;
+using GitCredentialManager.Diagnostics;
+using GitCredentialManager.Tests.Objects;
+using Xunit;
+
+namespace Core.Tests.Commands;
+
+public class DiagnoseCommandTests
+{
+    [Fact]
+    public void NetworkingDiagnostic_SendHttpRequest_Primary_OK()
+    {
+        var primaryUriString = "http://example.com";
+        var sb = new StringBuilder();
+        var context = new TestCommandContext();
+        var networkingDiagnostic = new NetworkingDiagnostic(context);
+        var primaryUri = new Uri(primaryUriString);
+        var httpHandler = new TestHttpMessageHandler();
+        var httpResponse = new HttpResponseMessage();
+        var expected = $"Sending HEAD request to {primaryUriString}... OK{Environment.NewLine}";
+
+        httpHandler.Setup(HttpMethod.Head, primaryUri, httpResponse);
+
+        networkingDiagnostic.SendHttpRequest(sb, new HttpClient(httpHandler));
+
+        httpHandler.AssertRequest(HttpMethod.Head, primaryUri, expectedNumberOfCalls: 1);
+        Assert.Contains(expected, sb.ToString());
+    }
+
+    [Fact]
+    public void NetworkingDiagnostic_SendHttpRequest_Backup_OK()
+    {
+        var primaryUriString = "http://example.com";
+        var backupUriString = "http://httpforever.com";
+        var sb = new StringBuilder();
+        var context = new TestCommandContext();
+        var networkingDiagnostic = new NetworkingDiagnostic(context);
+        var primaryUri = new Uri(primaryUriString);
+        var backupUri = new Uri(backupUriString);
+        var httpHandler = new TestHttpMessageHandler { SimulatePrimaryUriFailure = true };
+        var httpResponse = new HttpResponseMessage();
+        var expected = $"Sending HEAD request to {primaryUriString}... warning: HEAD request failed{Environment.NewLine}" +
+                       $"Sending HEAD request to {backupUriString}... OK{Environment.NewLine}";
+
+        httpHandler.Setup(HttpMethod.Head, primaryUri, httpResponse);
+        httpHandler.Setup(HttpMethod.Head, backupUri, httpResponse);
+
+        networkingDiagnostic.SendHttpRequest(sb, new HttpClient(httpHandler));
+
+        httpHandler.AssertRequest(HttpMethod.Head, primaryUri, expectedNumberOfCalls: 1);
+        httpHandler.AssertRequest(HttpMethod.Head, backupUri, expectedNumberOfCalls: 1);
+        Assert.Contains(expected, sb.ToString());
+    }
+
+    [Fact]
+    public void NetworkingDiagnostic_SendHttpRequest_No_Network()
+    {
+        var primaryUriString = "http://example.com";
+        var backupUriString = "http://httpforever.com";
+        var sb = new StringBuilder();
+        var context = new TestCommandContext();
+        var networkingDiagnostic = new NetworkingDiagnostic(context);
+        var primaryUri = new Uri(primaryUriString);
+        var backupUri = new Uri(backupUriString);
+        var httpHandler = new TestHttpMessageHandler { SimulateNoNetwork = true };
+        var httpResponse = new HttpResponseMessage();
+        var expected = $"Sending HEAD request to {primaryUriString}... warning: HEAD request failed{Environment.NewLine}" +
+                       $"Sending HEAD request to {backupUriString}... warning: HEAD request failed{Environment.NewLine}";
+
+        httpHandler.Setup(HttpMethod.Head, primaryUri, httpResponse);
+        httpHandler.Setup(HttpMethod.Head, backupUri, httpResponse);
+
+        networkingDiagnostic.SendHttpRequest(sb, new HttpClient(httpHandler));
+
+        httpHandler.AssertRequest(HttpMethod.Head, primaryUri, expectedNumberOfCalls: 1);
+        httpHandler.AssertRequest(HttpMethod.Head, backupUri, expectedNumberOfCalls: 1);
+        Assert.Contains(expected, sb.ToString());
+    }
+}
diff --git a/src/shared/Core/Authentication/MicrosoftAuthentication.cs b/src/shared/Core/Authentication/MicrosoftAuthentication.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using System.Net.Http;
 using System.Threading.Tasks;
 using GitCredentialManager.Interop.Windows.Native;
@@ -23,7 +24,7 @@ namespace GitCredentialManager.Authentication
     public interface IMicrosoftAuthentication
     {
         Task<IMicrosoftAuthenticationResult> GetTokenAsync(string authority, string clientId, Uri redirectUri,
-            string[] scopes, string userName);
+            string[] scopes, string userName, bool msaPt = false);
     }
 
     public interface IMicrosoftAuthenticationResult
@@ -59,26 +60,31 @@ public MicrosoftAuthentication(ICommandContext context)
         #region IMicrosoftAuthentication
 
         public async Task<IMicrosoftAuthenticationResult> GetTokenAsync(
-            string authority, string clientId, Uri redirectUri, string[] scopes, string userName)
+            string authority, string clientId, Uri redirectUri, string[] scopes, string userName, bool msaPt)
         {
             // Check if we can and should use OS broker authentication
             bool useBroker = CanUseBroker();
             Context.Trace.WriteLine(useBroker
                 ? "OS broker is available and enabled."
                 : "OS broker is not available or enabled.");
 
+            if (msaPt)
+            {
+                Context.Trace.WriteLine("MSA passthrough is enabled.");
+            }
+
             try
             {
                 // Create the public client application for authentication
-                IPublicClientApplication app = await CreatePublicClientApplicationAsync(authority, clientId, redirectUri, useBroker);
+                IPublicClientApplication app = await CreatePublicClientApplicationAsync(authority, clientId, redirectUri, useBroker, msaPt);
 
                 AuthenticationResult result = null;
 
                 // Try silent authentication first if we know about an existing user
                 bool hasExistingUser = !string.IsNullOrWhiteSpace(userName);
                 if (hasExistingUser)
                 {
-                    result = await GetAccessTokenSilentlyAsync(app, scopes, userName);
+                    result = await GetAccessTokenSilentlyAsync(app, scopes, userName, msaPt);
                 }
 
                 //
@@ -116,7 +122,7 @@ public MicrosoftAuthentication(ICommandContext context)
                         // account then the user may become stuck in a loop of authentication failures.
                         if (!hasExistingUser && Context.Settings.UseMsAuthDefaultAccount)
                         {
-                            result = await GetAccessTokenSilentlyAsync(app, scopes, null);
+                            result = await GetAccessTokenSilentlyAsync(app, scopes, null, msaPt);
 
                             if (result is null || !await UseDefaultAccountAsync(result.Account.Username))
                             {
@@ -281,34 +287,62 @@ internal MicrosoftAuthenticationFlowType GetFlowType()
         /// <summary>
         /// Obtain an access token without showing UI or prompts.
         /// </summary>
-        private async Task<AuthenticationResult> GetAccessTokenSilentlyAsync(IPublicClientApplication app, string[] scopes, string userName)
+        private async Task<AuthenticationResult> GetAccessTokenSilentlyAsync(
+            IPublicClientApplication app, string[] scopes, string userName, bool msaPt)
         {
             try
             {
                 if (userName is null)
                 {
-                    Context.Trace.WriteLine("Attempting to acquire token silently for current operating system account...");
+                    Context.Trace.WriteLine(
+                        "Attempting to acquire token silently for current operating system account...");
 
-                    return await app.AcquireTokenSilent(scopes, PublicClientApplication.OperatingSystemAccount).ExecuteAsync();
+                    return await app.AcquireTokenSilent(scopes, PublicClientApplication.OperatingSystemAccount)
+                        .ExecuteAsync();
                 }
                 else
                 {
                     Context.Trace.WriteLine($"Attempting to acquire token silently for user '{userName}'...");
 
-                    // We can either call `app.GetAccountsAsync` and filter through the IAccount objects for the instance with the correct user name,
-                    // or we can just pass the user name string we have as the `loginHint` and let MSAL do exactly that for us instead!
-                    return await app.AcquireTokenSilent(scopes, loginHint: userName).ExecuteAsync();
+                    // Enumerate all accounts and find the one matching the user name
+                    IEnumerable<IAccount> accounts = await app.GetAccountsAsync();
+                    IAccount account = accounts.FirstOrDefault(x =>
+                        StringComparer.OrdinalIgnoreCase.Equals(x.Username, userName));
+                    if (account is null)
+                    {
+                        Context.Trace.WriteLine($"No cached account found for user '{userName}'...");
+                        return null;
+                    }
+
+                    var atsBuilder = app.AcquireTokenSilent(scopes, account);
+
+                    // Is we are operating with an MSA passthrough app we need to ensure that we target the
+                    // special MSA 'transfer' tenant explicitly. This is a workaround for MSAL issue:
+                    // https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/3077
+                    if (msaPt && Guid.TryParse(account.HomeAccountId.TenantId, out Guid homeTenantId) &&
+                        homeTenantId == Constants.MsaHomeTenantId)
+                    {
+                        atsBuilder = atsBuilder.WithTenantId(Constants.MsaTransferTenantId.ToString("D"));
+                    }
+
+                    return await atsBuilder.ExecuteAsync();
                 }
             }
             catch (MsalUiRequiredException)
             {
                 Context.Trace.WriteLine("Failed to acquire token silently; user interaction is required.");
                 return null;
             }
+            catch (Exception ex)
+            {
+                Context.Trace.WriteLine("Failed to acquire token silently.");
+                Context.Trace.WriteException(ex);
+                return null;
+            }
         }
 
         private async Task<IPublicClientApplication> CreatePublicClientApplicationAsync(
-            string authority, string clientId, Uri redirectUri, bool enableBroker)
+            string authority, string clientId, Uri redirectUri, bool enableBroker, bool msaPt)
         {
             var httpFactoryAdaptor = new MsalHttpClientFactoryAdaptor(Context.HttpClientFactory);
 
@@ -370,7 +404,7 @@ private async Task<AuthenticationResult> GetAccessTokenSilentlyAsync(IPublicClie
                     new BrokerOptions(BrokerOptions.OperatingSystems.Windows)
                     {
                         Title = "Git Credential Manager",
-                        MsaPassthrough = true,
+                        MsaPassthrough = msaPt,
                     }
                 );
 #endif

diff --git a/src/shared/Core/Constants.cs b/src/shared/Core/Constants.cs
@@ -18,6 +18,17 @@ public static class Constants
 
         public static readonly Guid DevBoxPartnerId = new("e3171dd9-9a5f-e5be-b36c-cc7c4f3f3bcf");
 
+        /// <summary>
+        /// Home tenant ID for Microsoft Accounts (MSA).
+        /// </summary>
+        public static readonly Guid MsaHomeTenantId = new("9188040d-6c67-4c5b-b112-36a304b66dad");
+
+        /// <summary>
+        /// Special tenant ID for transferring between Microsoft Account (MSA) native tokens
+        /// and AAD tokens. Only required for MSA-Passthrough applications.
+        /// </summary>
+        public static readonly Guid MsaTransferTenantId = new("f8cdef31-a31e-4b4a-93e4-5f571e91255a");
+
         public static class CredentialStoreNames
         {
             public const string WindowsCredentialManager = "wincredman";
@@ -210,7 +221,6 @@ public static class HelpUrls
             public const string GcmCredentialStores    = "https://aka.ms/gcm/credstores";
             public const string GcmWamComSecurity      = "https://aka.ms/gcm/wamadmin";
             public const string GcmAutoDetect          = "https://aka.ms/gcm/autodetect";
-            public const string GcmExecRename          = "https://aka.ms/gcm/rename";
             public const string GcmDefaultAccount      = "https://aka.ms/gcm/defaultaccount";
             public const string GcmMultipleUsers       = "https://aka.ms/gcm/multipleusers";
         }

diff --git a/src/shared/Core/Diagnostics/NetworkingDiagnostic.cs b/src/shared/Core/Diagnostics/NetworkingDiagnostic.cs
@@ -12,6 +12,7 @@ namespace GitCredentialManager.Diagnostics
     public class NetworkingDiagnostic : Diagnostic
     {
         private const string TestHttpUri = "http://example.com";
+        private const string TestHttpUriFallback = "http://httpforever.com";
         private const string TestHttpsUri = "https://example.com";
 
         public NetworkingDiagnostic(ICommandContext commandContext)
@@ -28,9 +29,7 @@ protected override async Task<bool> RunInternalAsync(StringBuilder log, IList<st
             bool hasNetwork = NetworkInterface.GetIsNetworkAvailable();
             log.AppendLine($"IsNetworkAvailable: {hasNetwork}");
 
-            log.Append($"Sending HEAD request to {TestHttpUri}...");
-            using var httpResponse = await httpClient.HeadAsync(TestHttpUri);
-            log.AppendLine(" OK");
+            SendHttpRequest(log, httpClient);
 
             log.Append($"Sending HEAD request to {TestHttpsUri}...");
             using var httpsResponse = await httpClient.HeadAsync(TestHttpsUri);
@@ -98,5 +97,23 @@ protected override async Task<bool> RunInternalAsync(StringBuilder log, IList<st
 
             return true;
         }
+
+        internal /* For testing purposes */ async void SendHttpRequest(StringBuilder log, HttpClient httpClient)
+        {
+            foreach (var uri in new List<string> { TestHttpUri, TestHttpUriFallback })
+            {
+                try
+                {
+                    log.Append($"Sending HEAD request to {uri}...");
+                    using var httpResponse = await httpClient.HeadAsync(uri);
+                    log.AppendLine(" OK");
+                    break;
+                }
+                catch (HttpRequestException)
+                {
+                    log.AppendLine(" warning: HEAD request failed");
+                }
+            }
+        }
     }
 }
diff --git a/src/shared/Core/Trace2FileWriter.cs b/src/shared/Core/Trace2FileWriter.cs
@@ -1,4 +1,5 @@
 using System.IO;
+using System;
 
 namespace GitCredentialManager;
 
@@ -13,6 +14,22 @@ public Trace2FileWriter(Trace2FormatTarget formatTarget, string path) : base(for
 
     public override void Write(Trace2Message message)
     {
-        File.AppendAllText(_path, Format(message));
+        try
+        {
+            File.AppendAllText(_path, Format(message));
+        }
+        catch (DirectoryNotFoundException)
+        {
+            // Do nothing, as this either means we don't have the
+            // parent directories above the file, or this trace2
+            // target points to a directory.
+        }
+        catch (UnauthorizedAccessException)
+        {
+            // Do nothing, as this either means the file is not
+            // accessible with current permissions, or we are on
+            // Windows and the file is currently open for writing
+            // by another process (likely Git itself.)
+        }
     }
 }