2
\$\begingroup\$

This is my first time using password_hash and password_verify in PHP. Would this be the correct usage of password_hash and password_verify to log the user into the site? Is there anything I could do to make this function more secure?

/**
 * Initiates User Login
 * @param  mixed $data Post data
 * @return mixed Checks if the user wants to be remembered and runs the corresponding function ('loginRemember', 'loginNoRemember')
 */
public function initiateLogin ($data) {
    //Database Connection 
    $db = Database::getInstance();

    //Defining values for query
    $username = trim($data['login_username']);

    //Grab password from database based on username
    $query = $db->getConnection()->prepare("SELECT password FROM `users` WHERE username = :username");
    $query->execute(array(
        ':username' => $username
    ));

    //Query Results
    $queryResults = $query->fetch(PDO::FETCH_ASSOC);

    //Check if the password is correct
    if(password_verify($data['login_password'], $queryResults['password'])) {
        if (isset($data['login_remember_me'])) {
            $this->loginRemember($data);
        } else {
            $this->loginNoRemember($data);
        }
    } else {
        echo "Invalid password";
    }
}
\$\endgroup\$
2
\$\begingroup\$

Would this be the correct usage of password_hash and password_verify to log the user into the site

Yes. You didn't post your password_hash code, but you are using password_verify correctly, so if your code is working, everything is correct.

Is there anything I could do to make this function more secure?

Not really. You are using prepared statements as you should, and you are using the currently recommended hashing approach.

Misc

  • Your in-function comments are just repeating your code, which is not ideal. Readers of your code will notice this and start ignoring all your comments, even important ones.
  • I do understand the desire to structure your code via these kinds of comments, but it would be better to either just remove them, or to introduce additional functions, and to slightly change your method signature. You could for example add a function such as getUserPassword($connection, $username) (possibly in a User class, or possibly just a method getByUsername($connection, $username) in a user class, which returns a User object, on which you could call getPassword). That way, you don't need the comment, as the code is even clearer about what it does.
  • I would pass the username to your function, not some data array, from which only the username is needed.
  • If possible, functions shouldn't have side effects as it makes them harder to (re-)use. Instead of echoing, return true on successful login, and false otherwise. That way, the calling code can decide what to display. That way, you can have all the code that does display something in one place, making it easier to change.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.