Add test coverage to cehck the authorization state of client resources

database/factories/ModelFactory.php

@@ -7,6 +7,8 @@ use Illuminate\Support\Str;
 use Pterodactyl\Models\Node;
 use Faker\Generator as Faker;
 use Pterodactyl\Models\ApiKey;
+use Pterodactyl\Models\Backup;
+use Pterodactyl\Models\Permission;
 
 /** @var \Illuminate\Database\Eloquent\Factory $factory */
 /*
@@ -134,7 +136,9 @@ $factory->state(Pterodactyl\Models\EggVariable::class, 'editable', function () {
 });
 
 $factory->define(Pterodactyl\Models\Subuser::class, function (Faker $faker) {
-    return [];
+    return [
+        'permissions' => [Permission::ACTION_WEBSOCKET_CONNECT],
+    ];
 });
 
 $factory->define(Pterodactyl\Models\Allocation::class, function (Faker $faker) {
@@ -161,7 +165,7 @@ $factory->define(Pterodactyl\Models\Database::class, function (Faker $faker) {
         'database' => str_random(10),
         'username' => str_random(10),
         'remote' => '%',
-        'password' => $password ?: bcrypt('test123'),
+        'password' => $password ?: encrypt('test123'),
         'created_at' => Carbon::now()->toDateTimeString(),
         'updated_at' => Carbon::now()->toDateTimeString(),
     ];
@@ -196,3 +200,12 @@ $factory->define(Pterodactyl\Models\ApiKey::class, function (Faker $faker) {
         'updated_at' => Carbon::now()->toDateTimeString(),
     ];
 });
+
+$factory->define(Pterodactyl\Models\Backup::class, function (Faker $faker) {
+    return [
+        'uuid' => Uuid::uuid4()->toString(),
+        'is_successful' => true,
+        'name' => $faker->sentence,
+        'disk' => Backup::ADAPTER_WINGS,
+    ];
+});

tests/Integration/Api/Client/ClientApiIntegrationTestCase.php

@@ -10,11 +10,14 @@ use Pterodactyl\Models\Task;
 use Pterodactyl\Models\User;
 use Webmozart\Assert\Assert;
 use Pterodactyl\Models\Server;
+use Pterodactyl\Models\Backup;
 use Pterodactyl\Models\Subuser;
 use Pterodactyl\Models\Location;
 use Pterodactyl\Models\Schedule;
+use Pterodactyl\Models\Database;
 use Illuminate\Support\Collection;
 use Pterodactyl\Models\Allocation;
+use Pterodactyl\Models\DatabaseHost;
 use Pterodactyl\Tests\Integration\TestResponse;
 use Pterodactyl\Tests\Integration\IntegrationTestCase;
 use Pterodactyl\Transformers\Api\Client\BaseClientTransformer;
@@ -26,6 +29,9 @@ abstract class ClientApiIntegrationTestCase extends IntegrationTestCase
      */
     protected function tearDown(): void
     {
+        Database::query()->forceDelete();
+        DatabaseHost::query()->forceDelete();
+        Backup::query()->forceDelete();
         Server::query()->forceDelete();
         Node::query()->forceDelete();
         Location::query()->forceDelete();

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

@@ -0,0 +1,63 @@
+<?php
+
+namespace Pterodactyl\Tests\Integration\Api\Client\Server\Allocation;
+
+use Pterodactyl\Models\Subuser;
+use Pterodactyl\Models\Schedule;
+use Pterodactyl\Models\Allocation;
+use Pterodactyl\Tests\Integration\Api\Client\ClientApiIntegrationTestCase;
+
+class AllocationAuthorizationTest extends ClientApiIntegrationTestCase
+{
+    /**
+     * @param string $method
+     * @param string $endpoint
+     * @dataProvider methodDataProvider
+     */
+    public function testAccessToAServersAllocationsIsRestrictedProperly(string $method, string $endpoint)
+    {
+        // The API $user is the owner of $server1.
+        [$user, $server1] = $this->generateTestAccount();
+        // Will be a subuser of $server2.
+        $server2 = $this->createServerModel();
+        // And as no access to $server3.
+        $server3 = $this->createServerModel();
+
+        // Set the API $user as a subuser of server 2, but with no permissions
+        // to do anything with the allocations for that server.
+        factory(Subuser::class)->create(['server_id' => $server2->id, 'user_id' => $user->id]);
+
+        $allocation1 = factory(Allocation::class)->create(['server_id' => $server1->id, 'node_id' => $server1->node_id]);
+        $allocation2 = factory(Allocation::class)->create(['server_id' => $server2->id, 'node_id' => $server2->node_id]);
+        $allocation3 = factory(Allocation::class)->create(['server_id' => $server3->id, 'node_id' => $server3->node_id]);
+
+        // This is the only valid call for this test, accessing the allocation for the same
+        // server that the API user is the owner of.
+        $response = $this->actingAs($user)->json($method, $this->link($server1, "/network/allocations/" . $allocation1->id . $endpoint));
+        $this->assertTrue($response->status() <= 204 || $response->status() === 400 || $response->status() === 422);
+
+        // This request fails because the allocation is valid for that server but the user
+        // making the request is not authorized to perform that action.
+        $this->actingAs($user)->json($method, $this->link($server2, "/network/allocations/" . $allocation2->id . $endpoint))->assertForbidden();
+
+        // Both of these should report a 404 error due to the allocations being linked to
+        // servers that are not the same as the server in the request, or are assigned
+        // to a server for which the user making the request has no access to.
+        $this->actingAs($user)->json($method, $this->link($server1, "/network/allocations/" . $allocation2->id . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server1, "/network/allocations/" . $allocation3->id . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server2, "/network/allocations/" . $allocation3->id . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server3, "/network/allocations/" . $allocation3->id . $endpoint))->assertNotFound();
+    }
+
+    /**
+     * @return \string[][]
+     */
+    public function methodDataProvider(): array
+    {
+        return [
+            ["POST", ""],
+            ["DELETE", ""],
+            ["POST", "/primary"],
+        ];
+    }
+}

tests/Integration/Api/Client/Server/Backup/BackupAuthorizationTest.php

@@ -0,0 +1,71 @@
+<?php
+
+namespace Pterodactyl\Tests\Integration\Api\Client\Server\Backup;
+
+use Mockery;
+use Carbon\CarbonImmutable;
+use Pterodactyl\Models\Backup;
+use Pterodactyl\Models\Subuser;
+use Pterodactyl\Services\Backups\DeleteBackupService;
+use Pterodactyl\Tests\Integration\Api\Client\ClientApiIntegrationTestCase;
+
+class BackupAuthorizationTest extends ClientApiIntegrationTestCase
+{
+    /**
+     * @param string $method
+     * @param string $endpoint
+     * @dataProvider methodDataProvider
+     */
+    public function testAccessToAServersBackupIsRestrictedProperly(string $method, string $endpoint)
+    {
+        // The API $user is the owner of $server1.
+        [$user, $server1] = $this->generateTestAccount();
+        // Will be a subuser of $server2.
+        $server2 = $this->createServerModel();
+        // And as no access to $server3.
+        $server3 = $this->createServerModel();
+
+        // Set the API $user as a subuser of server 2, but with no permissions
+        // to do anything with the backups for that server.
+        factory(Subuser::class)->create(['server_id' => $server2->id, 'user_id' => $user->id]);
+
+        $backup1 = factory(Backup::class)->create(['server_id' => $server1->id, 'completed_at' => CarbonImmutable::now()]);
+        $backup2 = factory(Backup::class)->create(['server_id' => $server2->id, 'completed_at' => CarbonImmutable::now()]);
+        $backup3 = factory(Backup::class)->create(['server_id' => $server3->id, 'completed_at' => CarbonImmutable::now()]);
+
+        $this->instance(DeleteBackupService::class, $mock = Mockery::mock(DeleteBackupService::class));
+
+        if ($method === 'DELETE') {
+            $mock->expects('handle')->andReturnUndefined();
+        }
+
+        // This is the only valid call for this test, accessing the backup for the same
+        // server that the API user is the owner of.
+        $this->actingAs($user)->json($method, $this->link($server1, "/backups/" . $backup1->uuid . $endpoint))
+            ->assertStatus($method === 'DELETE' ? 204 : 200);
+
+        // This request fails because the backup is valid for that server but the user
+        // making the request is not authorized to perform that action.
+        $this->actingAs($user)->json($method, $this->link($server2, "/backups/" . $backup2->uuid . $endpoint))->assertForbidden();
+
+        // Both of these should report a 404 error due to the backup being linked to
+        // servers that are not the same as the server in the request, or are assigned
+        // to a server for which the user making the request has no access to.
+        $this->actingAs($user)->json($method, $this->link($server1, "/backups/" . $backup2->uuid . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server1, "/backups/" . $backup3->uuid . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server2, "/backups/" . $backup3->uuid . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server3, "/backups/" . $backup3->uuid . $endpoint))->assertNotFound();
+    }
+
+    /**
+     * @return \string[][]
+     */
+    public function methodDataProvider(): array
+    {
+        return [
+            ["GET", ""],
+            ["GET", "/download"],
+            ["DELETE", ""],
+        ];
+    }
+}

tests/Integration/Api/Client/Server/Database/DatabaseAuthorizationTest.php

@@ -0,0 +1,78 @@
+<?php
+
+namespace Pterodactyl\Tests\Integration\Api\Client\Server\Database;
+
+use Mockery;
+use Pterodactyl\Models\Subuser;
+use Pterodactyl\Models\Database;
+use Pterodactyl\Models\DatabaseHost;
+use Pterodactyl\Contracts\Extensions\HashidsInterface;
+use Pterodactyl\Services\Databases\DatabasePasswordService;
+use Pterodactyl\Services\Databases\DatabaseManagementService;
+use Pterodactyl\Tests\Integration\Api\Client\ClientApiIntegrationTestCase;
+
+class DatabaseAuthorizationTest extends ClientApiIntegrationTestCase
+{
+    /**
+     * @param string $method
+     * @param string $endpoint
+     * @dataProvider methodDataProvider
+     */
+    public function testAccessToAServersDatabasesIsRestrictedProperly(string $method, string $endpoint)
+    {
+        // The API $user is the owner of $server1.
+        [$user, $server1] = $this->generateTestAccount();
+        // Will be a subuser of $server2.
+        $server2 = $this->createServerModel();
+        // And as no access to $server3.
+        $server3 = $this->createServerModel();
+
+        $host = factory(DatabaseHost::class)->create([]);
+
+        // Set the API $user as a subuser of server 2, but with no permissions
+        // to do anything with the databases for that server.
+        factory(Subuser::class)->create(['server_id' => $server2->id, 'user_id' => $user->id]);
+
+        $database1 = factory(Database::class)->create(['server_id' => $server1->id, 'database_host_id' => $host->id]);
+        $database2 = factory(Database::class)->create(['server_id' => $server2->id, 'database_host_id' => $host->id]);
+        $database3 = factory(Database::class)->create(['server_id' => $server3->id, 'database_host_id' => $host->id]);
+
+        $this->instance(DatabasePasswordService::class, $mock = Mockery::mock(DatabasePasswordService::class));
+        $this->instance(DatabaseManagementService::class, $mock2 = Mockery::mock(DatabaseManagementService::class));
+
+        if ($method === 'POST') {
+            $mock->expects('handle')->andReturnUndefined();
+        } else {
+            $mock2->expects('delete')->andReturnUndefined();
+        }
+
+        $hashids = $this->app->make(HashidsInterface::class);
+        // This is the only valid call for this test, accessing the database for the same
+        // server that the API user is the owner of.
+        $this->actingAs($user)->json($method, $this->link($server1, "/databases/" . $hashids->encode($database1->id) . $endpoint))
+            ->assertStatus($method === 'DELETE' ? 204 : 200);
+
+        // This request fails because the database is valid for that server but the user
+        // making the request is not authorized to perform that action.
+        $this->actingAs($user)->json($method, $this->link($server2, "/databases/" . $hashids->encode($database2->id) . $endpoint))->assertForbidden();
+
+        // Both of these should report a 404 error due to the database being linked to
+        // servers that are not the same as the server in the request, or are assigned
+        // to a server for which the user making the request has no access to.
+        $this->actingAs($user)->json($method, $this->link($server1, "/databases/" . $hashids->encode($database2->id) . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server1, "/databases/" . $hashids->encode($database3->id) . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server2, "/databases/" . $hashids->encode($database3->id) . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server3, "/databases/" . $hashids->encode($database3->id) . $endpoint))->assertNotFound();
+    }
+
+    /**
+     * @return \string[][]
+     */
+    public function methodDataProvider(): array
+    {
+        return [
+            ["POST", "/rotate-password"],
+            ["DELETE", ""],
+        ];
+    }
+}

tests/Integration/Api/Client/Server/Schedule/ScheduleAuthorizationTest.php

@@ -0,0 +1,72 @@
+<?php
+
+namespace Pterodactyl\Tests\Integration\Api\Client\Server\Schedule;
+
+use Pterodactyl\Models\Subuser;
+use Pterodactyl\Models\Schedule;
+use Pterodactyl\Tests\Integration\Api\Client\ClientApiIntegrationTestCase;
+
+class ScheduleAuthorizationTest extends ClientApiIntegrationTestCase
+{
+    /**
+     * Tests that a subuser with access to two servers cannot improperly access a resource
+     * on Server A when providing a URL that points to Server B. This prevents a regression
+     * in the code where controllers didn't properly validate that a resource was assigned
+     * to the server that was also present in the URL.
+     *
+     * The comments within the test code itself are better at explaining exactly what is
+     * being tested and protected against.
+     *
+     * @param string $method
+     * @param string $endpoint
+     * @dataProvider methodDataProvider
+     */
+    public function testAccessToAServersSchedulesIsRestrictedProperly(string $method, string $endpoint)
+    {
+        // The API $user is the owner of $server1.
+        [$user, $server1] = $this->generateTestAccount();
+        // Will be a subuser of $server2.
+        $server2 = $this->createServerModel();
+        // And as no access to $server3.
+        $server3 = $this->createServerModel();
+
+        // Set the API $user as a subuser of server 2, but with no permissions
+        // to do anything with the schedules for that server.
+        factory(Subuser::class)->create(['server_id' => $server2->id, 'user_id' => $user->id]);
+
+        $schedule1 = factory(Schedule::class)->create(['server_id' => $server1->id]);
+        $schedule2 = factory(Schedule::class)->create(['server_id' => $server2->id]);
+        $schedule3 = factory(Schedule::class)->create(['server_id' => $server3->id]);
+
+        // This is the only valid call for this test, accessing the schedule for the same
+        // server that the API user is the owner of.
+        $response = $this->actingAs($user)->json($method, $this->link($server1, "/schedules/" . $schedule1->id . $endpoint));
+        $this->assertTrue($response->status() <= 204 || $response->status() === 400 || $response->status() === 422);
+
+        // This request fails because the schedule is valid for that server but the user
+        // making the request is not authorized to perform that action.
+        $this->actingAs($user)->json($method, $this->link($server2, "/schedules/" . $schedule2->id . $endpoint))->assertForbidden();
+
+        // Both of these should report a 404 error due to the schedules being linked to
+        // servers that are not the same as the server in the request, or are assigned
+        // to a server for which the user making the request has no access to.
+        $this->actingAs($user)->json($method, $this->link($server1, "/schedules/" . $schedule2->id . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server1, "/schedules/" . $schedule3->id . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server2, "/schedules/" . $schedule3->id . $endpoint))->assertNotFound();
+        $this->actingAs($user)->json($method, $this->link($server3, "/schedules/" . $schedule3->id . $endpoint))->assertNotFound();
+    }
+
+    /**
+     * @return \string[][]
+     */
+    public function methodDataProvider(): array
+    {
+        return [
+            ["GET", ""],
+            ["POST", ""],
+            ["DELETE", ""],
+            ["POST", "/execute"],
+            ["POST", "/tasks"],
+        ];
+    }
+}

tests/Integration/Api/Client/Server/Subuser/SubuserAuthorizationTest.php

@@ -0,0 +1,61 @@
+<?php
+
+namespace Pterodactyl\Tests\Integration\Api\Client\Server\Subuser;
+
+use Mockery;
+use Pterodactyl\Models\User;
+use Pterodactyl\Models\Subuser;
+use Pterodactyl\Repositories\Wings\DaemonServerRepository;
+use Pterodactyl\Tests\Integration\Api\Client\ClientApiIntegrationTestCase;
+
+class SubuserAuthorizationTest extends ClientApiIntegrationTestCase
+{
+    /**
+     * Test that mismatched subusers are not accessible to a server.
+     *
+     * @param string $method
+     * @dataProvider methodDataProvider
+     */
+    public function testUserCannotAccessResourceBelongingToOtherServers(string $method)
+    {
+        // Generic subuser, the specific resource we're trying to access.
+        /** @var \Pterodactyl\Models\User $internal */
+        $internal = factory(User::class)->create();
+
+        // The API $user is the owner of $server1.
+        [$user, $server1] = $this->generateTestAccount();
+        // Will be a subuser of $server2.
+        $server2 = $this->createServerModel();
+        // And as no access to $server3.
+        $server3 = $this->createServerModel();
+
+        // Set the API $user as a subuser of server 2, but with no permissions
+        // to do anything with the subusers for that server.
+        factory(Subuser::class)->create(['server_id' => $server2->id, 'user_id' => $user->id]);
+
+        factory(Subuser::class)->create(['server_id' => $server1->id, 'user_id' => $internal->id]);
+        factory(Subuser::class)->create(['server_id' => $server2->id, 'user_id' => $internal->id]);
+        factory(Subuser::class)->create(['server_id' => $server3->id, 'user_id' => $internal->id]);
+
+        $this->instance(DaemonServerRepository::class, $mock = Mockery::mock(DaemonServerRepository::class));
+        if ($method === 'DELETE') {
+            $mock->expects('setServer->revokeUserJTI')->with($internal->id)->andReturnUndefined();
+        }
+
+        // This route is acceptable since they're accessing a subuser on their own server.
+        $this->actingAs($user)->json($method, $this->link($server1, "/users/" . $internal->uuid))->assertStatus($method === 'POST' ? 422 : ($method === 'DELETE' ? 204 : 200));
+
+        // This route can be revealed since the subuser belongs to the correct server, but
+        // errors out with a 403 since $user does not have the right permissions for this.
+        $this->actingAs($user)->json($method, $this->link($server2, "/users/" . $internal->uuid))->assertForbidden();
+        $this->actingAs($user)->json($method, $this->link($server3, "/users/" . $internal->uuid))->assertNotFound();
+    }
+
+    /**
+     * @return \string[][]
+     */
+    public function methodDataProvider(): array
+    {
+        return [["GET"], ["POST"], ["DELETE"]];
+    }
+}

tests/Integration/TestResponse.php

@@ -3,6 +3,8 @@
 namespace Pterodactyl\Tests\Integration;
 
 use Illuminate\Testing\Assert as PHPUnit;
+use Pterodactyl\Exceptions\DisplayException;
+use Illuminate\Validation\ValidationException;
 use Illuminate\Testing\TestResponse as IlluminateTestResponse;
 
 class TestResponse extends IlluminateTestResponse
@@ -23,7 +25,7 @@ class TestResponse extends IlluminateTestResponse
         // to fail so that debugging isn't such a nightmare.
         if ($actual !== $status && $status !== 500) {
             $this->dump();
-            if (! is_null($this->exception)) {
+            if (! is_null($this->exception) && ! $this->exception instanceof DisplayException && ! $this->exception instanceof ValidationException) {
                 dump($this->exception);
             }
         }