Login system with password reset sent to e-mail using PHP and PDO

Question

I really like to use PDO because it's simple and easy to make a safe query, i prepared all queries and used placeholders, i think it's safe but i'm not sure at all, and I'm thinking if i used trim() the right way. What you guys think? Any doubt please ask me on the comments section.

login.php

<?php

if($_SERVER['REQUEST_METHOD'] == 'POST'){ 

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

try{
  $Query = "SELECT * FROM users WHERE email = :email";
  $statement = $conn->prepare($Query);
  $statement->bindValue(':email', $email);
  $statement->execute();
  $user = $statement->fetch(PDO::FETCH_ASSOC);    
  $RowCount = $statement->rowCount();
} catch (PDOerrorInfo $e){}

  if( $RowCount == 0 ){
   // User doesn't exist
    $_SESSION['message'] = "Não existe um usuário com este e-mail.";
    header("location: error.php");

  } else{ // User exists

      if( password_verify($_POST['password'], $user['password'])){
        $_SESSION['email']  = $user['email'];
        $_SESSION['first_name'] = $user['first_name'];
        $_SESSION['last_name'] = $user['last_name'];
        $_SESSION['username'] = $user['username'];
        $_SESSION['img'] = $user['img'];
        $_SESSION['active'] = $user['active'];
        $_SESSION['logged_in'] = true;
        header("location: ../index.php");
      } else {
          $_SESSION['message'] = "Senha incorreta, tente novamente!";
          header("location: error.php");
        }      
    }  
}

register.php

<?php

$img = rand(0,30);
$first_name = trim($_POST['first_name']);
$last_name = trim($_POST['last_name']);
$username = trim($_POST['username']);
$email = trim($_POST['email']);
$password = password_hash($_POST['password'], PASSWORD_BCRYPT);
$hash = md5( rand(0,1000) );

// Check if user with that email already exists
$result = $conn->prepare("SELECT * FROM users WHERE email = :email");
$result->bindParam(':email', $email);
$result->execute();
$RowCount = $result->rowCount();

if ( $RowCount > 0 ) { 
    $_SESSION['message'] = 'Já existe um usuário com este e-mail!';
    header("location: error.php");   
} else {
    $sql = "INSERT INTO users (first_name, last_name, username, img, email, password, hash) VALUES (:first_name, :last_name, :username, :img, :email, :password, :hash)";
    $sql = $conn->prepare($sql);
    $sql->bindParam(':first_name', $first_name);
    $sql->bindParam(':last_name', $last_name);
    $sql->bindParam(':username', $username);
    $sql->bindParam(':img', $img);
    $sql->bindParam(':email', $email);
    $sql->bindParam(':password', $password);
    $sql->bindParam(':hash', $hash);
    $sql->execute();

}

forgot.php

<?php 
require 'db.php';
session_start();
if ( $_SERVER['REQUEST_METHOD'] == 'POST' ) 
{   
    $email = trim($_POST['email']);
    $result = $conn->prepare("SELECT * FROM users WHERE email = :email");
    $result->bindValue(':email', $email);
    $result->execute();
    $RowCount = $result->rowCount();

    if ( $RowCount == 0 )
    { 
        $_SESSION['message'] = "Não existe um usuário com este e-mail.";
        header("location: error.php");
    }
    else {
        $user = $result->fetch(PDO::FETCH_ASSOC);      
        $email = $user['email'];
        $hash = $user['hash'];
        $first_name = $user['first_name'];

        $_SESSION['message'] = "<p>Link de confirmação enviado para <span>$email</span>"
        . " clique no link para resetar a sua senha!</p>";

        $to      = $email;
        $subject = 'Resetar senha - AnimeFire';
        $message_body = '
        Olá '.$first_name.' :),

        Você solicitou o resete de sua senha.

        Clique no link para resetar:

        https://localhost/login-system/reset.php?email='.$email.'&hash='.$hash;  

        mail($to, $subject, $message_body);

        header("location: success.php");
  }
}
?>

reset.php

<?php
require 'db.php';
session_start();
if( isset($_GET['email']) && !empty($_GET['email']) AND isset($_GET['hash']) && !empty($_GET['hash']) )
{
    $email = trim($_GET['email']); 
    $hash = trim($_GET['hash']); 

    $result = $conn->prepared("SELECT * FROM users WHERE email = :email AND hash = :hash");
    $result->bindValue(':email', $email);
    $result->bindValue(':hash', $hash);
    $result->execute();
    $RowCount = $result->rowCount();

    if ( $RowCount == 0 )
    { 
        $_SESSION['message'] = "A conta já foi verificada ou o URL é inválido!";
        header("location: error.php");
    }
}else {
    $_SESSION['message'] = "A verificação falhou :/ tente novamente!";
    header("location: error.php");  
}
?>

reset_password.php

<?php
require 'db.php';
session_start();
if ($_SERVER['REQUEST_METHOD'] == 'POST') { 
    if ( $_POST['newpassword'] == $_POST['confirmpassword'] ) { 

        $new_password = password_hash($_POST['newpassword'], PASSWORD_BCRYPT);        
        $email = trim($_GET['email']); 
        $hash = trim($_GET['hash']); 

        $sql = $conn->prepare("UPDATE users SET password = :new_password, hash = :hash WHERE email = :email");
        $sql->bindValue(':new_password', $new_password);
        $sql->bindValue(':hash', $hash);
        $sql->bindValue(':email', $email);
        $sql->execute();

        if ( $conn->prepare($sql) ) {

        $_SESSION['message'] = "Sua senha foi resetada com sucesso ^^";
        header("location: success.php");    

        }
    } else {
        $_SESSION['message'] = "As senhas não estão iguais, tente novamente!";
        header("location: error.php");    
    }
}
?>

profile.php

<?php
if (empty($_SESSION['email'])) {
  $_SESSION['message'] = "Você precisa estar logado para vizualizar esta página!";
  header("location: error.php");    
}
else {
  $first_name = $_SESSION['first_name'];
  $last_name = $_SESSION['last_name'];
  $email = $_SESSION['email'];
  $username = $_SESSION['username'];
  $img = $_SESSION['img'];
}
?>
<img src="img/avatar/<?= $img ?>.jpg">
<h3 ><?= $username ?></h3>   
<h6 >Nome: <?= $first_name.' '.$last_name ?></h6>
<h6 >Email: <?= $email ?></h6>

Answer 1

The trim() function usage is OK.

The biggest problem here is a hash security. A permanent md5( rand(0,1000) ); hash is anything but security. It's so easily guessable that you can count it doesn't exist at all.

Password reminder hashes are generated per request, each time anew. And it should be something less predictable, http://php.net/manual/en/function.random-bytes.php for example.

Other issues are so common that they made into my list of Top 10 PHP delusions:

• Empty try..catch is a big no-no. Whatever your goal is, there are proper ways to achieve it.
• If (isset($var) && !empty($var)) is essentially a tautology. You can and should use only empty() in this case.
• You don't really need to call rowCount(). It does no harm if you do, but there is no reason. Better fetch the selected data, it can serve you as good as the number of rows.
  • besides, it makes the code in login.php a little bit more complicated that it could be. See my canonical example, Authenticating a user using PDO and password_verify()

There is also a strange code snippet in reset_password.php, checking the result of prepare to test the success of the previous query makes no sense. Besides, given your error reporting is right, there is no need to check for the success at all, just do your redirect right away:

  $sql->bindValue(':email', $email);
  $sql->execute();
  $_SESSION['message'] = "Sua senha foi resetada com sucesso ^^";
  header("location: success.php");    
  exit;

it is also a very good habit to always add exit after every Location header call in your code, as a header itself doesn't mean thet the code execution has been stopped.