microsoftgraph / microsoft-graph-toolkit

Authentication Providers and UI components for Microsoft Graph 🦒
https://docs.microsoft.com/graph/toolkit/overview
Other
924 stars 287 forks source link

[BUG] The event 'logoutCompleted' for the component mgt-login is not firing. #2951

Open GyllionElderen opened 5 months ago

GyllionElderen commented 5 months ago

Describe the bug The event 'logoutCompleted' for the component mgt-login is not firing.

To Reproduce Steps to reproduce the behavior:

  1. Create a new .NET 6.0 application
  2. Enter the following script in the header section to include the mgt library: <script src="@("https://unpkg.com/@microsoft/mgt@3/dist/bundle/mgt-loader.js")"></script> (also make sure to that JQuery has been included for the following steps).
  3. In the Index.html put the following elements to get the "Sign In" button. Make sure to replace the "(YOUR CLIENT ID)" section with the client ID of your app registration in Azure Entra ID:
    
    <mgt-msal2-provider client-id="(YOUR CLIENT ID)"></mgt-msal2-provider>
    <mgt-login></mgt-login>
5. In the site.js file, put the following code:

$(document).ready(function () { document.querySelector('mgt-login').addEventListener('logoutCompleted', function () { console.log('Logout Completed!') }); });


6. Run the application and sign in. Afterwards sign out again and check the console. No "Logout Completed!" log is visible in the console.

**Expected behavior**
The 'logoutCompleted' is firing and the "Logout Completed!" message is visible in the console.

**Environment (please complete the following information):**
 - OS: Windows 10
 - Browser: Chrome, Firefox, Edge
 - Framework: ASP.NET Core, .NET 6.0
 - MGT Version: 3.1.3
 - Provider: mgt-msal2-provider
sebastienlevert commented 5 months ago

I was able to reproduce with the same experience using 3.1.3. Would you be willing to help us fix this issue by submitting a PR? Thanks!

gavinbarron commented 5 months ago

@sebastienlevert is this possibly resolved in #2934 by https://github.com/microsoftgraph/microsoft-graph-toolkit/pull/2934/files#r1447609895 ?

sebastienlevert commented 5 months ago

This PR would only fix the loginInitiated. This line is not touched but I don't understand why it's not reached though... https://github.com/microsoftgraph/microsoft-graph-toolkit/blob/fffb12b9cf06041b37bc857854942ec44a2a01f7/packages/mgt-components/src/components/mgt-login/mgt-login.ts#L300

gavinbarron commented 5 months ago

OP is interested in logoutCompleted not loginCompleted

sebastienlevert commented 5 months ago

My mistake, Copy/Past failures.

https://github.com/microsoftgraph/microsoft-graph-toolkit/blob/fffb12b9cf06041b37bc857854942ec44a2a01f7/packages/mgt-components/src/components/mgt-login/mgt-login.ts#L254

The same happens here. It should get triggered (we don't have the same weird logic we had with loginInitiated.

gavinbarron commented 5 months ago

I wonder if it's the second attempt to clear the cached data after provider.logout and there not being an active account?

sebastienlevert commented 5 months ago

I wonder if it's the second attempt to clear the cached data after provider.logout and there not being an active account?

This should only when there are multiple accounts logged in... But maybe?

gavinbarron commented 5 months ago

There's no check there about the number of accounts, just the configuration of the provider.