From ce8e63e050e173427f4b2c41b13b0b23a47e4497 Mon Sep 17 00:00:00 2001 From: Bastien Philippe Date: Thu, 12 Oct 2023 21:45:19 +0200 Subject: [PATCH 1/3] Use Hash::needsRehash in HasAttributes::castAttributeAsHashedString instead of Hash::isHashed --- .../Database/Eloquent/Concerns/HasAttributes.php | 2 +- .../Database/EloquentModelHashedCastingTest.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php b/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php index 4ef0f5314062..ba8089e68451 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php +++ b/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php @@ -1316,7 +1316,7 @@ public static function encryptUsing($encrypter) */ protected function castAttributeAsHashedString($key, $value) { - return $value !== null && ! Hash::isHashed($value) ? Hash::make($value) : $value; + return $value !== null && Hash::needsRehash($value) ? Hash::make($value) : $value; } /** diff --git a/tests/Integration/Database/EloquentModelHashedCastingTest.php b/tests/Integration/Database/EloquentModelHashedCastingTest.php index df3402f8e46f..85b700c07f59 100644 --- a/tests/Integration/Database/EloquentModelHashedCastingTest.php +++ b/tests/Integration/Database/EloquentModelHashedCastingTest.php @@ -30,9 +30,9 @@ protected function defineDatabaseMigrationsAfterDatabaseRefreshed() public function testHashed() { - $this->hasher->expects('isHashed') + $this->hasher->expects('needsRehash') ->with('this is a password') - ->andReturnFalse(); + ->andReturnTrue(); $this->hasher->expects('make') ->with('this is a password') @@ -51,9 +51,9 @@ public function testHashed() public function testNotHashedIfAlreadyHashed() { - $this->hasher->expects('isHashed') + $this->hasher->expects('needsRehash') ->with('already-hashed-password') - ->andReturnTrue(); + ->andReturnFalse(); $subject = HashedCast::create([ 'password' => 'already-hashed-password', From a53227dacb3b8d139dcc8ea455e9efd78576b03f Mon Sep 17 00:00:00 2001 From: Bastien Philippe Date: Tue, 17 Oct 2023 16:36:00 +0200 Subject: [PATCH 2/3] Introduce Hasher::isAcceptable --- src/Illuminate/Contracts/Hashing/Hasher.php | 8 ++++++++ src/Illuminate/Hashing/AbstractHasher.php | 11 +++++++++++ src/Illuminate/Hashing/ArgonHasher.php | 15 +++++++++++++++ src/Illuminate/Hashing/BcryptHasher.php | 13 +++++++++++++ src/Illuminate/Hashing/HashManager.php | 11 +++++++++++ src/Illuminate/Support/Facades/Hash.php | 1 + tests/Hashing/HasherTest.php | 16 ++++++++++++++++ 7 files changed, 75 insertions(+) diff --git a/src/Illuminate/Contracts/Hashing/Hasher.php b/src/Illuminate/Contracts/Hashing/Hasher.php index b5e4d4c8a823..aa055cfce08d 100644 --- a/src/Illuminate/Contracts/Hashing/Hasher.php +++ b/src/Illuminate/Contracts/Hashing/Hasher.php @@ -39,4 +39,12 @@ public function check($value, $hashedValue, array $options = []); * @return bool */ public function needsRehash($hashedValue, array $options = []); + + /** + * Check if the given hash is acceptable for the hasher. + * + * @param string $hashedValue + * @return bool + */ + public function isAcceptable($hashedValue); } diff --git a/src/Illuminate/Hashing/AbstractHasher.php b/src/Illuminate/Hashing/AbstractHasher.php index f10371290b1d..3c74ae7bed2f 100644 --- a/src/Illuminate/Hashing/AbstractHasher.php +++ b/src/Illuminate/Hashing/AbstractHasher.php @@ -31,4 +31,15 @@ public function check($value, $hashedValue, array $options = []) return password_verify($value, $hashedValue); } + + /** + * Check if the given hash is acceptable for the hasher. + * + * @param string $hashedValue + * @return bool + */ + public function isAcceptable($hashedValue) + { + return method_exists($this, 'needsRehash') && ! $this->needsRehash($hashedValue); + } } diff --git a/src/Illuminate/Hashing/ArgonHasher.php b/src/Illuminate/Hashing/ArgonHasher.php index b999257f4b52..c062269429da 100644 --- a/src/Illuminate/Hashing/ArgonHasher.php +++ b/src/Illuminate/Hashing/ArgonHasher.php @@ -144,6 +144,21 @@ public function setTime(int $time) return $this; } + /** + * Check if the given hash is acceptable for the hasher. + * + * @param string $hashedValue + * @return bool + */ + public function isAcceptable($hashedValue) + { + $info = $this->info($hashedValue); + + return $info['algoName'] === $this->algorithm() + && $info['options']['memory_cost'] / $info['options']['threads'] <= 4 * 65536 + && $info['options']['threads'] <= 16; + } + /** * Set the default password threads factor. * diff --git a/src/Illuminate/Hashing/BcryptHasher.php b/src/Illuminate/Hashing/BcryptHasher.php index f74edab88805..51191cebc642 100755 --- a/src/Illuminate/Hashing/BcryptHasher.php +++ b/src/Illuminate/Hashing/BcryptHasher.php @@ -88,6 +88,19 @@ public function needsRehash($hashedValue, array $options = []) ]); } + /** + * Check if the given hash is acceptable for the hasher. + * + * @param string $hashedValue + * @return bool + */ + public function isAcceptable($hashedValue) + { + $info = $this->info($hashedValue); + + return $info['algoName'] === 'bcrypt' && $info['options']['cost'] <= 16; + } + /** * Set the default password work factor. * diff --git a/src/Illuminate/Hashing/HashManager.php b/src/Illuminate/Hashing/HashManager.php index 564411e4df54..05862ecf2fc9 100644 --- a/src/Illuminate/Hashing/HashManager.php +++ b/src/Illuminate/Hashing/HashManager.php @@ -88,6 +88,17 @@ public function needsRehash($hashedValue, array $options = []) return $this->driver()->needsRehash($hashedValue, $options); } + /** + * Check if the given hash is acceptable for the hasher. + * + * @param string $hashedValue + * @return bool + */ + public function isAcceptable($hashedValue) + { + return $this->driver()->isAcceptable($hashedValue); + } + /** * Determine if a given string is already hashed. * diff --git a/src/Illuminate/Support/Facades/Hash.php b/src/Illuminate/Support/Facades/Hash.php index 280585d6e374..2a54a8775b75 100755 --- a/src/Illuminate/Support/Facades/Hash.php +++ b/src/Illuminate/Support/Facades/Hash.php @@ -10,6 +10,7 @@ * @method static string make(string $value, array $options = []) * @method static bool check(string $value, string $hashedValue, array $options = []) * @method static bool needsRehash(string $hashedValue, array $options = []) + * @method static bool isAcceptable(string $hashedValue) * @method static bool isHashed(string $value) * @method static string getDefaultDriver() * @method static mixed driver(string|null $driver = null) diff --git a/tests/Hashing/HasherTest.php b/tests/Hashing/HasherTest.php index 3559cc0303d3..76f14e07711c 100755 --- a/tests/Hashing/HasherTest.php +++ b/tests/Hashing/HasherTest.php @@ -56,6 +56,10 @@ public function testBasicBcryptHashing() $this->assertSame('bcrypt', password_get_info($value)['algoName']); $this->assertGreaterThanOrEqual(12, password_get_info($value)['options']['cost']); $this->assertTrue($this->hashManager->isHashed($value)); + $this->assertTrue($hasher->isAcceptable($value)); + $this->assertFalse($hasher->isAcceptable('password')); + $this->assertFalse($hasher->isAcceptable('$2y$17$1iPpw8cxiw6.ijzD2Ry1mOvBMM2kPu6wayaIXWLMG5fhFX5ejCEa6')); + $this->assertFalse($hasher->isAcceptable('$argon2i$v=19$m=65536,t=4,p=1$eE4vbkhJTm54M0k4OU1LTw$C9JCrLeNkNHI1jWx3pBqpK2bTgFrtcVcIfARjCN0218')); } public function testBasicArgon2iHashing() @@ -68,6 +72,12 @@ public function testBasicArgon2iHashing() $this->assertTrue($hasher->needsRehash($value, ['threads' => 1])); $this->assertSame('argon2i', password_get_info($value)['algoName']); $this->assertTrue($this->hashManager->isHashed($value)); + $this->assertTrue($hasher->isAcceptable($value)); + $this->assertTrue($hasher->isAcceptable('$argon2i$v=19$m=4194304,t=4,p=16$c01ieWxxZWozSmtHTzd5Vw$y9hJhd9Ip28ZFbh4BEVpPYSA6n017UIBdPcuTVna4hw')); + $this->assertFalse($hasher->isAcceptable('password')); + $this->assertFalse($hasher->isAcceptable('$argon2i$v=19$m=4194304,t=4,p=8$Ri5lRGt5VFMvMEtiLkYxQg$sPuFc8V0SKB1gmOJXmqcXscTZ8Awdkihf7m0Y/bskSg')); + $this->assertFalse($hasher->isAcceptable('$argon2i$v=19$m=8388608,t=4,p=32$Z0JUVVFTMTBVRnZlRHhldQ$sQrSwO1zcTFOseS56GZOd27SR9c05YUXPK7Np+gJpv4')); + $this->assertFalse($hasher->isAcceptable('$2y$10$PCXl4nmz2z8vckcBFi2AQObDvYOIlNa99REfp0dQN/Hq7Lc1wA5qC')); } public function testBasicArgon2idHashing() @@ -80,6 +90,12 @@ public function testBasicArgon2idHashing() $this->assertTrue($hasher->needsRehash($value, ['threads' => 1])); $this->assertSame('argon2id', password_get_info($value)['algoName']); $this->assertTrue($this->hashManager->isHashed($value)); + $this->assertTrue($hasher->isAcceptable($value)); + $this->assertTrue($hasher->isAcceptable('$argon2id$v=19$m=4194304,t=4,p=16$WmJySGpROWJuMUJxZXQ5Rw$u96pRIoI4xsj+OfFoluc+iEng3jkDfuTFDIJOYbRml0')); + $this->assertFalse($hasher->isAcceptable('password')); + $this->assertFalse($hasher->isAcceptable('$argon2id$v=19$m=4194304,t=4,p=8$VmZWVE5Uc2xDbklQVlhBWA$59KcqVqTfDt4WjoFIQkFIuXQEZBuRN7+G/YR7BDb9i8')); + $this->assertFalse($hasher->isAcceptable('$argon2id$v=19$m=8388608,t=4,p=32$dVFMcDB4WWkvRU41bGtDMQ$q4Y/26s5RVLn3tInzMgh/jUKeoOj/BXINARKQsvvhC4')); + $this->assertFalse($hasher->isAcceptable('$2y$10$PCXl4nmz2z8vckcBFi2AQObDvYOIlNa99REfp0dQN/Hq7Lc1wA5qC')); } /** From e52fc3fa6e38af6af9e46397029f54960f3955eb Mon Sep 17 00:00:00 2001 From: Bastien Philippe Date: Tue, 17 Oct 2023 16:45:14 +0200 Subject: [PATCH 3/3] Use Hash::isAcceptable to verify if hashed cast should hash the value --- .../Database/Eloquent/Concerns/HasAttributes.php | 2 +- .../Database/EloquentModelHashedCastingTest.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php b/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php index ba8089e68451..7d59a8e44a6b 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php +++ b/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php @@ -1316,7 +1316,7 @@ public static function encryptUsing($encrypter) */ protected function castAttributeAsHashedString($key, $value) { - return $value !== null && Hash::needsRehash($value) ? Hash::make($value) : $value; + return $value !== null && ! Hash::isAcceptable($value) ? Hash::make($value) : $value; } /** diff --git a/tests/Integration/Database/EloquentModelHashedCastingTest.php b/tests/Integration/Database/EloquentModelHashedCastingTest.php index 85b700c07f59..1f21a8374399 100644 --- a/tests/Integration/Database/EloquentModelHashedCastingTest.php +++ b/tests/Integration/Database/EloquentModelHashedCastingTest.php @@ -30,9 +30,9 @@ protected function defineDatabaseMigrationsAfterDatabaseRefreshed() public function testHashed() { - $this->hasher->expects('needsRehash') + $this->hasher->expects('isAcceptable') ->with('this is a password') - ->andReturnTrue(); + ->andReturnFalse(); $this->hasher->expects('make') ->with('this is a password') @@ -49,11 +49,11 @@ public function testHashed() ]); } - public function testNotHashedIfAlreadyHashed() + public function testNotHashedIfAlreadyCorrectlyHashed() { - $this->hasher->expects('needsRehash') + $this->hasher->expects('isAcceptable') ->with('already-hashed-password') - ->andReturnFalse(); + ->andReturnTrue(); $subject = HashedCast::create([ 'password' => 'already-hashed-password',