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.
This commit is contained in:
parent
39588ea8e8
commit
b7ff4e074d
|
|
@ -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 [
|
||||
|
|
|
|||
|
|
@ -0,0 +1,57 @@
|
|||
<?php
|
||||
|
||||
namespace App\Jobs;
|
||||
|
||||
use App\Models\User;
|
||||
use Illuminate\Contracts\Queue\ShouldQueue;
|
||||
use Illuminate\Database\Eloquent\Builder;
|
||||
use Illuminate\Foundation\Queue\Queueable;
|
||||
|
||||
/**
|
||||
* Once a user has decrypted (or removed) every client-side encrypted account and
|
||||
* transaction, the leftover encryption salt and EncryptedMessage row serve no
|
||||
* purpose. This job clears them so `hasEncryptionSetup` stops reporting true.
|
||||
*
|
||||
* It re-checks the condition on execution and is therefore idempotent: dispatching
|
||||
* it more than once (or after the salt was already cleared elsewhere) is a no-op.
|
||||
*/
|
||||
class PurgeResidualEncryptionArtifactsJob implements ShouldQueue
|
||||
{
|
||||
use Queueable;
|
||||
|
||||
public function __construct(public User $user) {}
|
||||
|
||||
public function handle(): void
|
||||
{
|
||||
$user = $this->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();
|
||||
}
|
||||
}
|
||||
|
|
@ -1,8 +1,10 @@
|
|||
<?php
|
||||
|
||||
use App\Enums\BankingConnectionStatus;
|
||||
use App\Jobs\PurgeResidualEncryptionArtifactsJob;
|
||||
use App\Models\BankingConnection;
|
||||
use App\Models\User;
|
||||
use Illuminate\Support\Facades\Queue;
|
||||
use Inertia\Testing\AssertableInertia as Assert;
|
||||
|
||||
use function Pest\Laravel\actingAs;
|
||||
|
|
@ -52,6 +54,35 @@ test('shared auth user does not expose sensitive fields', function () {
|
|||
);
|
||||
});
|
||||
|
||||
test('a web GET does not mutate the user inline but queues the encryption cleanup', function () {
|
||||
Queue::fake();
|
||||
|
||||
$user = User::factory()->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'));
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,65 @@
|
|||
<?php
|
||||
|
||||
use App\Jobs\PurgeResidualEncryptionArtifactsJob;
|
||||
use App\Models\Account;
|
||||
use App\Models\EncryptedMessage;
|
||||
use App\Models\Transaction;
|
||||
use App\Models\User;
|
||||
|
||||
function userWithEncryptionArtifacts(): User
|
||||
{
|
||||
$user = User::factory()->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();
|
||||
});
|
||||
Loading…
Reference in New Issue