diff --git a/sdk/identity/Azure.Identity/CHANGELOG.md b/sdk/identity/Azure.Identity/CHANGELOG.md index 62b0239e60ca4..fdbc5b142917e 100644 --- a/sdk/identity/Azure.Identity/CHANGELOG.md +++ b/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 diff --git a/sdk/identity/Azure.Identity/src/AzureIdentityEventSource.cs b/sdk/identity/Azure.Identity/src/AzureIdentityEventSource.cs index d7cd520bb219f..875d3acb6855c 100644 --- a/sdk/identity/Azure.Identity/src/AzureIdentityEventSource.cs +++ b/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); + } } } diff --git a/sdk/identity/Azure.Identity/src/IdentityCompatSwitches.cs b/sdk/identity/Azure.Identity/src/IdentityCompatSwitches.cs new file mode 100644 index 0000000000000..44b49802122c8 --- /dev/null +++ b/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; + } + } + } +} diff --git a/sdk/identity/Azure.Identity/src/MsalPublicClient.cs b/sdk/identity/Azure.Identity/src/MsalPublicClient.cs index 4ffef0aea9cc2..0a117a4effb45 100644 --- a/sdk/identity/Azure.Identity/src/MsalPublicClient.cs +++ b/sdk/identity/Azure.Identity/src/MsalPublicClient.cs @@ -43,13 +43,23 @@ protected override ValueTask CreateClientAsync(bool as return new ValueTask(pubAppBuilder.Build()); } - public virtual async ValueTask> GetAccountsAsync(bool async, CancellationToken cancellationToken) + public async ValueTask> GetAccountsAsync(bool async, CancellationToken cancellationToken) + { + return await GetAccountsCoreAsync(async, cancellationToken).ConfigureAwait(false); + } + + protected virtual async ValueTask> GetAccountsCoreAsync(bool async, CancellationToken cancellationToken) { IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false); return await GetAccountsAsync(client, async).ConfigureAwait(false); } - public virtual async ValueTask AcquireTokenSilentAsync(string[] scopes, string claims, IAccount account, bool async, CancellationToken cancellationToken) + public async ValueTask 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 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) @@ -57,7 +67,13 @@ public virtual async ValueTask AcquireTokenSilentAsync(str .ExecuteAsync(async, cancellationToken) .ConfigureAwait(false); } - public virtual async ValueTask AcquireTokenSilentAsync(string[] scopes, string claims, AuthenticationRecord record, bool async, CancellationToken cancellationToken) + + public async ValueTask 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 AcquireTokenSilentCoreAsync(string[] scopes, string claims, AuthenticationRecord record, bool async, CancellationToken cancellationToken) { IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false); @@ -71,9 +87,34 @@ public virtual async ValueTask AcquireTokenSilentAsync(str .ConfigureAwait(false); } - public virtual async ValueTask AcquireTokenInteractiveAsync(string[] scopes, string claims, Prompt prompt, bool async, CancellationToken cancellationToken) + public async ValueTask 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 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) @@ -81,7 +122,12 @@ public virtual async ValueTask AcquireTokenInteractiveAsyn .ConfigureAwait(false); } - public virtual async ValueTask AcquireTokenByUsernamePasswordAsync(string[] scopes, string claims, string username, SecureString password, bool async, CancellationToken cancellationToken) + public async ValueTask 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 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 AcquireTokenByUsernamePassw .ConfigureAwait(false); } - public virtual async ValueTask AcquireTokenWithDeviceCodeAsync(string[] scopes, string claims, Func deviceCodeCallback, bool async, CancellationToken cancellationToken) + public async ValueTask AcquireTokenWithDeviceCodeAsync(string[] scopes, string claims, Func deviceCodeCallback, bool async, CancellationToken cancellationToken) + { + return await AcquireTokenWithDeviceCodeCoreAsync(scopes, claims, deviceCodeCallback, async, cancellationToken).ConfigureAwait(false); + } + + protected virtual async ValueTask AcquireTokenWithDeviceCodeCoreAsync(string[] scopes, string claims, Func 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 AcquireTokenWithDeviceCodeA .ConfigureAwait(false); } - public virtual async ValueTask AcquireTokenByRefreshToken(string[] scopes, string claims, string refreshToken, AzureCloudInstance azureCloudInstance, string tenant, bool async, CancellationToken cancellationToken) + public async ValueTask 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 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) diff --git a/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs b/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs index 1f31e38294ac3..8fb07397c7e34 100644 --- a/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs +++ b/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs @@ -71,7 +71,7 @@ private async ValueTask 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) diff --git a/sdk/identity/Azure.Identity/tests/InteractiveBrowserCredentialTests.cs b/sdk/identity/Azure.Identity/tests/InteractiveBrowserCredentialTests.cs index fc242976089f2..027ba227c9d2b 100644 --- a/sdk/identity/Azure.Identity/tests/InteractiveBrowserCredentialTests.cs +++ b/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() { diff --git a/sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs b/sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs index 2d1b0bf9f6f5e..3c644a3694dd3 100644 --- a/sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs +++ b/sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs @@ -27,12 +27,12 @@ internal class MockMsalPublicClient : MsalPublicClient public Func DeviceCodeAuthFactory { get; set; } - public override ValueTask> GetAccountsAsync(bool async, CancellationToken cancellationToken) + protected override ValueTask> GetAccountsCoreAsync(bool async, CancellationToken cancellationToken) { return new ValueTask>(Accounts); } - public override ValueTask AcquireTokenByUsernamePasswordAsync(string[] scopes, string claims, string username, SecureString password, bool async, CancellationToken cancellationToken) + protected override ValueTask AcquireTokenByUsernamePasswordCoreAsync(string[] scopes, string claims, string username, SecureString password, bool async, CancellationToken cancellationToken) { Func factory = UserPassAuthFactory ?? AuthFactory; @@ -44,7 +44,7 @@ public override ValueTask AcquireTokenByUsernamePasswordAs throw new NotImplementedException(); } - public override ValueTask AcquireTokenInteractiveAsync(string[] scopes, string claims, Prompt prompt, bool async, CancellationToken cancellationToken) + protected override ValueTask AcquireTokenInteractiveCoreAsync(string[] scopes, string claims, Prompt prompt, bool async, CancellationToken cancellationToken) { Func factory = InteractiveAuthFactory ?? AuthFactory; @@ -56,7 +56,7 @@ public override ValueTask AcquireTokenInteractiveAsync(str throw new NotImplementedException(); } - public override ValueTask AcquireTokenSilentAsync(string[] scopes, string claims, IAccount account, bool async, CancellationToken cancellationToken) + protected override ValueTask AcquireTokenSilentCoreAsync(string[] scopes, string claims, IAccount account, bool async, CancellationToken cancellationToken) { if (ExtendedSilentAuthFactory != null) { @@ -73,7 +73,7 @@ public override ValueTask AcquireTokenSilentAsync(string[] throw new NotImplementedException(); } - public override ValueTask AcquireTokenSilentAsync(string[] scopes, string claims, AuthenticationRecord record, bool async, CancellationToken cancellationToken) + protected override ValueTask AcquireTokenSilentCoreAsync(string[] scopes, string claims, AuthenticationRecord record, bool async, CancellationToken cancellationToken) { Func factory = SilentAuthFactory ?? AuthFactory; @@ -85,7 +85,7 @@ public override ValueTask AcquireTokenSilentAsync(string[] throw new NotImplementedException(); } - public override ValueTask AcquireTokenWithDeviceCodeAsync(string[] scopes, string claims, Func deviceCodeCallback, bool async, CancellationToken cancellationToken) + protected override ValueTask AcquireTokenWithDeviceCodeCoreAsync(string[] scopes, string claims, Func deviceCodeCallback, bool async, CancellationToken cancellationToken) { Func factory = DeviceCodeAuthFactory ?? AuthFactory;