fix: Clear persisted MSAL token cache on Disconnect-MgGraph

[gavinbarron]

Summary

Fixes #3648

When Disconnect-MgGraph is called with ContextScope == CurrentUser, the persisted MSAL token cache on disk was not being cleared. This PR adds cache clearing using MsalCacheHelper.Clear() from the MSAL Extensions library.

Changes

• Added explicit Microsoft.Identity.Client.Extensions.Msal v4.78.0 package reference to Authentication.Core.csproj
• Added TokenCacheUtilities.cs — builds StorageCreationProperties matching Azure.Identity's internal cache layout (Windows DPAPI, macOS Keychain, Linux KeyRing) and clears both .cae and .nocae cache variants
• Modified AuthenticationHelpers.LogoutAsync() — calls TokenCacheUtilities.ClearPersistedTokenCacheAsync() when ContextScope == CurrentUser, with non-fatal error handling (disconnect still succeeds if cache clear fails)
• Added unit tests for the new behavior
• Updated Disconnect-MgGraph documentation

Design Decisions

• Non-fatal error handling: Cache clearing failure does not prevent disconnect from succeeding — it's wrapped in try/catch
• Both CAE variants cleared: Azure.Identity creates separate .cae and .nocae cache files; both are cleared
• Matches Azure.Identity internals: StorageCreationProperties are configured to match the exact storage layout Azure.Identity uses (directory, keychain service, keyring schema/attributes)

Testing

• 3 new unit tests added (all pass on net8.0 and net472)
• All existing 77 tests continue to pass on net8.0

[jsquire]

This looks correct to me, though I don't have a deep understanding in the area. I've asked @g2vinay and @JonathanCrd to also review. I'd consider them to be the authoritative voices from the Azure.Identity perspective.

[ramsessanchez]

Approving the changes, however, in line with @jsquire 's comments, we should await feedback from the folks he mentioned. In order to reduce the change of needing a patch release later on, lets await their sign-off and before merging and releasing.

[gavinbarron]

Agreed

[JonathanCrd]

on Windows, WAM is on by default for interactive sign-in, and WAM keeps the account + refresh token in its own cache. So cleaning this file won't actually sign the user out of WAM.

For the broker we would need to ask WAM to forget the account. Is the Windows Broker in scope for this fix?

Copilot suggested something like:

if (ShouldUseWam(authContext))
{
    var pca = PublicClientApplicationBuilder
        .Create(authContext.ClientId)
        .WithAuthority(GetAuthorityUrl(authContext))
        .WithBroker(new BrokerOptions(BrokerOptions.OperatingSystems.Windows))
        .Build();
    foreach (var account in await pca.GetAccountsAsync().ConfigureAwait(false))
        await pca.RemoveAsync(account).ConfigureAwait(false);
}

[gavinbarron]

@JonathanCrd I think we probably need to clear the WAM token cache if possible to ensure a more complete fix.

[gavinbarron]

Additionally, would using this approach avoid the need to have the TokenCacheUtilities class?

[JonathanCrd]

I don't think so, the broker only covers Windows, and there are non-broker auth flows on windows too.

[gavinbarron]

@JonathanCrd Sorry, I wasn't very clear here I was more meaning the general approach of using a PCA instance to clear the cached tokens not that specific implementation. Maybe something like this to provide broad cover and eliminate the additional code that knows about the caching specifics

      var builder = PublicClientApplicationBuilder
              .Create(authContext.ClientId)
              .WithAuthority(GetAuthorityUrl(authContext));
      if (ShouldUseWam(authContext))
      {
          builder = builder
              .WithBroker(new BrokerOptions(BrokerOptions.OperatingSystems.Windows));
      }
      var pca = builder.Build();
      foreach (var account in await pca.GetAccountsAsync().ConfigureAwait(false))
          await pca.RemoveAsync(account).ConfigureAwait(false);

[g2vinay]

TokenCacheUtilities -> ClearPersistedTokenCacheAsync still needs to be retained as it deletes the file based caches. PCA code above is needed on top to clear the WAM caches.

Just heads up
WAM is shared broker storage across Windows Azure tools. Removing accounts would also sign out:
Visual Studio
Azure CLI
Azure PowerShell
VS Code Azure extensions
Any app using Azure.Identity with broker enabled

This can impact UX for other logged in flows.

[JonathanCrd]

Why is Clear() the intentional approach here? This would wipe out the entire MSAL Cache, removing all accounts, not only the ones in the current context.

[gavinbarron]

My thinking here is that it would be roughly equivalent to the approach of iterating over the results from .GetAccounts() to remove each or them, how does this differ given that the cache files are managed per machine user?

[JonathanCrd]

I see. My comment was more about removing only the account being disconnected rather than all accounts, mainly from a UX perspective. That said, if there aren't any issues with clearing all accounts, then this approach works for me.

[gavinbarron]

The CLI does not offer an account parameter, so any user account selection is done during the browser flow, which requires Disconnect-MgGraph to trigger the account selection again, so this feel like the right option. I suppose that there could be the case where a user has closed a terminal and is running Connect-MgGraph again triggering account selection.
But either way I'm comfortable with fully clearing out any cached tokens,

[g2vinay]

Msal cache scope is per window user, so .clear() will only scope to disconnecting user, I believe.

[JonathanCrd]

Overall it looks good but I left a couple of comments about certain scenarios that might be missing and some questions too.

[JonathanCrd]

Any actual failure here would be silent, could you add a log here so it's diagnosable?

[g2vinay]

Looks fine to me, left my comments on the ongoing discussions.