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 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 => u.UserName == model.Email).SingleOrDefault();
if (user != null)
{
if (user.Locked

Only focusing on this part

var user = UserManager.Users.Where(u => 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 => 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 => 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");
    }

StackExchange Code Review Q#107605, answer score: 4