microsoft-authentication-library-for-dotnet icon indicating copy to clipboard operation
microsoft-authentication-library-for-dotnet copied to clipboard

[Bug] NativeInterop SignInSilentlyAsync never returns, hangs indefinitely

Open mcgeeky opened this issue 2 years ago • 5 comments

Library version used

Microsoft.Identity.Client.NativeInterop 0.14.1

.NET version

.Net Framework 4.8

Scenario

PublicClient - desktop app

Is this a new or an existing app?

None

Issue description and reproduction steps

After integrating WAM in to my Windows desktop application, I have received a few reports back from users, with Entra ID joined computers, that the app will freeze. It freezes on a call to SignInSilentlyAsync in Microsoft.Identity.Client.NativeInterop.Core.

The call to SignInSilentlyAsync never returns so they have to terminate the app via Task Manager.

I would expect that SignInSilentlyAsync would return eventually.

I am now implementing a cancellation token and wait on the Task returned from SignInSilentlyAsync. If the task does not complete in time I cancel it.

But what if this keeps occurring? Does the NativeClient deal with cancelled tasks graciously or could this cause a thread pool to exhaust in the NativeClient if repeated calls to SignInSilentlyAsync are made and then cancelled?

Why does it never return, what can it be waiting on indefinitely for?

Relevant code snippets

var core = new Microsoft.Identity.Client.NativeInterop.Core();

var authParams = new Microsoft.Identity.Client.NativeInterop.AuthParameters(ClientId, Authority);

authParams.RequestedScopes = "openid";

authParams.RedirectUri = "https://login.windows.net/common/oauth2/nativeclient";

core.SignInSilentlyAsync(authParams, Guid.NewGuid().ToString("D")).GetAwaiter().GetResult();

Expected behavior

SignInSilentlyAsync will not hang forever.

Identity provider

Microsoft Entra ID (Work and School accounts and Personal Microsoft accounts)

Regression

No response

Solution and workarounds

Implement a cancellation token and timeout but the impact of cancelling the task is unknown - will NativeClient fail graciously?

mcgeeky avatar Feb 02 '24 01:02 mcgeeky

Hi @mcgeeky - we aren't supporting this code path at the moment, i.e. calling directly into NativeInterop (which is really a thin layer over MSAL C++).

The code pattern we recommend is the MSAL public API (AcquireTokenInteractive / AcquireTokenSilent) https://learn.microsoft.com/en-us/entra/msal/dotnet/acquiring-tokens/desktop-mobile/wam

There is some logic that translates from MSAL public API to NativeInterop API - see https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/main/src/client/Microsoft.Identity.Client/Platforms/Features/RuntimeBroker/RuntimeBroker.cs

bgavrilMS avatar Feb 02 '24 10:02 bgavrilMS

That said, hangs could be due to not using async patten and relying on GetAwaiter.GetResult

bgavrilMS avatar Feb 02 '24 10:02 bgavrilMS

@bgavrilMS Thanks for your feedback Bogdan and the link to the MSAL source code which is very helpful. I am going direct to Native Interop as I only require the silent sign in and not the other features that MSAL provides (my app already handles browser based auth). I have updated my app to use the cancellation token similar to the MSAL code you linked to; hopefully this will resolve the issue or at least if it does occur, it will timeout gracefully and not block calling the silent sign in method subsequently.

So do you think it more likely that the hanging is due to not using the async pattern rather than an internal issue within the Native C++ runtime library and Win32 API that it may be calling? That's my concern that there is an issue deep within the native code.

mcgeeky avatar Feb 02 '24 11:02 mcgeeky

What are you trying to achieve @mcgeeky ? Do you want to leverage MSAL / NativeInterop to get a token silently for the current Windows user? This functionality is exposed in MSAL .NET as well, see https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/main/src/client/Microsoft.Identity.Client/Platforms/Features/RuntimeBroker/RuntimeBroker.cs#L334

Note that while this works in the majority of cases, you should expect it to fail with an exception indicating that UI is required, for example:

  • when the user changes their password
  • when the tenant admin configures MFA

bgavrilMS avatar Feb 02 '24 13:02 bgavrilMS

@bgavrilMS yes that is correct; I want to leverage NativeInterop to acquire a token silently to support the new conditional access token protection. I am trying to avoid the broader MSAL libraries to keep the footprint of the app smaller as it is only the sign in silently that I require. Where the interaction_required status is returned then the app defers to the browser which it already handles.

mcgeeky avatar Feb 02 '24 15:02 mcgeeky