From a31e5875dcb545a7c36739870992671549d15747 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Thu, 11 Jan 2018 22:49:46 -0600 Subject: [PATCH] First round of changes to API to support simpler permissions. --- .../Repository/ApiKeyRepositoryInterface.php | 1 + .../API/Admin/Users/UserController.php | 18 ++-- app/Http/Kernel.php | 4 - .../API/HasPermissionToResource.php | 58 ----------- .../Requests/API/Admin/ApiAdminRequest.php | 98 +++++++++++++++++++ .../API/Admin/Users/GetUserRequest.php | 32 ++++++ .../API/Admin/Users/GetUsersRequest.php | 19 ++++ .../Remote/SftpAuthenticationFormRequest.php | 4 +- app/Http/Requests/Request.php | 9 -- app/Models/APIKey.php | 45 +++++++-- app/Providers/RouteServiceProvider.php | 6 +- .../Eloquent/ApiKeyRepository.php | 1 + app/Services/Acl/Api/AdminAcl.php | 66 +++++++++++++ .../Api/Admin/AllocationTransformer.php | 4 +- .../Api/Admin/BaseTransformer.php | 69 +++++++++++++ .../Api/Admin/LocationTransformer.php | 4 +- .../Api/Admin/NodeTransformer.php | 4 +- .../Api/Admin/UserTransformer.php | 14 ++- app/Transformers/Api/ApiTransformer.php | 60 ------------ ...1_11_213943_AddApiKeyPermissionColumns.php | 50 ++++++++++ routes/api-admin.php | 6 ++ 21 files changed, 403 insertions(+), 169 deletions(-) delete mode 100644 app/Http/Middleware/API/HasPermissionToResource.php create mode 100644 app/Http/Requests/API/Admin/ApiAdminRequest.php create mode 100644 app/Http/Requests/API/Admin/Users/GetUserRequest.php create mode 100644 app/Http/Requests/API/Admin/Users/GetUsersRequest.php delete mode 100644 app/Http/Requests/Request.php create mode 100644 app/Services/Acl/Api/AdminAcl.php create mode 100644 app/Transformers/Api/Admin/BaseTransformer.php delete mode 100644 app/Transformers/Api/ApiTransformer.php create mode 100644 database/migrations/2018_01_11_213943_AddApiKeyPermissionColumns.php diff --git a/app/Contracts/Repository/ApiKeyRepositoryInterface.php b/app/Contracts/Repository/ApiKeyRepositoryInterface.php index 2fce09cd..c9fcb192 100644 --- a/app/Contracts/Repository/ApiKeyRepositoryInterface.php +++ b/app/Contracts/Repository/ApiKeyRepositoryInterface.php @@ -18,6 +18,7 @@ interface ApiKeyRepositoryInterface extends RepositoryInterface * * @param \Pterodactyl\Models\APIKey $model * @param bool $refresh + * @deprecated * @return \Pterodactyl\Models\APIKey */ public function loadPermissions(APIKey $model, bool $refresh = false): APIKey; diff --git a/app/Http/Controllers/API/Admin/Users/UserController.php b/app/Http/Controllers/API/Admin/Users/UserController.php index d2cd65d5..9801fa6e 100644 --- a/app/Http/Controllers/API/Admin/Users/UserController.php +++ b/app/Http/Controllers/API/Admin/Users/UserController.php @@ -15,6 +15,8 @@ use Pterodactyl\Http\Requests\Admin\UserFormRequest; use Pterodactyl\Transformers\Api\Admin\UserTransformer; use League\Fractal\Pagination\IlluminatePaginatorAdapter; use Pterodactyl\Contracts\Repository\UserRepositoryInterface; +use Pterodactyl\Http\Requests\API\Admin\Users\GetUserRequest; +use Pterodactyl\Http\Requests\API\Admin\Users\GetUsersRequest; class UserController extends Controller { @@ -67,19 +69,19 @@ class UserController extends Controller } /** - * Handle request to list all users on the panel. Returns a JSONAPI representation + * Handle request to list all users on the panel. Returns a JSON-API representation * of a collection of users including any defined relations passed in * the request. * - * @param \Illuminate\Http\Request $request + * @param \Pterodactyl\Http\Requests\API\Admin\Users\GetUsersRequest $request * @return array */ - public function index(Request $request): array + public function index(GetUsersRequest $request): array { $users = $this->repository->paginated(100); return $this->fractal->collection($users) - ->transformWith(new UserTransformer($request)) + ->transformWith((new UserTransformer)->setKey($request->key())) ->withResourceName('user') ->paginateWith(new IlluminatePaginatorAdapter($users)) ->toArray(); @@ -89,14 +91,14 @@ class UserController extends Controller * Handle a request to view a single user. Includes any relations that * were defined in the request. * - * @param \Illuminate\Http\Request $request - * @param \Pterodactyl\Models\User $user + * @param \Pterodactyl\Http\Requests\API\Admin\Users\GetUserRequest $request + * @param \Pterodactyl\Models\User $user * @return array */ - public function view(Request $request, User $user): array + public function view(GetUserRequest $request, User $user): array { return $this->fractal->item($user) - ->transformWith(new UserTransformer($request)) + ->transformWith((new UserTransformer)->setKey($request->key())) ->withResourceName('user') ->toArray(); } diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 9f418488..58772699 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -25,7 +25,6 @@ use Pterodactyl\Http\Middleware\API\AuthenticateIPAccess; use Pterodactyl\Http\Middleware\Daemon\DaemonAuthenticate; use Illuminate\Foundation\Http\Middleware\ValidatePostSize; use Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse; -use Pterodactyl\Http\Middleware\API\HasPermissionToResource; use Pterodactyl\Http\Middleware\Server\AuthenticateAsSubuser; use Pterodactyl\Http\Middleware\Server\SubuserBelongsToServer; use Pterodactyl\Http\Middleware\RequireTwoFactorAuthentication; @@ -98,9 +97,6 @@ class Kernel extends HttpKernel 'bindings' => SubstituteBindings::class, 'recaptcha' => VerifyReCaptcha::class, - // API specific middleware. - 'api..user_level' => HasPermissionToResource::class, - // Server specific middleware (used for authenticating access to resources) // // These are only used for individual server authentication, and not gloabl diff --git a/app/Http/Middleware/API/HasPermissionToResource.php b/app/Http/Middleware/API/HasPermissionToResource.php deleted file mode 100644 index 1d99ffbf..00000000 --- a/app/Http/Middleware/API/HasPermissionToResource.php +++ /dev/null @@ -1,58 +0,0 @@ -repository = $repository; - } - - /** - * Determine if an API key has permission to access the given route. - * - * @param \Illuminate\Http\Request $request - * @param \Closure $next - * @param string $role - * @return mixed - */ - public function handle(Request $request, Closure $next, string $role = 'admin') - { - /** @var \Pterodactyl\Models\APIKey $model */ - $model = $request->attributes->get('api_key'); - - if ($role === 'admin' && ! $request->user()->root_admin) { - throw new NotFoundHttpException; - } - - $this->repository->loadPermissions($model); - $routeKey = str_replace(['api.', 'admin.'], '', $request->route()->getName()); - - $count = $model->getRelation('permissions')->filter(function ($permission) use ($routeKey) { - return $routeKey === str_replace('-', '.', $permission->permission); - })->count(); - - if ($count === 1) { - return $next($request); - } - - throw new AccessDeniedHttpException('Cannot access resource without required `' . $routeKey . '` permission.'); - } -} diff --git a/app/Http/Requests/API/Admin/ApiAdminRequest.php b/app/Http/Requests/API/Admin/ApiAdminRequest.php new file mode 100644 index 00000000..f9f9a063 --- /dev/null +++ b/app/Http/Requests/API/Admin/ApiAdminRequest.php @@ -0,0 +1,98 @@ +resource)) { + throw new PterodactylException('An ACL resource must be defined on API requests.'); + } + + return Acl::check($this->key(), $this->resource, $this->permission); + } + + /** + * Determine if the requested resource exists on the server. + * + * @return bool + */ + public function resourceExists(): bool + { + return true; + } + + /** + * Default set of rules to apply to API requests. + * + * @return array + */ + public function rules(): array + { + return []; + } + + /** + * Return the API key being used for the request. + * + * @return \Pterodactyl\Models\APIKey + */ + public function key(): APIKey + { + return $this->attributes->get('api_key'); + } + + /** + * Determine if the request passes the authorization check as well + * as the exists check. + * + * @return bool + * + * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException + */ + protected function passesAuthorization() + { + $passes = parent::passesAuthorization(); + + // Only let the user know that a resource does not exist if they are + // authenticated to access the endpoint. This avoids exposing that + // an item exists (or does not exist) to the user until they can prove + // that they have permission to know about it. + if ($passes && ! $this->resourceExists()) { + throw new NotFoundHttpException('The requested resource does not exist on this server.'); + } + + return $passes; + } +} diff --git a/app/Http/Requests/API/Admin/Users/GetUserRequest.php b/app/Http/Requests/API/Admin/Users/GetUserRequest.php new file mode 100644 index 00000000..a699adcb --- /dev/null +++ b/app/Http/Requests/API/Admin/Users/GetUserRequest.php @@ -0,0 +1,32 @@ +route()->parameter('user'); + + return $user instanceof User && $user->exists; + } +} diff --git a/app/Http/Requests/API/Admin/Users/GetUsersRequest.php b/app/Http/Requests/API/Admin/Users/GetUsersRequest.php new file mode 100644 index 00000000..a7cd4417 --- /dev/null +++ b/app/Http/Requests/API/Admin/Users/GetUsersRequest.php @@ -0,0 +1,19 @@ +. - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Pterodactyl\Models; use Sofa\Eloquence\Eloquence; use Sofa\Eloquence\Validable; use Illuminate\Database\Eloquent\Model; +use Pterodactyl\Services\Acl\Api\AdminAcl; use Sofa\Eloquence\Contracts\CleansAttributes; use Sofa\Eloquence\Contracts\Validable as ValidableContract; @@ -35,14 +29,29 @@ class APIKey extends Model implements CleansAttributes, ValidableContract */ protected $casts = [ 'allowed_ips' => 'json', + 'user_id' => 'int', + 'r_' . AdminAcl::RESOURCE_USERS => 'int', + 'r_' . AdminAcl::RESOURCE_ALLOCATIONS => 'int', + 'r_' . AdminAcl::RESOURCE_DATABASES => 'int', + 'r_' . AdminAcl::RESOURCE_EGGS => 'int', + 'r_' . AdminAcl::RESOURCE_LOCATIONS => 'int', + 'r_' . AdminAcl::RESOURCE_NESTS => 'int', + 'r_' . AdminAcl::RESOURCE_NODES => 'int', + 'r_' . AdminAcl::RESOURCE_PACKS => 'int', + 'r_' . AdminAcl::RESOURCE_SERVERS => 'int', ]; /** - * Fields that are not mass assignable. + * Fields that are mass assignable. * * @var array */ - protected $guarded = ['id', 'created_at', 'updated_at']; + protected $fillable = [ + 'token', + 'allowed_ips', + 'memo', + 'expires_at', + ]; /** * Rules defining what fields must be passed when making a model. @@ -66,6 +75,24 @@ class APIKey extends Model implements CleansAttributes, ValidableContract 'memo' => 'nullable|string|max:500', 'allowed_ips' => 'nullable|json', 'expires_at' => 'nullable|datetime', + 'r_' . AdminAcl::RESOURCE_USERS => 'integer|min:0|max:3', + 'r_' . AdminAcl::RESOURCE_ALLOCATIONS => 'integer|min:0|max:3', + 'r_' . AdminAcl::RESOURCE_DATABASES => 'integer|min:0|max:3', + 'r_' . AdminAcl::RESOURCE_EGGS => 'integer|min:0|max:3', + 'r_' . AdminAcl::RESOURCE_LOCATIONS => 'integer|min:0|max:3', + 'r_' . AdminAcl::RESOURCE_NESTS => 'integer|min:0|max:3', + 'r_' . AdminAcl::RESOURCE_NODES => 'integer|min:0|max:3', + 'r_' . AdminAcl::RESOURCE_PACKS => 'integer|min:0|max:3', + 'r_' . AdminAcl::RESOURCE_SERVERS => 'integer|min:0|max:3', + ]; + + /** + * @var array + */ + protected $dates = [ + self::CREATED_AT, + self::UPDATED_AT, + 'expires_at', ]; /** diff --git a/app/Providers/RouteServiceProvider.php b/app/Providers/RouteServiceProvider.php index a0f90285..5849bd98 100644 --- a/app/Providers/RouteServiceProvider.php +++ b/app/Providers/RouteServiceProvider.php @@ -29,10 +29,6 @@ class RouteServiceProvider extends ServiceProvider */ public function map() { -// Route::middleware(['api'])->prefix('/api/user') -// ->namespace($this->namespace . '\API\User') -// ->group(base_path('routes/api.php')); - Route::middleware(['web', 'auth', 'csrf']) ->namespace($this->namespace . '\Base') ->group(base_path('routes/base.php')); @@ -49,7 +45,7 @@ class RouteServiceProvider extends ServiceProvider ->namespace($this->namespace . '\Server') ->group(base_path('routes/server.php')); - Route::middleware(['api', 'api..user_level:admin'])->prefix('/api/admin') + Route::middleware(['api'])->prefix('/api/admin') ->namespace($this->namespace . '\API\Admin') ->group(base_path('routes/api-admin.php')); diff --git a/app/Repositories/Eloquent/ApiKeyRepository.php b/app/Repositories/Eloquent/ApiKeyRepository.php index f6169c93..f62499b2 100644 --- a/app/Repositories/Eloquent/ApiKeyRepository.php +++ b/app/Repositories/Eloquent/ApiKeyRepository.php @@ -22,6 +22,7 @@ class ApiKeyRepository extends EloquentRepository implements ApiKeyRepositoryInt * * @param \Pterodactyl\Models\APIKey $model * @param bool $refresh + * @deprecated * @return \Pterodactyl\Models\APIKey */ public function loadPermissions(APIKey $model, bool $refresh = false): APIKey diff --git a/app/Services/Acl/Api/AdminAcl.php b/app/Services/Acl/Api/AdminAcl.php new file mode 100644 index 00000000..3416c1a4 --- /dev/null +++ b/app/Services/Acl/Api/AdminAcl.php @@ -0,0 +1,66 @@ +key = $key; + + return $this; + } + + /** + * Return the request instance being used for this transformer. + * + * @return \Pterodactyl\Models\APIKey + */ + public function getKey(): APIKey + { + return $this->key; + } + + /** + * Determine if the API key loaded onto the transformer has permission + * to access a different resource. This is used when including other + * models on a transformation request. + * + * @param string $resource + * @return bool + */ + protected function authorize(string $resource): bool + { + return AdminAcl::check($this->getKey(), $resource, AdminAcl::READ); + } + + /** + * Create a new instance of the transformer and pass along the currently + * set API key. + * + * @param string $abstract + * @param array $parameters + * @return \Pterodactyl\Transformers\Api\Admin\BaseTransformer + */ + protected function makeTransformer(string $abstract, array $parameters = []): self + { + /** @var \Pterodactyl\Transformers\Api\Admin\BaseTransformer $transformer */ + $transformer = Container::getInstance()->makeWith($abstract, $parameters); + $transformer->setKey($this->getKey()); + + return $transformer; + } +} diff --git a/app/Transformers/Api/Admin/LocationTransformer.php b/app/Transformers/Api/Admin/LocationTransformer.php index 27a4ae9f..7d4b9fff 100644 --- a/app/Transformers/Api/Admin/LocationTransformer.php +++ b/app/Transformers/Api/Admin/LocationTransformer.php @@ -3,9 +3,9 @@ namespace Pterodactyl\Transformers\Api\Admin; use Pterodactyl\Models\Location; -use Pterodactyl\Transformers\Api\ApiTransformer; +use Pterodactyl\Transformers\Api\BaseTransformer; -class LocationTransformer extends ApiTransformer +class LocationTransformer extends BaseTransformer { /** * List of resources that can be included. diff --git a/app/Transformers/Api/Admin/NodeTransformer.php b/app/Transformers/Api/Admin/NodeTransformer.php index b6502339..e4630699 100644 --- a/app/Transformers/Api/Admin/NodeTransformer.php +++ b/app/Transformers/Api/Admin/NodeTransformer.php @@ -3,9 +3,9 @@ namespace Pterodactyl\Transformers\Api\Admin; use Pterodactyl\Models\Node; -use Pterodactyl\Transformers\Api\ApiTransformer; +use Pterodactyl\Transformers\Api\BaseTransformer; -class NodeTransformer extends ApiTransformer +class NodeTransformer extends BaseTransformer { /** * List of resources that can be included. diff --git a/app/Transformers/Api/Admin/UserTransformer.php b/app/Transformers/Api/Admin/UserTransformer.php index 292818b3..54a38469 100644 --- a/app/Transformers/Api/Admin/UserTransformer.php +++ b/app/Transformers/Api/Admin/UserTransformer.php @@ -3,9 +3,9 @@ namespace Pterodactyl\Transformers\Api\Admin; use Pterodactyl\Models\User; -use Pterodactyl\Transformers\Api\ApiTransformer; +use Pterodactyl\Services\Acl\Api\AdminAcl; -class UserTransformer extends ApiTransformer +class UserTransformer extends BaseTransformer { /** * List of resources that can be included. @@ -29,18 +29,16 @@ class UserTransformer extends ApiTransformer * Return the servers associated with this user. * * @param \Pterodactyl\Models\User $user - * @return bool|\League\Fractal\Resource\Collection - * - * @throws \Pterodactyl\Exceptions\PterodactylException + * @return \League\Fractal\Resource\Collection|\League\Fractal\Resource\NullResource */ public function includeServers(User $user) { - if (! $this->authorize('server-list')) { - return false; + if (! $this->authorize(AdminAcl::RESOURCE_SERVERS)) { + return $this->null(); } $user->loadMissing('servers'); - return $this->collection($user->getRelation('servers'), new ServerTransformer($this->getRequest()), 'server'); + return $this->collection($user->getRelation('servers'), $this->makeTransformer(ServerTransformer::class), 'server'); } } diff --git a/app/Transformers/Api/ApiTransformer.php b/app/Transformers/Api/ApiTransformer.php deleted file mode 100644 index 3620af4d..00000000 --- a/app/Transformers/Api/ApiTransformer.php +++ /dev/null @@ -1,60 +0,0 @@ -request = $request; - } - - /** - * Return the request instance being used for this transformer. - * - * @return \Illuminate\Http\Request - */ - public function getRequest(): Request - { - return $this->request; - } - - /** - * Determine if an API key from the request has permission to access - * a resource. This is used when loading includes on the transformed - * models. - * - * @param string $permission - * @return bool - * - * @throws \Pterodactyl\Exceptions\PterodactylException - */ - protected function authorize(string $permission): bool - { - /** @var \Pterodactyl\Models\APIKey $model */ - $model = $this->request->attributes->get('api_key'); - if (! $model->relationLoaded('permissions')) { - throw new PterodactylException('Permissions must be loaded onto a model before passing to transformer authorize function.'); - } - - $count = $model->getRelation('permissions')->filter(function ($model) use ($permission) { - return $model->permission === $permission; - })->count(); - - return $count > 0; - } -} diff --git a/database/migrations/2018_01_11_213943_AddApiKeyPermissionColumns.php b/database/migrations/2018_01_11_213943_AddApiKeyPermissionColumns.php new file mode 100644 index 00000000..e2c78d2e --- /dev/null +++ b/database/migrations/2018_01_11_213943_AddApiKeyPermissionColumns.php @@ -0,0 +1,50 @@ +unsignedTinyInteger('r_servers')->default(0); + $table->unsignedTinyInteger('r_nodes')->default(0); + $table->unsignedTinyInteger('r_allocations')->default(0); + $table->unsignedTinyInteger('r_users')->default(0); + $table->unsignedTinyInteger('r_locations')->default(0); + $table->unsignedTinyInteger('r_nests')->default(0); + $table->unsignedTinyInteger('r_eggs')->default(0); + $table->unsignedTinyInteger('r_databases')->default(0); + $table->unsignedTinyInteger('r_packs')->default(0); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('api_keys', function (Blueprint $table) { + $table->dropColumn([ + 'r_servers', + 'r_nodes', + 'r_allocations', + 'r_users', + 'r_locations', + 'r_nests', + 'r_eggs', + 'r_databases', + 'r_packs', + ]); + }); + } +} diff --git a/routes/api-admin.php b/routes/api-admin.php index cd253b1b..2f51be62 100644 --- a/routes/api-admin.php +++ b/routes/api-admin.php @@ -1,5 +1,7 @@ '/users'], function () { + Route::bind('user', function ($value) { + return User::find($value) ?? new User; + }); + Route::get('/', 'Users\UserController@index')->name('api.admin.user.list'); Route::get('/{user}', 'Users\UserController@view')->name('api.admin.user.view');