Very complex configuration

[fatalistt]

In Startup.ConfigureServices of Admin and AdminApi projects there are lots of Type parameters (Admin, for example).
It's very difficult to understand, which parameter user should to change if he want to change something.
I suppose to create a builder to split this method. It'll look something like this:

services.AddAdminAspNetIdentityServices(); // With default values like string keys, DbContexts, etc.
services.AddAdminAspNetIdentityServices(admin => {
    admin.ConfigureIdentity(identity => {
        identity.ConfigureKeys(keys => {
            keys.UseUserKey<string>();
            ...
        })
        .ConfigureEntities(entities => {
            entities.UseUser<IdentityUser<string>>();
            ...
        });
    }
    ...
});

In configuration we start from default values, so user can skip some blocks like ConfigureKeys.
I start to making this, but I have some question for now (may be I'll add some more later):
In Skoruba.IdentityServer4.Admin.EntityFramework.Identity.Repositories.IdentityRepository you have TUserKey, TRoleKey, and TKey parameters (src).
TUserKey and TRoleKey are never used except these 2 methods: ConvertUserKeyFromString and ConvertRoleKeyFromString
All other parameters are depend from TKey (TUser : IdentityUser<TKey>, for example).
Later you use UserManager<IdentityUser<TKey>>.Users.AnyAsync(x => x.Id.Equals(_instance of TUserKey_)); here.
If TKey and TUserKey will be a different types, this code will fall, isn't it?

[skoruba]

You are right, this configuration is maybe too complex.
Yes, TKey and TUserKey must be same type. I like your approach, we can rewrite this, if you can send some PR, it would be great.

[fatalistt]

Yes, I will make a PR later when i finished it.
If TKey and TUserKey (TRoleKey) must be the same type, why do we need TUserKey (TRoleKey) ever?

[skoruba]

This is great question, I would prefer separate keys for User and Role, maybe we can remove TKey at all and keep only TUserKey and TRoleKey. What do you think?

[fatalistt]

Yes, it's better to remove TKey, but what about the keys in TUserClaim, TUserRole, TUserLogin, TRoleClaim and TUserToken? Which keys they should to use? I didn't work with ASP.Net Identity so deep before, so I know nothing about these keys.

[skoruba]

These objects are connected with TUserKey and TRoleKey. For example: TUserClaim is object which uses TUserKey. You are not able to specify primary key for TUserClaim but you can specify primary key for User which is used in TUserClaim. Does it make sense for you?

[fatalistt]

Yes, I think, it does. But after looking to sources of Identity there are some questions.
IdentityUserClaim<TUserKey>, IdentityUserLogin<TUserKey>, IdentityRoleClaim<TRoleKey> and IdentityUserToken<TUserKey> are contains only one of [TUserKey UserId, TRoleKey RoleId].
But IdentityUserClaim<TKey> contains both TKey RoleId and TKey UserId, which means that TUserKey and TRoleKey are the same type TKey, so we're not able to split TKey to two different types. Am i right?

[skoruba]

Can you send me which part do you mean?

[fatalistt]

Yes, of course.
IdentityUserRole from AspNetCore.
Constraint for TUserRole in your code.

[skoruba]

You are right, thanks for this point. We should use only one key - TKey, becuase of this limitation.

[b0]

Hi @skoruba and @fatalistt,

I was looking at this issue and associated PR and me too find that configuration is a bit complex and definitively there is TRoleKey and TUserKey type params too much.

I find PR maybe a bit too big so maybe it's possible to split it in two, one for TRoleKey and TUserKey removal and another for simplified configuration?

What do you think?

Thank you

[skoruba]

Hi @b0 I definitely agree with you, I am happy for this PR, but it is too complex - splitting would be perfect.