[Feature Request] In-memory shared cache cache in MSAL (like ADAL), with good performance

[bgavrilMS]

Many customers have problems migrating from ADAL to MSAL because of the token caching. The cache is no longer static / shared.

Our current solutions to this problem are not great because:

• Microsoft.Identity.Web has many dependencies, making it difficult to adopt due to diamond dependency hell.
• implemeting cache serialization using the callbacks is easy to get wrong and isn't even the fully documented anymore (Microsoft.Identity.Web being the recommended solution).
• in-memory caching today relies on JSON serialization, which is a bad solution from a perf perspective. As @pmaytak documented here - the json serialization operation is about 1000x slower than using in-memory representations only. This is not noticeable for a small number of tokens per tenant, but we cannot guarantee this setup.

Proposal:

Add a feature to allow app developers some control over how tokens are stored in memory, directly from MSAL. This can be added to both PCA and CCA. Should not be exposed on mobile platforms and UWP.

Solution 1

appBuilder.WithStaticMemoryCache() // this will make the internal storage static / shared and will achieve ADAL parity

This simply involves adding static version of our Accessor objects, which are already thread safe.

Solution 2 (preferred)

A more general approach, allowing us to move to use Wilson's Event based LRU cache which can add eviction policies.

appBuilder.WithMemoryCache(MemoryCacheOptions)

MemoryCacheOptions // not all options need to be implemented at once
+ MaxNumberOfItems: int // P2
+ Expiration: DateTime // P2
+ Static : bool // P1

Solution 3

Full extensibility. We would still need to provide a few implementations.

appBuilder.WithMemoryCache(ITokenCacheAccessor) where ITokenCacheAccessor is our (currently internal) interface https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/cfa8c9193e0797eeca1927729b980acd1dd18b06/src/client/Microsoft.Identity.Client/Cache/ITokenCacheAccessor.cs

[bgavrilMS]

CC @jmprieur @henrik-me @pmaytak

[jmprieur]

I would recommend:

appBuilder.WithSharedDefaultCache(bool=false) 

This is:

• to avoid confusing customers with the existing in memory token cache. Maybe the fact that we used that term (in memory) in the current implementations for a serialization which is outside the internal MSAL memory is unfortunate, but it's there. We've to do with it. With this github issue, we are really talking here of the internal MSAL token cache (default cache), which we want to be shared between instances of confidential client applications. Specifying this in the builder seems to make sense to me?
• to express that there is not even serialization (and therefore, this can be optimal in MSAL internal memory)
• Apply to both and app and user token cache

If customers use this option and then if they try to add a custom cache (if the serialization events are subscribed to), MSAL.NET should throw with a meaningful exception, as the default cache is incompatible with a serialization.

Later we could decide to:

• change the default to true (breaking change) unless customers add a serializer.
• Add more options (like the solution 2 above), but again that's not serialization. It's about the internal memory of MSAL instance.

[bgavrilMS]

I agree with the some of the wording suggestions, but the current proposal:

• leads the develop to believe that if they use WithSharedDefaultCache(false) they will disable in-memory caching. This is not possible.
• is confusing if app developer wants to add customization of the non-static in-memory cache:

// looks ok:
appBuilder.WithSharedDefaultCache(enabled: true, maxSize: 1000, ttl: TimeSpan.FromHours(36))

// a configurable non-static cache is confusing:
appBuilder.WithSharedDefaultCache(enabled: false, maxSize: 1000, ttl: TimeSpan.FromHours(36))

Perhaps .WithDefaultCache(shared: bool) is better? Which would then evolve to.WithDefaultCache(shared: bool, maxSize: int, ttl: TimeSpan) .