From f91e4c511ec7061050f41997674d9d83dda11b1b Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Feb 2017 21:45:11 -0500 Subject: [PATCH 1/6] Attach user to cache to prevent showing servers they can't access. --- CHANGELOG.md | 4 ++++ app/Models/Server.php | 2 +- app/Observers/ServerObserver.php | 5 +++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c186e143..d90ec523e 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.6.0-pre.4 (Courageous Carniadactylus) +### Fixed +* `[pre.3]` — Fixes bug in cache handler that doesn't cache against the user making the request. Would have allowed for users to access servers not belonging to themselves in production. + ## v0.6.0-pre.3 (Courageous Carniadactylus) ### Fixed * `[pre.2]` — Fixes bug where servers could not be manually deployed to nodes due to a broken SQL call. diff --git a/app/Models/Server.php b/app/Models/Server.php index 8527e6a14..7c69c763f 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -96,7 +96,7 @@ class Server extends Model public static function byUuid($uuid) { // Results are cached because we call this functions a few times on page load. - $result = Cache::remember('Server.byUuid.' . $uuid, 60, function () use ($uuid) { + $result = Cache::remember('Server.byUuid.' . $uuid . Auth::user()->uuid, 60, function () use ($uuid) { $query = self::with('service', 'node')->where(function ($q) use ($uuid) { $q->where('uuidShort', $uuid)->orWhere('uuid', $uuid); }); diff --git a/app/Observers/ServerObserver.php b/app/Observers/ServerObserver.php index 556d1c165..5d919a559 100644 --- a/app/Observers/ServerObserver.php +++ b/app/Observers/ServerObserver.php @@ -24,6 +24,7 @@ namespace Pterodactyl\Observers; +use Auth; use Cache; use Carbon; use Pterodactyl\Events; @@ -141,8 +142,8 @@ class ServerObserver public function updated(Server $server) { // Clear Caches - Cache::forget('Server.byUuid.' . $server->uuid); - Cache::forget('Server.byUuid.' . $server->uuidShort); + Cache::forget('Server.byUuid.' . $server->uuid . Auth::user()->uuid); + Cache::forget('Server.byUuid.' . $server->uuidShort . Auth::user()->uuid); event(new Events\Server\Updated($server)); } From efdc3e6fd8f43165bb694cf8987b57baac0c3087 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Feb 2017 22:26:07 -0500 Subject: [PATCH 2/6] Add cache policy for ServerPolicy 10 second cache, just long enough to handle the page load without making more than one MySQL call. --- CHANGELOG.md | 3 +++ app/Policies/ServerPolicy.php | 11 ++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d90ec523e..82fc3ecfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ This project follows [Semantic Versioning](http://semver.org) guidelines. ### Fixed * `[pre.3]` — Fixes bug in cache handler that doesn't cache against the user making the request. Would have allowed for users to access servers not belonging to themselves in production. +### Added +* New cache policy for ServerPolicy to avoid making 15+ queries per page load when confirming if a user has permission to perform an action. + ## v0.6.0-pre.3 (Courageous Carniadactylus) ### Fixed * `[pre.2]` — Fixes bug where servers could not be manually deployed to nodes due to a broken SQL call. diff --git a/app/Policies/ServerPolicy.php b/app/Policies/ServerPolicy.php index d67f3aced..adb4f3850 100644 --- a/app/Policies/ServerPolicy.php +++ b/app/Policies/ServerPolicy.php @@ -24,11 +24,14 @@ namespace Pterodactyl\Policies; +use Cache; +use Carbon; use Pterodactyl\Models\User; use Pterodactyl\Models\Server; class ServerPolicy { + /** * Create a new policy instance. * @@ -53,7 +56,13 @@ class ServerPolicy return true; } - return $user->permissions()->server($server)->permission($permission)->exists(); + $permissions = Cache::remember('ServerPolicy.' . $user->uuid . $server->uuid, Carbon::now()->addSeconds(10), function () use ($user, $server) { + return $user->permissions()->server($server)->get()->transform(function ($item) { + return $item->permission; + })->values(); + }); + + return ($permissions->search($permission, true) !== false); } /** From 644c07ea3a847747c3f121e10adeec9a1de2b706 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Feb 2017 22:32:16 -0500 Subject: [PATCH 3/6] Fix broken port deletion --- CHANGELOG.md | 1 + app/Http/Controllers/Admin/NodesController.php | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82fc3ecfe..e88c0ec6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ This project follows [Semantic Versioning](http://semver.org) guidelines. ## v0.6.0-pre.4 (Courageous Carniadactylus) ### Fixed * `[pre.3]` — Fixes bug in cache handler that doesn't cache against the user making the request. Would have allowed for users to access servers not belonging to themselves in production. +* `[pre.3]` — Fixes misnamed MySQL column that was causing the inability to delete certain port ranges from the database. ### Added * New cache policy for ServerPolicy to avoid making 15+ queries per page load when confirming if a user has permission to perform an action. diff --git a/app/Http/Controllers/Admin/NodesController.php b/app/Http/Controllers/Admin/NodesController.php index f4a60b5b2..877c44995 100644 --- a/app/Http/Controllers/Admin/NodesController.php +++ b/app/Http/Controllers/Admin/NodesController.php @@ -163,7 +163,7 @@ class NodesController extends Controller public function deallocateBlock(Request $request, $node) { - $query = Models\Allocation::where('node', $node)->whereNull('server_id')->where('ip', $request->input('ip'))->delete(); + $query = Models\Allocation::where('node_id', $node)->whereNull('server_id')->where('ip', $request->input('ip'))->delete(); if ((int) $query === 0) { Alert::danger('There was an error while attempting to delete allocations on that IP.')->flash(); @@ -199,7 +199,7 @@ class NodesController extends Controller public function getAllocationsJson(Request $request, $id) { - $allocations = Models\Allocation::select('ip')->where('node', $id)->groupBy('ip')->get(); + $allocations = Models\Allocation::select('ip')->where('node_id', $id)->groupBy('ip')->get(); return response()->json($allocations); } From ed4068bdb9038d72e27158e5ba89cb62d837e5ed Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Feb 2017 22:33:15 -0500 Subject: [PATCH 4/6] Fix bug preventing server container rebuilds. --- CHANGELOG.md | 1 + app/Http/Controllers/Admin/ServersController.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e88c0ec6d..8b856fd7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ This project follows [Semantic Versioning](http://semver.org) guidelines. ### Fixed * `[pre.3]` — Fixes bug in cache handler that doesn't cache against the user making the request. Would have allowed for users to access servers not belonging to themselves in production. * `[pre.3]` — Fixes misnamed MySQL column that was causing the inability to delete certain port ranges from the database. +* `[pre.3]` — Fixes bug preventing rebuilding server containers through the Admin CP. ### Added * New cache policy for ServerPolicy to avoid making 15+ queries per page load when confirming if a user has permission to perform an action. diff --git a/app/Http/Controllers/Admin/ServersController.php b/app/Http/Controllers/Admin/ServersController.php index 0f5cf495d..017dc1fcc 100644 --- a/app/Http/Controllers/Admin/ServersController.php +++ b/app/Http/Controllers/Admin/ServersController.php @@ -251,7 +251,7 @@ class ServersController extends Controller try { $res = $server->node->guzzleClient([ 'X-Access-Server' => $server->uuid, - 'X-Access-Token' => $node->daemonSecret, + 'X-Access-Token' => $server->node->daemonSecret, ])->request('POST', '/server/rebuild'); Alert::success('A rebuild has been queued successfully. It will run the next time this server is booted.')->flash(); } catch (\GuzzleHttp\Exception\TransferException $ex) { From b11029a666b0491c600811f34fcd7deba6139cee Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Feb 2017 22:57:50 -0500 Subject: [PATCH 5/6] Apply fixes from StyleCI (#312) * Bump for release * Apply fixes from StyleCI --- app/Http/Controllers/Base/APIController.php | 1 + app/Policies/ServerPolicy.php | 3 +-- app/Repositories/APIRepository.php | 2 +- app/Repositories/Daemon/CommandRepository.php | 1 - app/Repositories/Daemon/PowerRepository.php | 1 - app/Repositories/ServerRepository.php | 4 ++-- app/Repositories/UserRepository.php | 5 ++--- config/app.php | 2 +- 8 files changed, 8 insertions(+), 11 deletions(-) diff --git a/app/Http/Controllers/Base/APIController.php b/app/Http/Controllers/Base/APIController.php index e80f07033..68045a722 100644 --- a/app/Http/Controllers/Base/APIController.php +++ b/app/Http/Controllers/Base/APIController.php @@ -80,6 +80,7 @@ class APIController extends Controller return response('', 204); } catch (\Exception $ex) { Log::error($ex); + return response()->json([ 'error' => 'An error occured while attempting to remove this key.', ], 503); diff --git a/app/Policies/ServerPolicy.php b/app/Policies/ServerPolicy.php index adb4f3850..767dec13c 100644 --- a/app/Policies/ServerPolicy.php +++ b/app/Policies/ServerPolicy.php @@ -31,7 +31,6 @@ use Pterodactyl\Models\Server; class ServerPolicy { - /** * Create a new policy instance. * @@ -62,7 +61,7 @@ class ServerPolicy })->values(); }); - return ($permissions->search($permission, true) !== false); + return $permissions->search($permission, true) !== false; } /** diff --git a/app/Repositories/APIRepository.php b/app/Repositories/APIRepository.php index c18f1f993..5f148eda6 100644 --- a/app/Repositories/APIRepository.php +++ b/app/Repositories/APIRepository.php @@ -225,7 +225,7 @@ class APIRepository try { $model = Models\APIKey::with('permissions')->where('public', $key)->where('user_id', $this->user->id)->firstOrFail(); - foreach($model->permissions as &$permission) { + foreach ($model->permissions as &$permission) { $permission->delete(); } diff --git a/app/Repositories/Daemon/CommandRepository.php b/app/Repositories/Daemon/CommandRepository.php index 1905478e7..76ac93178 100644 --- a/app/Repositories/Daemon/CommandRepository.php +++ b/app/Repositories/Daemon/CommandRepository.php @@ -24,7 +24,6 @@ namespace Pterodactyl\Repositories\Daemon; -use GuzzleHttp\Client; use Pterodactyl\Models; use GuzzleHttp\Exception\RequestException; use Pterodactyl\Exceptions\DisplayException; diff --git a/app/Repositories/Daemon/PowerRepository.php b/app/Repositories/Daemon/PowerRepository.php index d43ec1a7d..b31c0cda0 100644 --- a/app/Repositories/Daemon/PowerRepository.php +++ b/app/Repositories/Daemon/PowerRepository.php @@ -24,7 +24,6 @@ namespace Pterodactyl\Repositories\Daemon; -use GuzzleHttp\Client; use Pterodactyl\Models; use Pterodactyl\Exceptions\DisplayException; diff --git a/app/Repositories/ServerRepository.php b/app/Repositories/ServerRepository.php index b6047739d..4b5ed1f15 100644 --- a/app/Repositories/ServerRepository.php +++ b/app/Repositories/ServerRepository.php @@ -791,8 +791,8 @@ class ServerRepository Models\ServerVariable::where('server_id', $server->id)->delete(); // Remove SubUsers - foreach(Models\Subuser::with('permissions')->where('server_id', $server->id)->get() as &$subuser) { - foreach($subuser->permissions as &$permission) { + foreach (Models\Subuser::with('permissions')->where('server_id', $server->id)->get() as &$subuser) { + foreach ($subuser->permissions as &$permission) { $permission->delete(); } $subuser->delete(); diff --git a/app/Repositories/UserRepository.php b/app/Repositories/UserRepository.php index c59637783..e9cd8580f 100644 --- a/app/Repositories/UserRepository.php +++ b/app/Repositories/UserRepository.php @@ -34,7 +34,6 @@ use Validator; use Pterodactyl\Models; use Pterodactyl\Services\UuidService; use Pterodactyl\Exceptions\DisplayException; -use Pterodactyl\Notifications\AccountCreated; use Pterodactyl\Exceptions\DisplayValidationException; class UserRepository @@ -177,8 +176,8 @@ class UserRepository DB::beginTransaction(); try { - foreach(Models\Subuser::with('permissions')->where('user_id', $id)->get() as &$subuser) { - foreach($subuser->permissions as &$permission) { + foreach (Models\Subuser::with('permissions')->where('user_id', $id)->get() as &$subuser) { + foreach ($subuser->permissions as &$permission) { $permission->delete(); } diff --git a/config/app.php b/config/app.php index 622098a9b..e6b955156 100644 --- a/config/app.php +++ b/config/app.php @@ -4,7 +4,7 @@ return [ 'env' => env('APP_ENV', 'production'), - 'version' => env('APP_VERSION', 'canary'), + 'version' => env('APP_VERSION', '0.6.0-pre.4'), 'phrase_in_context' => env('PHRASE_IN_CONTEXT', false), From b82c67424ff9ed510467cab546213146e1f348f3 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Feb 2017 22:59:08 -0500 Subject: [PATCH 6/6] Slow ya roll StyleCI... --- config/app.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/app.php b/config/app.php index e6b955156..622098a9b 100644 --- a/config/app.php +++ b/config/app.php @@ -4,7 +4,7 @@ return [ 'env' => env('APP_ENV', 'production'), - 'version' => env('APP_VERSION', '0.6.0-pre.4'), + 'version' => env('APP_VERSION', 'canary'), 'phrase_in_context' => env('PHRASE_IN_CONTEXT', false),