Microsoft-authentication-library-for-dotnet: [Bug] AcquireTokenSilent always calls a new refresh token when .WithAuthority is set on it.

Created on 18 Oct 2019  路  6Comments  路  Source: AzureAD/microsoft-authentication-library-for-dotnet

Which Version of MSAL are you using ?
MSAL 4.5.0

Platform
net45, netcore

What authentication flow has the issue?
AcquireTokenSilent

Other? - please describe;

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

Repro

public async Task<AuthenticationResult> GetAuthTokenForGraphAsync(string clientId, string cacheFileName, string cacheFilePath)
{
    IPublicClientApplication publicClientApp = PublicClientApplicationBuilder
                           .Create(clientId)
                           .Build();

    StorageCreationProperties storageCacheProperty = new StorageCreationPropertiesBuilder(cacheFileName, cacheFilePath, clientId)
        .Build();

    MsalCacheHelper msalCacheHelper = await MsalCacheHelper.CreateAsync(storageCacheProperty);
    msalCacheHelper.RegisterCache(publicClientApp.UserTokenCache);

    List<string> scopes = new List<string> { "User.Read" };

    var accounts = await publicClientApp.GetAccountsAsync();
    try
    {
        return await publicClientApp.AcquireTokenSilent(scopes, accounts.FirstOrDefault())
        // .WithAuthority(publicClientApp.Authority) causes MSAL to always call a new refresh token even if the token cache is in a valid state.
                .WithAuthority(publicClientApp.Authority)
                .ExecuteAsync();
    }
    catch (MsalUiRequiredException)
    {
        return await publicClientApp.AcquireTokenWithDeviceCode(scopes,
                    deviceCodeResult =>
                    {
                        Console.WriteLine(deviceCodeResult.Message);
                        return Task.FromResult(0);
                    }).ExecuteAsync();
    }
}

var authenticationProvider = new DelegateAuthenticationProvider(async (request) =>
{
    string clientId = YOUR_CLIENT_ID;
    string cacheFileName = "UserCacheFileName.bin3";
    string cacheFilePath = YOUR_CACHE_FILE_PATH;
    AuthenticationResult result = await GetAuthTokenForGraphAsync(clientId, cacheFileName, cacheFilePath);
    request.Headers.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", result.AccessToken);
});

GraphServiceClient graphClient = new GraphServiceClient(authenticationProvider);

// Make request. 
for (int i = 0; i < 25; i++)
{
    User me = await graphClient.Me.Request().GetAsync();
    Console.WriteLine(me.Id);
}

Expected behavior
MSAL should always use a valid token from the token cache when .WithAuthority(URI) is set on publicClientApp.AcquireTokenSilent(......).

Actual behavior
When .WithAuthority(URI) is set on publicClientApp.AcquireTokenSilent(......), MSAL always calls https://login.microsoftonline.com/common/oauth2/v2.0/token to get a refresh token for every call. This behavior is expensive, and the calling app occasionally gets throttled for making looping calls.

Additional context/ Logs / Screenshots
Current call pattern:
image

Expected call pattern:
image

This issue could potentially be present in other OAuth flows besides device code flow.

Fixed P2 bug

All 6 comments

@peombwa.
Thanks for the heads-up.
We'll investigate.

Which value if authority do you have? only the default one?

The authority passed in is login.microsoftonline.com/common and we wrongly try to use common as tenant ID (we should use the tenant ID of the account in this case).

Generally I don't think passing common to AcquireTokenSilent should be allowed, the authority override is there to help disambiguate between access tokens that are created for different tenantIDs. @jmprieur - would you agree?

Workaround: do not pass in the authority or use a tenanted authority

I agree @bgavrilMS: the scenario for using .WithAuthority on AcquireTokenSilent, is really to get a token for the user in a differnt tenant (guest scenario). However, we have not really documented the constraint yet, and I could se why customers would think it's fine to use common.

I need to think more about what we want to do.
cc: @henrik-me @jennyf19 @trwalke what do you think?

@jmprieur By default, we pass login.microsoftonline.com/common, but we allow our customers to specify their own.

@peombwa As a workaround, we can check if the first segment of Authority is "common", "organizations", or "consumers" and if so, just not call WithAuthority() on AcquireTokenSilent.

@peombwa Included in 4.6 release

Was this page helpful?
0 / 5 - 0 ratings