[MCC-732241] Fix caching with authentication when using utility extension method

[lschreck-mdsol]

Background

Although MAuth provides a variety of extensions to use with external applications (ASP.NET Core middleware, ASP.NET WebAPI, OWIN etc.) there are cases when authentication is required in a way that's not supported by the library. For those cases the library provides utility extension methods to call externally (like the UtilityExtensions.Authenticate() method).

The Issue

Under the hood the library is using an internal class called MAuthAuthenticator which sets up its own memory cache. As long as this class is used, caching will work. The utility method is using this class.
The issue is that the above mentioned utility method has to instantiate this class for every call (as it's a static method therefore its options will be passed with every call), therefore a new memory cache will be also instantiated for every call effectively disabling the caching.

The Solution

For the MAuthAuthenticator instances, we are providing an optional constructor argument to set the cache externally. This will enable the utility extension to reuse its own cache therefore it won't recreate again and agin.

@mdsol/libraries_dotnet @mdsol/architecture-enablement Please review and merge - thanks!

[h-kurniawan]

Why don't we declare this as

private static readonly IMemoryCache _cache = new MemoryCache(new MemoryCacheOptions());

here instead of in UtilityExtensions?
Having a variable in an extension method class feels a bit weird.

[lschreck-mdsol]

Ah, that looks like a better solution! Thanks, let me update the code.

[h-kurniawan]

Sorry to unresolve this, can we have a test for this?

[lschreck-mdsol]

I reverted back to my original solution because having the static cache in the MauthAuthenticator will screw up the unit tests.

[h-kurniawan]

Should we take a look at pika pika, find out which apps/services use 5.1.0 and urge the team to upgrade to 5.1.1?
If the app/service is a busy one, the memory usage can go high quite a bit which may cause the ECS instance to crash.

[lschreck-mdsol]

@hkurniawan-mdsol This only applies for cases where the relevant framework extension does not exist (i.e. where the utility extension must be used).

So far I only know about PB using this utility so I don't think this is an urgent patch.

[h-kurniawan]

@lschreck-mdsol

So far I only know about PB using this utility so I don't think this is an urgent patch.

Okay cool. If we're sure that only PB is affected then we're fine then.

[jfeltesse-mdsol]

Just a friendly reminder: this repository is public.

[h-kurniawan]

Just a friendly reminder: this repository is public.

Okay maybe we do need to urge the public to update then.

[jfeltesse-mdsol]

@hkurniawan-mdsol

[prajon84]

@lschreck-mdsol : I see version is bumped, but there is no update on changelog. Missed that ?

Also, this unit test is failing too.
AuthenticateRequest_AfterNumberOfAttempts_WillThrowExceptionWithRequestFailure(policy: Agressive) [FAIL]

Found one unit test is failing.

[lschreck-mdsol]

@prajon84 Thanks, you are right I forgot to update the changelog (but now it's done: 33f59f6).

I am checking the unit test now.

[h-kurniawan]

Can we have a preview version released for testing?
@psangha-mdsol is working on both load testing and memory consumption story right now, it'd be good if he can prove that the changes work.