Most secure php user athentication function

What is the best way to create a secure user authentication function? Below is the core of a php function that takes in the username and password and checks it against the database.

I am specifically interested in the query and its return value. Is using the 'else if($query1)' the best way to validate and set the session variable? Also, what value is best to set for the session variable? An email address, username, bool variable, primary key index, etc?

$query1 = mysql_fetch_array(mysql_query("SELECT primaryKey 
                                           FROM loginInfo 
                                          WHERE email = md5('$email') 
                                            AND password = md5(CONCAT('$password',salt)) 
                                          LIMIT 1"));
if (!$query1)
  return fa开发者_Go百科lse;
else if ($query1) {
  $_SESSION['userNumber'] = $query1[primaryKey];
  return true;
}
else
  return false;

MD5 has known vulnerabilities and is no longer considered secure. You should switch to a stronger hash such as SHA-2.

Also, $query1 can only evaluate to true or false, so the final else part is useless and will never be reached. Your 3 branches are equivalent to just this:

if (!$query1)
    return false;
else { // else $query1 is obviously true
    $_SESSION['userNumber'] = $query1[primaryKey];
    return true;
}

There is no such thing as a "best value" to store in the session, but the primary key is usually a convenient choice, since it is guaranteed to be unique and also provides an easy way to look up the remaining details. Additionally, if you find yourself frequently displaying some information such as the user's name, you could additionally store that in the session for easy access.

There are multiple issues with this code:

• SQL injection vulnerability. (What happens when a user enters an email address of ') OR 1=1 OR '' = ('?) You should have a look at mysql_real_escape_string, or consider using parametrized queries.
• Your never call mysql_free_result on the resource returned from mysql_query, which will leak resources on the MySQL server (until the script terminates), and may prevent future queries in the same script from executing.
• MD5 is deprecated due to vulnerabilities. Consider using a hash in the SHA family instead.

It depends on where the attacker will have access. If he somehow has access to the database, the suggested changes of the hash-type are important.

If he doesn't, it's more important to restrict the number of failed logins to avoid bruteforce-attacks.

However, the vulnerability cdhowie pointed at has to be fixed at all.