feat: apply hash on setter

beliven-it · Nov 23, 2024 · 24837a4 · 24837a4
1 parent a4b79bd
commit 24837a4
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 44 deletions.
diff --git 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
@@ -12,52 +12,52 @@ 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;
             }
         }
 
         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
@@ -6,14 +6,15 @@
 use Illuminate\Database\Eloquent\Casts\Attribute;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Hash;
 
 trait HasPasswordHistory
 {
     protected string $password_field_column = 'password';
 
     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
@@ -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
@@ -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',
+    ];
+}