Problems with PHP login code validation and password_verify function

[vexdarkness]

I’m working with a code snippet to login users to my site. I had to alter the code to get it to work, but in doing so, errors with the form now aren’t shown (the page just refreshes if a non-user tries to login, or the form is blank). It works great if the member exists.

Here’s my code:

if (! empty($_POST["login"])) {
    $isAuthenticated = false;
    
    $username = $_POST["member_name"];
    $password = $_POST["member_password"];

    $hash_pass = password_hash($password, PASSWORD_BCRYPT);
    
    $user = $auth->getMemberByUsername($username);
    if (password_verify($password, $hash_pass)) {
       $isAuthenticated = true;
    }    
    if ($isAuthenticated) {
        $_SESSION["member_id"] = $user[0]["member_id"];
        
        // Set Auth Cookies if 'Remember Me' checked
        if (! empty($_POST["remember"])) {
            setcookie("member_login", $username, $cookie_expiration_time);
            
            $random_password = $util->getToken(16);
            setcookie("random_password", $random_password, $cookie_expiration_time);
            
            $random_selector = $util->getToken(32);
            setcookie("random_selector", $random_selector, $cookie_expiration_time);
            
            $random_password_hash = password_hash($random_password, PASSWORD_DEFAULT);
            $random_selector_hash = password_hash($random_selector, PASSWORD_DEFAULT);
            
            $expiry_date = date("Y-m-d H:i:s", $cookie_expiration_time);
            
            // mark existing token as expired
            $userToken = $auth->getTokenByUsername($username, 0);
            if (! empty($userToken[0]["id"])) {
                $auth->markAsExpired($userToken[0]["id"]);
            }
            // Insert new token
            $auth->insertToken($username, $random_password_hash, $random_selector_hash, $expiry_date);
        } else {
            $util->clearAuthCookie();
        }
        $_SESSION['member_name'] = $row['members'];
        $util->redirect("main.php");
    } else {
        $message = "Invalid Login";
    }
}

The only difference is in the “password_verify” statement, which in the original code is:

if (password_verify($password, $user[0]["member_password"])) {
        $isAuthenticated = true;
    }

That shows the “Invalid Login” error if there are issues (blank, or non-users), but for some reason does NOT log my users in, whereas my code does, but won’t show the error in any case.

What could be wrong? I’ve tried several variations of code to display the error if needed, and nothing works! Weird!

[benanamen]

Starting from the top…

1. You have not posted anything that checks for a POST REQUEST
2. You show no code that trims the post array which means blank spaces will be not empty and code will run.
3. Do not create variables for nothing.
4. I want to see the code for the method getMemberByUsername
5. The if ($isAuthenticated) is redundant of the if before it. Not needed.
6. $user[0] is odd. I need to see the method that creates it.
7. if (! empty($_POST[“remember”])) See #2
8. I assume the token is for CSRF. It is overly complicated.
9. It would help to see all the methods this code is using.

[vexdarkness]

Okay thanks! Yes, the code does seem a bit redundant. I have cleaned it up and will see about trimming the post arrays.

[droopsnoot]

This bit

$password = $_POST["member_password"];

$hash_pass = password_hash($password, PASSWORD_BCRYPT);

$user = $auth->getMemberByUsername($username);
if (password_verify($password, $hash_pass)) {
   $isAuthenticated = true;
}    

surely isn’t going to work? Calling password_verify() with your user-supplier unencrypted password and the encrypted version of that same password will surely always return true. You should be comparing the user entry with the encrypted password that you store in the database.

The version in your note at the end of the post should work, as long as you stored the password using password_hash() in the first place.

In here

 $_SESSION['member_name'] = $row['members'];

where does $row come from?

[benanamen]

@droopsnoot

You should be comparing the encrypted version of the user entry with the encrypted password that you store in the database.

Correction, the HASHED version…with the HASHED password.

* I 'm sure that’s what you meant

[droopsnoot]

Yes, but I think several other bits of what I wrote there are wrong as well. I’ve edited it now.

On another note:

$user = $auth->getMemberByUsername($username);

Why does this function return an array of users? Do you allow more than one user to have the same username? How do you tell them apart?

ETA - when you changed the code to use password_hash() to store the password, did you expand the column to make it long enough?

[vexdarkness]

@droopsnoot

surely isn’t going to work? Calling password_verify() with your user-supplier unencrypted password and the encrypted version of that same password will surely always return true. You should be comparing the user entry with the encrypted password that you store in the database.

The version in your note at the end of the post should work, as long as you stored the password using password_hash() in the first place.

Seems you are correct, ANY password with a registered username works. That’s also probably why the “Invalid Login” error message doesn’t display. So how do I fix this?

The user entered password is stored in $password, and the hashed version is BCRYPTed and stored in the DB as “$member_password”. I tried this and it still returns ANY password as true as you said:

if (! empty($_POST[“login”])) {
// $isAuthenticated = false;

$username = $_POST["username"];
$password = $_POST["password"];

$hash_pass = password_hash($member_password, PASSWORD_BCRYPT);

$user = $auth->getMemberByUsername($name);
if (password_verify($password, $hash_pass)) {
   // $isAuthenticated = true;

@droopsnoot

$_SESSION[‘member_name’] = $row[‘members’];

I removed that part as it didn’t seem to do anything. lol

So how do I get it to compare the entered password to the hashed version in the DB? I’m at a loss.

[chorn]

@vexdarkness

and the hashed version is BCRYPTed and stored in the DB as “$member_password”

So why do you use password_hash twice then?

@vexdarkness

I tried this and it still returns ANY password as true as you said

Show us an example where the problem can be seen, as this should not happen

<?php

$member_password = 'test123';
$hash_pass = password_hash($member_password, PASSWORD_BCRYPT);
$passwords = [1,2,3, $member_password, 'abc', '123test'];

foreach($passwords as $password){
    var_dump(password_verify($password, $hash_pass));
}

[vexdarkness]

@chorn

So why do you use password_hash twice then?

I use it once. The other uses are for cookies. I don’t mess with that code as it appears to work.

@chorn

Show us an example where the problem can be seen, as this should not happen

What do you mean? Copy paste your code and execute and post the results?

[chorn]

@vexdarkness

Copy paste your code and execute and post the results?

You have at least to modify it with all the data you tested.

[vexdarkness]

@chorn

You have at least to modify it with all the data you tested.

I’m still not sure what you mean.

[chorn]

Show which exact code, including data, supports your theory.

[droopsnoot]

@vexdarkness

I tried this and it still returns ANY password as true as you said:

Yes, because you are not comparing it with the value coming from the database. You are comparing it with the hash of password the user typed in, immediately after the user typed it in. Of course it will match. Your code is:

Get password from user
Create hash from that password
Use password-verify on those two values

They will always match, because they are the same thing.

For your login code, there is no need to use password_hash() at all. You get the password from the form, you read the hashed password from the database, you call password_verify() using those two variables. password_hash() is only for when you are generating the hashed password to store.

(Actually, now I read it properly, your new code is:

$username = $_POST["username"];
$password = $_POST["password"];

$hash_pass = password_hash($member_password, PASSWORD_BCRYPT);

and where does $member_password come from? You haven’t defined that anywhere. But no matter, because there’s no need to be hashing anything here.)

@droopsnoot

ETA - when you changed the code to use password_hash() to store the password, did you expand the column to make it long enough?

Asking this again as I can’t see that you answered it. If you read the documentation on password_hash() it tells you what size of hash string the function returns, and makes some recommendations on what column size you should use. I only wonder whether this is the issue because you were originally (I think, unless I’ve confused you with another poster) storing the passwords in plain text, and might only have allowed enough size for that text.

[vexdarkness]

So how do I change it to compare what the user entered to what’s stored in the DB? I tinkered with this for quite a bit, and still couldn’t get it to work. I also discovered that password_verify automatically creates the hash from the first variable input so I removed my “hash_pass” code and simply compared them. Not sure how to specify that I used BCRYPT tho. And besides, it still didn’t work. I even hashed the inputted password and directly compared that to the DB one using “if xxx = xxx” and nothing.

I think my problem is I’m not correctly querying the database. When I used password_verify to check user input ($password) to what’s in the DB ($member _password), it doesn’t know what I’m talking about because I didn’t reference the exact row, etc…

Comments?

P.S. Yes the column has enough space.

[droopsnoot]

@vexdarkness

I think my problem is I’m not correctly querying the database. When I used password_verify to check user input ($password) to what’s in the DB ($member _password), it doesn’t know what I’m talking about because I didn’t reference the exact row, etc…

In your code above, you are referencing $member_password in your call to password_hash() before you retrieve the members information from the database. So even though you know now that you need to use password_verify() instead, have you moved the code to after you retrieve the members details? And have you changed it so that you either assign some kind of value to $member_password, or use the variable name from your $user array?

Perhaps you could show the updated code.

[droopsnoot]

@vexdarkness

Not sure how to specify that I used BCRYPT tho.

You could always check the documentation:

@Documentation for password_hash

The used algorithm, cost and salt are returned as part of the hash. Therefore, all information that’s needed to verify the hash is included in it.

[vexdarkness]

Here is the updated code (it loads, but when I try to logon, I get a blank page):

if (!empty($_POST["login"])) {
  $select = "SELECT password FROM members WHERE password = ?;";
  $stmt = $db_handle->prepare($select);

  $stmt->bind_param("s", $_POST['password']);
  $stmt->execute();
  $result = $stmt->get_result();
  $row = $result->fetch_assoc();

 // if($row && password_verify($row['password'], $_POST['password'])) {

  if ($row && password_verify($_POST['$password'], ($row['member_password']))) {
    $user = $auth->getMemberByUsername($name);
    //if password_verify($_POST['password'], $row['member_password'])) {
    // if (password_verify($password, $member_password)) {
        $isAuthenticated = true;
        $_SESSION["member_id"] = $user[0]["member_id"];
        
        // Set Auth Cookies if 'Remember Me' checked
        if (! empty($_POST["remember"])) {
            setcookie("member_login", $username, $cookie_expiration_time);
            
            $random_password = $util->getToken(16);
            setcookie("random_password", $random_password, $cookie_expiration_time);
            
            $random_selector = $util->getToken(32);
            setcookie("random_selector", $random_selector, $cookie_expiration_time);
            
            $random_password_hash = password_hash($random_password, PASSWORD_DEFAULT);
            $random_selector_hash = password_hash($random_selector, PASSWORD_DEFAULT);
            
            $expiry_date = date("Y-m-d H:i:s", $cookie_expiration_time);
            
            // mark existing token as expired
            $userToken = $auth->getTokenByUsername($username, 0);
            if (! empty($userToken[0]["id"])) {
                $auth->markAsExpired($userToken[0]["id"]);
            }
            // Insert new token
            $auth->insertToken($username, $random_password_hash, $random_selector_hash, $expiry_date);
	
        } else {
            $util->clearAuthCookie();
        }            
        $util->redirect("main.php");
    } else {
        $message = "Invalid Login";
  }
}

[droopsnoot]

Well, this won’t work, for a start:

  $select = "SELECT password FROM members WHERE password = ?;";
  $stmt = $db_handle->prepare($select);

  $stmt->bind_param("s", $_POST['password']);

You’re trying to retrieve the user by their plain-text password, even though you now store the password as a hash. Normally you’d retrieve the user by their username, or their email address, whatever it is it has to be something you can store in plain text.

And then a bit later

 if ($row && password_verify($_POST['$password'], ($row['member_password']))) {

Is your password stored in member_password or password? If it’s in member_password your first query is probably giving an error message because of the invalid column name. You could trap that by looking at what comes back in $result instead of just using it and presuming it’s worked.

The blank screen is hard to explain, it depends on what code you have after the code you posted. If the user isn’t found, you assign a value to $message but you don’t show where you output that.

[vexdarkness]

@droopsnoot

You’re trying to retrieve the user by their plain-text password, even though you now store the password as a hash. Normally you’d retrieve the user by their username, or their email address, whatever it is it has to be something you can store in plain text.

Okay gotcha. So then how do I “retrieve” the user by username, and then compare passwords (which is the problem I’m trying to fix)?

@droopsnoot

Is your password stored in member_password or password ? If it’s in member_password your first query is probably giving an error message because of the invalid column name. You could trap that by looking at what comes back in $result instead of just using it and presuming it’s worked.

The blank screen is hard to explain, it depends on what code you have after the code you posted. If the user isn’t found, you assign a value to $message but you don’t show where you output that.

The password is stored in the DB under members > member_password. There is no more PHP after that code. It goes right to the HTML portion.

I’m at a complete loss. I even dabbled back with md5 to encrypt the password and couldn’t that to work either, and that was from my original “working” code, before I tried to integrate with the “remember me” code.

[spaceshiptrooper]

@vexdarkness

So then how do I “retrieve” the user by username, and then compare passwords (which is the problem I’m trying to fix)?

Instead of selecting the password in your WHERE clause, you’d just replace it with the username column. Then you’d just have to get the password column and compare it with the unhashed password.

@vexdarkness

I’m at a complete loss. I even dabbled back with md5 to encrypt the password and couldn’t that to work either, and that was from my original “working” code, before I tried to integrate with the “remember me” code.

You shouldn’t have been using MD5. It’s completely different and it’s much much more weaker. Since you’re trying to get on the right foot, here is something to keep you reminded of how these hashing functions work.

password_hash() takes in 3 arguments, but the last one is optional. First argument is always the unhashed password. Second argument is always the password algorithm. This algorithm is always a constant. The third argument is an array and it’s the cost. The higher the cost, the more resource this function takes, but the harder it’ll take a hacker to crack those passwords. The lower the cost, the less resource it uses and the weaker the password gets. It’ll depend on you to determine which cost you’ll need. Or you can just keep it at the default which is 10. This function should return a string data type starting with $2y.

password_verify() takes in only 2 arguments. First argument is always the unhashed password. Second argument is always the password from the database. This function should return a Boolean data type.

To figure out which to use, it’s not that hard. You have to just remember that if you’re inserting or updating into the database, you always want to run the password_hash() function. If you’re trying to figure out if the password matches, you run the password_verify() function.