From 24837a45da367ff7f60ad403a9126360c0378b2f Mon Sep 17 00:00:00 2001 From: Fabrizio Gortani Date: Sat, 23 Nov 2024 18:04:08 +0100 Subject: [PATCH] feat: apply hash on setter --- src/Models/PasswordHash.php | 6 ++-- src/PasswordHistory.php | 30 ++++++++-------- src/Traits/HasPasswordHistory.php | 25 ++++++------- tests/PasswordHistoryTest.php | 58 +++++++++++++++++++++++-------- tests/Pest.php | 15 ++++++++ 5 files changed, 90 insertions(+), 44 deletions(-) diff --git a/src/Models/PasswordHash.php b/src/Models/PasswordHash.php index 15f87df..7baf4be 100644 --- a/src/Models/PasswordHash.php +++ b/src/Models/PasswordHash.php @@ -2,6 +2,7 @@ namespace Beliven\PasswordHistory\Models; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\MorphTo; @@ -29,11 +30,12 @@ public function model(): MorphTo return $this->morphTo('model'); } - public function scopeByModel($query, Model $model) + public function scopeByModel(Builder $query, Model $model): Builder { $id = $model->getAttribute('id'); - return $query->where('model_type', $model::class) + return $query + ->where('model_type', $model::class) ->where('model_id', $id); } } diff --git a/src/PasswordHistory.php b/src/PasswordHistory.php index 522b4d6..df5ec53 100755 --- a/src/PasswordHistory.php +++ b/src/PasswordHistory.php @@ -12,27 +12,27 @@ class PasswordHistory { private function getModelPasswordHistoryCount(Model $model): int { - return PasswordHash::query()->byModel($model)->count(); + return PasswordHash::byModel($model)->count(); } private function removeModelOldestHash(Model $model): void { - PasswordHash::orderBy('created_at', 'asc') - ->byModel($model) + PasswordHash::byModel($model) + ->orderBy('created_at', 'asc') ->first() ->delete(); } - public function hasPasswordInHistory(Model $model, string $new_password): bool + public function hasPasswordInHistory(Model $model, string $newPassword): bool { - $list_of_passwords = PasswordHash::query() + $listOfPasswords = PasswordHash::query() ->whereHasMorph('model', $model::class) ->get(); - foreach ($list_of_passwords as $password) { + foreach ($listOfPasswords as $password) { $hash = $password->getAttribute('hash'); - if (Hash::check($new_password, $hash)) { + if (Hash::check($newPassword, $hash)) { return true; } } @@ -40,24 +40,24 @@ public function hasPasswordInHistory(Model $model, string $new_password): bool return false; } - public function addPasswordToHistory(Model $model, string $new_password): ?PasswordHash + public function addPasswordToHistory(Model $model, string $newPassword): ?PasswordHash { - $history_depth = config('password-history.depth'); + $historyDepth = config('password-history.depth'); - if ($this->hasPasswordInHistory($model, $new_password)) { + if ($this->hasPasswordInHistory($model, $newPassword)) { throw new PasswordInHistoryException; } - return DB::transaction(function () use ($model, $new_password, $history_depth) { + return DB::transaction(function () use ($model, $newPassword, $historyDepth) { $password_instance = new PasswordHash; - $password_instance->hash = Hash::make($new_password); + $password_instance->hash = Hash::make($newPassword); $password_instance->model()->associate($model); $password_instance->save(); - $password_history_count = $this->getModelPasswordHistoryCount($model); + $passwordHistoryCount = $this->getModelPasswordHistoryCount($model); - if ($history_depth > 0) { - if ($password_history_count > $history_depth) { + if ($historyDepth > 0) { + if ($passwordHistoryCount > $historyDepth) { $this->removeModelOldestHash($model); } } diff --git a/src/Traits/HasPasswordHistory.php b/src/Traits/HasPasswordHistory.php index 4a73a0c..1038f4d 100644 --- a/src/Traits/HasPasswordHistory.php +++ b/src/Traits/HasPasswordHistory.php @@ -6,6 +6,7 @@ use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Hash; trait HasPasswordHistory { @@ -13,7 +14,7 @@ trait HasPasswordHistory private static ?string $plain_text_password = null; - protected static function bootHasPasswordHistory() + protected static function bootHasPasswordHistory(): void { static::saving(function ($model) { if (is_null($model->id)) { @@ -31,30 +32,30 @@ public function password(): Attribute set: function ($value) { self::$plain_text_password = $value; - return $value; + return Hash::make($value); } ); } - public function hasPasswordInHistory(string $new_password): bool + public function hasPasswordInHistory(string $newPassword): bool { - $password_history_service = new PasswordHistoryService; + $passwordHistoryService = new PasswordHistoryService; - return $password_history_service->hasPasswordInHistory($this, $new_password); + return $passwordHistoryService->hasPasswordInHistory($this, $newPassword); } - public function addPasswordInHistory(string $new_password): void + public function addPasswordInHistory(string $newPassword): void { - $this->savePasswordInHistory($new_password); + $this->savePasswordInHistory($newPassword); } - protected function savePasswordInHistory(string $new_password, bool $explicit = true): void + protected function savePasswordInHistory(string $newPassword, bool $explicit = true): void { - DB::transaction(function () use ($new_password, $explicit) { - $password_history_service = new PasswordHistoryService; - $password_entry = $password_history_service->addPasswordToHistory($this, $new_password); + DB::transaction(function () use ($newPassword, $explicit) { + $passwordHistoryService = new PasswordHistoryService; + $passwordEntry = $passwordHistoryService->addPasswordToHistory($this, $newPassword); - $this[$this->password_field_column] = $password_entry->hash; + $this[$this->password_field_column] = $passwordEntry->hash; if ($explicit) { $this->save(); diff --git a/tests/PasswordHistoryTest.php b/tests/PasswordHistoryTest.php index 3b6bdf6..b326ea4 100644 --- a/tests/PasswordHistoryTest.php +++ b/tests/PasswordHistoryTest.php @@ -21,11 +21,11 @@ it('should found password already used', function () { $model = TestModel::create(); - $password_hash = new PasswordHash; - $password_hash->hash = Hash::make('password'); - $password_hash->model_type = get_class($model); - $password_hash->model_id = $model->id; - $password_hash->save(); + $passwordHash = new PasswordHash; + $passwordHash->hash = Hash::make('password'); + $passwordHash->model_type = get_class($model); + $passwordHash->model_id = $model->id; + $passwordHash->save(); $result = $this->passwordHistory->hasPasswordInHistory($model, 'password'); expect($result)->toBeTrue(); @@ -48,11 +48,11 @@ $model = TestModel::create(); $existingPassword = 'existing_password'; - $password_hash = new PasswordHash; - $password_hash->hash = Hash::make($existingPassword); - $password_hash->model_type = get_class($model); - $password_hash->model_id = $model->id; - $password_hash->save(); + $passwordHash = new PasswordHash; + $passwordHash->hash = Hash::make($existingPassword); + $passwordHash->model_type = get_class($model); + $passwordHash->model_id = $model->id; + $passwordHash->save(); $this->passwordHistory->addPasswordToHistory($model, $existingPassword); })->throws(PasswordInHistoryException::class); @@ -65,21 +65,21 @@ $this->passwordHistory->addPasswordToHistory($model, 'password2'); $this->passwordHistory->addPasswordToHistory($model, 'password3'); - $valid_password_count = 0; + $count = 0; if ($this->passwordHistory->hasPasswordInHistory($model, 'password1')) { - $valid_password_count++; + $count++; } if ($this->passwordHistory->hasPasswordInHistory($model, 'password2')) { - $valid_password_count++; + $count++; } if ($this->passwordHistory->hasPasswordInHistory($model, 'password3')) { - $valid_password_count++; + $count++; } - expect($valid_password_count)->toBe(2); + expect($count)->toBe(2); }); }); @@ -95,6 +95,8 @@ 'model_type' => get_class($model), 'model_id' => $model->id, ]); + + expect($model->password)->not()->toBe('password'); }); it('should not create alredy used entry', function () { @@ -111,3 +113,29 @@ $model->save(); })->throws(PasswordInHistoryException::class); }); + +describe('Password history edge cases', function () { + it('should not create an entry using the update quietly method', function () { + $model = new TestModelWihTrait; + $model->id = 123; + $model->save(); + + $count = PasswordHash::byModel($model)->count(); + expect($count)->toBe(0); + + $model->updateQuietly(['password' => 'test']); + + $count = PasswordHash::byModel($model)->count(); + expect($count)->toBe(0); + }); + + it('should not create an entry using the save quietly method', function () { + $model = new TestModelWihTrait; + $model->id = 123; + $model->password = 'password'; + $model->saveQuietly(); + + $count = PasswordHash::byModel($model)->count(); + expect($count)->toBe(0); + }); +}); diff --git a/tests/Pest.php b/tests/Pest.php index b799083..340e83b 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -25,3 +25,18 @@ class TestModelWihTrait extends Model protected $table = 'test_models'; } + +class TestModelWithCast extends Model +{ + use HasPasswordHistory; + + protected $guarded = []; + + public $timestamps = false; + + protected $table = 'test_models'; + + protected $casts = [ + 'password' => 'hashed', + ]; +}