fix(api): enforce destination access and cleanup networks

Require admin team membership for destination mutations, return invalid-token
responses for tokenless requests, and remove standalone Docker networks when
deleting destinations.
This commit is contained in:
Andras Bacsai 2026-06-15 17:15:56 +02:00
parent 9665aa292c
commit 9e021c4037
3 changed files with 162 additions and 55 deletions

View file

@ -0,0 +1,16 @@
<?php
namespace App\Actions\Destination;
use App\Models\StandaloneDocker;
class RemoveStandaloneDockerNetwork
{
public function handle(StandaloneDocker $destination): void
{
$safeNetwork = escapeshellarg($destination->network);
instant_remote_process(["docker network disconnect {$safeNetwork} coolify-proxy"], $destination->server, throwError: false);
instant_remote_process(["docker network rm -f {$safeNetwork}"], $destination->server);
}
}

View file

@ -2,38 +2,37 @@
namespace App\Http\Controllers\Api; namespace App\Http\Controllers\Api;
use App\Actions\Destination\RemoveStandaloneDockerNetwork;
use App\Http\Controllers\Controller; use App\Http\Controllers\Controller;
use App\Models\Server; use App\Models\Server;
use App\Models\StandaloneDocker; use App\Models\StandaloneDocker;
use App\Models\SwarmDocker; use App\Models\SwarmDocker;
use Illuminate\Http\JsonResponse; use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request; use Illuminate\Http\Request;
use Illuminate\Support\Facades\Validator;
class DestinationsController extends Controller class DestinationsController extends Controller
{ {
private function transform($d): array private function transform(StandaloneDocker|SwarmDocker $destination): array
{ {
return [ return [
'id' => $d->id, 'uuid' => $destination->uuid,
'uuid' => $d->uuid, 'name' => $destination->name,
'name' => $d->name, 'network' => $destination->network,
'network' => $d->network, 'type' => $destination instanceof SwarmDocker ? 'swarm' : 'standalone',
'type' => $d instanceof SwarmDocker ? 'swarm' : 'standalone', 'server_uuid' => $destination->server?->uuid,
'server_uuid' => $d->server?->uuid, 'created_at' => $destination->created_at,
'created_at' => $d->created_at, 'updated_at' => $destination->updated_at,
'updated_at' => $d->updated_at,
]; ];
} }
/** /**
* Resolve the calling token's team id, or return a 403 response. * Resolve the calling token's team id, or return an invalid-token response.
*/ */
private function teamIdOrAbort(): int|JsonResponse private function teamIdOrAbort(): int|JsonResponse
{ {
$teamId = getTeamIdFromToken(); $teamId = getTeamIdFromToken();
if (is_null($teamId)) { if (is_null($teamId)) {
return response()->json(['message' => 'You are not allowed to access the API.'], 403); return invalidTokenResponse();
} }
return $teamId; return $teamId;
@ -45,15 +44,21 @@ private function teamIdOrAbort(): int|JsonResponse
* controller works on Coolify versions that pre-date that scope being added * controller works on Coolify versions that pre-date that scope being added
* to the destination models (e.g. 4.0.0-beta.470). * to the destination models (e.g. 4.0.0-beta.470).
*/ */
private function teamScopedDockers(int $teamId) private function teamScopedDockers(int $teamId): array
{ {
return [ return [
'standalone' => StandaloneDocker::whereHas('server', fn ($q) => $q->whereTeamId($teamId))->get(), 'standalone' => StandaloneDocker::with('server:id,uuid')->whereHas('server', fn ($query) => $query->whereTeamId($teamId))->get(),
'swarm' => SwarmDocker::whereHas('server', fn ($q) => $q->whereTeamId($teamId))->get(), 'swarm' => SwarmDocker::with('server:id,uuid')->whereHas('server', fn ($query) => $query->whereTeamId($teamId))->get(),
]; ];
} }
public function index(Request $request) private function findDestinationForTeam(int $teamId, string $uuid): StandaloneDocker|SwarmDocker
{
return StandaloneDocker::with('server:id,uuid,team_id,ip,user,port,private_key_id')->whereHas('server', fn ($query) => $query->whereTeamId($teamId))->whereUuid($uuid)->first()
?? SwarmDocker::with('server:id,uuid,team_id')->whereHas('server', fn ($query) => $query->whereTeamId($teamId))->whereUuid($uuid)->firstOrFail();
}
public function index(Request $request): JsonResponse
{ {
$teamId = $this->teamIdOrAbort(); $teamId = $this->teamIdOrAbort();
if (! is_int($teamId)) { if (! is_int($teamId)) {
@ -63,36 +68,38 @@ public function index(Request $request)
return response()->json( return response()->json(
$sets['standalone']->concat($sets['swarm']) $sets['standalone']->concat($sets['swarm'])
->map(fn ($d) => $this->transform($d)) ->map(fn ($destination) => $this->transform($destination))
->values() ->values()
); );
} }
public function index_by_server(Request $request, string $server_uuid) public function index_by_server(Request $request, string $server_uuid): JsonResponse
{ {
$teamId = $this->teamIdOrAbort(); $teamId = $this->teamIdOrAbort();
if (! is_int($teamId)) { if (! is_int($teamId)) {
return $teamId; return $teamId;
} }
$server = Server::whereTeamId($teamId)->whereUuid($server_uuid)->firstOrFail(); $server = Server::with(['standaloneDockers.server:id,uuid', 'swarmDockers.server:id,uuid'])
->whereTeamId($teamId)
->whereUuid($server_uuid)
->firstOrFail();
$list = $server->standaloneDockers->concat($server->swarmDockers); $list = $server->standaloneDockers->concat($server->swarmDockers);
return response()->json($list->map(fn ($d) => $this->transform($d))->values()); return response()->json($list->map(fn ($destination) => $this->transform($destination))->values());
} }
public function show(Request $request, string $uuid) public function show(Request $request, string $uuid): JsonResponse
{ {
$teamId = $this->teamIdOrAbort(); $teamId = $this->teamIdOrAbort();
if (! is_int($teamId)) { if (! is_int($teamId)) {
return $teamId; return $teamId;
} }
$d = StandaloneDocker::whereHas('server', fn ($q) => $q->whereTeamId($teamId))->whereUuid($uuid)->first() $destination = $this->findDestinationForTeam($teamId, $uuid);
?? SwarmDocker::whereHas('server', fn ($q) => $q->whereTeamId($teamId))->whereUuid($uuid)->firstOrFail();
return response()->json($this->transform($d)); return response()->json($this->transform($destination));
} }
public function create(Request $request, string $server_uuid) public function create(Request $request, string $server_uuid): JsonResponse
{ {
$teamId = $this->teamIdOrAbort(); $teamId = $this->teamIdOrAbort();
if (! is_int($teamId)) { if (! is_int($teamId)) {
@ -107,18 +114,22 @@ public function create(Request $request, string $server_uuid)
$server = Server::whereTeamId($teamId)->whereUuid($server_uuid)->firstOrFail(); $server = Server::whereTeamId($teamId)->whereUuid($server_uuid)->firstOrFail();
$allowed = ['name', 'network', 'type']; $allowed = ['name', 'network', 'type'];
$extra = array_diff(array_keys($request->all()), $allowed);
if (! empty($extra)) {
return response()->json(['message' => 'Unknown fields', 'fields' => array_values($extra)], 422);
}
$validator = Validator::make($request->all(), [ $validator = customApiValidator($request->all(), [
'name' => 'nullable|string|max:255', 'name' => 'nullable|string|max:255',
'network' => ['required', 'string', 'max:255', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'], 'network' => ['required', 'string', 'max:255', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'],
'type' => 'nullable|in:standalone,swarm', 'type' => 'nullable|in:standalone,swarm',
]); ]);
if ($validator->fails()) { $extra = array_diff(array_keys($request->all()), $allowed);
return response()->json(['message' => 'Validation failed', 'errors' => $validator->errors()], 422); if ($validator->fails() || ! empty($extra)) {
$errors = $validator->errors();
if (! empty($extra)) {
foreach ($extra as $field) {
$errors->add($field, 'This field is not allowed.');
}
}
return response()->json(['message' => 'Validation failed.', 'errors' => $errors], 422);
} }
$expectedType = $server->isSwarm() ? 'swarm' : 'standalone'; $expectedType = $server->isSwarm() ? 'swarm' : 'standalone';
@ -130,52 +141,80 @@ public function create(Request $request, string $server_uuid)
$name = $request->input('name') ?: ($server->name.'-'.$request->input('network')); $name = $request->input('name') ?: ($server->name.'-'.$request->input('network'));
$class = $type === 'swarm' ? SwarmDocker::class : StandaloneDocker::class; $class = $type === 'swarm' ? SwarmDocker::class : StandaloneDocker::class;
$this->authorize('create', $class);
$exists = $class::where('server_id', $server->id)->where('network', $request->input('network'))->exists(); $exists = $class::where('server_id', $server->id)->where('network', $request->input('network'))->exists();
if ($exists) { if ($exists) {
return response()->json(['message' => 'A destination with this network already exists on the server.'], 409); return response()->json(['message' => 'A destination with this network already exists on the server.'], 409);
} }
$d = $class::create([ $destination = $class::create([
'name' => $name, 'name' => $name,
'network' => $request->input('network'), 'network' => $request->input('network'),
'server_id' => $server->id, 'server_id' => $server->id,
]); ]);
return response()->json(['uuid' => $d->uuid], 201); auditLog('api.destination.created', [
'team_id' => $teamId,
'destination_uuid' => $destination->uuid,
'destination_name' => $destination->name,
'destination_type' => $type,
'server_uuid' => $server->uuid,
]);
return response()->json($this->transform($destination->load('server:id,uuid')), 201);
} }
public function delete(Request $request, string $uuid) public function delete(Request $request, string $uuid): JsonResponse
{ {
$teamId = $this->teamIdOrAbort(); $teamId = $this->teamIdOrAbort();
if (! is_int($teamId)) { if (! is_int($teamId)) {
return $teamId; return $teamId;
} }
$d = StandaloneDocker::whereHas('server', fn ($q) => $q->whereTeamId($teamId))->whereUuid($uuid)->first() $destination = $this->findDestinationForTeam($teamId, $uuid);
?? SwarmDocker::whereHas('server', fn ($q) => $q->whereTeamId($teamId))->whereUuid($uuid)->firstOrFail();
$this->authorize('delete', $destination);
// Guard against deleting destinations with attached resources. attachedTo() // Guard against deleting destinations with attached resources. attachedTo()
// is recent on the destination models; fall back to a manual check for // is recent on the destination models; fall back to a manual check for
// older Coolify versions (e.g. 4.0.0-beta.470). // older Coolify versions (e.g. 4.0.0-beta.470).
if (method_exists($d, 'attachedTo')) { if (method_exists($destination, 'attachedTo')) {
if ($d->attachedTo()) { if ($destination->attachedTo()) {
return response()->json(['message' => 'Destination has attached resources, detach first.'], 409); return response()->json(['message' => 'Destination has attached resources, detach first.'], 409);
} }
} else { } else {
$hasAttached = $d->applications()->exists() $hasAttached = $destination->applications()->exists()
|| $d->postgresqls()->exists() || $destination->postgresqls()->exists()
|| (method_exists($d, 'mysqls') && $d->mysqls()->exists()) || (method_exists($destination, 'mysqls') && $destination->mysqls()->exists())
|| (method_exists($d, 'mariadbs') && $d->mariadbs()->exists()) || (method_exists($destination, 'mariadbs') && $destination->mariadbs()->exists())
|| (method_exists($d, 'mongodbs') && $d->mongodbs()->exists()) || (method_exists($destination, 'mongodbs') && $destination->mongodbs()->exists())
|| (method_exists($d, 'redis') && $d->redis()->exists()) || (method_exists($destination, 'redis') && $destination->redis()->exists())
|| (method_exists($d, 'keydbs') && $d->keydbs()->exists()) || (method_exists($destination, 'keydbs') && $destination->keydbs()->exists())
|| (method_exists($d, 'dragonflies') && $d->dragonflies()->exists()) || (method_exists($destination, 'dragonflies') && $destination->dragonflies()->exists())
|| (method_exists($d, 'clickhouses') && $d->clickhouses()->exists()) || (method_exists($destination, 'clickhouses') && $destination->clickhouses()->exists())
|| (method_exists($d, 'services') && $d->services()->exists()); || (method_exists($destination, 'services') && $destination->services()->exists());
if ($hasAttached) { if ($hasAttached) {
return response()->json(['message' => 'Destination has attached resources, detach first.'], 409); return response()->json(['message' => 'Destination has attached resources, detach first.'], 409);
} }
} }
$d->delete(); if ($destination instanceof StandaloneDocker) {
app(RemoveStandaloneDockerNetwork::class)->handle($destination);
}
$destinationUuid = $destination->uuid;
$destinationName = $destination->name;
$destinationType = $destination instanceof SwarmDocker ? 'swarm' : 'standalone';
$serverUuid = $destination->server?->uuid;
$destination->delete();
auditLog('api.destination.deleted', [
'team_id' => $teamId,
'destination_uuid' => $destinationUuid,
'destination_name' => $destinationName,
'destination_type' => $destinationType,
'server_uuid' => $serverUuid,
]);
return response()->json(['message' => 'Deleted.']); return response()->json(['message' => 'Deleted.']);
} }

View file

@ -1,5 +1,6 @@
<?php <?php
use App\Actions\Destination\RemoveStandaloneDockerNetwork;
use App\Models\InstanceSettings; use App\Models\InstanceSettings;
use App\Models\Project; use App\Models\Project;
use App\Models\Server; use App\Models\Server;
@ -69,7 +70,8 @@ function destinationsApiToken(User $user, Team $team, array $abilities): string
$response->assertOk(); $response->assertOk();
$uuids = collect($response->json())->pluck('uuid'); $uuids = collect($response->json())->pluck('uuid');
expect($uuids)->toContain($this->destination->uuid) expect($response->json('0'))->not->toHaveKey('id')
->and($uuids)->toContain($this->destination->uuid)
->not->toContain($otherDestination->uuid); ->not->toContain($otherDestination->uuid);
}); });
}); });
@ -110,6 +112,20 @@ function destinationsApiToken(User $user, Team $team, array $abilities): string
$response->assertForbidden(); $response->assertForbidden();
}); });
test('rejects create requests from non-admin team members', function () {
$member = User::factory()->create();
$this->team->members()->attach($member->id, ['role' => 'member']);
$memberToken = destinationsApiToken($member, $this->team, ['*']);
$response = $this->withHeaders(destinationsApiHeaders($memberToken))
->postJson("/api/v1/servers/{$this->server->uuid}/destinations", [
'network' => 'member-network',
]);
$response->assertForbidden();
expect(StandaloneDocker::where('server_id', $this->server->id)->where('network', 'member-network')->exists())->toBeFalse();
});
test('rejects non-json requests before creating a destination', function () { test('rejects non-json requests before creating a destination', function () {
$response = $this->withHeaders([ $response = $this->withHeaders([
'Authorization' => 'Bearer '.$this->bearerToken, 'Authorization' => 'Bearer '.$this->bearerToken,
@ -129,8 +145,8 @@ function destinationsApiToken(User $user, Team $team, array $abilities): string
'unexpected' => 'value', 'unexpected' => 'value',
]); ]);
$response->assertStatus(422); $response->assertUnprocessable();
$response->assertJsonPath('fields.0', 'unexpected'); $response->assertJsonValidationErrors(['unexpected']);
}); });
test('rejects unsafe docker network names', function () { test('rejects unsafe docker network names', function () {
@ -139,7 +155,7 @@ function destinationsApiToken(User $user, Team $team, array $abilities): string
'network' => 'bad;network', 'network' => 'bad;network',
]); ]);
$response->assertStatus(422); $response->assertUnprocessable();
$response->assertJsonValidationErrors(['network']); $response->assertJsonValidationErrors(['network']);
}); });
@ -150,7 +166,7 @@ function destinationsApiToken(User $user, Team $team, array $abilities): string
'type' => 'swarm', 'type' => 'swarm',
]); ]);
$response->assertStatus(422); $response->assertUnprocessable();
expect(SwarmDocker::where('server_id', $this->server->id)->where('network', 'wrong-type-network')->exists())->toBeFalse(); expect(SwarmDocker::where('server_id', $this->server->id)->where('network', 'wrong-type-network')->exists())->toBeFalse();
}); });
@ -180,6 +196,42 @@ function destinationsApiToken(User $user, Team $team, array $abilities): string
}); });
describe('DELETE /api/v1/destinations/{uuid}', function () { describe('DELETE /api/v1/destinations/{uuid}', function () {
test('requires a write token', function () {
$readOnlyToken = destinationsApiToken($this->user, $this->team, ['read']);
$response = $this->withHeaders(destinationsApiHeaders($readOnlyToken))
->deleteJson("/api/v1/destinations/{$this->destination->uuid}");
$response->assertForbidden();
$this->assertModelExists($this->destination);
});
test('rejects delete requests from non-admin team members', function () {
$member = User::factory()->create();
$this->team->members()->attach($member->id, ['role' => 'member']);
$memberToken = destinationsApiToken($member, $this->team, ['*']);
$response = $this->withHeaders(destinationsApiHeaders($memberToken))
->deleteJson("/api/v1/destinations/{$this->destination->uuid}");
$response->assertForbidden();
$this->assertModelExists($this->destination);
});
test('deletes standalone destinations after removing the docker network', function () {
$cleanup = Mockery::mock(RemoveStandaloneDockerNetwork::class);
$cleanup->shouldReceive('handle')
->once()
->with(Mockery::on(fn (StandaloneDocker $destination) => $destination->is($this->destination)));
$this->app->instance(RemoveStandaloneDockerNetwork::class, $cleanup);
$response = $this->withHeaders(destinationsApiHeaders($this->bearerToken))
->deleteJson("/api/v1/destinations/{$this->destination->uuid}");
$response->assertOk();
$this->assertModelMissing($this->destination);
});
test('blocks deleting a destination with an attached service', function () { test('blocks deleting a destination with an attached service', function () {
$project = Project::factory()->create(['team_id' => $this->team->id]); $project = Project::factory()->create(['team_id' => $this->team->id]);
$environment = $project->environments()->first(); $environment = $project->environments()->first();