20

I've created a custom IUserStore<TUser,int> for my application. I've implemented the interfaces I need,

   IUserStore<TUser, int>,
   IUserRoleStore<TUser, int>,
   IUserLockoutStore<TUser, int>,
   IUserPasswordStore<TUser, int>

but when I call

var result = await SignInManager.PasswordSignInAsync(model.UserName, model.Password, model.RememberMe, shouldLockout: false);

I get an exception saying

Store does not implement IUserTwoFactorStore<TUser>.

I'm not using two factor authentication anywhere in my application. Why does it expect me to implement that interface? Is it required that I implement all of these interfaces, even if I don't actually use them?

Stephen Collins
  • 3,523
  • 8
  • 40
  • 61

3 Answers3

16

Actually the IUserTwoFactorStore interface is really simple, so far my implementation (I don't use two factor auth either) is this:

 ....
 public Task<bool> GetTwoFactorEnabledAsync(User user)
 {
     return Task.FromResult(false);
 }

 public Task SetTwoFactorEnabledAsync(User user, bool enabled)
 {
     throw new NotImplementedException();
 }

It works, although I just did it couple minutes ago and didn't test whole app thoroughly.

Ignas Vyšnia
  • 2,079
  • 1
  • 16
  • 16
  • 9
    Yea, that will work, but it's pretty nasty. It kills the liskov substitution principle. I think it's safe to say that not checking for an IUserTwoFactorStore implementation is a design flaw in the Manager class of Identity. – Leon Cullens Dec 04 '14 at 15:16
15

I had the same problem. For the moment, as the SignInManager.SignInOrTwoFactor method blindly checks for the GetTwoFactorAuthentication it throws an exception when the UserStore doesn't implement the IUserTwoFactorStore.

I believe Microsoft intended that the SignInManager PasswordSignInAsync method must be overriden in a custom SignInManager class. Unfortunately I couldn't find any documentation or samples pointing so.

Here is the SignInManager wrapper class I implemented to solve this issue:

public class EnhancedSignInManager<TUser, TKey> : SignInManager<TUser, TKey>
    where TUser : class, IUser<TKey>
    where TKey : IEquatable<TKey>
{
    public EnhancedSignInManager(
        UserManager<TUser, TKey> userManager, 
        IAuthenticationManager authenticationManager)
        : base(userManager, authenticationManager)
    {
    }

    public override async Task SignInAsync(
        TUser user, 
        bool isPersistent, 
        bool rememberBrowser)
    {
        var userIdentity = await CreateUserIdentityAsync(user).WithCurrentCulture();

        // Clear any partial cookies from external or two factor partial sign ins
        AuthenticationManager.SignOut(
            DefaultAuthenticationTypes.ExternalCookie, 
            DefaultAuthenticationTypes.TwoFactorCookie);

        if (rememberBrowser)
        {
            var rememberBrowserIdentity = AuthenticationManager
                .CreateTwoFactorRememberBrowserIdentity(ConvertIdToString(user.Id));

            AuthenticationManager.SignIn(
                new AuthenticationProperties { IsPersistent = isPersistent }, 
                userIdentity, 
                rememberBrowserIdentity);
        }
        else
        {
            AuthenticationManager.SignIn(
                new AuthenticationProperties { IsPersistent = isPersistent }, 
                userIdentity);
        }
    }

    private async Task<SignInStatus> SignInOrTwoFactor(TUser user, bool isPersistent)
    {
        var id = Convert.ToString(user.Id);

        if (UserManager.SupportsUserTwoFactor 
            && await UserManager.GetTwoFactorEnabledAsync(user.Id)
                                .WithCurrentCulture()
            && (await UserManager.GetValidTwoFactorProvidersAsync(user.Id)
                                 .WithCurrentCulture()).Count > 0
                && !await AuthenticationManager.TwoFactorBrowserRememberedAsync(id)
                                               .WithCurrentCulture())
        {
            var identity = new ClaimsIdentity(
                DefaultAuthenticationTypes.TwoFactorCookie);

            identity.AddClaim(new Claim(ClaimTypes.NameIdentifier, id));

            AuthenticationManager.SignIn(identity);

            return SignInStatus.RequiresVerification;
        }
        await SignInAsync(user, isPersistent, false).WithCurrentCulture();
        return SignInStatus.Success;
    }

    public override async Task<SignInStatus> PasswordSignInAsync(
        string userName, 
        string password, 
        bool isPersistent, 
        bool shouldLockout)
    {
        if (UserManager == null)
        {
            return SignInStatus.Failure;
        }

        var user = await UserManager.FindByNameAsync(userName).WithCurrentCulture();
        if (user == null)
        {
            return SignInStatus.Failure;
        }

        if (UserManager.SupportsUserLockout 
            && await UserManager.IsLockedOutAsync(user.Id).WithCurrentCulture())
        {
            return SignInStatus.LockedOut;
        }

        if (UserManager.SupportsUserPassword 
            && await UserManager.CheckPasswordAsync(user, password)
                                .WithCurrentCulture())
        {
            return await SignInOrTwoFactor(user, isPersistent).WithCurrentCulture();
        }
        if (shouldLockout && UserManager.SupportsUserLockout)
        {
            // If lockout is requested, increment access failed count
            // which might lock out the user
            await UserManager.AccessFailedAsync(user.Id).WithCurrentCulture();
            if (await UserManager.IsLockedOutAsync(user.Id).WithCurrentCulture())
            {
                return SignInStatus.LockedOut;
            }
        }
        return SignInStatus.Failure;
    }
}

I hope it helps. Cheers

dav_i
  • 27,509
  • 17
  • 104
  • 136
Guilherme Duarte
  • 3,371
  • 1
  • 28
  • 35
  • Thanks, helped me over the current hurdle. – Andez May 22 '15 at 20:47
  • 5
    This isn't exactly what I need, but this helped point me in the right path. I must admin, I'm so annoyed that I have to spend a bunch of hours to make Identity work with my existing database. – Jeremy Ray Brown Jul 11 '15 at 19:56
  • 1
    I'm getting this error: 'System.Threading.Tasks.Task' does not contain a definition for 'WithCurrentCulture'. Did I miss something? It seems the usings are correct. – fcaldera Dec 04 '15 at 01:30
  • if this fulfils my requirements then thank you very much. – user786 Mar 24 '17 at 05:10
  • @fcaldera the 'WithCurrentCultre' is available to any object within the namespace: Microsoft.AspNet.Identity', because the class TaskExtensions in the project 'Microsoft.AspNet.Identity.Core' is marked as 'internal' – OzBob Jun 10 '19 at 08:49
4

I had the same problem and I don't want to implement the IUserTwoFactorStore<TUser, TKey> just to say that I don't implement it. But I also don't want to go back and muck around if I end up wanting to implement it (which I anticipate I will). So what I consider a future proof (and reusable) solution would be: (inspired by @gjsduarte's answer)

public class SafeUserManager<TUser, TKey> : UserManager<TUser, TKey>
{
    public override Task<bool> GetTwoFactorEnabledAsync(TKey userId)
    {
        return Store is IUserTwoFactorStore<TUser, TKey>
            ? base.GetTwoFactorEnabledAsync(userId)
            : Task.FromResult(false);
    }
}

It would probably be a good idea to do the same for the other Get[feature]EnabledAsync(TKey userId) methods.

Emil Badh
  • 4,562
  • 1
  • 17
  • 20