JSkimming / AspNet.Identity.EntityFramework.Multitenant

Multi-tenant support for ASP.NET Identity using Entity Framework
Apache License 2.0
132 stars 61 forks source link

MultitenantUserStore.FindByEmailAsync doesn't take tenantId into consideration #5

Closed lukasz-karolewski closed 9 years ago

lukasz-karolewski commented 10 years ago

TenantId is not taken into consideration when using with RequireUniqueEmail = true on UserValidator

        manager.UserValidator = new UserValidator<ApplicationUser>(manager)
        {
            AllowOnlyAlphanumericUserNames = false,
            RequireUniqueEmail = false
        };

Root cause is that IUserEmailStore doesn't mark FindByEmailAsync as virtual and since MultitenantUserStore extends default UserStore which doesn't know anything about tenantId it cannot work properly.

Solution is to copy&paste UserStore<TUser, TRole, TKey, TUserLogin, TUserRole, TUserClaim> into and modify IUserEmailStore.FindByEmailAsync implementation...

or make MS mark FindByEmailAsync as virtual

JSkimming commented 10 years ago

Hi @kether667 yes that is rather strange, it's not a situation I've tried.

I'm not in a position to test this but maybe does overriding the method with new work, e.g.

public new Task<TUser> FindByEmailAsync(string email)
{
    return GetUserAggregateAsync(u => u.Email.ToUpper() == email.ToUpper() && u.TenantId.Equals(TenantId));
}

If it does, let me know and I'll look to update MultitenantUserStore, though if you send a PR you're likely to get a faster turn around.

lukasz-karolewski commented 10 years ago

unfortunately this won't help UserManager is using Interface and new keyword will still access base class implementation... function would have to be marked as virtual.

I've filed a bug in asp.net identity https://aspnetidentity.codeplex.com/workitem/2368

please vote up

Bartmax commented 9 years ago

did you found a workaround instead of installing alpha release ?

lukasz-karolewski commented 9 years ago

your own implementation of IUserStore ... i'm waiting for 2.2 release myself : P as it's not critical in my case

Bartmax commented 9 years ago

thank you @kether667 I guess I will be using alpha ... not so happy to put that on release but ... i need that :(

rikkreeftenberg commented 9 years ago

Hi James,

I was wondering if you could add the MultitenantUserStore.FindByEmailAsync:

    public override Task<TUser> FindByEmailAsync(string email)
    {
        if (string.IsNullOrWhiteSpace(email))
            throw new ArgumentNullException("email");

        ThrowIfInvalid();

        return Users.Where(u => u.Email == email && u.TenantId.Equals(TenantId)).FirstOrDefaultAsync();
    }

Since the Microsoft ASP.NET Identity Entityframeword version 2.2 is released, you can push the new Multitenant Library to Nuget. I'm using the library in a production system and I'm very glad with it!

So thank you for your effort.

Regards,

Rik

JSkimming commented 9 years ago

Release 1.2.0 has just been pushed to NuGet