My lost password function

I'm trying to create a lost password function on my site. When a user enter their email it updates 2 columns in the users table with that email called: new_password_key and new_password_requested (int, using PHP time()), I then send them a link with that password key to a page where they may change their password.

It seems to work, and if they don't visit the link within 24 hours it's considered invalid and they have to request a new one.

Is it OK? Safe? Is there something else I should think of?

This is the https://mysite.com/password/key_goes_here page:

$value = isset($_GET['key']) ? $_GET['key'] : null;
$query = $db->prepare("SELECT new_password_key, new_password_requested FROM users WHERE new_password_key = ?");
$query->execute(array($value));
$row = $query->fetch();

if (!$row) {
    // not found
    header("Location: /password");
    die;
}

// if this key is older than 24 hours its invalid (expired)
if ($row['new_password_requested'] prepare("UPDATE users SET new_password_key = NULL, new_password_requested = 0 WHERE new_password_key = ?");
    $query->execute(array($value));
    header("Refresh: 3;url='/password'");
    return false;
} 

if (count($_POST) > 0) {

    $new_password1 = trim($_POST['new_password1']);
    $new_password2 = trim($_POST['new_password2']);

    if (strlen($new_password1) prepare("UPDATE users SET password = ?, new_password_key = NULL WHERE new_password_key = ?");
        $query->execute(array($hash, $row['new_password_key']));

    }

}

This is the form where you enter your email to get a key:

```
if (count($_POST) > 0) {

$email = trim($_POST['email']);

$query = $db->prepare("SELECT email, new_password_key, new_password_requested FROM users WHERE email = ?");
$query->execute(array($email));
$row = $query->fetch();

if (empty($email))
$error = "this field can not be empty";
else if (!filter_var($email, FILTER_VALIDATE_EMAIL))
$error = "not a valid email address";
else if (!empty($row['new_password_key

This seems quite fine. It's good that you are using prepared statements for all database queries.

What bugs me the most is that the users table is reused for multiple purposes:

• Store essential user information such as email and password

• Store information about tracking reset password requests

It would be better to use another table for the 2nd purpose.
The reason is that the users table is an essential piece of your site and you wouldn't want anything to happen to it by accident.
So the less you touch that table, the better.
Besides, for most users the new_password_key and new_password_requested fields will be blank / unused most of the time.

The variable names are sometimes not so great. For example:

$value = isset($_GET['key']) ? $_GET['key'] : null;

$value is a key, so I suggest to name it accordingly.

One last small thing, I recommend to use braces with all if, else if statements.

StackExchange Code Review Q#83015, answer score: 3