From 28c5729e4865dcaf7feed79c0ce99094c9a63d72 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 28 Jun 2020 14:41:22 -0700 Subject: [PATCH] Add test coverage for creating tasks --- .../Service/ServiceLimitExceededException.php | 21 +++ .../Client/Servers/ScheduleTaskController.php | 9 +- config/pterodactyl.php | 5 + .../Client/ClientApiIntegrationTestCase.php | 30 +++ .../Schedule/DeleteServerScheduleTest.php | 2 +- .../Schedule/GetServerSchedulesTest.php | 2 +- .../Schedule/UpdateServerScheduleTest.php | 2 +- .../CreateServerScheduleTaskTest.php | 177 ++++++++++++++++++ 8 files changed, 243 insertions(+), 5 deletions(-) create mode 100644 app/Exceptions/Service/ServiceLimitExceededException.php create mode 100644 tests/Integration/Api/Client/Server/ScheduleTask/CreateServerScheduleTaskTest.php diff --git a/app/Exceptions/Service/ServiceLimitExceededException.php b/app/Exceptions/Service/ServiceLimitExceededException.php new file mode 100644 index 000000000..55ee6c94b --- /dev/null +++ b/app/Exceptions/Service/ServiceLimitExceededException.php @@ -0,0 +1,21 @@ +server_id !== $server->id) { - throw new NotFoundHttpException; + $limit = config('pterodactyl.client_features.schedules.per_schedule_task_limit', 10); + if ($schedule->tasks()->count() >= $limit) { + throw new ServiceLimitExceededException( + "Schedules may not have more than {$limit} tasks associated with them. Creating this task would put this schedule over the limit." + ); } $lastTask = $schedule->tasks->last(); diff --git a/config/pterodactyl.php b/config/pterodactyl.php index fe4f2372b..92404cd98 100644 --- a/config/pterodactyl.php +++ b/config/pterodactyl.php @@ -162,6 +162,11 @@ return [ 'enabled' => env('PTERODACTYL_CLIENT_DATABASES_ENABLED', true), 'allow_random' => env('PTERODACTYL_CLIENT_DATABASES_ALLOW_RANDOM', true), ], + + 'schedules' => [ + // The total number of tasks that can exist for any given schedule at once. + 'per_schedule_task_limit' => 10, + ], ], /* diff --git a/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php b/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php index 8fdb969fa..e6dbf0974 100644 --- a/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php +++ b/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php @@ -6,11 +6,14 @@ use Carbon\Carbon; use ReflectionClass; use Carbon\CarbonImmutable; use Pterodactyl\Models\Node; +use Pterodactyl\Models\Task; use Pterodactyl\Models\User; +use Webmozart\Assert\Assert; use Pterodactyl\Models\Model; use Pterodactyl\Models\Server; use Pterodactyl\Models\Subuser; use Pterodactyl\Models\Location; +use Pterodactyl\Models\Schedule; use Illuminate\Support\Collection; use Pterodactyl\Tests\Integration\IntegrationTestCase; use Pterodactyl\Transformers\Api\Client\BaseClientTransformer; @@ -41,6 +44,33 @@ abstract class ClientApiIntegrationTestCase extends IntegrationTestCase CarbonImmutable::setTestNow(Carbon::now()); } + /** + * Returns a link to the specific resource using the client API. + * + * @param mixed $model + * @param string|null $append + * @return string + */ + protected function link($model, $append = null): string + { + Assert::isInstanceOfAny($model, [Server::class, Schedule::class, Task::class]); + + $link = ''; + switch (get_class($model)) { + case Server::class: + $link = "/api/client/servers/{$model->uuid}"; + break; + case Schedule::class: + $link = "/api/client/servers/{$model->server->uuid}/schedules/{$model->id}"; + break; + case Task::class: + $link = "/api/client/servers/{$model->schedule->server->uuid}/schedules/{$model->schedule->id}/tasks/{$model->id}"; + break; + } + + return $link . ($append ? '/' . ltrim($append, '/') : ''); + } + /** * Generates a user and a server for that user. If an array of permissions is passed it * is assumed that the user is actually a subuser of the server. diff --git a/tests/Integration/Api/Client/Server/Schedule/DeleteServerScheduleTest.php b/tests/Integration/Api/Client/Server/Schedule/DeleteServerScheduleTest.php index d5f002b49..cff7591ba 100644 --- a/tests/Integration/Api/Client/Server/Schedule/DeleteServerScheduleTest.php +++ b/tests/Integration/Api/Client/Server/Schedule/DeleteServerScheduleTest.php @@ -50,7 +50,7 @@ class DeleteServerScheduleTest extends ClientApiIntegrationTestCase public function testNotFoundErrorIsReturnedIfScheduleDoesNotBelongToServer() { [$user, $server] = $this->generateTestAccount(); - [, $server2] = $this->generateTestAccount(); + [, $server2] = $this->generateTestAccount(['user_id' => $user->id]); $schedule = factory(Schedule::class)->create(['server_id' => $server2->id]); diff --git a/tests/Integration/Api/Client/Server/Schedule/GetServerSchedulesTest.php b/tests/Integration/Api/Client/Server/Schedule/GetServerSchedulesTest.php index d7afa0d09..67be3fd84 100644 --- a/tests/Integration/Api/Client/Server/Schedule/GetServerSchedulesTest.php +++ b/tests/Integration/Api/Client/Server/Schedule/GetServerSchedulesTest.php @@ -64,7 +64,7 @@ class GetServerSchedulesTest extends ClientApiIntegrationTestCase public function testScheduleBelongingToAnotherServerCannotBeViewed() { [$user, $server] = $this->generateTestAccount(); - [, $server2] = $this->generateTestAccount(); + [, $server2] = $this->generateTestAccount(['user_id' => $user->id]); $schedule = factory(Schedule::class)->create(['server_id' => $server2->id]); diff --git a/tests/Integration/Api/Client/Server/Schedule/UpdateServerScheduleTest.php b/tests/Integration/Api/Client/Server/Schedule/UpdateServerScheduleTest.php index d5c5cf235..d2d3132ff 100644 --- a/tests/Integration/Api/Client/Server/Schedule/UpdateServerScheduleTest.php +++ b/tests/Integration/Api/Client/Server/Schedule/UpdateServerScheduleTest.php @@ -50,7 +50,7 @@ class UpdateServerScheduleTest extends ClientApiIntegrationTestCase public function testErrorIsReturnedIfScheduleDoesNotBelongToServer() { [$user, $server] = $this->generateTestAccount(); - [, $server2] = $this->generateTestAccount(); + [, $server2] = $this->generateTestAccount(['user_id' => $user->id]); $schedule = factory(Schedule::class)->create(['server_id' => $server2->id]); diff --git a/tests/Integration/Api/Client/Server/ScheduleTask/CreateServerScheduleTaskTest.php b/tests/Integration/Api/Client/Server/ScheduleTask/CreateServerScheduleTaskTest.php new file mode 100644 index 000000000..2044221e5 --- /dev/null +++ b/tests/Integration/Api/Client/Server/ScheduleTask/CreateServerScheduleTaskTest.php @@ -0,0 +1,177 @@ +generateTestAccount($permissions); + + /** @var \Pterodactyl\Models\Schedule $schedule */ + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + $this->assertEmpty($schedule->tasks); + + $response = $this->actingAs($user)->postJson($this->link($schedule, '/tasks'), [ + 'action' => 'command', + 'payload' => 'say Test', + 'time_offset' => 10, + 'sequence_id' => 1, + ]); + + $response->assertOk(); + /** @var \Pterodactyl\Models\Task $task */ + $task = Task::query()->findOrFail($response->json('attributes.id')); + + $this->assertSame($schedule->id, $task->schedule_id); + $this->assertSame(1, $task->sequence_id); + $this->assertSame('command', $task->action); + $this->assertSame('say Test', $task->payload); + $this->assertSame(10, $task->time_offset); + $this->assertJsonTransformedWith($response->json('attributes'), $task); + } + + /** + * Test that validation errors are returned correctly if bad data is passed into the API. + */ + public function testValidationErrorsAreReturned() + { + [$user, $server] = $this->generateTestAccount(); + + /** @var \Pterodactyl\Models\Schedule $schedule */ + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + + $response = $this->actingAs($user)->postJson($this->link($schedule, '/tasks'))->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY); + + foreach (['action', 'payload', 'time_offset'] as $i => $field) { + $response->assertJsonPath("errors.{$i}.code", $field === 'payload' ? 'required_unless' : 'required'); + $response->assertJsonPath("errors.{$i}.source.field", $field); + } + + $this->actingAs($user)->postJson($this->link($schedule, '/tasks'), [ + 'action' => 'hodor', + 'payload' => 'say Test', + 'time_offset' => 0, + ]) + ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) + ->assertJsonPath('errors.0.code', 'in') + ->assertJsonPath('errors.0.source.field', 'action'); + + $this->actingAs($user)->postJson($this->link($schedule, '/tasks'), [ + 'action' => 'command', + 'time_offset' => 0, + ]) + ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) + ->assertJsonPath('errors.0.code', 'required_unless') + ->assertJsonPath('errors.0.source.field', 'payload'); + + $this->actingAs($user)->postJson($this->link($schedule, '/tasks'), [ + 'action' => 'command', + 'payload' => 'say Test', + 'time_offset' => 0, + 'sequence_id' => 'hodor', + ]) + ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) + ->assertJsonPath('errors.0.code', 'numeric') + ->assertJsonPath('errors.0.source.field', 'sequence_id'); + } + + /** + * Test that backups can be tasked out correctly since they do not require a payload. + */ + public function testBackupsCanBeTaskedCorrectly() + { + [$user, $server] = $this->generateTestAccount(); + + /** @var \Pterodactyl\Models\Schedule $schedule */ + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + + $this->actingAs($user)->postJson($this->link($schedule, '/tasks'), [ + 'action' => 'backup', + 'time_offset' => 0, + ])->assertOk(); + + $this->actingAs($user)->postJson($this->link($schedule, '/tasks'), [ + 'action' => 'backup', + 'payload' => "file.txt\nfile2.log", + 'time_offset' => 0, + ])->assertOk(); + } + + /** + * Test that an error is returned if the user attempts to create an additional task that + * would put the schedule over the task limit. + */ + public function testErrorIsReturnedIfTooManyTasksExistForSchedule() + { + config()->set('pterodactyl.client_features.schedules.per_schedule_task_limit', 2); + + [$user, $server] = $this->generateTestAccount(); + + /** @var \Pterodactyl\Models\Schedule $schedule */ + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + factory(Task::class)->times(2)->create(['schedule_id' => $schedule->id]); + + $this->actingAs($user)->postJson($this->link($schedule, '/tasks'), [ + 'action' => 'command', + 'payload' => 'say test', + 'time_offset' => 0, + ]) + ->assertStatus(Response::HTTP_BAD_REQUEST) + ->assertJsonPath('errors.0.code', 'ServiceLimitExceededException') + ->assertJsonPath('errors.0.detail', 'Schedules may not have more than 2 tasks associated with them. Creating this task would put this schedule over the limit.'); + } + + /** + * Test that an error is returned if the targeted schedule does not belong to the server + * in the request. + */ + public function testErrorIsReturnedIfScheduleDoesNotBelongToServer() + { + [$user, $server] = $this->generateTestAccount(); + [, $server2] = $this->generateTestAccount(['user_id' => $user->id]); + + /** @var \Pterodactyl\Models\Schedule $schedule */ + $schedule = factory(Schedule::class)->create(['server_id' => $server2->id]); + + $this->actingAs($user) + ->postJson("/api/client/servers/{$server->uuid}/schedules/{$schedule->id}/tasks") + ->assertNotFound(); + } + + /** + * Test that an error is returned if the subuser making the request does not have permission + * to update a schedule. + */ + public function testErrorIsReturnedIfSubuserDoesNotHaveScheduleUpdatePermissions() + { + [$user, $server] = $this->generateTestAccount([Permission::ACTION_SCHEDULE_CREATE]); + + /** @var \Pterodactyl\Models\Schedule $schedule */ + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + + $this->actingAs($user) + ->postJson($this->link($schedule, '/tasks')) + ->assertForbidden(); + } + + /** + * @return array + */ + public function permissionsDataProvider(): array + { + return [[[]], [[Permission::ACTION_SCHEDULE_UPDATE]]]; + } +}