[Identity] Fix InteractiveBrowserCredential.GetToken deadlock with em…

…bedded browser (#19864)

* [Identity] Fix InteractiveBrowserCredential.GetToken deadlock when using embedded browser

* adding compat switch to opt out of workaround

* adding test for compat switch

* updating compat switch test

* fix blankline issue from merge

sdk/identity/Azure.Identity/CHANGELOG.md

@@ -15,7 +15,8 @@ Thank you to our developer community members who helped to make Azure Identity b
 ### Fixes and improvements
 
 - When logging is enabled, the log output from MSAL is also logged.
-- Fixed an issue where an account credential fails to load from the cache when EnableGuestTenantAuthentication is true and the account found in the cache has multiple matching tenantIds.
+- Fixed an issue where an account credential fails to load from the cache when EnableGuestTenantAuthentication is true and the account found in the cache has multiple matching tenantIds ([#18276](https://github.com/Azure/azure-sdk-for-net/issues/18276)).
+- Fixed deadlock issue in `InteractiveBrowserCredential` when running in a UI application ([#18418](https://github.com/Azure/azure-sdk-for-net/issues/18418)).
 
 ### Breaking Changes
 

sdk/identity/Azure.Identity/src/AzureIdentityEventSource.cs

@@ -25,6 +25,8 @@ internal sealed class AzureIdentityEventSource : EventSource
         private const int MsalLogInfoEvent = 8;
         private const int MsalLogWarningEvent = 9;
         private const int MsalLogErrorEvent = 10;
+        private const int InteractiveAuthenticationThreadPoolExecutionEvent = 11;
+        private const int InteractiveAuthenticationInlineExecutionEvent = 12;
 
         private AzureIdentityEventSource() : base(EventSourceName, EventSourceSettings.Default, AzureEventSourceListener.TraitName, AzureEventSourceListener.TraitValue) { }
 
@@ -209,5 +211,17 @@ private static string FormatStringArray(string[] array)
         {
             return new StringBuilder("[ ").Append(string.Join(", ", array)).Append(" ]").ToString();
         }
+
+        [Event(InteractiveAuthenticationThreadPoolExecutionEvent, Level = EventLevel.Informational, Message = "Executing interactive authentication workflow via Task.Run.")]
+        public void InteractiveAuthenticationExecutingOnThreadPool()
+        {
+            WriteEvent(InteractiveAuthenticationThreadPoolExecutionEvent);
+        }
+
+        [Event(InteractiveAuthenticationInlineExecutionEvent, Level = EventLevel.Informational, Message = "Executing interactive authentication workflow inline.")]
+        public void InteractiveAuthenticationExecutingInline()
+        {
+            WriteEvent(InteractiveAuthenticationInlineExecutionEvent);
+        }
     }
 }

sdk/identity/Azure.Identity/src/IdentityCompatSwitches.cs

@@ -0,0 +1,32 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+
+namespace Azure.Identity
+{
+    internal class IdentityCompatSwitches
+    {
+        private const string DisableInteractiveThreadpoolExecutionSwitchName = "Azure.Identity.DisableInteractiveBrowserThreadpoolExecution";
+        private const string DisableInteractiveThreadpoolExecutionEnvVar = "AZURE_IDENTITY_DISABLE_INTERACTIVEBROWSERTHREADPOOLEXECUTION";
+
+        public static bool DisableInteractiveBrowserThreadpoolExecution
+        {
+            get
+            {
+                if (!AppContext.TryGetSwitch(DisableInteractiveThreadpoolExecutionSwitchName, out bool ret))
+                {
+                    string switchValue = Environment.GetEnvironmentVariable(DisableInteractiveThreadpoolExecutionEnvVar);
+
+                    if (switchValue != null)
+                    {
+                        ret = string.Equals("true", switchValue, StringComparison.InvariantCultureIgnoreCase) ||
+                              switchValue.Equals("1", StringComparison.InvariantCultureIgnoreCase);
+                    }
+                }
+
+                return ret;
+            }
+        }
+    }
+}

sdk/identity/Azure.Identity/src/MsalPublicClient.cs

@@ -43,21 +43,37 @@ protected override ValueTask<IPublicClientApplication> CreateClientAsync(bool as
             return new ValueTask<IPublicClientApplication>(pubAppBuilder.Build());
         }
 
-        public virtual async ValueTask<List<IAccount>> GetAccountsAsync(bool async, CancellationToken cancellationToken)
+        public async ValueTask<List<IAccount>> GetAccountsAsync(bool async, CancellationToken cancellationToken)
+        {
+            return await GetAccountsCoreAsync(async, cancellationToken).ConfigureAwait(false);
+        }
+
+        protected virtual async ValueTask<List<IAccount>> GetAccountsCoreAsync(bool async, CancellationToken cancellationToken)
         {
             IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false);
             return await GetAccountsAsync(client, async).ConfigureAwait(false);
         }
 
-        public virtual async ValueTask<AuthenticationResult> AcquireTokenSilentAsync(string[] scopes, string claims, IAccount account, bool async, CancellationToken cancellationToken)
+        public async ValueTask<AuthenticationResult> AcquireTokenSilentAsync(string[] scopes, string claims, IAccount account, bool async, CancellationToken cancellationToken)
+        {
+            return await AcquireTokenSilentCoreAsync(scopes, claims, account, async, cancellationToken).ConfigureAwait(false);
+        }
+
+        protected virtual async ValueTask<AuthenticationResult> AcquireTokenSilentCoreAsync(string[] scopes, string claims, IAccount account, bool async, CancellationToken cancellationToken)
         {
             IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false);
             return await client.AcquireTokenSilent(scopes, account)
                 .WithClaims(claims)
                 .ExecuteAsync(async, cancellationToken)
                 .ConfigureAwait(false);
         }
-        public virtual async ValueTask<AuthenticationResult> AcquireTokenSilentAsync(string[] scopes, string claims, AuthenticationRecord record, bool async, CancellationToken cancellationToken)
+
+        public async ValueTask<AuthenticationResult> AcquireTokenSilentAsync(string[] scopes, string claims, AuthenticationRecord record, bool async, CancellationToken cancellationToken)
+        {
+            return await AcquireTokenSilentCoreAsync(scopes, claims, record, async, cancellationToken).ConfigureAwait(false);
+        }
+
+        protected virtual async ValueTask<AuthenticationResult> AcquireTokenSilentCoreAsync(string[] scopes, string claims, AuthenticationRecord record, bool async, CancellationToken cancellationToken)
         {
             IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false);
 
@@ -71,17 +87,47 @@ public virtual async ValueTask<AuthenticationResult> AcquireTokenSilentAsync(str
                 .ConfigureAwait(false);
         }
 
-        public virtual async ValueTask<AuthenticationResult> AcquireTokenInteractiveAsync(string[] scopes, string claims, Prompt prompt, bool async, CancellationToken cancellationToken)
+        public async ValueTask<AuthenticationResult> AcquireTokenInteractiveAsync(string[] scopes, string claims, Prompt prompt, bool async, CancellationToken cancellationToken)
+        {
+#pragma warning disable AZC0109 // Misuse of 'async' parameter.
+            if (!async && !IdentityCompatSwitches.DisableInteractiveBrowserThreadpoolExecution)
+#pragma warning restore AZC0109 // Misuse of 'async' parameter.
+            {
+                // In the synchronous case we need to use Task.Run to execute on the call to MSAL on the threadpool.
+                // On certain platforms MSAL will use the embedded browser instead of launching the browser as a separate
+                // process. Executing with Task.Run prevents possibly deadlocking the UI thread in these cases.
+                // This workaround can be disabled by using the "Azure.Identity.DisableInteractiveBrowserThreadpoolExecution" app switch
+                // or setting the AZURE_IDENTITY_DISABLE_INTERACTIVEBROWSERTHREADPOOLEXECUTION environment variable to true
+                AzureIdentityEventSource.Singleton.InteractiveAuthenticationExecutingOnThreadPool();
+
+#pragma warning disable AZC0102 // Do not use GetAwaiter().GetResult().
+                return Task.Run(async () => await AcquireTokenInteractiveCoreAsync(scopes, claims, prompt, true, cancellationToken).ConfigureAwait(false)).GetAwaiter().GetResult();
+#pragma warning restore AZC0102 // Do not use GetAwaiter().GetResult().
+
+            }
+
+            AzureIdentityEventSource.Singleton.InteractiveAuthenticationExecutingInline();
+
+            return await AcquireTokenInteractiveCoreAsync(scopes, claims, prompt, async, cancellationToken).ConfigureAwait(false);
+        }
+
+        protected virtual async ValueTask<AuthenticationResult> AcquireTokenInteractiveCoreAsync(string[] scopes, string claims, Prompt prompt, bool async, CancellationToken cancellationToken)
         {
             IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false);
+
             return await client.AcquireTokenInteractive(scopes)
                 .WithPrompt(prompt)
                 .WithClaims(claims)
                 .ExecuteAsync(async, cancellationToken)
                 .ConfigureAwait(false);
         }
 
-        public virtual async ValueTask<AuthenticationResult> AcquireTokenByUsernamePasswordAsync(string[] scopes, string claims, string username, SecureString password, bool async, CancellationToken cancellationToken)
+        public async ValueTask<AuthenticationResult> AcquireTokenByUsernamePasswordAsync(string[] scopes, string claims, string username, SecureString password, bool async, CancellationToken cancellationToken)
+        {
+            return await AcquireTokenByUsernamePasswordCoreAsync(scopes, claims, username, password, async, cancellationToken).ConfigureAwait(false);
+        }
+
+        protected virtual async ValueTask<AuthenticationResult> AcquireTokenByUsernamePasswordCoreAsync(string[] scopes, string claims, string username, SecureString password, bool async, CancellationToken cancellationToken)
         {
             IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false);
             return await client.AcquireTokenByUsernamePassword(scopes, username, password)
@@ -90,7 +136,12 @@ public virtual async ValueTask<AuthenticationResult> AcquireTokenByUsernamePassw
                 .ConfigureAwait(false);
         }
 
-        public virtual async ValueTask<AuthenticationResult> AcquireTokenWithDeviceCodeAsync(string[] scopes, string claims, Func<DeviceCodeResult, Task> deviceCodeCallback, bool async, CancellationToken cancellationToken)
+        public async ValueTask<AuthenticationResult> AcquireTokenWithDeviceCodeAsync(string[] scopes, string claims, Func<DeviceCodeResult, Task> deviceCodeCallback, bool async, CancellationToken cancellationToken)
+        {
+            return await AcquireTokenWithDeviceCodeCoreAsync(scopes, claims, deviceCodeCallback, async, cancellationToken).ConfigureAwait(false);
+        }
+
+        protected virtual async ValueTask<AuthenticationResult> AcquireTokenWithDeviceCodeCoreAsync(string[] scopes, string claims, Func<DeviceCodeResult, Task> deviceCodeCallback, bool async, CancellationToken cancellationToken)
         {
             IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false);
             return await client.AcquireTokenWithDeviceCode(scopes, deviceCodeCallback)
@@ -99,7 +150,12 @@ public virtual async ValueTask<AuthenticationResult> AcquireTokenWithDeviceCodeA
                 .ConfigureAwait(false);
         }
 
-        public virtual async ValueTask<AuthenticationResult> AcquireTokenByRefreshToken(string[] scopes, string claims, string refreshToken, AzureCloudInstance azureCloudInstance, string tenant, bool async, CancellationToken cancellationToken)
+        public async ValueTask<AuthenticationResult> AcquireTokenByRefreshTokenAsync(string[] scopes, string claims, string refreshToken, AzureCloudInstance azureCloudInstance, string tenant, bool async, CancellationToken cancellationToken)
+        {
+            return await AcquireTokenByRefreshTokenCoreAsync(scopes, claims, refreshToken, azureCloudInstance, tenant, async, cancellationToken).ConfigureAwait(false);
+        }
+
+        protected virtual async ValueTask<AuthenticationResult> AcquireTokenByRefreshTokenCoreAsync(string[] scopes, string claims, string refreshToken, AzureCloudInstance azureCloudInstance, string tenant, bool async, CancellationToken cancellationToken)
         {
             IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false);
             return await ((IByRefreshToken)client).AcquireTokenByRefreshToken(scopes, refreshToken)

sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs

@@ -71,7 +71,7 @@ private async ValueTask<AccessToken> GetTokenImplAsync(TokenRequestContext reque
                 var cloudInstance = GetAzureCloudInstance(environmentName);
                 string storedCredentials = GetStoredCredentials(environmentName);
 
-                var result = await _client.AcquireTokenByRefreshToken(requestContext.Scopes, requestContext.Claims, storedCredentials, cloudInstance, tenant, async, cancellationToken).ConfigureAwait(false);
+                var result = await _client.AcquireTokenByRefreshTokenAsync(requestContext.Scopes, requestContext.Claims, storedCredentials, cloudInstance, tenant, async, cancellationToken).ConfigureAwait(false);
                 return scope.Succeeded(new AccessToken(result.AccessToken, result.ExpiresOn));
             }
             catch (MsalUiRequiredException e)

sdk/identity/Azure.Identity/tests/InteractiveBrowserCredentialTests.cs

@@ -2,12 +2,12 @@
 // Licensed under the MIT License.
 
 using Azure.Core;
+using Azure.Core.Diagnostics;
 using Azure.Core.TestFramework;
 using Azure.Identity.Tests.Mock;
 using Microsoft.Identity.Client;
 using NUnit.Framework;
 using System;
-using System.Threading;
 using System.Threading.Tasks;
 
 namespace Azure.Identity.Tests
@@ -104,6 +104,75 @@ public async Task InteractiveBrowserRefreshException()
             await Task.CompletedTask;
         }
 
+        [Test]
+        [NonParallelizable]
+        public async Task InteractiveBrowserValidateSyncWorkaroundCompatSwitch()
+        {
+            // once the AppContext switch is set it cannot be unset for this reason this test must sequentially test the following
+            // neither Environment variable or AppContext switch is set.
+            // environment variable is set and AppContext switch is not set
+            // AppContext switch is set
+            await ValidateSyncWorkaroundCompatSwitch(!IsAsync);
+
+            using (var envVar = new TestEnvVar("AZURE_IDENTITY_DISABLE_INTERACTIVEBROWSERTHREADPOOLEXECUTION", string.Empty))
+            {
+                await ValidateSyncWorkaroundCompatSwitch(!IsAsync);
+            }
+
+            using (var envVar = new TestEnvVar("AZURE_IDENTITY_DISABLE_INTERACTIVEBROWSERTHREADPOOLEXECUTION", "false"))
+            {
+                await ValidateSyncWorkaroundCompatSwitch(!IsAsync);
+            }
+
+            using (var envVar = new TestEnvVar("AZURE_IDENTITY_DISABLE_INTERACTIVEBROWSERTHREADPOOLEXECUTION", "true"))
+            {
+                await ValidateSyncWorkaroundCompatSwitch(false);
+            }
+
+            using (var envVar = new TestEnvVar("AZURE_IDENTITY_DISABLE_INTERACTIVEBROWSERTHREADPOOLEXECUTION", "1"))
+            {
+                await ValidateSyncWorkaroundCompatSwitch(false);
+            }
+
+            AppContext.SetSwitch("Azure.Identity.DisableInteractiveBrowserThreadpoolExecution", true);
+
+            await ValidateSyncWorkaroundCompatSwitch(false);
+
+            AppContext.SetSwitch("Azure.Identity.DisableInteractiveBrowserThreadpoolExecution", false);
+
+            await ValidateSyncWorkaroundCompatSwitch(!IsAsync);
+        }
+
+        private async Task ValidateSyncWorkaroundCompatSwitch(bool expectedThreadPoolExecution)
+        {
+            bool threadPoolExec = false;
+            bool inlineExec = false;
+
+            AzureEventSourceListener listener = new AzureEventSourceListener((args, text) =>
+            {
+                if (args.EventName == "InteractiveAuthenticationExecutingOnThreadPool")
+                {
+                    threadPoolExec = true;
+                }
+                if (args.EventName == "InteractiveAuthenticationExecutingInline")
+                {
+                    inlineExec = true;
+                }
+            }, System.Diagnostics.Tracing.EventLevel.Informational);
+
+            var mockMsalClient = new MockMsalPublicClient
+            {
+                InteractiveAuthFactory = (_) => { return AuthenticationResultFactory.Create(accessToken: Guid.NewGuid().ToString(), expiresOn: DateTimeOffset.UtcNow.AddMinutes(5)); }
+            };
+
+            var credential = InstrumentClient(new InteractiveBrowserCredential(default, "", default, default, mockMsalClient));
+
+            AccessToken token = await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default));
+
+            Assert.AreEqual(expectedThreadPoolExecution, threadPoolExec);
+            Assert.AreEqual(!expectedThreadPoolExecution, inlineExec);
+        }
+
         [Test]
         public void DisableAutomaticAuthenticationException()
         {