Update of Classes for user registration and authentication

Old Post: Classes for user registration and authentication

This post is an update of the old post of mine. This is the code after suggestions were implemented in the review. Do you think I could make this even better?

Please criticize as thoroughly as possible for the smallest thing possible as it will be very useful for me.

```
checkLogin($username, $password) === true) {
$userMapper->fetch($username);
$userMapper->createSession($username);
return true;
} else {
return false;
}
}
public function handleUserCreation($username, $password, $rank, $name) {
if($userMapper->newUserCreation($username, $password, $rank, $name) === true) {
return true;
} else {
$errors['count'] = count($errors);
return $errors;
}
}
}

class User {
public $id;
public $username;
public $password;
public $rank;
public $name;

public function __construct($id, $username, $password, $rank, $name) {
$this->id = $id;
$this->username = $username;
$this->password = $password;
$this->rank = $rank;
$this->name = $name;
}
}

class UserMapper {
private $db;
public function __construct(DB $db) {
$this->db = $db;
}

public function checkLogin($username, $password) {
$query = $this->db->prepare("SELECT username, password FROM users WHERE username=? AND password=? LIMIT 1");
$password = $this->hash($password);
$query->bind_param('ss', $username, $password);
$query->execute();
$query->bind_result($username, $password);
$query->store_result();
$query->num_rows == 1 ? true : false;
$query->free_result();
$query->close();
}

public function fetch($username) {
$query = $this->db->prepare("SELECT id, username, password, rank, name FROM users WHERE username=? LIMIT 1");
$query->bin

Right now, you're using the same salt for every user. This pretty much defeats the purpose of using salt in the first place.

The point of using salt is to ensure that even if two users choose the same password, the hashed value will still be unique for each. So, to be useful, you compute the salt as a random string for each user, then store that random string in the user's record in the database. Although you don't particularly try to make it public, it's not necessary that you take any particular pains to keep it private either.

I believe it's been commented on already, but I'll be a little more direct: I would eliminate the check for maximum password length, or at least increase it to some "ridiculous" length (e.g., 4 kilobytes). If somebody wants to use a 20- or even 50-character password, let them. The only reason to limit the length is to prevent wasting a lot of time reading a bunch of data that clearly wasn't ever intended to be a password at all.

So, when you're going to create a new user, you want to compute a random string as the salt for this particular user. Then you store their user name and the hash of the salted password in the database (you don't store their raw password anywhere, at all).

Better still is to have most of this done on the client machine--the user enters their password, and code (usually in JavaScript) on their machine computes a salt and a hash. That is then transmitted to the server. The server stores the user name, salt and hash, but never sees the raw password at all.

When you're going to authenticate a user, the server should not plan on receiving the password in the clear either. What you typically want to do is use a challenge/response protocol. The server generates a (cryptographically strong) random number. It transmits that random number (and optionally that user's salt) to the user as the challenge.

The user enters their password, and code on their machine (typically in JavaScript) puts the password together with the salt, and hashes the result. They then use that to encrypt the random number using some agreed-upon algorithm (e.g., AES) and transmit the result back to the server.

The server does roughly the same thing: uses the stored value of the hashed/salted password to encrypt the random number. When the encrypted random number arrives back from the user, it compares those to see if the user entered the same password. Alternatively (and equivalently) it can decrypt the response from the user with their hashed/salted password, and see if the result equals the random number they originally sent out.

Why you want to do this:

• It means even if all the data on your servers is made public, you haven't revealed a single user's password.

• Since each user has their own unique salt, a dictionary attack on one user's password reveals nothing about any other user's password.

• A user's password is never transmitted over the network.

• The data for each login is unique, so if somebody snoops on a user logging in, they won't be able to play that back to log in as that user.

StackExchange Code Review Q#54945, answer score: 10