Does TwoFactorSignIn contain a bug or am I configuring Identity incorrectly?

[CamiloTerevinto]

Plase refer to Does Identity Core TwoFactorSignIn contain a bug?

[blowdart]

Copying S/O post, so everything is in one place.

I have been working on an ASP.NET Core application for a couple months. Now near finishing the first beta I realized I hadn't enabled Two-Factor Authentication, and now I think I uncovered a bug in the implementation for Microsoft.AspNetCore.Identity. If we look at how a user is retrieved, it does this:

    /// <summary>
    /// Returns the User ID claim value if present otherwise returns null.
    /// </summary>
    /// <param name="principal">The <see cref="ClaimsPrincipal"/> instance.</param>
    /// <returns>The User ID claim value, or null if the claim is not present.</returns>
    /// <remarks>The User ID claim is identified by <see cref="ClaimTypes.NameIdentifier"/>.</remarks>
    public virtual string GetUserId(ClaimsPrincipal principal)
    {
        if (principal == null)
        {
            throw new ArgumentNullException(nameof(principal));
        }
        return principal.FindFirstValue(Options.ClaimsIdentity.UserIdClaimType);
    }

    /// <summary>
    /// Returns the user corresponding to the IdentityOptions.ClaimsIdentity.UserIdClaimType claim in
    /// the principal or null.
    /// </summary>
    /// <param name="principal">The principal which contains the user id claim.</param>
    /// <returns>The user corresponding to the IdentityOptions.ClaimsIdentity.UserIdClaimType claim in
    /// the principal or null</returns>
    public virtual Task<TUser> GetUserAsync(ClaimsPrincipal principal)
    {
        if (principal == null)
        {
            throw new ArgumentNullException(nameof(principal));
        }
        var id = GetUserId(principal);
        return id == null ? Task.FromResult<TUser>(null) : FindByIdAsync(id);
    }

However, the TwoFactorSignInAsync method in the SignInManager never sets a Claims of type UserIdClaimType, but it sets 4 times the same Name claim, containing the User's Id.
Is this a bug in the implementation of TwoFactorSignInAsync or some configuration is not correct in my configuration of Identity? Which is this:

CookieAuthenticationOptions cookieOptions = new CookieAuthenticationOptions
{
   CookieHttpOnly = true,
   LoginPath = "/User/Login",
   CookieSecure = CookieSecurePolicy.Always,
   LogoutPath = "/User/Logout"
 };

 services.AddIdentity<User, Role>(options =>
 {
     options.Cookies.ApplicationCookie = cookieOptions;
     options.Cookies.ExternalCookie = cookieOptions;
     options.Cookies.TwoFactorRememberMeCookie = cookieOptions;
     options.Cookies.TwoFactorUserIdCookie = cookieOptions;

     options.Password = new PasswordOptions
     {
         RequiredLength = 8,
         RequireLowercase = true,
         RequireUppercase = true,
         RequireNonAlphanumeric = true
     };

     options.SignIn.RequireConfirmedEmail = true;
 })
 .AddUserStore<MyStore>()
 .AddRoleStore<MyStore>()
 .AddDefaultTokenProviders();

EDIT:

By the way, this is easily solved by changing GetUserId to something like this:

    /// <summary>
    /// Returns the User ID claim value if present otherwise returns null.
    /// </summary>
    /// <param name="principal">The <see cref="ClaimsPrincipal"/> instance.</param>
    /// <returns>The User ID claim value, or null if the claim is not present.</returns>
    /// <remarks>The User ID claim is identified by <see cref="ClaimTypes.NameIdentifier"/>.</remarks>
    public virtual string GetUserId(ClaimsPrincipal principal)
    {
        if (principal == null)
        {
            throw new ArgumentNullException(nameof(principal));
        }

        string userIdClaim = principal.FindFirstValue(Options.ClaimsIdentity.UserIdClaimType);
        if (string.IsNullOrEmpty(userIdClaim))
        {
            // THIS is the actual ClaimType that the TwoFactorSignInAsync puts on the User
            return principal.FindFirstValue(Options.ClaimsIdentity.UserNameClaimType); 
        }

        return userIdClaim;
    }

[HaoK]

I'm not sure what you mean by TwoFactorSignInAsync sets the username 4 times.

So this method is what generates the principal for sign in: https://github.com/aspnet/Identity/blob/dev/src/Microsoft.AspNetCore.Identity/UserClaimsPrincipalFactory.cs#L78

SignIn manager does some work around verifying the two factor code, and the user, but the actual application cookie generation is done by the UserClaimsPrincipalFactory. Perhaps you are looking at the wrong cookie (there are 4 of them) In general its only the application cookie that you should care about, the rest are there for identity APIs to work

[CamiloTerevinto]

Hello @HaoK,

If I debug right after var result = await _SignInManager.TwoFactorSignInAsync("Email", model.Code, false, model.RememberBrowser); the HttpContext.User property contains 4 times exactly the same claim:

{http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name: MYGUID}

Thus, either how I set up Identity in Startup is wrong or the 1.0.0 version of TwoFactorSignInAsync does not put the expected claim type "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier"

[HaoK]

Can you make sure that AutomaticAuthenticate is off for all of the cookie middleware other than the ApplicationCookie (inside of IdentityOptions.Cookies.XyzCookie)

[CamiloTerevinto]

@HaoK,

AutomaticAuthenticate is a bool, and I am not setting it explicitly in my code, so my guess is that it's false by default?
If I debug up to the UserManager (code downloaded from GitHub) and enter any method and go to Options.Cookies, all 4 (Application, External, TwoFactorRememberMe and TwoFactoUserId) have AutomaticAuthenticate = true

[HaoK]

What version of the packages are you using, there was a bug fixed in older versions where the defaults used to be true, you can turn all of the AutomaticAuthenticates to false other than the Application if don't want to upgrade

[CamiloTerevinto]

I have no problem upgrading, I am even on EF Core 1.0.1 already. But I have the latest stable as per Nuget (1.0.0). I'll try turn them off explicitly and see what it does

[HaoK]

The defaults should be set to off: https://github.com/aspnet/Identity/blob/dev/src/Microsoft.AspNetCore.Identity/IdentityCookieOptions.cs

[CamiloTerevinto]

I followed the link provided and changed the Identity setup to this:

services.AddIdentity<User, Role>(options =>
{
    options.Cookies.ApplicationCookie.CookieHttpOnly = true;
    options.Cookies.ApplicationCookie.CookieSecure = CookieSecurePolicy.Always;
    options.Cookies.ApplicationCookie.LoginPath = "/User/Login";
    options.Cookies.ApplicationCookie.LogoutPath = "/User/Logout";
    //..............

Now after a successful "TwoFactorSignInAsync", HttpContext.User contains 0 identities/claims so not even my workaround works. I checked and all but ApplicationCookie have AutomaticAuthenticate = false

[HaoK]

Just to clarify expectations, assuming TwoFactorSignIn returned success, it should be setting a cookie which contains the application cookie which will have the user on the next request. Is that what you are seeing? I'd also try getting rid of any HttpOnly/Cookie secure extraneous settings for now.

[CamiloTerevinto]

Using this

var result = await _SignInManager.TwoFactorSignInAsync("Email", model.Code, false, model.RememberBrowser);
if (result.Succeeded)
{
    User user = await userManager.GetUserAsync(HttpContext.User);
}

I'd expect that if I enter the if it's because all is good, the user entered the correct code and etc. Under normal "_SignInManager.PasswordSignInAsync" this is true; however, for TwoFactorSignInAsync I get null.
I am now debugging TwoFactorSignInAsync: SignInManager L379

twoFactorInfo = good, retrieved the expected entity
user = good, found the user from the database
error = null, all is good so far
VerifyTwoFactorTokenAsync = true
RememberTwoFactorClientAsync =>
userId = good
rememberBrowserIdentity = good
await SignInAsync = ? not sure what happens there
await SignInAsync (OUTSIDE RememberTwoFactorClientAsync) =>
userPrincipal = good, contains 3 claims: NameIdentifier, Name and SecurityStamp and all correspond to my user
await SignInAsync = ? again, not sure what happens there

GOING BACK (all TwoFactorSignInAsync apparently ran fine hence result.Succeeded == true) => HttpContext.User contains absolutely no claims.

[HaoK]

Again, two factor sign in if successful, means the NEXT request will have the User set. Authentication for the current request has already happened. None of the SignIn's have any effect on the current request.

[CamiloTerevinto]

Ahhh, I now get what you are saying. I did not know that. In fact, I never really saw that mentioned anywhere.
Ok, I removed the get user part for that action and all is working fine, the redirection is already authorized properly. Thank you for your time

[inexuscore]

Can someone please, for the love of god, provide an example project for a basic 2FA setup in Web API 2. I'm using Identity 2.2.2 and I need to implement 2FA ASAP, I'm already behind schedule and I need this NOW. Been struggling with this for days and the MVC5 template from Visual Studio 2017 wasn't helpful.

The SignInManager.PasswordSignInAsync() method works fine, it returns RequiresVerification. But I'm not sure what to do from there.

In my AuthorizationServerProvider class, in the GrantResourceOwnerCredentials() method, what do I do after receiving a status of RequiresVerification, do I have to sign the user in by generating the user identity - for the SignInManager to work properly - or do I set an error on the context and return. What I need to do is this: if the /token end-point results in RequiresVerification, return an error to notify the client (angular app) that the user needs to verify the 2FA security code. Then I generate and send the code, and display some UI for verification. The user receives the code, sends it to another end-point and it gets verified successfully. But then when I try to call SignInManager.TwoFactorSignInAsync() it always fails with a status of Failure.

This is my code:

public override async Task GrantResourceOwnerCredentials(OAuthGrantResourceOwnerCredentialsContext context)
{
    var user = await UserManager.FindAsync(context.UserName, context.Password);
    if (user == null)
    {
        context.SetError("invalid_grant", "The user name or password is incorrect.");
        return;
    }

    var signInResult = await ServiceContext.SignInManager.PasswordSignInAsync(context.UserName, context.Password, isPersistent: true, shouldLockout: false);

    // other statuses .. removed for brevity

    if (signInResult == SignInStatus.RequiresVerification)
    {
        var code = await UserManager.GenerateTwoFactorTokenAsync(user.Id, "Email Code");
        IdentityResult result = await UserManager.NotifyTwoFactorTokenAsync(user.Id, "Email Code", code);
                    
        if (!result.Succeeded)
        {
            context.SetError("two_factor_error", "failed to send 2FA code");
            return;
        }
        else
        {
            // user needs to verify security code
            context.SetError("requires_verification");
            return;
        }
    }
}

I'm generating and sending the code using the UserManager class because the SignInManager.SendTwoFactorCodeAsync() method doesn't work (User Id not found).

And this is how I verify the security code in another end-point:

[HttpPost]
[AllowAnonymous]
[Route("VerifySecurityCode")]
public async Task<IHttpActionResult> VerifySecurityCode(VerifySecurityCodeModel model)
{
    try
    {
        var user = await UserManager.FindAsync(model.UserName, model.Password);
        if (user == null)
            return BadRequest("username or password is invalid");

        var isTokenValid = await UserManager.VerifyTwoFactorTokenAsync(user.Id, "Email Code", model.Token);
        if (!isTokenValid)
            return BadRequest("Invalid token. try again");

        // this returns empty GUID
        //var userId = await ServiceContext.SignInManager.GetVerifiedUserIdAsync();

        // this sets the 2FA browser cookie, but doesn't actually sign-in
        //await ServiceContext.SignInManager.SignInAsync(user, isPersistent: true, rememberBrowser: true);

        // this always returns SignInStatus.Failure
        var result = await ServiceContext.SignInManager.TwoFactorSignInAsync("Email Code", model.Token, isPersistent: true, rememberBrowser: true);

        return Ok("Security code verified successfully");
    }
    catch (Exception ex)
    {
        return InternalServerError(ex);
    }
}

I'm getting the user by username and password again, because for some messed up reason, the SignInManager.GetVerifiedUserIdAsync() method returns an empty GUID. But the SignInManager.HasBeenVerifiedAsync() returns true (?!).

I'm nearly there, it shouldn't be this difficult, I think I'm missing something here. What I want is this:

• The user sends a POST request to /token with their username/password
• If 2FA is enabled (confirmed email, or a verified phone number), we don't sign the user in, but require them to verify the security code first. They shouldn't be able to request ANY resource (end-point) until they have verified their 2FA code
• We generate the code and send it via email/sms. The user verifies the code, and is signed-in. Of course a 2FA cookie should be set at this point to remember the browser/device. We don't want them to go through 2FA on subsequent requests.

This is my Startup.cs config:

// Configure the db context, user manager and signin manager to use a single instance per request
app.CreatePerOwinContext(SurveyContext.Create);
app.CreatePerOwinContext<ApplicationUserManager>(ApplicationUserManager.Create);
app.CreatePerOwinContext<ApplicationSignInManager>(ApplicationSignInManager.Create);

OAuthAuthorizationServerOptions OAuthServerOptions = new OAuthAuthorizationServerOptions()
{
    AllowInsecureHttp = true,
    TokenEndpointPath = new PathString("/token"),
    AccessTokenExpireTimeSpan = TimeSpan.FromDays(7),
    Provider = new AuthorizationServerProvider()
};

// Configure the application for OAuth based flow
PublicClientId = "self";
OAuthOptions = new OAuthAuthorizationServerOptions
{
    TokenEndpointPath = new PathString("/Token"),
    Provider = new ApplicationOAuthProvider(PublicClientId),
    AuthorizeEndpointPath = new PathString("/api/Account/ExternalLogin"),
    AccessTokenExpireTimeSpan = TimeSpan.FromDays(7),
    AllowInsecureHttp = true
};

// Token Generation
app.UseOAuthAuthorizationServer(OAuthServerOptions);
app.UseOAuthBearerAuthentication(new OAuthBearerAuthenticationOptions());

// Cookie Configuration
app.UseExternalSignInCookie(DefaultAuthenticationTypes.ExternalCookie);
app.UseTwoFactorSignInCookie(DefaultAuthenticationTypes.TwoFactorCookie, TimeSpan.FromMinutes(5));
app.UseTwoFactorRememberBrowserCookie(DefaultAuthenticationTypes.TwoFactorRememberBrowserCookie);

I have implemented 2FA in MVC 5 apps before, nothing fancy there. But in Web API + OWIN projects, I think I'm lost. I'd really appreciate it if someone took the time to guide me in the right direction or perhaps provide example code. I need to deploy my 2FA implementation in less than a week.

Thank in advance, fingers crossed, I'll get a helpful response soon. Cheers.

[CamiloTerevinto]

@inexuscore That is completely unrelated to this question I had. You should either open a new bug report or question in Stack Overflow (you'll likely get better answers there).

[inexuscore]

@CamiloTerevinto I understand that. I couldn't find anything useful on SO, and since you guys were talking about the 2FA bug, I thought I post my question here. I'll open a thread on SO but doubt I'd get any answers there. Bug reports won't help either, the repo is closed and they've moved on to Identity 3 and .NETCore .. any ideas are appreciated.

[CamiloTerevinto]

@inexuscore The problem is that here there was no bug. This was my own problem of not knowing how the framework works :) post it there and email me the link, I'll try to check it out

[inexuscore]

@CamiloTerevinto most probably, that's what I was thinking. I think I'm missing something. In the config, or the way I'm using the framework. Sure, I'll link you up as soon as I've created the SO thread.