diff --git a/app/Actions/Proxy/StartProxy.php b/app/Actions/Proxy/StartProxy.php index 641e47469f..99992c0cae 100644 --- a/app/Actions/Proxy/StartProxy.php +++ b/app/Actions/Proxy/StartProxy.php @@ -48,13 +48,13 @@ class StartProxy if ($async) { $server->markProxyTransitionStarted(); - return remote_process($commands, $server, callEventOnFinish: 'ProxyStatusChanged', callEventData: $server->id, queue: deployment_queue()); + return remote_process($commands, $server, callEventOnFinish: 'ProxyStatusChanged', callEventData: ProxyStatusChanged::dataFor($server), queue: deployment_queue()); } else { instant_remote_process($commands, $server); $server->proxy->set('type', $proxyType); $server->save(); - ProxyStatusChanged::dispatch($server->id); + ProxyStatusChanged::dispatch(ProxyStatusChanged::dataFor($server)); return 'OK'; } diff --git a/app/Actions/Proxy/StopProxy.php b/app/Actions/Proxy/StopProxy.php index 881aa0252f..0fa045d84b 100644 --- a/app/Actions/Proxy/StopProxy.php +++ b/app/Actions/Proxy/StopProxy.php @@ -51,7 +51,7 @@ class StopProxy ProxyDashboardCacheService::clearCache($server); if (! $restarting) { - ProxyStatusChanged::dispatch($server->id); + ProxyStatusChanged::dispatch(ProxyStatusChanged::dataFor($server)); } } } diff --git a/app/Actions/Server/ConfigureCloudflared.php b/app/Actions/Server/ConfigureCloudflared.php index 0f7786d7cf..f0d5e6266f 100644 --- a/app/Actions/Server/ConfigureCloudflared.php +++ b/app/Actions/Server/ConfigureCloudflared.php @@ -54,6 +54,7 @@ class ConfigureCloudflared return remote_process($commands, $server, callEventOnFinish: 'CloudflareTunnelChanged', callEventData: [ 'server_id' => $server->id, + 'team_id' => $server->team_id, 'ssh_domain' => $ssh_domain, ]); } catch (\Throwable $e) { diff --git a/app/Actions/Service/DeployServiceApplication.php b/app/Actions/Service/DeployServiceApplication.php index abf66941a6..c5370eb51f 100644 --- a/app/Actions/Service/DeployServiceApplication.php +++ b/app/Actions/Service/DeployServiceApplication.php @@ -3,6 +3,7 @@ namespace App\Actions\Service; use App\Actions\Shared\EnsureContentFilesOnServer; +use App\Events\ServiceStartFinished; use App\Models\ServiceApplication; use App\Models\ServiceDatabase; use Lorisleiva\Actions\Concerns\AsAction; @@ -72,6 +73,6 @@ class DeployServiceApplication $commands->push("docker network connect {$safeProjectName} coolify-proxy >/dev/null 2>&1 || true"); - return remote_process($commands->toArray(), $service->server, type_uuid: $service->uuid, callEventOnFinish: 'ServiceStartFinished', callEventData: $service->id, queue: deployment_queue()); + return remote_process($commands->toArray(), $service->server, type_uuid: $service->uuid, callEventOnFinish: 'ServiceStartFinished', callEventData: ServiceStartFinished::dataFor($service), queue: deployment_queue()); } } diff --git a/app/Actions/Service/StartService.php b/app/Actions/Service/StartService.php index c9f0d6e079..fa4d517487 100644 --- a/app/Actions/Service/StartService.php +++ b/app/Actions/Service/StartService.php @@ -3,6 +3,7 @@ namespace App\Actions\Service; use App\Actions\Shared\EnsureContentFilesOnServer; +use App\Events\ServiceStartFinished; use App\Jobs\ServerFilesFromServerJob; use App\Models\LocalFileVolume; use App\Models\Service; @@ -84,7 +85,7 @@ class StartService } } - return remote_process($commands, $service->server, type_uuid: $service->uuid, callEventOnFinish: 'ServiceStartFinished', callEventData: $service->id, queue: deployment_queue()); + return remote_process($commands, $service->server, type_uuid: $service->uuid, callEventOnFinish: 'ServiceStartFinished', callEventData: ServiceStartFinished::dataFor($service), queue: deployment_queue()); } /** diff --git a/app/Events/ProxyStatusChanged.php b/app/Events/ProxyStatusChanged.php index 0cb775eb35..7dc455b935 100644 --- a/app/Events/ProxyStatusChanged.php +++ b/app/Events/ProxyStatusChanged.php @@ -2,6 +2,7 @@ namespace App\Events; +use App\Models\Server; use Illuminate\Broadcasting\InteractsWithSockets; use Illuminate\Foundation\Events\Dispatchable; use Illuminate\Queue\SerializesModels; @@ -10,5 +11,16 @@ class ProxyStatusChanged { use Dispatchable, InteractsWithSockets, SerializesModels; + /** + * @param array{server_id: int, team_id: int}|mixed $data Build it with dataFor(); the listener ignores any other shape. + */ public function __construct(public $data) {} + + /** + * @return array{server_id: int, team_id: int} + */ + public static function dataFor(Server $server): array + { + return ['server_id' => $server->id, 'team_id' => $server->team_id]; + } } diff --git a/app/Events/ServiceStartFinished.php b/app/Events/ServiceStartFinished.php index 3496a490ae..3ddde6b1a8 100644 --- a/app/Events/ServiceStartFinished.php +++ b/app/Events/ServiceStartFinished.php @@ -2,6 +2,7 @@ namespace App\Events; +use App\Models\Service; use Illuminate\Foundation\Events\Dispatchable; /** @@ -12,5 +13,16 @@ class ServiceStartFinished { use Dispatchable; - public function __construct(public int $serviceId) {} + /** + * @param array{service_id: int, team_id: int} $data Build it with dataFor(); the listener ignores a service of another team. + */ + public function __construct(public array $data) {} + + /** + * @return array{service_id: int, team_id: int} + */ + public static function dataFor(Service $service): array + { + return ['service_id' => $service->id, 'team_id' => $service->team()?->id]; + } } diff --git a/app/Jobs/RestartProxyJob.php b/app/Jobs/RestartProxyJob.php index 5008ff4ce9..45672fec70 100644 --- a/app/Jobs/RestartProxyJob.php +++ b/app/Jobs/RestartProxyJob.php @@ -5,6 +5,7 @@ namespace App\Jobs; use App\Actions\Proxy\GetProxyConfiguration; use App\Actions\Proxy\SaveProxyConfiguration; use App\Actions\Proxy\StartProxy; +use App\Events\ProxyStatusChanged; use App\Events\ProxyStatusChangedUI; use App\Models\Server; use App\Services\ProxyDashboardCacheService; @@ -62,7 +63,7 @@ class RestartProxyJob implements ShouldBeEncrypted, ShouldQueue $commands, $this->server, callEventOnFinish: 'ProxyStatusChanged', - callEventData: $this->server->id, + callEventData: ProxyStatusChanged::dataFor($this->server), queue: deployment_queue(), ); diff --git a/app/Listeners/CloudflareTunnelChangedNotification.php b/app/Listeners/CloudflareTunnelChangedNotification.php index c1804f0955..81c1f46ce1 100644 --- a/app/Listeners/CloudflareTunnelChangedNotification.php +++ b/app/Listeners/CloudflareTunnelChangedNotification.php @@ -16,13 +16,18 @@ class CloudflareTunnelChangedNotification public function handle(CloudflareTunnelChanged $event): void { $server_id = data_get($event, 'data.server_id'); + $team_id = data_get($event, 'data.team_id'); $ssh_domain = data_get($event, 'data.ssh_domain'); - - $this->server = Server::where('id', $server_id)->firstOrFail(); - if ($this->server->isTransferredAway()) { + if (is_null($server_id) || is_null($team_id)) { return; } + $server = Server::where('id', $server_id)->where('team_id', $team_id)->first(); + if (is_null($server) || $server->isTransferredAway()) { + return; + } + $this->server = $server; + // Check if cloudflare tunnel is running (container is healthy) - try 3 times with 5 second intervals $cloudflareHealthy = false; $attempts = 3; diff --git a/app/Listeners/ProxyStatusChangedNotification.php b/app/Listeners/ProxyStatusChangedNotification.php index 73b43d551c..7752e96519 100644 --- a/app/Listeners/ProxyStatusChangedNotification.php +++ b/app/Listeners/ProxyStatusChangedNotification.php @@ -16,11 +16,12 @@ class ProxyStatusChangedNotification implements ShouldQueueAfterCommit public function handle(ProxyStatusChanged $event) { - $serverId = $event->data; - if (is_null($serverId)) { + $serverId = data_get($event->data, 'server_id'); + $teamId = data_get($event->data, 'team_id'); + if (is_null($serverId) || is_null($teamId)) { return; } - $server = Server::where('id', $serverId)->first(); + $server = Server::where('id', $serverId)->where('team_id', $teamId)->first(); if (is_null($server)) { return; } diff --git a/app/Listeners/RefreshServiceStatusAfterStart.php b/app/Listeners/RefreshServiceStatusAfterStart.php index 094d9b3425..aa89034ce3 100644 --- a/app/Listeners/RefreshServiceStatusAfterStart.php +++ b/app/Listeners/RefreshServiceStatusAfterStart.php @@ -19,8 +19,9 @@ class RefreshServiceStatusAfterStart public function handle(ServiceStartFinished $event): void { - $service = Service::query()->find($event->serviceId); - if (! $service) { + $service = Service::query()->find($event->data['service_id'] ?? null); + $teamId = $event->data['team_id'] ?? null; + if (! $service || is_null($teamId) || $service->team()?->id !== $teamId) { return; } diff --git a/app/Livewire/ActivityMonitor.php b/app/Livewire/ActivityMonitor.php index 12095942c0..cabf9bd320 100644 --- a/app/Livewire/ActivityMonitor.php +++ b/app/Livewire/ActivityMonitor.php @@ -2,6 +2,7 @@ namespace App\Livewire; +use App\Events\ServiceStatusChanged; use App\Models\Server; use App\Models\Service; use App\Models\User; @@ -12,13 +13,27 @@ use Spatie\Activitylog\Models\Activity; class ActivityMonitor extends Component { + /** + * Events that may be dispatched when an activity finishes. The event name comes from the + * client, so application events must be listed here explicitly. + */ + private const ALLOWED_EVENTS = [ + 'activityFinished', + 'init', + 'prerequisitesInstalled', + 'sysboxInstalled', + ServiceStatusChanged::class, + ]; + public ?string $header = null; #[Locked] public $activityId = null; + #[Locked] public $eventToDispatch = 'activityFinished'; + #[Locked] public $eventData = null; public $isPollingActive = false; @@ -42,7 +57,7 @@ class ActivityMonitor extends Component self::$eventDispatched = false; $this->activityId = $activityId; - $this->eventToDispatch = $eventToDispatch; + $this->eventToDispatch = in_array($eventToDispatch, self::ALLOWED_EVENTS, true) ? $eventToDispatch : null; $this->eventData = $eventData; // Update header if provided @@ -130,11 +145,7 @@ class ActivityMonitor extends Component ?? $user->currentTeam()?->id ?? $user->teams->first()?->id; if ($teamId && ! self::$eventDispatched) { - if (filled($this->eventData)) { - $this->eventToDispatch::dispatch($teamId, $this->eventData); - } else { - $this->eventToDispatch::dispatch($teamId); - } + $this->eventToDispatch::dispatch($teamId); self::$eventDispatched = true; } } diff --git a/tests/Feature/ActivityMonitorCrossTeamTest.php b/tests/Feature/ActivityMonitorCrossTeamTest.php index 49d0b33491..54aefa1483 100644 --- a/tests/Feature/ActivityMonitorCrossTeamTest.php +++ b/tests/Feature/ActivityMonitorCrossTeamTest.php @@ -1,5 +1,8 @@ [0, 'finished'], 'failed' => [1, 'failed'], ]); + +describe('event dispatched when an activity finishes', function () { + beforeEach(function () { + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->activity = Activity::create([ + 'log_name' => 'default', + 'description' => 'finished activity', + 'causer_id' => $this->user->id, + 'causer_type' => User::class, + 'properties' => ['team_id' => $this->team->id, 'server_uuid' => $this->server->uuid, 'exitCode' => 0], + ]); + ActivityMonitor::$eventDispatched = false; + + $this->actingAs($this->user); + session(['currentTeam' => ['id' => $this->team->id]]); + }); + + test('the event properties are locked', function (string $property) { + Livewire::test(ActivityMonitor::class)->set($property, ProxyStatusChanged::class); + })->with(['eventToDispatch', 'eventData'])->throws(CannotUpdateLockedPropertyException::class); + + test('an event that is not allowed is not dispatched', function (string $event) { + Event::fake([ProxyStatusChanged::class, ServerValidated::class]); + + Livewire::test(ActivityMonitor::class) + ->call('newMonitorActivity', $this->activity->id, $event) + ->assertSet('eventToDispatch', null) + ->call('polling') + ->assertNotDispatched($event); + + Event::assertNotDispatched(ProxyStatusChanged::class); + Event::assertNotDispatched(ServerValidated::class); + })->with([ProxyStatusChanged::class, ServerValidated::class, 'someOtherEvent']); + + test('an allowed application event is dispatched with the team of the activity and no client data', function () { + Event::fake([ServiceStatusChanged::class]); + + Livewire::test(ActivityMonitor::class) + ->call('newMonitorActivity', $this->activity->id, ServiceStatusChanged::class, $this->otherTeam->id) + ->call('polling'); + + Event::assertDispatched(ServiceStatusChanged::class, fn (ServiceStatusChanged $event) => $event->teamId === $this->team->id); + }); + + test('allowed browser events are dispatched with their data', function (string $event, mixed $data) { + Livewire::test(ActivityMonitor::class) + ->call('newMonitorActivity', $this->activity->id, $event, $data) + ->call('polling') + ->assertDispatched($event); + })->with([ + ['activityFinished', null], + ['prerequisitesInstalled', null], + ['sysboxInstalled', null], + ['init', 2], + ]); +}); diff --git a/tests/Feature/Proxy/ProxyStatusChangedNotificationTest.php b/tests/Feature/Proxy/ProxyStatusChangedNotificationTest.php new file mode 100644 index 0000000000..2b7ebd43b5 --- /dev/null +++ b/tests/Feature/Proxy/ProxyStatusChangedNotificationTest.php @@ -0,0 +1,75 @@ + Server::flushIdentityMap()); + +beforeEach(function () { + Server::flushIdentityMap(); + InstanceSettings::forceCreate(['id' => 0]); + config(['constants.ssh.mux_enabled' => false]); + Process::fake(); + + $this->team = Team::factory()->create(); + $this->server = Server::factory()->create([ + 'team_id' => $this->team->id, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $this->team->id])->id, + ]); + $this->server->settings->update(['is_reachable' => true, 'is_usable' => true, 'force_disabled' => false]); + $this->server->markProxyTransitionStarted(); +}); + +test('the listener updates the proxy of the server of the event team', function () { + Event::fake([ProxyStatusChangedUI::class]); + + (new ProxyStatusChangedNotification)->handle(new ProxyStatusChanged(ProxyStatusChanged::dataFor($this->server))); + + expect($this->server->fresh()->isProxyTransitionInProgress())->toBeFalse(); + Process::assertRan(fn () => true); +}); + +test('the listener ignores a server that is not in the event team', function () { + $otherTeam = Team::factory()->create(); + + (new ProxyStatusChangedNotification)->handle(new ProxyStatusChanged([ + 'server_id' => $this->server->id, + 'team_id' => $otherTeam->id, + ])); + + expect($this->server->fresh()->isProxyTransitionInProgress())->toBeTrue(); + Process::assertNothingRan(); +}); + +test('the listener ignores event data without a team', function (mixed $data) { + (new ProxyStatusChangedNotification)->handle(new ProxyStatusChanged($data)); + + expect($this->server->fresh()->isProxyTransitionInProgress())->toBeTrue(); + Process::assertNothingRan(); +})->with([ + 'bare server id' => fn () => $this->server->id, + 'no team id' => fn () => ['server_id' => $this->server->id], +]); + +test('stopping the proxy dispatches the event with the server and its team', function () { + Event::fake([ProxyStatusChanged::class, ProxyStatusChangedUI::class]); + + StopProxy::run($this->server); + + Event::assertDispatched(ProxyStatusChanged::class, fn (ProxyStatusChanged $event) => $event->data === [ + 'server_id' => $this->server->id, + 'team_id' => $this->team->id, + ]); +}); diff --git a/tests/Feature/PushServerUpdateJobOptimizationTest.php b/tests/Feature/PushServerUpdateJobOptimizationTest.php index 0ae7a420cd..1083420121 100644 --- a/tests/Feature/PushServerUpdateJobOptimizationTest.php +++ b/tests/Feature/PushServerUpdateJobOptimizationTest.php @@ -216,7 +216,7 @@ it('ends the proxy restart when the proxy status listener runs', function () { $server->markProxyTransitionStarted(); Process::fake(); - (new ProxyStatusChangedNotification)->handle(new ProxyStatusChanged($server->id)); + (new ProxyStatusChangedNotification)->handle(new ProxyStatusChanged(ProxyStatusChanged::dataFor($server))); expect($server->isProxyTransitionInProgress())->toBeFalse(); }); diff --git a/tests/Feature/ResourceStatusRefresherTest.php b/tests/Feature/ResourceStatusRefresherTest.php index 35b589cad6..ad19885465 100644 --- a/tests/Feature/ResourceStatusRefresherTest.php +++ b/tests/Feature/ResourceStatusRefresherTest.php @@ -176,9 +176,29 @@ test('a finished service start stores the status of each service part and broadc inspectedContainer("db-{$service->uuid}", 'running', 'healthy', $labels('database', $db->uuid, 'db')), ])]); - event(new ServiceStartFinished($service->id)); + event(new ServiceStartFinished(ServiceStartFinished::dataFor($service))); expect($app->fresh()->status)->toBe('running:unknown') ->and($db->fresh()->status)->toBe('running:healthy'); Event::assertDispatched(ServiceStatusChanged::class, fn (ServiceStatusChanged $event) => $event->teamId === $this->team->id); }); + +test('a finished service start is ignored when the event team does not own the service', function (string $case) { + Event::fake([ServiceStatusChanged::class]); + Process::fake(); + $service = Service::factory()->create([ + 'environment_id' => $this->environment->id, + 'server_id' => $this->server->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + $data = match ($case) { + 'other team' => ['service_id' => $service->id, 'team_id' => Team::factory()->create()->id], + 'no team' => ['service_id' => $service->id], + }; + + event(new ServiceStartFinished($data)); + + Process::assertNothingRan(); + Event::assertNotDispatched(ServiceStatusChanged::class); +})->with(['other team', 'no team']); diff --git a/tests/Feature/Server/CloudflareTunnelChangedNotificationTest.php b/tests/Feature/Server/CloudflareTunnelChangedNotificationTest.php index a6481e4489..51ee5f3098 100644 --- a/tests/Feature/Server/CloudflareTunnelChangedNotificationTest.php +++ b/tests/Feature/Server/CloudflareTunnelChangedNotificationTest.php @@ -5,6 +5,7 @@ use App\Listeners\CloudflareTunnelChangedNotification; use App\Models\InstanceSettings; use App\Models\PrivateKey; use App\Models\Server; +use App\Models\Team; use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Event; @@ -41,9 +42,25 @@ it('uses sudo for the health check on a non-root server', function () { (new CloudflareTunnelChangedNotification)->handle(new CloudflareTunnelChanged([ 'server_id' => $this->server->id, + 'team_id' => $this->server->team_id, 'ssh_domain' => 'ssh.example.com', ])); expect($this->server->settings->refresh()->is_cloudflare_tunnel)->toBeTruthy() ->and($this->server->refresh()->ip)->toBe('ssh.example.com'); }); + +it('ignores a server that is not in the event team', function (string $case) { + Event::fake(); + Process::fake(); + $data = match ($case) { + 'other team' => ['server_id' => $this->server->id, 'team_id' => Team::factory()->create()->id, 'ssh_domain' => 'ssh.example.com'], + 'no team' => ['server_id' => $this->server->id, 'ssh_domain' => 'ssh.example.com'], + }; + + (new CloudflareTunnelChangedNotification)->handle(new CloudflareTunnelChanged($data)); + + Process::assertNothingRan(); + expect($this->server->settings->refresh()->is_cloudflare_tunnel)->toBeFalsy() + ->and($this->server->refresh()->ip)->not->toBe('ssh.example.com'); +})->with(['other team', 'no team']);