fullstackhero / docs

docs for fullstackhero project.
https://fullstackhero.net/
MIT License
58 stars 58 forks source link

Understanding Identity integration #71

Open HybridSolutions opened 2 years ago

HybridSolutions commented 2 years ago

Hi, first of all, great project!

When I found it I was interested on how you integrated .NET Core identity into it but I got confused. Hope you can clarify my doubts. I can see you have in the migration files

modelBuilder.Entity("FSH.WebApi.Infrastructure.Identity.ApplicationRole", b =>
                {
                    b.Property<string>("Id")
                        .HasColumnType("nvarchar(450)");

                    ...

                    b.Property<string>("TenantId")
                        .IsRequired()
                        .HasMaxLength(64)
                        .HasColumnType("nvarchar(64)");

                    b.HasKey("Id");

                    b.HasIndex("NormalizedName", "TenantId")
                        .IsUnique()
                        .HasDatabaseName("RoleNameIndex")
                        .HasFilter("[NormalizedName] IS NOT NULL");

                   ...

but you haven't added any additional properties to ApplicationRole or any other classes

public ApplicationRole(string roleName, string? description = null)
        : base(roleName)
    {
        Description = description;
        NormalizedName = roleName.ToUpperInvariant();
    }

So my question is, did you generate the migration files manually? How is TenantId appearing in the ApplicationRole entity? If yes, why didn't you customize Identity classes to receive these new properties?

My other question is, do all Identity User and Role methods work taking into account the TenantId? I'm asking this because I didn't find any UserStore or RoleStore customizations to deal with this property with CRUD operations.

Thank you and please keep up with this project!

fretje commented 2 years ago

The TenantId has actually not much to do with the Identity integration, but rather with the MultiTenancy integration. Although they do run a bit into each other.

The BaseDbContext is derived from MultiTenantIdentityDbContext, which is a type from the Finbuckle.MultiTenant library. It's that library that takes care of adding the TenantId property to all the Multitenant entities (using a shadow property) and also takes care of setting that property when saving entities and generally guarding against cross-tenant data access (using global query filters). So everything should be taken care of in the background... without it getting in the way of developers too much. The only thing to remember is specifying that your entity is a multitenant entity in its configuration (see how that's done for the brands and products).

We're definitely not manually generating the migration files ;-)

All the Identity entities are configured as multitenant, so yes querying these only works for the current tenant. There are some caveats to be aware of though, which is explained on their docs site.

HybridSolutions commented 2 years ago

The TenantId has actually not much to do with the Identity integration, but rather with the MultiTenancy integration. Although they do run a bit into each other.

The BaseDbContext is derived from MultiTenantIdentityDbContext, which is a type from the Finbuckle.MultiTenant library. It's that library that takes care of adding the TenantId property to all the Multitenant entities (using a shadow property) and also takes care of setting that property when saving entities and generally guarding against cross-tenant data access (using global query filters).

We're definitely not manually generating the migration files ;-)

All the Identity entities are registered as multitenant, so yes querying these only works for the current tenant. There are some caveats to be aware of though, which is explained on their docs site.

Thanks for the quick response! Didn't occur to me that you might have config files for entities. Will check that out and the package you mentioned. It seems great.

fretje commented 2 years ago

@iammukeshm maybe turn this into a docs issue, so this information gets added to the documentation somehow ;-)