diff --git a/app/Http/Controllers/Api/DeployController.php b/app/Http/Controllers/Api/DeployController.php index 5c405f3e17..cc785bbf8f 100644 --- a/app/Http/Controllers/Api/DeployController.php +++ b/app/Http/Controllers/Api/DeployController.php @@ -74,8 +74,7 @@ class DeployController extends Controller if (is_null($teamId)) { return invalidTokenResponse(); } - $servers = Server::whereTeamId($teamId)->get(); - $deployments_per_server = ApplicationDeploymentQueue::whereIn('status', ['in_progress', 'queued'])->whereIn('server_id', $servers->pluck('id'))->get()->sortBy('id'); + $deployments_per_server = ApplicationDeploymentQueue::ownedByTeam((int) $teamId)->whereIn('status', ['in_progress', 'queued'])->get()->sortBy('id'); $deployments_per_server = $deployments_per_server->map(function ($deployment) { return $this->removeSensitiveData($deployment); }); @@ -228,9 +227,9 @@ class DeployController extends Controller return response()->json(['message' => 'Deployment not found.'], 404); } - // Check if the deployment belongs to the user's team - $servers = Server::whereTeamId($teamId)->pluck('id'); - if (! $servers->contains($deployment->server_id)) { + // Include soft-deleted apps so in-flight deploys stay cancellable. + $application = $deployment->application()->withTrashed()->first(); + if (! $application || data_get($application->team(), 'id') !== (int) $teamId) { return response()->json(['message' => 'You do not have permission to cancel this deployment.'], 403); } @@ -278,7 +277,7 @@ class DeployController extends Controller 'team_id' => $teamId, 'deployment_uuid' => $deployment->deployment_uuid, 'application_id' => $deployment->application_id, - 'application_uuid' => $deployment->application?->uuid, + 'application_uuid' => $application->uuid, 'server_id' => $deployment->server_id, ]); @@ -294,7 +293,7 @@ class DeployController extends Controller } finally { if ($cancelled) { try { - next_after_cancel($deploymentServer, $deployment->application); + next_after_cancel($deploymentServer, $application); } catch (\Throwable $e) { \Log::warning("Failed to advance deployment queue after cancelling deployment {$deployment->id}: {$e->getMessage()}"); } @@ -698,8 +697,6 @@ class DeployController extends Controller if (is_null($teamId)) { return invalidTokenResponse(); } - $servers = Server::whereTeamId($teamId)->get(); - if (is_null($app_uuid)) { return response()->json(['message' => 'Application uuid is required'], 400); } diff --git a/app/Livewire/Dashboard/ActiveDeployments.php b/app/Livewire/Dashboard/ActiveDeployments.php index 29e0c1cd24..216c1f0bc4 100644 --- a/app/Livewire/Dashboard/ActiveDeployments.php +++ b/app/Livewire/Dashboard/ActiveDeployments.php @@ -4,7 +4,6 @@ namespace App\Livewire\Dashboard; use App\Enums\ApplicationDeploymentStatus; use App\Models\ApplicationDeploymentQueue; -use App\Models\Server; use Illuminate\Support\Collection; use Livewire\Component; @@ -21,8 +20,6 @@ class ActiveDeployments extends Component public function refreshDeployments(): void { - $serverIds = Server::ownedByCurrentTeamCached()->pluck('id'); - $columns = [ 'id', 'application_id', @@ -39,7 +36,7 @@ class ActiveDeployments extends Component $baseQuery = ApplicationDeploymentQueue::query() ->with(['application.environment.project']) - ->whereIn('server_id', $serverIds); + ->ownedByTeam(currentTeam()->id); $this->activeDeployments = (clone $baseQuery) ->whereIn('status', [ diff --git a/app/Livewire/DeploymentsIndicator.php b/app/Livewire/DeploymentsIndicator.php index e6b68a62b7..a59ccf9142 100644 --- a/app/Livewire/DeploymentsIndicator.php +++ b/app/Livewire/DeploymentsIndicator.php @@ -3,7 +3,6 @@ namespace App\Livewire; use App\Models\ApplicationDeploymentQueue; -use App\Models\Server; use Livewire\Attributes\Computed; use Livewire\Component; @@ -17,11 +16,9 @@ class DeploymentsIndicator extends Component #[Computed] public function deployments() { - $servers = Server::ownedByCurrentTeamCached(); - return ApplicationDeploymentQueue::with(['application.environment.project']) + ->ownedByTeam(currentTeam()->id) ->whereIn('status', ['in_progress', 'queued']) - ->whereIn('server_id', $servers->pluck('id')) ->orderBy('id') ->get([ 'id', diff --git a/app/Livewire/Project/Application/Deployment/Show.php b/app/Livewire/Project/Application/Deployment/Show.php index a33efe5f80..338a15d023 100644 --- a/app/Livewire/Project/Application/Deployment/Show.php +++ b/app/Livewire/Project/Application/Deployment/Show.php @@ -51,7 +51,9 @@ class Show extends Component if (! $application) { return redirect()->route('dashboard'); } - $application_deployment_queue = ApplicationDeploymentQueue::where('deployment_uuid', $deploymentUuid)->first(); + $application_deployment_queue = ApplicationDeploymentQueue::where('deployment_uuid', $deploymentUuid) + ->where('application_id', $application->id) + ->first(); if (! $application_deployment_queue) { return redirect()->route('project.application.deployment.index', [ 'project_uuid' => $project->uuid, diff --git a/app/Models/ApplicationDeploymentQueue.php b/app/Models/ApplicationDeploymentQueue.php index 5994f187de..e25b032630 100644 --- a/app/Models/ApplicationDeploymentQueue.php +++ b/app/Models/ApplicationDeploymentQueue.php @@ -4,6 +4,7 @@ namespace App\Models; use App\Casts\EncryptedArrayCast; use App\Enums\ApplicationDeploymentStatus; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Carbon; @@ -149,6 +150,20 @@ class ApplicationDeploymentQueue extends Model return $this->belongsTo(Application::class); } + /** + * Scope to deployments of the team's applications. + */ + public function scopeOwnedByTeam(Builder $query, int $teamId): Builder + { + // application_id is varchar; whereHas joins to applications.id (bigint) and breaks on PostgreSQL. + $applicationIds = Application::withTrashed() + ->whereRelation('environment.project.team', 'id', $teamId) + ->pluck('id') + ->map(fn ($id) => (string) $id); + + return $query->whereIn('application_id', $applicationIds); + } + public function server(): Attribute { return Attribute::make( diff --git a/tests/Feature/Api/DeploymentCancellationApiTest.php b/tests/Feature/Api/DeploymentCancellationApiTest.php index 81087a68dd..da16ecc001 100644 --- a/tests/Feature/Api/DeploymentCancellationApiTest.php +++ b/tests/Feature/Api/DeploymentCancellationApiTest.php @@ -41,8 +41,22 @@ beforeEach(function () { // Create a server for the team $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = StandaloneDocker::where('server_id', $this->server->id)->firstOrFail(); + $this->application = createCancellationApiApplication($this->team, $this->destination); }); +function createCancellationApiApplication(Team $team, StandaloneDocker $destination): Application +{ + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + return Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); +} + describe('POST /api/v1/deployments/{uuid}/cancel', function () { test('returns 401 when not authenticated', function () { $response = $this->postJson('/api/v1/deployments/fake-uuid/cancel'); @@ -61,14 +75,14 @@ describe('POST /api/v1/deployments/{uuid}/cancel', function () { }); test('returns 403 when user does not own the deployment', function () { - // Create another team and server $otherTeam = Team::factory()->create(); $otherServer = Server::factory()->create(['team_id' => $otherTeam->id]); + $otherDestination = StandaloneDocker::where('server_id', $otherServer->id)->firstOrFail(); + $otherApplication = createCancellationApiApplication($otherTeam, $otherDestination); - // Create a deployment on the other team's server $deployment = ApplicationDeploymentQueue::create([ 'deployment_uuid' => 'test-deployment-uuid', - 'application_id' => 1, + 'application_id' => $otherApplication->id, 'server_id' => $otherServer->id, 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, ]); @@ -82,10 +96,30 @@ describe('POST /api/v1/deployments/{uuid}/cancel', function () { $response->assertJson(['message' => 'You do not have permission to cancel this deployment.']); }); + test('returns 403 for a deployment of an application the caller does not own', function () { + $otherTeam = Team::factory()->create(); + $otherApplication = createCancellationApiApplication($otherTeam, $this->destination); + + $deployment = ApplicationDeploymentQueue::create([ + 'deployment_uuid' => 'unowned-deployment-uuid', + 'application_id' => $otherApplication->id, + 'server_id' => $this->server->id, + 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, + ]); + + $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->postJson("/api/v1/deployments/{$deployment->deployment_uuid}/cancel") + ->assertForbidden(); + + expect($deployment->fresh()->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value); + }); + test('returns 400 when deployment is already finished', function () { $deployment = ApplicationDeploymentQueue::create([ 'deployment_uuid' => 'finished-deployment-uuid', - 'application_id' => 1, + 'application_id' => $this->application->id, 'server_id' => $this->server->id, 'status' => ApplicationDeploymentStatus::FINISHED->value, ]); @@ -102,7 +136,7 @@ describe('POST /api/v1/deployments/{uuid}/cancel', function () { test('returns 400 when deployment is already failed', function () { $deployment = ApplicationDeploymentQueue::create([ 'deployment_uuid' => 'failed-deployment-uuid', - 'application_id' => 1, + 'application_id' => $this->application->id, 'server_id' => $this->server->id, 'status' => ApplicationDeploymentStatus::FAILED->value, ]); @@ -119,7 +153,7 @@ describe('POST /api/v1/deployments/{uuid}/cancel', function () { test('returns 400 when deployment is already cancelled', function () { $deployment = ApplicationDeploymentQueue::create([ 'deployment_uuid' => 'cancelled-deployment-uuid', - 'application_id' => 1, + 'application_id' => $this->application->id, 'server_id' => $this->server->id, 'status' => ApplicationDeploymentStatus::CANCELLED_BY_USER->value, ]); @@ -138,7 +172,7 @@ describe('POST /api/v1/deployments/{uuid}/cancel', function () { $buildServer = Server::factory()->create(['team_id' => $otherTeam->id]); $deployment = ApplicationDeploymentQueue::create([ 'deployment_uuid' => 'queued-deployment-uuid', - 'application_id' => 1, + 'application_id' => $this->application->id, 'server_id' => $this->server->id, 'build_server_id' => $buildServer->id, 'status' => ApplicationDeploymentStatus::QUEUED->value, @@ -164,14 +198,8 @@ describe('POST /api/v1/deployments/{uuid}/cancel', function () { $otherTeam = Team::factory()->create(); $buildServer = Server::factory()->create(['team_id' => $otherTeam->id]); - $destination = StandaloneDocker::where('server_id', $this->server->id)->firstOrFail(); - $project = Project::factory()->create(['team_id' => $this->team->id]); - $environment = Environment::factory()->create(['project_id' => $project->id]); - $application = Application::factory()->create([ - 'environment_id' => $environment->id, - 'destination_id' => $destination->id, - 'destination_type' => $destination->getMorphClass(), - ]); + $destination = $this->destination; + $application = $this->application; $deployment = ApplicationDeploymentQueue::create([ 'deployment_uuid' => 'cancelled-queue-head-uuid', 'application_id' => $application->id, @@ -207,7 +235,7 @@ describe('POST /api/v1/deployments/{uuid}/cancel', function () { $deployment = ApplicationDeploymentQueue::create([ 'deployment_uuid' => 'atomic-cancellation-uuid', - 'application_id' => 1, + 'application_id' => $this->application->id, 'server_id' => $this->server->id, 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, ]); @@ -235,7 +263,7 @@ describe('POST /api/v1/deployments/{uuid}/cancel', function () { test('cancels in-progress deployment and updates status in database', function () { $deployment = ApplicationDeploymentQueue::create([ 'deployment_uuid' => 'in-progress-deployment-uuid', - 'application_id' => 1, + 'application_id' => $this->application->id, 'server_id' => $this->server->id, 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, ]); @@ -250,3 +278,30 @@ describe('POST /api/v1/deployments/{uuid}/cancel', function () { expect($deployment->status)->toBe(ApplicationDeploymentStatus::CANCELLED_BY_USER->value); }); }); + +describe('GET /api/v1/deployments', function () { + test('lists only deployments of the caller team applications', function () { + $otherTeam = Team::factory()->create(); + $otherApplication = createCancellationApiApplication($otherTeam, $this->destination); + + ApplicationDeploymentQueue::create([ + 'deployment_uuid' => 'own-deployment-uuid', + 'application_id' => $this->application->id, + 'server_id' => $this->server->id, + 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, + ]); + ApplicationDeploymentQueue::create([ + 'deployment_uuid' => 'unowned-deployment-uuid', + 'application_id' => $otherApplication->id, + 'server_id' => $this->server->id, + 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, + ]); + + $response = $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + ])->getJson('/api/v1/deployments'); + + $response->assertSuccessful(); + expect(collect($response->json())->pluck('deployment_uuid')->all())->toBe(['own-deployment-uuid']); + }); +}); diff --git a/tests/Feature/DashboardDeploymentsTest.php b/tests/Feature/DashboardDeploymentsTest.php index 6ef6f05ab0..805aa49af8 100644 --- a/tests/Feature/DashboardDeploymentsTest.php +++ b/tests/Feature/DashboardDeploymentsTest.php @@ -151,27 +151,28 @@ it('does not include active deployments in the recent list', function () { ->assertSee('Failed'); }); -it('only includes deployments for servers owned by the current team', function () { +it('only includes deployments of applications owned by the current team', function () { $otherTeam = Team::factory()->create(); - $otherServer = Server::factory()->create(['team_id' => $otherTeam->id]); + $otherProject = Project::factory()->create(['team_id' => $otherTeam->id]); + $otherEnvironment = Environment::factory()->create(['project_id' => $otherProject->id]); + $otherApplication = Application::factory()->create([ + 'environment_id' => $otherEnvironment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); createDashboardDeployment([ 'application_name' => 'Team Deployment', 'status' => ApplicationDeploymentStatus::FINISHED->value, ]); - - ApplicationDeploymentQueue::create([ - 'application_id' => $this->application->id, + createDashboardDeployment([ + 'application_id' => $otherApplication->id, 'application_name' => 'Other Team Deployment', - 'deployment_uuid' => 'deploy-other-'.fake()->uuid(), - 'deployment_url' => '/deployments/other', - 'server_id' => $otherServer->id, - 'server_name' => $otherServer->name, - 'status' => ApplicationDeploymentStatus::FINISHED->value, - 'pull_request_id' => 0, + 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, ]); Livewire::test(ActiveDeployments::class) ->assertSee('Team Deployment') - ->assertDontSee('Other Team Deployment'); + ->assertDontSee('Other Team Deployment') + ->assertDontSee('Running or queued right now'); }); diff --git a/tests/Feature/DeploymentShowMissingEnvironmentTest.php b/tests/Feature/DeploymentShowMissingEnvironmentTest.php index 92be923d98..9b9775ca48 100644 --- a/tests/Feature/DeploymentShowMissingEnvironmentTest.php +++ b/tests/Feature/DeploymentShowMissingEnvironmentTest.php @@ -1,6 +1,8 @@ 'missing-backup', ]))->assertNotFound(); }); + +it('does not show a deployment of another application', function () { + $otherTeam = Team::factory()->create(); + $otherProject = Project::factory()->create(['team_id' => $otherTeam->id]); + $otherEnvironment = Environment::factory()->create(['project_id' => $otherProject->id]); + $otherApplication = Application::factory()->create([ + 'environment_id' => $otherEnvironment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + $deployment = ApplicationDeploymentQueue::create([ + 'deployment_uuid' => 'unowned-deployment-uuid', + 'application_id' => $otherApplication->id, + 'server_id' => $this->server->id, + 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, + ]); + + $this->get(route('project.application.deployment.show', [ + 'project_uuid' => $this->project->uuid, + 'environment_uuid' => $this->environment->uuid, + 'application_uuid' => $this->application->uuid, + 'deployment_uuid' => $deployment->deployment_uuid, + ]))->assertRedirect(route('project.application.deployment.index', [ + 'project_uuid' => $this->project->uuid, + 'environment_uuid' => $this->environment->uuid, + 'application_uuid' => $this->application->uuid, + ])); +}); diff --git a/tests/Feature/DeploymentsIndicatorTest.php b/tests/Feature/DeploymentsIndicatorTest.php index faa6262ad2..5391826444 100644 --- a/tests/Feature/DeploymentsIndicatorTest.php +++ b/tests/Feature/DeploymentsIndicatorTest.php @@ -64,11 +64,11 @@ it('hides the indicator when no deployment is running', function () { ->assertDontSee('Active deployments'); }); -it('does not show deployments of another team', function () { +it('does not show deployments of applications owned by another team', function () { $otherTeam = Team::factory()->create(); - $otherServer = Server::factory()->create(['team_id' => $otherTeam->id]); + $otherProject = Project::factory()->create(['team_id' => $otherTeam->id]); - ApplicationDeploymentQueue::query()->update(['server_id' => $otherServer->id]); + $this->environment->update(['project_id' => $otherProject->id]); Livewire::test(DeploymentsIndicator::class) ->assertDontSee('Indicator App')