tugberkugurlu / AspNet.Identity.RavenDB

Fully asynchronous, new and sweet ASP.NET Identity implementation for RavenDB
MIT License
42 stars 28 forks source link

added implementation of IUserRoleStore to RavenUserStore #2

Closed thoemmi closed 11 years ago

thoemmi commented 11 years ago

Unit tests are included

tugberkugurlu commented 11 years ago

Wow, thanks! I was planning to implement this really soon but you have beaten me to it :)

However, I don't want to store the roles separately. What I want is to store them as claim value. If you could make that change too, I would be more than happy to check this in. After that and a few more tests, I would like to push the stable v1 to nuget.org :)

thoemmi commented 11 years ago

I've added one more commit. Is it like what you had in mind?

tugberkugurlu commented 11 years ago

@thoemmi yes, exactly like that :) merged it and pushed the new version to nuget.org: https://www.nuget.org/packages/AspNet.Identity.RavenDB will add some more tests and sample before releasing the stable v1.

Thanks again for the contribution and feel free to push any more code.

tugberkugurlu commented 11 years ago

Hi @thoemmi,

I decided the remove the IUserRoleStore support from this library. There are several reasons. A role is also a claim. We managed to do that a little by storing roles as claims but ClaimsIdentityFactory currently have the following code inside the CreateAsync method:

if (manager.SupportsUserRole)
{
    IList<string> list = await manager.GetRolesAsync(user.Id);
    foreach (string current in list)
    {
        claimsIdentity.AddClaim(new Claim(this.RoleClaimType, current, "http://www.w3.org/2001/XMLSchema#string"));
    }
}
if (manager.SupportsUserClaim)
{
    claimsIdentity.AddClaims(await manager.GetClaimsAsync(user.Id));
}

So, a role will be represented as a claim on the ClaimsIdentity twice.

thoemmi commented 11 years ago

So the identity framework demands that you either implement IUserRoleStore or IUserClaimStore, but not both? Or should we store roles not as claims but as a separate user property as I coded in my original PR?

tugberkugurlu commented 11 years ago

@thoemmi yep, if you want to support IUserRoleStore and IUserClaimStore, you should store roles separately. Or, replace the default ClaimsIdentityFactory on UserManager<TUser> at the application level.

I really don't want to store roles separately.