Change how API keys are validated (#771)

tests/Unit/Http/Controllers/Base/APIControllerTest.php

@@ -1,54 +1,34 @@
 <?php
-/**
- * Pterodactyl - Panel
- * Copyright (c) 2015 - 2017 Dane Everitt <dane@daneeveritt.com>.
- *
- * This software is licensed under the terms of the MIT license.
- * https://opensource.org/licenses/MIT
- */
 
 namespace Tests\Unit\Http\Controllers\Base;
 
 use Mockery as m;
-use Tests\TestCase;
-use Illuminate\Http\Request;
 use Pterodactyl\Models\User;
+use Pterodactyl\Models\APIKey;
 use Prologue\Alerts\AlertsMessageBag;
-use Tests\Assertions\ControllerAssertionsTrait;
 use Pterodactyl\Services\Api\KeyCreationService;
+use Tests\Unit\Http\Controllers\ControllerTestCase;
 use Pterodactyl\Http\Controllers\Base\APIController;
 use Pterodactyl\Http\Requests\Base\ApiKeyFormRequest;
 use Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface;
 
-class APIControllerTest extends TestCase
+class APIControllerTest extends ControllerTestCase
 {
-    use ControllerAssertionsTrait;
-
     /**
-     * @var \Prologue\Alerts\AlertsMessageBag
+     * @var \Prologue\Alerts\AlertsMessageBag|\Mockery\Mock
      */
     protected $alert;
 
     /**
-     * @var \Pterodactyl\Http\Controllers\Base\APIController
-     */
-    protected $controller;
-
-    /**
-     * @var \Pterodactyl\Services\Api\KeyCreationService
+     * @var \Pterodactyl\Services\Api\KeyCreationService|\Mockery\Mock
      */
     protected $keyService;
 
     /**
-     * @var \Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface
+     * @var \Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface|\Mockery\Mock
      */
     protected $repository;
 
-    /**
-     * @var \Illuminate\Http\Request
-     */
-    protected $request;
-
     /**
      * Setup tests.
      */
@@ -59,9 +39,6 @@ class APIControllerTest extends TestCase
         $this->alert = m::mock(AlertsMessageBag::class);
         $this->keyService = m::mock(KeyCreationService::class);
         $this->repository = m::mock(ApiKeyRepositoryInterface::class);
-        $this->request = m::mock(Request::class);
-
-        $this->controller = new APIController($this->alert, $this->repository, $this->keyService);
     }
 
     /**
@@ -69,12 +46,11 @@ class APIControllerTest extends TestCase
      */
     public function testIndexController()
     {
-        $model = factory(User::class)->make();
+        $model = $this->setRequestUser();
 
-        $this->request->shouldReceive('user')->withNoArgs()->once()->andReturn($model);
         $this->repository->shouldReceive('findWhere')->with([['user_id', '=', $model->id]])->once()->andReturn(['testkeys']);
 
-        $response = $this->controller->index($this->request);
+        $response = $this->getController()->index($this->request);
         $this->assertIsViewResponse($response);
         $this->assertViewNameEquals('base.api.index', $response);
         $this->assertViewHasKey('keys', $response);
@@ -88,10 +64,9 @@ class APIControllerTest extends TestCase
      */
     public function testCreateController($admin)
     {
-        $model = factory(User::class)->make(['root_admin' => $admin]);
-        $this->request->shouldReceive('user')->withNoArgs()->once()->andReturn($model);
+        $this->setRequestUser(factory(User::class)->make(['root_admin' => $admin]));
 
-        $response = $this->controller->create($this->request);
+        $response = $this->getController()->create($this->request);
         $this->assertIsViewResponse($response);
         $this->assertViewNameEquals('base.api.new', $response);
         $this->assertViewHasKey('permissions.user', $response);
@@ -111,8 +86,9 @@ class APIControllerTest extends TestCase
      */
     public function testStoreController($admin)
     {
-        $this->request = m::mock(ApiKeyFormRequest::class);
-        $model = factory(User::class)->make(['root_admin' => $admin]);
+        $this->setRequestMockClass(ApiKeyFormRequest::class);
+        $model = $this->setRequestUser(factory(User::class)->make(['root_admin' => $admin]));
+        $keyModel = factory(APIKey::class)->make();
 
         if ($admin) {
             $this->request->shouldReceive('input')->with('admin_permissions', [])->once()->andReturn(['admin.permission']);
@@ -127,12 +103,12 @@ class APIControllerTest extends TestCase
             'user_id' => $model->id,
             'allowed_ips' => null,
             'memo' => null,
-        ], ['test.permission'], ($admin) ? ['admin.permission'] : [])->once()->andReturn('testToken');
+        ], ['test.permission'], ($admin) ? ['admin.permission'] : [])->once()->andReturn($keyModel);
 
-        $this->alert->shouldReceive('success')->with(trans('base.api.index.keypair_created', ['token' => 'testToken']))->once()->andReturnSelf()
-            ->shouldReceive('flash')->withNoArgs()->once()->andReturnNull();
+        $this->alert->shouldReceive('success')->with(trans('base.api.index.keypair_created'))->once()->andReturnSelf();
+        $this->alert->shouldReceive('flash')->withNoArgs()->once()->andReturnNull();
 
-        $response = $this->controller->store($this->request);
+        $response = $this->getController()->store($this->request);
         $this->assertIsRedirectResponse($response);
         $this->assertRedirectRouteEquals('account.api', $response);
     }
@@ -142,15 +118,14 @@ class APIControllerTest extends TestCase
      */
     public function testRevokeController()
     {
-        $model = factory(User::class)->make();
-        $this->request->shouldReceive('user')->withNoArgs()->once()->andReturn($model);
+        $model = $this->setRequestUser();
 
         $this->repository->shouldReceive('deleteWhere')->with([
             ['user_id', '=', $model->id],
-            ['public', '=', 'testKey123'],
+            ['token', '=', 'testKey123'],
         ])->once()->andReturnNull();
 
-        $response = $this->controller->revoke($this->request, 'testKey123');
+        $response = $this->getController()->revoke($this->request, 'testKey123');
         $this->assertIsResponse($response);
         $this->assertEmpty($response->getContent());
         $this->assertResponseCodeEquals(204, $response);
@@ -165,4 +140,14 @@ class APIControllerTest extends TestCase
     {
         return [[0], [1]];
     }
+
+    /**
+     * Return an instance of the controller with mocked dependencies for testing.
+     *
+     * @return \Pterodactyl\Http\Controllers\Base\APIController
+     */
+    private function getController(): APIController
+    {
+        return new APIController($this->alert, $this->repository, $this->keyService);
+    }
 }

tests/Unit/Http/Middleware/API/AuthenticateIPAccessTest.php

@@ -0,0 +1,82 @@
+<?php
+
+namespace Tests\Unit\Http\Middleware\API;
+
+use Pterodactyl\Models\APIKey;
+use Tests\Unit\Http\Middleware\MiddlewareTestCase;
+use Pterodactyl\Http\Middleware\API\AuthenticateIPAccess;
+
+class AuthenticateIPAccessTest extends MiddlewareTestCase
+{
+    /**
+     * Setup tests.
+     */
+    public function setUp()
+    {
+        parent::setUp();
+    }
+
+    /**
+     * Test middleware when there are no IP restrictions.
+     */
+    public function testWithNoIPRestrictions()
+    {
+        $model = factory(APIKey::class)->make(['allowed_ips' => []]);
+        $this->setRequestAttribute('api_key', $model);
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+    }
+
+    /**
+     * Test middleware works correctly when a valid IP accesses
+     * and there is an IP restriction.
+     */
+    public function testWithValidIP()
+    {
+        $model = factory(APIKey::class)->make(['allowed_ips' => ['127.0.0.1']]);
+        $this->setRequestAttribute('api_key', $model);
+
+        $this->request->shouldReceive('ip')->withNoArgs()->once()->andReturn('127.0.0.1');
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+    }
+
+    /**
+     * Test that a CIDR range can be used.
+     */
+    public function testValidIPAganistCIDRRange()
+    {
+        $model = factory(APIKey::class)->make(['allowed_ips' => ['192.168.1.1/28']]);
+        $this->setRequestAttribute('api_key', $model);
+
+        $this->request->shouldReceive('ip')->withNoArgs()->once()->andReturn('192.168.1.15');
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+    }
+
+    /**
+     * Test that an exception is thrown when an invalid IP address
+     * tries to connect and there is an IP restriction.
+     *
+     * @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
+     */
+    public function testWithInvalidIP()
+    {
+        $model = factory(APIKey::class)->make(['allowed_ips' => ['127.0.0.1']]);
+        $this->setRequestAttribute('api_key', $model);
+
+        $this->request->shouldReceive('ip')->withNoArgs()->once()->andReturn('127.0.0.2');
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+    }
+
+    /**
+     * Return an instance of the middleware to be used when testing.
+     *
+     * @return \Pterodactyl\Http\Middleware\API\AuthenticateIPAccess
+     */
+    private function getMiddleware(): AuthenticateIPAccess
+    {
+        return new AuthenticateIPAccess();
+    }
+}

tests/Unit/Http/Middleware/API/AuthenticateKeyTest.php

@@ -0,0 +1,90 @@
+<?php
+
+namespace Tests\Unit\Http\Middleware\API;
+
+use Mockery as m;
+use Pterodactyl\Models\APIKey;
+use Illuminate\Auth\AuthManager;
+use Tests\Unit\Http\Middleware\MiddlewareTestCase;
+use Pterodactyl\Http\Middleware\API\AuthenticateKey;
+use Symfony\Component\HttpKernel\Exception\HttpException;
+use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
+use Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface;
+
+class AuthenticateKeyTest extends MiddlewareTestCase
+{
+    /**
+     * @var \Illuminate\Auth\AuthManager|\Mockery\Mock
+     */
+    private $auth;
+
+    /**
+     * @var \Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface|\Mockery\Mock
+     */
+    private $repository;
+
+    /**
+     * Setup tests.
+     */
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->auth = m::mock(AuthManager::class);
+        $this->repository = m::mock(ApiKeyRepositoryInterface::class);
+    }
+
+    /**
+     * Test that a missing bearer token will throw an exception.
+     */
+    public function testMissingBearerTokenThrowsException()
+    {
+        $this->request->shouldReceive('bearerToken')->withNoArgs()->once()->andReturnNull();
+
+        try {
+            $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+        } catch (HttpException $exception) {
+            $this->assertEquals(401, $exception->getStatusCode());
+            $this->assertEquals(['WWW-Authenticate' => 'Bearer'], $exception->getHeaders());
+        }
+    }
+
+    /**
+     * Test that an invalid API token throws an exception.
+     *
+     * @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
+     */
+    public function testInvalidTokenThrowsException()
+    {
+        $this->request->shouldReceive('bearerToken')->withNoArgs()->twice()->andReturn('abcd1234');
+        $this->repository->shouldReceive('findFirstWhere')->andThrow(new RecordNotFoundException);
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+    }
+
+    /**
+     * Test that a valid token can continue past the middleware.
+     */
+    public function testValidToken()
+    {
+        $model = factory(APIKey::class)->make();
+
+        $this->request->shouldReceive('bearerToken')->withNoArgs()->twice()->andReturn($model->token);
+        $this->repository->shouldReceive('findFirstWhere')->with([['token', '=', $model->token]])->once()->andReturn($model);
+
+        $this->auth->shouldReceive('guard->loginUsingId')->with($model->user_id)->once()->andReturnNull();
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+        $this->assertEquals($model, $this->request->attributes->get('api_key'));
+    }
+
+    /**
+     * Return an instance of the middleware with mocked dependencies for testing.
+     *
+     * @return \Pterodactyl\Http\Middleware\API\AuthenticateKey
+     */
+    private function getMiddleware(): AuthenticateKey
+    {
+        return new AuthenticateKey($this->repository, $this->auth);
+    }
+}

tests/Unit/Http/Middleware/API/HasPermissionToResourceTest.php

@@ -0,0 +1,109 @@
+<?php
+
+namespace Tests\Unit\Http\Middleware\API;
+
+use Mockery as m;
+use Pterodactyl\Models\User;
+use Pterodactyl\Models\APIKey;
+use Pterodactyl\Models\APIPermission;
+use Tests\Unit\Http\Middleware\MiddlewareTestCase;
+use Pterodactyl\Http\Middleware\API\HasPermissionToResource;
+use Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface;
+
+class HasPermissionToResourceTest extends MiddlewareTestCase
+{
+    /**
+     * @var \Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface|\Mockery\Mock
+     */
+    private $repository;
+
+    /**
+     * Setup tests.
+     */
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->repository = m::mock(ApiKeyRepositoryInterface::class);
+    }
+
+    /**
+     * Test that a non-admin user cannot access admin level routes.
+     *
+     * @expectedException \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
+     */
+    public function testNonAdminAccessingAdminLevel()
+    {
+        $model = factory(APIKey::class)->make();
+        $this->setRequestAttribute('api_key', $model);
+        $this->setRequestUser(factory(User::class)->make(['root_admin' => false]));
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+    }
+
+    /**
+     * Test non-admin accessing non-admin route.
+     */
+    public function testAccessingAllowedRoute()
+    {
+        $model = factory(APIKey::class)->make();
+        $model->setRelation('permissions', collect([
+            factory(APIPermission::class)->make(['permission' => 'user.test-route']),
+        ]));
+        $this->setRequestAttribute('api_key', $model);
+        $this->setRequestUser(factory(User::class)->make(['root_admin' => false]));
+
+        $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('api.user.test.route');
+        $this->repository->shouldReceive('loadPermissions')->with($model)->once()->andReturn($model);
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions(), 'user');
+    }
+
+    /**
+     * Test admin accessing administrative route.
+     */
+    public function testAccessingAllowedAdminRoute()
+    {
+        $model = factory(APIKey::class)->make();
+        $model->setRelation('permissions', collect([
+            factory(APIPermission::class)->make(['permission' => 'test-route']),
+        ]));
+        $this->setRequestAttribute('api_key', $model);
+        $this->setRequestUser(factory(User::class)->make(['root_admin' => true]));
+
+        $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('api.admin.test.route');
+        $this->repository->shouldReceive('loadPermissions')->with($model)->once()->andReturn($model);
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+    }
+
+    /**
+     * Test a user accessing a disallowed route.
+     *
+     * @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
+     */
+    public function testAccessingDisallowedRoute()
+    {
+        $model = factory(APIKey::class)->make();
+        $model->setRelation('permissions', collect([
+            factory(APIPermission::class)->make(['permission' => 'user.other-route']),
+        ]));
+        $this->setRequestAttribute('api_key', $model);
+        $this->setRequestUser(factory(User::class)->make(['root_admin' => false]));
+
+        $this->request->shouldReceive('route->getName')->withNoArgs()->once()->andReturn('api.user.test.route');
+        $this->repository->shouldReceive('loadPermissions')->with($model)->once()->andReturn($model);
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions(), 'user');
+    }
+
+    /**
+     * Return an instance of the middleware with mocked dependencies for testing.
+     *
+     * @return \Pterodactyl\Http\Middleware\API\HasPermissionToResource
+     */
+    private function getMiddleware(): HasPermissionToResource
+    {
+        return new HasPermissionToResource($this->repository);
+    }
+}

tests/Unit/Http/Middleware/API/SetSessionDriverTest.php

@@ -0,0 +1,69 @@
+<?php
+
+namespace Tests\Unit\Http\Middleware\API;
+
+use Mockery as m;
+use Barryvdh\Debugbar\LaravelDebugbar;
+use Illuminate\Contracts\Config\Repository;
+use Illuminate\Contracts\Foundation\Application;
+use Tests\Unit\Http\Middleware\MiddlewareTestCase;
+use Pterodactyl\Http\Middleware\API\SetSessionDriver;
+
+class SetSessionDriverTest extends MiddlewareTestCase
+{
+    /**
+     * @var \Illuminate\Contracts\Foundation\Application|\Mockery\Mock
+     */
+    private $appMock;
+
+    /**
+     * @var \Illuminate\Contracts\Config\Repository|\Mockery\Mock
+     */
+    private $config;
+
+    /**
+     * Setup tests.
+     */
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->appMock = m::mock(Application::class);
+        $this->config = m::mock(Repository::class);
+    }
+
+    /**
+     * Test that a production environment does not try to disable debug bar.
+     */
+    public function testProductionEnvironment()
+    {
+        $this->appMock->shouldReceive('environment')->withNoArgs()->once()->andReturn('production');
+        $this->config->shouldReceive('set')->with('session.driver', 'array')->once()->andReturnNull();
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+    }
+
+    /**
+     * Test that a local environment does disable debug bar.
+     */
+    public function testLocalEnvironment()
+    {
+        $this->appMock->shouldReceive('environment')->withNoArgs()->once()->andReturn('local');
+        $this->appMock->shouldReceive('make')->with(LaravelDebugbar::class)->once()->andReturnSelf();
+        $this->appMock->shouldReceive('disable')->withNoArgs()->once()->andReturnNull();
+
+        $this->config->shouldReceive('set')->with('session.driver', 'array')->once()->andReturnNull();
+
+        $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
+    }
+
+    /**
+     * Return an instance of the middleware with mocked dependencies for testing.
+     *
+     * @return \Pterodactyl\Http\Middleware\API\SetSessionDriver
+     */
+    private function getMiddleware(): SetSessionDriver
+    {
+        return new SetSessionDriver($this->appMock, $this->config);
+    }
+}

tests/Unit/Services/Api/KeyCreationServiceTest.php

@@ -12,8 +12,8 @@ namespace Tests\Unit\Services\Api;
 use Mockery as m;
 use Tests\TestCase;
 use phpmock\phpunit\PHPMock;
+use Pterodactyl\Models\APIKey;
 use Illuminate\Database\ConnectionInterface;
-use Illuminate\Contracts\Encryption\Encrypter;
 use Pterodactyl\Services\Api\PermissionService;
 use Pterodactyl\Services\Api\KeyCreationService;
 use Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface;
@@ -23,45 +23,30 @@ class KeyCreationServiceTest extends TestCase
     use PHPMock;
 
     /**
-     * @var \Illuminate\Database\ConnectionInterface
+     * @var \Illuminate\Database\ConnectionInterface|\Mockery\Mock
      */
-    protected $connection;
+    private $connection;
 
     /**
-     * @var \Illuminate\Contracts\Encryption\Encrypter
+     * @var \Pterodactyl\Services\Api\PermissionService|\Mockery\Mock
      */
-    protected $encrypter;
+    private $permissionService;
 
     /**
-     * @var \Pterodactyl\Services\Api\PermissionService
+     * @var \Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface|\Mockery\Mock
      */
-    protected $permissions;
+    private $repository;
 
     /**
-     * @var \Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface
+     * Setup tests.
      */
-    protected $repository;
-
-    /**
-     * @var \Pterodactyl\Services\Api\KeyCreationService
-     */
-    protected $service;
-
     public function setUp()
     {
         parent::setUp();
 
         $this->connection = m::mock(ConnectionInterface::class);
-        $this->encrypter = m::mock(Encrypter::class);
-        $this->permissions = m::mock(PermissionService::class);
+        $this->permissionService = m::mock(PermissionService::class);
         $this->repository = m::mock(ApiKeyRepositoryInterface::class);
-
-        $this->service = new KeyCreationService(
-            $this->repository,
-            $this->connection,
-            $this->encrypter,
-            $this->permissions
-        );
     }
 
     /**
@@ -69,37 +54,48 @@ class KeyCreationServiceTest extends TestCase
      */
     public function testKeyIsCreated()
     {
+        $model = factory(APIKey::class)->make();
+
         $this->getFunctionMock('\\Pterodactyl\\Services\\Api', 'str_random')
-            ->expects($this->exactly(2))->willReturn('random_string');
+            ->expects($this->exactly(1))->with(APIKey::KEY_LENGTH)->willReturn($model->token);
 
         $this->connection->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull();
-        $this->encrypter->shouldReceive('encrypt')->with('random_string')->once()->andReturn('encrypted-secret');
 
         $this->repository->shouldReceive('create')->with([
             'test-data' => 'test',
-            'public' => 'random_string',
-            'secret' => 'encrypted-secret',
-        ], true, true)->once()->andReturn((object) ['id' => 1]);
+            'token' => $model->token,
+        ], true, true)->once()->andReturn($model);
 
-        $this->permissions->shouldReceive('getPermissions')->withNoArgs()->once()->andReturn([
+        $this->permissionService->shouldReceive('getPermissions')->withNoArgs()->once()->andReturn([
             '_user' => ['server' => ['list', 'multiple-dash-test']],
             'server' => ['create', 'admin-dash-test'],
         ]);
 
-        $this->permissions->shouldReceive('create')->with(1, 'user.server-list')->once()->andReturnNull();
-        $this->permissions->shouldReceive('create')->with(1, 'user.server-multiple-dash-test')->once()->andReturnNull();
-        $this->permissions->shouldReceive('create')->with(1, 'server-create')->once()->andReturnNull();
-        $this->permissions->shouldReceive('create')->with(1, 'server-admin-dash-test')->once()->andReturnNull();
+        $this->permissionService->shouldReceive('create')->with($model->id, 'user.server-list')->once()->andReturnNull();
+        $this->permissionService->shouldReceive('create')->with($model->id, 'user.server-multiple-dash-test')->once()->andReturnNull();
+        $this->permissionService->shouldReceive('create')->with($model->id, 'server-create')->once()->andReturnNull();
+        $this->permissionService->shouldReceive('create')->with($model->id, 'server-admin-dash-test')->once()->andReturnNull();
 
         $this->connection->shouldReceive('commit')->withNoArgs()->once()->andReturnNull();
 
-        $response = $this->service->handle(
+        $response = $this->getService()->handle(
             ['test-data' => 'test'],
             ['invalid-node', 'server-list', 'server-multiple-dash-test'],
             ['invalid-node', 'server-create', 'server-admin-dash-test']
         );
 
         $this->assertNotEmpty($response);
-        $this->assertEquals('random_string', $response);
+        $this->assertInstanceOf(APIKey::class, $response);
+        $this->assertSame($model, $response);
+    }
+
+    /**
+     * Return an instance of the service with mocked dependencies for testing.
+     *
+     * @return \Pterodactyl\Services\Api\KeyCreationService
+     */
+    private function getService(): KeyCreationService
+    {
+        return new KeyCreationService($this->repository, $this->connection, $this->permissionService);
     }
 }