grandchamp / Identity.Dapper

Identity package that uses Dapper instead EntityFramework for use with .NET Core
MIT License
268 stars 62 forks source link

Assign Claims #71

Open bagzli opened 6 years ago

bagzli commented 6 years ago

Hi,

I'm trying to figure out how I would add claims to your project. For a simple example, I would like to attach MyCustomField that sits in the Users table. How would accomplish that? I was looking at ServiceCollectionExtension and I see you have some overrides that let you pass in TRoleClaim, but I'm not sure if this is what I'm looking for. And to be honest, I'm not sure how I would pass this along at startup.

grandchamp commented 6 years ago

MyCustomField is a property on User class?

bagzli commented 6 years ago

Yes in this instance.

grandchamp commented 6 years ago

The sample already do that.

Check https://github.com/grandchamp/Identity.Dapper/blob/master/samples/Identity.Dapper.Samples.Web/Entities/CustomUser.cs, there's a Address property.

And you'll have to do services.AddIdentity<CustomUser, CustomRole>.

bagzli commented 6 years ago

I don't think I fully understand what you mean or I may have just poorly explained what I am trying to do. Here is a better example.

In my controller, I would like to retrieve that Address field you mentioned above. I would like to do it by typing the following: HttpContext.User.Identity.GetAddress();. If you open the code, you will notice that by default, Name is already in there as part of it, but nothing else. Any other field we have to add ourselves.

When I use to work with EF6, I would do that in the following way. I would first create IIdentity extensions such as this:

    public static class IdentityExtensions
    {
        public static string GetAddress(this IIdentity identity)
        {
            return ((ClaimsIdentity) identity).FindFirst("Address").Value;
        }
    }

Now the above method only gets the value out of claims. The problem is putting the value as part of the claim. EF6 would let you do it like this:

public async Task<ClaimsIdentity> GenerateUserIdentityAsync(UserManager<ApplicationUser> manager)
{

  var userIdentity = await manager.CreateIdentityAsync(this, DefaultAuthenticationTypes.ApplicationCookie);

  userIdentity.AddClaim(new Claim("CompanyId", (this.CompanyId + "" ?? "0")));

  return userIdentity;
}}

This is inside the CustomUser.cs file and I believe the internal workings of the EF6 knew to call that method and assign claims. But with dapper and your project, I am not sure how I would go about assigning the claim value.

I've looked at what you wrote above, but I can't seem to find anywhere where you Assign the Address value to the claims.

grandchamp commented 6 years ago

Oh, now i understand what you meant. If you use userManager.AddClaimAsync(), when you login the Identity don't automatically generates the claim for your user?

bagzli commented 6 years ago

Are you saying that upon login I should retrieve the user from the database and via the UserManager assign claims at that point?

grandchamp commented 6 years ago

Yep. Then you'll logout and login, check if Identity auto-fills your custom claim.

bagzli commented 6 years ago

I think I misunderstood you there then. If I write the code to happen on login, that will happen each time they login, there won't be any auto-fill going on, my code will be adding it each time.

grandchamp commented 6 years ago

I think so.

What i'm telling you is:

Somewhere your code (maybe on your register page?) you'll call userManager.AddClaimAsync() to add your custom claim to your user.

Then, i think Identity will automatically add your custom claim to your user, then you'll be able to do HttpContext.User.Identity.GetAddress().

bagzli commented 6 years ago

Ok, I'll experiment more this weekend. I'm wondering now what does AddClaimAsync do actually. Does it write to the database? maybe it writes to session? Anywho, I'll investigate and let you know where I'm at. Thanks for the help.

grandchamp commented 6 years ago

It calls userRepository.InsertClaimsAsync that will INSERT the claim on UserClaimTable.

bagzli commented 6 years ago

Ok then yeah this is not it. From my understanding, these claims come from the third party. The claims I am talking about are stored in memory and come from the data in the database. Because, if I have a field on User Table called Address, why would I want to copy address value into claims table, it's not right, I shouldn't duplicate my data, especially if one is bound to change eventually.

grandchamp commented 6 years ago

I understand your point.

Maybe this helps: http://benfoster.io/blog/customising-claims-transformation-in-aspnet-core-identity

bagzli commented 6 years ago

I don't have time to go through this thoroughly and write some tests to make sure, but the article seems to be on the money at the first glance, I wish I found it sooner. I'll write back here with my findings. If I end up figuring it out and its something that can benefit the project, I'll create a pull request for you and you can decide if you want it.

grandchamp commented 6 years ago

Well, maybe it's out of scope of this project, but surely it'll help on your personal needs. I was thinking that it was something related to any implementation here.

bagzli commented 6 years ago

It's up to you, many of the projects I have worked on ended up needing this functionality as it makes things much simpler. At the very least I'll come back and let you know how I resolved this.

bagzli commented 6 years ago

I think I may have found another bug in the code.

If I call await _userManager.AddClaimAsync(userInfo, new Claim("GivenName", "My Test 123")); nothing happens that I can tell. The value is not found in my claims on next request and the database also has nothing in the table.

Calling that function is supposed to add it to the database right? Or is it only suppose to add it to the cache? In either case, I can't find it.

The other thing is, the article you have linked me basically does what I want it to, but looking at your implementation, I can't find anywhere that you make use of CreateAsync method.

The only thing I have found is inside DapperUserStore you have a CreateAsync method, but all that method does is insert a user into the database. It never actually calls CreateAsync from UserClaimsPrincipalFactory.

Maybe I'm reading it wrong, but do you call CreateAsync from UserClaimsPrincipalFactory anywhere at any point? I can't find it being called directly or indirectly through any of the code.

grandchamp commented 6 years ago

For the first, it should been added to database. Probably a bug or misconfiguration.

For the second, i don't call directly, nor i need it. Identity does this by itself.

bagzli commented 6 years ago

The login and registration seem to work fine so I think the configuration is fine. It must be a bug, but I can't seem to figure out if its a bug in Identity or Identity.Dapper library.