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

[Bug] Microsoft.Identity.Client 4.37.0 InMemoryPartitionedAppTokenCacheAccessor.SaveIdToken throws NotSupportedException #2987

Closed
adarce opened this issue Nov 2, 2021 · 4 comments
Assignees
Labels
bug P1 regression Behavior that worked in a previous release that no longer works in a newer release requires more info

Comments

@adarce
Copy link

adarce commented Nov 2, 2021

Which version of MSAL.NET are you using?
Microsoft.Identity.Client 4.37.0

Platform
.NET 4.7.2

What authentication flow has the issue?
During AcquireTokenForClient for a confidential client app.

Is this a new or existing app?
a. The app is in production, and I have upgraded to a new version of MSAL.

Repro

private static async Task<string> GetAccessTokenAsync(string authority, string resource, string clientId, string clientSecret)
{
    IConfidentialClientApplication application = ConfidentialClientApplicationBuilder.Create(clientId)
        .WithAuthority(authority, false)
        .WithClientSecret(clientSecret)
        .Build();

    var scopes = new[] { resource + MsalScopeSuffix };
    var result = await application.AcquireTokenForClient(scopes)
        .ExecuteAsync()
        .ConfigureAwait(false);
    return result.AccessToken;
}

Expected behavior
An access token is retrieved.

Actual behavior
InMemoryPartitionedAppTokenCacheAccessor.SaveIdToken throws a NotSupportedException.

Additional context / logs / screenshots
The regression occurred when updating the Microsoft.Identity.Client NuGet from 4.35.1 to 4.37.0. It seems that version 4.37.0 also introduced a partitioned user cache (#2881). The source code has a comment explaining that InMemoryPartitionedAppTokenCacheAccessor.SaveIdToken throws a NotSupportedException because for the app token cache, there are no ID tokens in a client credential flow:

/// <summary>
/// This method is not supported for the app token cache because
/// there are no ID tokens in a client credential flow.
/// </summary>
public void SaveIdToken(MsalIdTokenCacheItem item)
{
throw new NotSupportedException();
}

Partial stack trace:

System.NotSupportedException: Specified method is not supported.
Stack Trace:
   at Microsoft.Identity.Client.PlatformsCommon.Shared.InMemoryPartitionedAppTokenCacheAccessor.SaveIdToken(MsalIdTokenCacheItem item)
   at Microsoft.Identity.Client.TokenCache.<Microsoft-Identity-Client-ITokenCacheInternal-SaveTokenResponseAsync>d__56.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.Identity.Client.TokenCache.<Microsoft-Identity-Client-ITokenCacheInternal-SaveTokenResponseAsync>d__56.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Cache.CacheSessionManager.<SaveTokenResponseAsync>d__10.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.<CacheTokenResponseAndCreateAuthenticationResultAsync>d__16.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.ClientCredentialRequest.<FetchNewAccessTokenAsync>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.ClientCredentialRequest.<ExecuteAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.<RunAsync>d__12.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.ApiConfig.Executors.ConfidentialClientExecutor.<ExecuteAsync>d__3.MoveNext()
@pmaytak pmaytak added bug P1 regression Behavior that worked in a previous release that no longer works in a newer release labels Nov 3, 2021
@pmaytak pmaytak added this to Triage in MSAL.NET (legacy) via automation Nov 3, 2021
@pmaytak
Copy link
Contributor

pmaytak commented Nov 3, 2021

@adarce In the auth flow section you mentioned Web app / Auth code flow that has the issue but the repro and error is for AcquireTokenForClient. To clarify - this only happens when you call AcquireTokenForClient? Is that repro the exact code you call in your app or do you reuse the ConfidentialClientApplication instance to call AcquireByAuthorizationCode also?

@adarce
Copy link
Author

adarce commented Nov 3, 2021

In the auth flow section you mentioned Web app / Auth code flow that has the issue but the repro and error is for AcquireTokenForClient. To clarify - this only happens when you call AcquireTokenForClient? Is that repro the exact code you call in your app or do you reuse the ConfidentialClientApplication instance to call AcquireByAuthorizationCode also?

Sorry, this only happens when we call AcquireTokenForClient. I'll update the section. This is the exact code we use.

@trwalke
Copy link
Member

trwalke commented Nov 4, 2021

Hi @adarce, Can you please provide us with more info here so that we can investigate further? We are unable to repro and identify the issue right now.

Thanks

@trwalke trwalke moved this from Triage to In Progress in MSAL.NET (legacy) Nov 4, 2021
@adarce
Copy link
Author

adarce commented Nov 4, 2021

After debugging a bit further, I think we've found the problem. Our failing test acquires a token from a mock AAD server that may not have been implemented correctly. The mock AAD server's response contains an IdToken property, and so when the new version of MSAL sees this in the response and tries to save it to the cache, it throws an exception (SaveIdToken is not supported for the app token cache because there are no ID tokens in a client credential flow.)

I'll work with my team to fix our mock AAD server as per these documentation links:

@adarce adarce closed this as completed Nov 4, 2021
MSAL.NET (legacy) automation moved this from In Progress to Fixed Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug P1 regression Behavior that worked in a previous release that no longer works in a newer release requires more info
Projects
No open projects
Development

No branches or pull requests

3 participants