Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type consistraints in IdentityDbContext as an inerface #12242

Closed
json1982 opened this issue Jul 16, 2019 · 14 comments
Closed

Type consistraints in IdentityDbContext as an inerface #12242

json1982 opened this issue Jul 16, 2019 · 14 comments
Labels
area-identity Includes: Identity and providers
Milestone

Comments

@json1982
Copy link

json1982 commented Jul 16, 2019

I am looking to implement my own base class for the identity tables. The issue is the IdentityDbContext<> type constraints only allow for concrete types. This requires any custom class to extend those base classes. For example when using a custom user you would need to define your user class like below

public ApplicationUser : IdentityUser{}

Since you can only extend one class, it makes my goal impossible.

I would suggest using interfaces for your type constraints to decouple the base identity tables from the db context allowing me to implement the interface and extend my own base class. Instead of the above example I would define my custom user class like below

public ApplicationUser : CustomBasClass, IIdentityUser{}

Now the base table is decoupled and I can extend my own base class if needed.

You can still directly extend the identity base classes, but this way you can do both

@mkArtakMSFT mkArtakMSFT added the area-identity Includes: Identity and providers label Jul 16, 2019
@blowdart
Copy link
Contributor

@ajcvickers Is this even possible with EF?

@ajcvickers
Copy link
Contributor

@blowdart Kind of, but it's not as clean as it could be and it has limitations. But regardless, this would be a massive breaking change to the Identity model, which I don't think we can justify at this time.

@blowdart
Copy link
Contributor

Alight, backlog and we'll think about it for v5

@blowdart blowdart added this to the Backlog milestone Jul 17, 2019
@tb-mtg
Copy link

tb-mtg commented Jul 30, 2019

Could the following interfaces be introduced and referenced by the appropriate classes?

User related interfaces (would be implemented by IdentityUser, IdentityUserClaim, IdentityUserLogin, IdentityUserToken):

  public interface IIdentityUser<TKey> where TKey : IEquatable<TKey> {
    int AccessFailedCount { get; set; }
    string ConcurrencyStamp { get; set; }
    bool LockoutEnabled { get; set; }
    DateTimeOffset? LockoutEnd { get; set; }
    string PasswordHash { get; set; }
    string NormalizedEmail { get; set; }
    string NormalizedUserName { get; set; }
    string SecurityStamp { get; set; }
    [PersonalData] TKey Id { get; set; }
    [PersonalData] bool EmailConfirmed { get; set; }
    [PersonalData] bool PhoneNumberConfirmed { get; set; }
    [PersonalData] bool TwoFactorEnabled { get; set; }
    [ProtectedPersonalData] string Email { get; set; }
    [ProtectedPersonalData] string PhoneNumber { get; set; }
    [ProtectedPersonalData] string UserName { get; set; }
  }

  public interface IIdentityUserClaim<TKey> where TKey : IEquatable<TKey> {
    int Id { get; set; }
    TKey UserId { get; set; }
    string ClaimType { get; set; }
    string ClaimValue { get; set; }
    void InitializeFromClaim(Claim other);
    Claim ToClaim();
  }

 public interface IIdentityUserLogin<TKey> where TKey : IEquatable<TKey> {
    string LoginProvider { get; set; }
    string ProviderKey { get; set; }
    string ProviderDisplayName { get; set; }
    TKey UserId { get; set; }
  }

  public interface IIdentityUserToken<TKey> where TKey : IEquatable<TKey> {
    TKey UserId { get; set; }
    string LoginProvider { get; set; }
    string Name { get; set; }
    [ProtectedPersonalData] string Value { get; set; }
  }

Role related interfaces (would be implemented by IdentityRole, IdentityRoleClaim, IdentityUserRole):

  public interface IIdentityRole<TKey> where TKey : IEquatable<TKey> {
    TKey Id { get; set; }
    string ConcurrencyStamp { get; set; }
    string Name { get; set; }
    string NormalizedName { get; set; }
  }

 public interface IIdentityRoleClaim<TKey> where TKey : IEquatable<TKey> {
    int Id { get; set; }
    TKey RoleId { get; set; }
    string ClaimType { get; set; }
    string ClaimValue { get; set; }
    void InitializeFromClaim(Claim other);
    Claim ToClaim();
  }

 public interface IIdentityUserRole<TKey> where TKey : IEquatable<TKey> {
    TKey UserId { get; set; }
    TKey RoleId { get; set; }
  }

@tb-mtg
Copy link

tb-mtg commented Jul 30, 2019

This could then be expanded on by adding interfaces with Navigation Properties as shown here.

User related "nav" interfaces:

 public interface IIdentityUserNav<TKey> : IIdentityUser<TKey> where TKey : IEquatable<TKey> {
    ICollection<IIdentityUserClaimNav<TKey>> UserClaims { get; set; }
    ICollection<IIdentityUserLoginNav<TKey>> UserLogins { get; set; }
    ICollection<IIdentityUserRoleNav<TKey>> UserRoles { get; set; }
    ICollection<IIdentityUserTokenNav<TKey>> UserTokens { get; set; }
  }

 public interface IIdentityUserClaimNav<TKey> : IIdentityUserClaim<TKey> where TKey : IEquatable<TKey> {
    IIdentityUserNav<TKey> User { get; set; }
  }

  public interface IIdentityUserLoginNav<TKey> : IIdentityUserLogin<TKey> where TKey : IEquatable<TKey> {
    IIdentityUserNav<TKey> User { get; set; }
  }

 public interface IIdentityUserTokenNav<TKey> : IIdentityUserToken<TKey> where TKey : IEquatable<TKey> {
    IIdentityUserNav<TKey> User { get; set; }
  }

Role related "nav" interfaces:

 public interface IIdentityRoleNav<TKey> : IIdentityRole<TKey> where TKey : IEquatable<TKey> {
    ICollection<IIdentityRoleClaimNav<TKey>> RoleClaims { get; set; }
    ICollection<IIdentityUserRoleNav<TKey>> UserRoles { get; set; }
  }

 public interface IIdentityRoleClaimNav<TKey> : IIdentityRoleClaim<TKey> where TKey : IEquatable<TKey> {
    IIdentityRoleNav<TKey> Role { get; set; }
  }

  public interface IIdentityUserRoleNav<TKey> : IIdentityUserRole<TKey> where TKey : IEquatable<TKey> {
    IIdentityUserNav<TKey> User { get; set; }
    IIdentityRoleNav<TKey> Role { get; set; }
  }

Note: I have used Nav as a suffix just to demonstrate, but I'm sure something better could be used.

@tb-mtg
Copy link

tb-mtg commented Jul 30, 2019

This would also then allow for (provided IDbContext was created also which has been requested here):

  public interface IIdentityUserDbContext<TKey> : IDbContext where TKey : IEquatable<TKey> {
    DbSet<IIdentityUserNav<TKey>> AspNetUsers { get; set; }
    DbSet<IIdentityUserClaimNav<TKey>> AspNetUserClaims { get; set; }
    DbSet<IIdentityUserLoginNav<TKey>> AspNetUserLogins { get; set; }
    DbSet<IIdentityUserTokenNav<TKey>> AspNetUserTokens { get; set; }
  }

  public interface IIdentityDbContext<TKey> : IIdentityUserDbContext<TKey>
    where TKey : IEquatable<TKey> {
    DbSet<IIdentityRoleNav<TKey>> AspNetRoles { get; set; }
    DbSet<IIdentityRoleClaimNav<TKey>> AspNetRoleClaims { get; set; }
    DbSet<IIdentityUserRoleNav<TKey>> AspNetUserRoles { get; set; }
  }

@json1982
Copy link
Author

@blowdart After looking through much of the code for Identity and Ef core, I can see the complications. Thanks for looking into this. I hope you can come up with a good solution in V5

@blowdart blowdart reopened this Jul 30, 2019
@blowdart
Copy link
Contributor

Reopening, and leaving open because otherwise we won't ever triage this when we backlog groom

@json1982
Copy link
Author

@blowdart Sure thing. Thanks again for taking a serious look at my request!

@Seeker9889
Copy link

I'm experiencing a similar issue. We already have a base class we need to implement on the class which would be our IdentityUser.

@thepaulpage
Copy link

yes please.

@StevenRasmussen
Copy link

@blowdart , @ajcvickers - Would you consider a contribution from the community on this one? I am running into the same issue as others have stated here. I have forked the aspnetcore repo and made the appropriate modifications to extract an IIdentityUser<T> interface in the PR here: #21097.

All tests are currently passing. I only worked on extracting an interface for IdentityUser at this point. I wanted to see if you would accept a PR from the community on this before proceeding with extracting an interface for IdentityRole. Thoughts?

@HaoK
Copy link
Member

HaoK commented Jun 29, 2020

@blowdart we should chat about his next triage since there's a PR open that i need to make a call on

@blowdart
Copy link
Contributor

blowdart commented Jul 1, 2020

This is all too big a breaking change I'm afraid, sorry @StevenRasmussen

@blowdart blowdart closed this as completed Jul 1, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jul 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-identity Includes: Identity and providers
Projects
None yet
Development

No branches or pull requests

9 participants