Best login practice

Question

I'm building a local application that has a login form.
I'm retrieving the Username and Password from a database.

What is the safest way to proceed from here because there are many things I can do and i'm wondering if one way is better than another one.

I can simply SELECT directly from my textbox:

SELECT UserId, Password FROM Users_Table WHERE UserId = '" + userIdTextBox.Text + "' AND Password = '" + passwordTextBox.Text + "'".

I can SELECT everything from that table and then compare a SqlDataReader with the textbox.

SELECT UserId, Password FROM Users_Table`<br>
While (myReader.Read())
{
   If (myReader["UserId"] == UserIdTextBox.Text && ...password)
   {
   }  
}

And there are many other way to do it. What is the best/safest way to proceed ?

Gah. Those passwords in the database are salted hashes- right? — Chris Shain, Jul 27 '12 at 14:22
Doesn't that `SELECT` leave you open to SQL injection attacks? — dtsg, Jul 27 '12 at 14:22
Two problems: 1) SQL injection vunerability, 2) your passwords are plain text in DB? I'd consider using hashes if I was you. And also use parameters not string manipulation to form SQL. — KingCronus, Jul 27 '12 at 14:23
Per @Duane's point- what happens if I type into the password text box "'; delete from Users_Table where 'a' = 'a"? **WARNING: This might delete all your user records- be careful.** — Chris Shain, Jul 27 '12 at 14:24
@ChrisShain, careful making that sound like a suggestion....don't want OP testing it to find out! — KingCronus, Jul 27 '12 at 14:24
Also important: you should not be *selecting* all users and passwords from the database into memory. Avoid "retrieving the Username and Password" at all, and just have a query that confirms whether the user's input was valid, and doesn't return the password. — Dan Puzey, Jul 27 '12 at 14:26
Ok guys I realize all the flaws of my login. But seriously all youre telling me right now is chinese. What are theses salted hashes? How can I acces without a select ? — phadaphunk, Jul 27 '12 at 14:29
@PhaDaPhunk - You can do a `SELECT COUNT(UserID) WHERE...` and check the count, then you don't bring anything back across except the count. Another idea you could use is to add a timestamp column in and then do an update query to update the timestamp and then just check the affected row count. It should be 1. — pstrjds, Jul 27 '12 at 14:30

score 4 · Accepted Answer · edited May 23 '17 at 11:43

4

Use parameterized queries, not concatenated text as a SQL command.
Hash your passwords and store the hashses. Add salt to taste.
Upon a login attempt, compare the hash of the user-input to the hashed password.

Basically, under no circumstances should you want anyone, including sysadmins, to possibly know the value of the password itself. If you design and code with this rule in mind, and try not to re-invent the wheel, you'll be fine. If you're manually creating passwords, then you should probably have a "must change password" option enabled for first logon (thanks, Chris Shain).

edited May 23 '17 at 11:43

Community

1
1

answered Jul 27 '12 at 14:26

Tim Lehner

14,813
4
59
76

Definitely. I'll add some description and links. – Tim Lehner Jul 27 '12 at 14:27
@PhaDaPhunk - [http://crackstation.net/hashing-security.htm](http://crackstation.net/hashing-security.htm) - discusses salting and how to do it properly. – pstrjds Jul 27 '12 at 14:28
Should be add a *randomly generated, unique* salt, and store the hash of ( the salt + the hash of ( the password ) ) – Chris Shain Jul 27 '12 at 14:29
Ok when is the password transformed into this ? In the application when it's retreived ? In sql when it's created ? – phadaphunk Jul 27 '12 at 14:33
Your call, but [this](http://stackoverflow.com/questions/481098/c-sharp-and-sql-server-passwords-where-to-do-what) is good advice on the [MembershipProvider](http://msdn.microsoft.com/en-us/library/f1kyba5e.aspx) – Tim Lehner Jul 27 '12 at 14:35
What if there is no way to create an user ? The only way is WITHIN sql server. That means i absolutly have to insert in my table a plain text field – phadaphunk Jul 27 '12 at 14:40
I know but this means that when I go inside Sql Management studio, the whole Insert into Users_Table etc.. will be followed by an SQL way of hashing my insert – phadaphunk Jul 27 '12 at 14:46
2

You shouldn't be adding users via SQL management studio. Either that or you should set a flag on that user's account that makes the application UI *force* the user to set a new password the next time they log in. – Chris Shain Jul 27 '12 at 14:49
@PhaDaPhunk, [this article](http://weblogs.sqlteam.com/mladenp/archive/2009/04/28/Comparing-SQL-Server-HASHBYTES-function-and-.Net-hashing.aspx) might help if you're thinking of hashing (creating password, comparing passwords) in different tiers of your app. – Tim Lehner Jul 27 '12 at 14:51

score 4 · Answer 2 · answered Jul 27 '12 at 14:35

1. Only return what you need (performance part. 1)

You're looking to log a single user in, therefore SELECTing any other information is redundant. Using the SELECT TOP 1 keyword will allow you to select only a single entry, when the WHERE clause is fulfilled.

2. Stored procedures (performance part. 2)

Although optional, stored procedures are fantastic for optimising SQL. Essentially they're compiled SQL, and are perfect for SQL that will be executed a lot, and that could potentially get quite complex. In addition stored procedures use parameters, which leads me to my next point.

3. SQL Parameters (security part. 1)

Use parameters. Parametrizing your SQL helps to prevent SQL injection, which can cause a world of hurt when overlooked. It's easy and straight forward to use, and a definite must when communicating with any database!

4. Salt and hash your password (security part. 2)

Storing a password as plain text, not that I'm implying you are, is very unsafe. Your strongest option is to salt your passwords, and then hash them. Then when attempting to login, apply the salt and hash. This is what you'll compare.

`TOP 1` shouldn't be necessary since all userIds should be unique by design (which is strongly recommended!) — libjup, Jul 27 '12 at 14:38
What if there is no way to create an user ? The only way is WITHIN sql server. — phadaphunk, Jul 27 '12 at 14:39

score 0 · Answer 3 · answered Jul 27 '12 at 14:37

People have mentioned SQL injection vulnerabilities, and hashes/salts so I won't address them here.

However, the general rule of thumb is to do as much of the work as possible as close to the database as possible. That means do not load more from the database into memory than is necessary.

So, a SQL data reader or something that loads the entire table into memory would be an inefficient choice. The best choice would be to query for username/password (hashed password) combinations that match user input. Then check to ensure that the count() of these records is equal to EXACTLY 1. If less then 1 then login has failed. If greater then 1, something is wrong with your database. If exactly 1, the login was successful.

score 0 · Answer 4 · edited May 23 '17 at 11:51

The best thing to do here would be to have a stored procedure on your database. This way you will also never be exposing the password on the client-side.

To create the stored procedure on your server, try something like this:

         CREATE PROCEDURE AuthenticateUser
(@username VarChar(25),
            @password VarChar(25)) AS
SELECT
COUNT(*)
FROM COMPANY
WHERE Login = @username
AND Password = @password
RETURN @@Rowcount

And the code to access this stored procedure would be along the lines of:

        //Open a connection to the database using your connection string
        using (SqlConnection con = new SqlConnection("My Configuration string"))
        {
            //Open the connection
            con.Open();
            //Create a new command for the stored proc, using the existing connection just opened
            using(SqlCommand cmd = new SqlCommand("AuthenticateUser", con))
            {
            cmd.CommandType = CommandType.StoredProcedure;

                //Add the username and password to the command, as paramaters (Prevents a lot of security issues, such as SQL Injection)
            cmd.Parameters.Add("@username", SqlDbType.VarChar, 25).Value = UserIdTextBox.Text;
            cmd.Parameters.Add("@password", SqlDbType.VarChar, 25).Value = "Password";
                //A paramater for the return value, which will be a bool (Only 0 or 1 should be returned from the database/stored proc)
            SqlParameter ret = new SqlParameter("ret", SqlDbType.Int);
            ret.Direction = ParameterDirection.ReturnValue;
            cmd.Parameters.Add(ret);
                //Execute the query
            cmd.ExecuteNonQuery();
            if (Convert.ToBoolean(ret.Value) == true)
            {
                //Login Successful
            }
            else
            {
                //Login Failed
            }
        }
    }

The SQLParamaters help reduce the amount of interaction users can have with your database, as it sanitises the input instead of blindly accepting anything they input (Which could lead to them deleting your entire database).

But bear in mind. You should also hash and salt passwords. If you want to look at this, there's a question about it already on stack overflow. And it's very bad practice not to do this.

Best login practice

4 Answers4

1. Only return what you need (performance part. 1)

2. Stored procedures (performance part. 2)

3. SQL Parameters (security part. 1)

4. Salt and hash your password (security part. 2)

Linked