From 26634f6897dfe6dc69b13c580170290d0036821b Mon Sep 17 00:00:00 2001 From: Saeed Vaziry Date: Fri, 2 Oct 2026 19:08:23 +0200 Subject: [PATCH 1/2] [Fix] Enforce project authorization across web and API routes --- app/Actions/Backup/ManageBackup.php | 10 +- app/Actions/Server/TransferServer.php | 15 +- app/Actions/Site/CreateSite.php | 11 +- app/Actions/Site/UpdateSourceControl.php | 16 +- app/Actions/Workflow/RunWorkflow.php | 29 ++- .../API/FirewallRuleController.php | 4 +- app/Http/Controllers/API/SiteController.php | 2 +- app/Http/Controllers/API/WorkerController.php | 48 ++--- .../Controllers/ApplicationController.php | 4 + app/Http/Controllers/BackupController.php | 2 +- app/Http/Controllers/CommandController.php | 2 +- app/Http/Controllers/PHPController.php | 8 +- app/Http/Controllers/SiteController.php | 2 +- .../Controllers/SiteSettingController.php | 4 +- .../Workflow/WorkflowRunController.php | 8 + app/Http/Resources/SiteResource.php | 4 +- app/Jobs/Workflow/RunJob.php | 38 ++-- app/Models/SourceControl.php | 6 +- app/Policies/CommandPolicy.php | 2 + app/Policies/CronJobPolicy.php | 15 +- app/Policies/DatabasePolicy.php | 7 +- app/Policies/DatabaseUserPolicy.php | 14 +- app/Policies/NotificationChannelPolicy.php | 9 +- app/Policies/RedirectPolicy.php | 1 + app/Policies/ServicePolicy.php | 16 +- app/Policies/WorkerPolicy.php | 9 + app/SiteTypes/Blank.php | 9 + app/Traits/HasRolePolicies.php | 6 +- app/WorkflowActions/General/Notify.php | 8 +- app/WorkflowActions/Site/CreateSite.php | 1 + public/api-docs/openapi/schemas/Site.yaml | 1 + tests/Feature/API/FirewallTest.php | 50 +++++ tests/Feature/API/SiteCronjobTest.php | 2 +- tests/Feature/API/SitesTest.php | 118 +++++++++++ tests/Feature/API/WorkersTest.php | 140 ++++++++++++- tests/Feature/API/WorkflowRunTest.php | 114 ++++++++++- tests/Feature/ApplicationTest.php | 20 ++ tests/Feature/BackupTest.php | 51 +++++ tests/Feature/CommandsTest.php | 129 +++++++++++- tests/Feature/DatabaseTest.php | 29 +++ tests/Feature/DatabaseUserTest.php | 39 ++++ tests/Feature/Jobs/WorkflowRunJobTest.php | 184 ++++++++++++++++++ tests/Feature/PHPTest.php | 40 ++++ tests/Feature/RedirectsTest.php | 21 ++ tests/Feature/ServerTest.php | 64 +++++- tests/Feature/SiteCronjobTest.php | 85 ++++++++ tests/Feature/SitesTest.php | 122 ++++++++++++ tests/Feature/WorkersTest.php | 87 +++++++++ tests/Feature/WorkflowTest.php | 136 +++++++++++++ 49 files changed, 1632 insertions(+), 110 deletions(-) diff --git a/app/Actions/Backup/ManageBackup.php b/app/Actions/Backup/ManageBackup.php index 27008b3dd..40f36938c 100644 --- a/app/Actions/Backup/ManageBackup.php +++ b/app/Actions/Backup/ManageBackup.php @@ -8,8 +8,11 @@ use App\Jobs\Backup\DeleteJob; use App\Models\Backup; use App\Models\Server; +use App\Models\StorageProvider; +use App\Models\User; use App\ValidationRules\CronRule; use Illuminate\Auth\Access\AuthorizationException; +use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Validator; use Illuminate\Validation\Rule; use Illuminate\Validation\ValidationException; @@ -22,10 +25,12 @@ class ManageBackup * @throws AuthorizationException * @throws ValidationException */ - public function create(Server $server, array $input): Backup + public function create(Server $server, array $input, User $actor): Backup { $this->validate($server, $input); + Gate::forUser($actor)->authorize('view', StorageProvider::query()->findOrFail($input['storage'])); + $backupType = BackupType::from($input['type'] ?? BackupType::DATABASE->value); $backup = new Backup([ @@ -95,7 +100,8 @@ private function validate(Server $server, array $input): void ], 'storage' => [ 'required', - Rule::exists('storage_providers', 'id'), + Rule::exists('storage_providers', 'id') + ->where(fn ($query) => $query->where('project_id', $server->project_id)->orWhereNull('project_id')), ], 'keep' => [ 'required', diff --git a/app/Actions/Server/TransferServer.php b/app/Actions/Server/TransferServer.php index b34edc88b..852c6fbf5 100755 --- a/app/Actions/Server/TransferServer.php +++ b/app/Actions/Server/TransferServer.php @@ -2,8 +2,11 @@ namespace App\Actions\Server; +use App\Models\NetworkServer; +use App\Models\Project; use App\Models\Server; use App\Models\User; +use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Validator; use Illuminate\Validation\Rule; use Illuminate\Validation\ValidationException; @@ -19,7 +22,17 @@ public function transfer(User $user, Server $server, array $input): Server { $this->validate($user, $input); - $server->project_id = $input['project_id']; + $project = Project::query()->findOrFail($input['project_id']); + Gate::forUser($user)->authorize('create', [Server::class, $project]); + + if ($server->project_id !== $project->id + && NetworkServer::query()->where('server_id', $server->id)->exists()) { + throw ValidationException::withMessages([ + 'project_id' => 'Remove the server from its networks and wait for removal to complete before transferring it.', + ]); + } + + $server->project_id = $project->id; $server->save(); return $server; diff --git a/app/Actions/Site/CreateSite.php b/app/Actions/Site/CreateSite.php index 0054a1744..e8aedeb92 100755 --- a/app/Actions/Site/CreateSite.php +++ b/app/Actions/Site/CreateSite.php @@ -13,11 +13,14 @@ use App\Models\Server; use App\Models\Service; use App\Models\Site; +use App\Models\User; use App\Services\Webserver\Webserver; use App\Tooling\ToolingRegistry; use App\ValidationRules\DomainRule; use Exception; +use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Validator; use Illuminate\Validation\Rule; use Illuminate\Validation\ValidationException; @@ -32,7 +35,7 @@ class CreateSite * * @throws Throwable */ - public function create(Server $server, array $input): Site + public function create(Server $server, array $input, User $actor): Site { $input = $this->lockRuntimeVersionsToExistingUser($server, $input); @@ -78,6 +81,8 @@ public function create(Server $server, array $input): Site // check has access to repository try { if ($site->sourceControl) { + Gate::forUser($actor)->authorize('view', $site->sourceControl); + $site->sourceControl->getRepo($site->repository); } } catch (SourceControlIsNotConnected) { @@ -128,6 +133,10 @@ public function create(Server $server, array $input): Site return $site; } catch (Exception $e) { DB::rollBack(); + if ($e instanceof AuthorizationException) { + throw $e; + } + throw ValidationException::withMessages([ 'type' => $e->getMessage(), ]); diff --git a/app/Actions/Site/UpdateSourceControl.php b/app/Actions/Site/UpdateSourceControl.php index 84f486054..e6ca48814 100644 --- a/app/Actions/Site/UpdateSourceControl.php +++ b/app/Actions/Site/UpdateSourceControl.php @@ -8,8 +8,11 @@ use App\Exceptions\SSHError; use App\Models\Site; use App\Models\SourceControl; +use App\Models\User; use App\SSH\OS\Git; +use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Validator; use Illuminate\Validation\ValidationException; @@ -20,10 +23,11 @@ class UpdateSourceControl /** * @param array $input * + * @throws AuthorizationException * @throws ValidationException * @throws SSHError */ - public function update(Site $site, array $input): void + public function update(Site $site, array $input, User $actor): void { Validator::make($input, [ 'source_control' => SourceControl::siteValidationRules($site->server), @@ -31,10 +35,6 @@ public function update(Site $site, array $input): void $newSourceControlId = (int) $input['source_control']; - if ($site->source_control_id === $newSourceControlId) { - return; - } - $newSourceControl = SourceControl::find($newSourceControlId); if (! $newSourceControl instanceof SourceControl) { throw ValidationException::withMessages([ @@ -42,6 +42,12 @@ public function update(Site $site, array $input): void ]); } + Gate::forUser($actor)->authorize('view', $newSourceControl); + + if ($site->source_control_id === $newSourceControlId) { + return; + } + try { $newSourceControl->getRepo($site->repository); } catch (SourceControlIsNotConnected) { diff --git a/app/Actions/Workflow/RunWorkflow.php b/app/Actions/Workflow/RunWorkflow.php index 771fd599f..c29e30a7d 100644 --- a/app/Actions/Workflow/RunWorkflow.php +++ b/app/Actions/Workflow/RunWorkflow.php @@ -9,6 +9,8 @@ use App\Models\User; use App\Models\Workflow; use App\Models\WorkflowRun; +use Illuminate\Auth\Access\AuthorizationException; +use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Validator; class RunWorkflow @@ -44,12 +46,14 @@ public function run(User $user, Workflow $workflow, array $input): WorkflowRun return $run; } - public function executeAction(WorkflowRun $run, User $user, Workflow $workflow, ?WorkflowActionDTO $workflowActionDto, ?array $input): void + public function executeAction(WorkflowRun $run, User $user, Workflow $workflow, ?WorkflowActionDTO $workflowActionDto, ?array $input, ?int $accessTokenId = null): void { if (! $workflowActionDto) { return; } + $this->authorizeExecution($user, $workflow, $accessTokenId); + // Merge input with $workflowActionDto->inputs and resolve placeholders $resolvedInput = $this->resolveInputs($input ?? [], $workflowActionDto->inputs ?? []); @@ -61,11 +65,30 @@ public function executeAction(WorkflowRun $run, User $user, Workflow $workflow, try { $output = $workflowActionDto->handler($user, $workflow)->run($resolvedInput); - $this->executeAction($run, $user, $workflow, $workflowActionDto->success, $output); + $this->executeAction($run, $user, $workflow, $workflowActionDto->success, $output, $accessTokenId); } catch (\Throwable $e) { $run->log('Workflow action failed: '.$e->getMessage()); - $this->executeAction($run, $user, $workflow, $workflowActionDto->failure, $input); + $this->executeAction($run, $user, $workflow, $workflowActionDto->failure, $input, $accessTokenId); + } + } + + private function authorizeExecution(User $user, Workflow $workflow, ?int $accessTokenId): void + { + if ($accessTokenId !== null) { + $token = $user->tokens()->find($accessTokenId); + $expiration = config('sanctum.expiration'); + + if (! $token || + ($token->expires_at && $token->expires_at->isPast()) || + ($expiration && $token->created_at->lte(now()->subMinutes($expiration))) || + ! $token->can('write')) { + throw new AuthorizationException('The workflow API token is no longer valid.'); + } + + $user->withAccessToken($token); } + + Gate::forUser($user)->authorize('update', $workflow); } /** diff --git a/app/Http/Controllers/API/FirewallRuleController.php b/app/Http/Controllers/API/FirewallRuleController.php index 4317772cc..6239bb223 100644 --- a/app/Http/Controllers/API/FirewallRuleController.php +++ b/app/Http/Controllers/API/FirewallRuleController.php @@ -47,9 +47,9 @@ public function create(Request $request, Project $project, Server $server): Fire #[Put('{firewallRule}', name: 'api.projects.servers.firewall-rules.edit', middleware: 'ability:write')] public function edit(Request $request, Project $project, Server $server, FirewallRule $firewallRule): FirewallRuleResource { - $this->authorize('update', [FirewallRule::class, $firewallRule]); + $this->validateRoute($project, $server, $firewallRule); - $this->validateRoute($project, $server); + $this->authorize('update', [FirewallRule::class, $firewallRule]); $firewallRule = app(ManageRule::class)->update($firewallRule, $request->all()); diff --git a/app/Http/Controllers/API/SiteController.php b/app/Http/Controllers/API/SiteController.php index a45ab4d5c..1cb924abb 100644 --- a/app/Http/Controllers/API/SiteController.php +++ b/app/Http/Controllers/API/SiteController.php @@ -53,7 +53,7 @@ public function create(Request $request, Project $project, Server $server): Site $this->validateRoute($project, $server); - $site = app(CreateSite::class)->create($server, $request->all()); + $site = app(CreateSite::class)->create($server, $request->all(), $request->user()); return new SiteResource($site); } diff --git a/app/Http/Controllers/API/WorkerController.php b/app/Http/Controllers/API/WorkerController.php index 9b7e9bed4..0b87daf97 100644 --- a/app/Http/Controllers/API/WorkerController.php +++ b/app/Http/Controllers/API/WorkerController.php @@ -34,10 +34,10 @@ class WorkerController extends Controller #[Get('/workers', name: 'api.projects.servers.workers', middleware: 'ability:read')] public function serverIndex(Project $project, Server $server): ResourceCollection { - $this->authorize('view', [$project, $server]); - $this->validateRoute($project, $server); + $this->authorize('viewAny', [Worker::class, $server]); + $workers = $server->workers() ->latest() ->simplePaginate(25); @@ -48,10 +48,10 @@ public function serverIndex(Project $project, Server $server): ResourceCollectio #[Get('/sites/{site}/workers', name: 'api.projects.servers.sites.workers', middleware: 'ability:read')] public function siteIndex(Project $project, Server $server, Site $site): ResourceCollection { - $this->authorize('view', [$project, $server, $site]); - $this->validateRoute($project, $server, $site); + $this->authorize('viewAny', [Worker::class, $server, $site]); + $workers = $site->workers() ->latest() ->simplePaginate(25); @@ -62,20 +62,20 @@ public function siteIndex(Project $project, Server $server, Site $site): Resourc #[Get('/workers/{worker}', name: 'api.projects.servers.workers.show', middleware: 'ability:read')] public function serverShow(Project $project, Server $server, Worker $worker): WorkerResource { - $this->authorize('view', [$project, $server, $worker]); - $this->validateRoute($project, $server, worker: $worker); + $this->authorize('view', [$worker, $server]); + return new WorkerResource($worker); } #[Get('/sites/{site}/workers/{worker}', name: 'api.projects.servers.sites.workers.show', middleware: 'ability:read')] public function siteShow(Project $project, Server $server, Site $site, Worker $worker): WorkerResource { - $this->authorize('view', [$project, $server, $site, $worker]); - $this->validateRoute($project, $server, $site, $worker); + $this->authorize('view', [$worker, $server, $site]); + return new WorkerResource($worker); } @@ -83,10 +83,10 @@ public function siteShow(Project $project, Server $server, Site $site, Worker $w #[WhereNumber('site')] public function resync(Project $project, Server $server, ?Site $site = null): JsonResponse { - $this->authorize('update', [$project, $server]); - $this->validateRoute($project, $server, $site); + $this->authorize('manage', [Worker::class, $server, $site]); + $count = app(SyncWorkerStatuses::class)->sync($server, $site); return response()->json([ @@ -98,10 +98,10 @@ public function resync(Project $project, Server $server, ?Site $site = null): Js #[WhereNumber('site')] public function restartAll(Project $project, Server $server, ?Site $site = null): JsonResponse { - $this->authorize('update', [$project, $server]); - $this->validateRoute($project, $server, $site); + $this->authorize('manage', [Worker::class, $server, $site]); + app(RestartAllWorkers::class)->restart($server, $site); return response()->json([ @@ -113,10 +113,10 @@ public function restartAll(Project $project, Server $server, ?Site $site = null) #[WhereNumber('site')] public function create(Request $request, Project $project, Server $server, ?Site $site = null): WorkerResource { - $this->authorize('create', [$project, $server, $site]); - $this->validateRoute($project, $server, $site); + $this->authorize('create', [Worker::class, $server, $site]); + $worker = app(CreateWorker::class)->create($server, $request->all(), $site); return new WorkerResource($worker); @@ -125,10 +125,10 @@ public function create(Request $request, Project $project, Server $server, ?Site #[Put('/workers/{worker}/{site?}', name: 'api.projects.servers.workers.update', middleware: 'ability:write')] public function update(Request $request, Project $project, Server $server, Worker $worker, ?Site $site = null): WorkerResource { - $this->authorize('update', [$project, $server, $site]); - $this->validateRoute($project, $server, $site, $worker); + $this->authorize('update', [$worker, $server, $site]); + $worker = app(EditWorker::class)->edit($worker, $request->all()); return new WorkerResource($worker); @@ -137,10 +137,10 @@ public function update(Request $request, Project $project, Server $server, Worke #[Post('/workers/{worker}/start', name: 'api.projects.servers.workers.start', middleware: 'ability:write')] public function start(Request $request, Project $project, Server $server, Worker $worker): WorkerResource { - $this->authorize('update', [$project, $server]); - $this->validateRoute($project, $server, worker: $worker); + $this->authorize('update', [$worker, $server]); + app(ManageWorker::class)->start($worker); return new WorkerResource($worker); @@ -149,10 +149,10 @@ public function start(Request $request, Project $project, Server $server, Worker #[Post('/workers/{worker}/restart', name: 'api.projects.servers.workers.restart', middleware: 'ability:write')] public function restart(Request $request, Project $project, Server $server, Worker $worker): WorkerResource { - $this->authorize('update', [$project, $server]); - $this->validateRoute($project, $server, worker: $worker); + $this->authorize('update', [$worker, $server]); + app(ManageWorker::class)->restart($worker); return new WorkerResource($worker); @@ -161,10 +161,10 @@ public function restart(Request $request, Project $project, Server $server, Work #[Get('/workers/{worker}/logs', name: 'api.projects.servers.workers.logs', middleware: 'ability:read')] public function logs(Project $project, Server $server, Worker $worker): JsonResponse { - $this->authorize('view', [$project, $server, $worker]); - $this->validateRoute($project, $server, worker: $worker); + $this->authorize('view', [$worker, $server]); + $logs = app(GetWorkerLogs::class)->getLogs($worker); return response()->json([ @@ -175,10 +175,10 @@ public function logs(Project $project, Server $server, Worker $worker): JsonResp #[Delete('/workers/{worker}/{site?}', name: 'api.projects.servers.workers.delete', middleware: 'ability:write')] public function delete(Request $request, Project $project, Server $server, Worker $worker, ?Site $site = null): Response { - $this->authorize('delete', [$project, $server, $site, $worker]); - $this->validateRoute($project, $server, $site, $worker); + $this->authorize('delete', [$worker, $server, $site]); + app(DeleteWorker::class)->delete($worker); return response()->noContent(); diff --git a/app/Http/Controllers/ApplicationController.php b/app/Http/Controllers/ApplicationController.php index 7136e74d6..f99d18ab9 100644 --- a/app/Http/Controllers/ApplicationController.php +++ b/app/Http/Controllers/ApplicationController.php @@ -70,6 +70,10 @@ public function updateScript(Request $request, Server $server, Site $site, Deplo { $this->authorize('update', [$site, $server]); + if ($deploymentScript->site_id !== $site->id) { + abort(404); + } + app(UpdateDeploymentScript::class)->update($deploymentScript, $request->input()); return back()->with('success', 'Deployment script updated successfully.'); diff --git a/app/Http/Controllers/BackupController.php b/app/Http/Controllers/BackupController.php index 0faed851f..597dd3668 100644 --- a/app/Http/Controllers/BackupController.php +++ b/app/Http/Controllers/BackupController.php @@ -62,7 +62,7 @@ public function store(Request $request, Server $server): RedirectResponse { $this->authorize('create', [Backup::class, $server]); - app(ManageBackup::class)->create($server, $request->all()); + app(ManageBackup::class)->create($server, $request->all(), $request->user()); return back() ->with('info', 'Backup is being created...'); diff --git a/app/Http/Controllers/CommandController.php b/app/Http/Controllers/CommandController.php index 997fcc2f1..039d3e4f7 100644 --- a/app/Http/Controllers/CommandController.php +++ b/app/Http/Controllers/CommandController.php @@ -82,7 +82,7 @@ public function destroy(Server $server, Site $site, Command $command): RedirectR #[Post('/{command}/execute', name: 'commands.execute')] public function execute(Request $request, Server $server, Site $site, Command $command): RedirectResponse { - $this->authorize('update', [$site, $server]); + $this->authorize('update', [$command, $site, $server]); app(ExecuteCommand::class)->execute($command, user(), $request->input()); diff --git a/app/Http/Controllers/PHPController.php b/app/Http/Controllers/PHPController.php index b0573154c..4eba0cfda 100644 --- a/app/Http/Controllers/PHPController.php +++ b/app/Http/Controllers/PHPController.php @@ -47,7 +47,7 @@ public function index(Server $server): Response #[Get('/{service}/ini', name: 'php.ini')] public function ini(Request $request, Server $server, Service $service): JsonResponse { - $this->authorize('view', $service); + $this->authorize('view', [$service, $server]); $ini = app(GetPHPIni::class)->getIni($server, $request->input()); @@ -59,7 +59,7 @@ public function ini(Request $request, Server $server, Service $service): JsonRes #[Patch('/{service}/ini', name: 'php.ini.update')] public function updateIni(Request $request, Server $server, Service $service): RedirectResponse { - $this->authorize('update', $service); + $this->authorize('update', [$service, $server]); app(UpdatePHPIni::class)->update($server, $request->input()); @@ -69,7 +69,7 @@ public function updateIni(Request $request, Server $server, Service $service): R #[Post('/{service}/install-extension', name: 'php.install-extension')] public function installExtension(Request $request, Server $server, Service $service): RedirectResponse { - $this->authorize('update', $service); + $this->authorize('update', [$service, $server]); app(InstallPHPExtension::class)->install($server, $request->input()); @@ -82,7 +82,7 @@ public function installExtension(Request $request, Server $server, Service $serv #[Post('/{service}/default-cli', name: 'php.default-cli')] public function defaultCli(Request $request, Server $server, Service $service): RedirectResponse { - $this->authorize('update', $service); + $this->authorize('update', [$service, $server]); app(ChangeDefaultCli::class)->change($server, $request->input()); diff --git a/app/Http/Controllers/SiteController.php b/app/Http/Controllers/SiteController.php index 563127990..f2e03cc28 100644 --- a/app/Http/Controllers/SiteController.php +++ b/app/Http/Controllers/SiteController.php @@ -75,7 +75,7 @@ public function store(Request $request, Server $server): RedirectResponse { $this->authorize('create', [Site::class, $server]); - $site = app(CreateSite::class)->create($server, $request->all()); + $site = app(CreateSite::class)->create($server, $request->all(), $request->user()); return redirect()->route('application', ['server' => $server, 'site' => $site]) ->with('info', 'Installing site, please wait...'); diff --git a/app/Http/Controllers/SiteSettingController.php b/app/Http/Controllers/SiteSettingController.php index c3af0f12e..4716b6257 100644 --- a/app/Http/Controllers/SiteSettingController.php +++ b/app/Http/Controllers/SiteSettingController.php @@ -48,6 +48,8 @@ class SiteSettingController extends Controller #[Get('/', name: 'site-settings')] public function index(Server $server, Site $site): Response { + $this->authorize('view', [$site, $server]); + return Inertia::render('site-settings/index', [ 'sourceControl' => $site->sourceControl ? SourceControlResource::make($site->sourceControl) : null, ]); @@ -71,7 +73,7 @@ public function updateSourceControl(Request $request, Server $server, Site $site { $this->authorize('update', [$site, $server]); - app(UpdateSourceControl::class)->update($site, $request->input()); + app(UpdateSourceControl::class)->update($site, $request->input(), $request->user()); return back()->with('success', 'Source control updated successfully.'); } diff --git a/app/Http/Controllers/Workflow/WorkflowRunController.php b/app/Http/Controllers/Workflow/WorkflowRunController.php index 6e678a64a..1116eb827 100644 --- a/app/Http/Controllers/Workflow/WorkflowRunController.php +++ b/app/Http/Controllers/Workflow/WorkflowRunController.php @@ -49,6 +49,10 @@ public function show(Workflow $workflow, WorkflowRun $workflowRun): Response { $this->authorize('view', $workflow); + if ($workflowRun->workflow_id !== $workflow->id) { + abort(404, 'Workflow run not found for this workflow'); + } + return inertia('workflow-runs/show', [ 'workflow' => WorkflowResource::make($workflow), 'workflowRun' => WorkflowRunResource::make($workflowRun), @@ -60,6 +64,10 @@ public function log(Workflow $workflow, WorkflowRun $workflowRun): string { $this->authorize('view', $workflow); + if ($workflowRun->workflow_id !== $workflow->id) { + abort(404, 'Workflow run not found for this workflow'); + } + return $workflowRun->getLogContent(); } } diff --git a/app/Http/Resources/SiteResource.php b/app/Http/Resources/SiteResource.php index 19b878d77..3c4b243b3 100644 --- a/app/Http/Resources/SiteResource.php +++ b/app/Http/Resources/SiteResource.php @@ -72,7 +72,7 @@ public function toArray(Request $request): array } /** - * Strip basic-auth password hashes from type_data before sending to the client. + * Strip installation passwords and basic-auth hashes before sending to the client. * * @return array */ @@ -80,7 +80,7 @@ private function sanitisedTypeData(): array { $typeData = $this->type_data ?? []; - unset($typeData['php']); + unset($typeData['php'], $typeData['password'], $typeData['database_password']); if (isset($typeData['basic_auth']['users']) && is_array($typeData['basic_auth']['users'])) { $typeData['basic_auth']['users'] = array_map( diff --git a/app/Jobs/Workflow/RunJob.php b/app/Jobs/Workflow/RunJob.php index 6de05f759..5148ce8e7 100644 --- a/app/Jobs/Workflow/RunJob.php +++ b/app/Jobs/Workflow/RunJob.php @@ -9,6 +9,7 @@ use App\Events\SocketEvent; use App\Facades\SSH; use App\Http\Resources\WorkflowRunResource; +use App\Models\PersonalAccessToken; use App\Models\User; use App\Models\Workflow; use App\Models\WorkflowRun; @@ -23,38 +24,51 @@ class RunJob implements ShouldQueue use Queueable; use UniqueQueue; + /** + * Jobs queued before token context was captured must fail closed. + */ + protected ?int $accessTokenId = 0; + public function __construct( protected WorkflowRun $run, protected User $user, protected Workflow $workflow, protected WorkflowActionDTO $executionTree, protected array $input, - ) {} + ) { + $token = $user->currentAccessToken(); + $this->accessTokenId = $token instanceof PersonalAccessToken ? $token->id : null; + } public function handle(): void { $this->run("workflow-{$this->workflow->id}", function () { - // set all queue drivers to sync for underlying actions - config()->set('queue.connections.ssh.driver', 'sync'); - config()->set('queue.connections.default.driver', 'sync'); + $token = $this->user->currentAccessToken(); + $queueDrivers = config()->get(['queue.connections.ssh.driver', 'queue.connections.default.driver']); - if ($this->run->verbose && $this->run->log_disk && $this->run->log_path) { - SSH::useLog($this->run->log_disk, $this->run->log_path, function (string $chunk): void { - $this->broadcastLogContent($chunk); - }); - } - if ($this->input['inputs']) { - $this->executionTree->inputs = $this->input['inputs']; - } try { + config()->set('queue.connections.ssh.driver', 'sync'); + config()->set('queue.connections.default.driver', 'sync'); + + if ($this->run->verbose && $this->run->log_disk && $this->run->log_path) { + SSH::useLog($this->run->log_disk, $this->run->log_path, function (string $chunk): void { + $this->broadcastLogContent($chunk); + }); + } + if ($this->input['inputs'] ?? []) { + $this->executionTree->inputs = $this->input['inputs']; + } app(RunWorkflow::class)->executeAction( $this->run, $this->user, $this->workflow, $this->executionTree, $this->input['inputs'] ?? [], + $this->accessTokenId, ); } finally { + $this->user->withAccessToken($token); + config()->set($queueDrivers); SSH::clearLog(); } $this->run->status = WorkflowRunStatus::COMPLETED; diff --git a/app/Models/SourceControl.php b/app/Models/SourceControl.php index 830a3ac77..549525221 100755 --- a/app/Models/SourceControl.php +++ b/app/Models/SourceControl.php @@ -73,11 +73,9 @@ public static function siteValidationRules(Server $server): array 'required', Rule::exists('source_controls', 'id') ->whereIn('provider', $usableProviders) + ->withoutTrashed() ->where(function ($query) use ($server): void { - $query->where('user_id', $server->user_id) - ->where(function ($q) use ($server): void { - $q->where('project_id', $server->project_id)->orWhereNull('project_id'); - }); + $query->where('project_id', $server->project_id)->orWhereNull('project_id'); }), ]; } diff --git a/app/Policies/CommandPolicy.php b/app/Policies/CommandPolicy.php index 9c2c2a138..cbeef8c3c 100644 --- a/app/Policies/CommandPolicy.php +++ b/app/Policies/CommandPolicy.php @@ -17,6 +17,7 @@ class CommandPolicy public function viewAny(User $user, Site $site, Server $server): bool { return $this->hasReadAccess($user, $server->project) && + $site->server_id === $server->id && $server->isReady() && $site->isReady(); } @@ -33,6 +34,7 @@ public function view(User $user, Command $command, Site $site, Server $server): public function create(User $user, Site $site, Server $server): bool { return $this->hasWriteAccess($user, $server->project) && + $site->server_id === $server->id && $server->isReady() && $site->isReady(); } diff --git a/app/Policies/CronJobPolicy.php b/app/Policies/CronJobPolicy.php index e57950cca..3fe0513b8 100644 --- a/app/Policies/CronJobPolicy.php +++ b/app/Policies/CronJobPolicy.php @@ -16,7 +16,8 @@ class CronJobPolicy public function viewAny(User $user, Server $server, ?Site $site = null): bool { - return $this->hasReadAccess($user, $server->project) && $server->isReady(); + return $this->hasReadAccess($user, $server->project) && $server->isReady() && + (! $site || $site->server_id === $server->id); } public function view(User $user, CronJob $cronjob, Server $server, ?Site $site = null): bool @@ -25,12 +26,14 @@ public function view(User $user, CronJob $cronjob, Server $server, ?Site $site = return $this->hasReadAccess($user, $cronJobServer->project) && $cronJobServer->isReady() && - $cronjob->server_id === $server->id; + $cronjob->server_id === $server->id && + (! $site || ($site->server_id === $server->id && $cronjob->site_id === $site->id)); } public function create(User $user, Server $server, ?Site $site = null): bool { - return $this->hasWriteAccess($user, $server->project) && $server->isReady(); + return $this->hasWriteAccess($user, $server->project) && $server->isReady() && + (! $site || $site->server_id === $server->id); } public function update(User $user, CronJob $cronjob, Server $server, ?Site $site = null): bool @@ -39,7 +42,8 @@ public function update(User $user, CronJob $cronjob, Server $server, ?Site $site return $this->hasWriteAccess($user, $cronJobServer->project) && $cronJobServer->isReady() && - $cronjob->server_id === $server->id; + $cronjob->server_id === $server->id && + (! $site || ($site->server_id === $server->id && $cronjob->site_id === $site->id)); } public function delete(User $user, CronJob $cronjob, Server $server, ?Site $site = null): bool @@ -48,6 +52,7 @@ public function delete(User $user, CronJob $cronjob, Server $server, ?Site $site return $this->hasWriteAccess($user, $cronJobServer->project) && $cronJobServer->isReady() && - $cronjob->server_id === $server->id; + $cronjob->server_id === $server->id && + (! $site || ($site->server_id === $server->id && $cronjob->site_id === $site->id)); } } diff --git a/app/Policies/DatabasePolicy.php b/app/Policies/DatabasePolicy.php index c47a71dd5..bfa755fe8 100644 --- a/app/Policies/DatabasePolicy.php +++ b/app/Policies/DatabasePolicy.php @@ -45,11 +45,12 @@ public function update(User $user, Database $database): bool $server->database(); } - public function delete(User $user, Database $database): bool + public function delete(User $user, Database $database, ?Server $server = null): bool { - $server = $database->server; + $server ??= $database->server; - return $this->hasWriteAccess($user, $server->project) && + return $database->server_id === $server->id && + $this->hasWriteAccess($user, $server->project) && $server->isReady() && $server->database(); } diff --git a/app/Policies/DatabaseUserPolicy.php b/app/Policies/DatabaseUserPolicy.php index aa1f87a52..fcb9d5768 100644 --- a/app/Policies/DatabaseUserPolicy.php +++ b/app/Policies/DatabaseUserPolicy.php @@ -36,20 +36,22 @@ public function create(User $user, Server $server): bool && $server->database(); } - public function update(User $user, DatabaseUser $databaseUser): bool + public function update(User $user, DatabaseUser $databaseUser, ?Server $server = null): bool { - $server = $databaseUser->server; + $server ??= $databaseUser->server; - return $this->hasWriteAccess($user, $server->project) && + return $databaseUser->server_id === $server->id && + $this->hasWriteAccess($user, $server->project) && $server->isReady() && $server->database(); } - public function delete(User $user, DatabaseUser $databaseUser): bool + public function delete(User $user, DatabaseUser $databaseUser, ?Server $server = null): bool { - $server = $databaseUser->server; + $server ??= $databaseUser->server; - return $this->hasWriteAccess($user, $server->project) && + return $databaseUser->server_id === $server->id && + $this->hasWriteAccess($user, $server->project) && $server->isReady() && $server->database(); } diff --git a/app/Policies/NotificationChannelPolicy.php b/app/Policies/NotificationChannelPolicy.php index 71c6a0cb6..4e14c33ac 100644 --- a/app/Policies/NotificationChannelPolicy.php +++ b/app/Policies/NotificationChannelPolicy.php @@ -17,7 +17,8 @@ public function viewAny(User $user): bool public function view(User $user, NotificationChannel $notificationChannel): bool { - return $user->id === $notificationChannel->user_id; + return $user->id === $notificationChannel->user_id + && $user->tokenAllowsProject($notificationChannel->project_id); } public function create(User $user): bool @@ -27,11 +28,13 @@ public function create(User $user): bool public function update(User $user, NotificationChannel $notificationChannel): bool { - return $user->id === $notificationChannel->user_id; + return $user->id === $notificationChannel->user_id + && $user->tokenAllowsProject($notificationChannel->project_id, write: true); } public function delete(User $user, NotificationChannel $notificationChannel): bool { - return $user->id === $notificationChannel->user_id; + return $user->id === $notificationChannel->user_id + && $user->tokenAllowsProject($notificationChannel->project_id, write: true); } } diff --git a/app/Policies/RedirectPolicy.php b/app/Policies/RedirectPolicy.php index ffff80e47..13514c8ac 100644 --- a/app/Policies/RedirectPolicy.php +++ b/app/Policies/RedirectPolicy.php @@ -15,6 +15,7 @@ class RedirectPolicy public function viewAny(User $user, Site $site, Server $server): bool { return $this->hasReadAccess($user, $server->project) && + $site->server_id === $server->id && $server->isReady() && $site->isReady(); } diff --git a/app/Policies/ServicePolicy.php b/app/Policies/ServicePolicy.php index 81738fd76..39b6b1c43 100644 --- a/app/Policies/ServicePolicy.php +++ b/app/Policies/ServicePolicy.php @@ -18,11 +18,13 @@ public function viewAny(User $user, Server $server): bool return $this->hasReadAccess($user, $server->project) && $server->isReady(); } - public function view(User $user, Service $service): bool + public function view(User $user, Service $service, ?Server $server = null): bool { - $server = $service->server; + $server ??= $service->server; - return $this->hasReadAccess($user, $server->project) && $server->isReady(); + return $service->server_id === $server->id && + $this->hasReadAccess($user, $server->project) && + $server->isReady(); } public function create(User $user, Server $server): bool @@ -30,11 +32,13 @@ public function create(User $user, Server $server): bool return $this->hasWriteAccess($user, $server->project) && $server->isReady(); } - public function update(User $user, Service $service): bool + public function update(User $user, Service $service, ?Server $server = null): bool { - $server = $service->server; + $server ??= $service->server; - return $this->hasWriteAccess($user, $server->project) && $server->isReady(); + return $service->server_id === $server->id && + $this->hasWriteAccess($user, $server->project) && + $server->isReady(); } public function delete(User $user, Service $service): bool diff --git a/app/Policies/WorkerPolicy.php b/app/Policies/WorkerPolicy.php index 01f4907b8..265e09796 100644 --- a/app/Policies/WorkerPolicy.php +++ b/app/Policies/WorkerPolicy.php @@ -18,6 +18,7 @@ public function viewAny(User $user, Server $server, ?Site $site = null): bool { return $this->hasReadAccess($user, $server->project) && $server->isReady() && + (! $site || $site->server_id === $server->id) && $server->processManager(); } @@ -25,7 +26,9 @@ public function view(User $user, Worker $worker, Server $server, ?Site $site = n { return $this->hasReadAccess($user, $server->project) && $server->isReady() && + (! $site || $site->server_id === $server->id) && $worker->server_id === $server->id && + (! $site || $worker->site_id === $site->id) && $server->processManager(); } @@ -33,6 +36,7 @@ public function create(User $user, Server $server, ?Site $site = null): bool { return $this->hasWriteAccess($user, $server->project) && $server->isReady() && + (! $site || $site->server_id === $server->id) && $server->processManager(); } @@ -40,6 +44,7 @@ public function manage(User $user, Server $server, ?Site $site = null): bool { return $this->hasWriteAccess($user, $server->project) && $server->isReady() && + (! $site || $site->server_id === $server->id) && $server->processManager(); } @@ -47,7 +52,9 @@ public function update(User $user, Worker $worker, Server $server, ?Site $site = { return $this->hasWriteAccess($user, $server->project) && $server->isReady() && + (! $site || $site->server_id === $server->id) && $worker->server_id === $server->id && + (! $site || $worker->site_id === $site->id) && $server->processManager(); } @@ -55,7 +62,9 @@ public function delete(User $user, Worker $worker, Server $server, ?Site $site = { return $this->hasWriteAccess($user, $server->project) && $server->isReady() && + (! $site || $site->server_id === $server->id) && $worker->server_id === $server->id && + (! $site || $worker->site_id === $site->id) && $server->processManager() && ! $worker->isSiteBootstrap(); } diff --git a/app/SiteTypes/Blank.php b/app/SiteTypes/Blank.php index 42818038e..ff5b029b9 100644 --- a/app/SiteTypes/Blank.php +++ b/app/SiteTypes/Blank.php @@ -58,6 +58,15 @@ public function createRules(array $input): array return $rules; } + public function createFields(array $input): array + { + if (empty($input['use_source_control'])) { + unset($input['source_control'], $input['repository'], $input['branch']); + } + + return parent::createFields($input); + } + public function defaultDeploymentScript(): string { if (! $this->site->repository) { diff --git a/app/Traits/HasRolePolicies.php b/app/Traits/HasRolePolicies.php index 14405cfb6..d0aa1ddd1 100644 --- a/app/Traits/HasRolePolicies.php +++ b/app/Traits/HasRolePolicies.php @@ -10,7 +10,7 @@ trait HasRolePolicies { protected function hasReadAccess(User $user, Project $project): bool { - return $project->hasRoles($user, [ + return $user->tokenAllowsProject($project->id) && $project->hasRoles($user, [ UserRole::OWNER, UserRole::ADMIN, UserRole::USER, @@ -19,7 +19,7 @@ protected function hasReadAccess(User $user, Project $project): bool protected function hasWriteAccess(User $user, Project $project): bool { - return $project->hasRoles($user, [ + return $user->tokenAllowsProject($project->id, write: true) && $project->hasRoles($user, [ UserRole::OWNER, UserRole::ADMIN, ]); @@ -27,7 +27,7 @@ protected function hasWriteAccess(User $user, Project $project): bool protected function hasOwnerAccess(User $user, Project $project): bool { - return $project->hasRoles($user, [ + return $user->tokenAllowsProject($project->id, write: true) && $project->hasRoles($user, [ UserRole::OWNER, ]); } diff --git a/app/WorkflowActions/General/Notify.php b/app/WorkflowActions/General/Notify.php index 694fe0413..bc5b394b0 100644 --- a/app/WorkflowActions/General/Notify.php +++ b/app/WorkflowActions/General/Notify.php @@ -2,9 +2,7 @@ namespace App\WorkflowActions\General; -use App\Facades\Notifier; use App\Models\NotificationChannel; -use App\Models\User; use App\Notifications\GenericNotification; use App\WorkflowActions\AbstractWorkflowAction; use Illuminate\Support\Facades\Validator; @@ -35,11 +33,9 @@ public function run(array $input): array $notificationChannel = NotificationChannel::query()->findOrFail($input['notification_channel_id']); - $user = User::query()->where('email', $input['email'])->firstOrFail(); + $this->authorize('update', $notificationChannel); - $this->authorize('view', $notificationChannel); - - Notifier::send($user, new GenericNotification($input['message'])); + $notificationChannel->provider()->send($notificationChannel, new GenericNotification($input['message'])); return []; } diff --git a/app/WorkflowActions/Site/CreateSite.php b/app/WorkflowActions/Site/CreateSite.php index b6f3ae969..1009ef3c3 100644 --- a/app/WorkflowActions/Site/CreateSite.php +++ b/app/WorkflowActions/Site/CreateSite.php @@ -47,6 +47,7 @@ public function run(array $input): array $site = app(\App\Actions\Site\CreateSite::class)->create( $server, $input, + $this->user, ); return [ diff --git a/public/api-docs/openapi/schemas/Site.yaml b/public/api-docs/openapi/schemas/Site.yaml index fa1da9bd2..8eb6c379d 100644 --- a/public/api-docs/openapi/schemas/Site.yaml +++ b/public/api-docs/openapi/schemas/Site.yaml @@ -20,6 +20,7 @@ properties: type_data: type: object nullable: true + description: 'Site-specific metadata. Installation passwords and basic-auth password hashes are never returned.' basic_auth: type: object description: 'Basic authentication configuration. Password hashes are never returned.' diff --git a/tests/Feature/API/FirewallTest.php b/tests/Feature/API/FirewallTest.php index 94af2dce5..f70f71de8 100644 --- a/tests/Feature/API/FirewallTest.php +++ b/tests/Feature/API/FirewallTest.php @@ -1,9 +1,13 @@ create() : $this->server->project; + if ($differentProject) { + $project->users()->create([ + 'user_id' => $this->user->id, + 'role' => UserRole::ADMIN, + ]); + } + + $server = Server::factory()->create(['project_id' => $project->id]); + $rule = FirewallRule::factory()->create([ + 'server_id' => $server->id, + 'name' => 'Original rule', + 'port' => '1234', + ]); + $token = $this->user->createToken('scoped', [ + 'read', 'write', 'project:'.$this->server->project_id, + ]); + + $this->withToken($token->plainTextToken) + ->putJson(route('api.projects.servers.firewall-rules.edit', [ + 'project' => $this->server->project_id, + 'server' => $this->server, + 'firewallRule' => $rule, + ]), [ + 'name' => 'Unauthorized', + 'type' => 'allow', + 'protocol' => 'tcp', + 'port' => '55', + ]) + ->assertNotFound(); + + $this->assertDatabaseHas('firewall_rules', [ + 'id' => $rule->id, + 'name' => 'Original rule', + 'port' => '1234', + ]); + Queue::assertNothingPushed(); +})->with([ + 'same project' => false, + 'outside token scope' => true, +]); + test('see firewall rules', function () { Sanctum::actingAs($this->user, ['read', 'write']); diff --git a/tests/Feature/API/SiteCronjobTest.php b/tests/Feature/API/SiteCronjobTest.php index 143e08d8b..c06875dc1 100644 --- a/tests/Feature/API/SiteCronjobTest.php +++ b/tests/Feature/API/SiteCronjobTest.php @@ -207,7 +207,7 @@ 'site' => $site2, 'cronJob' => $cronjob, ])) - ->assertStatus(404); + ->assertForbidden(); }); test('cannot access cronjob from different server', function () { diff --git a/tests/Feature/API/SitesTest.php b/tests/Feature/API/SitesTest.php index 33950e127..074864fbd 100644 --- a/tests/Feature/API/SitesTest.php +++ b/tests/Feature/API/SitesTest.php @@ -2,15 +2,21 @@ use App\Enums\DeploymentStatus; use App\Enums\LoadBalancerMethod; +use App\Enums\UserRole; use App\Facades\SSH; use App\Models\Database; use App\Models\DatabaseUser; +use App\Models\Project; use App\Models\Server; use App\Models\Site; use App\Models\SourceControl; +use App\Models\User; +use App\SiteTypes\Blank; +use App\SiteTypes\Wordpress; use App\SourceControlProviders\Github; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Queue; use Laravel\Sanctum\Sanctum; use Tests\Traits\PrepareLoadBalancer; @@ -22,6 +28,118 @@ $this->prepare(); }); +test('api blank sites discard disabled source control input', function (?bool $enabled) { + SSH::fake(); + Http::fake(); + Queue::fake(); + + $sourceControl = SourceControl::factory()->create(['user_id' => User::factory()->create()->id]); + $token = $this->user->createToken('site-create', ['write', 'project:'.$this->server->project_id]); + $input = [ + 'type' => Blank::id(), + 'domain' => 'blank-disabled.example.com', + 'port' => 3000, + 'user' => 'blankdisabled', + 'source_control' => $sourceControl->id, + 'repository' => 'private/repository', + 'branch' => 'private', + ]; + if ($enabled !== null) { + $input['use_source_control'] = $enabled; + } + + $this->withToken($token->plainTextToken)->postJson(route('api.projects.servers.sites.create', [ + 'project' => $this->server->project_id, + 'server' => $this->server, + ]), $input)->assertSuccessful()->assertJsonPath('source_control_id', null); + + $this->assertDatabaseHas('sites', [ + 'domain' => $input['domain'], + 'source_control_id' => null, + 'repository' => '', + 'branch' => '', + ]); + Queue::assertPushed(\App\Jobs\Site\CreateJob::class, 1); + Http::assertNothingSent(); +})->with([false, null]); + +test('api site source selection uses the token actor rather than the server creator', function (string $scope) { + SSH::fake(); + Http::fake(); + Queue::fake(); + + $creator = User::factory()->create(); + $this->server->update(['user_id' => $creator->id]); + $this->server->project->users()->where('user_id', $this->user->id)->update(['role' => UserRole::ADMIN]); + $sourceControl = SourceControl::factory()->create([ + 'user_id' => $scope === 'creator' ? $creator->id : $this->user->id, + 'project_id' => match ($scope) { + 'project' => $this->server->project_id, + 'foreign-project' => Project::factory()->create()->id, + default => null, + }, + ]); + $token = $this->user->createToken('site-create', ['write', 'project:'.$this->server->project_id]); + + $response = $this->withToken($token->plainTextToken)->postJson(route('api.projects.servers.sites.create', [ + 'project' => $this->server->project_id, + 'server' => $this->server, + ]), [ + 'type' => Blank::id(), + 'domain' => 'actor-source.example.com', + 'port' => 3000, + 'user' => 'actorsource', + 'use_source_control' => true, + 'source_control' => $sourceControl->id, + 'repository' => 'private/repository', + 'branch' => 'main', + ]); + + if (in_array($scope, ['creator', 'foreign-project'])) { + if ($scope === 'creator') { + $response->assertForbidden(); + } else { + $response->assertUnprocessable()->assertJsonValidationErrors('source_control'); + } + $this->assertDatabaseMissing('sites', ['domain' => 'actor-source.example.com']); + Http::assertNothingSent(); + Queue::assertNothingPushed(); + } else { + $response->assertSuccessful()->assertJsonPath('source_control_id', $sourceControl->id); + $this->assertDatabaseHas('sites', ['source_control_id' => $sourceControl->id]); + Http::assertSentCount(1); + } +})->with(['creator', 'global', 'project', 'foreign-project']); + +test('api site resources hide installation passwords with persisted read tokens', function (UserRole $role) { + $this->server->project->users()->where('user_id', $this->user->id)->update(['role' => $role]); + $typeData = [ + 'title' => 'Private WordPress', + 'password' => 'wordpress-install-secret', + 'database_password' => 'database-install-secret', + ]; + $this->site->update(['type' => Wordpress::id(), 'type_data' => $typeData]); + $token = $this->user->createToken('read-sites', ['read', 'project:'.$this->server->project_id]); + + $this->withToken($token->plainTextToken)->getJson(route('api.projects.servers.sites.show', [ + 'project' => $this->server->project_id, + 'server' => $this->server, + 'site' => $this->site, + ]))->assertSuccessful() + ->assertJsonPath('type_data.title', $typeData['title']) + ->assertJsonMissingPath('type_data.password') + ->assertJsonMissingPath('type_data.database_password'); + + $this->getJson(route('api.projects.servers.sites', [ + 'project' => $this->server->project_id, + 'server' => $this->server, + ]))->assertSuccessful() + ->assertJsonMissingPath('data.0.type_data.password') + ->assertJsonMissingPath('data.0.type_data.database_password'); + + expect($this->site->fresh()->type_data)->toBe($typeData); +})->with([UserRole::OWNER, UserRole::USER]); + test('create site', function (array $inputs) { SSH::fake(); diff --git a/tests/Feature/API/WorkersTest.php b/tests/Feature/API/WorkersTest.php index a27a84c8d..aa9abe3a4 100644 --- a/tests/Feature/API/WorkersTest.php +++ b/tests/Feature/API/WorkersTest.php @@ -1,8 +1,11 @@ $site, 'worker' => $worker, ])) - ->assertStatus(422) - ->assertJsonValidationErrors(['name']); + ->assertForbidden(); $this->assertDatabaseHas('workers', ['id' => $worker->id]); }); @@ -460,3 +462,137 @@ ])) ->assertNotFound(); }); + +test('user role with a write bearer token cannot create workers', function (bool $siteWorker) { + $ssh = SSH::fake(); + Queue::fake(); + + $this->server->project->users()->where('user_id', $this->user->id)->update([ + 'role' => UserRole::USER, + ]); + $token = $this->user->createToken('worker-token', ['read', 'write', 'project:'.$this->server->project_id]); + + $this->withToken($token->plainTextToken)->postJson(route('api.projects.servers.workers.create', [ + 'project' => $this->server->project, + 'server' => $this->server, + 'site' => $siteWorker ? $this->site : null, + ]), [ + 'name' => 'Forbidden worker', + 'command' => 'php artisan queue:work', + 'user' => 'vito', + 'auto_start' => true, + 'auto_restart' => true, + 'numprocs' => 1, + ])->assertForbidden(); + + $this->assertDatabaseCount('workers', 0); + expect($ssh->getExecutedCommands())->toBeEmpty(); + Queue::assertNothingPushed(); +})->with([false, true]); + +test('admin with a write bearer token can create and delete workers', function (bool $siteWorker) { + SSH::fake(); + + $this->server->project->users()->where('user_id', $this->user->id)->update([ + 'role' => UserRole::ADMIN, + ]); + $token = $this->user->createToken('worker-token', ['read', 'write', 'project:'.$this->server->project_id]); + $parameters = [ + 'project' => $this->server->project, + 'server' => $this->server, + 'site' => $siteWorker ? $this->site : null, + ]; + + $response = $this->withToken($token->plainTextToken)->postJson(route('api.projects.servers.workers.create', $parameters), [ + 'name' => 'Admin worker', + 'command' => 'php artisan queue:work', + 'user' => 'vito', + 'auto_start' => true, + 'auto_restart' => true, + 'numprocs' => 1, + ])->assertSuccessful(); + + $workerId = $response->json('id'); + $this->assertDatabaseHas('workers', ['id' => $workerId, 'status' => WorkerStatus::RUNNING]); + + $this->deleteJson(route('api.projects.servers.workers.delete', $parameters + ['worker' => $workerId])) + ->assertNoContent(); + + $this->assertDatabaseMissing('workers', ['id' => $workerId]); +})->with([false, true]); + +test('worker api routes enforce worker server readiness', function (string $method, string $route) { + $ssh = SSH::fake(); + Queue::fake(); + Sanctum::actingAs($this->user, ['read', 'write']); + + $worker = Worker::factory()->create([ + 'server_id' => $this->server->id, + 'site_id' => $this->site->id, + ]); + $attributes = $worker->refresh()->getAttributes(); + $this->server->update(['status' => ServerStatus::INSTALLING]); + + $this->json($method, route($route, [ + 'project' => $this->server->project, + 'server' => $this->server, + 'site' => $this->site, + 'worker' => $worker, + ]), [ + 'name' => 'Changed worker', + 'command' => 'pwd', + 'user' => 'vito', + 'auto_start' => true, + 'auto_restart' => true, + 'numprocs' => 1, + ])->assertForbidden(); + + $this->assertDatabaseCount('workers', 1); + $this->assertDatabaseHas('workers', ['id' => $worker->id]); + expect($worker->refresh()->getAttributes())->toBe($attributes); + expect($ssh->getExecutedCommands())->toBeEmpty(); + Queue::assertNothingPushed(); +})->with([ + ['GET', 'api.projects.servers.workers'], + ['GET', 'api.projects.servers.sites.workers'], + ['GET', 'api.projects.servers.workers.show'], + ['GET', 'api.projects.servers.sites.workers.show'], + ['POST', 'api.projects.servers.workers.resync'], + ['POST', 'api.projects.servers.workers.restart-all'], + ['POST', 'api.projects.servers.workers.create'], + ['PUT', 'api.projects.servers.workers.update'], + ['POST', 'api.projects.servers.workers.start'], + ['POST', 'api.projects.servers.workers.restart'], + ['GET', 'api.projects.servers.workers.logs'], + ['DELETE', 'api.projects.servers.workers.delete'], +]); + +test('worker api routes keep mismatched resource responses not found', function (string $mismatch) { + $ssh = SSH::fake(); + Queue::fake(); + Sanctum::actingAs($this->user, ['read', 'write']); + + $otherServer = Server::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->server->project_id, + ]); + $site = Site::factory()->create(['server_id' => $this->server->id]); + $worker = Worker::factory()->create([ + 'server_id' => $this->server->id, + 'site_id' => $site->id, + ]); + + if ($mismatch === 'project') { + $otherServer->update(['project_id' => Project::factory()->create()->id]); + } + + $this->getJson(route('api.projects.servers.sites.workers.show', [ + 'project' => $this->server->project, + 'server' => in_array($mismatch, ['project', 'server']) ? $otherServer : $this->server, + 'site' => $mismatch === 'site' ? $this->site : $site, + 'worker' => $worker, + ]))->assertNotFound(); + + expect($ssh->getExecutedCommands())->toBeEmpty(); + Queue::assertNothingPushed(); +})->with(['project', 'server', 'site']); diff --git a/tests/Feature/API/WorkflowRunTest.php b/tests/Feature/API/WorkflowRunTest.php index 1dd862f50..b4f271fa6 100644 --- a/tests/Feature/API/WorkflowRunTest.php +++ b/tests/Feature/API/WorkflowRunTest.php @@ -3,18 +3,27 @@ use App\Actions\Workflow\RunWorkflow; use App\Enums\UserRole; use App\Enums\WorkflowRunStatus; +use App\Events\SocketEvent; use App\Facades\SSH; +use App\Jobs\Workflow\RunJob; use App\Models\Project; use App\Models\User; use App\Models\Workflow; use App\Models\WorkflowRun; +use App\WorkflowActions\General\RunCommand; +use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Storage; use Laravel\Sanctum\Sanctum; uses(RefreshDatabase::class); beforeEach(function () { + Http::fake(); + Event::fake([SocketEvent::class]); $this->user = User::factory()->create(); $this->project = Project::factory()->create(); $this->project->users()->create([ @@ -172,7 +181,7 @@ $this->assertStringContainsString("Log file doesn't exist or is empty!", $response->getContent()); }); -test('cannot access workflow run from different workflow', function () { +test('cannot access workflow run from different workflow', function (string $suffix) { Sanctum::actingAs($this->user, ['read', 'write']); $otherWorkflow = Workflow::factory()->create([ @@ -187,12 +196,12 @@ 'verbose' => true, ]); - $response = $this->getJson("/api/projects/{$this->project->id}/workflows/{$this->workflow->id}/runs/{$otherWorkflowRun->id}"); + $response = $this->getJson("/api/projects/{$this->project->id}/workflows/{$this->workflow->id}/runs/{$otherWorkflowRun->id}{$suffix}"); $response->assertNotFound(); -}); +})->with(['', '/log']); -test('cannot access workflow run from different project', function () { +test('cannot access workflow run from different project', function (string $suffix) { Sanctum::actingAs($this->user, ['read', 'write']); $otherProject = Project::factory()->create(); @@ -211,10 +220,10 @@ 'verbose' => true, ]); - $response = $this->getJson("/api/projects/{$this->project->id}/workflows/{$this->workflow->id}/runs/{$otherWorkflowRun->id}"); + $response = $this->getJson("/api/projects/{$this->project->id}/workflows/{$this->workflow->id}/runs/{$otherWorkflowRun->id}{$suffix}"); $response->assertNotFound(); -}); +})->with(['', '/log']); test('cannot access workflow run from different user', function () { Sanctum::actingAs($this->user, ['read', 'write']); @@ -335,3 +344,96 @@ // Should have pagination links since we have more than 25 workflow runs expect($response->json('links.next'))->not->toBeNull(); }); + +test('serialized workflow jobs enforce the initiating bearer token on resolved resources', function (string $scope, bool $allowed, bool $invalidToken) { + SSH::fake(); + Storage::fake('server-logs'); + Queue::fake([RunJob::class]); + $targetProject = $this->server->project; + if ($scope !== 'non-member') { + $targetProject->users()->create(['user_id' => $this->user->id, 'role' => UserRole::OWNER]); + } + $abilities = ['read', 'write', 'project:'.$this->project->id]; + if ($scope !== 'excluded') { + $abilities[] = 'project:'.$targetProject->id; + } + if (in_array($scope, ['unrestricted', 'non-member'], true)) { + $abilities = ['read', 'write']; + } + $token = $this->user->createToken('workflow', $abilities); + $this->workflow->update(['payload' => [ + 'nodes' => [[ + 'id' => 'command', + 'data' => ['action' => [ + 'label' => 'Run command', + 'handler' => RunCommand::class, + 'starting' => true, + 'inputs' => [], + 'outputs' => [], + ]], + ]], + 'edges' => [], + ]]); + + $response = $this->withToken($token->plainTextToken) + ->postJson("/api/projects/{$this->project->id}/workflows/{$this->workflow->id}/runs", [ + 'inputs' => [ + 'target_server_id' => $this->server->id, + 'server_id' => '{{target_server_id}}', + 'command' => 'echo workflow-token-scope', + 'user' => null, + ], + ])->assertCreated(); + + Queue::assertPushed(RunJob::class); + $serialized = serialize(Queue::pushed(RunJob::class)->first()); + expect($serialized)->not->toContain($token->plainTextToken) + ->not->toContain($token->accessToken->token); + + match ($scope) { + 'revoked' => $token->accessToken->delete(), + 'expired' => $token->accessToken->forceFill(['expires_at' => now()->subMinute()])->save(), + 'lifetime-expired' => $token->accessToken->forceFill(['created_at' => now()->subHours(2)])->save(), + 'mismatched' => $token->accessToken->forceFill(['tokenable_id' => $this->server->user_id])->save(), + 'narrowed' => $token->accessToken->forceFill(['abilities' => ['write', 'project:'.$this->project->id]])->save(), + 'workflow-excluded' => $token->accessToken->forceFill(['abilities' => ['write', 'project:'.$targetProject->id]])->save(), + 'read-only' => $token->accessToken->forceFill(['abilities' => ['read']])->save(), + default => null, + }; + if ($scope === 'lifetime-expired') { + config()->set('sanctum.expiration', 60); + } + $job = unserialize($serialized); + $job->withFakeQueueInteractions()->handle(); + + if ($allowed) { + SSH::assertExecutedContains('echo workflow-token-scope'); + } else { + SSH::assertNotExecutedContains('echo workflow-token-scope'); + } + if ($invalidToken) { + $job->assertFailedWith(AuthorizationException::class); + } else { + $job->assertNotFailed(); + $this->assertDatabaseHas('workflow_runs', [ + 'id' => $response->json('id'), + 'status' => WorkflowRunStatus::COMPLETED->value, + ]); + } + expect((new ReflectionProperty($job, 'user'))->getValue($job)->currentAccessToken())->toBeNull(); + $run = WorkflowRun::query()->findOrFail($response->json('id')); + expect($run->getLogContent())->not->toContain($token->plainTextToken) + ->not->toContain($token->accessToken->token); +})->with([ + 'excluded project' => ['excluded', false, false], + 'both projects scoped' => ['allowed', true, false], + 'unrestricted token' => ['unrestricted', true, false], + 'non-member' => ['non-member', false, false], + 'revoked token' => ['revoked', false, true], + 'expired token' => ['expired', false, true], + 'expired token lifetime' => ['lifetime-expired', false, true], + 'changed token owner' => ['mismatched', false, true], + 'narrowed token scope' => ['narrowed', false, false], + 'workflow removed from scope' => ['workflow-excluded', false, true], + 'removed write ability' => ['read-only', false, true], +]); diff --git a/tests/Feature/ApplicationTest.php b/tests/Feature/ApplicationTest.php index 172eec3c9..205c7981c 100644 --- a/tests/Feature/ApplicationTest.php +++ b/tests/Feature/ApplicationTest.php @@ -101,6 +101,26 @@ expect($deploymentScript->shouldRestartWorkers())->toBeTrue(); }); +test('cannot update a deployment script belonging to another site', function () { + $foreignSite = Site::factory()->create(['server_id' => $this->server->id]); + $script = $foreignSite->deploymentScript; + $originalContent = $script->content; + + $this->actingAs($this->user) + ->put(route('application.update-deployment-script', [ + 'server' => $this->server, + 'site' => $this->site, + 'deploymentScript' => $script, + ]), ['script' => 'unauthorized replacement']) + ->assertNotFound(); + + $this->assertDatabaseHas('deployment_scripts', [ + 'id' => $script->id, + 'site_id' => $foreignSite->id, + 'content' => $originalContent, + ]); +}); + test('deploy classic', function () { SSH::fake('fake output'); Http::fake([ diff --git a/tests/Feature/BackupTest.php b/tests/Feature/BackupTest.php index fbeccf9ce..f7f119d06 100644 --- a/tests/Feature/BackupTest.php +++ b/tests/Feature/BackupTest.php @@ -18,6 +18,7 @@ use App\Models\Project; use App\Models\Server; use App\Models\StorageProvider; +use App\Models\User; use App\StorageProviders\Local; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Bus; @@ -46,6 +47,56 @@ ]); }); +test('backup creation rejects inaccessible storage before dispatching work', function (string $scope) { + SSH::fake(); + Http::fake(); + Bus::fake(); + + $storage = StorageProvider::factory()->create([ + 'user_id' => $scope === 'foreign-owner' ? User::factory()->create()->id : $this->user->id, + 'project_id' => $scope === 'foreign-project' ? Project::factory()->create()->id : null, + ]); + + $response = $this->actingAs($this->user)->post(route('backups.store', ['server' => $this->server]), [ + 'type' => BackupType::FILE->value, + 'path' => '/home/vito/private', + 'storage' => $storage->id, + 'interval' => '0 * * * *', + 'keep' => 10, + ]); + + if ($scope === 'foreign-owner') { + $response->assertForbidden(); + } else { + $response->assertSessionHasErrors('storage'); + } + + $this->assertDatabaseMissing('backups', ['storage_id' => $storage->id]); + Bus::assertNothingDispatched(); + Http::assertNothingSent(); +})->with(['foreign-owner', 'foreign-project']); + +test('backup creation accepts owned storage in the server project or globally', function (bool $global) { + SSH::fake(); + Http::fake(); + Bus::fake(); + + $storage = StorageProvider::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $global ? null : $this->server->project_id, + ]); + + $this->actingAs($this->user)->post(route('backups.store', ['server' => $this->server]), [ + 'type' => BackupType::FILE->value, + 'path' => '/home/vito/private', + 'storage' => $storage->id, + 'interval' => '0 * * * *', + 'keep' => 10, + ])->assertSessionDoesntHaveErrors()->assertRedirect(); + + $this->assertDatabaseHas('backups', ['storage_id' => $storage->id, 'server_id' => $this->server->id]); +})->with([true, false]); + test('backup model can have path field', function () { $server = Server::factory()->create(); $storage = StorageProvider::factory()->create(); diff --git a/tests/Feature/CommandsTest.php b/tests/Feature/CommandsTest.php index 445bb4ef6..c495f6e1d 100644 --- a/tests/Feature/CommandsTest.php +++ b/tests/Feature/CommandsTest.php @@ -1,8 +1,15 @@ user->update(['is_admin' => false]); + $this->server->project->users()->where('user_id', $this->user->id)->update([ + 'role' => $role, + ]); + $this->actingAs($this->user); /** @var Command $command */ @@ -101,13 +113,126 @@ ]), [ 'MESSAGE' => 'Hello, world!', ]) + ->assertRedirect(route('commands.show', [ + 'server' => $this->server, + 'site' => $this->site, + 'command' => $command, + ])) ->assertSessionDoesntHaveErrors(); $this->assertDatabaseHas('command_executions', [ 'command_id' => $command->id, 'variables' => $this->castAsJson(['MESSAGE' => 'Hello, world!']), ]); -}); +})->with([UserRole::OWNER, UserRole::ADMIN]); + +test('cannot execute a command belonging to another site', function (bool $differentProject): void { + SSH::fake(); + Queue::fake(); + + $server = $differentProject ? Server::factory()->create([ + 'project_id' => Project::factory()->create()->id, + 'user_id' => $this->user->id, + ]) : $this->server; + $site = Site::factory()->create(['server_id' => $server->id]); + $command = $site->commands()->create([ + 'name' => 'Other Site Command', + 'command' => 'echo "Hello, world!"', + ]); + + $this->actingAs($this->user); + + $this->post(route('commands.execute', [ + 'server' => $this->server, + 'site' => $this->site, + 'command' => $command, + ])) + ->assertForbidden(); + + $this->assertDatabaseMissing('command_executions', ['command_id' => $command->id]); + Queue::assertNotPushed(ExecuteCommandJob::class); +})->with([ + 'same server' => false, + 'another project' => true, +]); + +test('cannot execute commands without project write access', function (?UserRole $role): void { + SSH::fake(); + Queue::fake(); + + $user = User::factory()->create(['is_admin' => false]); + $user->ensureHasDefaultProject(); + + if ($role !== null) { + $this->server->project->users()->create([ + 'user_id' => $user->id, + 'role' => $role, + ]); + } + + $command = $this->site->commands()->create([ + 'name' => 'Test Command', + 'command' => 'echo "Hello, world!"', + ]); + + $this->actingAs($user); + + $this->post(route('commands.execute', [ + 'server' => $this->server, + 'site' => $this->site, + 'command' => $command, + ])) + ->assertForbidden(); + + $this->assertDatabaseMissing('command_executions', ['command_id' => $command->id]); + Queue::assertNotPushed(ExecuteCommandJob::class); +})->with([ + 'non-member' => null, + 'read-only member' => UserRole::USER, +]); + +test('command routes reject a site belonging to another server', function (string $method, string $route): void { + SSH::fake(); + Queue::fake(); + + $server = Server::factory()->create([ + 'project_id' => Project::factory()->create()->id, + 'user_id' => $this->user->id, + ]); + $site = Site::factory()->create(['server_id' => $server->id]); + $command = $site->commands()->create([ + 'name' => 'Other Site Command', + 'command' => 'echo "Hello, world!"', + ]); + + $this->actingAs($this->user); + + $this->{$method}(route($route, [ + 'server' => $this->server, + 'site' => $site, + 'command' => $command, + ]), [ + 'name' => 'Unauthorized Command', + 'command' => 'whoami', + ]) + ->assertForbidden(); + + $this->assertDatabaseMissing('commands', ['name' => 'Unauthorized Command']); + $this->assertDatabaseHas('commands', [ + 'id' => $command->id, + 'name' => 'Other Site Command', + 'command' => 'echo "Hello, world!"', + ]); + $this->assertDatabaseMissing('command_executions', ['command_id' => $command->id]); + Queue::assertNotPushed(ExecuteCommandJob::class); +})->with([ + 'list' => ['get', 'commands'], + 'create' => ['post', 'commands.store'], + 'show' => ['get', 'commands.show'], + 'update' => ['put', 'commands.update'], + 'delete' => ['delete', 'commands.destroy'], + 'execute' => ['post', 'commands.execute'], +]); test('execute command validation error', function () { $this->actingAs($this->user); diff --git a/tests/Feature/DatabaseTest.php b/tests/Feature/DatabaseTest.php index 2ad9328d2..aa887b2e7 100644 --- a/tests/Feature/DatabaseTest.php +++ b/tests/Feature/DatabaseTest.php @@ -6,9 +6,11 @@ use App\Facades\SSH; use App\Models\Database; use App\Models\DatabaseUser; +use App\Models\Server; use App\Services\Database\Mysql; use App\Services\Database\Postgresql; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Queue; use Inertia\Testing\AssertableInertia; uses(RefreshDatabase::class); @@ -266,3 +268,30 @@ function vitoPestFeatureDatabaseTestUsePostgresql(): void ]); test()->server->refresh(); } + +test('database deletion rejects a database from another server', function () { + $ssh = SSH::fake(); + Queue::fake(); + + $otherServer = Server::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->server->project_id, + ]); + $service = $this->server->database()->replicate(); + $service->server_id = $otherServer->id; + $service->save(); + $database = Database::factory()->create(['server_id' => $this->server->id]); + + $this->actingAs($this->user)->delete(route('databases.destroy', [ + 'server' => $otherServer, + 'database' => $database, + ]))->assertForbidden(); + + $this->assertDatabaseHas('databases', [ + 'id' => $database->id, + 'server_id' => $this->server->id, + 'deleted_at' => null, + ]); + expect($ssh->getExecutedCommands())->toBeEmpty(); + Queue::assertNothingPushed(); +}); diff --git a/tests/Feature/DatabaseUserTest.php b/tests/Feature/DatabaseUserTest.php index 5802c85d9..b59d1dc76 100644 --- a/tests/Feature/DatabaseUserTest.php +++ b/tests/Feature/DatabaseUserTest.php @@ -6,9 +6,11 @@ use App\Facades\SSH; use App\Models\Database; use App\Models\DatabaseUser; +use App\Models\Server; use App\Services\Database\Mysql; use App\Services\Database\Postgresql; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Queue; use Inertia\Testing\AssertableInertia; uses(RefreshDatabase::class); @@ -749,3 +751,40 @@ function vitoPestFeatureDatabaseUserTestUsePostgresql(): void ]); test()->server->refresh(); } + +test('database user routes reject a user from another server', function (string $method, string $route) { + $ssh = SSH::fake(); + Queue::fake(); + + $otherServer = Server::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->server->project_id, + ]); + $service = $this->server->database()->replicate(); + $service->server_id = $otherServer->id; + $service->save(); + $databaseUser = DatabaseUser::factory()->create([ + 'server_id' => $this->server->id, + 'password' => 'old_password', + 'databases' => ['existing_database'], + ]); + $attributes = $databaseUser->refresh()->getAttributes(); + + $this->actingAs($this->user)->json($method, route($route, [ + 'server' => $otherServer, + 'databaseUser' => $databaseUser, + ]), [ + 'password' => 'new_password', + 'permission' => $databaseUser->permission->value, + 'databases' => [], + ])->assertForbidden(); + + $this->assertDatabaseHas('database_users', ['id' => $databaseUser->id]); + expect($databaseUser->refresh()->getAttributes())->toBe($attributes); + expect($ssh->getExecutedCommands())->toBeEmpty(); + Queue::assertNothingPushed(); +})->with([ + ['PUT', 'database-users.update'], + ['PUT', 'database-users.link'], + ['DELETE', 'database-users.destroy'], +]); diff --git a/tests/Feature/Jobs/WorkflowRunJobTest.php b/tests/Feature/Jobs/WorkflowRunJobTest.php index 741cff248..dd8493399 100644 --- a/tests/Feature/Jobs/WorkflowRunJobTest.php +++ b/tests/Feature/Jobs/WorkflowRunJobTest.php @@ -1,18 +1,32 @@ assertStringNotContainsString('after-run', $run->refresh()->getLogContent()); }); + +test('workflow workers preserve session project access and restore execution context', function (bool $member, bool $legacy) { + SSH::fake(); + Storage::fake('server-logs'); + Queue::fake(); + $project = Project::factory()->create(); + if ($member) { + $project->users()->create(['user_id' => $this->user->id, 'role' => UserRole::OWNER]); + } + $workflow = Workflow::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $project->id, + ]); + $run = WorkflowRun::factory()->create([ + 'workflow_id' => $workflow->id, + 'user_id' => $this->user->id, + 'status' => WorkflowRunStatus::RUNNING, + ]); + $tree = new WorkflowActionDTO('Command', RunCommand::class, [], [ + 'server_id' => $this->server->id, + 'command' => 'echo session-workflow', + 'user' => null, + ], 'command'); + $job = new RunJob($run, $this->user, $workflow, $tree, []); + if ($legacy) { + (function (): void { + unset($this->accessTokenId); + })->call($job); + } + $job = unserialize(serialize($job)); + config()->set('queue.connections.ssh.driver', 'database'); + config()->set('queue.connections.default.driver', 'database'); + + $job->withFakeQueueInteractions()->handle(); + + if ($member && ! $legacy) { + SSH::assertExecutedContains('echo session-workflow'); + $job->assertNotFailed(); + } else { + SSH::assertNotExecutedContains('echo session-workflow'); + $job->assertFailedWith(AuthorizationException::class); + } + expect(config('queue.connections.ssh.driver'))->toBe('database') + ->and(config('queue.connections.default.driver'))->toBe('database') + ->and($this->user->currentAccessToken())->toBeNull() + ->and((new ReflectionProperty($job, 'user'))->getValue($job)->currentAccessToken())->toBeNull(); +})->with([ + 'authorized cross-project session' => [true, false], + 'session without workflow membership' => [false, false], + 'legacy job without authentication context' => [true, true], +]); + +test('workflow jobs recheck current token scope between actions and failure branches', function (string $change) { + SSH::fake(); + Storage::fake('server-logs'); + Queue::fake(); + $project = Project::factory()->create(); + $project->users()->create(['user_id' => $this->user->id, 'role' => UserRole::OWNER]); + $workflow = Workflow::factory()->create(['user_id' => $this->user->id, 'project_id' => $project->id]); + $token = $this->user->createToken('workflow', ['write', 'project:'.$project->id, 'project:'.$this->server->project_id]); + $this->user->withAccessToken($token->accessToken); + $run = WorkflowRun::factory()->create([ + 'workflow_id' => $workflow->id, + 'user_id' => $this->user->id, + 'status' => WorkflowRunStatus::RUNNING, + ]); + $command = new WorkflowActionDTO('Command', RunCommand::class, [], [ + 'server_id' => '{{response_body_raw}}', + 'command' => 'echo chained-workflow', + 'user' => null, + ], 'command'); + $failure = new WorkflowActionDTO('Failure command', RunCommand::class, [], [ + 'server_id' => $this->server->id, + 'command' => 'echo failure-workflow', + 'user' => null, + ], 'failure'); + $tree = new WorkflowActionDTO('HTTP request', HttpCall::class, [], [ + 'url' => 'https://example.com/workflow', + 'method' => 'GET', + ], 'http', success: $command, failure: $failure); + Http::fake(function () use ($token, $project, $change) { + match ($change) { + 'narrowed' => $token->accessToken->forceFill(['abilities' => ['write', 'project:'.$project->id]])->save(), + 'revoked' => $token->accessToken->delete(), + default => null, + }; + + return Http::response((string) $this->server->id); + }); + $job = unserialize(serialize(new RunJob($run, $this->user, $workflow, $tree, []))); + + $job->withFakeQueueInteractions()->handle(); + + Http::assertSentCount(1); + if ($change === 'unchanged') { + SSH::assertExecutedContains('echo chained-workflow'); + } else { + SSH::assertNotExecutedContains('echo chained-workflow'); + } + SSH::assertNotExecutedContains('echo failure-workflow'); + if ($change === 'revoked') { + $job->assertFailedWith(AuthorizationException::class); + } + expect((new ReflectionProperty($job, 'user'))->getValue($job)->currentAccessToken())->toBeNull(); +})->with(['unchanged', 'narrowed', 'revoked']); + +test('nested workflows inherit token scope without changing the outer execution context', function (bool $revoked) { + SSH::fake(); + Storage::fake('server-logs'); + Queue::fake([RunJob::class]); + $project = Project::factory()->create(); + $project->users()->create(['user_id' => $this->user->id, 'role' => UserRole::OWNER]); + $workflow = Workflow::factory()->create(['user_id' => $this->user->id, 'project_id' => $project->id]); + $nested = Workflow::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $project->id, + 'payload' => [ + 'nodes' => [[ + 'id' => 'command', + 'data' => ['action' => [ + 'label' => 'Nested command', + 'handler' => RunCommand::class, + 'starting' => true, + 'inputs' => [ + 'server_id' => $this->server->id, + 'command' => 'echo nested-workflow', + 'user' => null, + ], + ]], + ]], + 'edges' => [], + ], + ]); + $token = $this->user->createToken('workflow', ['write', 'project:'.$project->id]); + $this->user->withAccessToken($token->accessToken); + $run = WorkflowRun::factory()->create([ + 'workflow_id' => $workflow->id, + 'user_id' => $this->user->id, + 'status' => WorkflowRunStatus::RUNNING, + ]); + $tree = new WorkflowActionDTO('HTTP request', HttpCall::class, [], [ + 'url' => 'https://example.com/workflow', + 'method' => 'GET', + ], 'http'); + $job = unserialize(serialize(new RunJob($run, $this->user, $workflow, $tree, []))); + $actor = (new ReflectionProperty($job, 'user'))->getValue($job); + Http::fake(function () use ($actor, $nested, $token, $revoked) { + if ($revoked) { + $actor->currentAccessToken()->delete(); + } + app(RunWorkflow::class)->run($actor, $nested, ['inputs' => []]); + $nestedJob = unserialize(serialize(Queue::pushed(RunJob::class)->first())); + $nestedJob->withFakeQueueInteractions()->handle(); + + if ($revoked) { + $nestedJob->assertFailedWith(AuthorizationException::class); + } + expect($actor->currentAccessToken()->id)->toBe($token->accessToken->id); + + return Http::response('ok'); + }); + + $job->withFakeQueueInteractions()->handle(); + + Http::assertSentCount(1); + Queue::assertPushed(RunJob::class, 1); + SSH::assertNotExecutedContains('echo nested-workflow'); + $job->assertNotFailed(); + expect($actor->currentAccessToken())->toBeNull(); +})->with([false, true]); diff --git a/tests/Feature/PHPTest.php b/tests/Feature/PHPTest.php index b0ad37b23..ac5c44f91 100644 --- a/tests/Feature/PHPTest.php +++ b/tests/Feature/PHPTest.php @@ -4,9 +4,11 @@ use App\Enums\PHPIniType; use App\Enums\ServiceStatus; use App\Facades\SSH; +use App\Models\Server; use App\Models\Service; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Queue; uses(RefreshDatabase::class); @@ -178,3 +180,41 @@ ['8.2', PHPIniType::CLI], ]; }); + +test('php routes reject a service from another server', function (string $method, string $route) { + $ssh = SSH::fake(); + Queue::fake(); + + $otherServer = Server::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->server->project_id, + ]); + $service = $this->server->php('8.2'); + $attributes = $service->refresh()->getAttributes(); + $targetService = $service->replicate(); + $targetService->server_id = $otherServer->id; + $targetService->is_default = false; + $targetService->save(); + $targetAttributes = $targetService->refresh()->getAttributes(); + + $this->actingAs($this->user)->json($method, route($route, [ + 'server' => $otherServer, + 'service' => $service, + ]), [ + 'version' => '8.2', + 'type' => PHPIniType::CLI->value, + 'ini' => '[PHP]', + 'extension' => 'gmp', + ])->assertForbidden(); + + expect($service->refresh()->getAttributes())->toBe($attributes); + expect($targetService->refresh()->getAttributes())->toBe($targetAttributes); + expect($ssh->getExecutedCommands())->toBeEmpty(); + expect($ssh->getUploadedContent())->toBeEmpty(); + Queue::assertNothingPushed(); +})->with([ + ['GET', 'php.ini'], + ['PATCH', 'php.ini.update'], + ['POST', 'php.install-extension'], + ['POST', 'php.default-cli'], +]); diff --git a/tests/Feature/RedirectsTest.php b/tests/Feature/RedirectsTest.php index 1c9584a50..67acb3463 100644 --- a/tests/Feature/RedirectsTest.php +++ b/tests/Feature/RedirectsTest.php @@ -3,6 +3,8 @@ use App\Enums\RedirectStatus; use App\Facades\SSH; use App\Models\Redirect; +use App\Models\Server; +use App\Models\Site; use Illuminate\Foundation\Testing\RefreshDatabase; use Inertia\Testing\AssertableInertia; @@ -204,3 +206,22 @@ 'websocket' => false, ]); }); + +test('redirect listing rejects a site from another server', function () { + $otherServer = Server::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->server->project_id, + ]); + $service = $this->server->webserver()->replicate(); + $service->server_id = $otherServer->id; + $service->save(); + $site = Site::factory()->create(['server_id' => $otherServer->id]); + $redirect = Redirect::factory()->create(['site_id' => $site->id]); + + $this->actingAs($this->user)->get(route('redirects', [ + 'server' => $this->server, + 'site' => $site, + ]))->assertForbidden(); + + $this->assertDatabaseHas('redirects', ['id' => $redirect->id, 'site_id' => $site->id]); +}); diff --git a/tests/Feature/ServerTest.php b/tests/Feature/ServerTest.php index 72a36bfc0..6d485882b 100644 --- a/tests/Feature/ServerTest.php +++ b/tests/Feature/ServerTest.php @@ -1,6 +1,7 @@ assertForbidden(); }); -test('transfer server', function () { +test('transfer server', function (UserRole $role): void { $this->actingAs($this->user); $oldProject = $this->server->project; @@ -637,12 +640,13 @@ function (object $notifiable, object $notification): bool { ]); $newProject->users()->create([ 'user_id' => $this->user->id, - 'role' => UserRole::OWNER, + 'role' => $role, ]); $this->post(route('servers.transfer', $this->server), [ 'project_id' => $newProject->id, ]) + ->assertRedirect(route('server-settings', $this->server)) ->assertSessionDoesntHaveErrors(); $this->assertDatabaseHas('servers', [ @@ -651,8 +655,64 @@ function (object $notifiable, object $notification): bool { ]); expect($this->user->refresh()->current_project_id)->toEqual($newProject->id); +})->with([UserRole::OWNER, UserRole::ADMIN]); + +test('cannot transfer a server to a read-only project', function (): void { + $project = Project::factory()->create(); + $project->users()->create([ + 'user_id' => $this->user->id, + 'role' => UserRole::USER, + ]); + + $this->actingAs($this->user) + ->post(route('servers.transfer', $this->server), [ + 'project_id' => $project->id, + ]) + ->assertForbidden(); + + $this->assertDatabaseHas('servers', [ + 'id' => $this->server->id, + 'project_id' => $this->server->project_id, + ]); + expect($this->user->refresh()->current_project_id)->toBe($this->server->project_id); }); +test('cannot transfer a server while network memberships remain', function (NetworkServerStatus $status): void { + SSH::fake(); + Queue::fake(); + + $network = Network::factory()->create(['project_id' => $this->server->project_id]); + $member = NetworkServer::factory()->create([ + 'network_id' => $network->id, + 'server_id' => $this->server->id, + 'status' => $status, + ]); + $project = Project::factory()->create(); + $project->users()->create([ + 'user_id' => $this->user->id, + 'role' => UserRole::OWNER, + ]); + + $this->actingAs($this->user) + ->postJson(route('servers.transfer', $this->server), [ + 'project_id' => $project->id, + ]) + ->assertUnprocessable() + ->assertJsonValidationErrors('project_id'); + + $this->assertDatabaseHas('servers', [ + 'id' => $this->server->id, + 'project_id' => $network->project_id, + ]); + $this->assertDatabaseHas('network_servers', [ + 'id' => $member->id, + 'network_id' => $network->id, + 'server_id' => $this->server->id, + 'status' => $status->value, + ]); + Queue::assertNothingPushed(); +})->with(NetworkServerStatus::cases()); + test('user role can view server', function () { $this->server->project->users()->where('user_id', $this->user->id)->update([ 'role' => UserRole::USER, diff --git a/tests/Feature/SiteCronjobTest.php b/tests/Feature/SiteCronjobTest.php index e2533399d..79f48dce0 100644 --- a/tests/Feature/SiteCronjobTest.php +++ b/tests/Feature/SiteCronjobTest.php @@ -3,7 +3,10 @@ use App\Enums\CronjobStatus; use App\Facades\SSH; use App\Models\CronJob; +use App\Models\Server; +use App\Models\Site; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Queue; use Inertia\Testing\AssertableInertia; uses(RefreshDatabase::class); @@ -249,3 +252,85 @@ SSH::assertExecutedContains("echo '0 * * * * bash -lc '\\''php artisan schedule:run'\\''' | sudo -u vito crontab -"); SSH::assertExecutedContains('sudo -u vito crontab -l'); }); + +test('site cronjob routes reject a site from another server', function (string $method, string $route) { + $ssh = SSH::fake(); + Queue::fake(); + + $otherServer = Server::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->server->project_id, + ]); + $service = $this->server->webserver()->replicate(); + $service->server_id = $otherServer->id; + $service->save(); + $site = Site::factory()->create(['server_id' => $otherServer->id]); + + $this->actingAs($this->user)->json($method, route($route, [ + 'server' => $this->server, + 'site' => $site, + ]), [ + 'command' => 'ls -la', + 'user' => 'vito', + 'frequency' => '* * * * *', + ])->assertForbidden(); + + $this->assertDatabaseCount('cron_jobs', 0); + expect($ssh->getExecutedCommands())->toBeEmpty(); + Queue::assertNothingPushed(); +})->with([ + ['GET', 'cronjobs.site'], + ['POST', 'cronjobs.site.store'], +]); + +test('site cronjob routes reject a cronjob from another site', function (string $method, string $route) { + $ssh = SSH::fake(); + Queue::fake(); + + $site = Site::factory()->create(['server_id' => $this->server->id]); + $cronjob = CronJob::factory()->create([ + 'server_id' => $this->server->id, + 'site_id' => $this->site->id, + 'user' => 'vito', + ]); + $attributes = $cronjob->refresh()->getAttributes(); + + $this->actingAs($this->user)->json($method, route($route, [ + 'server' => $this->server, + 'site' => $site, + 'cronJob' => $cronjob, + ]), [ + 'command' => 'pwd', + 'user' => 'vito', + 'frequency' => '* * * * *', + ])->assertForbidden(); + + $this->assertDatabaseHas('cron_jobs', ['id' => $cronjob->id]); + expect($cronjob->refresh()->getAttributes())->toBe($attributes); + expect($ssh->getExecutedCommands())->toBeEmpty(); + Queue::assertNothingPushed(); +})->with([ + ['PUT', 'cronjobs.site.update'], + ['POST', 'cronjobs.site.enable'], + ['POST', 'cronjobs.site.disable'], + ['DELETE', 'cronjobs.site.destroy'], +]); + +test('cronjob policies validate site ownership while preserving server contexts', function (string $ability) { + $resource = CronJob::factory()->create([ + 'server_id' => $this->server->id, + 'site_id' => $this->site->id, + ]); + $subject = in_array($ability, ['viewAny', 'create', 'manage']) ? CronJob::class : $resource; + + expect($this->user->can($ability, [$subject, $this->server]))->toBeTrue(); + expect($this->user->can($ability, [$subject, $this->server, $this->site]))->toBeTrue(); + + $otherServer = Server::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->server->project_id, + ]); + $this->site->update(['server_id' => $otherServer->id]); + + expect($this->user->can($ability, [$subject, $this->server, $this->site]))->toBeFalse(); +})->with(['viewAny', 'create', 'view', 'update', 'delete']); diff --git a/tests/Feature/SitesTest.php b/tests/Feature/SitesTest.php index a9251034d..9f71749f7 100644 --- a/tests/Feature/SitesTest.php +++ b/tests/Feature/SitesTest.php @@ -2,22 +2,144 @@ use App\Enums\ServiceStatus; use App\Enums\SiteStatus; +use App\Enums\UserRole; use App\Facades\SSH; use App\Models\Database; use App\Models\DatabaseUser; +use App\Models\Project; +use App\Models\Server; use App\Models\Service; use App\Models\Site; use App\Models\SourceControl; +use App\Models\User; use App\SiteTypes\Blank; use App\SiteTypes\Laravel; use App\SiteTypes\PHPBlank; +use App\SiteTypes\Wordpress; use App\SourceControlProviders\Github; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Queue; use Inertia\Testing\AssertableInertia; uses(RefreshDatabase::class); +test('site settings rejects outsiders and mismatched servers', function (bool $outsider) { + if ($outsider) { + $this->server->project->users()->where('user_id', $this->user->id)->delete(); + } + + $this->actingAs($this->user)->get(route('site-settings', [ + 'server' => $outsider ? $this->server : Server::factory()->create(), + 'site' => $this->site, + ]))->assertForbidden(); +})->with([true, false]); + +test('blank sites discard disabled source control input', function (?bool $enabled) { + SSH::fake(); + Http::fake(); + Queue::fake(); + + $sourceControl = SourceControl::factory()->create(['user_id' => User::factory()->create()->id]); + $input = [ + 'type' => Blank::id(), + 'domain' => 'blank-disabled.example.com', + 'port' => 3000, + 'user' => 'blankdisabled', + 'source_control' => $sourceControl->id, + 'repository' => 'private/repository', + 'branch' => 'private', + ]; + if ($enabled !== null) { + $input['use_source_control'] = $enabled; + } + + $this->actingAs($this->user)->post(route('sites.store', ['server' => $this->server]), $input) + ->assertRedirect()->assertSessionDoesntHaveErrors(); + + $this->assertDatabaseHas('sites', [ + 'domain' => $input['domain'], + 'source_control_id' => null, + 'repository' => '', + 'branch' => '', + ]); + Queue::assertPushed(\App\Jobs\Site\CreateJob::class, 1); + Http::assertNothingSent(); +})->with([false, null]); + +test('site source selection uses the actor rather than the server creator', function (string $operation, string $scope) { + SSH::fake(); + Http::fake(); + Queue::fake(); + + $creator = User::factory()->create(); + $this->server->update(['user_id' => $creator->id]); + $this->server->project->users()->where('user_id', $this->user->id)->update(['role' => UserRole::ADMIN]); + $sourceControl = SourceControl::factory()->create([ + 'user_id' => $scope === 'creator' ? $creator->id : $this->user->id, + 'project_id' => match ($scope) { + 'project' => $this->server->project_id, + 'foreign-project' => Project::factory()->create()->id, + default => null, + }, + ]); + $originalSource = $this->site->source_control_id; + $this->actingAs($this->user); + + $response = $operation === 'create' + ? $this->post(route('sites.store', ['server' => $this->server]), [ + 'type' => Blank::id(), + 'domain' => 'actor-source.example.com', + 'port' => 3000, + 'user' => 'actorsource', + 'use_source_control' => true, + 'source_control' => $sourceControl->id, + 'repository' => 'private/repository', + 'branch' => 'main', + ]) + : $this->patch(route('site-settings.update-source-control', [ + 'server' => $this->server, + 'site' => $this->site, + ]), ['source_control' => $sourceControl->id]); + + if (in_array($scope, ['creator', 'foreign-project'])) { + if ($scope === 'creator') { + $response->assertForbidden(); + } else { + $response->assertSessionHasErrors('source_control'); + } + $this->assertDatabaseHas('sites', ['id' => $this->site->id, 'source_control_id' => $originalSource]); + $this->assertDatabaseMissing('sites', ['domain' => 'actor-source.example.com']); + Http::assertNothingSent(); + Queue::assertNothingPushed(); + } else { + $response->assertRedirect()->assertSessionDoesntHaveErrors(); + $this->assertDatabaseHas('sites', ['source_control_id' => $sourceControl->id]); + Http::assertSent(fn ($request) => $request->hasHeader('Authorization', 'Bearer '.$sourceControl->access_token)); + } +})->with(['create', 'update'])->with(['creator', 'global', 'project', 'foreign-project']); + +test('web site resources hide installation passwords from read-only users', function () { + $this->server->project->users()->where('user_id', $this->user->id)->update(['role' => UserRole::USER]); + $typeData = [ + 'title' => 'Private WordPress', + 'password' => 'wordpress-install-secret', + 'database_password' => 'database-install-secret', + ]; + $this->site->update(['type' => Wordpress::id(), 'type_data' => $typeData]); + + $this->actingAs($this->user)->get(route('site-settings', [ + 'server' => $this->server, + 'site' => $this->site, + ]))->assertSuccessful()->assertInertia(fn (AssertableInertia $page) => $page + ->where('site.type_data.title', $typeData['title']) + ->missing('site.type_data.password') + ->missing('site.type_data.database_password') + ); + + expect($this->site->fresh()->type_data)->toBe($typeData); +}); + test('create site', function (array $inputs) { SSH::fake(); diff --git a/tests/Feature/WorkersTest.php b/tests/Feature/WorkersTest.php index e68ac1cc1..e7d281149 100644 --- a/tests/Feature/WorkersTest.php +++ b/tests/Feature/WorkersTest.php @@ -985,3 +985,90 @@ expect($bootstrap->isSiteBootstrap())->toBeTrue(); expect($other->isSiteBootstrap())->toBeFalse(); }); + +test('worker routes reject a site from another server', function (string $method, string $route) { + $ssh = SSH::fake(); + Queue::fake(); + + $otherServer = Server::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->server->project_id, + ]); + $service = $this->server->webserver()->replicate(); + $service->server_id = $otherServer->id; + $service->save(); + $site = Site::factory()->create(['server_id' => $otherServer->id]); + + $this->actingAs($this->user)->json($method, route($route, [ + 'server' => $this->server, + 'site' => $site, + ]), [ + 'name' => 'Forbidden worker', + 'command' => 'php artisan queue:work', + 'user' => 'vito', + 'auto_start' => true, + 'auto_restart' => true, + 'numprocs' => 1, + ])->assertForbidden(); + + $this->assertDatabaseCount('workers', 0); + expect($ssh->getExecutedCommands())->toBeEmpty(); + Queue::assertNothingPushed(); +})->with([ + ['GET', 'workers.site'], + ['POST', 'workers.store'], + ['POST', 'workers.resync'], + ['POST', 'workers.restart-all'], +]); + +test('worker routes reject a worker from another site', function (string $method, string $route) { + $ssh = SSH::fake(); + Queue::fake(); + + $site = Site::factory()->create(['server_id' => $this->server->id]); + $worker = Worker::factory()->create([ + 'server_id' => $this->server->id, + 'site_id' => $this->site->id, + ]); + $attributes = $worker->refresh()->getAttributes(); + + $this->actingAs($this->user)->json($method, route($route, [ + 'server' => $this->server, + 'site' => $site, + 'worker' => $worker, + ]), [ + 'name' => 'Changed worker', + 'command' => 'pwd', + 'user' => 'vito', + 'auto_start' => true, + 'auto_restart' => true, + 'numprocs' => 1, + ])->assertForbidden(); + + $this->assertDatabaseHas('workers', ['id' => $worker->id]); + expect($worker->refresh()->getAttributes())->toBe($attributes); + expect($ssh->getExecutedCommands())->toBeEmpty(); + Queue::assertNothingPushed(); +})->with([ + ['PUT', 'workers.update'], + ['DELETE', 'workers.destroy'], +]); + +test('worker policies validate site ownership while preserving server contexts', function (string $ability) { + $resource = Worker::factory()->create([ + 'server_id' => $this->server->id, + 'site_id' => $this->site->id, + ]); + $subject = in_array($ability, ['viewAny', 'create', 'manage']) ? Worker::class : $resource; + + expect($this->user->can($ability, [$subject, $this->server]))->toBeTrue(); + expect($this->user->can($ability, [$subject, $this->server, $this->site]))->toBeTrue(); + + $otherServer = Server::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->server->project_id, + ]); + $this->site->update(['server_id' => $otherServer->id]); + + expect($this->user->can($ability, [$subject, $this->server, $this->site]))->toBeFalse(); +})->with(['viewAny', 'create', 'manage', 'view', 'update', 'delete']); diff --git a/tests/Feature/WorkflowTest.php b/tests/Feature/WorkflowTest.php index dcaddbd2c..d86c8df78 100644 --- a/tests/Feature/WorkflowTest.php +++ b/tests/Feature/WorkflowTest.php @@ -1,11 +1,31 @@ actingAs($this->user); @@ -62,3 +82,119 @@ 'id' => $workflow->id, ]); }); + +test('workflow run routes reject runs belonging to another workflow', function (string $route, bool $foreignProject) { + Storage::fake('server-logs'); + $this->actingAs($this->user); + + $workflow = Workflow::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->user->current_project_id, + ]); + $otherWorkflow = Workflow::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $foreignProject ? Project::factory()->create()->id : $workflow->project_id, + ]); + $run = WorkflowRun::factory()->create([ + 'workflow_id' => $otherWorkflow->id, + 'user_id' => $this->user->id, + 'status' => WorkflowRunStatus::COMPLETED, + ]); + $run->log('Private workflow output'); + + $this->get(route($route, ['workflow' => $workflow, 'workflowRun' => $run])) + ->assertNotFound() + ->assertDontSee('Private workflow output'); +})->with(['workflow-runs.show', 'workflow-runs.log'])->with([false, true]); + +test('workflow run routes allow runs belonging to the authorized workflow', function () { + Storage::fake('server-logs'); + $this->actingAs($this->user); + $workflow = Workflow::factory()->create([ + 'user_id' => $this->user->id, + 'project_id' => $this->user->current_project_id, + ]); + $run = WorkflowRun::factory()->create([ + 'workflow_id' => $workflow->id, + 'user_id' => $this->user->id, + 'status' => WorkflowRunStatus::COMPLETED, + ]); + $run->log('Authorized workflow output'); + + $this->get(route('workflow-runs.show', ['workflow' => $workflow, 'workflowRun' => $run])) + ->assertSuccessful() + ->assertInertia(fn (AssertableInertia $page) => $page + ->component('workflow-runs/show') + ->where('workflowRun.id', $run->id)); + $this->get(route('workflow-runs.log', ['workflow' => $workflow, 'workflowRun' => $run])) + ->assertSuccessful() + ->assertSee('Authorized workflow output'); +}); + +test('workflow notification sends only through the selected authorized channel', function (string $scope) { + Mail::fake(); + Http::fake(); + $workflow = Workflow::factory()->create(['user_id' => $this->user->id, 'project_id' => $this->user->current_project_id]); + $channel = NotificationChannel::factory()->create([ + 'user_id' => $this->user->id, + 'provider' => Email::id(), + 'data' => ['email' => 'selected@example.com'], + 'project_id' => $scope === 'scoped' ? $workflow->project_id : null, + ]); + NotificationChannel::factory()->create([ + 'provider' => Slack::id(), + 'data' => ['webhook_url' => 'https://example.com/foreign-channel'], + ]); + + if ($scope !== 'session') { + $abilities = $scope === 'scoped' ? ['write', 'project:'.$workflow->project_id] : ['write']; + $this->user->withAccessToken($this->user->createToken('workflow', $abilities)->accessToken); + } + + (new Notify($this->user, $workflow))->run([ + 'notification_channel_id' => $channel->id, + 'email' => $this->user->email, + 'message' => 'Private workflow notification', + ]); + + Mail::assertSent(NotificationMail::class, fn (NotificationMail $mail) => $mail->hasTo('selected@example.com')); + Mail::assertSentCount(1); + Http::assertNothingSent(); +})->with(['session', 'scoped', 'unrestricted']); + +test('workflow notification rejects foreign and token excluded channels', function (string $scope) { + Mail::fake(); + Http::fake(); + $project = Project::factory()->create(); + $project->users()->create(['user_id' => $this->user->id, 'role' => UserRole::OWNER]); + $workflow = Workflow::factory()->create(['user_id' => $this->user->id, 'project_id' => $this->user->current_project_id]); + $channel = NotificationChannel::factory()->create([ + 'user_id' => $scope === 'foreign-owner' ? $this->notificationChannel->user_id : $this->user->id, + 'project_id' => $scope === 'global' ? null : $project->id, + 'provider' => Email::id(), + 'data' => ['email' => 'excluded@example.com'], + ]); + if ($scope !== 'foreign-owner') { + $token = $this->user->createToken('workflow', ['write', 'project:'.$workflow->project_id]); + $this->user->withAccessToken($token->accessToken); + } + + expect(fn () => (new Notify($this->user, $workflow))->run([ + 'notification_channel_id' => $channel->id, + 'email' => $this->user->email, + 'message' => 'Private workflow notification', + ]))->toThrow(AuthorizationException::class); + + Mail::assertNothingSent(); + Http::assertNothingSent(); +})->with(['foreign-owner', 'other-project', 'global']); + +test('project role policies enforce the token project scope', function (string $ability) { + $project = Project::factory()->create(); + $project->users()->create(['user_id' => $this->user->id, 'role' => UserRole::OWNER]); + $token = $this->user->createToken('workflow', ['read', 'write', 'project:'.$this->user->current_project_id]); + $this->user->withAccessToken($token->accessToken); + + expect($this->user->can($ability, $this->user->currentProject))->toBeTrue() + ->and($this->user->can($ability, $project))->toBeFalse(); +})->with(['view', 'update', 'delete']); From 0d2c4842187ef9e4c9f338f5f24a84bdded27107 Mon Sep 17 00:00:00 2001 From: Saeed Vaziry Date: Fri, 2 Oct 2026 19:54:55 +0200 Subject: [PATCH 2/2] [Fix] Correct workflow error handling and notification inputs - Propagate downstream authorization errors instead of triggering earlier failure branches. - Remove the unused notification email requirement while accepting legacy inputs. - Expand token-scope and notification tests and normalize site job imports. --- app/Actions/Workflow/RunWorkflow.php | 5 ++++- app/WorkflowActions/General/Notify.php | 2 -- tests/Feature/API/SitesTest.php | 3 ++- tests/Feature/Jobs/WorkflowRunJobTest.php | 8 +++++--- tests/Feature/SitesTest.php | 3 ++- tests/Feature/WorkflowTest.php | 14 +++++++++----- 6 files changed, 22 insertions(+), 13 deletions(-) diff --git a/app/Actions/Workflow/RunWorkflow.php b/app/Actions/Workflow/RunWorkflow.php index c29e30a7d..6d10a3ec1 100644 --- a/app/Actions/Workflow/RunWorkflow.php +++ b/app/Actions/Workflow/RunWorkflow.php @@ -65,11 +65,14 @@ public function executeAction(WorkflowRun $run, User $user, Workflow $workflow, try { $output = $workflowActionDto->handler($user, $workflow)->run($resolvedInput); - $this->executeAction($run, $user, $workflow, $workflowActionDto->success, $output, $accessTokenId); } catch (\Throwable $e) { $run->log('Workflow action failed: '.$e->getMessage()); $this->executeAction($run, $user, $workflow, $workflowActionDto->failure, $input, $accessTokenId); + + return; } + + $this->executeAction($run, $user, $workflow, $workflowActionDto->success, $output, $accessTokenId); } private function authorizeExecution(User $user, Workflow $workflow, ?int $accessTokenId): void diff --git a/app/WorkflowActions/General/Notify.php b/app/WorkflowActions/General/Notify.php index bc5b394b0..9e8abf834 100644 --- a/app/WorkflowActions/General/Notify.php +++ b/app/WorkflowActions/General/Notify.php @@ -13,7 +13,6 @@ public function inputs(): array { return [ 'notification_channel_id' => 'The ID of the notification channel to send the notification to', - 'email' => 'The email address of the user on Vito', 'message' => 'The message to send', ]; } @@ -28,7 +27,6 @@ public function run(array $input): array Validator::make($input, [ 'notification_channel_id' => ['required', 'integer', 'exists:notification_channels,id'], 'message' => ['required', 'string'], - 'email' => ['required', 'email', 'exists:users,email'], ])->validate(); $notificationChannel = NotificationChannel::query()->findOrFail($input['notification_channel_id']); diff --git a/tests/Feature/API/SitesTest.php b/tests/Feature/API/SitesTest.php index 074864fbd..4270a616f 100644 --- a/tests/Feature/API/SitesTest.php +++ b/tests/Feature/API/SitesTest.php @@ -4,6 +4,7 @@ use App\Enums\LoadBalancerMethod; use App\Enums\UserRole; use App\Facades\SSH; +use App\Jobs\Site\CreateJob; use App\Models\Database; use App\Models\DatabaseUser; use App\Models\Project; @@ -59,7 +60,7 @@ 'repository' => '', 'branch' => '', ]); - Queue::assertPushed(\App\Jobs\Site\CreateJob::class, 1); + Queue::assertPushed(CreateJob::class, 1); Http::assertNothingSent(); })->with([false, null]); diff --git a/tests/Feature/Jobs/WorkflowRunJobTest.php b/tests/Feature/Jobs/WorkflowRunJobTest.php index dd8493399..f22ae74bf 100644 --- a/tests/Feature/Jobs/WorkflowRunJobTest.php +++ b/tests/Feature/Jobs/WorkflowRunJobTest.php @@ -198,7 +198,7 @@ 'legacy job without authentication context' => [true, true], ]); -test('workflow jobs recheck current token scope between actions and failure branches', function (string $change) { +test('workflow jobs recheck current token scope between actions and failure branches', function (string $change, bool $hasFailureBranch) { SSH::fake(); Storage::fake('server-logs'); Queue::fake(); @@ -225,7 +225,7 @@ $tree = new WorkflowActionDTO('HTTP request', HttpCall::class, [], [ 'url' => 'https://example.com/workflow', 'method' => 'GET', - ], 'http', success: $command, failure: $failure); + ], 'http', success: $command, failure: $hasFailureBranch ? $failure : null); Http::fake(function () use ($token, $project, $change) { match ($change) { 'narrowed' => $token->accessToken->forceFill(['abilities' => ['write', 'project:'.$project->id]])->save(), @@ -248,9 +248,11 @@ SSH::assertNotExecutedContains('echo failure-workflow'); if ($change === 'revoked') { $job->assertFailedWith(AuthorizationException::class); + } else { + $job->assertNotFailed(); } expect((new ReflectionProperty($job, 'user'))->getValue($job)->currentAccessToken())->toBeNull(); -})->with(['unchanged', 'narrowed', 'revoked']); +})->with(['unchanged', 'narrowed', 'revoked'])->with([false, true]); test('nested workflows inherit token scope without changing the outer execution context', function (bool $revoked) { SSH::fake(); diff --git a/tests/Feature/SitesTest.php b/tests/Feature/SitesTest.php index 9f71749f7..441dc07b6 100644 --- a/tests/Feature/SitesTest.php +++ b/tests/Feature/SitesTest.php @@ -4,6 +4,7 @@ use App\Enums\SiteStatus; use App\Enums\UserRole; use App\Facades\SSH; +use App\Jobs\Site\CreateJob; use App\Models\Database; use App\Models\DatabaseUser; use App\Models\Project; @@ -63,7 +64,7 @@ 'repository' => '', 'branch' => '', ]); - Queue::assertPushed(\App\Jobs\Site\CreateJob::class, 1); + Queue::assertPushed(CreateJob::class, 1); Http::assertNothingSent(); })->with([false, null]); diff --git a/tests/Feature/WorkflowTest.php b/tests/Feature/WorkflowTest.php index d86c8df78..c5006df96 100644 --- a/tests/Feature/WorkflowTest.php +++ b/tests/Feature/WorkflowTest.php @@ -131,7 +131,7 @@ ->assertSee('Authorized workflow output'); }); -test('workflow notification sends only through the selected authorized channel', function (string $scope) { +test('workflow notification sends only through the selected authorized channel', function (string $scope, bool $legacyEmail) { Mail::fake(); Http::fake(); $workflow = Workflow::factory()->create(['user_id' => $this->user->id, 'project_id' => $this->user->current_project_id]); @@ -151,16 +151,20 @@ $this->user->withAccessToken($this->user->createToken('workflow', $abilities)->accessToken); } - (new Notify($this->user, $workflow))->run([ + $input = [ 'notification_channel_id' => $channel->id, - 'email' => $this->user->email, 'message' => 'Private workflow notification', - ]); + ]; + if ($legacyEmail) { + $input['email'] = 'deleted-user@example.com'; + } + + (new Notify($this->user, $workflow))->run($input); Mail::assertSent(NotificationMail::class, fn (NotificationMail $mail) => $mail->hasTo('selected@example.com')); Mail::assertSentCount(1); Http::assertNothingSent(); -})->with(['session', 'scoped', 'unrestricted']); +})->with(['session', 'scoped', 'unrestricted'])->with([false, true]); test('workflow notification rejects foreign and token excluded channels', function (string $scope) { Mail::fake();