diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index bd31ab0c3..64e900b49 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -668,12 +668,14 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue private function upload_to_s3(): void { if (is_null($this->s3)) { + $previousS3StorageId = $this->backup->s3_storage_id; + $this->backup->update([ 'save_s3' => false, 's3_storage_id' => null, ]); - throw new \Exception('S3 storage configuration is missing or has been deleted (S3 storage ID: '.($this->backup->s3_storage_id ?? 'null').'). S3 backup has been disabled for this schedule.'); + throw new \Exception('S3 storage configuration is missing or has been deleted (S3 storage ID: '.($previousS3StorageId ?? 'null').'). S3 backup has been disabled for this schedule.'); } try { diff --git a/app/Livewire/Project/Database/BackupEdit.php b/app/Livewire/Project/Database/BackupEdit.php index a18022882..ef106a65f 100644 --- a/app/Livewire/Project/Database/BackupEdit.php +++ b/app/Livewire/Project/Database/BackupEdit.php @@ -3,6 +3,7 @@ namespace App\Livewire\Project\Database; use App\Models\ScheduledDatabaseBackup; +use App\Models\ServiceDatabase; use Exception; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Livewire\Attributes\Locked; @@ -144,7 +145,7 @@ class BackupEdit extends Component try { $server = null; - if ($this->backup->database instanceof \App\Models\ServiceDatabase) { + if ($this->backup->database instanceof ServiceDatabase) { $server = $this->backup->database->service->destination->server; } elseif ($this->backup->database->destination && $this->backup->database->destination->server) { $server = $this->backup->database->destination->server; @@ -170,7 +171,7 @@ class BackupEdit extends Component $this->backup->delete(); - if ($this->backup->database->getMorphClass() === \App\Models\ServiceDatabase::class) { + if ($this->backup->database->getMorphClass() === ServiceDatabase::class) { $serviceDatabase = $this->backup->database; return redirect()->route('project.service.database.backups', [ @@ -182,7 +183,7 @@ class BackupEdit extends Component } else { return redirect()->route('project.database.backup.index', $this->parameters); } - } catch (\Exception $e) { + } catch (Exception $e) { $this->dispatch('error', 'Failed to delete backup: '.$e->getMessage()); return handleError($e, $this); @@ -207,6 +208,13 @@ class BackupEdit extends Component $this->backup->s3_storage_id = null; } + // S3 backup cannot be enabled without a valid S3 storage owned by the team + $availableS3Ids = collect($this->s3s)->pluck('id'); + if ($this->backup->save_s3 && ! $availableS3Ids->contains($this->backup->s3_storage_id)) { + $this->backup->save_s3 = $this->saveS3 = false; + $this->backup->s3_storage_id = $this->s3StorageId = null; + } + // Validate that disable_local_backup can only be true when S3 backup is enabled if ($this->backup->disable_local_backup && ! $this->backup->save_s3) { $this->backup->disable_local_backup = $this->disableLocalBackup = false; @@ -214,7 +222,7 @@ class BackupEdit extends Component $isValid = validate_cron_expression($this->backup->frequency); if (! $isValid) { - throw new \Exception('Invalid Cron / Human expression'); + throw new Exception('Invalid Cron / Human expression'); } $this->validate(); } diff --git a/app/Livewire/Project/Database/CreateScheduledBackup.php b/app/Livewire/Project/Database/CreateScheduledBackup.php index 7f807afe2..bb8b8b9c7 100644 --- a/app/Livewire/Project/Database/CreateScheduledBackup.php +++ b/app/Livewire/Project/Database/CreateScheduledBackup.php @@ -3,6 +3,7 @@ namespace App\Livewire\Project\Database; use App\Models\ScheduledDatabaseBackup; +use App\Models\ServiceDatabase; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Collection; use Livewire\Attributes\Locked; @@ -48,6 +49,14 @@ class CreateScheduledBackup extends Component $this->validate(); + if ($this->saveToS3) { + if (is_null($this->s3StorageId) || ! $this->definedS3s->contains('id', $this->s3StorageId)) { + $this->dispatch('error', 'Please select a valid S3 storage to enable S3 backups.'); + + return; + } + } + $isValid = validate_cron_expression($this->frequency); if (! $isValid) { $this->dispatch('error', 'Invalid Cron / Human expression.'); @@ -74,7 +83,7 @@ class CreateScheduledBackup extends Component } $databaseBackup = ScheduledDatabaseBackup::create($payload); - if ($this->database->getMorphClass() === \App\Models\ServiceDatabase::class) { + if ($this->database->getMorphClass() === ServiceDatabase::class) { $this->dispatch('refreshScheduledBackups', $databaseBackup->id); } else { $this->dispatch('refreshScheduledBackups'); diff --git a/app/Models/S3Storage.php b/app/Models/S3Storage.php index 3f6ee51cc..fca74170d 100644 --- a/app/Models/S3Storage.php +++ b/app/Models/S3Storage.php @@ -15,6 +15,7 @@ class S3Storage extends BaseModel use HasFactory, HasSafeStringAttribute; protected $fillable = [ + 'team_id', 'name', 'description', 'region', diff --git a/app/Models/ScheduledDatabaseBackupExecution.php b/app/Models/ScheduledDatabaseBackupExecution.php index 51ad46de9..1d5f5f9ce 100644 --- a/app/Models/ScheduledDatabaseBackupExecution.php +++ b/app/Models/ScheduledDatabaseBackupExecution.php @@ -23,6 +23,7 @@ class ScheduledDatabaseBackupExecution extends BaseModel protected function casts(): array { return [ + 'size' => 'integer', 's3_uploaded' => 'boolean', 'local_storage_deleted' => 'boolean', 's3_storage_deleted' => 'boolean', diff --git a/tests/Feature/BackupEditValidationTest.php b/tests/Feature/BackupEditValidationTest.php new file mode 100644 index 000000000..8894f0f69 --- /dev/null +++ b/tests/Feature/BackupEditValidationTest.php @@ -0,0 +1,85 @@ +create(['team_id' => $team->id]); + $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + $database = StandalonePostgresql::create([ + 'name' => 'pg-backup-edit-validation', + 'image' => 'postgres:16-alpine', + 'postgres_user' => 'postgres', + 'postgres_password' => 'password', + 'postgres_db' => 'postgres', + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); + + return ScheduledDatabaseBackup::create(array_merge([ + 'frequency' => '0 0 * * *', + 'save_s3' => true, + 's3_storage_id' => null, + 'database_type' => $database->getMorphClass(), + 'database_id' => $database->id, + 'team_id' => $team->id, + ], $overrides)); +} + +beforeEach(function () { + if (InstanceSettings::find(0) === null) { + $settings = new InstanceSettings; + $settings->id = 0; + $settings->save(); + } + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->user->teams()->attach($this->team, ['role' => 'owner']); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); +}); + +it('disables S3 backup when saved without a selected S3 storage', function () { + $backup = createBackupForEditValidationTest($this->team); + + Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 's3s' => $this->team->s3s]) + ->call('submit') + ->assertDispatched('success'); + + $backup->refresh(); + expect($backup->save_s3)->toBeFalsy(); + expect($backup->s3_storage_id)->toBeNull(); +}); + +it('cascades to disabling local backup deletion when S3 is force-disabled', function () { + $backup = createBackupForEditValidationTest($this->team, [ + 'disable_local_backup' => true, + ]); + + Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 's3s' => $this->team->s3s]) + ->call('submit') + ->assertDispatched('success'); + + $backup->refresh(); + expect($backup->save_s3)->toBeFalsy(); + expect($backup->s3_storage_id)->toBeNull(); + expect($backup->disable_local_backup)->toBeFalsy(); +}); diff --git a/tests/Feature/CreateScheduledBackupValidationTest.php b/tests/Feature/CreateScheduledBackupValidationTest.php new file mode 100644 index 000000000..a167511e2 --- /dev/null +++ b/tests/Feature/CreateScheduledBackupValidationTest.php @@ -0,0 +1,102 @@ +create(['team_id' => $team->id]); + $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + return StandalonePostgresql::create([ + 'name' => 'pg-scheduled-backup-validation', + 'image' => 'postgres:16-alpine', + 'postgres_user' => 'postgres', + 'postgres_password' => 'password', + 'postgres_db' => 'postgres', + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); +} + +function createS3StorageForTeam(Team $team, string $name = 'Test S3'): S3Storage +{ + return S3Storage::create([ + 'name' => $name, + 'region' => 'us-east-1', + 'key' => 'test-key', + 'secret' => 'test-secret', + 'bucket' => 'test-bucket', + 'endpoint' => 'https://s3.example.com', + 'is_usable' => true, + 'team_id' => $team->id, + ]); +} + +beforeEach(function () { + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->user->teams()->attach($this->team, ['role' => 'owner']); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); +}); + +it('rejects enabling S3 backup without a selected S3 storage', function () { + $database = createDatabaseForScheduledBackupTest($this->team); + + Livewire::test(CreateScheduledBackup::class, ['database' => $database]) + ->set('frequency', '0 0 * * *') + ->set('saveToS3', true) + ->set('s3StorageId', null) + ->call('submit') + ->assertDispatched('error'); + + expect(ScheduledDatabaseBackup::count())->toBe(0); +}); + +it('rejects an S3 storage not owned by the current team', function () { + $database = createDatabaseForScheduledBackupTest($this->team); + + $foreignS3 = createS3StorageForTeam(Team::factory()->create(), 'Foreign S3'); + + Livewire::test(CreateScheduledBackup::class, ['database' => $database]) + ->set('frequency', '0 0 * * *') + ->set('saveToS3', true) + ->set('s3StorageId', $foreignS3->id) + ->call('submit') + ->assertDispatched('error'); + + expect(ScheduledDatabaseBackup::count())->toBe(0); +}); + +it('creates a scheduled backup with a valid team-owned S3 storage', function () { + $database = createDatabaseForScheduledBackupTest($this->team); + $s3 = createS3StorageForTeam($this->team); + + Livewire::test(CreateScheduledBackup::class, ['database' => $database]) + ->set('frequency', '0 0 * * *') + ->set('saveToS3', true) + ->set('s3StorageId', $s3->id) + ->call('submit') + ->assertDispatched('refreshScheduledBackups'); + + $backup = ScheduledDatabaseBackup::first(); + expect($backup)->not->toBeNull(); + expect($backup->save_s3)->toBeTruthy(); + expect($backup->s3_storage_id)->toBe($s3->id); +}); diff --git a/tests/Feature/DatabaseBackupJobTest.php b/tests/Feature/DatabaseBackupJobTest.php index 05cb21f12..2d549c1ca 100644 --- a/tests/Feature/DatabaseBackupJobTest.php +++ b/tests/Feature/DatabaseBackupJobTest.php @@ -66,6 +66,48 @@ test('upload_to_s3 throws exception and disables s3 when storage is null', funct expect($backup->s3_storage_id)->toBeNull(); }); +test('upload_to_s3 exception message reports the previous s3 storage id', function () { + $backup = ScheduledDatabaseBackup::create([ + 'frequency' => '0 0 * * *', + 'save_s3' => true, + 's3_storage_id' => 12345, + 'database_type' => 'App\Models\StandalonePostgresql', + 'database_id' => 1, + 'team_id' => Team::factory()->create()->id, + ]); + + $job = new DatabaseBackupJob($backup); + + $reflection = new ReflectionClass($job); + $reflection->getProperty('s3')->setValue($job, null); + + expect(fn () => $reflection->getMethod('upload_to_s3')->invoke($job)) + ->toThrow(Exception::class, 'S3 storage ID: 12345'); + + $backup->refresh(); + expect($backup->save_s3)->toBeFalsy(); + expect($backup->s3_storage_id)->toBeNull(); +}); + +test('upload_to_s3 exception message reports null when no previous s3 storage id exists', function () { + $backup = ScheduledDatabaseBackup::create([ + 'frequency' => '0 0 * * *', + 'save_s3' => true, + 's3_storage_id' => null, + 'database_type' => 'App\Models\StandalonePostgresql', + 'database_id' => 1, + 'team_id' => Team::factory()->create()->id, + ]); + + $job = new DatabaseBackupJob($backup); + + $reflection = new ReflectionClass($job); + $reflection->getProperty('s3')->setValue($job, null); + + expect(fn () => $reflection->getMethod('upload_to_s3')->invoke($job)) + ->toThrow(Exception::class, 'S3 storage ID: null'); +}); + test('deleting s3 storage disables s3 on linked backups', function () { $team = Team::factory()->create();