From b7ff4e074da1ee4cf427a26d4b0746d15048cab0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vi=CC=81ctor=20Falco=CC=81n?= Date: Sat, 4 Jul 2026 20:18:49 +0200 Subject: [PATCH] Move residual-encryption cleanup out of Inertia share() into a queued job HandleInertiaRequests::share() is an Inertia shared-data provider and should be read-only, but it ran a DELETE + UPDATE against the user on every non-API web GET to purge the leftover encryption salt and EncryptedMessage once a user no longer had any encrypted accounts or transactions. Writing to the database while resolving render props is a side effect in the wrong place. The existing encryption commands do not cover this case: both NotifyEncryptedDataRemovalCommand and DeleteEncryptedDataAccountsCommand target users who *still* have encrypted data, whereas this cleanup finalizes users who have *finished* decrypting. To preserve that eventual-cleanup semantics without writing during the render, the work now goes to a new PurgeResidualEncryptionArtifactsJob dispatched from share(). The job re-checks the condition on execution and is idempotent, so repeat dispatches are no-ops. The two ->exists() checks stay because they still feed the hasEncryptedAccounts and hasEncryptedTransactions props. Tests: InertiaSharedDataTest asserts a web GET no longer mutates the user inline and instead queues the job (and does not queue it when there is no salt); a new PurgeResidualEncryptionArtifactsJobTest covers the clear, the two 'still has encrypted data' guards, and the null-salt no-op. --- app/Http/Middleware/HandleInertiaRequests.php | 13 ++-- .../PurgeResidualEncryptionArtifactsJob.php | 57 ++++++++++++++++ tests/Feature/InertiaSharedDataTest.php | 31 +++++++++ ...urgeResidualEncryptionArtifactsJobTest.php | 65 +++++++++++++++++++ 4 files changed, 160 insertions(+), 6 deletions(-) create mode 100644 app/Jobs/PurgeResidualEncryptionArtifactsJob.php create mode 100644 tests/Feature/PurgeResidualEncryptionArtifactsJobTest.php diff --git a/app/Http/Middleware/HandleInertiaRequests.php b/app/Http/Middleware/HandleInertiaRequests.php index f7edc8fe..a7991136 100644 --- a/app/Http/Middleware/HandleInertiaRequests.php +++ b/app/Http/Middleware/HandleInertiaRequests.php @@ -5,6 +5,7 @@ namespace App\Http\Middleware; use App\Enums\BankingConnectionStatus; use App\Enums\BankingProvider; use App\Features\CalculateBalancesOnImport; +use App\Jobs\PurgeResidualEncryptionArtifactsJob; use App\Models\BankingConnection; use App\Services\CurrencyOptions; use Illuminate\Foundation\Inspiring; @@ -56,12 +57,12 @@ class HandleInertiaRequests extends Middleware ->where(fn ($q) => $q->whereNotNull('description_iv')->orWhereNotNull('notes_iv')) ->exists() ?? false; - // Clean up encryption data if no encrypted accounts or transactions remain - if (! $request->is('api/*') && $user?->encryption_salt !== null) { - if (! $hasEncryptedAccounts && ! $hasEncryptedTransactions) { - $user->encryptedMessage()->delete(); - $user->update(['encryption_salt' => null]); - } + // A shared-data provider must stay read-only, so hand the residual + // encryption cleanup off to a queued job instead of mutating the user + // inline during the render. The job re-checks the condition and is + // idempotent, so dispatching it on repeat requests is harmless. + if (! $request->is('api/*') && $user?->encryption_salt !== null && ! $hasEncryptedAccounts && ! $hasEncryptedTransactions) { + PurgeResidualEncryptionArtifactsJob::dispatch($user); } return [ diff --git a/app/Jobs/PurgeResidualEncryptionArtifactsJob.php b/app/Jobs/PurgeResidualEncryptionArtifactsJob.php new file mode 100644 index 00000000..f121a739 --- /dev/null +++ b/app/Jobs/PurgeResidualEncryptionArtifactsJob.php @@ -0,0 +1,57 @@ +user->fresh(); + + if ($user === null || $user->encryption_salt === null) { + return; + } + + if ($this->hasResidualEncryptedData($user)) { + return; + } + + $user->encryptedMessage()->delete(); + $user->update(['encryption_salt' => null]); + } + + private function hasResidualEncryptedData(User $user): bool + { + $hasEncryptedAccounts = $user->accounts() + ->where('encrypted', true) + ->exists(); + + if ($hasEncryptedAccounts) { + return true; + } + + return $user->transactions() + ->where(function (Builder $query): void { + $query->whereNotNull('description_iv') + ->orWhereNotNull('notes_iv'); + }) + ->exists(); + } +} diff --git a/tests/Feature/InertiaSharedDataTest.php b/tests/Feature/InertiaSharedDataTest.php index cb9d8a75..f86c4ec1 100644 --- a/tests/Feature/InertiaSharedDataTest.php +++ b/tests/Feature/InertiaSharedDataTest.php @@ -1,8 +1,10 @@ onboarded()->create([ + 'encryption_salt' => str_repeat('a', 24), + ]); + + actingAs($user)->withoutVite()->get(route('dashboard'))->assertSuccessful(); + + // Rendering the page must stay read-only: the salt is untouched inline. + expect($user->fresh()->encryption_salt)->toBe(str_repeat('a', 24)); + + // The eventual cleanup is handed off to the queued job instead. + Queue::assertPushed( + PurgeResidualEncryptionArtifactsJob::class, + fn (PurgeResidualEncryptionArtifactsJob $job) => $job->user->is($user), + ); +}); + +test('a web GET does not queue encryption cleanup when the user has no salt', function () { + Queue::fake(); + + $user = User::factory()->onboarded()->create(['encryption_salt' => null]); + + actingAs($user)->withoutVite()->get(route('dashboard'))->assertSuccessful(); + + Queue::assertNotPushed(PurgeResidualEncryptionArtifactsJob::class); +}); + test('all pages receive app url in shared props', function () { $response = $this->withoutVite()->get(route('home')); diff --git a/tests/Feature/PurgeResidualEncryptionArtifactsJobTest.php b/tests/Feature/PurgeResidualEncryptionArtifactsJobTest.php new file mode 100644 index 00000000..79dea336 --- /dev/null +++ b/tests/Feature/PurgeResidualEncryptionArtifactsJobTest.php @@ -0,0 +1,65 @@ +onboarded()->create([ + 'encryption_salt' => str_repeat('a', 24), + ]); + + EncryptedMessage::query()->create([ + 'user_id' => $user->id, + 'encrypted_content' => 'encrypted_test_content', + 'iv' => str_repeat('b', 16), + ]); + + return $user; +} + +test('it clears the salt and encrypted message when no encrypted data remains', function () { + $user = userWithEncryptionArtifacts(); + + PurgeResidualEncryptionArtifactsJob::dispatchSync($user); + + expect($user->fresh()->encryption_salt)->toBeNull(); + expect(EncryptedMessage::query()->where('user_id', $user->id)->exists())->toBeFalse(); +}); + +test('it keeps the salt when an encrypted transaction still exists', function () { + $user = userWithEncryptionArtifacts(); + Transaction::factory()->create([ + 'user_id' => $user->id, + 'description_iv' => str_repeat('c', 16), + ]); + + PurgeResidualEncryptionArtifactsJob::dispatchSync($user); + + expect($user->fresh()->encryption_salt)->toBe(str_repeat('a', 24)); + expect(EncryptedMessage::query()->where('user_id', $user->id)->exists())->toBeTrue(); +}); + +test('it keeps the salt when an encrypted account still exists', function () { + $user = userWithEncryptionArtifacts(); + Account::factory()->create([ + 'user_id' => $user->id, + 'encrypted' => true, + ]); + + PurgeResidualEncryptionArtifactsJob::dispatchSync($user); + + expect($user->fresh()->encryption_salt)->toBe(str_repeat('a', 24)); + expect(EncryptedMessage::query()->where('user_id', $user->id)->exists())->toBeTrue(); +}); + +test('it is a no-op when the salt is already null', function () { + $user = User::factory()->onboarded()->create(['encryption_salt' => null]); + + PurgeResidualEncryptionArtifactsJob::dispatchSync($user); + + expect($user->fresh()->encryption_salt)->toBeNull(); +});