Improve client API route model binding and prevent accidental route access without valid model binds

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

@@ -88,10 +88,6 @@ class ScheduleController extends ClientApiController
      */
     public function view(ViewScheduleRequest $request, Server $server, Schedule $schedule): array
     {
-        if ($schedule->server_id !== $server->id) {
-            throw new NotFoundHttpException();
-        }
-
         $schedule->loadMissing('tasks');
 
         return $this->fractal->item($schedule)

app/Http/Kernel.php

@@ -21,6 +21,7 @@ use Illuminate\View\Middleware\ShareErrorsFromSession;
 use Pterodactyl\Http\Middleware\MaintenanceMiddleware;
 use Pterodactyl\Http\Middleware\RedirectIfAuthenticated;
 use Illuminate\Auth\Middleware\AuthenticateWithBasicAuth;
+use Pterodactyl\Http\Middleware\Api\PreventUnboundModels;
 use Pterodactyl\Http\Middleware\Api\ApiSubstituteBindings;
 use Illuminate\Foundation\Http\Middleware\ValidatePostSize;
 use Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse;
@@ -76,6 +77,7 @@ class Kernel extends HttpKernel
             EnsureFrontendRequestsAreStateful::class,
             'auth:sanctum',
             SubstituteClientApiBindings::class,
+            PreventUnboundModels::class,
             // This is perhaps a little backwards with the Client API, but logically you'd be unable
             // to create/get an API key without first enabling 2FA on the account, so I suppose in the
             // end it makes sense.

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

@@ -3,60 +3,93 @@
 namespace Pterodactyl\Http\Middleware\Api\Client;
 
 use Closure;
-use Pterodactyl\Models\User;
+use Illuminate\Support\Str;
-use Pterodactyl\Models\Backup;
+use Illuminate\Routing\Route;
-use Pterodactyl\Models\Database;
+use Pterodactyl\Models\Server;
 use Illuminate\Container\Container;
+use Illuminate\Database\Query\JoinClause;
+use Illuminate\Contracts\Routing\Registrar;
 use Pterodactyl\Contracts\Extensions\HashidsInterface;
-use Pterodactyl\Http\Middleware\Api\ApiSubstituteBindings;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
-use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
-use Pterodactyl\Contracts\Repository\ServerRepositoryInterface;
 
-class SubstituteClientApiBindings extends ApiSubstituteBindings
+class SubstituteClientApiBindings
 {
+    private Registrar $router;
+
+    public function __construct(Registrar $router)
+    {
+        $this->router = $router;
+    }
+
     /**
-     * Perform substitution of route parameters without triggering
+     * Perform substitution of route parameters for the Client API.
-     * a 404 error if a model is not found.
-     *
-     * @param \Illuminate\Http\Request $request
      *
+     * @param \Illuminate\Http\Request
+     * @param \Closure $next
      * @return mixed
      */
     public function handle($request, Closure $next)
     {
-        // Override default behavior of the model binding to use a specific table
+        $this->router->bind('server', function ($value) {
-        // column rather than the default 'id'.
+            return Server::query()->where(Str::length($value) === 8 ? 'uuidShort' : 'uuid', $value)->firstOrFail();
-        $this->router->bind('server', function ($value) use ($request) {
-            try {
-                $column = 'uuidShort';
-                if (preg_match('/^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i', $value)) {
-                    $column = 'uuid';
-                }
-
-                return Container::getInstance()->make(ServerRepositoryInterface::class)->findFirstWhere([
-                    [$column, '=', $value],
-                ]);
-            } catch (RecordNotFoundException $ex) {
-                $request->attributes->set('is_missing_model', true);
-
-                return null;
-            }
         });
 
-        $this->router->bind('database', function ($value) {
+        $this->router->bind('allocation', function ($value, $route) {
+            return $this->server($route)->allocations()->where('id', $value)->firstOrFail();
+        });
+
+        $this->router->bind('schedule', function ($value, $route) {
+            return $this->server($route)->schedule()->where('id', $value)->firstOrFail();
+        });
+
+        $this->router->bind('database', function ($value, $route) {
             $id = Container::getInstance()->make(HashidsInterface::class)->decodeFirst($value);
 
-            return Database::query()->where('id', $id)->firstOrFail();
+            return $this->server($route)->where('id', $id)->firstOrFail();
         });
 
-        $this->router->bind('backup', function ($value) {
+        $this->router->bind('backup', function ($value, $route) {
-            return Backup::query()->where('uuid', $value)->firstOrFail();
+            return $this->server($route)->backups()->where('uuid', $value)->firstOrFail();
         });
 
-        $this->router->bind('user', function ($value) {
+        $this->router->bind('user', function ($value, $route) {
-            return User::query()->where('uuid', $value)->firstOrFail();
+            // TODO: is this actually a valid binding for users on the server?
+            return $this->server($route)->subusers()
+                ->join('users', function (JoinClause $join) {
+                    $join->on('subusers.user_id', 'users.id')
+                        ->where('subusers.server_id', 'servers.id');
+                })
+                ->where('users.uuid', $value)
+                ->firstOrFail();
         });
 
-        return parent::handle($request, $next);
+        try {
+            /** @var \Illuminate\Routing\Route $route */
+            $this->router->substituteBindings($route = $request->route());
+        } catch (ModelNotFoundException $exception) {
+            if (isset($route) && $route->getMissing()) {
+                $route->getMissing()($request);
+            }
+
+            throw $exception;
+        }
+
+        return $next($request);
+    }
+
+    /**
+     * Plucks the server model off the route. If no server model is present a
+     * ModelNotFound exception will be thrown.
+     *
+     * @throws \Illuminate\Database\Eloquent\ModelNotFoundException
+     */
+    private function server(Route $route): Server
+    {
+        $server = $route->parameter('server');
+        if (!$server instanceof Server) {
+            throw (new ModelNotFoundException())->setModel(Server::class, [$route->parameter('server')]);
+        }
+
+        return $server;
     }
 }

app/Http/Middleware/Api/PreventUnboundModels.php

@@ -0,0 +1,49 @@
+<?php
+
+namespace Pterodactyl\Http\Middleware\Api;
+
+use Closure;
+use Illuminate\Support\Reflector;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Contracts\Routing\UrlRoutable;
+use Illuminate\Contracts\Container\BindingResolutionException;
+
+class PreventUnboundModels
+{
+    /**
+     * Prevents a request from making it to a controller action if there is a model
+     * injection on the controller that has not been explicitly bound by the request.
+     * This prevents empty models from being valid in scenarios where a new model is
+     * added but not properly defined in the substitution middleware.
+     *
+     * @param \Illuminate\Http\Request $request
+     * @param \Closure $next
+     * @return mixed
+     */
+    public function handle($request, Closure $next)
+    {
+        $route = $request->route();
+
+        $parameters = $route->signatureParameters(UrlRoutable::class);
+        for ($i = 0; $i < count($route->parameters()); $i++) {
+            $class = Reflector::getParameterClassName($parameters[$i + 1]);
+
+            // Skip anything that isn't explicitly requested as a model.
+            if (is_null($class) || !is_subclass_of($class, Model::class)) {
+                continue;
+            }
+
+            if (!array_values($route->parameters())[$i] instanceof $class) {
+                throw new BindingResolutionException(
+                    sprintf(
+                        'No parameter binding has been defined for model [%s] using route parameter key "%s".',
+                        $class,
+                        array_keys($route->parameters())[$i]
+                    )
+                );
+            }
+        }
+
+        return $next($request);
+    }
+}