From b9a451b528cbe1f9aebda6fcfc2d596ecded582b Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 28 Jun 2020 13:50:07 -0700 Subject: [PATCH] Add test coverage for schedules --- .../Api/Client/Servers/ScheduleController.php | 2 +- .../Schedules/StoreScheduleRequest.php | 2 +- .../Client/ClientApiIntegrationTestCase.php | 38 +++++++ .../Schedule/CreateServerScheduleTest.php | 96 ++++++++++++++++ .../Schedule/DeleteServerScheduleTest.php | 88 +++++++++++++++ .../Schedule/GetServerSchedulesTest.php | 106 ++++++++++++++++++ .../Schedule/UpdateServerScheduleTest.php | 84 ++++++++++++++ 7 files changed, 414 insertions(+), 2 deletions(-) create mode 100644 tests/Integration/Api/Client/Server/Schedule/CreateServerScheduleTest.php create mode 100644 tests/Integration/Api/Client/Server/Schedule/DeleteServerScheduleTest.php create mode 100644 tests/Integration/Api/Client/Server/Schedule/GetServerSchedulesTest.php create mode 100644 tests/Integration/Api/Client/Server/Schedule/UpdateServerScheduleTest.php diff --git a/app/Http/Controllers/Api/Client/Servers/ScheduleController.php b/app/Http/Controllers/Api/Client/Servers/ScheduleController.php index d1a6b9f4..593bbc25 100644 --- a/app/Http/Controllers/Api/Client/Servers/ScheduleController.php +++ b/app/Http/Controllers/Api/Client/Servers/ScheduleController.php @@ -147,7 +147,7 @@ class ScheduleController extends ClientApiController { $this->repository->delete($schedule->id); - return JsonResponse::create([], Response::HTTP_NO_CONTENT); + return new JsonResponse([], Response::HTTP_NO_CONTENT); } /** diff --git a/app/Http/Requests/Api/Client/Servers/Schedules/StoreScheduleRequest.php b/app/Http/Requests/Api/Client/Servers/Schedules/StoreScheduleRequest.php index 3db1aca6..618cd9f4 100644 --- a/app/Http/Requests/Api/Client/Servers/Schedules/StoreScheduleRequest.php +++ b/app/Http/Requests/Api/Client/Servers/Schedules/StoreScheduleRequest.php @@ -21,7 +21,7 @@ class StoreScheduleRequest extends ViewScheduleRequest { return [ 'name' => 'required|string|min:1', - 'is_active' => 'boolean', + 'is_active' => 'filled|boolean', 'minute' => 'required|string', 'hour' => 'required|string', 'day_of_month' => 'required|string', diff --git a/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php b/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php index ed36e3dd..8fdb969f 100644 --- a/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php +++ b/tests/Integration/Api/Client/ClientApiIntegrationTestCase.php @@ -2,12 +2,18 @@ namespace Pterodactyl\Tests\Integration\Api\Client; +use Carbon\Carbon; +use ReflectionClass; +use Carbon\CarbonImmutable; use Pterodactyl\Models\Node; use Pterodactyl\Models\User; +use Pterodactyl\Models\Model; use Pterodactyl\Models\Server; use Pterodactyl\Models\Subuser; use Pterodactyl\Models\Location; +use Illuminate\Support\Collection; use Pterodactyl\Tests\Integration\IntegrationTestCase; +use Pterodactyl\Transformers\Api\Client\BaseClientTransformer; abstract class ClientApiIntegrationTestCase extends IntegrationTestCase { @@ -24,6 +30,17 @@ abstract class ClientApiIntegrationTestCase extends IntegrationTestCase parent::tearDown(); } + /** + * Setup tests and ensure all of the times are always the same. + */ + public function setUp(): void + { + parent::setUp(); + + Carbon::setTestNow(Carbon::now()); + CarbonImmutable::setTestNow(Carbon::now()); + } + /** * 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. @@ -50,4 +67,25 @@ abstract class ClientApiIntegrationTestCase extends IntegrationTestCase return [$user, $server]; } + + /** + * Asserts that the data passed through matches the output of the data from the transformer. This + * will remove the "relationships" key when performing the comparison. + * + * @param array $data + * @param \Pterodactyl\Models\Model|\Illuminate\Database\Eloquent\Model $model + */ + protected function assertJsonTransformedWith(array $data, $model) + { + $reflect = new ReflectionClass($model); + $transformer = sprintf('\\Pterodactyl\\Transformers\\Api\\Client\\%sTransformer', $reflect->getShortName()); + + $transformer = new $transformer; + $this->assertInstanceOf(BaseClientTransformer::class, $transformer); + + $this->assertSame( + $transformer->transform($model), + Collection::make($data)->except(['relationships'])->toArray() + ); + } } diff --git a/tests/Integration/Api/Client/Server/Schedule/CreateServerScheduleTest.php b/tests/Integration/Api/Client/Server/Schedule/CreateServerScheduleTest.php new file mode 100644 index 00000000..5b45e7fe --- /dev/null +++ b/tests/Integration/Api/Client/Server/Schedule/CreateServerScheduleTest.php @@ -0,0 +1,96 @@ +generateTestAccount($permissions); + + $response = $this->actingAs($user)->postJson("/api/client/servers/{$server->uuid}/schedules", [ + 'name' => 'Test Schedule', + 'is_active' => false, + 'minute' => '0', + 'hour' => '*/2', + 'day_of_week' => '2', + 'day_of_month' => '*', + ]); + + $response->assertOk(); + + $this->assertNotNull($id = $response->json('attributes.id')); + + /** @var \Pterodactyl\Models\Schedule $schedule */ + $schedule = Schedule::query()->findOrFail($id); + $this->assertFalse($schedule->is_active); + $this->assertFalse($schedule->is_processing); + $this->assertSame('0', $schedule->cron_minute); + $this->assertSame('*/2', $schedule->cron_hour); + $this->assertSame('2', $schedule->cron_day_of_week); + $this->assertSame('*', $schedule->cron_day_of_month); + $this->assertSame('Test Schedule', $schedule->name); + + $this->assertJsonTransformedWith($response->json('attributes'), $schedule); + $response->assertJsonCount(0, 'attributes.relationships.tasks.data'); + } + + /** + * Test that the validation rules for scheduling work as expected. + */ + public function testScheduleValidationRules() + { + [$user, $server] = $this->generateTestAccount(); + + $response = $this->actingAs($user)->postJson("/api/client/servers/{$server->uuid}/schedules", []); + + $response->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY); + foreach (['name', 'minute', 'hour', 'day_of_month', 'day_of_week'] as $i => $field) { + $response->assertJsonPath("errors.{$i}.code", 'required'); + $response->assertJsonPath("errors.{$i}.source.field", $field); + } + + $this->actingAs($user) + ->postJson("/api/client/servers/{$server->uuid}/schedules", [ + 'name' => 'Testing', + 'is_active' => 'no', + 'minute' => '*', + 'hour' => '*', + 'day_of_month' => '*', + 'day_of_week' => '*', + ]) + ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) + ->assertJsonPath('errors.0.code', 'boolean'); + } + + /** + * Test that a subuser without required permissions cannot create a schedule. + */ + public function testSubuserCannotCreateScheduleWithoutPermissions() + { + [$user, $server] = $this->generateTestAccount([Permission::ACTION_SCHEDULE_UPDATE]); + + $this->actingAs($user) + ->postJson("/api/client/servers/{$server->uuid}/schedules", []) + ->assertForbidden(); + } + + /** + * @return array + */ + public function permissionsDataProvider(): array + { + return [[[]], [[Permission::ACTION_SCHEDULE_CREATE]]]; + } +} diff --git a/tests/Integration/Api/Client/Server/Schedule/DeleteServerScheduleTest.php b/tests/Integration/Api/Client/Server/Schedule/DeleteServerScheduleTest.php new file mode 100644 index 00000000..d5f002b4 --- /dev/null +++ b/tests/Integration/Api/Client/Server/Schedule/DeleteServerScheduleTest.php @@ -0,0 +1,88 @@ +generateTestAccount($permissions); + + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + $task = factory(Task::class)->create(['schedule_id' => $schedule->id]); + + $this->actingAs($user) + ->deleteJson("/api/client/servers/{$server->uuid}/schedules/{$schedule->id}") + ->assertStatus(Response::HTTP_NO_CONTENT); + + $this->assertDatabaseMissing('schedules', ['id' => $schedule->id]); + $this->assertDatabaseMissing('tasks', ['id' => $task->id]); + } + + /** + * Test that no error is returned if the schedule does not exist on the system at all. + */ + public function testNotFoundErrorIsReturnedIfScheduleDoesNotExistAtAll() + { + [$user, $server] = $this->generateTestAccount(); + + $this->actingAs($user) + ->deleteJson("/api/client/servers/{$server->uuid}/schedules/123456789") + ->assertStatus(Response::HTTP_NOT_FOUND); + } + + /** + * Ensure that a schedule belonging to another server cannot be deleted and its presence is not + * revealed to the user. + */ + public function testNotFoundErrorIsReturnedIfScheduleDoesNotBelongToServer() + { + [$user, $server] = $this->generateTestAccount(); + [, $server2] = $this->generateTestAccount(); + + $schedule = factory(Schedule::class)->create(['server_id' => $server2->id]); + + $this->actingAs($user) + ->deleteJson("/api/client/servers/{$server->uuid}/schedules/{$schedule->id}") + ->assertStatus(Response::HTTP_NOT_FOUND); + + $this->assertDatabaseHas('schedules', ['id' => $schedule->id]); + } + + /** + * Test that an error is returned if the subuser does not have the required permissions to + * delete the schedule from the server. + */ + public function testErrorIsReturnedIfSubuserDoesNotHaveRequiredPermissions() + { + [$user, $server] = $this->generateTestAccount([Permission::ACTION_SCHEDULE_UPDATE]); + + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + + $this->actingAs($user) + ->deleteJson("/api/client/servers/{$server->uuid}/schedules/{$schedule->id}") + ->assertStatus(Response::HTTP_FORBIDDEN); + + $this->assertDatabaseHas('schedules', ['id' => $schedule->id]); + } + + /** + * @return array + */ + public function permissionsDataProvider(): array + { + return [[[]], [[Permission::ACTION_SCHEDULE_DELETE]]]; + } +} diff --git a/tests/Integration/Api/Client/Server/Schedule/GetServerSchedulesTest.php b/tests/Integration/Api/Client/Server/Schedule/GetServerSchedulesTest.php new file mode 100644 index 00000000..d7afa0d0 --- /dev/null +++ b/tests/Integration/Api/Client/Server/Schedule/GetServerSchedulesTest.php @@ -0,0 +1,106 @@ +forceDelete(); + Schedule::query()->forceDelete(); + + parent::tearDown(); + } + + /** + * Test that schedules for a server are returned. + * + * @param array $permissions + * @param bool $individual + * @dataProvider permissionsDataProvider + */ + public function testServerSchedulesAreReturned($permissions, $individual) + { + [$user, $server] = $this->generateTestAccount($permissions); + + /** @var \Pterodactyl\Models\Schedule $schedule */ + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + /** @var \Pterodactyl\Models\Task $task */ + $task = factory(Task::class)->create(['schedule_id' => $schedule->id, 'sequence_id' => 1, 'time_offset' => 0]); + + $response = $this->actingAs($user) + ->getJson( + $individual + ? "/api/client/servers/{$server->uuid}/schedules/{$schedule->id}" + : "/api/client/servers/{$server->uuid}/schedules" + ) + ->assertOk(); + + $prefix = $individual ? '' : 'data.0.'; + if (! $individual) { + $response->assertJsonCount(1, 'data'); + } + + $response->assertJsonCount(1, $prefix . 'attributes.relationships.tasks.data'); + + $response->assertJsonPath($prefix . 'object', Schedule::RESOURCE_NAME); + $response->assertJsonPath($prefix . 'attributes.relationships.tasks.data.0.object', Task::RESOURCE_NAME); + + $this->assertJsonTransformedWith($response->json($prefix . 'attributes'), $schedule); + $this->assertJsonTransformedWith($response->json($prefix . 'attributes.relationships.tasks.data.0.attributes'), $task); + } + + /** + * Test that a schedule belonging to another server cannot be viewed. + */ + public function testScheduleBelongingToAnotherServerCannotBeViewed() + { + [$user, $server] = $this->generateTestAccount(); + [, $server2] = $this->generateTestAccount(); + + $schedule = factory(Schedule::class)->create(['server_id' => $server2->id]); + + $this->actingAs($user) + ->getJson("/api/client/servers/{$server->uuid}/schedules/{$schedule->id}") + ->assertNotFound(); + } + + /** + * Test that a subuser without the required permissions is unable to access the schedules endpoint. + */ + public function testUserWithoutPermissionCannotViewSchedules() + { + [$user, $server] = $this->generateTestAccount([Permission::ACTION_WEBSOCKET_CONNECT]); + + $this->actingAs($user) + ->getJson("/api/client/servers/{$server->uuid}/schedules") + ->assertForbidden(); + + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + + $this->actingAs($user) + ->getJson("/api/client/servers/{$server->uuid}/schedules/{$schedule->id}") + ->assertForbidden(); + } + + /** + * @return array + */ + public function permissionsDataProvider(): array + { + return [ + [[], false], + [[], true], + [[Permission::ACTION_SCHEDULE_READ], false], + [[Permission::ACTION_SCHEDULE_READ], true], + ]; + } +} diff --git a/tests/Integration/Api/Client/Server/Schedule/UpdateServerScheduleTest.php b/tests/Integration/Api/Client/Server/Schedule/UpdateServerScheduleTest.php new file mode 100644 index 00000000..d5c5cf23 --- /dev/null +++ b/tests/Integration/Api/Client/Server/Schedule/UpdateServerScheduleTest.php @@ -0,0 +1,84 @@ +generateTestAccount($permissions); + + /** @var \Pterodactyl\Models\Schedule $schedule */ + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + $expected = Utilities::getScheduleNextRunDate('5', '*', '*', '*'); + + $response = $this->actingAs($user) + ->postJson("/api/client/servers/{$server->uuid}/schedules/{$schedule->id}", [ + 'name' => 'Updated Schedule Name', + 'minute' => '5', + 'hour' => '*', + 'day_of_week' => '*', + 'day_of_month' => '*', + 'is_active' => false, + ]); + + $schedule = $schedule->refresh(); + + $response->assertOk(); + $this->assertSame('Updated Schedule Name', $schedule->name); + $this->assertFalse($schedule->is_active); + $this->assertJsonTransformedWith($response->json('attributes'), $schedule); + + $this->assertSame($expected->toIso8601String(), $schedule->next_run_at->toIso8601String()); + } + + /** + * Test that an error is returned if the schedule exists but does not belong to this + * specific server instance. + */ + public function testErrorIsReturnedIfScheduleDoesNotBelongToServer() + { + [$user, $server] = $this->generateTestAccount(); + [, $server2] = $this->generateTestAccount(); + + $schedule = factory(Schedule::class)->create(['server_id' => $server2->id]); + + $this->actingAs($user) + ->postJson("/api/client/servers/{$server->uuid}/schedules/{$schedule->id}") + ->assertNotFound(); + } + + /** + * Test that an error is returned if the subuser does not have permission to modify a + * server schedule. + */ + public function testErrorIsReturnedIfSubuserDoesNotHavePermissionToModifySchedule() + { + [$user, $server] = $this->generateTestAccount([Permission::ACTION_SCHEDULE_CREATE]); + + $schedule = factory(Schedule::class)->create(['server_id' => $server->id]); + + $this->actingAs($user) + ->postJson("/api/client/servers/{$server->uuid}/schedules/{$schedule->id}") + ->assertForbidden(); + } + + /** + * @return array + */ + public function permissionsDataProvider(): array + { + return [[[]], [[Permission::ACTION_SCHEDULE_UPDATE]]]; + } +}