From 379838251b662fbf9761bdffa9a60856340ffbc9 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 5 Oct 2026 21:22:33 +0200 Subject: [PATCH] fix(auth): use the resource team in database import, SQLite, and tokens These paths used the session team after the resource was loaded. After a team switch in another tab they used the wrong team or denied access. - ImportForm: use the database team for the server, S3 storages, and the import - SQLite ConnectApplication: list applications of the database team - IntegrationTokenPolicy: check admin of the token team for view, update, and delete --- app/Livewire/Project/Database/ImportForm.php | 19 ++- .../Database/Sqlite/ConnectApplication.php | 8 +- app/Policies/IntegrationTokenPolicy.php | 12 +- .../DatabaseImportCommandInjectionTest.php | 4 +- .../DatabaseImportFormResourceTeamTest.php | 153 ++++++++++++++++++ ...IntegrationTokenPolicyResourceTeamTest.php | 36 +++++ ...liteConnectApplicationResourceTeamTest.php | 110 +++++++++++++ 7 files changed, 329 insertions(+), 13 deletions(-) create mode 100644 tests/Feature/DatabaseImportFormResourceTeamTest.php create mode 100644 tests/Feature/IntegrationTokenPolicyResourceTeamTest.php create mode 100644 tests/Feature/SqliteConnectApplicationResourceTeamTest.php diff --git a/app/Livewire/Project/Database/ImportForm.php b/app/Livewire/Project/Database/ImportForm.php index 638101b3c1..23fafd969a 100644 --- a/app/Livewire/Project/Database/ImportForm.php +++ b/app/Livewire/Project/Database/ImportForm.php @@ -216,7 +216,16 @@ class ImportForm extends Component return null; } - return Server::ownedByCurrentTeam()->find($this->serverId); + return Server::query()->where('team_id', $this->resourceTeamId())->find($this->serverId); + } + + /** + * Team of the loaded database. The session team can differ: a user can switch teams in + * another tab while this form stays open. + */ + private function resourceTeamId(): ?int + { + return $this->resource?->team()?->id; } protected $listeners = [ @@ -467,7 +476,7 @@ class ImportForm extends Component $source = Storage::exists("upload/{$this->resourceUuid}/restore") ? new DatabaseImportSource('upload', dumpAll: $this->dumpAll, replaceExisting: $this->replaceExisting, keepOwners: $this->keepOwners, sqliteDatabase: $this->sqliteDatabase, restoreMysqlUsers: $this->restoreMysqlUsers) : new DatabaseImportSource('server', path: $this->customLocation, dumpAll: $this->dumpAll, replaceExisting: $this->replaceExisting, keepOwners: $this->keepOwners, sqliteDatabase: $this->sqliteDatabase, restoreMysqlUsers: $this->restoreMysqlUsers); - $activity = StartDatabaseImport::run($this->resource, $source, (int) currentTeam()->id); + $activity = StartDatabaseImport::run($this->resource, $source, (int) $this->resourceTeamId()); $this->activityId = $activity->id; $this->dispatch('activityMonitor', $activity->id); $this->dispatch('databaserestore'); @@ -497,7 +506,7 @@ class ImportForm extends Component public function loadAvailableS3Storages() { try { - $this->availableS3Storages = S3Storage::ownedByCurrentTeam(['id', 'name', 'description']) + $this->availableS3Storages = S3Storage::ownedByCurrentTeamAPI((int) $this->resourceTeamId(), ['id', 'name', 'description']) ->where('is_usable', true) ->get() ->map(fn ($s) => ['id' => $s->id, 'name' => $s->name, 'description' => $s->description]) @@ -551,7 +560,7 @@ class ImportForm extends Component } try { - $s3Storage = S3Storage::ownedByCurrentTeam()->findOrFail($this->s3StorageId); + $s3Storage = S3Storage::ownedByCurrentTeamAPI((int) $this->resourceTeamId())->findOrFail($this->s3StorageId); // Validate bucket name early if (! $this->validateBucketName($s3Storage->bucket)) { @@ -629,7 +638,7 @@ class ImportForm extends Component try { $this->importRunning = true; $source = new DatabaseImportSource('s3', path: $this->s3Path, s3StorageUuid: (string) $this->s3StorageId, dumpAll: $this->dumpAll, replaceExisting: $this->replaceExisting, keepOwners: $this->keepOwners, sqliteDatabase: $this->sqliteDatabase, restoreMysqlUsers: $this->restoreMysqlUsers); - $activity = StartDatabaseImport::run($this->resource, $source, (int) currentTeam()->id); + $activity = StartDatabaseImport::run($this->resource, $source, (int) $this->resourceTeamId()); $this->activityId = $activity->id; $this->dispatch('activityMonitor', $activity->id); $this->dispatch('databaserestore'); diff --git a/app/Livewire/Project/Database/Sqlite/ConnectApplication.php b/app/Livewire/Project/Database/Sqlite/ConnectApplication.php index c600810c1c..b2482378a7 100644 --- a/app/Livewire/Project/Database/Sqlite/ConnectApplication.php +++ b/app/Livewire/Project/Database/Sqlite/ConnectApplication.php @@ -184,18 +184,20 @@ class ConnectApplication extends Component } /** - * Applications owned by the current team that run on the same server as the database. + * Applications of the database's team that run on the same server as the database. The session + * team can differ: a user can switch teams in another tab while this component is still open. * * @return Collection */ private function connectableApplications(): Collection { $serverId = $this->database->destination?->server_id; - if ($serverId === null) { + $teamId = $this->database->team()?->id; + if ($serverId === null || $teamId === null) { return collect(); } - return Application::ownedByCurrentTeam() + return Application::ownedByCurrentTeamAPI($teamId) ->with(['destination', 'environment.project']) ->get() ->filter(fn (Application $application) => $application->destination?->server_id === $serverId) diff --git a/app/Policies/IntegrationTokenPolicy.php b/app/Policies/IntegrationTokenPolicy.php index 309c8167f2..671a8b33c6 100644 --- a/app/Policies/IntegrationTokenPolicy.php +++ b/app/Policies/IntegrationTokenPolicy.php @@ -7,6 +7,9 @@ use App\Models\User; class IntegrationTokenPolicy { + /** + * Listing and creating tokens happen on the session team's security pages. + */ public function viewAny(User $user): bool { return $user->isAdmin(); @@ -17,18 +20,21 @@ class IntegrationTokenPolicy return $user->isAdmin(); } + /** + * A loaded token is checked against its own team: the session team can differ. + */ public function view(User $user, IntegrationToken $integrationToken): bool { - return $user->isAdmin() && $integrationToken->team_id === currentTeam()->id; + return $user->isAdminOfTeam((int) $integrationToken->team_id); } public function update(User $user, IntegrationToken $integrationToken): bool { - return $user->isAdmin() && $integrationToken->team_id === currentTeam()->id; + return $user->isAdminOfTeam((int) $integrationToken->team_id); } public function delete(User $user, IntegrationToken $integrationToken): bool { - return $user->isAdmin() && $integrationToken->team_id === currentTeam()->id; + return $user->isAdminOfTeam((int) $integrationToken->team_id); } } diff --git a/tests/Feature/DatabaseImportCommandInjectionTest.php b/tests/Feature/DatabaseImportCommandInjectionTest.php index e2985e44bf..932709d30d 100644 --- a/tests/Feature/DatabaseImportCommandInjectionTest.php +++ b/tests/Feature/DatabaseImportCommandInjectionTest.php @@ -89,7 +89,7 @@ describe('locked properties', function () { }); describe('server method uses team scoping', function () { - test('server computed property calls ownedByCurrentTeam', function () { + test('server computed property is scoped to the database team', function () { $method = new ReflectionMethod(ImportForm::class, 'server'); // Extract the server method body @@ -98,7 +98,7 @@ describe('server method uses team scoping', function () { $lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1); $methodBody = implode('', $lines); - expect($methodBody)->toContain('ownedByCurrentTeam'); + expect($methodBody)->toContain('where(\'team_id\', $this->resourceTeamId())'); expect($methodBody)->not->toContain('Server::find($this->serverId)'); }); }); diff --git a/tests/Feature/DatabaseImportFormResourceTeamTest.php b/tests/Feature/DatabaseImportFormResourceTeamTest.php new file mode 100644 index 0000000000..29db848a8b --- /dev/null +++ b/tests/Feature/DatabaseImportFormResourceTeamTest.php @@ -0,0 +1,153 @@ + 0, + 'disable_two_step_confirmation' => true, + ]); + + $this->resourceTeam = Team::factory()->create(); + $this->otherTeam = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->resourceTeam->members()->attach($this->user, ['role' => 'admin']); + $this->otherTeam->members()->attach($this->user, ['role' => 'owner']); + $this->actingAs($this->user); + session(['currentTeam' => $this->resourceTeam]); + + $this->server = Server::factory()->create(['team_id' => $this->resourceTeam->id]); + $destination = StandaloneDocker::firstOrCreate( + ['server_id' => $this->server->id, 'network' => 'coolify'], + ['uuid' => (string) Str::uuid(), 'name' => 'docker'] + ); + $project = Project::factory()->create(['team_id' => $this->resourceTeam->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + $this->database = StandalonePostgresql::create([ + 'uuid' => (string) Str::uuid(), + 'name' => 'db', + 'postgres_user' => 'postgres', + 'postgres_password' => 'password', + 'postgres_db' => 'db', + 'image' => 'postgres:17', + 'status' => 'running', + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); + + $s3Attributes = ['region' => 'us-east-1', 'key' => 'key', 'secret' => 'secret', 'bucket' => 'backups', 'endpoint' => 'https://s3.example.com', 'is_usable' => true]; + $this->resourceTeamStorage = S3Storage::create([...$s3Attributes, 'team_id' => $this->resourceTeam->id, 'name' => 'resource-team-s3']); + $this->otherTeamStorage = S3Storage::create([...$s3Attributes, 'team_id' => $this->otherTeam->id, 'name' => 'other-team-s3']); +}); + +class ImportFormResourceTeamTestComponent extends ImportForm +{ + public function mount($database = null, $server = null): void + { + $this->resourceId = $database->id; + $this->resourceType = $database::class; + $this->serverId = $server->id; + $this->container = $database->uuid; + $this->resourceUuid = $database->uuid; + $this->resourceStatus = $database->status ?? ''; + $this->resourceDbType = $database->type(); + $this->loadAvailableS3Storages(); + } + + public function render() + { + return view('livewire.project.database.import-form'); + } +} + +function importFormAfterTeamSwitch(): Testable +{ + $component = Livewire::test(ImportFormResourceTeamTestComponent::class, [ + 'database' => test()->database, + 'server' => test()->server, + ]); + + session(['currentTeam' => test()->otherTeam]); + + return $component; +} + +function fakeStartedDatabaseImport(): Activity +{ + return Activity::create([ + 'log_name' => 'default', + 'description' => 'queued', + 'properties' => ['status' => 'queued'], + ]); +} + +test('a file import started after a session team switch runs for the database team', function () { + StartDatabaseImport::shouldRun()->once() + ->withArgs(fn ($resource, $source, int $teamId): bool => $teamId === $this->resourceTeam->id) + ->andReturn(fakeStartedDatabaseImport()); + + importFormAfterTeamSwitch() + ->set('filename', 'backup.dump') + ->call('runImport') + ->assertNotDispatched('error') + ->assertDispatched('databaserestore'); +}); + +test('an S3 restore started after a session team switch runs for the database team', function () { + StartDatabaseImport::shouldRun()->once() + ->withArgs(fn ($resource, $source, int $teamId): bool => $teamId === $this->resourceTeam->id + && $source->s3StorageUuid === (string) $this->resourceTeamStorage->id) + ->andReturn(fakeStartedDatabaseImport()); + + importFormAfterTeamSwitch() + ->set('s3StorageId', $this->resourceTeamStorage->id) + ->set('s3Path', '/backups/backup.dump') + ->set('s3FileSize', 1024) + ->call('restoreFromS3') + ->assertNotDispatched('error') + ->assertDispatched('databaserestore'); +}); + +test('the S3 storage list contains only storages of the database team', function () { + importFormAfterTeamSwitch() + ->call('loadAvailableS3Storages') + ->assertSet('availableS3Storages', fn (array $storages): bool => collect($storages)->pluck('id')->all() === [$this->resourceTeamStorage->id]); +}); + +test('an S3 storage of another team cannot be checked for the database', function () { + importFormAfterTeamSwitch() + ->set('s3StorageId', $this->otherTeamStorage->id) + ->set('s3Path', '/backups/backup.dump') + ->call('checkS3File') + ->assertNotFound(); +}); + +test('a member of the database team cannot start an import after switching to a team they own', function () { + $this->resourceTeam->members()->updateExistingPivot($this->user->id, ['role' => 'member']); + $this->user->refresh(); + StartDatabaseImport::shouldRun()->never(); + + importFormAfterTeamSwitch() + ->set('filename', 'backup.dump') + ->call('runImport') + ->assertForbidden(); +}); diff --git a/tests/Feature/IntegrationTokenPolicyResourceTeamTest.php b/tests/Feature/IntegrationTokenPolicyResourceTeamTest.php new file mode 100644 index 0000000000..a7cb034ab8 --- /dev/null +++ b/tests/Feature/IntegrationTokenPolicyResourceTeamTest.php @@ -0,0 +1,36 @@ +tokenTeam = Team::factory()->create(); + $this->otherTeam = Team::factory()->create(); + $this->token = IntegrationToken::factory()->create(['team_id' => $this->tokenTeam->id]); + $this->user = User::factory()->create(); + $this->otherTeam->members()->attach($this->user->id, ['role' => 'owner']); + $this->actingAs($this->user); + session(['currentTeam' => $this->otherTeam]); +}); + +it('lets an admin of the token team manage the token while the session team is another team', function (string $ability) { + $this->tokenTeam->members()->attach($this->user->id, ['role' => 'admin']); + + expect($this->user->fresh()->can($ability, $this->token))->toBeTrue(); +})->with(['view', 'update', 'delete']); + +it('denies a member of the token team who owns the session team', function (string $ability) { + $this->tokenTeam->members()->attach($this->user->id, ['role' => 'member']); + + expect($this->user->fresh()->can($ability, $this->token))->toBeFalse(); +})->with(['view', 'update', 'delete']); + +it('denies a user outside the token team even when the session points to the token team', function (string $ability) { + session(['currentTeam' => $this->tokenTeam]); + + expect($this->user->fresh()->can($ability, $this->token))->toBeFalse(); +})->with(['view', 'update', 'delete']); diff --git a/tests/Feature/SqliteConnectApplicationResourceTeamTest.php b/tests/Feature/SqliteConnectApplicationResourceTeamTest.php new file mode 100644 index 0000000000..c30cdbc597 --- /dev/null +++ b/tests/Feature/SqliteConnectApplicationResourceTeamTest.php @@ -0,0 +1,110 @@ +withoutVite(); + InstanceSettings::unguarded(fn () => InstanceSettings::updateOrCreate(['id' => 0], ['id' => 0])); + + $this->resourceTeam = Team::factory()->create(); + $this->otherTeam = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->resourceTeam->members()->attach($this->user->id, ['role' => 'admin']); + $this->otherTeam->members()->attach($this->user->id, ['role' => 'owner']); + $this->actingAs($this->user); + session(['currentTeam' => $this->resourceTeam]); + + $server = Server::factory()->create(['team_id' => $this->resourceTeam->id]); + $this->destination = StandaloneDocker::withoutEvents(fn () => StandaloneDocker::firstOrCreate( + ['server_id' => $server->id, 'network' => 'coolify'], + ['uuid' => (string) Str::uuid(), 'name' => 'test-docker'] + )); + + $this->sqlite = StandaloneSqlite::create([ + 'name' => 'app-sqlite', + 'environment_id' => sqliteResourceTeamEnvironment($this->resourceTeam)->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + $this->resourceTeamApplication = sqliteResourceTeamApplication(sqliteResourceTeamEnvironment($this->resourceTeam), $this->destination, 'Resource Team App'); + $this->otherTeamApplication = sqliteResourceTeamApplication(sqliteResourceTeamEnvironment($this->otherTeam), $this->destination, 'Other Team App'); +}); + +function sqliteResourceTeamEnvironment(Team $team): Environment +{ + return Environment::factory()->create(['project_id' => Project::factory()->create(['team_id' => $team->id])->id]); +} + +function sqliteResourceTeamApplication(Environment $environment, StandaloneDocker $destination, string $name): Application +{ + return Application::factory()->create([ + 'uuid' => (string) Str::uuid(), + 'name' => $name, + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + 'build_pack' => 'dockerimage', + 'docker_registry_image_name' => 'nginx', + ]); +} + +it('offers only applications of the database team when the session team is another team', function () { + session(['currentTeam' => $this->otherTeam]); + + Livewire::test(ConnectApplication::class, ['database' => $this->sqlite]) + ->assertSet('applicationOptions', fn (array $options): bool => collect($options)->pluck('value')->all() === [$this->resourceTeamApplication->uuid]); +}); + +it('connects an application of the database team after a session team switch', function () { + $component = Livewire::test(ConnectApplication::class, ['database' => $this->sqlite]); + session(['currentTeam' => $this->otherTeam]); + + $component->set('applicationUuid', $this->resourceTeamApplication->uuid) + ->call('connect') + ->assertNotDispatched('error') + ->assertRedirect(); + + expect($this->resourceTeamApplication->persistentStorages()->where('standalone_sqlite_id', $this->sqlite->id)->exists())->toBeTrue(); +}); + +it('does not connect an application of another team after a session team switch', function () { + $component = Livewire::test(ConnectApplication::class, ['database' => $this->sqlite]); + session(['currentTeam' => $this->otherTeam]); + + $component->set('applicationUuid', $this->otherTeamApplication->uuid) + ->call('connect') + ->assertDispatched('error') + ->assertNoRedirect(); + + expect($this->otherTeamApplication->persistentStorages()->count())->toBe(0); +}); + +it('forbids a member of the database team from connecting after switching to a team they own', function () { + $this->resourceTeam->members()->updateExistingPivot($this->user->id, ['role' => 'member']); + $this->user->refresh(); + + $component = Livewire::test(ConnectApplication::class, ['database' => $this->sqlite]); + session(['currentTeam' => $this->otherTeam]); + + $component->set('applicationUuid', $this->resourceTeamApplication->uuid) + ->call('connect') + ->assertForbidden(); + + expect($this->resourceTeamApplication->persistentStorages()->count())->toBe(0); +});