Should IdentityUser live in Core?

[atuzovic]

The way projects in this repository are coupled now, Microsoft.AspNetCore.Identity.EntityFrameworkCore package has dependency on Microsoft.AspNetCore.Identity package.

IdentityDbContext class is defined in Microsoft.AspNetCore.Identity.EntityFrameworkCore package.

If I want to create my own clean DAL project and only have ApplicationDbContext class + POCO entities there, and I decide that my ApplicationDbContext class will inherit from IdentityDbContext in order to be able to plug in to middleware later on - logically, I have to add reference to Microsoft.AspNetCore.Identity.EntityFrameworkCore package to DAL project, which results in Microsoft.AspNetCore.Identity being pulled in to DAL project as well, which gives me dependency on other packages that are related to cookies, HTTP, ...

What kind of architecture and dependency chain is this? I have web-related stuff in my DAL project which makes absolutely no sense.

I understand that I can copy+paste stuff from projects in this repository and arrange it the way I like it since it is all open source now, but is that really way to go?

Can you please shed some light on why is all this built this way? I could be just blind to something obvious.

Thank you!

[brockallen]

Perhaps you should keep your business related entities separate from the identity management related entities.

[atuzovic]

@brockallen Well, even though that makes some sense when observed from DDD perspective, it still does not explain this dependency chain.

As a matter of fact, I've taken a close look into your MembershipReboot projects structure, and that's a great example of clear separation of concerns. I don't see your MembershipReboot.Ef project referencing any of the web assemblies, which is what I was expecting to see here as well.

Thanks for the suggestion though, appreciated. :)

[brockallen]

Ok, I see your point about the web dependencies. I agree with that sentiment.

[divega]

Not sure it is feasible at this point, but one way we could achieve that is by moving all interfaces and abstractions to a separate abstractions package, then have store packages depend on that. Not sure what the goal is though: if you want a DAL depend on IdentityDbContext you kind of carry a lot of baggage with that anyway.

[divega]

E.g. ASP.NET Core Identity is Web specific.

[brockallen]

E.g. ASP.NET Core Identity is Web specific.

This is a superficial design limitation that you've boxed yourselves into. It was brought up as a complaint early during this v3 rewrite (here's one open issue that mentions it: #651). The prior versions of ASP.NET Identity were not web-specific, and to be blunt all ASP.NET Identity is is logic around DB queries, password validation, and brute force protection. The cookie authentication middleware is not part of ASP.NET identity, and it's disingenuous and confusing to tell people that that it is. The comment in #651 is a good one that speaks about OAuth2 -- that's something that has nothing to do with cookies, yet would need end-user authentication. That's a valid use case that's not "Web specific".

[HaoK]

We discussed this when we were deciding whether Identity should go in the AspNet namespace, and the outcome was that we intentionally decided against specifically supporting non web scenarios.

[brockallen]

Hao, can you go into the rationale for it? It would be nice to see this explained and documented (and thus you can just point people to it). After all, this is all out in the open, right? cringe

[atuzovic]

@divega Well yes, ideal solution would be to move out required abstractions to new package, and then to have Microsoft.AspNetCore.Identity and Microsoft.AspNetCore.Identity.EntityFrameworkCore both depend on that package. That would allow to remove dependency from Microsoft.AspNetCore.Identity.EntityFrameworkCore on Microsoft.AspNetCore.Identity and therefore remove need to include dependencies on web related packages in tiers that are responsible for data access.

When it comes to patterns, it's never about them being a must, but about them being a good practice. If I was about to write my own store implementation, I would have to include Microsoft.AspNetCore.Identity in my project and pull web-related packages. I really don't think that's a good example on how to do stuff.

Thank you for considering this at least, and thanks @brockallen for giving your voice on this as well.

Keep up the good work! :)

[cwe1ss]

I also think that depending on web assemblies/cookies is a huge disadvantage of this library. This could have been avoided easily and would have made this project massively more useful.

[mariuszkochanowski]

You could also consider decoupling DbContext from default identity domain model classes defined in Microsoft.AspNetCore.Identity.EntityFrameworkCore that are used and exposed by implementations of UserStore and RoleStore classes #920.

[giggio]

This is specially painful if you want to be able to replace Microsoft.AspNetCore.Identity.EntityFrameworkCore for some other implementation. You simply can't do it.
The UserManager<IdentityUser> class is directly dependent of IdentityUser. I can't use UserManager<OtherUser> because that will not work with the current implementation, as it is tied to the IdentityUser. IdentityUser cannot be swapped out.
Ideally, I should be able to simple swap the Entity Framework implementation, at runtime, for some other one. That just can't be done. At the current moment, the only solution is to fork Microsoft.AspNetCore.Identity.EntityFrameworkCore, which is what I am going to do now.
But that is also a pain, as I will have to keep my library in sync with your, which I shouldn't need to do.
At least this library is small. But still.

[ardalis]

Agreed this lack of separation of concerns and tight coupling to IdentityUser is causing real problems when trying to work with this library. Should strongly consider addressing in a future release. To easily detect these problems, write samples that use more than one persistence technology. Write a sample that, using DI, can swap between using EF Core and Dapper for the implementation of its Identity classes. When you can write that sample without a great deal of pain, you're on the right track.

[HaoK]

IdentityUser lives in EntityFrameworkCore, you don't have to derive from IdentityUser, you can implement your own TUser so I'm not sure I understand the issue, IdentityCore isn't tied to the EF implementation at all.

[HaoK]

Plugging in your own non EF identity store should look basically like this:

    services.AddIdentity<YourUser, YourRole>().AddUserStore<YourUserStore>.AddRoleStore<YourRoleStore>();

No EF involved.

[ardalis]

I want to optionally use EF for my persistence of my identity type. Here's a good example of what I'm looking to achieve: http://timschreiber.com/2015/01/14/persistence-ignorant-asp-net-identity-with-patterns-part-1/

I want to use POCOs for my identity types (user, role) and decouple their use from the decision of how I want to persist them (that decision likely will be made in another project from my web project). Does that make sense?

[ardalis]

Part of the pain here is that the default templates make heavy use of IdentityUser in the AccountController and ManageController types. If these used a simple POCO class and some interfaces for working with such a class, it would make it much easier to extract things from the web project.

[HaoK]

Fair enough, I believe the reason the IdentityUser/associated pocos don't live in core was due to a few navigation properties they have. @divega might recall more details about that debate as well.

Would things be better for your optional EF scenario if the simple IdentityUser POCO lived in core for everyone to use if they desire?

cc @ajcvickers @blowdart if we want to move our pocos around, this might be a good thing to do in 2.0...

[brockallen]

You'd have to put base classes in the core repo and then derived classes in the EF repo, I'd think. The derived classes would add the EF specific goo.

[HaoK]

Yeah now I remember what the point of contention was, the placeholder navigation properties that exist for EF only. That's really the only thing that would get removed for the entities to live on core...

Clearing for triage

Main issue:
#332

Related issues:
#547
#489

[blowdart]

If we're splitting stuff, we ought to split out the MVC specific bits, so folks can use identity from other types of apps, as we've had requests for.

[ardalis]

Any chance just an interface would work for the identityuser? Let me mark my own user class as implementing IIDentityUser (or similar) which defines the necessary properties for AspNetCore.Identity to get the values it requires? Then I wouldn't need to inherit from or directly use a framework type.

[divega]

@HaoK another point of contention is that the design of Identity intentionally does not prescribe how the types look like. E.g. the store interfaces are there to completely isolate the implementation of the managers from how the classes look like. They can be anything that the stores and the application understand. If you include these "POCOs" in the core package, it might mean one of two things:

1. All providers should use them as the base for their implementation: this pushes a new burden on providers, which now need to support this kind of "POCOs" (which are very EF-idiomatic, not just because of the navigation properties)

2. They are there just as a hint that a provider can use them: this can be potentially confusing and personally I don't think it provide a lot of value

Based on this and what @ardalis says his requirement is in #883 (comment), I think a more useful way to think about this is that we only ship the POCOs in the turn-key EF Core implementation of the stores for convenience, but that if an application wants complete freedom and control on the types and that store it uses it can do so by providing its own implementation of the store interfaces.

Now, let's assume that:

• You want to make it easier, i.e. you want to make it possible for an application to do what @ardalis wants without having to write that much code.
• The EF-friendly "POCOs" that we define are really viable base types for a significant subset of all the providers

Then the way I would go about it is to move the "POCOs" into a base Identity store provider package that not only brings the POCOs but also a partial implementation of the store interfaces, in particular an implementation of all the repetitive property access code that is bound to the POCOs. Then it should be very easy to refactor the current EF Core provider to depend on this base provider, and for someone wanting to do the same thing @ardalis wants to do to create a non-EF Core based implementation of the stores that shares a lot of code with the EF Core implementation.

[divega]

Also, while we are at it, it would be nice if we could address what this issue was originally about: move the definitions of the Identity interfaces into a Abstractions package so that customers can write a DAL and providers can write a provider without bringing the whole graph of dependencies of ASP.NET.

[HaoK]

Sure we've been overdue for cleaning up things in Identity...

So something like:

Microsoft.Extensions.Identity.Abstractions (UserManager/SignInManager/interfaces
Microsoft.Extensions.Identity.Stores.Core (IdentityUser/basic stores)
Microsoft.AspNetCore.Identity (SignInManager that works with AspNet)
Microsoft.AspNetCore.Identity.EntityFrameworkCore (EFStore)

[divega]

I believe @ajcvickers was also compiling a list of the different improvements that have come up in this issue. I think template code that is not tied to the "POCOs" is part of it.

FWIW, I would probably pick a name do the base "POCO"-based stores pacakage that conveys the fact that it is an opinionated implementation.

[HaoK]

e6f3ebf

[giggio]

Good job, everyone, as soon as preview 2 is out I will try this out. Thanks!

[BenjaminCharlton]

Hello all! I was happy to see that IdentityUser had been moved out of EntityFramework and into Microsoft.AspNetCore.Identity in version 2.0.0 Preview2-Final. That was very helpful.

But I just upgraded from the preview version to the final release of ASPNETCore 2.1 and it's not there anymore. Where does IdentityUser live now please?

In my AuthorizationController.cs class I'm now getting the compilation error below:

Error CS7069 Reference to type 'IdentityUser<,,,,>' claims it is defined in 'Microsoft.Extensions.Identity.Stores', but it could not be found ....\Controllers\AuthorizationController.cs

[BenjaminCharlton]

The solution to my problem above is:

1. Updating all the references to Nuget packages in each of my projects to the latest versions
2. Replace references to IdentityUser<TKey, TLogin, TRole, TClaim> (now obsolete, it seems) with references to the simpler IdentityUser<TKey> class.