diff --git a/CHANGELOG.md b/CHANGELOG.md index b0d21c7c..82217506 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ 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. +## v0.7.0-beta.3 (Derelict Dermodactylus) +### Changed +* API keys have been changed to only use a single public key passed in a bearer token. All existing keys can continue being used, however only the first 32 characters should be sent. + ## v0.7.0-beta.2 (Derelict Dermodactylus) ### Fixed * `[beta.1]` — Fixes a CORS header issue due to a wrong API endpoint being provided in the administrative node listing. diff --git a/app/Http/Controllers/Base/APIController.php b/app/Http/Controllers/Base/APIController.php index 4bf89012..c7366177 100644 --- a/app/Http/Controllers/Base/APIController.php +++ b/app/Http/Controllers/Base/APIController.php @@ -1,27 +1,4 @@ - * Some Modifications (c) 2015 Dylan Seidt . - * - * 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\Http\Controllers\Base; @@ -120,7 +97,7 @@ class APIController extends Controller 'memo' => $request->input('memo'), ], $request->input('permissions', []), $adminPermissions); - $this->alert->success(trans('base.api.index.keypair_created', ['token' => $secret]))->flash(); + $this->alert->success(trans('base.api.index.keypair_created'))->flash(); return redirect()->route('account.api'); } @@ -136,7 +113,7 @@ class APIController extends Controller { $this->repository->deleteWhere([ ['user_id', '=', $request->user()->id], - ['public', '=', $key], + ['token', '=', $key], ]); return response('', 204); diff --git a/app/Http/Middleware/API/AuthenticateKey.php b/app/Http/Middleware/API/AuthenticateKey.php new file mode 100644 index 00000000..da833d89 --- /dev/null +++ b/app/Http/Middleware/API/AuthenticateKey.php @@ -0,0 +1,40 @@ +repository = $repository; + } + + /** + * Handle an API request by verifying that the provided API key + * is in a valid format, and the route being accessed is allowed + * for the given key. + * + * @param \Illuminate\Http\Request $request + * @param \Closure $next + */ + public function handle(Request $request, Closure $next) + { + $this->repository->findFirstWhere([ + '', + ]); + } +} diff --git a/app/Models/APIKey.php b/app/Models/APIKey.php index cbfc996d..0542fa02 100644 --- a/app/Models/APIKey.php +++ b/app/Models/APIKey.php @@ -19,6 +19,8 @@ class APIKey extends Model implements CleansAttributes, ValidableContract { use Eloquence, Validable; + const KEY_LENGTH = 32; + /** * The table associated with the model. * @@ -26,13 +28,6 @@ class APIKey extends Model implements CleansAttributes, ValidableContract */ protected $table = 'api_keys'; - /** - * The attributes excluded from the model's JSON form. - * - * @var array - */ - protected $hidden = ['secret']; - /** * Cast values to correct type. * @@ -57,8 +52,7 @@ class APIKey extends Model implements CleansAttributes, ValidableContract protected static $applicationRules = [ 'memo' => 'required', 'user_id' => 'required', - 'secret' => 'required', - 'public' => 'required', + 'token' => 'required', ]; /** @@ -68,8 +62,7 @@ class APIKey extends Model implements CleansAttributes, ValidableContract */ protected static $dataIntegrityRules = [ 'user_id' => 'exists:users,id', - 'public' => 'string|size:16', - 'secret' => 'string', + 'token' => 'string|size:32', 'memo' => 'nullable|string|max:500', 'allowed_ips' => 'nullable|json', 'expires_at' => 'nullable|datetime', diff --git a/app/Services/Api/KeyCreationService.php b/app/Services/Api/KeyCreationService.php index 1a431254..891a3243 100644 --- a/app/Services/Api/KeyCreationService.php +++ b/app/Services/Api/KeyCreationService.php @@ -1,60 +1,42 @@ . - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Pterodactyl\Services\Api; +use Pterodactyl\Models\APIKey; use Illuminate\Database\ConnectionInterface; -use Illuminate\Contracts\Encryption\Encrypter; use Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface; class KeyCreationService { - const PUB_CRYPTO_LENGTH = 16; - const PRIV_CRYPTO_LENGTH = 64; - /** * @var \Illuminate\Database\ConnectionInterface */ - protected $connection; - - /** - * @var \Illuminate\Contracts\Encryption\Encrypter - */ - protected $encrypter; + private $connection; /** * @var \Pterodactyl\Services\Api\PermissionService */ - protected $permissionService; + private $permissionService; /** * @var \Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface */ - protected $repository; + private $repository; /** * ApiKeyService constructor. * * @param \Pterodactyl\Contracts\Repository\ApiKeyRepositoryInterface $repository * @param \Illuminate\Database\ConnectionInterface $connection - * @param \Illuminate\Contracts\Encryption\Encrypter $encrypter * @param \Pterodactyl\Services\Api\PermissionService $permissionService */ public function __construct( ApiKeyRepositoryInterface $repository, ConnectionInterface $connection, - Encrypter $encrypter, PermissionService $permissionService ) { $this->repository = $repository; $this->connection = $connection; - $this->encrypter = $encrypter; $this->permissionService = $permissionService; } @@ -64,24 +46,17 @@ class KeyCreationService * @param array $data * @param array $permissions * @param array $administrative - * @return string + * @return \Pterodactyl\Models\APIKey * * @throws \Exception * @throws \Pterodactyl\Exceptions\Model\DataValidationException */ - public function handle(array $data, array $permissions, array $administrative = []) + public function handle(array $data, array $permissions, array $administrative = []): APIKey { - $publicKey = str_random(self::PUB_CRYPTO_LENGTH); - $secretKey = str_random(self::PRIV_CRYPTO_LENGTH); + $token = str_random(APIKey::KEY_LENGTH); + $data = array_merge($data, ['token' => $token]); - // Start a Transaction $this->connection->beginTransaction(); - - $data = array_merge($data, [ - 'public' => $publicKey, - 'secret' => $this->encrypter->encrypt($secretKey), - ]); - $instance = $this->repository->create($data, true, true); $nodes = $this->permissionService->getPermissions(); @@ -115,6 +90,6 @@ class KeyCreationService $this->connection->commit(); - return $secretKey; + return $instance; } } diff --git a/database/factories/ModelFactory.php b/database/factories/ModelFactory.php index 1df550e0..16c8d08b 100644 --- a/database/factories/ModelFactory.php +++ b/database/factories/ModelFactory.php @@ -221,3 +221,14 @@ $factory->define(Pterodactyl\Models\DaemonKey::class, function (Faker\Generator 'expires_at' => \Carbon\Carbon::now()->addMinutes(10)->toDateTimeString(), ]; }); + +$factory->define(Pterodactyl\Models\APIKey::class, function (Faker\Generator $faker) { + return [ + 'id' => $faker->unique()->randomNumber(), + 'user_id' => $faker->randomNumber(), + 'token' => str_random(Pterodactyl\Models\APIKey::KEY_LENGTH), + 'memo' => 'Test Function Key', + 'created_at' => \Carbon\Carbon::now()->toDateTimeString(), + 'updated_at' => \Carbon\Carbon::now()->toDateTimeString(), + ]; +}); diff --git a/resources/lang/en/base.php b/resources/lang/en/base.php index 9c7bd9d0..234452da 100644 --- a/resources/lang/en/base.php +++ b/resources/lang/en/base.php @@ -33,7 +33,7 @@ return [ 'header_sub' => 'Manage your API access keys.', 'list' => 'API Keys', 'create_new' => 'Create New API key', - 'keypair_created' => 'An API Key-Pair has been generated. Your API secret token is :token. Please take note of this key as it will not be displayed again.', + 'keypair_created' => 'An API Key-Pair has been generated and is listed below.', ], 'new' => [ 'header' => 'New API Key', diff --git a/resources/lang/en/strings.php b/resources/lang/en/strings.php index d98a7d01..c9983e64 100644 --- a/resources/lang/en/strings.php +++ b/resources/lang/en/strings.php @@ -32,7 +32,7 @@ return [ 'memo' => 'Memo', 'created' => 'Created', 'expires' => 'Expires', - 'public_key' => 'Public key', + 'public_key' => 'Token', 'api_access' => 'Api Access', 'never' => 'never', 'sign_out' => 'Sign out', diff --git a/resources/themes/pterodactyl/base/api/index.blade.php b/resources/themes/pterodactyl/base/api/index.blade.php index a07657c9..0d8fc0a3 100644 --- a/resources/themes/pterodactyl/base/api/index.blade.php +++ b/resources/themes/pterodactyl/base/api/index.blade.php @@ -20,12 +20,7 @@ @section('content')
-
- API functionality is disabled in this beta release. -
-
-

@lang('base.api.index.list')

@@ -44,7 +39,7 @@ @foreach ($keys as $key) - {{ $key->public }} + {{ $key->token }} {{ $key->memo }} {{ (new Carbon($key->created_at))->toDayDateTimeString() }} @@ -57,7 +52,7 @@ @endif - + @endforeach diff --git a/tests/Unit/Http/Controllers/Base/APIControllerTest.php b/tests/Unit/Http/Controllers/Base/APIControllerTest.php index 055cbd84..579fb43b 100644 --- a/tests/Unit/Http/Controllers/Base/APIControllerTest.php +++ b/tests/Unit/Http/Controllers/Base/APIControllerTest.php @@ -1,54 +1,34 @@ . - * - * 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); + } } diff --git a/tests/Unit/Services/Api/KeyCreationServiceTest.php b/tests/Unit/Services/Api/KeyCreationServiceTest.php index 159d6425..49e0a2fd 100644 --- a/tests/Unit/Services/Api/KeyCreationServiceTest.php +++ b/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); } }