Simple bcrypt Register User and User Login using PHP – is this approach acceptable?

Posted on

Problem

I am learning PHP and have been looking into a suitable way to safely store password data in MySQL.

Following advice from here (https://stackoverflow.com/questions/4795385/how-do-you-use-bcrypt-for-hashing-passwords-in-php), is this an acceptable way to deal with passwords?

The code is a very basic example and I haven’t included things like error checking / checking if user already exists etc. to keep my example concise.

New Register User:

$fld_email =    $_POST['fld_email'];
$fld_name =     $_POST['fld_name'];
$fld_pwd =      $_POST['fld_pwd'];

$hashToStoreInDb = password_hash($fld_pwd, PASSWORD_BCRYPT, array("cost" => 11));

  $sql = "INSERT INTO tbl_a_users (fld_email
                                 , fld_name
                                 , fld_pwd
                                 , fld_date) 
                           VALUES (:fld_email
                                 , :fld_name
                                 , :fld_pwd
                                 , now())";

$stmt = $pdo->prepare($sql);

$stmt->bindParam(':fld_email', $fld_email);
$stmt->bindParam(':fld_name', $fld_name);
$stmt->bindParam(':fld_pwd', $hashToStoreInDb);

$stmt->execute();

New Process Login:

$fld_email    = $_POST['fld_email'];
$fld_pwd_form = $_POST['fld_pwd'];

$stmt = $pdo->prepare('SELECT fld_pwd FROM tbl_a_users WHERE fld_email = :email LIMIT 1');
$stmt->bindParam(':email', $fld_email);
$stmt->execute();
$row = $stmt->fetch(PDO::FETCH_OBJ);

$isPasswordCorrect = password_verify($fld_pwd_form, $row->fld_pwd);

if ($isPasswordCorrect == true) {
    // do something
} else {
    // do something else
}

There is no error message here, but I wanted to check with experts about whether this is an acceptable approach before I continue work on this area.

I will also be using HTTPS for the login / register pages.

Solution

Yes, your approach to hashing and storing passwords is acceptable.

Misc

  • == true is not needed, and should never be used. Either directly write if (password_verify(...)) {...}, or use === (if you don’t trust ==, which you shouldn’t in many situations[*]).
  • you should generally try to be consistent with your variable naming schemes. Either use camelCase or snake_case, don’t mix them.
  • I appreciate that you are mostly consistent with your naming overall, as it really helps readability. :email is the exception here, so you should change it.
  • fld_pwd_form also does not fit your general naming scheme. Either append _form to all values that are input, or to none (and think about changing it to _input).
  • Is there a reason for the fld_ prefix? It reduces readability, so if you can still change your database names, I would remove it.
  • hash is enough as a name, you don’t need ToStoreInDb.


[*] == should be ok for password_verify, as it is very unlikely that the function will be changed to return anything else than true or false in the future. On the other hand, it is PHP, so who knows. And "1", 1, -1, ["error"], "error" are all true.

Leave a Reply

Your email address will not be published. Required fields are marked *