From 4479d3bf197b0256284d2ab017b121ded35f225f Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 8 Apr 2017 12:05:29 -0400 Subject: [PATCH] Improved logic for handling permissions on API routes. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Still only partially implemented, however this method will allow the inclusion of data that is granted with servers (such as viewing more about the node, node location, allocations, etc) while still limiting someone from doing `?include=node.servers` and listing all servers when they don’t have list-servers as a permission. --- .../API/Admin/ServerController.php | 13 +----- app/Models/APIKey.php | 7 +++ app/Policies/APIKeyPolicy.php | 4 +- app/Providers/MacroServiceProvider.php | 23 +++++++++- .../Admin/AllocationTransformer.php | 20 +++++++-- .../Admin/LocationTransformer.php | 23 ++++++++++ app/Transformers/Admin/NodeTransformer.php | 27 ++++++++++++ app/Transformers/Admin/OptionTransformer.php | 23 ++++++++++ app/Transformers/Admin/PackTransformer.php | 23 ++++++++++ app/Transformers/Admin/ServerTransformer.php | 43 ++++++++++++++----- .../Admin/ServerVariableTransformer.php | 23 ++++++++++ app/Transformers/Admin/ServiceTransformer.php | 23 ++++++++++ .../Admin/ServiceVariableTransformer.php | 23 ++++++++++ app/Transformers/Admin/SubuserTransformer.php | 23 ++++++++++ app/Transformers/Admin/UserTransformer.php | 23 ++++++++++ config/debugbar.php | 4 +- 16 files changed, 296 insertions(+), 29 deletions(-) diff --git a/app/Http/Controllers/API/Admin/ServerController.php b/app/Http/Controllers/API/Admin/ServerController.php index 793af13ec..4fc4a3a14 100644 --- a/app/Http/Controllers/API/Admin/ServerController.php +++ b/app/Http/Controllers/API/Admin/ServerController.php @@ -45,7 +45,7 @@ class ServerController extends Controller return Fractal::create() ->collection($servers) - ->transformWith(new ServerTransformer) + ->transformWith(new ServerTransformer($request)) ->paginateWith(new IlluminatePaginatorAdapter($servers)) ->withResourceName('server') ->toArray(); @@ -62,20 +62,11 @@ class ServerController extends Controller $server = Server::findOrFail($id); $fractal = Fractal::create()->item($server); - // dd($request->user()->can('view-node', $request->apiKey())); - - // Have the api key model return a list of includes that would be allowed - // given the permissions they have aleady been granted? - // - // If someone has 'view-node' they would then be able to use ->parseIncludes(['*.node.*']); - // How that logic will work is beyond me currently, but should keep things - // fairly clean? - if ($request->input('include')) { $fractal->parseIncludes(explode(',', $request->input('include'))); } - return $fractal->transformWith(new ServerTransformer) + return $fractal->transformWith(new ServerTransformer($request)) ->withResourceName('server') ->toArray(); } diff --git a/app/Models/APIKey.php b/app/Models/APIKey.php index f560fda85..6ed73b7c2 100644 --- a/app/Models/APIKey.php +++ b/app/Models/APIKey.php @@ -28,6 +28,13 @@ use Illuminate\Database\Eloquent\Model; class APIKey extends Model { + /** + * Public key defined length used in verification methods. + * + * @var int + */ + const PUBLIC_KEY_LEN = 16; + /** * The table associated with the model. * diff --git a/app/Policies/APIKeyPolicy.php b/app/Policies/APIKeyPolicy.php index caeaae5df..10aee1e4d 100644 --- a/app/Policies/APIKeyPolicy.php +++ b/app/Policies/APIKeyPolicy.php @@ -42,7 +42,9 @@ class APIKeyPolicy */ private function checkPermission(User $user, Key $key, $permission) { - $permissions = Cache::remember('APIKeyPolicy.' . $user->uuid . $key->public, Carbon::now()->addSeconds(5), function () use ($key) { + // We don't tag this cache key with the user uuid because the key is already unique, + // and multiple users are not defiend for a single key. + $permissions = Cache::remember('APIKeyPolicy.' . $key->public, Carbon::now()->addSeconds(5), function () use ($key) { return $key->permissions()->get()->transform(function ($item) { return $item->permission; })->values(); diff --git a/app/Providers/MacroServiceProvider.php b/app/Providers/MacroServiceProvider.php index eb5a52c56..980634dd6 100644 --- a/app/Providers/MacroServiceProvider.php +++ b/app/Providers/MacroServiceProvider.php @@ -25,7 +25,10 @@ namespace Pterodactyl\Providers; use File; +use Cache; +use Carbon; use Request; +use Pterodactyl\Models\APIKey; use Illuminate\Support\ServiceProvider; class MacroServiceProvider extends ServiceProvider @@ -57,11 +60,27 @@ class MacroServiceProvider extends ServiceProvider $parts = explode('.', Request::bearerToken()); - if (count($parts) === 2) { - return \Pterodactyl\Models\APIKey::where('public', $parts[0])->first(); + if (count($parts) === 2 && strlen($parts[0]) === APIKey::PUBLIC_KEY_LEN) { + // Because the key itself isn't changing frequently, we simply cache this for + // 15 minutes to speed up the API and keep requests flowing. + return Cache::tags([ + 'ApiKeyMacro', + 'ApiKeyMacro:Key:' . $parts[0], + ])->remember('ApiKeyMacro.' . $parts[0], Carbon::now()->addMinutes(15), function() use ($parts) { + return APIKey::where('public', $parts[0])->first(); + }); } return false; }); + + Request::macro('apiKeyHasPermission', function($permission) { + $key = Request::apiKey(); + if (! $key) { + return false; + } + + return Request::user()->can($permission, $key); + }); } } diff --git a/app/Transformers/Admin/AllocationTransformer.php b/app/Transformers/Admin/AllocationTransformer.php index c7fa85724..e7bd15c36 100644 --- a/app/Transformers/Admin/AllocationTransformer.php +++ b/app/Transformers/Admin/AllocationTransformer.php @@ -24,6 +24,7 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\Allocation; use League\Fractal\TransformerAbstract; @@ -37,13 +38,26 @@ class AllocationTransformer extends TransformerAbstract protected $filter; /** - * Transformer constructor. + * The Illuminate Request object if provided. * - * @param bool|string $filter + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @param bool $filter * @return void */ - public function __construct($filter = false) + public function __construct($request = false, $filter = false) { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; $this->filter = $filter; } diff --git a/app/Transformers/Admin/LocationTransformer.php b/app/Transformers/Admin/LocationTransformer.php index f41ead289..86980917b 100644 --- a/app/Transformers/Admin/LocationTransformer.php +++ b/app/Transformers/Admin/LocationTransformer.php @@ -24,6 +24,7 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\Location; use League\Fractal\TransformerAbstract; @@ -39,6 +40,28 @@ class LocationTransformer extends TransformerAbstract 'servers', ]; + /** + * The Illuminate Request object if provided. + * + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @return void + */ + public function __construct($request = false) + { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; + } + /** * Return a generic transformed pack array. * diff --git a/app/Transformers/Admin/NodeTransformer.php b/app/Transformers/Admin/NodeTransformer.php index 779f96059..cc42719a9 100644 --- a/app/Transformers/Admin/NodeTransformer.php +++ b/app/Transformers/Admin/NodeTransformer.php @@ -24,6 +24,7 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\Node; use League\Fractal\TransformerAbstract; @@ -40,6 +41,28 @@ class NodeTransformer extends TransformerAbstract 'servers', ]; + /** + * The Illuminate Request object if provided. + * + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @return void + */ + public function __construct($request = false) + { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; + } + /** * Return a generic transformed pack array. * @@ -77,6 +100,10 @@ class NodeTransformer extends TransformerAbstract */ public function includeServers(Node $node) { + if ($this->request && ! $this->request->apiKeyHasPermission('list-servers')) { + return; + } + return $this->collection($node->servers, new ServerTransformer, 'server'); } } diff --git a/app/Transformers/Admin/OptionTransformer.php b/app/Transformers/Admin/OptionTransformer.php index bdc0b1baf..1d38b4a94 100644 --- a/app/Transformers/Admin/OptionTransformer.php +++ b/app/Transformers/Admin/OptionTransformer.php @@ -24,6 +24,7 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\ServiceOption; use League\Fractal\TransformerAbstract; @@ -41,6 +42,28 @@ class OptionTransformer extends TransformerAbstract 'variables', ]; + /** + * The Illuminate Request object if provided. + * + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @return void + */ + public function __construct($request = false) + { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; + } + /** * Return a generic transformed service option array. * diff --git a/app/Transformers/Admin/PackTransformer.php b/app/Transformers/Admin/PackTransformer.php index 309280fd1..50160744e 100644 --- a/app/Transformers/Admin/PackTransformer.php +++ b/app/Transformers/Admin/PackTransformer.php @@ -24,6 +24,7 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\Pack; use League\Fractal\TransformerAbstract; @@ -39,6 +40,28 @@ class PackTransformer extends TransformerAbstract 'servers', ]; + /** + * The Illuminate Request object if provided. + * + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @return void + */ + public function __construct($request = false) + { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; + } + /** * Return a generic transformed pack array. * diff --git a/app/Transformers/Admin/ServerTransformer.php b/app/Transformers/Admin/ServerTransformer.php index e17ba8ae5..f33afce76 100644 --- a/app/Transformers/Admin/ServerTransformer.php +++ b/app/Transformers/Admin/ServerTransformer.php @@ -24,6 +24,7 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\Server; use League\Fractal\TransformerAbstract; @@ -46,6 +47,28 @@ class ServerTransformer extends TransformerAbstract 'node', ]; + /** + * The Illuminate Request object if provided. + * + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @return void + */ + public function __construct($request = false) + { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; + } + /** * Return a generic transformed server array. * @@ -63,7 +86,7 @@ class ServerTransformer extends TransformerAbstract */ public function includeAllocations(Server $server) { - return $this->collection($server->allocations, new AllocationTransformer('server'), 'allocation'); + return $this->collection($server->allocations, new AllocationTransformer($this->request, 'server'), 'allocation'); } /** @@ -73,7 +96,7 @@ class ServerTransformer extends TransformerAbstract */ public function includeSubusers(Server $server) { - return $this->collection($server->subusers, new SubuserTransformer, 'subuser'); + return $this->collection($server->subusers, new SubuserTransformer($this->request), 'subuser'); } /** @@ -83,7 +106,7 @@ class ServerTransformer extends TransformerAbstract */ public function includeUser(Server $server) { - return $this->item($server->user, new UserTransformer, 'user'); + return $this->item($server->user, new UserTransformer($this->request), 'user'); } /** @@ -93,7 +116,7 @@ class ServerTransformer extends TransformerAbstract */ public function includePack(Server $server) { - return $this->item($server->pack, new PackTransformer, 'pack'); + return $this->item($server->pack, new PackTransformer($this->request), 'pack'); } /** @@ -103,7 +126,7 @@ class ServerTransformer extends TransformerAbstract */ public function includeService(Server $server) { - return $this->item($server->service, new ServiceTransformer, 'service'); + return $this->item($server->service, new ServiceTransformer($this->request), 'service'); } /** @@ -113,7 +136,7 @@ class ServerTransformer extends TransformerAbstract */ public function includeOption(Server $server) { - return $this->item($server->option, new OptionTransformer, 'option'); + return $this->item($server->option, new OptionTransformer($this->request), 'option'); } /** @@ -123,7 +146,7 @@ class ServerTransformer extends TransformerAbstract */ public function includeVariables(Server $server) { - return $this->collection($server->variables, new ServerVariableTransformer, 'server_variable'); + return $this->collection($server->variables, new ServerVariableTransformer($this->request), 'server_variable'); } /** @@ -133,16 +156,16 @@ class ServerTransformer extends TransformerAbstract */ public function includeLocation(Server $server) { - return $this->item($server->location, new LocationTransformer, 'location'); + return $this->item($server->location, new LocationTransformer($this->request), 'location'); } /** * Return a generic array with pack information for this server. * - * @return \Leauge\Fractal\Resource\Item + * @return \Leauge\Fractal\Resource\Item|void */ public function includeNode(Server $server) { - return $this->item($server->node, new NodeTransformer, 'node'); + return $this->item($server->node, new NodeTransformer($this->request), 'node'); } } diff --git a/app/Transformers/Admin/ServerVariableTransformer.php b/app/Transformers/Admin/ServerVariableTransformer.php index 9d2247418..3521e0840 100644 --- a/app/Transformers/Admin/ServerVariableTransformer.php +++ b/app/Transformers/Admin/ServerVariableTransformer.php @@ -24,6 +24,7 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\ServerVariable; use League\Fractal\TransformerAbstract; @@ -36,6 +37,28 @@ class ServerVariableTransformer extends TransformerAbstract */ protected $availableIncludes = ['parent']; + /** + * The Illuminate Request object if provided. + * + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @return void + */ + public function __construct($request = false) + { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; + } + /** * Return a generic transformed server variable array. * diff --git a/app/Transformers/Admin/ServiceTransformer.php b/app/Transformers/Admin/ServiceTransformer.php index eb1fb2ab1..15e8859b2 100644 --- a/app/Transformers/Admin/ServiceTransformer.php +++ b/app/Transformers/Admin/ServiceTransformer.php @@ -24,6 +24,7 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\Service; use League\Fractal\TransformerAbstract; @@ -40,6 +41,28 @@ class ServiceTransformer extends TransformerAbstract 'packs', ]; + /** + * The Illuminate Request object if provided. + * + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @return void + */ + public function __construct($request = false) + { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; + } + /** * Return a generic transformed service array. * diff --git a/app/Transformers/Admin/ServiceVariableTransformer.php b/app/Transformers/Admin/ServiceVariableTransformer.php index 1a15c7ee0..9f87a1a81 100644 --- a/app/Transformers/Admin/ServiceVariableTransformer.php +++ b/app/Transformers/Admin/ServiceVariableTransformer.php @@ -24,6 +24,7 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\ServiceVariable; use League\Fractal\TransformerAbstract; @@ -36,6 +37,28 @@ class ServiceVariableTransformer extends TransformerAbstract */ protected $availableIncludes = ['variables']; + /** + * The Illuminate Request object if provided. + * + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @return void + */ + public function __construct($request = false) + { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; + } + /** * Return a generic transformed server variable array. * diff --git a/app/Transformers/Admin/SubuserTransformer.php b/app/Transformers/Admin/SubuserTransformer.php index 1c76dcc99..ab95ceb27 100644 --- a/app/Transformers/Admin/SubuserTransformer.php +++ b/app/Transformers/Admin/SubuserTransformer.php @@ -24,12 +24,35 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\Subuser; use Pterodactyl\Models\Permission; use League\Fractal\TransformerAbstract; class SubuserTransformer extends TransformerAbstract { + /** + * The Illuminate Request object if provided. + * + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @return void + */ + public function __construct($request = false) + { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; + } + /** * Return a generic transformed subuser array. * diff --git a/app/Transformers/Admin/UserTransformer.php b/app/Transformers/Admin/UserTransformer.php index 75dd950a3..95e5bef09 100644 --- a/app/Transformers/Admin/UserTransformer.php +++ b/app/Transformers/Admin/UserTransformer.php @@ -24,11 +24,34 @@ namespace Pterodactyl\Transformers\Admin; +use Illuminate\Http\Request; use Pterodactyl\Models\User; use League\Fractal\TransformerAbstract; class UserTransformer extends TransformerAbstract { + /** + * The Illuminate Request object if provided. + * + * @var \Illuminate\Http\Request|bool + */ + protected $request; + + /** + * Setup request object for transformer. + * + * @param \Illuminate\Http\Request|bool $request + * @return void + */ + public function __construct($request = false) + { + if (! $request instanceof Request && $request !== false) { + throw new DisplayException('Request passed to constructor must be of type Request or false.'); + } + + $this->request = $request; + } + /** * Return a generic transformed subuser array. * diff --git a/config/debugbar.php b/config/debugbar.php index 4d9d8c45e..05e78c34c 100644 --- a/config/debugbar.php +++ b/config/debugbar.php @@ -28,7 +28,7 @@ return [ */ 'storage' => [ 'enabled' => true, - 'driver' => 'file', // redis, file, pdo + 'driver' => env('DEBUGBAR_DRIVER', 'file'), // redis, file, pdo 'path' => storage_path() . '/debugbar', // For file driver 'connection' => null, // Leave null for default connection (Redis/PDO) ], @@ -125,7 +125,7 @@ return [ 'enabled' => false, 'types' => ['SELECT', 'INSERT', 'UPDATE', 'DELETE'], // array('SELECT', 'INSERT', 'UPDATE', 'DELETE'); for MySQL 5.6.3+ ], - 'hints' => true, // Show hints for common mistakes + 'hints' => false, // Show hints for common mistakes ], 'mail' => [ 'full_log' => false,