From 9665aa292c935004781b90bec0b79270ded873d6 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:03:01 +0200 Subject: [PATCH] fix(api): block invalid destination types and service deletions --- .../Api/DestinationsController.php | 16 +- app/Models/StandaloneDocker.php | 2 +- app/Models/SwarmDocker.php | 2 +- tests/Feature/Api/DestinationsApiTest.php | 200 ++++++++++++++++++ 4 files changed, 216 insertions(+), 4 deletions(-) create mode 100644 tests/Feature/Api/DestinationsApiTest.php diff --git a/app/Http/Controllers/Api/DestinationsController.php b/app/Http/Controllers/Api/DestinationsController.php index 5db11624b..8b54913e7 100644 --- a/app/Http/Controllers/Api/DestinationsController.php +++ b/app/Http/Controllers/Api/DestinationsController.php @@ -6,6 +6,7 @@ use App\Models\Server; use App\Models\StandaloneDocker; use App\Models\SwarmDocker; +use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\Validator; @@ -28,7 +29,7 @@ private function transform($d): array /** * Resolve the calling token's team id, or return a 403 response. */ - private function teamIdOrAbort(): int|\Illuminate\Http\JsonResponse + private function teamIdOrAbort(): int|JsonResponse { $teamId = getTeamIdFromToken(); if (is_null($teamId)) { @@ -97,6 +98,12 @@ public function create(Request $request, string $server_uuid) if (! is_int($teamId)) { return $teamId; } + + $return = validateIncomingRequest($request); + if ($return instanceof JsonResponse) { + return $return; + } + $server = Server::whereTeamId($teamId)->whereUuid($server_uuid)->firstOrFail(); $allowed = ['name', 'network', 'type']; @@ -114,7 +121,12 @@ public function create(Request $request, string $server_uuid) return response()->json(['message' => 'Validation failed', 'errors' => $validator->errors()], 422); } - $type = $request->input('type', 'standalone'); + $expectedType = $server->isSwarm() ? 'swarm' : 'standalone'; + $type = $request->input('type', $expectedType); + if ($type !== $expectedType) { + return response()->json(['message' => "Destination type must be {$expectedType} for this server."], 422); + } + $name = $request->input('name') ?: ($server->name.'-'.$request->input('network')); $class = $type === 'swarm' ? SwarmDocker::class : StandaloneDocker::class; diff --git a/app/Models/StandaloneDocker.php b/app/Models/StandaloneDocker.php index 1c5cfd342..c1dd4bf67 100644 --- a/app/Models/StandaloneDocker.php +++ b/app/Models/StandaloneDocker.php @@ -144,6 +144,6 @@ public function databases(): Collection public function attachedTo() { - return $this->applications?->count() > 0 || $this->databases()->count() > 0; + return $this->applications()->exists() || $this->databases()->count() > 0 || $this->services()->exists(); } } diff --git a/app/Models/SwarmDocker.php b/app/Models/SwarmDocker.php index 0e9620457..02b8381d9 100644 --- a/app/Models/SwarmDocker.php +++ b/app/Models/SwarmDocker.php @@ -124,6 +124,6 @@ public function databases() public function attachedTo() { - return $this->applications?->count() > 0 || $this->databases()->count() > 0; + return $this->applications()->exists() || $this->databases()->count() > 0 || $this->services()->exists(); } } diff --git a/tests/Feature/Api/DestinationsApiTest.php b/tests/Feature/Api/DestinationsApiTest.php new file mode 100644 index 000000000..cbbd6d0a6 --- /dev/null +++ b/tests/Feature/Api/DestinationsApiTest.php @@ -0,0 +1,200 @@ + 'array', + 'session.driver' => 'array', + 'queue.default' => 'sync', + 'app.maintenance.driver' => 'file', + ]); + + InstanceSettings::unguarded(fn () => InstanceSettings::firstOrCreate( + ['id' => 0], + ['is_api_enabled' => true], + )); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + session(['currentTeam' => $this->team]); + + $this->bearerToken = destinationsApiToken($this->user, $this->team, ['*']); + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = StandaloneDocker::where('server_id', $this->server->id)->first(); +}); + +function destinationsApiHeaders(string $bearerToken): array +{ + return [ + 'Authorization' => 'Bearer '.$bearerToken, + 'Content-Type' => 'application/json', + ]; +} + +function destinationsApiToken(User $user, Team $team, array $abilities): string +{ + $plainTextToken = Str::random(40); + $token = $user->tokens()->create([ + 'name' => 'destinations-api-test-'.Str::random(6), + 'token' => hash('sha256', $plainTextToken), + 'abilities' => $abilities, + 'team_id' => $team->id, + ]); + + return $token->getKey().'|'.$plainTextToken; +} + +describe('GET /api/v1/destinations', function () { + test('lists only destinations owned by the token team', function () { + $otherTeam = Team::factory()->create(); + $otherServer = Server::factory()->create(['team_id' => $otherTeam->id]); + $otherDestination = StandaloneDocker::where('server_id', $otherServer->id)->first(); + + $response = $this->withHeaders(destinationsApiHeaders($this->bearerToken)) + ->getJson('/api/v1/destinations'); + + $response->assertOk(); + $uuids = collect($response->json())->pluck('uuid'); + + expect($uuids)->toContain($this->destination->uuid) + ->not->toContain($otherDestination->uuid); + }); +}); + +describe('GET /api/v1/destinations/{uuid}', function () { + test('does not expose another team destination', function () { + $otherTeam = Team::factory()->create(); + $otherServer = Server::factory()->create(['team_id' => $otherTeam->id]); + $otherDestination = StandaloneDocker::where('server_id', $otherServer->id)->first(); + + $response = $this->withHeaders(destinationsApiHeaders($this->bearerToken)) + ->getJson("/api/v1/destinations/{$otherDestination->uuid}"); + + $response->assertNotFound(); + }); +}); + +describe('GET /api/v1/servers/{server_uuid}/destinations', function () { + test('lists destinations for a team server', function () { + $response = $this->withHeaders(destinationsApiHeaders($this->bearerToken)) + ->getJson("/api/v1/servers/{$this->server->uuid}/destinations"); + + $response->assertOk(); + expect($response->json())->toHaveCount(1) + ->and($response->json('0.uuid'))->toBe($this->destination->uuid); + }); +}); + +describe('POST /api/v1/servers/{server_uuid}/destinations', function () { + test('requires a write token', function () { + $readOnlyToken = destinationsApiToken($this->user, $this->team, ['read']); + + $response = $this->withHeaders(destinationsApiHeaders($readOnlyToken)) + ->postJson("/api/v1/servers/{$this->server->uuid}/destinations", [ + 'network' => 'new-network', + ]); + + $response->assertForbidden(); + }); + + test('rejects non-json requests before creating a destination', function () { + $response = $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + ])->post("/api/v1/servers/{$this->server->uuid}/destinations", [ + 'network' => 'api-swarm-network', + 'type' => 'swarm', + ]); + + $response->assertStatus(400); + expect(SwarmDocker::where('server_id', $this->server->id)->where('network', 'api-swarm-network')->exists())->toBeFalse(); + }); + + test('rejects unknown fields', function () { + $response = $this->withHeaders(destinationsApiHeaders($this->bearerToken)) + ->postJson("/api/v1/servers/{$this->server->uuid}/destinations", [ + 'network' => 'new-network', + 'unexpected' => 'value', + ]); + + $response->assertStatus(422); + $response->assertJsonPath('fields.0', 'unexpected'); + }); + + test('rejects unsafe docker network names', function () { + $response = $this->withHeaders(destinationsApiHeaders($this->bearerToken)) + ->postJson("/api/v1/servers/{$this->server->uuid}/destinations", [ + 'network' => 'bad;network', + ]); + + $response->assertStatus(422); + $response->assertJsonValidationErrors(['network']); + }); + + test('rejects a destination type that does not match the server mode', function () { + $response = $this->withHeaders(destinationsApiHeaders($this->bearerToken)) + ->postJson("/api/v1/servers/{$this->server->uuid}/destinations", [ + 'network' => 'wrong-type-network', + 'type' => 'swarm', + ]); + + $response->assertStatus(422); + expect(SwarmDocker::where('server_id', $this->server->id)->where('network', 'wrong-type-network')->exists())->toBeFalse(); + }); + + test('creates a swarm destination on a swarm server', function () { + $this->server->settings()->update(['is_swarm_manager' => true]); + + $response = $this->withHeaders(destinationsApiHeaders($this->bearerToken)) + ->postJson("/api/v1/servers/{$this->server->uuid}/destinations", [ + 'name' => 'API Swarm', + 'network' => 'api-swarm-network', + 'type' => 'swarm', + ]); + + $response->assertCreated(); + $response->assertJsonStructure(['uuid']); + expect(SwarmDocker::where('server_id', $this->server->id)->where('network', 'api-swarm-network')->exists())->toBeTrue(); + }); + + test('rejects duplicate networks on the same server and type', function () { + $response = $this->withHeaders(destinationsApiHeaders($this->bearerToken)) + ->postJson("/api/v1/servers/{$this->server->uuid}/destinations", [ + 'network' => $this->destination->network, + ]); + + $response->assertStatus(409); + }); +}); + +describe('DELETE /api/v1/destinations/{uuid}', function () { + test('blocks deleting a destination with an attached service', function () { + $project = Project::factory()->create(['team_id' => $this->team->id]); + $environment = $project->environments()->first(); + + Service::factory()->create([ + 'environment_id' => $environment->id, + 'server_id' => $this->server->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + $response = $this->withHeaders(destinationsApiHeaders($this->bearerToken)) + ->deleteJson("/api/v1/destinations/{$this->destination->uuid}"); + + $response->assertStatus(409); + $this->assertModelExists($this->destination); + }); +});