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

[Unexpected behaviour] WAM Broker with CAE token cache

Open peter1155 opened this issue 2 years ago • 4 comments

Library version used

4.56.0

.NET version

net6.0-windows10.0.17763.0

Scenario

PublicClient - desktop app

Is this a new or an existing app?

The app is in production, I haven't upgraded MSAL, but started seeing this issue

Issue description and reproduction steps

We recently switched from usage of SystemWebBrowser to WAM and started to see some strange behavior regarding continuous access evaluation. After user session is revoked and token requested the MsalUiRequiredException is raised by MSAL then after successful interactive login the token is acquired. Then later token is acquired by AcquireTokenSilent and send to MsGraph API. The API retruns HTTP 401 with claim challange. (I am not sure whether this is expected after interactive login.) After passing the claim challange to MSAL using .WithClaims() access token is successfully acquired. But the next call to acquire token without the claim challange returns invalid token and again MsGraph API returns 401.

So I am not sure is CAE supported in conjunction with WAM ? We are using Microsoft.Identiti.Client.Broker 4.56.0.

Relevant code snippets

...
    public async Task<AuthenticationResult> GetAccessTokenByEmailWithPossibleUiPromptAsync(IEnumerable<AudiencePermissions> audiencePermissionsList,
    string email,
    IEnumerable<AudiencePermissions> consentAudiencePermissionsList = null,
    CancellationToken cancellationToken = default,
    bool retriedOnFailure = false,
    bool accountSwitchAllowed = false,
    string claimChallenge = null)
{
    var account = await FindLocalAccount(email);
    var scopes = GetScopes(audiencePermissionsList);
    var consentScopes = consentAudiencePermissionsList != null
        ? GetScopes(consentAudiencePermissionsList)
        : Enumerable.Empty<string>();

    // Try get token from cache
    try
    {
        if (account != null)
        {
            var silentAuthResult = await AcquireTokenSilentlyAsync(scopes, account, claimChallenge, cancellationToken);
            if (silentAuthResult != null)
            {
                log.LogInformation("Token acquired silently");
                telemetryLogger.LogTrace(LogEvent.BusinessLogic.Authentication.AccessToken.AcquireSilently, "Token acquired silently.");
                return Parse(silentAuthResult, false);
            }
        }

        return await AcquireTokenInteractivelyAsync(scopes, consentScopes, email, cancellationToken, true, accountSwitchAllowed, claimChallenge);
    }
    catch (MsalUiRequiredException ex)
    {        
        return await AcquireTokenInteractivelyAsync(scopes, consentScopes, email, cancellationToken, retriedOnFailure, accountSwitchAllowed, claimChallenge ?? ex.Claims);

    }
}
....

private async Task<AuthenticationResult> AcquireTokenInteractivelyAsync(IEnumerable<string> scopes,
    IEnumerable<string> consentScopes,
    string email,
    CancellationToken cancellationToken = default,
    bool retriedOnFailure = false,
    bool accountSwitchAllowed = false,
    string claimChallenge = null)
{
   
 ....
        async () =>
        {
            try
            {
                var builder = PublicClient.AcquireTokenInteractive(scopes);
                builder = builder
                            .WithLoginHint(email)
                            .WithClaims(claimChallenge)
                            .WithParentActivityOrWindow(appWindowHandleProvider.GetAppWindowHandle())
                            .WithPrompt(Prompt.ForceLogin)
                            .WithCorrelationId(CorrelationContext.Current.CorrelationId);

                if (!accountServiceOptions.UseMsalWindowsBroker)
                {
                    builder.WithUseEmbeddedWebView(false);
                }

                if (consentScopes.Any())
                {
                    builder = builder.WithExtraScopesToConsent(consentScopes);
                }

                var authResponse = await builder.ExecuteAsync(CancellationToken.None);
                var wasAccountSwitched = authResponse?.Account?.Username != email;

                if (wasAccountSwitched && !accountSwitchAllowed)
                {
                    throw new AccountSwitchDeniedException(email);
                }

                log.LogInformation("User {User} was successfully authenticated", authResponse.Account.Username);

                return Parse(authResponse, wasAccountSwitched);
            }
            catch (MsalClientException ex) when (ex.ErrorCode?.Equals("authentication_canceled") ?? false)
            {
                throw new MsalAuthenticationCanceledException(ex);
            }
            catch (MsalServiceException ex) when (ex.ErrorCode?.Equals("access_denied") ?? false)
            {
                throw new OperationCanceledException(ex.Message, ex);
            }            
        },
....
....
        private async Task<MSIdentityClient.AuthenticationResult> AcquireTokenSilentlyAsync(IEnumerable<string> scopes, IAccount account, string claimChallenge = null, CancellationToken cancellationToken = default)
        {
            return await client.AcquireTokenSilent(scopes, account)
                               .WithClaims(claimChallenge)
                               .ExecuteAsync(cancellationToken);
        }
...

Expected behavior

I would expect that after acquiring token with non empty claim challenge the previous cached token should be invalidated. And each subsequent call to AcquireTokenSilent/AcquireTokenInteractive should return updated token.

Identity provider

Other

Regression

No response

Solution and workarounds

No response

peter1155 avatar Nov 22 '23 14:11 peter1155

Thanks for reporting. @ashok672, @iulico-1 - this is potentially breaking the CAE scenario, so marking as P1.

bgavrilMS avatar Nov 22 '23 15:11 bgavrilMS

@iulico-1 @ashok672 - I think this one needs to be investigated with priority, as it seems to break critical CAE scenario.

I'm wondering if there's a mismatch in how MSAL C++ and other MSALs handle claims?

With MSAL.NET alone, once a claims challenge comes in, we ask the app developer to use WithClaims(challenge) - this bypasses the token cache. The new token overrides the old token. Claims are not part of the cache key.

With MSAL C++, afaik claims are part of the key. Are apps supposed to send the claim over and over ?

bgavrilMS avatar Dec 12 '23 10:12 bgavrilMS