From 350ddca273f903b0e3c8d00b556c2291a7b19c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Labrecque?= Date: Mon, 6 Jan 2025 16:21:43 -0500 Subject: [PATCH] Revert https://github.com/spatie/laravel-ciphersweet/pull/74 Fix Model getChanges() --- src/Concerns/UsesCipherSweet.php | 37 +++++++++++++- src/Observers/ModelObserver.php | 9 ++++ tests/ModelTest.php | 77 ++++++++++++++++++++++++++++++ tests/ObserverTest.php | 31 +++++++++++- tests/TestClasses/UserObserver.php | 2 +- 5 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 tests/ModelTest.php diff --git a/src/Concerns/UsesCipherSweet.php b/src/Concerns/UsesCipherSweet.php index 1c07548..b9dfc54 100644 --- a/src/Concerns/UsesCipherSweet.php +++ b/src/Concerns/UsesCipherSweet.php @@ -13,6 +13,9 @@ trait UsesCipherSweet { public static EncryptedRow $cipherSweetEncryptedRow; + // Keeps which attributes were really dirty when saving + protected array $cipherSweetSavingUnencryptedAttributes = []; + protected static function bootUsesCipherSweet() { static::observe(ModelObserver::class); @@ -73,7 +76,7 @@ public function deleteBlindIndexes(): void public function decryptRow(): void { $this->setRawAttributes(static::$cipherSweetEncryptedRow->setPermitEmpty(config('ciphersweet.permit_empty', false)) - ->decryptRow($this->getAttributes()), false); + ->decryptRow($this->getAttributes()), true); } public function scopeWhereBlind( @@ -94,6 +97,38 @@ public function scopeOrWhereBlind( return $query->orWhereExists(fn (Builder $query): Builder => $this->buildBlindQuery($query, $column, $indexName, $value)); } + public function excludeNonChangedEncryptedAttributesFromChanges(): self + { + // Changes will contain the encrypted fields, event when none of these fields were changed + // (because Laravel will compare the unencrypted value with the encrypted one which will never match) + $changes = $this->getChanges(); + + // Remove all encrypted attributes that were not previously dirty + if (! empty($changes)) { + foreach (static::$cipherSweetEncryptedRow->listEncryptedFields() as $field) { + if (! array_key_exists($field, $this->cipherSweetSavingUnencryptedAttributes)) { + unset($changes[$field]); + } else { + // Use unencrypted value instead of encrypted + $changes[$field] = $this->cipherSweetSavingUnencryptedAttributes[$field]; + } + } + + $this->changes = $changes; + } + + $this->cipherSweetSavingUnencryptedAttributes = []; + + return $this; + } + + public function saveDirtyAttributesForCipherSweet(): self + { + $this->cipherSweetSavingUnencryptedAttributes = $this->getDirty(); + + return $this; + } + private function buildBlindQuery( Builder $query, string $column, diff --git a/src/Observers/ModelObserver.php b/src/Observers/ModelObserver.php index ee10968..ffc70ae 100644 --- a/src/Observers/ModelObserver.php +++ b/src/Observers/ModelObserver.php @@ -23,13 +23,22 @@ public function retrieved(CipherSweetEncrypted $model): void public function saving(CipherSweetEncrypted $model): void { + $model->saveDirtyAttributesForCipherSweet(); + $model->encryptRow(); + + // NOTE: If other listeners are called after this, all the encrypted attributes will appear in the dirty list + // since each field will contain their encrypted value. + // Having "Listener priority" might fix this (put the encrypter at the lowest priority + // so all other listeners are called first), but Laravel doesn't support that (yet). } public function saved(CipherSweetEncrypted $model): void { $model->decryptRow(); + $model->excludeNonChangedEncryptedAttributesFromChanges(); + $model->updateBlindIndexes(); } } diff --git a/tests/ModelTest.php b/tests/ModelTest.php new file mode 100644 index 0000000..0740663 --- /dev/null +++ b/tests/ModelTest.php @@ -0,0 +1,77 @@ + 'John Doe', + 'password' => bcrypt('password'), + 'email' => 'john@example.com', + ]); + + expect($user) + ->name->toBe('John Doe') + ->getAttribute('name')->toBe('John Doe') + ->created_at->not()->toBeNull() + ->updated_at->not()->toBeNull() + ->isDirty()->toBeFalse() + ->isClean()->toBeTrue() + ->getDirty()->toBeEmpty() + ->getOriginal('name')->toBe('John Doe') + ->wasChanged()->toBeFalse() + ->getChanges()->toBeEmpty(); + + expect(User::count(), 1); + + $user = User::first(); + + expect($user) + ->name->toBe('John Doe') + ->getAttribute('name')->toBe('John Doe') + ->created_at->not()->toBeNull() + ->updated_at->not()->toBeNull() + ->isDirty()->toBeFalse() + ->isClean()->toBeTrue() + ->getDirty()->toBeEmpty() + ->getOriginal('name')->toBe('John Doe') + ->wasChanged()->toBeFalse() + ->getChanges()->toBeEmpty(); + + $user->name = 'New name'; + + expect($user) + ->name->toBe('New name') + ->getAttribute('name')->toBe('New name') + ->isDirty()->toBeTrue() + ->isClean()->toBeFalse() + ->getDirty()->toBe(['name' => 'New name']) + ->getOriginal('name')->toBe('John Doe') + ->wasChanged()->toBeFalse() + ->getChanges()->toBeEmpty(); + + $createdAt = $user->created_at; + $updatedAt = $user->updated_at; + + $this->travel(1)->seconds(); + $user->save(); + $this->travelBack(); + + expect($user) + ->name->toBe('New name') + ->getAttribute('name')->toBe('New name') + ->created_at->toEqual($createdAt) + ->updated_at->toBeGreaterThan($updatedAt) + ->isDirty()->toBeFalse() + ->isClean()->toBeTrue() + ->getDirty()->toBeEmpty() + ->getOriginal('name')->toBe('New name') + ->wasChanged('name')->toBeTrue() + ->wasChanged('password')->toBeFalse() + ->wasChanged('email')->toBeFalse() + ->getChanges()->toBe([ + 'name' => 'New name', + 'updated_at' => (string) $user->updated_at, + ]); +}); diff --git a/tests/ObserverTest.php b/tests/ObserverTest.php index 4b96470..ba78fa5 100644 --- a/tests/ObserverTest.php +++ b/tests/ObserverTest.php @@ -14,13 +14,40 @@ Log::spy(); User::observe(UserObserver::class); + + $this->travel(1)->seconds(); // to force updated_at to be updated $user->update(['email' => 'NewEmail@example.com']); + $this->travelBack(); + + Log::shouldHaveReceived('info') + ->with("saving: dirty=2") // name attribute + encrypted email attribute + ->once(); + + Log::shouldHaveReceived('info') + ->with("saved: changed=3") // the name, updated_at & email attribute + ->once(); +}); + +it('can exclude unmodified attributes from changes', function () { + $user = User::create([ + 'name' => 'John Doe', + 'password' => bcrypt('password'), + 'email' => 'john@example.com', + ]); + + Log::spy(); + + User::observe(UserObserver::class); + + $this->travel(1)->seconds(); // to force updated_at to be updated + $user->update(['name' => 'John Doe2']); + $this->travelBack(); Log::shouldHaveReceived('info') - ->with("saving: dirty=2") + ->with("saving: dirty=2") // name attribute + encrypted email attribute (encrypted are always present) ->once(); Log::shouldHaveReceived('info') - ->with("saved: dirty=2") + ->with("saved: changed=2") // the name & updated_at attribute ->once(); }); diff --git a/tests/TestClasses/UserObserver.php b/tests/TestClasses/UserObserver.php index 14fbe91..e3da385 100644 --- a/tests/TestClasses/UserObserver.php +++ b/tests/TestClasses/UserObserver.php @@ -6,7 +6,7 @@ class UserObserver { public function saved(User $user) { - $msg = "saved: dirty=".sizeof($user->getDirty()); + $msg = "saved: changed=".sizeof($user->getChanges()); \Log::info($msg); }