From f31a6d3967b89b7b37ef41d677efd0add73de606 Mon Sep 17 00:00:00 2001
From: Dane Everitt <dane@daneeveritt.com>
Date: Sun, 27 Sep 2020 10:39:18 -0700
Subject: [PATCH] Fix parameter bindings for client API routes; closes
 pterodactyl/panel#2359

---
 .../Client/SubstituteClientApiBindings.php    |  4 +-
 .../Subuser/CreateServerSubuserTest.php       |  2 +-
 .../Server/Subuser/DeleteSubuserTest.php      | 59 +++++++++++++++++++
 3 files changed, 62 insertions(+), 3 deletions(-)
 create mode 100644 tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php

diff --git a/app/Http/Middleware/Api/Client/SubstituteClientApiBindings.php b/app/Http/Middleware/Api/Client/SubstituteClientApiBindings.php
index 77879c97f..7ab597b63 100644
--- a/app/Http/Middleware/Api/Client/SubstituteClientApiBindings.php
+++ b/app/Http/Middleware/Api/Client/SubstituteClientApiBindings.php
@@ -49,11 +49,11 @@ class SubstituteClientApiBindings extends ApiSubstituteBindings
             return Database::query()->where('id', $id)->firstOrFail();
         });
 
-        $this->router->model('backup', Backup::class, function ($value) {
+        $this->router->bind('backup', function ($value) {
             return Backup::query()->where('uuid', $value)->firstOrFail();
         });
 
-        $this->router->model('user', User::class, function ($value) {
+        $this->router->bind('user', function ($value) {
             return User::query()->where('uuid', $value)->firstOrFail();
         });
 
diff --git a/tests/Integration/Api/Client/Server/Subuser/CreateServerSubuserTest.php b/tests/Integration/Api/Client/Server/Subuser/CreateServerSubuserTest.php
index 53a299e02..3a1f8bc6b 100644
--- a/tests/Integration/Api/Client/Server/Subuser/CreateServerSubuserTest.php
+++ b/tests/Integration/Api/Client/Server/Subuser/CreateServerSubuserTest.php
@@ -1,6 +1,6 @@
 <?php
 
-namespace Pterodactyl\Tests\Integration\Api\Client\Server\Schedule;
+namespace Pterodactyl\Tests\Integration\Api\Client\Server\Subuser;
 
 use Illuminate\Support\Str;
 use Pterodactyl\Models\User;
diff --git a/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php b/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php
new file mode 100644
index 000000000..3118baa24
--- /dev/null
+++ b/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php
@@ -0,0 +1,59 @@
+<?php
+
+namespace Pterodactyl\Tests\Integration\Api\Client\Server\Subuser;
+
+use Ramsey\Uuid\Uuid;
+use Pterodactyl\Models\User;
+use Pterodactyl\Models\Subuser;
+use Pterodactyl\Models\Permission;
+use Pterodactyl\Tests\Integration\Api\Client\ClientApiIntegrationTestCase;
+
+class DeleteSubuserTest extends ClientApiIntegrationTestCase
+{
+    /**
+     * Guards against PHP's exciting behavior where a string can be cast to an int and only
+     * the first numeric digits are returned. This causes UUIDs to be returned as an int when
+     * looking up users, thus returning the wrong subusers (or no subuser at all).
+     *
+     * For example, 12aaaaaa-bbbb-cccc-ddddeeeeffff would be cast to "12" if you tried to cast
+     * it to an integer. Then, in the deep API middlewares you would end up trying to load a user
+     * with an ID of 12, which may or may not exist and be wrongly assigned to the model object.
+     *
+     * @see https://github.com/pterodactyl/panel/issues/2359
+     */
+    public function testCorrectSubuserIsDeletedFromServer()
+    {
+        [$user, $server] = $this->generateTestAccount();
+
+        /** @var \Pterodactyl\Models\User $differentUser */
+        $differentUser = factory(User::class)->create();
+
+        // Generate a UUID that lines up with a user in the database if it were to be cast to an int.
+        $uuid = $differentUser->id . str_repeat('a', strlen((string)$differentUser->id)) . substr(Uuid::uuid4()->toString(), 8);
+
+        /** @var \Pterodactyl\Models\User $subuser */
+        $subuser = factory(User::class)->create(['uuid' => $uuid]);
+
+        Subuser::query()->forceCreate([
+            'user_id' => $subuser->id,
+            'server_id' => $server->id,
+            'permissions' => [ Permission::ACTION_WEBSOCKET_CONNECT ],
+        ]);
+
+        $this->actingAs($user)->deleteJson($this->link($server) . "/users/{$subuser->uuid}")->assertNoContent();
+
+        // Try the same test, but this time with a UUID that if cast to an int (shouldn't) line up with
+        // anything in the database.
+        $uuid = '18180000' . substr(Uuid::uuid4()->toString(), 8);
+        /** @var \Pterodactyl\Models\User $subuser */
+        $subuser = factory(User::class)->create(['uuid' => $uuid]);
+
+        Subuser::query()->forceCreate([
+            'user_id' => $subuser->id,
+            'server_id' => $server->id,
+            'permissions' => [ Permission::ACTION_WEBSOCKET_CONNECT ],
+        ]);
+
+        $this->actingAs($user)->deleteJson($this->link($server) . "/users/{$subuser->uuid}")->assertNoContent();
+    }
+}