From d49368551833418c27f94721021e13fb9da7218c Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 31 Oct 2020 21:57:27 -0700 Subject: [PATCH] Add test coverage for allocation assignment endpoint --- .../Servers/NetworkAllocationController.php | 6 ++ .../FindAssignableAllocationService.php | 6 -- .../Allocation/CreateNewAllocationTest.php | 97 +++++++++++++++++++ .../Allocation/DeleteAllocationTest.php | 91 +++++++++++++++++ .../NetworkAllocationControllerTest.php | 81 ++-------------- .../Traits/Integration/CreatesTestModels.php | 2 + 6 files changed, 204 insertions(+), 79 deletions(-) create mode 100644 tests/Integration/Api/Client/Server/Allocation/CreateNewAllocationTest.php create mode 100644 tests/Integration/Api/Client/Server/Allocation/DeleteAllocationTest.php diff --git a/app/Http/Controllers/Api/Client/Servers/NetworkAllocationController.php b/app/Http/Controllers/Api/Client/Servers/NetworkAllocationController.php index b26107649..43140f622 100644 --- a/app/Http/Controllers/Api/Client/Servers/NetworkAllocationController.php +++ b/app/Http/Controllers/Api/Client/Servers/NetworkAllocationController.php @@ -122,6 +122,12 @@ class NetworkAllocationController extends ClientApiController */ public function store(NewAllocationRequest $request, Server $server): array { + if ($server->allocations()->count() >= $server->allocation_limit) { + throw new DisplayException( + 'Cannot assign additional allocations to this server: limit has been reached.' + ); + } + $allocation = $this->assignableAllocationService->handle($server); return $this->fractal->item($allocation) diff --git a/app/Services/Allocations/FindAssignableAllocationService.php b/app/Services/Allocations/FindAssignableAllocationService.php index ec816cb81..ed3638265 100644 --- a/app/Services/Allocations/FindAssignableAllocationService.php +++ b/app/Services/Allocations/FindAssignableAllocationService.php @@ -46,12 +46,6 @@ class FindAssignableAllocationService throw new AutoAllocationNotEnabledException; } - if ($server->allocations()->count() >= $server->allocation_limit) { - throw new DisplayException( - 'Cannot assign additional allocations to this server: limit has been reached.' - ); - } - // Attempt to find a given available allocation for a server. If one cannot be found // we will fall back to attempting to create a new allocation that can be used for the // server. diff --git a/tests/Integration/Api/Client/Server/Allocation/CreateNewAllocationTest.php b/tests/Integration/Api/Client/Server/Allocation/CreateNewAllocationTest.php new file mode 100644 index 000000000..ab33ba81b --- /dev/null +++ b/tests/Integration/Api/Client/Server/Allocation/CreateNewAllocationTest.php @@ -0,0 +1,97 @@ +set('pterodactyl.client_features.allocations.enabled', true); + config()->set('pterodactyl.client_features.allocations.range_start', 5000); + config()->set('pterodactyl.client_features.allocations.range_end', 5050); + } + + /** + * Tests that a new allocation can be properly assigned to a server. + * + * @param array $permission + * @dataProvider permissionDataProvider + */ + public function testNewAllocationCanBeAssignedToServer(array $permission) + { + /** @var \Pterodactyl\Models\Server $server */ + [$user, $server] = $this->generateTestAccount($permission); + $server->update(['allocation_limit' => 2]); + + $response = $this->actingAs($user)->postJson($this->link($server, "/network/allocations")); + $response->assertJsonPath('object', Allocation::RESOURCE_NAME); + + $matched = Allocation::query()->findOrFail($response->json('attributes.id')); + + $this->assertSame($server->id, $matched->server_id); + $this->assertJsonTransformedWith($response->json('attributes'), $matched); + } + + /** + * Test that a user without the required permissions cannot create an allocation for + * the server instance. + */ + public function testAllocationCannotBeCreatedIfUserDoesNotHavePermission() + { + /** @var \Pterodactyl\Models\Server $server */ + [$user, $server] = $this->generateTestAccount([Permission::ACTION_ALLOCATION_UPDATE]); + $server->update(['allocation_limit' => 2]); + + $this->actingAs($user)->postJson($this->link($server, "/network/allocations"))->assertForbidden(); + } + + /** + * Test that an error is returned to the user if this feature is not enabled on the system. + */ + public function testAllocationCannotBeCreatedIfNotEnabled() + { + config()->set('pterodactyl.client_features.allocations.enabled', false); + + /** @var \Pterodactyl\Models\Server $server */ + [$user, $server] = $this->generateTestAccount(); + $server->update(['allocation_limit' => 2]); + + $this->actingAs($user)->postJson($this->link($server, "/network/allocations")) + ->assertStatus(Response::HTTP_BAD_REQUEST) + ->assertJsonPath('errors.0.code', 'AutoAllocationNotEnabledException') + ->assertJsonPath('errors.0.detail', 'Server auto-allocation is not enabled for this instance.'); + } + + /** + * Test that an allocation cannot be created if the server has reached it's allocation limit. + */ + public function testAllocationCannotBeCreatedIfServerIsAtLimit() + { + /** @var \Pterodactyl\Models\Server $server */ + [$user, $server] = $this->generateTestAccount(); + $server->update(['allocation_limit' => 1]); + + $this->actingAs($user)->postJson($this->link($server, "/network/allocations")) + ->assertStatus(Response::HTTP_BAD_REQUEST) + ->assertJsonPath('errors.0.code', 'DisplayException') + ->assertJsonPath('errors.0.detail', 'Cannot assign additional allocations to this server: limit has been reached.'); + } + + /** + * @return array + */ + public function permissionDataProvider() + { + return [[[Permission::ACTION_ALLOCATION_CREATE]], [[]]]; + } +} diff --git a/tests/Integration/Api/Client/Server/Allocation/DeleteAllocationTest.php b/tests/Integration/Api/Client/Server/Allocation/DeleteAllocationTest.php new file mode 100644 index 000000000..5b14c1a7e --- /dev/null +++ b/tests/Integration/Api/Client/Server/Allocation/DeleteAllocationTest.php @@ -0,0 +1,91 @@ +generateTestAccount($permission); + + /** @var \Pterodactyl\Models\Allocation $allocation */ + $allocation = factory(Allocation::class)->create([ + 'server_id' => $server->id, + 'node_id' => $server->node_id, + 'notes' => 'hodor', + ]); + + $this->actingAs($user)->deleteJson($this->link($allocation))->assertStatus(Response::HTTP_NO_CONTENT); + + $this->assertDatabaseHas('allocations', ['id' => $allocation->id, 'server_id' => null, 'notes' => null]); + } + + /** + * Test that an error is returned if the user does not have permissiont to delete an allocation. + */ + public function testErrorIsReturnedIfUserDoesNotHavePermission() + { + /** @var \Pterodactyl\Models\Server $server */ + [$user, $server] = $this->generateTestAccount([Permission::ACTION_ALLOCATION_CREATE]); + + /** @var \Pterodactyl\Models\Allocation $allocation */ + $allocation = factory(Allocation::class)->create([ + 'server_id' => $server->id, + 'node_id' => $server->node_id, + 'notes' => 'hodor', + ]); + + $this->actingAs($user)->deleteJson($this->link($allocation))->assertForbidden(); + + $this->assertDatabaseHas('allocations', ['id' => $allocation->id, 'server_id' => $server->id]); + } + + /** + * Test that an allocation is not deleted if it is currently marked as the primary allocation + * for the server. + */ + public function testErrorIsReturnedIfAllocationIsPrimary() + { + /** @var \Pterodactyl\Models\Server $server */ + [$user, $server] = $this->generateTestAccount(); + + $this->actingAs($user)->deleteJson($this->link($server->allocation)) + ->assertStatus(Response::HTTP_BAD_REQUEST) + ->assertJsonPath('errors.0.code', 'DisplayException') + ->assertJsonPath('errors.0.detail', 'You cannot delete the primary allocation for this server.'); + } + + /** + * Test that an allocation cannot be deleted if it does not belong to the server instance. + */ + public function testErrorIsReturnedIfAllocationDoesNotBelongToServer() + { + /** @var \Pterodactyl\Models\Server $server */ + [$user, $server] = $this->generateTestAccount(); + [, $server2] = $this->generateTestAccount(); + + $this->actingAs($user)->deleteJson($this->link($server2->allocation))->assertNotFound(); + $this->actingAs($user)->deleteJson($this->link($server, "/network/allocations/{$server2->allocation_id}"))->assertNotFound(); + } + + /** + * @return array + */ + public function permissionDataProvider() + { + return [[[Permission::ACTION_ALLOCATION_DELETE]], [[]]]; + } +} diff --git a/tests/Integration/Api/Client/Server/NetworkAllocationControllerTest.php b/tests/Integration/Api/Client/Server/NetworkAllocationControllerTest.php index ffebc83b8..185375e86 100644 --- a/tests/Integration/Api/Client/Server/NetworkAllocationControllerTest.php +++ b/tests/Integration/Api/Client/Server/NetworkAllocationControllerTest.php @@ -4,7 +4,6 @@ namespace Pterodactyl\Tests\Integration\Api\Client\Server; use Pterodactyl\Models\User; use Illuminate\Http\Response; -use Pterodactyl\Models\Server; use Pterodactyl\Models\Allocation; use Pterodactyl\Models\Permission; use Pterodactyl\Tests\Integration\Api\Client\ClientApiIntegrationTestCase; @@ -17,7 +16,6 @@ class NetworkAllocationControllerTest extends ClientApiIntegrationTestCase public function testServerAllocationsAreReturned() { [$user, $server] = $this->generateTestAccount(); - $allocation = $this->getAllocation($server); $response = $this->actingAs($user)->getJson($this->link($server, '/network/allocations')); @@ -25,7 +23,7 @@ class NetworkAllocationControllerTest extends ClientApiIntegrationTestCase $response->assertJsonPath('object', 'list'); $response->assertJsonCount(1, 'data'); - $this->assertJsonTransformedWith($response->json('data.0.attributes'), $allocation); + $this->assertJsonTransformedWith($response->json('data.0.attributes'), $server->allocation); } /** @@ -57,7 +55,7 @@ class NetworkAllocationControllerTest extends ClientApiIntegrationTestCase public function testAllocationNotesCanBeUpdated(array $permissions) { [$user, $server] = $this->generateTestAccount($permissions); - $allocation = $this->getAllocation($server); + $allocation = $server->allocation; $this->assertNull($allocation->notes); @@ -92,13 +90,11 @@ class NetworkAllocationControllerTest extends ClientApiIntegrationTestCase $server->owner_id = $user2->id; $server->save(); - $this->actingAs($user)->postJson($this->link($this->getAllocation($server))) - ->assertNotFound(); + $this->actingAs($user)->postJson($this->link($server->allocation))->assertNotFound(); [$user, $server] = $this->generateTestAccount([Permission::ACTION_ALLOCATION_CREATE]); - $this->actingAs($user)->postJson($this->link($this->getAllocation($server))) - ->assertForbidden(); + $this->actingAs($user)->postJson($this->link($server->allocation))->assertForbidden(); } /** @@ -108,8 +104,8 @@ class NetworkAllocationControllerTest extends ClientApiIntegrationTestCase public function testPrimaryAllocationCanBeModified(array $permissions) { [$user, $server] = $this->generateTestAccount($permissions); - $allocation = $this->getAllocation($server); - $allocation2 = $this->getAllocation($server); + $allocation = $server->allocation; + $allocation2 = factory(Allocation::class)->create(['node_id' => $server->node_id, 'server_id' => $server->id]); $server->allocation_id = $allocation->id; $server->save(); @@ -130,61 +126,12 @@ class NetworkAllocationControllerTest extends ClientApiIntegrationTestCase $server->owner_id = $user2->id; $server->save(); - $this->actingAs($user)->postJson($this->link($this->getAllocation($server), '/primary')) + $this->actingAs($user)->postJson($this->link($server->allocation, '/primary')) ->assertNotFound(); [$user, $server] = $this->generateTestAccount([Permission::ACTION_ALLOCATION_CREATE]); - $this->actingAs($user)->postJson($this->link($this->getAllocation($server), '/primary')) - ->assertForbidden(); - } - - /** - * @param array $permissions - * @dataProvider deletePermissionsDataProvider - */ - public function testAllocationCanBeDeleted(array $permissions) - { - [$user, $server] = $this->generateTestAccount($permissions); - $allocation = $this->getAllocation($server); - $allocation2 = $this->getAllocation($server); - - $allocation2->notes = 'Filled notes'; - $allocation2->save(); - - $server->allocation_id = $allocation->id; - $server->save(); - - $this->actingAs($user)->deleteJson($this->link($allocation)) - ->assertStatus(Response::HTTP_BAD_REQUEST) - ->assertJsonPath('errors.0.code', 'DisplayException') - ->assertJsonPath('errors.0.detail', 'Cannot delete the primary allocation for a server.'); - - $this->actingAs($user)->deleteJson($this->link($allocation2)) - ->assertStatus(Response::HTTP_NO_CONTENT); - - $server = $server->refresh(); - $allocation2 = $allocation2->refresh(); - - $this->assertSame($allocation->id, $server->allocation_id); - $this->assertNull($allocation2->server_id); - $this->assertNull($allocation2->notes); - } - - public function testAllocationCannotBeDeletedByInvalidUser() - { - [$user, $server] = $this->generateTestAccount(); - $user2 = factory(User::class)->create(); - - $server->owner_id = $user2->id; - $server->save(); - - $this->actingAs($user)->deleteJson($this->link($this->getAllocation($server))) - ->assertNotFound(); - - [$user, $server] = $this->generateTestAccount([Permission::ACTION_ALLOCATION_CREATE]); - - $this->actingAs($user)->deleteJson($this->link($this->getAllocation($server))) + $this->actingAs($user)->postJson($this->link($server->allocation, '/primary')) ->assertForbidden(); } @@ -197,16 +144,4 @@ class NetworkAllocationControllerTest extends ClientApiIntegrationTestCase { return [[[]], [[Permission::ACTION_ALLOCATION_DELETE]]]; } - - /** - * @param \Pterodactyl\Models\Server $server - * @return \Pterodactyl\Models\Allocation - */ - protected function getAllocation(Server $server): Allocation - { - return factory(Allocation::class)->create([ - 'server_id' => $server->id, - 'node_id' => $server->node_id, - ]); - } } diff --git a/tests/Traits/Integration/CreatesTestModels.php b/tests/Traits/Integration/CreatesTestModels.php index ecd4e0d53..0d3390f63 100644 --- a/tests/Traits/Integration/CreatesTestModels.php +++ b/tests/Traits/Integration/CreatesTestModels.php @@ -71,6 +71,8 @@ trait CreatesTestModels $server = $factory->of(Server::class)->create($attributes); + Allocation::query()->where('id', $server->allocation_id)->update(['server_id' => $server->id]); + return Server::with([ 'location', 'user', 'node', 'allocation', 'nest', 'egg', ])->findOrFail($server->id);