From c20d53bb174539c9ca6bf6ed77b44a9cd0b3fe37 Mon Sep 17 00:00:00 2001
From: Dane Everitt <dane@daneeveritt.com>
Date: Sat, 7 Nov 2020 09:57:53 -0800
Subject: [PATCH] Always return the primary allocation for a server, even
 without the allocation permissions

---
 app/Models/Permission.php                     |  2 +-
 .../Api/Client/ServerTransformer.php          | 20 +++++++++----
 .../Api/Client/ClientControllerTest.php       | 28 +++++++++++++++++++
 3 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/app/Models/Permission.php b/app/Models/Permission.php
index 96429f31c..d18e72009 100644
--- a/app/Models/Permission.php
+++ b/app/Models/Permission.php
@@ -163,7 +163,7 @@ class Permission extends Model
         'allocation' => [
             'description' => 'Permissions that control a user\'s ability to modify the port allocations for this server.',
             'keys' => [
-                'read' => 'Allows a user to view the allocations assigned to this server.',
+                'read' => 'Allows a user to view all allocations currently assigned to this server. Users with any level of access to this server can always view the primary allocation.',
                 'create' => 'Allows a user to assign additional allocations to the server.',
                 'update' => 'Allows a user to change the primary server allocation and attach notes to each allocation.',
                 'delete' => 'Allows a user to delete an allocation from the server.',
diff --git a/app/Transformers/Api/Client/ServerTransformer.php b/app/Transformers/Api/Client/ServerTransformer.php
index a787246bf..ac3ab26ba 100644
--- a/app/Transformers/Api/Client/ServerTransformer.php
+++ b/app/Transformers/Api/Client/ServerTransformer.php
@@ -83,15 +83,23 @@ class ServerTransformer extends BaseClientTransformer
      */
     public function includeAllocations(Server $server)
     {
+        $transformer = $this->makeTransformer(AllocationTransformer::class);
+
+        // While we include this permission, we do need to actually handle it slightly different here
+        // for the purpose of keeping things functionally working. If the user doesn't have read permissions
+        // for the allocations we'll only return the primary server allocation, and any notes associated
+        // with it will be hidden.
+        //
+        // This allows us to avoid too much permission regression, without also hiding information that
+        // is generally needed for the frontend to make sense when browsing or searching results.
         if (! $this->getUser()->can(Permission::ACTION_ALLOCATION_READ, $server)) {
-            return $this->null();
+            $primary = clone $server->allocation;
+            $primary->notes = null;
+
+            return $this->collection([$primary], $transformer, Allocation::RESOURCE_NAME);
         }
 
-        return $this->collection(
-            $server->allocations,
-            $this->makeTransformer(AllocationTransformer::class),
-            Allocation::RESOURCE_NAME
-        );
+        return $this->collection($server->allocations, $transformer, Allocation::RESOURCE_NAME);
     }
 
     /**
diff --git a/tests/Integration/Api/Client/ClientControllerTest.php b/tests/Integration/Api/Client/ClientControllerTest.php
index 30d837815..fc8c2c1e1 100644
--- a/tests/Integration/Api/Client/ClientControllerTest.php
+++ b/tests/Integration/Api/Client/ClientControllerTest.php
@@ -304,6 +304,34 @@ class ClientControllerTest extends ClientApiIntegrationTestCase
         $response->assertJsonCount(0, 'data');
     }
 
+    /**
+     * Test that a subuser without the allocation.read permission is only able to see the primary
+     * allocation for the server.
+     */
+    public function testOnlyPrimaryAllocationIsReturnedToSubuser()
+    {
+        /** @var \Pterodactyl\Models\Server $server */
+        [$user, $server] = $this->generateTestAccount([Permission::ACTION_WEBSOCKET_CONNECT]);
+        $server->allocation->notes = 'Test notes';
+        $server->allocation->save();
+
+        factory(Allocation::class)->times(2)->create([
+            'node_id' => $server->node_id,
+            'server_id' => $server->id,
+        ]);
+
+        $server->refresh();
+        $response = $this->actingAs($user)->getJson('/api/client');
+
+        $response->assertOk();
+        $response->assertJsonCount(1, 'data');
+        $response->assertJsonPath('data.0.attributes.server_owner', false);
+        $response->assertJsonPath('data.0.attributes.uuid', $server->uuid);
+        $response->assertJsonCount(1, 'data.0.attributes.relationships.allocations.data');
+        $response->assertJsonPath('data.0.attributes.relationships.allocations.data.0.attributes.id', $server->allocation->id);
+        $response->assertJsonPath('data.0.attributes.relationships.allocations.data.0.attributes.notes', null);
+    }
+
     /**
      * @return array
      */