Merge branch 'develop' into laravel-8

CHANGELOG.md

@@ -3,6 +3,9 @@ This file is a running track of new features and fixes to each version of the pa
 
 This project follows [Semantic Versioning](http://semver.org) guidelines.
 
+## v1.2.2
+* **[security]** Fixes authentication bypass allowing a user to take control of specific server actions such as executing schedules, rotating database passwords, and viewing or deleting a backup.
+
 ## v1.2.1
 ### Fixed
 * Fixes URL-encoding of filenames when working in the filemanager to fix issues when moving, renaming, or deleting files.

app/Http/Middleware/Api/Client/Server/AllocationBelongsToServer.php

@@ -1,33 +0,0 @@
-<?php
-
-namespace Pterodactyl\Http\Middleware\Api\Client\Server;
-
-use Closure;
-use Illuminate\Http\Request;
-use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
-
-class AllocationBelongsToServer
-{
-    /**
-     * Ensure that the allocation found in the URL belongs to the server being queried.
-     *
-     * @param \Illuminate\Http\Request $request
-     * @param \Closure $next
-     * @return mixed
-     *
-     * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
-     */
-    public function handle(Request $request, Closure $next)
-    {
-        /** @var \Pterodactyl\Models\Server $server */
-        $server = $request->route()->parameter('server');
-        /** @var \Pterodactyl\Models\Allocation|null $allocation */
-        $allocation = $request->route()->parameter('allocation');
-
-        if ($allocation && $allocation->server_id !== $server->id) {
-            throw new NotFoundHttpException;
-        }
-
-        return $next($request);
-    }
-}

app/Http/Middleware/Api/Client/Server/ResourceBelongsToServer.php

@@ -0,0 +1,92 @@
+<?php
+
+namespace Pterodactyl\Http\Middleware\Api\Client\Server;
+
+use Closure;
+use Illuminate\Http\Request;
+use Pterodactyl\Models\Task;
+use Pterodactyl\Models\User;
+use InvalidArgumentException;
+use Pterodactyl\Models\Server;
+use Pterodactyl\Models\Backup;
+use Pterodactyl\Models\Subuser;
+use Pterodactyl\Models\Schedule;
+use Pterodactyl\Models\Database;
+use Pterodactyl\Models\Allocation;
+use Illuminate\Database\Eloquent\Model;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+
+class ResourceBelongsToServer
+{
+    /**
+     * Looks at the request parameters to determine if the given resource belongs
+     * to the requested server. If not, a 404 error will be returned to the caller.
+     *
+     * This is critical to ensuring that all subsequent logic is using exactly the
+     * server that is expected, and that we're not accessing a resource completely
+     * unrelated to the server provided in the request.
+     *
+     * @param \Illuminate\Http\Request $request
+     * @param \Closure $next
+     * @return mixed
+     */
+    public function handle(Request $request, Closure $next)
+    {
+        $params = $request->route()->parameters();
+        if (is_null($params) || ! $params['server'] instanceof Server) {
+            throw new InvalidArgumentException('This middleware cannot be used in a context that is missing a server in the parameters.');
+        }
+
+        /** @var \Pterodactyl\Models\Server $server */
+        $server = $request->route()->parameter('server');
+        $exception = new NotFoundHttpException('The requested resource was not found for this server.');
+        foreach ($params as $key => $model) {
+            // Specifically skip the server, we're just trying to see if all of the
+            // other resources are assigned to this server. Also skip anything that
+            // is not currently a Model instance since those will just end up being
+            // a 404 down the road.
+            if ($key === 'server' || ! $model instanceof Model) {
+                continue;
+            }
+
+            switch (get_class($model)) {
+                // All of these models use "server_id" as the field key for the server
+                // they are assigned to, so the logic is identical for them all.
+                case Allocation::class:
+                case Backup::class:
+                case Database::class:
+                case Schedule::class:
+                case Subuser::class:
+                    if ($model->server_id !== $server->id) {
+                        throw $exception;
+                    }
+                    break;
+                // Regular users are a special case here as we need to make sure they're
+                // currently assigned as a subuser on the server.
+                case User::class:
+                    $subuser = $server->subusers()->where('user_id', $model->id)->first();
+                    if (is_null($subuser)) {
+                        throw $exception;
+                    }
+                    // This is a special case to avoid an additional query being triggered
+                    // in the underlying logic.
+                    $request->attributes->set('subuser', $subuser);
+                    break;
+                // Tasks are special since they're (currently) the only item in the API
+                // that requires something in addition to the server in order to be accessed.
+                case Task::class:
+                    $schedule = $request->route()->parameter('schedule');
+                    if ($model->schedule_id !== $schedule->id || $schedule->server_id !== $server->id) {
+                        throw $exception;
+                    }
+                    break;
+                default:
+                    // Don't return a 404 here since we want to make sure no one relies
+                    // on this middleware in a context in which it will not work. Fail safe.
+                    throw new InvalidArgumentException('There is no handler configured for a resource of this type: ' . get_class($model));
+            }
+        }
+
+        return $next($request);
+    }
+}

app/Http/Middleware/Api/Client/Server/SubuserBelongsToServer.php

@@ -1,36 +0,0 @@
-<?php
-
-namespace Pterodactyl\Http\Middleware\Api\Client\Server;
-
-use Closure;
-use Illuminate\Http\Request;
-
-class SubuserBelongsToServer
-{
-    /**
-     * Ensure that the user being accessed in the request is a user that is currently assigned
-     * as a subuser for this server instance. We'll let the requests themselves handle wether or
-     * not the user making the request can actually modify or delete the subuser record.
-     *
-     * @param \Illuminate\Http\Request $request
-     * @param \Closure $next
-     *
-     * @return mixed
-     */
-    public function handle(Request $request, Closure $next)
-    {
-        /** @var \Pterodactyl\Models\Server $server */
-        $server = $request->route()->parameter('server');
-        /** @var \Pterodactyl\Models\User $user */
-        $user = $request->route()->parameter('user');
-
-        // Don't do anything if there isn't a user present in the request.
-        if (is_null($user)) {
-            return $next($request);
-        }
-
-        $request->attributes->set('subuser', $server->subusers()->where('user_id', $user->id)->firstOrFail());
-
-        return $next($request);
-    }
-}

database/factories/BackupFactory.php

@@ -0,0 +1,32 @@
+<?php
+
+namespace Database\Factories;
+
+use Ramsey\Uuid\Uuid;
+use Pterodactyl\Models\Backup;
+use Illuminate\Database\Eloquent\Factories\Factory;
+
+class BackupFactory extends Factory
+{
+    /**
+     * The name of the factory's corresponding model.
+     *
+     * @var string
+     */
+    protected $model = Backup::class;
+
+    /**
+     * Define the model's default state.
+     *
+     * @return array
+     */
+    public function definition(): array
+    {
+        return [
+            'uuid' => Uuid::uuid4()->toString(),
+            'is_successful' => true,
+            'name' => $this->faker->sentence,
+            'disk' => Backup::ADAPTER_WINGS,
+        ];
+    }
+}

database/factories/DatabaseFactory.php

@@ -29,7 +29,7 @@ class DatabaseFactory extends Factory
             'database' => Str::random(10),
             'username' => Str::random(10),
             'remote' => '%',
-            'password' => $password ?: bcrypt('test123'),
+            'password' => $password ?: encrypt('test123'),
             'created_at' => Carbon::now(),
             'updated_at' => Carbon::now(),
         ];

database/factories/SubuserFactory.php

@@ -3,6 +3,7 @@
 namespace Database\Factories;
 
 use Pterodactyl\Models\Subuser;
+use Pterodactyl\Models\Permission;
 use Illuminate\Database\Eloquent\Factories\Factory;
 
 class SubuserFactory extends Factory
@@ -21,6 +22,10 @@ class SubuserFactory extends Factory
      */
     public function definition(): array
     {
-        return [];
+        return [
+            'permissions' => [
+                Permission::ACTION_WEBSOCKET_CONNECT
+            ],
+        ];
     }
 }

routes/api-client.php

@@ -3,6 +3,7 @@
 use Illuminate\Support\Facades\Route;
 use Pterodactyl\Http\Middleware\RequireTwoFactorAuthentication;
 use Pterodactyl\Http\Middleware\Api\Client\Server\SubuserBelongsToServer;
+use Pterodactyl\Http\Middleware\Api\Client\Server\ResourceBelongsToServer;
 use Pterodactyl\Http\Middleware\Api\Client\Server\AuthenticateServerAccess;
 use Pterodactyl\Http\Middleware\Api\Client\Server\AllocationBelongsToServer;
 
@@ -39,7 +40,7 @@ Route::group(['prefix' => '/account'], function () {
 | Endpoint: /api/client/servers/{server}
 |
 */
-Route::group(['prefix' => '/servers/{server}', 'middleware' => [AuthenticateServerAccess::class]], function () {
+Route::group(['prefix' => '/servers/{server}', 'middleware' => [AuthenticateServerAccess::class, ResourceBelongsToServer::class]], function () {
     Route::get('/', 'Servers\ServerController@index')->name('api:client:server.view');
     Route::get('/websocket', 'Servers\WebsocketController')->name('api:client:server.ws');
     Route::get('/resources', 'Servers\ResourceUtilizationController')->name('api:client:server.resources');
@@ -83,7 +84,7 @@ Route::group(['prefix' => '/servers/{server}', 'middleware' => [AuthenticateServ
         Route::delete('/{schedule}/tasks/{task}', 'Servers\ScheduleTaskController@delete');
     });
 
-    Route::group(['prefix' => '/network', 'middleware' => [AllocationBelongsToServer::class]], function () {
+    Route::group(['prefix' => '/network'], function () {
         Route::get('/allocations', 'Servers\NetworkAllocationController@index');
         Route::post('/allocations', 'Servers\NetworkAllocationController@store');
         Route::post('/allocations/{allocation}', 'Servers\NetworkAllocationController@update');
@@ -91,7 +92,7 @@ Route::group(['prefix' => '/servers/{server}', 'middleware' => [AuthenticateServ
         Route::delete('/allocations/{allocation}', 'Servers\NetworkAllocationController@delete');
     });
 
-    Route::group(['prefix' => '/users', 'middleware' => [SubuserBelongsToServer::class]], function () {
+    Route::group(['prefix' => '/users'], function () {
         Route::get('/', 'Servers\SubuserController@index');
         Route::post('/', 'Servers\SubuserController@store');
         Route::get('/{user}', 'Servers\SubuserController@view');

tests/Integration/Api/Client/ClientApiIntegrationTestCase.php

@@ -10,11 +10,15 @@ 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;
 
@@ -25,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();
@@ -44,6 +51,19 @@ abstract class ClientApiIntegrationTestCase extends IntegrationTestCase
         CarbonImmutable::setTestNow(Carbon::now());
     }
 
+    /**
+     * Override the default createTestResponse from Illuminate so that we can
+     * just dump 500-level errors to the screen in the tests without having
+     * to keep re-assigning variables.
+     *
+     * @param \Illuminate\Http\Response $response
+     * @return \Illuminate\Testing\TestResponse
+     */
+    protected function createTestResponse($response)
+    {
+        return TestResponse::fromBaseResponse($response);
+    }
+
     /**
      * Returns a link to the specific resource using the client API.
      *

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

@@ -0,0 +1,37 @@
+<?php
+
+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
+{
+    /**
+     * Overrides the default assert status logic to dump out the error to the
+     * test output if it is caused by a 500 level error and we were not specifically
+     * look for that status response.
+     *
+     * @param int $status
+     * @return \Pterodactyl\Tests\Integration\TestResponse
+     */
+    public function assertStatus($status)
+    {
+        $actual = $this->getStatusCode();
+
+        // Dump the response to the screen before making the assertion which is going
+        // to fail so that debugging isn't such a nightmare.
+        if ($actual !== $status && $status !== 500) {
+            $this->dump();
+            if (! is_null($this->exception) && ! $this->exception instanceof DisplayException && ! $this->exception instanceof ValidationException) {
+                dump($this->exception);
+            }
+        }
+
+        PHPUnit::assertSame($actual, $status, "Expected status code {$status} but received {$actual}.");
+
+        return $this;
+    }
+}