From cb945b1f1361566106202873cd167ae4b58ab4c1 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Fri, 27 Mar 2020 16:57:49 -0700 Subject: [PATCH] Fix permissions handling; do not allow a subuser to assign permissions they do not have --- .../Api/Client/Servers/SubuserController.php | 4 +- .../Subusers/AbstractSubuserRequest.php | 63 --------- .../Servers/Subusers/DeleteSubuserRequest.php | 2 +- .../Servers/Subusers/GetSubuserRequest.php | 3 +- .../Servers/Subusers/StoreSubuserRequest.php | 3 +- .../Servers/Subusers/SubuserRequest.php | 131 ++++++++++++++++++ .../Servers/Subusers/UpdateSubuserRequest.php | 2 +- .../Subusers/SubuserCreationService.php | 7 - 8 files changed, 137 insertions(+), 78 deletions(-) delete mode 100644 app/Http/Requests/Api/Client/Servers/Subusers/AbstractSubuserRequest.php create mode 100644 app/Http/Requests/Api/Client/Servers/Subusers/SubuserRequest.php diff --git a/app/Http/Controllers/Api/Client/Servers/SubuserController.php b/app/Http/Controllers/Api/Client/Servers/SubuserController.php index 79a90d84..ccfea5e6 100644 --- a/app/Http/Controllers/Api/Client/Servers/SubuserController.php +++ b/app/Http/Controllers/Api/Client/Servers/SubuserController.php @@ -91,7 +91,7 @@ class SubuserController extends ClientApiController */ public function update(UpdateSubuserRequest $request, Server $server): array { - $subuser = $request->subuser(); + $subuser = $request->endpointSubuser(); $this->repository->update($subuser->id, [ 'permissions' => $this->getDefaultPermissions($request), ]); @@ -110,7 +110,7 @@ class SubuserController extends ClientApiController */ public function delete(DeleteSubuserRequest $request, Server $server) { - $this->repository->delete($request->subuser()->id); + $this->repository->delete($request->endpointSubuser()->id); return JsonResponse::create([], JsonResponse::HTTP_NO_CONTENT); } diff --git a/app/Http/Requests/Api/Client/Servers/Subusers/AbstractSubuserRequest.php b/app/Http/Requests/Api/Client/Servers/Subusers/AbstractSubuserRequest.php deleted file mode 100644 index 0782aad6..00000000 --- a/app/Http/Requests/Api/Client/Servers/Subusers/AbstractSubuserRequest.php +++ /dev/null @@ -1,63 +0,0 @@ -subuser()->user_id === $this->user()->id) { - return false; - } - - return true; - } - - /** - * Return the subuser model for the given request which can then be validated. If - * required request parameters are missing a 404 error will be returned, otherwise - * a model exception will be returned if the model is not found. - * - * @return \Pterodactyl\Models\Subuser - * - * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException - * @throws \Illuminate\Database\Eloquent\ModelNotFoundException - */ - public function subuser() - { - /** @var \Pterodactyl\Repositories\Eloquent\SubuserRepository $repository */ - $repository = $this->container->make(SubuserRepository::class); - - $parameters = $this->route()->parameters(); - if ( - ! isset($parameters['server'], $parameters['server']) - || ! is_string($parameters['subuser']) - || ! $parameters['server'] instanceof Server - ) { - throw new NotFoundHttpException; - } - - return $this->model ?: $this->model = $repository->getUserForServer( - $parameters['server']->id, $parameters['subuser'] - ); - } -} diff --git a/app/Http/Requests/Api/Client/Servers/Subusers/DeleteSubuserRequest.php b/app/Http/Requests/Api/Client/Servers/Subusers/DeleteSubuserRequest.php index e9d05e2c..ef0a09aa 100644 --- a/app/Http/Requests/Api/Client/Servers/Subusers/DeleteSubuserRequest.php +++ b/app/Http/Requests/Api/Client/Servers/Subusers/DeleteSubuserRequest.php @@ -4,7 +4,7 @@ namespace Pterodactyl\Http\Requests\Api\Client\Servers\Subusers; use Pterodactyl\Models\Permission; -class DeleteSubuserRequest extends AbstractSubuserRequest +class DeleteSubuserRequest extends SubuserRequest { /** * @return string diff --git a/app/Http/Requests/Api/Client/Servers/Subusers/GetSubuserRequest.php b/app/Http/Requests/Api/Client/Servers/Subusers/GetSubuserRequest.php index e4d548f9..3affcbac 100644 --- a/app/Http/Requests/Api/Client/Servers/Subusers/GetSubuserRequest.php +++ b/app/Http/Requests/Api/Client/Servers/Subusers/GetSubuserRequest.php @@ -3,9 +3,8 @@ namespace Pterodactyl\Http\Requests\Api\Client\Servers\Subusers; use Pterodactyl\Models\Permission; -use Pterodactyl\Http\Requests\Api\Client\ClientApiRequest; -class GetSubuserRequest extends ClientApiRequest +class GetSubuserRequest extends SubuserRequest { /** * Confirm that a user is able to view subusers for the specified server. diff --git a/app/Http/Requests/Api/Client/Servers/Subusers/StoreSubuserRequest.php b/app/Http/Requests/Api/Client/Servers/Subusers/StoreSubuserRequest.php index 14fb7c43..effc3bf3 100644 --- a/app/Http/Requests/Api/Client/Servers/Subusers/StoreSubuserRequest.php +++ b/app/Http/Requests/Api/Client/Servers/Subusers/StoreSubuserRequest.php @@ -3,9 +3,8 @@ namespace Pterodactyl\Http\Requests\Api\Client\Servers\Subusers; use Pterodactyl\Models\Permission; -use Pterodactyl\Http\Requests\Api\Client\ClientApiRequest; -class StoreSubuserRequest extends ClientApiRequest +class StoreSubuserRequest extends SubuserRequest { /** * @return string diff --git a/app/Http/Requests/Api/Client/Servers/Subusers/SubuserRequest.php b/app/Http/Requests/Api/Client/Servers/Subusers/SubuserRequest.php new file mode 100644 index 00000000..d8c07936 --- /dev/null +++ b/app/Http/Requests/Api/Client/Servers/Subusers/SubuserRequest.php @@ -0,0 +1,131 @@ +route()->hasParameter('subuser')) { + if ($this->endpointSubuser()->user_id === $this->user()->id) { + return false; + } + } + + // If this is a POST request, validate that the user can even assign the permissions they + // have selected to assign. + if ($this->method() === Request::METHOD_POST && $this->has('permissions')) { + $this->validatePermissionsCanBeAssigned( + $this->input('permissions') ?? [] + ); + } + + return true; + } + + /** + * Validates that the permissions we are trying to assign can actually be assigned + * by the user making the request. + * + * @param array $permissions + */ + protected function validatePermissionsCanBeAssigned(array $permissions) + { + $user = $this->user(); + /** @var \Pterodactyl\Models\Server $server */ + $server = $this->route()->parameter('server'); + + // If we are a root admin or the server owner, no need to perform these checks. + if ($user->root_admin || $user->id === $server->owner_id) { + return; + } + + // Otherwise, get the current subuser's permission set, and ensure that the + // permissions they are trying to assign are not _more_ than the ones they + // already have. + if (count(array_diff($permissions, $this->currentUserPermissions())) > 0) { + throw new HttpForbiddenException( + 'Cannot assign permissions to a subuser that your account does not actively possess.' + ); + } + } + + /** + * Returns the currently authenticated user's permissions. + * + * @return array + */ + public function currentUserPermissions(): array + { + /** @var \Pterodactyl\Repositories\Eloquent\SubuserRepository $repository */ + $repository = $this->container->make(SubuserRepository::class); + + /* @var \Pterodactyl\Models\Subuser $model */ + try { + $model = $repository->findFirstWhere([ + ['server_id', $this->route()->parameter('server')->id], + ['user_id' => $this->user()->id], + ]); + } catch (RecordNotFoundException $exception) { + return []; + } + + return $model->permissions; + } + + /** + * Return the subuser model for the given request which can then be validated. If + * required request parameters are missing a 404 error will be returned, otherwise + * a model exception will be returned if the model is not found. + * + * This returns the subuser based on the endpoint being hit, not the actual subuser + * for the account making the request. + * + * @return \Pterodactyl\Models\Subuser + * + * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + * @throws \Illuminate\Database\Eloquent\ModelNotFoundException + */ + public function endpointSubuser() + { + /** @var \Pterodactyl\Repositories\Eloquent\SubuserRepository $repository */ + $repository = $this->container->make(SubuserRepository::class); + + $parameters = $this->route()->parameters(); + if ( + ! isset($parameters['server'], $parameters['server']) + || ! is_string($parameters['subuser']) + || ! $parameters['server'] instanceof Server + ) { + throw new NotFoundHttpException; + } + + return $this->model ?: $this->model = $repository->getUserForServer( + $parameters['server']->id, $parameters['subuser'] + ); + } +} diff --git a/app/Http/Requests/Api/Client/Servers/Subusers/UpdateSubuserRequest.php b/app/Http/Requests/Api/Client/Servers/Subusers/UpdateSubuserRequest.php index 41cdeb5a..3a84a027 100644 --- a/app/Http/Requests/Api/Client/Servers/Subusers/UpdateSubuserRequest.php +++ b/app/Http/Requests/Api/Client/Servers/Subusers/UpdateSubuserRequest.php @@ -4,7 +4,7 @@ namespace Pterodactyl\Http\Requests\Api\Client\Servers\Subusers; use Pterodactyl\Models\Permission; -class UpdateSubuserRequest extends AbstractSubuserRequest +class UpdateSubuserRequest extends SubuserRequest { /** * @return string diff --git a/app/Services/Subusers/SubuserCreationService.php b/app/Services/Subusers/SubuserCreationService.php index b959aece..e31e8ca8 100644 --- a/app/Services/Subusers/SubuserCreationService.php +++ b/app/Services/Subusers/SubuserCreationService.php @@ -1,11 +1,4 @@ . - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Pterodactyl\Services\Subusers;