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";
 }
}
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Apr 4, 2016 at 21:58
\$\endgroup\$

1 Answer 1

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.
answered Apr 5, 2016 at 10:24
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.