Code cleanup for subuser API endpoints; closes #2247

app/Exceptions/Handler.php

@@ -213,6 +213,13 @@ class Handler extends ExceptionHandler
             'detail' => 'An error was encountered while processing this request.',
         ];
 
+        if ($exception instanceof ModelNotFoundException || $exception->getPrevious() instanceof ModelNotFoundException) {
+            // Show a nicer error message compared to the standard "No query results for model"
+            // response that is normally returned. If we are in debug mode this will get overwritten
+            // with a more specific error message to help narrow down things.
+            $error['detail'] = 'The requested resource could not be found on the server.';
+        }
+
         if (config('app.debug')) {
             $error = array_merge($error, [
                 'detail' => $exception->getMessage(),

app/Http/Controllers/Api/Client/Servers/SubuserController.php

@@ -3,7 +3,9 @@
 namespace Pterodactyl\Http\Controllers\Api\Client\Servers;
 
 use Illuminate\Http\Request;
+use Pterodactyl\Models\User;
 use Pterodactyl\Models\Server;
+use Pterodactyl\Models\Subuser;
 use Illuminate\Http\JsonResponse;
 use Pterodactyl\Models\Permission;
 use Pterodactyl\Repositories\Eloquent\SubuserRepository;
@@ -57,6 +59,21 @@ class SubuserController extends ClientApiController
             ->toArray();
     }
 
+    /**
+     * Returns a single subuser associated with this server instance.
+     *
+     * @param \Pterodactyl\Http\Requests\Api\Client\Servers\Subusers\GetSubuserRequest $request
+     * @return array
+     */
+    public function view(GetSubuserRequest $request)
+    {
+        $subuser = $request->attributes->get('subuser');
+
+        return $this->fractal->item($subuser)
+            ->transformWith($this->getTransformer(SubuserTransformer::class))
+            ->toArray();
+    }
+
     /**
      * Create a new subuser for the given server.
      *
@@ -84,15 +101,16 @@ class SubuserController extends ClientApiController
      * Update a given subuser in the system for the server.
      *
      * @param \Pterodactyl\Http\Requests\Api\Client\Servers\Subusers\UpdateSubuserRequest $request
-     * @param \Pterodactyl\Models\Server $server
      * @return array
      *
      * @throws \Pterodactyl\Exceptions\Model\DataValidationException
      * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
      */
-    public function update(UpdateSubuserRequest $request, Server $server): array
+    public function update(UpdateSubuserRequest $request): array
     {
-        $subuser = $request->endpointSubuser();
+        /** @var \Pterodactyl\Models\Subuser $subuser */
+        $subuser = $request->attributes->get('subuser');
+
         $this->repository->update($subuser->id, [
             'permissions' => $this->getDefaultPermissions($request),
         ]);
@@ -106,14 +124,16 @@ class SubuserController extends ClientApiController
      * Removes a subusers from a server's assignment.
      *
      * @param \Pterodactyl\Http\Requests\Api\Client\Servers\Subusers\DeleteSubuserRequest $request
-     * @param \Pterodactyl\Models\Server $server
      * @return \Illuminate\Http\JsonResponse
      */
-    public function delete(DeleteSubuserRequest $request, Server $server)
+    public function delete(DeleteSubuserRequest $request)
     {
-        $this->repository->delete($request->endpointSubuser()->id);
+        /** @var \Pterodactyl\Models\Subuser $subuser */
+        $subuser = $request->attributes->get('subuser');
 
-        return JsonResponse::create([], JsonResponse::HTTP_NO_CONTENT);
+        $this->repository->delete($subuser->id);
+
+        return new JsonResponse([], JsonResponse::HTTP_NO_CONTENT);
     }
 
     /**

app/Http/Middleware/Api/Client/Server/SubuserBelongsToServer.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace Pterodactyl\Http\Middleware\Api\Client\Server;
+
+use Closure;
+use Exception;
+use Illuminate\Http\Request;
+
+class SubuserBelongsToServer
+{
+    /**
+     * Ensure that the user being accessed in the request is a user that is currently assigned
+     * as a subuser for this server instance. We'll let the requests themselves handle wether or
+     * not the user making the request can actually modify or delete the subuser record.
+     *
+     * @param \Illuminate\Http\Request $request
+     * @param \Closure $next
+     *
+     * @return mixed
+     */
+    public function handle(Request $request, Closure $next)
+    {
+        /** @var \Pterodactyl\Models\Server $server */
+        $server = $request->route()->parameter('server');
+        /** @var \Pterodactyl\Models\User $user */
+        $user = $request->route()->parameter('user');
+
+        // Don't do anything if there isn't a user present in the request.
+        if (is_null($user)) {
+            return $next($request);
+        }
+
+        $request->attributes->set('subuser', $server->subusers()->where('user_id', $user->id)->firstOrFail());
+
+        return $next($request);
+    }
+}

app/Http/Middleware/Api/Client/SubstituteClientApiBindings.php

@@ -3,6 +3,7 @@
 namespace Pterodactyl\Http\Middleware\Api\Client;
 
 use Closure;
+use Pterodactyl\Models\User;
 use Pterodactyl\Models\Backup;
 use Pterodactyl\Models\Database;
 use Illuminate\Container\Container;
@@ -52,6 +53,10 @@ class SubstituteClientApiBindings extends ApiSubstituteBindings
             return Backup::query()->where('uuid', $value)->firstOrFail();
         });
 
+        $this->router->model('user', User::class, function ($value) {
+            return User::query()->where('uuid', $value)->firstOrFail();
+        });
+
         return parent::handle($request, $next);
     }
 }

app/Http/Requests/Api/Client/Servers/Subusers/SubuserRequest.php

@@ -3,12 +3,10 @@
 namespace Pterodactyl\Http\Requests\Api\Client\Servers\Subusers;
 
 use Illuminate\Http\Request;
-use Pterodactyl\Models\Server;
+use Pterodactyl\Models\User;
 use Pterodactyl\Exceptions\Http\HttpForbiddenException;
-use Pterodactyl\Repositories\Eloquent\SubuserRepository;
 use Pterodactyl\Http\Requests\Api\Client\ClientApiRequest;
-use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
+use Pterodactyl\Services\Servers\GetUserPermissionsService;
-use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 
 abstract class SubuserRequest extends ClientApiRequest
 {
@@ -30,10 +28,10 @@ abstract class SubuserRequest extends ClientApiRequest
             return false;
         }
 
-        // If there is a subuser present in the URL, validate that it is not the same as the
+        $user = $this->route()->parameter('user');
-        // current request user. You're not allowed to modify yourself.
+        // Don't allow a user to edit themselves on the server.
-        if ($this->route()->hasParameter('subuser')) {
+        if ($user instanceof User) {
-            if ($this->endpointSubuser()->user_id === $this->user()->id) {
+            if ($user->uuid === $this->user()->uuid) {
                 return false;
             }
         }
@@ -71,68 +69,14 @@ abstract class SubuserRequest extends ClientApiRequest
         // 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) {
+        /** @var \Pterodactyl\Models\Subuser|null $subuser */
+        /** @var \Pterodactyl\Services\Servers\GetUserPermissionsService $service */
+        $service = $this->container->make(GetUserPermissionsService::class);
+
+        if (count(array_diff($permissions, $service->handle($server, $user))) > 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
-     *
-     * @throws \Illuminate\Contracts\Container\BindingResolutionException
-     */
-    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
-     * @throws \Illuminate\Contracts\Container\BindingResolutionException
-     */
-    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']
-        );
-    }
 }

app/Models/Server.php

@@ -38,7 +38,7 @@ use Znck\Eloquent\Traits\BelongsToThrough;
  * @property \Carbon\Carbon $updated_at
  *
  * @property \Pterodactyl\Models\User $user
- * @property \Pterodactyl\Models\User[]|\Illuminate\Database\Eloquent\Collection $subusers
+ * @property \Pterodactyl\Models\Subuser[]|\Illuminate\Database\Eloquent\Collection $subusers
  * @property \Pterodactyl\Models\Allocation $allocation
  * @property \Pterodactyl\Models\Allocation[]|\Illuminate\Database\Eloquent\Collection $allocations
  * @property \Pterodactyl\Models\Pack|null $pack

app/Repositories/Eloquent/SubuserRepository.php

@@ -18,30 +18,6 @@ class SubuserRepository extends EloquentRepository implements SubuserRepositoryI
         return Subuser::class;
     }
 
-    /**
-     * Returns a subuser model for the given user and server combination. If no record
-     * exists an exception will be thrown.
-     *
-     * @param int $server
-     * @param string $uuid
-     * @return \Pterodactyl\Models\Subuser
-     *
-     * @throws \Illuminate\Database\Eloquent\ModelNotFoundException
-     */
-    public function getUserForServer(int $server, string $uuid): Subuser
-    {
-        /** @var \Pterodactyl\Models\Subuser $model */
-        $model = $this->getBuilder()
-            ->with('server', 'user')
-            ->select('subusers.*')
-            ->join('users', 'users.id', '=', 'subusers.user_id')
-            ->where('subusers.server_id', $server)
-            ->where('users.uuid', $uuid)
-            ->firstOrFail();
-
-        return $model;
-    }
-
     /**
      * Return a subuser with the associated server relationship.
      *

app/Services/Servers/GetUserPermissionsService.php

@@ -30,7 +30,7 @@ class GetUserPermissionsService
         }
 
         /** @var \Pterodactyl\Models\Subuser|null $subuserPermissions */
-        $subuserPermissions = $server->subusers->where('user_id', $user->id)->first();
+        $subuserPermissions = $server->subusers()->where('user_id', $user->id)->first();
 
         return $subuserPermissions ? $subuserPermissions->permissions : [];
     }

routes/api-client.php

@@ -1,6 +1,7 @@
 <?php
 
 use Illuminate\Support\Facades\Route;
+use Pterodactyl\Http\Middleware\Api\Client\Server\SubuserBelongsToServer;
 use Pterodactyl\Http\Middleware\Api\Client\Server\AuthenticateServerAccess;
 use Pterodactyl\Http\Middleware\Api\Client\Server\AllocationBelongsToServer;
 
@@ -84,12 +85,12 @@ Route::group(['prefix' => '/servers/{server}', 'middleware' => [AuthenticateServ
         Route::delete('/allocations/{allocation}', 'Servers\NetworkAllocationController@delete');
     });
 
-    Route::group(['prefix' => '/users'], function () {
+    Route::group(['prefix' => '/users', 'middleware' => [SubuserBelongsToServer::class]], function () {
         Route::get('/', 'Servers\SubuserController@index');
         Route::post('/', 'Servers\SubuserController@store');
-        Route::get('/{subuser}', 'Servers\SubuserController@view');
+        Route::get('/{user}', 'Servers\SubuserController@view');
-        Route::post('/{subuser}', 'Servers\SubuserController@update');
+        Route::post('/{user}', 'Servers\SubuserController@update');
-        Route::delete('/{subuser}', 'Servers\SubuserController@delete');
+        Route::delete('/{user}', 'Servers\SubuserController@delete');
     });
 
     Route::group(['prefix' => '/backups'], function () {