views:

68

answers:

2

I'm still learning LINQ. I can do some simple queries, but this one is a little different. I have the following query that authenticates a user.

User user = (from u in db.Users
             where u.Username.Equals(username) &&  
                   u.Password.Equals(UserSecurity.GetPasswordHash(username, password)) &&
                   u.Status == true                  
             select u).FirstOrDefault();

This works but now I need to log why the login failed. If there was an invalid username, I'll log one thing. If the password was wrong, I'll log another. Or, if the user is inactive (status = false), then I'll log another. I realize I can separate this out into 3 separate queries to figure out what failed, but I was wondering if it would be more efficient to do it in one. If so, how? This is my (untested) 3 query approach.

            User user2 = null;

            var users1 = db.Users.Where(i => i.Username.Equals(username));

            if (users1 != null)
            {
                var users2 = users1.Where(i => i.Password.Equals(UserSecurity.GetPasswordHash(username, password)));

                if (users2 != null)
                {
                    var users3 = users2.Where(i => i.Status);

                    if (users3 != null)
                    {
                        user2 = users3.FirstOrDefault();
                    }
                    else
                    {
                        // User inactive
                    }
                }
                else
                {
                    // Password invalid.
                }
            }
            else
            {
                // Username invalid
            }
+2  A: 

You should not specify if the username was in correct or specify that the pasword was incorrect, "Invalid user name or Password" should be sufficient. And common to most systems.

This leaves you with the check for the status, if the username/password combo was correct then you have the user record returned and you can check the status from that.

User user = (from u in db.Users 
     where u.Username.Equals(username) &&   
           u.Password.Equals(UserSecurity.GetPasswordHash(username, password))
     select u).FirstOrDefault(); 

if (user == null)
{
  // Invalid user name or password
}
else if (user.Status != true)
{
  // User inactive
}
else
{
  // Success
}

If you must indicate the difference between invalid user name vs invalid password you can do the following.

User user = (from u in db.Users 
     where u.Username.Equals(username)
     select u).FirstOrDefault();

if  (user == null)
{
  // Invalid user name
}
else if (!user.password.Equals(...))
{
  // Invalid password
}
else if (user.Status != true)
{
  // User inactive
}
else
{
  // Success
}
Chris Taylor
I agree that you should only show the user an "Invalid username or password" dialog. But, this is for logging. And, a regular user does not have the permissions to read the log.
bsh152s
@bsh152s, following the same approach I added an example that would provide the granularity you require.
Chris Taylor
Thanks, that's what I came with after reading your first response. I guess I was overthinking it a bit.
bsh152s
+1  A: 

I don't think that using Linq only for "using linq" is required in this case.

That being said, I do use the extension method SequenceEqual of a byte array to compare the two hashed password. I usually store my password as a binary(64) in my database (along with a salt binary(16))

This is my typical approach (more or less) to validate a password. I modified it to include a Linq search like your example

public bool AuthenticateUser(string username, string password)
{
    // I don't validate password here (see TODO below)
    var user = db.User.FirstOrDefault(u => u.UserName == username && u.Status);

    if(user != null)
    {
        var rehash = Hashing.Hash(password, user.PasswordSalt); // PasswordSalt is a byte array
        if(rehash.SequenceEqual(user.Password))
        {
            return true;
        }
        else
        {
            Logger.LogUnsuccessfulAuthentication(user);

            // TODO: Increase user-login failure count and system-wide failure count
        }
    }

    return false;
}
Pierre-Alain Vigeant