magniloquent
magniloquent copied to clipboard
autoHash - Doesn't Hash Password When Set To Same Plaintext
I noticed an issue where autoHash didn't appear to be hashing a User's password and it ended up being stored in plaintext in my database. I did some investigation and determined that the user tried to change their password to the same plaintext string. Here's a simple test case:
$user = User::find(1);
// This user currently has a different password, that is currently hashed in the database
var_dump($user->password);
// Then we change the password
$user->password = '12345';
$user->save();
// The password has been hashed and saved successfully
var_dump($user->password);
var_dump($user->errors());
// We then try to change the password again, but to the same plaintext string
$user->password = '12345';
$user->save();
// This time, the password is not autohashed and is stored in plaintext
// There are still no errors however
var_dump($user->password);
var_dump($user->errors());
This results in output something like:
string '$2y$10$txasGsFjds0Er/MXzwgi/ue0/FO02txUFMpxm9eLILJveYLYpPsD.' (length=60)
string '$2y$10$zSc9EKVoqbtnlcsCujdkfO8fFs3IJonRU5saUBbD5.2C9WdPiTteK' (length=60)
object(Illuminate\Support\MessageBag)[137]
protected 'messages' =>
array (size=0)
empty
protected 'format' => string ':message' (length=8)
string '12345' (length=5)
object(Illuminate\Support\MessageBag)[137]
protected 'messages' =>
array (size=0)
empty
protected 'format' => string ':message' (length=8)
I think this is due to having a combination of the isDirty check along with the Hash::check that was introduced with pull request #40. Wouldn't it be better to always hash the password if it is considered "dirty"? Is there another scenario or use case I'm missing?
For a work around right now, I've added a Hash::check in my controller that prevents the user from changing their password to the same string to prevent this behavior.
I'm upgrading because of this bug. ( I wanted to upgrade anyways)