Handle missing daemon keys better and fix subuser missing key errors

Author: DaneEveritt

CHANGELOG.md

@@ -8,10 +8,12 @@ This project follows [Semantic Versioning](http://semver.org) guidelines.
 * Fixes application API keys being created as a client API key.
 * Search term is now passed through when using paginated result sets.
 * Reduces the number of SQL queries executed when rendering the server listing to increase performance.
+* Fixes exceptions being thrown for non-existent subuser permissions.
 
 ### Changed
 * Databases are now properly paginated when viewing a database host.
 * No more loading daemon keys for every server model being loaded, some of us value our databases.
+* Changed behavior of the subuser middleware to add a daemon access key if one is missing from the database for some reason.
 
 ## v0.7.4-h1 (Derelict Dermodactylus)
 ### Fixed

app/Http/Middleware/Server/AuthenticateAsSubuser.php

@@ -11,7 +11,6 @@
 
 use Closure;
 use Illuminate\Http\Request;
-use Illuminate\Contracts\Session\Session;
 use Pterodactyl\Services\DaemonKeys\DaemonKeyProviderService;
 use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
 use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
@@ -23,21 +22,14 @@ class AuthenticateAsSubuser
      */
     private $keyProviderService;
 
-    /**
-     * @var \Illuminate\Contracts\Session\Session
-     */
-    private $session;
-
     /**
      * SubuserAccessAuthenticate constructor.
      *
      * @param \Pterodactyl\Services\DaemonKeys\DaemonKeyProviderService $keyProviderService
-     * @param \Illuminate\Contracts\Session\Session                     $session
      */
-    public function __construct(DaemonKeyProviderService $keyProviderService, Session $session)
+    public function __construct(DaemonKeyProviderService $keyProviderService)
     {
         $this->keyProviderService = $keyProviderService;
-        $this->session = $session;
     }
 
     /**
@@ -60,7 +52,6 @@ public function handle(Request $request, Closure $next)
             throw new AccessDeniedHttpException('This account does not have permission to access this server.');
         }
 
-        $this->session->now('server_data.token', $token);
         $request->attributes->set('server_token', $token);
 
         return $next($request);

app/Services/DaemonKeys/DaemonKeyProviderService.php

@@ -28,6 +28,7 @@
 use Pterodactyl\Models\User;
 use Pterodactyl\Models\Server;
 use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
+use Pterodactyl\Contracts\Repository\SubuserRepositoryInterface;
 use Pterodactyl\Contracts\Repository\DaemonKeyRepositoryInterface;
 
 class DaemonKeyProviderService
@@ -47,21 +48,29 @@ class DaemonKeyProviderService
      */
     private $repository;
 
+    /**
+     * @var \Pterodactyl\Contracts\Repository\SubuserRepositoryInterface
+     */
+    private $subuserRepository;
+
     /**
      * GetDaemonKeyService constructor.
      *
      * @param \Pterodactyl\Services\DaemonKeys\DaemonKeyCreationService      $keyCreationService
      * @param \Pterodactyl\Contracts\Repository\DaemonKeyRepositoryInterface $repository
      * @param \Pterodactyl\Services\DaemonKeys\DaemonKeyUpdateService        $keyUpdateService
+     * @param \Pterodactyl\Contracts\Repository\SubuserRepositoryInterface   $subuserRepository
      */
     public function __construct(
         DaemonKeyCreationService $keyCreationService,
         DaemonKeyRepositoryInterface $repository,
-        DaemonKeyUpdateService $keyUpdateService
+        DaemonKeyUpdateService $keyUpdateService,
+        SubuserRepositoryInterface $subuserRepository
     ) {
         $this->keyCreationService = $keyCreationService;
         $this->keyUpdateService = $keyUpdateService;
         $this->repository = $repository;
+        $this->subuserRepository = $subuserRepository;
     }
 
     /**
@@ -89,10 +98,18 @@ public function handle(Server $server, User $user, $updateIfExpired = true): str
                 return $this->keyCreationService->handle($server->id, $user->id);
             }
 
-            // If they aren't the admin or owner of the server, they shouldn't get access.
-            // Subusers should always have an entry created when they are, so if there is
-            // no record, it should fail.
-            throw $exception;
+            // Check if user is a subuser for this server. Ideally they should always have
+            // a record associated with them in the database, but we should still handle
+            // that potentiality here.
+            //
+            // If no subuser is found, a RecordNotFoundException will be thrown, thus handling
+            // the parent error as well.
+            $subuser = $this->subuserRepository->findFirstWhere([
+                ['user_id', '=', $user->id],
+                ['server_id', '=', $server->id],
+            ]);
+
+            return $this->keyCreationService->handle($subuser->server_id, $subuser->user_id);
         }
 
         if (! $updateIfExpired || Carbon::now()->diffInSeconds($key->expires_at, false) > 0) {

app/Transformers/Daemon/ApiKeyTransformer.php

@@ -1,26 +1,4 @@
 <?php
-/*
- * Pterodactyl - Panel
- * Copyright (c) 2015 - 2017 Dane Everitt <dane@daneeveritt.com>.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in all
- * copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
- */
 
 namespace Pterodactyl\Transformers\Daemon;
 
@@ -83,8 +61,8 @@ public function transform(DaemonKey $key)
         $daemonPermissions = ['s:console'];
 
         foreach ($permissions as $permission) {
-            if (! is_null($mappings[$permission])) {
-                $daemonPermissions[] = $mappings[$permission];
+            if (! is_null(array_get($mappings, $permission))) {
+                $daemonPermissions[] = array_get($mappings, $permission);
             }
         }
 

tests/Unit/Http/Middleware/Server/AuthenticateAsSubuserTest.php

@@ -4,7 +4,6 @@
 
 use Mockery as m;
 use Pterodactyl\Models\Server;
-use Illuminate\Contracts\Session\Session;
 use Tests\Unit\Http\Middleware\MiddlewareTestCase;
 use Pterodactyl\Http\Middleware\Server\AuthenticateAsSubuser;
 use Pterodactyl\Services\DaemonKeys\DaemonKeyProviderService;
@@ -17,11 +16,6 @@ class AuthenticateAsSubuserTest extends MiddlewareTestCase
      */
     private $keyProviderService;
 
-    /**
-     * @var \Illuminate\Contracts\Session\Session|\Mockery\Mock
-     */
-    private $session;
-
     /**
      * Setup tests.
      */
@@ -30,7 +24,6 @@ public function setUp()
         parent::setUp();
 
         $this->keyProviderService = m::mock(DaemonKeyProviderService::class);
-        $this->session = m::mock(Session::class);
     }
 
     /**
@@ -43,7 +36,6 @@ public function testSuccessfulMiddleware()
         $this->setRequestAttribute('server', $model);
 
         $this->keyProviderService->shouldReceive('handle')->with($model, $user)->once()->andReturn('abc123');
-        $this->session->shouldReceive('now')->with('server_data.token', 'abc123')->once()->andReturnNull();
 
         $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
         $this->assertRequestHasAttribute('server_token');
@@ -74,6 +66,6 @@ public function testExceptionIsThrownIfNoTokenIsFound()
      */
     public function getMiddleware(): AuthenticateAsSubuser
     {
-        return new AuthenticateAsSubuser($this->keyProviderService, $this->session);
+        return new AuthenticateAsSubuser($this->keyProviderService);
     }
 }

tests/Unit/Services/DaemonKeys/DaemonKeyProviderServiceTest.php

@@ -14,11 +14,13 @@
 use Tests\TestCase;
 use Pterodactyl\Models\User;
 use Pterodactyl\Models\Server;
+use Pterodactyl\Models\Subuser;
 use Pterodactyl\Models\DaemonKey;
 use Pterodactyl\Services\DaemonKeys\DaemonKeyUpdateService;
 use Pterodactyl\Services\DaemonKeys\DaemonKeyCreationService;
 use Pterodactyl\Services\DaemonKeys\DaemonKeyProviderService;
 use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
+use Pterodactyl\Contracts\Repository\SubuserRepositoryInterface;
 use Pterodactyl\Contracts\Repository\DaemonKeyRepositoryInterface;
 
 class DaemonKeyProviderServiceTest extends TestCase
@@ -38,6 +40,11 @@ class DaemonKeyProviderServiceTest extends TestCase
      */
     private $repository;
 
+    /**
+     * @var \Pterodactyl\Contracts\Repository\SubuserRepositoryInterface|\Mockery\Mock
+     */
+    private $subuserRepository;
+
     /**
      * Setup tests.
      */
@@ -49,6 +56,7 @@ public function setUp()
         $this->keyCreationService = m::mock(DaemonKeyCreationService::class);
         $this->keyUpdateService = m::mock(DaemonKeyUpdateService::class);
         $this->repository = m::mock(DaemonKeyRepositoryInterface::class);
+        $this->subuserRepository = m::mock(SubuserRepositoryInterface::class);
     }
 
     /**
@@ -154,6 +162,33 @@ public function testMissingKeyIsCreatedIfUserIsServerOwner()
         $this->assertEquals($key->secret, $response);
     }
 
+    /**
+     * Test that a missing key is created for a subuser.
+     */
+    public function testMissingKeyIsCreatedForSubuser()
+    {
+        $user = factory(User::class)->make(['root_admin' => 0]);
+        $server = factory(Server::class)->make();
+        $key = factory(DaemonKey::class)->make(['expires_at' => Carbon::now()->subHour()]);
+        $subuser = factory(Subuser::class)->make(['user_id' => $user->id, 'server_id' => $server->id]);
+
+        $this->repository->shouldReceive('findFirstWhere')->with([
+            ['user_id', '=', $user->id],
+            ['server_id', '=', $server->id],
+        ])->once()->andThrow(new RecordNotFoundException);
+
+        $this->subuserRepository->shouldReceive('findFirstWhere')->once()->with([
+            ['user_id', '=', $user->id],
+            ['server_id', '=', $server->id],
+        ])->andReturn($subuser);
+
+        $this->keyCreationService->shouldReceive('handle')->with($server->id, $user->id)->once()->andReturn($key->secret);
+
+        $response = $this->getService()->handle($server, $user, false);
+        $this->assertNotEmpty($response);
+        $this->assertEquals($key->secret, $response);
+    }
+
     /**
      * Test that an exception is thrown if the user should not get a key.
      *
@@ -169,6 +204,11 @@ public function testExceptionIsThrownIfUserDoesNotDeserveKey()
             ['server_id', '=', $server->id],
         ])->once()->andThrow(new RecordNotFoundException);
 
+        $this->subuserRepository->shouldReceive('findFirstWhere')->once()->with([
+            ['user_id', '=', $user->id],
+            ['server_id', '=', $server->id],
+        ])->andThrow(new RecordNotFoundException);
+
         $this->getService()->handle($server, $user, false);
     }
 
@@ -179,6 +219,11 @@ public function testExceptionIsThrownIfUserDoesNotDeserveKey()
      */
     private function getService(): DaemonKeyProviderService
     {
-        return new DaemonKeyProviderService($this->keyCreationService, $this->repository, $this->keyUpdateService);
+        return new DaemonKeyProviderService(
+            $this->keyCreationService,
+            $this->repository,
+            $this->keyUpdateService,
+            $this->subuserRepository
+        );
     }
 }