Don't allow allocations to be deleted by users if no limit is defined; closes #3703

app/Http/Controllers/Api/Client/Servers/NetworkAllocationController.php

@@ -120,6 +120,12 @@ class NetworkAllocationController extends ClientApiController
      */
     public function delete(DeleteAllocationRequest $request, Server $server, Allocation $allocation)
     {
+        // Don't allow the deletion of allocations if the server does not have an
+        // allocation limit set.
+        if (empty($server->allocation_limit)) {
+            throw new DisplayException('You cannot delete allocations for this server: no allocation limit is set.');
+        }
+
         if ($allocation->id === $server->allocation_id) {
             throw new DisplayException('You cannot delete the primary allocation for this server.');
         }

app/Models/Allocation.php

@@ -3,6 +3,8 @@
 namespace Pterodactyl\Models;
 
 /**
+ * Pterodactyl\Models\Allocation.
+ *
  * @property int $id
  * @property int $node_id
  * @property string $ip
@@ -16,6 +18,22 @@ namespace Pterodactyl\Models;
  * @property bool $has_alias
  * @property \Pterodactyl\Models\Server|null $server
  * @property \Pterodactyl\Models\Node $node
+ * @property string $hashid
+ *
+ * @method static \Database\Factories\AllocationFactory factory(...$parameters)
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation newModelQuery()
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation newQuery()
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation query()
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation whereCreatedAt($value)
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation whereId($value)
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation whereIp($value)
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation whereIpAlias($value)
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation whereNodeId($value)
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation whereNotes($value)
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation wherePort($value)
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation whereServerId($value)
+ * @method static \Illuminate\Database\Eloquent\Builder|Allocation whereUpdatedAt($value)
+ * @mixin \Eloquent
  */
 class Allocation extends Model
 {

database/Factories/AllocationFactory.php

@@ -2,6 +2,7 @@
 
 namespace Database\Factories;
 
+use Pterodactyl\Models\Server;
 use Pterodactyl\Models\Allocation;
 use Illuminate\Database\Eloquent\Factories\Factory;
 
@@ -24,4 +25,12 @@ class AllocationFactory extends Factory
             'port' => $this->faker->unique()->randomNumber(5),
         ];
     }
+
+    /**
+     * Attaches the allocation to a specific server model.
+     */
+    public function forServer(Server $server): self
+    {
+        return $this->for($server)->for($server->node);
+    }
 }

resources/scripts/components/server/network/NetworkContainer.tsx

@@ -66,20 +66,22 @@ const NetworkContainer = () => {
                             />
                         ))
                     }
-                    <Can action={'allocation.create'}>
+                    {allocationLimit > 0 &&
-                        <SpinnerOverlay visible={loading}/>
+                        <Can action={'allocation.create'}>
-                        <div css={tw`mt-6 sm:flex items-center justify-end`}>
+                            <SpinnerOverlay visible={loading}/>
-                            <p css={tw`text-sm text-neutral-300 mb-4 sm:mr-6 sm:mb-0`}>
+                            <div css={tw`mt-6 sm:flex items-center justify-end`}>
-                                You are currently using {data.length} of {allocationLimit} allowed allocations for this
+                                <p css={tw`text-sm text-neutral-300 mb-4 sm:mr-6 sm:mb-0`}>
-                                server.
+                                    You are currently using {data.length} of {allocationLimit} allowed allocations for
-                            </p>
+                                    this server.
-                            {allocationLimit > data.length &&
+                                </p>
-                            <Button css={tw`w-full sm:w-auto`} color={'primary'} onClick={onCreateAllocation}>
+                                {allocationLimit > data.length &&
-                                Create Allocation
+                                    <Button css={tw`w-full sm:w-auto`} color={'primary'} onClick={onCreateAllocation}>
-                            </Button>
+                                        Create Allocation
-                            }
+                                    </Button>
-                        </div>
+                                }
-                    </Can>
+                            </div>
+                        </Can>
+                    }
                 </>
             }
         </ServerContentBlock>

tests/Integration/Api/Client/ClientApiIntegrationTestCase.php

@@ -89,6 +89,7 @@ abstract class ClientApiIntegrationTestCase extends IntegrationTestCase
      * is assumed that the user is actually a subuser of the server.
      *
      * @param string[] $permissions
+     * @return array{\Pterodactyl\Models\User, \Pterodactyl\Models\Server}
      */
     protected function generateTestAccount(array $permissions = []): array
     {

tests/Integration/Api/Client/Server/Allocation/DeleteAllocationTest.php

@@ -19,6 +19,7 @@ class DeleteAllocationTest extends ClientApiIntegrationTestCase
     {
         /** @var \Pterodactyl\Models\Server $server */
         [$user, $server] = $this->generateTestAccount($permission);
+        $server->update(['allocation_limit' => 2]);
 
         /** @var \Pterodactyl\Models\Allocation $allocation */
         $allocation = Allocation::factory()->create([
@@ -60,6 +61,7 @@ class DeleteAllocationTest extends ClientApiIntegrationTestCase
     {
         /** @var \Pterodactyl\Models\Server $server */
         [$user, $server] = $this->generateTestAccount();
+        $server->update(['allocation_limit' => 2]);
 
         $this->actingAs($user)->deleteJson($this->link($server->allocation))
             ->assertStatus(Response::HTTP_BAD_REQUEST)
@@ -67,6 +69,22 @@ class DeleteAllocationTest extends ClientApiIntegrationTestCase
             ->assertJsonPath('errors.0.detail', 'You cannot delete the primary allocation for this server.');
     }
 
+    public function testAllocationCannotBeDeletedIfServerLimitIsNotDefined()
+    {
+        [$user, $server] = $this->generateTestAccount();
+
+        /** @var \Pterodactyl\Models\Allocation $allocation */
+        $allocation = Allocation::factory()->forServer($server)->create(['notes' => 'Test notes']);
+
+        $this->actingAs($user)->deleteJson($this->link($allocation))
+            ->assertStatus(400)
+            ->assertJsonPath('errors.0.detail', 'You cannot delete allocations for this server: no allocation limit is set.');
+
+        $allocation->refresh();
+        $this->assertNotNull($allocation->notes);
+        $this->assertEquals($server->id, $allocation->server_id);
+    }
+
     /**
      * Test that an allocation cannot be deleted if it does not belong to the server instance.
      */

tests/Integration/Api/Client/Server/NetworkAllocationControllerTest.php

@@ -137,9 +137,4 @@ class NetworkAllocationControllerTest extends ClientApiIntegrationTestCase
     {
         return [[[]], [[Permission::ACTION_ALLOCATION_UPDATE]]];
     }
-
-    public function deletePermissionsDataProvider()
-    {
-        return [[[]], [[Permission::ACTION_ALLOCATION_DELETE]]];
-    }
 }