[Bug] WithExtraQueryParameters in AcquireToken(Silent/Interactive)ParameterBuilder persists "resource" parameter across token requests

[aries-zhang]

trafficstars

Library version used

4.51.0

.NET version

4.8.0

Scenario

PublicClient - desktop app

Is this a new or an existing app?

None

Issue description and reproduction steps

We have a 1st party desktop app built with WPF on .Net Framework 4.8.0 using the following MSAL libs for user authentication.

Microsoft.Identity.Client (4.51.0)
Microsoft.Identity.Client.Desktop (4.51.0)
Microsoft.Identity.Client.Extensions.Msal (2.22.0)

In previous versions we only support Graph tokens, e.g. a silent token request with the following parameters. This works well with various sets of graph scopes.

// account: an account returned by Client.GetAccountAsync(identifier)
// scopes: ["https://graph.microsoft.com/.default"]
AuthenticationResult result = await Client.AcquireTokenSilent(scopes ?? Array.Empty<string>(), account)
    .WithForceRefresh(true)
    .ExecuteAsync()
    .ConfigureAwait(false);

In a recent version we extended our support to a few other token types including Substrate tokens, which requires an extra "resource" parameter, so the code is updated to add WithExtraQueryParameters call in the acquire token parameter builder. e.g. a silent Substrate token request:

// account: an account returned by Client.GetAccountAsync(identifier)
// scopes: empty array
// extraParameters: {"resource": "https://substrate.office.com"}
AuthenticationResult result = await Client.AcquireTokenSilent(scopes ?? Array.Empty<string>(), account)
    .WithForceRefresh(true)
    .WithExtraQueryParameters(extraParameters)
    .ExecuteAsync()
    .ConfigureAwait(false);

The difference between a Substrate token request and a Graph token request is the Substrate token request has an extra query parameter {"resource": "https://substrate.office.com"}, and an empty scope array, while Graph token requests do not have the extra resource parameter, and typically have an non-empty scopes array.

So, with the same code (WithExtraQueryParameters(extraParameters) added), after a Substrate token request, it seems MSAL caches the extra "resource" parameter so a subsequent Graph token request with a null extraParameters is still added the "resource" parameter from the previous Substrate token request. As a result, MSAL incorrectly returns a Substrate token for the Graph token request.

Same issue happens in both AcquireTokenSilent and AcquireTokenInteractive with WithExtraQueryParameters.

After some trial-error we found when the subsequent Graph token request is specified a "resource" value in extraParameters: {"resource": "https://graph.microsoft.com"} instead of null, then the token is correctly returned in the expected format.

Although it seems to fix the issue if we always specify a "resource" value in "ExtraQueryParameters" for every token request, wondering if the "cached" extra query parameter is the intended behavior?

P.S.: how IPublicApplicationClient is initialized:

PublicClientApplicationBuilder.Create(AppId)
    .WithAuthority("https://login.microsoftonline.com/common")
    .WithWindowsBrokerOptions(new WindowsBrokerOptions { ListWindowsWorkAndSchoolAccounts = true })
    .WithWindowsBroker()
    .Build();

Relevant code snippets

No response

Expected behavior

No response

Identity provider

Microsoft Entra ID (Work and School accounts and Personal Microsoft accounts)

Regression

No response

Solution and workarounds

No response

[pmaytak]

There are two types of WithExtraQueryParameters, some on application builder (makes the parameters application-wide) and some on the acquire token builder (for per-request parameters). Took a quick look and they both set the correct config. We'll need to investigate more.

[aries-zhang]

There are two types of WithExtraQueryParameters, some on application builder (makes the parameters application-wide) and some on the acquire token builder (for per-request parameters). Took a quick look and they both set the correct config. We'll need to investigate more.

In this case it's the second type of WithExtraQueryParameters on acquire token builder, so the expectation seems correct if it should only take effect per request. Thanks for looking into it 👍

[aries-zhang]

Hi @pmaytak some more observations: when multiple token requests happen simultaneously there is a chance the resource is mis-applied on some of them and it returns an incorrect token, so the extra query parameter does not seem to be thread safe.

[aries-zhang]

Hi @pmaytak a gentle ping: any fix plans yet?