AzureAD / microsoft-authentication-library-for-dotnet

Microsoft Authentication Library (MSAL) for .NET
https://aka.ms/msal-net
MIT License
1.39k stars 340 forks source link

[Bug] MSAL throws exception when external browser is closed prior to the AcquireTokenInteractive call #4448

Open thecrazyonestherebels opened 11 months ago

thecrazyonestherebels commented 11 months ago

Library version used

4.58

.NET version

4.6.2

Scenario

PublicClient - desktop app

Is this a new or an existing app?

None

Issue description and reproduction steps

When following steps are executed, MSAL throws error described below:

  1. Make sure that default system browser is closed ( Edge browser in our case )
  2. Launch MSAL.NET application and run AcquireTokenInteractive
  3. Default system browser is launched and we can complete the authorization
  4. Upon completing the authorization - keep the 'Authentication complete. You can return to the application. Feel free to close this browser tab.' tab open in the browser and close the browser.
  5. Run the AcquireTokenInteractive again, and following exception is printed
Caught exception! MSAL.Desktop.4.58.0.0.MsalClientException:
        ErrorCode: loopback_response_uri_mismatch
Microsoft.Identity.Client.MsalClientException: Redirect Uri mismatch.  Expected (/favicon.ico) Actual (/).
   at Microsoft.Identity.Client.Platforms.Shared.Desktop.OsBrowser.DefaultOsBrowserWebUi.<AcquireAuthorizationAsync>d__7.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.AuthCodeRequestComponent.<FetchAuthCodeAndPkceInternalAsync>d__7.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.AuthCodeRequestComponent.<FetchAuthCodeAndPkceVerifierAsync>d__4.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.InteractiveRequest.<GetTokenResponseAsync>d__11.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.InteractiveRequest.<ExecuteAsync>d__9.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.<RunAsync>d__12.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.ApiConfig.Executors.PublicClientExecutor.<ExecuteAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult() 

Relevant code snippets

where ApplicationId should be substituted by real applicationId

        static async Task  Main(string[] args)
        {
            bool endApp = false;

            Console.WriteLine("Sample app\r");

            IPublicClientApplication publicClientApp;
            PublicClientApplicationBuilder applicationBuilder = PublicClientApplicationBuilder.Create(ApplicationId);
            applicationBuilder = applicationBuilder.WithRedirectUri("http://localhost:7890");
            publicClientApp = applicationBuilder.Build();

            while (!endApp)
            {
                string numInput1 = "";
                Console.Write("Hit any key to run acquire token async");

                numInput1 = Console.ReadLine();
                Console.Write("\nRunning the AcquireTokenInteractive step\n");

                string scopeUrl = "api://ApplicationId/access_as_user";
                string[] scopes = new string[] { scopeUrl };

                Prompt prompt = Prompt.ForceLogin;

                IntPtr consoleWindowHandle = Process.GetCurrentProcess().MainWindowHandle;
                string extraParams = "";
                var cancellationTokenSource = new CancellationTokenSource();
                CancellationToken cancellationToken = cancellationTokenSource.Token;
                try
                {
                    AuthenticationResult authResult = await publicClientApp.AcquireTokenInteractive(scopes)
                                    .WithUseEmbeddedWebView(false)
                                    .WithParentActivityOrWindow(consoleWindowHandle)
                                    .WithPrompt(prompt)
                                    .WithExtraQueryParameters(extraParams)
                                    .ExecuteAsync(cancellationToken);
                    Console.WriteLine(authResult.ToString());
                }
                catch (Exception ex)
                {
                    Console.WriteLine($"Caught exception! {ex.ToString()}");
                }
                Console.Write("Done\n");
            }
        }

Expected behavior

MSAL library should not throw an error in this case.

Identity provider

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

Regression

No response

Solution and workarounds

Issue happens highly intermittently ( 1 of 15 tries ) if applicationBuilder.WithRedirectUri("http://localhost"); is provided as redirect URL, if stable port is used, as : applicationBuilder.WithRedirectUri("http://localhost:7890"); the issue will happen at least 1 of 2 times

bgavrilMS commented 11 months ago

Why are you calling AcquireTokenInteractive so often? After the first call, there will be a refresh token in the cache, and AcquireTokenSilent will work, even if you call it for other resources.

I recommend you move away from the system browser and use WAM, see https:\aka.ms\msal-net-wam - this will solve the problem on Windows at least. Does the issue happen on Mac too?

thecrazyonestherebels commented 11 months ago

This is a minimal reproducible sample to showcase the issue, AcquireTokenInteractive is not necessarily called frequently, in actual use case this can be minutes / hours away in calls.

After the first call, there will be a refresh token in the cache, and AcquireTokenSilent will work, even if you call it for other resources.

The intention is not to refresh the token but ask it anew, as if application was locked and then unlock scenario happens and we want to ask the user to enter his credential again

I recommend you move away from the system browser and use WAM, see https:\aka.ms\msal-net-wam - this will solve the problem on Windows at least.

I am not sure WAM fits all my use cases, yet still the bug exists for external browser scenario, thus is there any chance to fix it or does external browser support is to be removed?

Does the issue happen on Mac too?

I have no access to Mac to check

bgavrilMS commented 11 months ago

Yes, we are moving away from "system browser" on desktop operating systems. Embedded browsers offer a slightly better user experience and the broker (which also uses an embedded browser) offers improved security. We could take a contribution on this one though.

Also, just calling AqcuireTokenIntreactive is not a strong way to enforce the "app lock". A malitious app could make pose as your app but make use of the refresh token. Have you considered https://learn.microsoft.com/en-us/entra/identity/conditional-access/howto-conditional-access-session-lifetime