PHP password_verify not matching hash

Question

So I have two functions for registering and logging in. Registering works fine, the user table is populated, the hash is stored in the user_pass column etc. When logging in, I keep getting the "Wrong Details" error message. It seems the password_verify isn't matching the hash with the inputted password. Can you guys see anything wrong with my code? I'm scratching my head here....

public function register($uname,$umail,$upass)
{
    try
    {
        $new_password = password_hash($upass, PASSWORD_DEFAULT);

        $stmt = $this->conn->prepare("INSERT INTO users(user_name,user_email,user_pass) 
                                                   VALUES(:uname, :umail, :upass)");

        $stmt->bindparam(":uname", $uname);
        $stmt->bindparam(":umail", $umail);
        $stmt->bindparam(":upass", $new_password);                                        

        $stmt->execute();   

        return $stmt;   
    }
    catch(PDOException $e)
    {
        echo $e->getMessage();
    }               
}


public function doLogin($uname,$umail,$upass)
{
    try
    {
        $stmt = $this->conn->prepare("SELECT user_id, user_name, user_email, user_pass FROM users WHERE user_name=:uname OR user_email=:umail ");
        $stmt->execute(array(':uname'=>$uname, ':umail'=>$umail));
        $userRow=$stmt->fetch(PDO::FETCH_ASSOC);
        if($stmt->rowCount() == 1)
        {
            if(password_verify($upass, $userRow['user_pass']))
            {
                $_SESSION['user_session'] = $userRow['user_id'];
                return true;
            }
            else
            {
                return false;
            }
        }
    }
    catch(PDOException $e)
    {
        echo $e->getMessage();
    }
}

Have you verified the results of the hash are the same that is being stored in DB and returned? var_dump() the return results of password_hash() and the user_pass column and compare. — Crackertastic
– Crackertastic, Commented May 2, 2016 at 15:40
I var_dump the $new_password and it's string(60) "$2y$10$Ygo1AXF4shTgslmTbLqSeeS0fOAU0Yy3.hX/X4.BBjJEYrUfa9h9S" then I checked the database column and it's $2y$10$Ygo1AXF4shTgslmTbLqSeeS0fOAU0Yy3.hX/X4.BBjJEYrUfa9h9S — Jamie O'Hanlon
– Jamie O'Hanlon, Commented May 2, 2016 at 16:07
Have you verified, that $stmt->fetch() actually returns you the record you're thinking of? Doing var_dump($userRow) can be really helpful. — Andrey Tserkus
– Andrey Tserkus, Commented May 2, 2016 at 16:47

Jay Blanchard · Accepted Answer · 2016-05-02 16:18:00Z

2

rowCount() does not return the number of rows in a SELECT statement. There is no need to test to see if the query succeeded, you can move right to testing the password:

public function doLogin($uname,$umail,$upass)
{
    try
    {
        $stmt = $this->conn->prepare("SELECT user_id, user_name, user_email, user_pass FROM users WHERE user_name=:uname OR user_email=:umail ");
        $stmt->execute(array(':uname'=>$uname, ':umail'=>$umail));
        $userRow=$stmt->fetch(PDO::FETCH_ASSOC);

        if(password_verify($upass, $userRow['user_pass']))
        {
            $_SESSION['user_session'] = $userRow['user_id'];
            return true;
        }
        else
        {
            return false;
        }

    }
    catch(PDOException $e)
    {
        echo $e->getMessage();
    }
}

edited May 2, 2016 at 16:18

answered May 2, 2016 at 16:11

Jay Blanchard

34.5k17 gold badges82 silver badges130 bronze badges

Sign up to request clarification or add additional context in comments.

11 Comments

Andrey Tserkus Over a year ago

Actually it is not a safe operation do proceed without checking the data. First of all - there's no verification, that more than 1 record was retrieved - which is a confusing situation for the presented algorithm, as it is not clear which user the author needs to log in. Second - when no records are retrieved, then there will be access of FALSE['user_pass'] - which results in warning of error (except when framework is a crappy one an just silences that). There should be at lease if ($userRow) check present.

Jay Blanchard Over a year ago

It's kind of a mixed bag on whether this is safe or not @AndreyTserkus If no rows an error will be thrown. If the OP is making sure the table contains unique values, which he should, then there will never be more than one row in the users table with the ID in question. We cannot write all of their code for them, but thanks for the DV - I'll take what you have said under consideration when answering this question again down the road.

Andrey Tserkus Over a year ago

Well, the check there was on purpose. So the right suggestion would be to advice a right function to use, not to just delete the incorrect one.

Jay Blanchard Over a year ago

We'll have to agree to disagree @AndreyTserkus. None of the systems I have ever participated in ever performed that check save for the olden days of the MySQL API. It is an unnecessary check that does not need replacing as the query (against a properly setup database as I mentioned before) will either return results, or it will not. The OP has handled that with a return false;

Andrey Tserkus Over a year ago

It doesn't make sense to continue. I'm done with this thread.

|

Collectives™ on Stack Overflow

PHP password_verify not matching hash

1 Answer 1

11 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

11 Comments

Your Answer

Sign up or log in

Post as a guest

Related