Checking locked users using Identity 2.0 I'm writing a theoretical MVC application to aid in learning more about ASP Identity 2. In real terms I'm new to ASP Identity as a whole but thought I'd jump in in this release as 


it's the default in new projects and 
everything I read about it seems to mostly suit my needs.


There is one issue that I need to be able to overcome. That issue is locking out users. I want to allow administrators the facility to be able to lock out a user should they wish to restrict their access.

From what I've read so far, there is a temporary lockout solution for example when the user tries their password incorrectly x number of times for y number of minutes. This doesn't seem to be the recommended method for more long term solutions.

For a longer term solution, I've added a Locked property to the ApplicationUser class in Entity Framework code first:

public class ApplicationUser : IdentityUser
{
    public string FirstName { get; set; }
    public string Surname { get; set; }
    public bool Locked { get; set; }
    [NotMapped]
    public string FullName 
    { 
        get 
        {
            return FirstName + " " + Surname;
        } 
    }
}


Getting to the point of my question though, which is, is my modified ActionResult Login method secure and efficient? I don't want to make unnecessary roundtrips to the database and I also don't want to unwillingly do anything unsecure.

public async Task&lt;ActionResult&gt; Login(LoginViewModel model, string returnUrl)
    {
        if (!ModelState.IsValid)
        {
            return View(model);
        }
        // MY LOCKED CHECK CODE:
        // Check if the user is locked out first
        // 1. Fail if it is
        // 2. If user isn't found then return invalid login atempt
        // 3. If the user is found and it isn't locked out then proceed with the login as usual
        var user = UserManager.Users.Where(u =&gt; u.UserName == model.Email).SingleOrDefault();
        if (user != null)
        {
            if (user.Locked)
            {
                return View("Lockout");
            }
        }
        else
        {
            ModelState.AddModelError("", "Invalid login attempt.");
            return View(model);
        }
        // END MY CODE

        // This doesn't count login failures towards account lockout
        // To enable password failures to trigger account lockout, change to shouldLockout: true
        var result = await SignInManager.PasswordSignInAsync(model.Email, model.Password, model.RememberMe, shouldLockout: false);
        switch (result)
        {
            case SignInStatus.Success:
                return RedirectToLocal(returnUrl);
            case SignInStatus.LockedOut:
                return View("Lockout");
            case SignInStatus.RequiresVerification:
                return RedirectToAction("SendCode", new { ReturnUrl = returnUrl, RememberMe = model.RememberMe });
            case SignInStatus.Failure:
            default:
                ModelState.AddModelError("", "Invalid login attempt.");
                return View(model);
        }
    }
Only focusing on this part 


var user = UserManager.Users.Where(u =&gt; u.UserName == model.Email).SingleOrDefault();
if (user != null)
{
    if (user.Locked)
    {
        return View("Lockout");
    }
}
else
{
    ModelState.AddModelError("", "Invalid login attempt.");
    return View(model);
}



Are you aware that if the Users contains more than one user with the same Email the call to SingleOrDefault will throw an InvalidOperationException ? If you are 100 percent sure that this won't ever happen then there won't be a problem. 

In its current state this linq query will for each user query the model for the Email property. If you store it in a variable the access will be much faster.  

Checking first if the user == null will make it more obvious what is happening. As I first looked over this code I thought hey, how should the code after the if..else ever be reached.

Applying this will lead to  

    string email = model.Email;
    var user = UserManager.Users.Where(u =&gt; u.UserName == email).SingleOrDefault();
    if (user == null)
    {
        ModelState.AddModelError("", "Invalid login attempt.");
        return View(model);
    }

    if (user.Locked)
    {
        return View("Lockout");
    }


This looks far better IMO and is more readable.  

Usually a username isn't checked in a case sensitive way so instead of u.UserName == email you should use the string.equals() method.  

This method takes as the third parameter a StringComparison enum which defines how the comparison will take place. The best one for all cultures, for instance using a turkish locale can make problems (does-your-code-pass-turkey-test), will be to use StringComparison.OrdinalIgnoreCase.  

Applying this will lead to  

    string email = model.Email;
    var user = UserManager.Users.Where(u =&gt; string.Equals(u.UserName, email, StringComparison.OrdinalIgnoreCase)).SingleOrDefault();
    if (user == null)
    {
        ModelState.AddModelError("", "Invalid login attempt.");
        return View(model);
    }

    if (user.Locked)
    {
        return View("Lockout");
    }