7

We're using ASP.NET Identity 2.2 with ASP.NET MVC 5.2, Entity Framework 6.2 and Unity 5.7.

We have a class, ConnectUserManager, that derives from ASP.NET Identity's UserManager. A newly constructed UserStore is passed to UserManager every time.

The lifetime of ConnectUserManager (and thus of UserManager) is per request:

Container.RegisterType<ConnectUserManager>(
    new PerRequestLifetimeManager(),
    new InjectionConstructor(
        Container.Resolve<ConnectDbContext>(),
        Container.Resolve<ITemplateManager>(),
        Settings.MaxFailedAccessAttemptsBeforeLockout,
        Settings.AccountLockoutTimeSpan));

When we need to display the details for a given user, a controller action retrieves the user as such:

public async Task<ActionResult> Details(int id)
{
    var user = await UserManager.FindByIdAsync(id);
    ...
}

where UserManager is an injected property:

[Dependency]
public ConnectUserManager UserManager { get; set; }

The problem is that user appears to come from a cache: modifications in the database don't appear to have any effect on what our application displays.

This code has been in production for a year and we never had any issue with it: the cache seems properly invalidated when our code modifies a user.

We only noticed the problem now because when Identity locks a user out, it updates the user's LockoutEndDateUtc property but seemingly without invalidating the cache, and we get a stale LockoutEndDateUtc value in our display.

What are we doing wrong?

EDIT:

@DotNetMatt linked the following question in a comment:
Entity Framework caching in aspnet Identity.

Unless I'm missing something, the accepted solution (at the time of writing anyway) seems completely unrelated to the original poster's and to my problem.

However, the original poster seems to have found the (a?) solution by himself: "What I did was implemented my own userstore and manually accessed EF and used .AsNoTracking() to avoid the caching."

Is there any way to do this without having to reimplement (or subclass) the user store?

François Beaune
  • 4,270
  • 7
  • 41
  • 65
  • Possible duplicate of [Entity Framework caching in aspnet Identity](https://stackoverflow.com/questions/21041352/entity-framework-caching-in-aspnet-identity) – DotNetMatt Mar 07 '18 at 15:19
  • @DotNetMatt While that link is very interesting (and I must admit I hadn't found it), I think it's a different problem. – François Beaune Mar 07 '18 at 15:32
  • @DotNetMatt Regarding the question you linked to: The accepted answer is really unrelated to the problem the original poster and I are having, but we seem to indeed both have the same problem. It appears the original poster found the right answer by himself (see Edit 2 of his question). – François Beaune Mar 07 '18 at 15:36
  • 1
    after reading your answer below, indeed it is a separate problem. Glad you could find a solution, and that you shared it with the community! – DotNetMatt Mar 08 '18 at 18:38

2 Answers2

3

This is EF fluke. Identity does not have any inbuilt caching. I suspect your lifetimescope for ConnectDbContext is per depedency, not per request.

So what happens is once instance of ConnectDbContext returns you with instance o ApplicationUser. Then another instance of ConnectDbContext does the lockout. But then when you go back to the first instance of ConnectDbContext, it does not know anything about the update that has been done by something else. Hence when you get the same instance second time, it does not go into the database, it returns you already tracked instance of this user.

Way to fix this - make sure your lifetime scopes match for all the moving parts involved: ConnectUserManager,UserStore and ConnectDbContext. So whenever you get objects form DB, it is always the same instance of ConnectDbContext that is providing this for you.

Also the answer you link to - see the very last comment, OP says he has the same scoping issue.

trailmax
  • 34,305
  • 22
  • 140
  • 234
  • On the contrary, a new instance of `ConnectDbContext` is created everytime: `Container.RegisterType();` – François Beaune Mar 07 '18 at 15:54
  • 1
    In other words, it is scoped per dependency. This is your problem. It should be scoped per request. – trailmax Mar 07 '18 at 15:55
  • Oh, really. Now that's interesting! I'm concerned about the unintended consequences of changing the DB context's lifetime though. – François Beaune Mar 07 '18 at 15:57
  • 2
    [DbContext should be scoped per request](https://stackoverflow.com/a/10588594/809357). It will possibly fix your other issues that you don't know about yet. – trailmax Mar 07 '18 at 15:59
  • Unfortunately changing the lifetime of the DbContext to be per-request instead of per injection doesn't appear to solve the problem. – François Beaune Mar 07 '18 at 16:05
  • 1
    Are you sure you are working with the same instance of dbcontxt? It is possible that you have a captive dependency somewhere that prevents you from having a desired lifetime scope – trailmax Mar 07 '18 at 20:05
  • Most of our uses of `DbContext` are very transient, we create them and dispose them immediately (with `using`). We don't need and don't rely on entities being still attached to their context. I guess that's why changing the lifetime of `DbContext` didn't change anything. – François Beaune Mar 08 '18 at 09:21
  • In that case I don't see an easy solution other than rewriting `UserStore` to add `.AsNoTracking()` all over the place. – trailmax Mar 08 '18 at 09:39
  • Or you can have a look on alternative DB storage providers for Identity like [Asp.Net.Identity.Dapper](https://github.com/whisperdancer/AspNet.Identity.Dapper) and get away from EF for Identity purposes. – trailmax Mar 08 '18 at 09:40
  • Your suggestion of rewriting `UserStore` using `AsNoTracking()` seems to indicate to me that there is a fundamental limitation in Identity's EF-based user store in that it does *not* support external modifications to the database, in which case the issue really has nothing to do with `DbContext`. Would you consider that statement correct? – François Beaune Mar 08 '18 at 09:59
  • Nope, I would apply that statement to EF rather than to Identity. This is all EF issue, not Identity. – trailmax Mar 08 '18 at 10:24
  • Out of interest, can you try this suggestion: https://stackoverflow.com/a/40615276/809357 I would not recommend using it all the time, as this will cause issues with using EF, but worth a test – trailmax Mar 08 '18 at 10:26
  • Sure, the problem is general in the sense that it's EF doing the caching, but Identity's `UserStore` not using `AsNoTracking()` could be considered a limitation of Identity ("Identity's user store does not force-disable caching"). Anyway I get your point. FWIW disabling `AutoDetectChangesEnabled` does not fix the problem either. – François Beaune Mar 08 '18 at 10:59
  • I think you may be right when you say "It is possible that you have a captive dependency somewhere that prevents you from having a desired lifetime scope." I don't see how entities can be cached when a new `DbContext` is created at every request (or at every dependency injection). – François Beaune Mar 08 '18 at 11:03
2

I found my problem. @trailmax was correct (in comments to his own answer) that I had captive dependencies lying around.

The bug was actually in the snippet of code from my question that configures injection of ConnectUserManager dependencies:

Container.RegisterType<ConnectUserManager>(
    new PerRequestLifetimeManager(),
    new InjectionConstructor(
        Container.Resolve<ConnectDbContext>(),
        Container.Resolve<ITemplateManager>(),
        Settings.MaxFailedAccessAttemptsBeforeLockout,
        Settings.AccountLockoutTimeSpan));

This resolves ConnectDbContext dependencies once, here and now, unlike the following version:

Container.RegisterType<ConnectUserManager>(
    new PerRequestLifetimeManager(),
    new InjectionFactory(
        container => new ConnectUserManager(
            container.Resolve<ConnectDbContext>(),
            container.Resolve<ITemplateManager>(),
            Settings.MaxFailedAccessAttemptsBeforeLockout,
            Settings.AccountLockoutTimeSpan)));

which correctly resolves dependencies every time a new ConnectUserManager is constructed.

François Beaune
  • 4,270
  • 7
  • 41
  • 65