CachedCatalogViewModelService risky?

[calbeda]

CachedCatalogViewModelService is registered as a Scoped, so one instance is created per http request.
As the cache is a property of service instance (not static) it will be empty for each request.
This makes all the cache trivially useless... I'm missing something?

[michelcedric]

The service

eShopOnWeb/src/Web/Services/CachedCatalogViewModelService.cs

Line 12 in 2414014

private readonly IMemoryCache _cache;

use IMemoryCache https://learn.microsoft.com/en-us/aspnet/core/performance/caching/memory?view=aspnetcore-8.0

This one is registered in https://github.com/dotnet-architecture/eShopOnWeb/blob/2414014bfa0f4d2021b5bc9061429a98d232f440/src/Web/Program.cs#L66C1-L66C28
That add a Memory cache as in Singleton.

[calbeda]

Thanks for the answer!
You are right, this cache is not trivially useless... is much worse: it is using a service that by specification has http request specific dependencies to get data that will be used in other requests.

This could generate wrong answers and very difficult to detect.
As this project is presented as a model architecture it could be used in many situations where developers probably will not be aware of this details. Is a potentially big security hole if used without much attention.
This "singleton cache in an http scoped service" is a not secure by default. Is a clear security risk.

[ardalis]

How exactly would you implement a memory cache in a monolithic application with a single production instance?

Yes, one could use Redis or a similar out of process cache to resolve some of the concerns you're mentioning, but in this case in-memory is the chosen approach (which is fully supported by dotnet).

What security risk are you implying? Please demonstrate an attack vector and mitigation.

[calbeda]

Ok, probably I have exaggerated the risk... you are using a request scoped service to get data that will be used in others requests. This is ok if you use it carefully, the risk is only if you use request context data in the methods used by the cache.

For example accessing a 3rd part service with current user credentials that maybe will return data only valid for that user, etc.

This request scoped service could in turn use other services to perform some actions and if some developer thinks that everything is request scoped they could for example inject request context information in the service constructor making not clear what methods use this information... real business services could become very complex.

Here they propose using IServiceScopeFactory and creating scoped services inside the singleton using that.
But maybe this could be excessive in many situations... I suppose everything depends of the situation and the size and communication inside the developers team...