PHP Password Rehashing in User Class (MySQL)

Question

So I am dealing with merging in functionality for rehashing to upgrade users to have bcrypt passwords, into a existing class I found and have set up quite successfully, its wonderful.

However, this class lacks rehashing check, which is terrible for legacy passwords on existing user databases. We need to handle SHA1 passwords! We use SHA1 + Salt, so I hope this is possible to convert.

Im using this class found here:

https://alexwebdevelop.com/user-authentication/

So using this class, I have added the following public function:

public function authenticate($username, $password)
{
    /* Global $pdo object */
    global $pdo;

    // Database lookup
    $stmt = $pdo->prepare("SELECT id, password, legacy_password FROM users WHERE username = ?");
    $stmt->execute([$username]);
    $stored = $stmt->fetch(PDO::FETCH_ASSOC);
    if (!$stored) {

        // No such user, throw an exception
        throw new Exception('Invalid user.');
    }
    if ($stored['legacy_password']) {

        // This is the legacy password upgrade code
        if (password_verify(sha1($password), $stored['password'])) {
            $newHash = password_hash($password, PASSWORD_DEFAULT);
            $stmt    = $pdo->prepare("UPDATE users SET password = ?, legacy_password = FALSE WHERE id = ?");
            $stmt->execute([$newhash, $stored['id']]);

            // Return the user ID (integer)
            return $stored['id'];
        }
    } elseif (password_verify($password, $stored['password'])) {

        // This is the general purpose upgrade code e.g. if a future version of PHP upgrades to Argon2
        if (password_needs_rehash($stored['password'], PASSWORD_DEFAULT)) {
            $newhash = password_hash($password, PASSWORD_BCRYPT);
            $stmt    = $pdo->prepare("UPDATE users SET password = ? WHERE id = ?");
            $stmt->execute([$newhash, $stored['id']]);
        }

        // Return the user ID (integer)
        return $stored['id'];
    }

    // When all else fails, throw an exception
    throw new Exception('Rehashing failed.');
}

Now inside the login() function of the class, I have replaced

public function login($name, $passwd)
    {
...
    if (is_array($row)) {

        if (password_verify($passwd, $row['password'])) {
                        /* Authentication succeeded. Set the class properties (id and name) */
                        $this->id            = intval($row['id'], 10);
                        $this->name          = $name;
                        $this->authenticated = TRUE;

                        /* Register the current Sessions on the database */
                        $this->registerLoginSession();

                        /* Finally, Return TRUE */
                        return TRUE;
                    }
    }
}

With this:

public function login($name, $passwd)
    {
...
        if (is_array($row)) {

            $userid = $this->authenticate($name, $row['password']);

            if (password_verify($passwd, $row['password'])) {
                /* Authentication succeeded. Set the class properties (id and name) */
                $this->id            = intval($userid);
                $this->name          = $name;
                $this->authenticated = TRUE;

                /* Register the current Sessions on the database */
                $this->registerLoginSession();

                /* Finally, Return TRUE */
                return TRUE;
            }

        }
}

And so it is supposed to return the hand back the ID after check / rehashing. So it finds me as a user, as tested. Good.. so now all authenticate() does is throw exception error of failure. I can't figure out how to get error messages out of this.

This seems like this the exact thing to do with this ID, what am I doing wrong?

This point of this: User logs in with SHA1 (salted) password in form, script rehashes password, and user logs in like nothing happened.

authenticate() conversion function I'm using:

https://paragonie.com/blog/2016/02/how-safely-store-password-in-2016#legacy-hashes

Answer 1

Im so sorry! I have learned from the suggestions here and I appreciate all the help!

So I solved this myself. What I did was remove authenticate() function, and instead tackled this directly based on the feedback comments (I couldn't agree more).

I replaced the last code block in the post, with this:

if (is_array($row)) {

    if (password_needs_rehash($row['password'], PASSWORD_DEFAULT)) {

        $newhash = password_hash($passwd, PASSWORD_BCRYPT);
        $stmt    = $pdo->prepare("UPDATE users SET password = ? WHERE id = ?");
        $stmt->execute([$newhash, $row['id']]);

    }
    if (password_verify($passwd, $row['password'])) {

        /* Authentication succeeded. Set the class properties (id and name) */
        $this->id            = intval($row['id'], 10);
        $this->name          = $name;
        $this->authenticated = TRUE;

        /* Register the current Sessions on the database */
        $this->registerLoginSession();

        /* Finally, Return TRUE */
        return TRUE;
    }

}

And users passwords are rehashing, and logging in!