feat: remove unused code and improve test coverage

README.md

@@ -66,15 +66,6 @@ $user->password = $password_from_request;
 $user->save();
 ```
 
-or instead use the trait method `addPasswordInHistory` like below:
-
-```php
-$user->addPasswordInHistory($password_from_request);
-```
-
-Both of these methods throws an exception if the password is already in the history. So make sure to catch it.
-
-
 You can also use a rule in your request validation:
 
 ```php
@@ -118,6 +109,22 @@ class UpdatePasswordRequest extends FormRequest
 }
 ```
 
+> **Warning!!**:
+> For password checking is important to **provide the plain text password** and **NOT the hashed password**.
+> otherwise an exception will be thrown. 
+> Make sure to check your code before using this package.
+
+```php
+<?php
+
+# Allowed
+$user->password = 'password';
+
+# Not allowed
+$user->password = Hash::make('password');
+
+# This throw an exception of type PasswordAlreadyHashedException
+```
 
 ## Testing
 

src/Entities/Enums/DomainErrorsEnum.php

@@ -5,15 +5,13 @@
 enum DomainErrorsEnum: string
 {
     case PASSWORD_IN_HISTORY = 'Password already in history';
-    case PASSWORD_HAS_HASHED_CAST = 'Password has hashed cast. The library cannot check if the password is in the history with this setting enabled';
-    case MISSING_GET_PASSWORD_FIELD_COLUMN_METHOD = 'The getPasswordFieldColumn method is missing in the model';
+    case PASSWORD_ALREADY_HASHED = 'Password already hashed';
 
     public function code(): int
     {
         return match ($this) {
-            DomainErrorsEnum::PASSWORD_IN_HISTORY                      => 400,
-            DomainErrorsEnum::PASSWORD_HAS_HASHED_CAST                 => 500,
-            DomainErrorsEnum::MISSING_GET_PASSWORD_FIELD_COLUMN_METHOD => 500,
+            DomainErrorsEnum::PASSWORD_IN_HISTORY     => 400,
+            DomainErrorsEnum::PASSWORD_ALREADY_HASHED => 400,
         };
     }
 }

src/Exceptions/BaseException.php

@@ -6,6 +6,7 @@
 use Exception;
 use Illuminate\Http\Request;
 
+// @codeCoverageIgnoreStart
 class BaseException extends Exception
 {
     public function __construct(DomainErrorsEnum $error, string $message = '')
@@ -28,3 +29,4 @@ public function render($request): void
         abort($this->getCode(), $this->getMessage());
     }
 }
+// @codeCoverageIgnoreEnd

src/Exceptions/PasswordHasHashedCastException.php → src/Exceptions/PasswordAlreadyHashedException.php

@@ -4,10 +4,10 @@
 
 use Beliven\PasswordHistory\Entities\Enums\DomainErrorsEnum;
 
-class PasswordHasHashedCastException extends BaseException
+class PasswordAlreadyHashedException extends BaseException
 {
     public function __construct()
     {
-        parent::__construct(DomainErrorsEnum::PASSWORD_HAS_HASHED_CAST);
+        parent::__construct(DomainErrorsEnum::PASSWORD_ALREADY_HASHED);
     }
 }

src/PasswordHistory.php

@@ -2,6 +2,7 @@
 
 namespace Beliven\PasswordHistory;
 
+use Beliven\PasswordHistory\Exceptions\PasswordAlreadyHashedException;
 use Beliven\PasswordHistory\Exceptions\PasswordInHistoryException;
 use Beliven\PasswordHistory\Models\PasswordHash;
 use Illuminate\Database\Eloquent\Model;
@@ -42,6 +43,10 @@ public function addPasswordToHistory(Model $model, string $newPassword): ?Passwo
     {
         $historyDepth = config('password-history.depth');
 
+        if (Hash::isHashed($newPassword)) {
+            throw new PasswordAlreadyHashedException;
+        }
+
         if ($this->hasPasswordInHistory($model, $newPassword)) {
             throw new PasswordInHistoryException;
         }

src/Traits/HasPasswordHistory.php

@@ -3,17 +3,23 @@
 namespace Beliven\PasswordHistory\Traits;
 
 use Beliven\PasswordHistory\PasswordHistory as PasswordHistoryService;
-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 function casts(): array
+    {
+        return [
+            ...parent::casts(),
+            $this->password_field_column => 'hashed',
+        ];
+    }
+
     protected static function bootHasPasswordHistory(): void
     {
         static::created(function ($model) {
@@ -25,16 +31,11 @@ protected static function bootHasPasswordHistory(): void
         });
     }
 
-    public function password(): Attribute
+    protected function castAttributeAsHashedString($key, $value): string
     {
-        return Attribute::make(
-            set: function ($value) {
-                self::$plain_text_password = $value;
-                $hash = Hash::make($value);
-
-                return $hash;
-            }
-        );
+        self::$plain_text_password = $value;
+
+        return parent::castAttributeAsHashedString($key, $value);
     }
 
     public function hasPasswordInHistory(string $newPassword): bool
@@ -44,21 +45,11 @@ public function hasPasswordInHistory(string $newPassword): bool
         return $passwordHistoryService->hasPasswordInHistory($this, $newPassword);
     }
 
-    public function addPasswordInHistory(string $newPassword): void
+    protected function savePasswordInHistory(string $newPassword): void
     {
-        $this->savePasswordInHistory($newPassword);
-    }
-
-    protected function savePasswordInHistory(string $newPassword, bool $explicit = true): void
-    {
-        DB::transaction(function () use ($newPassword, $explicit) {
+        DB::transaction(function () use ($newPassword) {
             $passwordHistoryService = new PasswordHistoryService;
             $passwordEntry = $passwordHistoryService->addPasswordToHistory($this, $newPassword);
-
-            if ($explicit) {
-                $this[$this->password_field_column] = $passwordEntry->hash;
-                $this->save();
-            }
         });
     }
 
@@ -68,7 +59,7 @@ private static function handleCreating(Model $model): void
             return;
         }
 
-        $model->savePasswordInHistory(self::$plain_text_password, false);
+        $model->savePasswordInHistory(self::$plain_text_password);
     }
 
     private static function handleUpdating(Model $model): void
@@ -81,6 +72,6 @@ private static function handleUpdating(Model $model): void
             return;
         }
 
-        $model->savePasswordInHistory(self::$plain_text_password, false);
+        $model->savePasswordInHistory(self::$plain_text_password);
     }
 }

tests/PasswordHistoryTest.php

@@ -1,8 +1,10 @@
 <?php
 
+use Beliven\PasswordHistory\Exceptions\PasswordAlreadyHashedException;
 use Beliven\PasswordHistory\Exceptions\PasswordInHistoryException;
 use Beliven\PasswordHistory\Models\PasswordHash;
 use Beliven\PasswordHistory\PasswordHistory;
+use Beliven\PasswordHistory\Rules\HasPasswordInHistory;
 use Illuminate\Support\Facades\Config;
 use Illuminate\Support\Facades\Hash;
 
@@ -12,7 +14,7 @@
     Config::set('password-history.depth', 5);
 });
 
-describe('Password history using trait methods', function () {
+describe('Password history methods', function () {
     it('should not found password not yet used', function () {
         $model = TestModel::create();
         $result = $this->passwordHistory->hasPasswordInHistory($model, 'password');
@@ -88,10 +90,9 @@
 
 describe('Password history via mutator', function () {
     it('should create a password entry', function () {
-        $model = new TestModelWihTrait;
+        $model = new TestModelWithTrait;
         $model->password = 'password';
         $model->id = 123;
-
         $model->save();
 
         $this->assertDatabaseHas('password_hashes', [
@@ -101,12 +102,37 @@
 
         expect($model->password)->not()->toBe('password');
 
+        $passwordHash = PasswordHash::byModel($model)->first();
+        $match = Hash::check('password', $passwordHash->hash);
+        expect($match)->toBeTrue();
+
         $match = Hash::check('password', $model->password);
         expect($match)->toBeTrue();
     });
 
-    it('should not create alredy used entry', function () {
-        $model = new TestModelWihTrait;
+    it('should update a user password', function () {
+        $model = new TestModelWithTrait;
+        $model->password = 'password';
+        $model->id = 123;
+        $model->save();
+
+        $passwordHash = PasswordHash::byModel($model)->orderBy('id', 'desc')->first();
+        $match = Hash::check('password', $passwordHash->hash);
+        expect($match)->toBeTrue();
+
+        $model->password = 'password1';
+        $model->save();
+
+        $passwordHash = PasswordHash::byModel($model)->orderBy('id', 'desc')->first();
+        $match = Hash::check('password1', $passwordHash->hash);
+        expect($match)->toBeTrue();
+
+        $match = Hash::check('password1', $model->password);
+        expect($match)->toBeTrue();
+    });
+
+    it('should not create already used entry', function () {
+        $model = new TestModelWithTrait;
         $model->id = 123;
 
         $model->password = 'password';
@@ -118,30 +144,56 @@
         $model->password = 'password';
         $model->save();
     })->throws(PasswordInHistoryException::class);
+
+    it('shoud not create a password history entry using quietly methods', function () {
+        $model = new TestModelWithTrait;
+        $model->id = 123;
+        $model->password = 'password';
+        $model->saveQuietly();
+
+        $this->assertDatabaseMissing('password_hashes', [
+            'model_type' => get_class($model),
+            'model_id'   => $model->id,
+        ]);
+    });
 });
 
 describe('Password history edge cases', function () {
     it('should not create an entry using the save quietly method', function () {
-        $model = new TestModelWihTrait;
+        $model = new TestModelWithTrait;
         $model->id = 123;
         $model->password = 'password';
         $model->saveQuietly();
 
+        $match = Hash::check('password', $model->password);
+        expect($match)->toBeTrue();
+
         $count = PasswordHash::byModel($model)->count();
         expect($count)->toBe(0);
     });
 
-    test('simulate password change', function () {
-        $model = new TestModelWihTrait;
+    it('should prevent the creation of a user password in history when using double casting', function () {
+        $passwordHash = Hash::make('password');
+
+        $model = new TestModelWithTrait;
+        $model->password = $passwordHash;
         $model->id = 123;
-        $model->password = 'password';
         $model->save();
+    })->throws(PasswordAlreadyHashedException::class);
 
-        $model->password = 'password1';
+});
+
+describe('Validation Rule', function () {
+    it('should block the validation when a password is already in history', function () {
+        $model = new TestModelWithTrait;
+        $model->id = 123;
+        $model->password = 'password';
         $model->save();
 
-        $match = Hash::check('password1', $model->password);
+        $rule = new HasPasswordInHistory($model);
 
-        expect($match)->toBeTrue();
+        $rule->validate('password', 'password', function ($message) {
+            expect($message)->toBe('Password already in history');
+        });
     });
 });

tests/Pest.php

@@ -15,7 +15,7 @@ class TestModel extends Model
     protected $table = 'test_models';
 }
 
-class TestModelWihTrait extends Model
+class TestModelWithTrait extends Model
 {
     use HasPasswordHistory;