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

MsalClientException during AcquireTokenSilentAsync with forcerefresh true #695

Closed
2 of 13 tasks
atiqhaqu opened this issue Nov 30, 2018 · 7 comments
Closed
2 of 13 tasks
Assignees
Milestone

Comments

@atiqhaqu
Copy link


name: Bug report
about: MsalClientException during AcquireTokenSilentAsync with forcerefresh true

Which Version of MSAL are you using ?
Note that to get help, you need to run the latest preview or non-preview version
For ADAL, please log issues to https://github.com/AzureAD/azure-activedirectory-library-for-dotnet

v2.5.0-preview

Which platform has the issue?

net45

What authentication flow has the issue?

  • Desktop
    • Interactive
    • Integrated Windows Auth
    • Username / Password
    • Device code flow (browserless)
  • Mobile
    • Xamarin.iOS
    • Xamarin.Android
    • UWP
  • Web App
    • Authorization code
    • OBO
  • Web API
    • OBO
  • Daemon App
    • Client credentials

Other? - please describe;

What is the identity provider ?

  • Azure AD
  • Azure AD B2C

If B2C, what social identity did you use?

Repro

public static async Task<AuthenticationResult> AcquireTokenWithPromptAsync(string resource, string tenant)
        {
            string authority = $"{ProductionAuthority}/{tenant}";
            string[] scopes = { resource + ".default" };
            string clientId = Client;
            var app = new PublicClientApplication(clientId: clientId, authority: authority, userTokenCache: TokenCacheHelper.GetUserCache());

            return await app.AcquireTokenAsync(scopes).ConfigureAwait(false);
        }

        public static async Task<AuthenticationResult> AcquireTokenSilentAsync(string resource, string tenant, string userId)
        {
            string authority = $"{ProductionAuthority}/{tenant}";
            string[] scopes = { resource + ".default" };
            string clientId = Client;
            AuthenticationResult result = null;
            var app = new PublicClientApplication(clientId: clientId, authority: authority, userTokenCache: TokenCacheHelper.GetUserCache());
            var account = await app.GetAccountAsync(userId).ConfigureAwait(false);
            result = await app.AcquireTokenSilentAsync(scopes, account, authority, true).ConfigureAwait(false);
            return result;
        }

var authResult = await AcquireTokenWithPromptAsync("https://management.azure.com", "Common");

// suppose the home tenantId for the user is foo and it is also part of another tenant with tenantId bar

// this call works and get a token for foo
var result = await AcquireTokenSilentAsync("https://graph.microsoft.com/", foo, authResult.Account.HomeAccountId.Identifier).ConfigureAwait(false);

// this one fails and throws an exception
var result = await AcquireTokenSilentAsync("https://graph.microsoft.com/", bar, authResult.Account.HomeAccountId.Identifier).ConfigureAwait(false);

Expected behavior
A token for the requested tenant

Actual behavior
An exception -
Microsoft.Identity.Client.MsalClientException: The cache contains multiple tokens satisfying the requirements. Try to clear token cache
at Microsoft.Identity.Client.TokenCache.FindAccessTokenCommon(AuthenticationRequestParameters requestParams, String preferredEnvironmentAlias, ISet`1 environmentAliases)
at Microsoft.Identity.Client.TokenCache.d__47.MoveNext()

If force refresh is set to false, then the call returns a token for the home tenant.

Possible Solution

Additional context/ Logs / Screenshots
Add any other context about the problem here, such as logs and screebshots. Logging is described at https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/logging

@jennyf19
Copy link
Collaborator

jennyf19 commented Dec 6, 2018

Expected behavior:
With ForceRefresh = true in the silent call, we should ignore any access token in the cache and attempt
to acquire a new access token using the refresh token

Issues:

  1. With ForceRefresh = true in the silent call, we check for access tokens in the cache (more below w/Fix 1)

  2. After fixing ForceRefresh logic, if passing in ForceRefresh = false, or nothing, the cache should add one account for each tenant, but the HomeAccountId.Identifier is the same for all the accounts, so even though the authority being used is different (different tenant), there will only ever be one access token because all the other look up logic is the same (clientId, environmentAlias, scopes, user)

Fixes:

  1. Move the logic to check for ForceRefresh = true so that we circumvent the cache lookup. Right now, even if the dev passes in true, we still
    look up the AT in the cache. Find AT lookup happening on line 63 and check for ForceRefresh on line 74

This later causes a MultipleTokensMatchedError because the look-up is happening when it should not.

  1. Moving ForceRefresh logic opened up another issue, which is when ForceRefresh is false or not passed at all.
    Here is an example:
    I sign-in with my corp account. I am part of 7 different tenants. After the interactive call, I'm going to do a silent call with each of my tenants.
    Interactive call authority w/corp account
    Silent call authority = https://login.microsoftonline.com/"72f...ab"/ -> Microsoft
    Silent call authority = https://login.microsoftonline.com/"913...f7"/ -> iddxb2csamplestesting
    Silent call authority = https://login.microsoftonline.com/"d33...27"/ -> CatnadoReturns
    etc...

The HomeAccountId.Identifier is the same across all the tenants.
So, when doing the cache lookup, we only match for HomeAccountId.Identifier, so I can never get another token for a different authority because we check for PreferredEnvironmentAlias, which is login.windows.net, across all these tenants.

PR in progress here
@atiqhaqu @henrik-me

@jennyf19
Copy link
Collaborator

jennyf19 commented Dec 7, 2018

fix will be out in msal2.6.1-release
@atiqhaqu

@jennyf19 jennyf19 added this to the 2.6.1 milestone Dec 7, 2018
@jmprieur jmprieur added the Fixed label Dec 7, 2018
@ferronsw
Copy link

This problem still occurs (in v2.7.1) when the CCA uses a ClientAssertionCertificate instead of a clientsecret.

@jennyf19
Copy link
Collaborator

@ferronsw Would you mind opening a new issue w/repro steps and logs from latest release (3.0.4-preview)? thank you.

@ferronsw
Copy link

@jennyf19 @jmprieur this problem does not occur in v3.0.4.

@jmprieur
Copy link
Contributor

Thanks for confirming, @ferronsw

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

No branches or pull requests

5 participants