Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Crashing Microsoft.Identity.Client.Broker - WithBrokerPreview() #3654

Closed
7 tasks
mcpat-it opened this issue Sep 7, 2022 · 10 comments · Fixed by #3664 or #3677
Closed
7 tasks

[Bug] Crashing Microsoft.Identity.Client.Broker - WithBrokerPreview() #3654

mcpat-it opened this issue Sep 7, 2022 · 10 comments · Fixed by #3664 or #3677
Assignees
Milestone

Comments

@mcpat-it
Copy link

mcpat-it commented Sep 7, 2022

Logs and network traces

During logout:
Ausnahme ausgelöst: "Microsoft.Identity.Client.NativeInterop.MsalRuntimeException" in Microsoft.Identity.Client.NativeInterop.dll
GlobalExceptionHandler: Status: ApiContractViolation
Context: (pii)
Tag: 0x2039c1cb 
Ausnahme ausgelöst: "Microsoft.Identity.Client.NativeInterop.MsalRuntimeException" in Microsoft.Identity.Client.NativeInterop.dll
GlobalExceptionHandler: Status: ApiContractViolation
Context: (pii)
Tag: 0x2039c1cb 
During login:

no logs, app close without any error in VisualStudio. But not always (~70% crash), sometimes login is working.

Which version of MSAL.NET are you using?
Microsoft.Identity.Client 4.46.2

Platform
.NET 6 WPF

What authentication flow has the issue?

  • Desktop / Mobile
    • Interactive
    • Integrated Windows Authentication
    • Username Password
    • Device code flow (browserless)
  • Web app
    • Authorization code
    • On-Behalf-Of
  • Daemon app
    • Service to Service calls

Is this a new or existing app?
This is a new app or experiment.

Repro

readonly string tenantId = "common";
private static string Instance = "https://login.microsoftonline.com/";
readonly string clientId = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx";
private IPublicClientApplication? _clientApp;
private IPublicClientApplication? PublicClientApp { get { return _clientApp; } }
private static AuthenticationResult? authResult;

...

_clientApp = PublicClientApplicationBuilder.Create(clientId)
                    .WithAuthority($"{Instance}{tenantId}")
                    .WithRedirectUri("http://localhost")
                    .WithBrokerPreview()
                    .Build();

...
authResult = await PublicClientApp.AcquireTokenInteractive(scopes)
                        .WithParentActivityOrWindow(new WindowInteropHelper(window).Handle)
                        .WithUseEmbeddedWebView(true)
                        .ExecuteAsync();
...

//logout
IEnumerable<IAccount> accounts = await PublicClientApp.GetAccountsAsync();
                if (accounts.Any())
                {
                    try
                    {
                        await PublicClientApp.RemoveAsync(accounts.First());
                    }
                    catch (MsalException ex)
                    {
                        Trace.WriteLine($"MsalException Error signing-out user: {ex.Message}");
                    }
                    catch (Exception ex)
                    {
                        Trace.WriteLine($"Exception Error signing-out user: {ex.Message}");
                    }
                }

Expected behavior
No crashing app, no errors on logout.

Actual behavior
During login, the app is crashing very often, during logout I get errors as above written

Possible solution
not using WithBrokerPreview(), instead see alternative code.

Alternative working code
same as above, just remove

.WithBrokerPreview()

and you receive the "old" Loginscreen from Microsoft.

@bgavrilMS bgavrilMS added the bug label Sep 7, 2022
@bgavrilMS bgavrilMS added this to the 4.47.0 milestone Sep 7, 2022
@gladjohn
Copy link
Contributor

gladjohn commented Sep 7, 2022

Filed an internal bug against the partner

@mcpat-it
Copy link
Author

mcpat-it commented Sep 8, 2022

I found new bugs,

  1. if I login and logout and login and logout (if app is not crashing...), then I receive this error and app crashes:
    Ein Ausnahmefehler des Typs "System.ExecutionEngineException" ist in Unbekanntes Modul. aufgetreten.
  2. If I cancel the login progress on closing the popup-window, and even I have the code in a try/catch (MsalClientException ex) or try/catch (Exception ex) a 70% chance of crash without any debug message.
    If no crash, then I get a MSAL.NetCore.4.46.2.0.MsalClientException: ErrorCode: authentication_canceled
try
{
...
authResult = await app.AcquireTokenInteractive(scopes)
                        .WithParentActivityOrWindow(new WindowInteropHelper(window).Handle)
                        .WithUseEmbeddedWebView(true)
                        .ExecuteAsync();
}
catch (MsalClientException ex)
{
...
}

vs.

try
{
...
authResult = await app.AcquireTokenInteractive(scopes)
                        .WithParentActivityOrWindow(new WindowInteropHelper(window).Handle)
                        .WithUseEmbeddedWebView(true)
                        .ExecuteAsync();
}
catch (Exception ex)
{
...
}
  1. If I wait (session timeout) I can see
Ausnahme ausgelöst: "System.IO.IOException" in System.Net.Sockets.dll
GlobalExceptionHandler: Unable to read data from the transport connection: Der E/A-Vorgang wurde wegen eines Threadendes oder einer Anwendungsanforderung abgebrochen.. System.Net.Sockets.SocketException (995): Der E/A-Vorgang wurde wegen eines Threadendes oder einer Anwendungsanforderung abgebrochen.
Ausnahme ausgelöst: "System.IO.IOException" in System.Net.Security.dll
GlobalExceptionHandler: Unable to read data from the transport connection: Der E/A-Vorgang wurde wegen eines Threadendes oder einer Anwendungsanforderung abgebrochen.. System.Net.Sockets.SocketException (995): Der E/A-Vorgang wurde wegen eines Threadendes oder einer Anwendungsanforderung abgebrochen.
Ausnahme ausgelöst: "System.IO.IOException" in System.Private.CoreLib.dll
GlobalExceptionHandler: Unable to read data from the transport connection: Der E/A-Vorgang wurde wegen eines Threadendes oder einer Anwendungsanforderung abgebrochen.. System.Net.Sockets.SocketException (995): Der E/A-Vorgang wurde wegen eines Threadendes oder einer Anwendungsanforderung abgebrochen.

and then, after clicking to an account, app crash silently without any debug message.

@gladjohn
Copy link
Contributor

gladjohn commented Sep 9, 2022

Hi @pwallner with the repro steps you provided I see this error that leads to the crash you are seeing,

Managed Debugging Assistant 'CallbackOnCollectedDelegate' : 'A callback was made on a garbage collected delegate of type 'Microsoft.Identity.Client.NativeInterop!Microsoft.Identity.Client.NativeInterop.API+MSALRUNTIME_COMPLETION_ROUTINE::Invoke'. This may cause application crashes, corruption and data loss. When passing delegates to unmanaged code, they must be kept alive by the managed application until it is guaranteed that they will never be called.'

Next MSAL release planned mid next week will have a fix for this issue.

@mcpat-it
Copy link
Author

mcpat-it commented Sep 9, 2022

Hi @gladjohn

This explains # 1 as I read, but not sure if it explains all other points. For now I use the broker (without preview), and there are no errors.

@gladjohn
Copy link
Contributor

gladjohn commented Sep 9, 2022

Hi @pwallner

for the original issue that you see,

GlobalExceptionHandler: Status: ApiContractViolation Context: (pii) Tag: 0x2039c1cb

I am able to repro this when no scopes is passed, this works with the old WAM experience but not the Preview. We will have a fix for this in the next MSAL release

gladjohn added a commit that referenced this issue Sep 13, 2022
* [Bug] Crashing Microsoft.Identity.Client.Broker - WithBrokerPreview()

#3654

* WamScopesRequired

* mesaage updated

* verbose

* removing oidc scopes check

Co-authored-by: Gladwin Johnson <gljohns@microsoft.com>
@gladjohn gladjohn reopened this Sep 15, 2022
@gladjohn
Copy link
Contributor

This will still fail, if user passes

var authResult = await pca.AcquireTokenInteractive(new[] { "" })

this is tracked by the MSALRuntime team for a fix at their end that will also include checks on the oidc scopes. This will be released after MSAL 4.47.0 release.

@gladjohn
Copy link
Contributor

MSAL 4.47.0 has been released, can you please see if the crashes are fixed for you.

@mcpat-it
Copy link
Author

mcpat-it commented Sep 18, 2022

hi @gladjohn

the app isn't crashing so far (did only some sign in and outs), but I can see during logout (see code above):

GlobalExceptionHandler: Status: ApiContractViolation
Context: (pii)
Tag: 0x2039c1cb 
GlobalExceptionHandler: Status: ApiContractViolation
Context: (pii)
Tag: 0x2039c1cb

if I catch the globalexceptions with

//catch exceptions
            AppDomain.CurrentDomain.FirstChanceException += (sender, eventArgs) =>
            {
                Trace.WriteLine("GlobalExceptionHandler: " + eventArgs.Exception.Message + " " + eventArgs.Exception.InnerException);
            };

@gladjohn
Copy link
Contributor

gladjohn commented Sep 18, 2022

Hi @pwallner can you please provide the authority, scopes and the account type (AAD/MSA) that you are using?

also, can you please open a separate issue for this?

@mcpat-it
Copy link
Author

mcpat-it commented Sep 19, 2022

Hi @gladjohn

I am using my common hotmail address for login.

Opened new issue #3685

here the complete code as requested:

using Microsoft.Graph;
using Microsoft.Identity.Client;
using Microsoft.Identity.Client.Broker;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Net.Http.Headers;
using System.Runtime.InteropServices;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Interop;
using WinCopies.Linq;

namespace mcpat
{
    public class GraphApi
    {
        private GraphServiceClient? graphClient;
        readonly string tenantId = "common";
        readonly string clientId = "xxxxxxxxxxxxxxxxxx";
        readonly string[] scopes = new[] { "User.Read" };
        private readonly IPublicClientApplication? _clientApp;
        private IPublicClientApplication? PublicClientApp { get { return _clientApp; } }
        public GraphApi()
        {
            try
            {
                _clientApp = PublicClientApplicationBuilder.Create(clientId)
                    .WithAuthority(AzureCloudInstance.AzurePublic, tenantId)
                    .WithDefaultRedirectUri()
                    .WithBrokerPreview()
                    .Build();

                TokenCacheHelper.EnableSerialization(_clientApp.UserTokenCache);
            }
            catch (Exception ex)
            {
                Trace.WriteLine("Error GraphApi: " + ex.Message);
            }
        }
        public async Task SignOut()
        {
            try
            {
                if (PublicClientApp == null)
                    return;
                IEnumerable<IAccount> accounts = await PublicClientApp.GetAccountsAsync();

                if (accounts.Any())
                {
                    try
                    {
                        await PublicClientApp.RemoveAsync(accounts.FirstOrDefault());
                        Trace.WriteLine("User has signed-out");
                    }
                    catch (MsalException ex)
                    {
                        Trace.WriteLine($"MsalException Error signing-out user: {ex.Message}");
                    }
                    catch (Exception ex)
                    {
                        Trace.WriteLine($"Exception Error signing-out user: {ex.Message}");
                    }
                }
            } catch (Exception ex)
            {
                Trace.WriteLine($"Error signing-out user: {ex.Message}");
            }  
        }
        public async Task<AuthenticationResult?> GetTokenForUserAsync(Window window)
        {
            try
            {
                IAccount? firstAccount;
                var app = PublicClientApp;
                if (app == null)
                    return null;
                var accounts = await app.GetAccountsAsync();
                firstAccount = accounts.FirstOrDefault();
                AuthenticationResult? authResult = null;

                if (firstAccount != null)
                    authResult = await app.AcquireTokenSilent(scopes, firstAccount).ExecuteAsync();
                else
                    authResult = await app.AcquireTokenInteractive(scopes)
                        .WithParentActivityOrWindow(new WindowInteropHelper(window).Handle)
                        .WithUseEmbeddedWebView(true)
                        .ExecuteAsync();

                return authResult;
            }
            catch (MsalClientException ex)
            {
                int errorCode = Marshal.GetHRForException(ex) & ((1 << 16) - 1);
                Trace.WriteLine("MsalClientException (ErrCode " + errorCode + "): " + ex.ErrorCode);
            }
            catch (Exception ex)
            {
                int errorCode = Marshal.GetHRForException(ex) & ((1 << 16) - 1);
                Trace.WriteLine("Error Acquiring Token (ErrCode " + errorCode + "): " + ex);
            }

            return null;
        }
        public async Task LoginAsync(Window window)
        {
            try
            {
                if (_clientApp == null)
                    return;
                AuthenticationResult? token = await GetTokenForUserAsync(window);
                if (token == null)
                    return;
                
                DelegateAuthenticationProvider authenticationProvider = new((request) =>
                {
                    request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token.AccessToken);
                    return Task.FromResult(0);
                });

                graphClient = new GraphServiceClient(authenticationProvider);

            } catch(Exception ex)
            {
                Trace.WriteLine("Error LoginAsync: " + ex.Message);
            }
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment