feat: Better error message + throttling

vanguardbackup · Aug 9, 2024 · 86ecd9e · 86ecd9e
1 parent d6e478c
commit 86ecd9e
Show file tree

Hide file tree

Showing 7 changed files with 268 additions and 49 deletions.
diff --git a/app/Exceptions/CustomMissingAbilityException.php b/app/Exceptions/CustomMissingAbilityException.php
@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Exceptions;
+
+use Illuminate\Http\JsonResponse;
+use Illuminate\Http\Request;
+use Laravel\Sanctum\Exceptions\MissingAbilityException;
+
+class CustomMissingAbilityException extends MissingAbilityException
+{
+    /**
+     * The abilities that the user does not have.
+     *
+     * @var array<int, string>
+     */
+    protected $abilities;
+
+    /**
+     * Create a new exception instance.
+     *
+     * @param  array<int, string>|string  $abilities
+     */
+    public function __construct(array|string $abilities = [])
+    {
+        parent::__construct($abilities);
+        $this->abilities = is_string($abilities) ? [$abilities] : $abilities;
+    }
+
+    /**
+     * Render the exception into an HTTP response.
+     */
+    public function render(Request $request): JsonResponse
+    {
+        $message = 'Access denied due to insufficient permissions. ';
+        $message .= 'Required token ability scopes: ' . implode(', ', $this->abilities);
+
+        return response()->json(['message' => $message], 403);
+    }
+}
diff --git a/app/Http/Middleware/CustomCheckForAnyAbility.php b/app/Http/Middleware/CustomCheckForAnyAbility.php
@@ -0,0 +1,111 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Http\Middleware;
+
+use App\Exceptions\CustomMissingAbilityException;
+use Closure;
+use Illuminate\Auth\AuthenticationException;
+use Illuminate\Http\Request;
+use InvalidArgumentException;
+use Laravel\Sanctum\Http\Middleware\CheckForAnyAbility as SanctumCheckForAnyAbility;
+
+class CustomCheckForAnyAbility extends SanctumCheckForAnyAbility
+{
+    /**
+     * Handle the incoming request.
+     *
+     * @param  mixed  $request
+     * @param  mixed  $next
+     * @param  mixed  ...$abilities
+     *
+     * @throws AuthenticationException
+     * @throws CustomMissingAbilityException
+     * @throws InvalidArgumentException
+     */
+    public function handle($request, $next, ...$abilities): mixed
+    {
+        $this->validateArguments($request, $next);
+        $this->ensureAuthenticated($request);
+
+        $stringAbilities = $this->validateAndFilterAbilities($abilities);
+
+        if ($this->userHasAnyAbility($request, $stringAbilities)) {
+            return $next($request);
+        }
+
+        throw new CustomMissingAbilityException($stringAbilities);
+    }
+
+    /**
+     * Validate the incoming arguments.
+     *
+     *
+     * @throws InvalidArgumentException
+     */
+    private function validateArguments(mixed $request, mixed $next): void
+    {
+        if (! $request instanceof Request) {
+            throw new InvalidArgumentException('$request must be an instance of Illuminate\Http\Request');
+        }
+
+        if (! $next instanceof Closure) {
+            throw new InvalidArgumentException('$next must be an instance of Closure');
+        }
+    }
+
+    /**
+     * Ensure the user is authenticated.
+     *
+     *
+     * @throws AuthenticationException
+     */
+    private function ensureAuthenticated(Request $request): void
+    {
+        if (! $request->user() || ! $request->user()->currentAccessToken()) {
+            throw new AuthenticationException('Authentication failed. Please log in.');
+        }
+    }
+
+    /**
+     * Validate and filter the abilities.
+     *
+     * @param  array<int|string, mixed>  $abilities
+     * @return array<int, string>
+     *
+     * @throws InvalidArgumentException
+     */
+    private function validateAndFilterAbilities(array $abilities): array
+    {
+        $stringAbilities = array_values(array_filter($abilities, 'is_string'));
+
+        if (count($stringAbilities) !== count($abilities)) {
+            throw new InvalidArgumentException('All abilities must be strings');
+        }
+
+        return $stringAbilities;
+    }
+
+    /**
+     * Check if the user has any of the required abilities.
+     *
+     * @param  array<int, string>  $abilities
+     */
+    private function userHasAnyAbility(Request $request, array $abilities): bool
+    {
+        $user = $request->user();
+
+        if (! $user) {
+            return false;
+        }
+
+        foreach ($abilities as $ability) {
+            if ($user->tokenCan($ability)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}
diff --git a/bootstrap/app.php b/bootstrap/app.php
@@ -1,11 +1,11 @@
 <?php
 
+use App\Http\Middleware\CustomCheckForAnyAbility;
 use App\Http\Middleware\UserLanguage;
 use Illuminate\Foundation\Application;
 use Illuminate\Foundation\Configuration\Exceptions;
 use Illuminate\Foundation\Configuration\Middleware;
 use Laravel\Sanctum\Http\Middleware\CheckAbilities;
-use Laravel\Sanctum\Http\Middleware\CheckForAnyAbility;
 
 /**
  * Application configuration and bootstrapping.
@@ -26,7 +26,7 @@
         $middleware->append(UserLanguage::class);
         $middleware->alias([
             'abilities' => CheckAbilities::class,
-            'ability' => CheckForAnyAbility::class,
+            'ability' => CustomCheckForAnyAbility::class,
         ]);
     })
     ->withExceptions(function (Exceptions $exceptions) {

diff --git a/routes/api.php b/routes/api.php
@@ -12,50 +12,117 @@
 use App\Http\Controllers\Api\UserController;
 use Illuminate\Support\Facades\Route;
 
-Route::middleware(['auth:sanctum'])->group(function () {
+/**
+ * API Routes
+ *
+ * This file defines the routes for the Vanguard API.
+ * All routes are protected by Sanctum authentication and rate limiting.
+ */
+Route::middleware(['auth:sanctum', 'throttle:60,1,default'])->group(function () {
+    /**
+     * Get authenticated user information
+     */
     Route::get('user', [UserController::class, '__invoke']);
 
-    // Backup Tasks
-    Route::get('backup-tasks', [BackupTaskController::class, 'index'])->middleware('ability:view-backup-tasks');
-    Route::post('backup-tasks', [BackupTaskController::class, 'store'])->middleware('ability:create-backup-tasks');
-    Route::get('backup-tasks/{id}', [BackupTaskController::class, 'show'])->middleware('ability:view-backup-tasks');
-    Route::put('backup-tasks/{id}', [BackupTaskController::class, 'update'])->middleware('ability:update-backup-tasks');
-    Route::delete('backup-tasks/{id}', [BackupTaskController::class, 'destroy'])->middleware('ability:delete-backup-tasks');
-
-    Route::post('backup-tasks/{id}/run', RunBackupTaskController::class)->middleware('ability:run-backup-tasks');
-    Route::get('backup-tasks/{id}/status', BackupTaskStatusController::class)->middleware('ability:view-backup-tasks');
-    Route::get('backup-tasks/{id}/latest-log', BackupTaskLatestLogController::class)->middleware('ability:view-backup-tasks');
-
-    // Backup Destinations
-    Route::get('backup-destinations', [BackupDestinationController::class, 'index'])->middleware('ability:view-backup-destinations');
-    Route::post('backup-destinations', [BackupDestinationController::class, 'store'])->middleware('ability:create-backup-destinations');
-    Route::get('backup-destinations/{id}', [BackupDestinationController::class, 'show'])->middleware('ability:view-backup-destinations');
-    Route::put('backup-destinations/{id}', [BackupDestinationController::class, 'update'])->middleware('ability:update-backup-destinations');
-    Route::delete('backup-destinations/{id}', [BackupDestinationController::class, 'destroy'])->middleware('ability:delete-backup-destinations');
-
-    // Tags
-    Route::get('tags', [TagController::class, 'index'])->middleware('ability:manage-tags');
-    Route::post('tags', [TagController::class, 'store'])->middleware('ability:manage-tags');
-    Route::get('tags/{id}', [TagController::class, 'show'])->middleware('ability:manage-tags');
-    Route::put('tags/{id}', [TagController::class, 'update'])->middleware('ability:manage-tags');
-    Route::delete('tags/{id}', [TagController::class, 'destroy'])->middleware('ability:manage-tags');
-
-    // Remote Servers
-    Route::get('remote-servers', [RemoteServerController::class, 'index'])->middleware('ability:view-remote-servers');
-    Route::post('remote-servers', [RemoteServerController::class, 'store'])->middleware('ability:create-remote-servers');
-    Route::get('remote-servers/{id}', [RemoteServerController::class, 'show'])->middleware('ability:view-remote-servers');
-    Route::put('remote-servers/{id}', [RemoteServerController::class, 'update'])->middleware('ability:update-remote-servers');
-    Route::delete('remote-servers/{id}', [RemoteServerController::class, 'destroy'])->middleware('ability:delete-remote-servers');
-
-    // Notification Streams
-    Route::get('notification-streams', [NotificationStreamController::class, 'index'])->middleware('ability:view-notification-streams');
-    Route::post('notification-streams', [NotificationStreamController::class, 'store'])->middleware('ability:create-notification-streams');
-    Route::get('notification-streams/{id}', [NotificationStreamController::class, 'show'])->middleware('ability:view-notification-streams');
-    Route::put('notification-streams/{id}', [NotificationStreamController::class, 'update'])->middleware('ability:update-notification-streams');
-    Route::delete('notification-streams/{id}', [NotificationStreamController::class, 'destroy'])->middleware('ability:delete-notification-streams');
-
-    // Backup Task Logs
-    Route::get('backup-task-logs', [BackupTaskLogController::class, 'index'])->middleware('ability:view-backup-tasks');
-    Route::get('backup-task-logs/{id}', [BackupTaskLogController::class, 'show'])->middleware('ability:view-backup-tasks');
-    Route::delete('backup-task-logs/{id}', [BackupTaskLogController::class, 'destroy'])->middleware('ability:delete-backup-tasks');
+    /**
+     * Read Operations
+     *
+     * These routes are for retrieving data and have a higher rate limit.
+     */
+    Route::middleware('throttle:100,1,default')->group(function () {
+        /**
+         * Backup Tasks
+         */
+        Route::get('backup-tasks', [BackupTaskController::class, 'index'])->middleware('ability:view-backup-tasks');
+        Route::get('backup-tasks/{id}', [BackupTaskController::class, 'show'])->middleware('ability:view-backup-tasks');
+        Route::get('backup-tasks/{id}/status', BackupTaskStatusController::class)
+            ->middleware(['ability:view-backup-tasks', 'throttle:20,1,backup-status']);
+        Route::get('backup-tasks/{id}/latest-log', BackupTaskLatestLogController::class)->middleware('ability:view-backup-tasks');
+
+        /**
+         * Backup Destinations
+         */
+        Route::get('backup-destinations', [BackupDestinationController::class, 'index'])->middleware('ability:view-backup-destinations');
+        Route::get('backup-destinations/{id}', [BackupDestinationController::class, 'show'])->middleware('ability:view-backup-destinations');
+
+        /**
+         * Tags
+         */
+        Route::get('tags', [TagController::class, 'index'])->middleware('ability:manage-tags');
+        Route::get('tags/{id}', [TagController::class, 'show'])->middleware('ability:manage-tags');
+
+        /**
+         * Remote Servers
+         */
+        Route::get('remote-servers', [RemoteServerController::class, 'index'])->middleware('ability:view-remote-servers');
+        Route::get('remote-servers/{id}', [RemoteServerController::class, 'show'])->middleware('ability:view-remote-servers');
+
+        /**
+         * Notification Streams
+         */
+        Route::get('notification-streams', [NotificationStreamController::class, 'index'])->middleware('ability:view-notification-streams');
+        Route::get('notification-streams/{id}', [NotificationStreamController::class, 'show'])->middleware('ability:view-notification-streams');
+
+        /**
+         * Backup Task Logs
+         */
+        Route::get('backup-task-logs', [BackupTaskLogController::class, 'index'])->middleware('ability:view-backup-tasks');
+        Route::get('backup-task-logs/{id}', [BackupTaskLogController::class, 'show'])->middleware('ability:view-backup-tasks');
+    });
+
+    /**
+     * Write Operations
+     *
+     * These routes are for creating, updating, or deleting data and have a lower rate limit.
+     */
+    Route::middleware('throttle:30,1,default')->group(function () {
+        /**
+         * Backup Tasks
+         */
+        Route::post('backup-tasks', [BackupTaskController::class, 'store'])->middleware('ability:create-backup-tasks');
+        Route::put('backup-tasks/{id}', [BackupTaskController::class, 'update'])->middleware('ability:update-backup-tasks');
+        Route::delete('backup-tasks/{id}', [BackupTaskController::class, 'destroy'])->middleware('ability:delete-backup-tasks');
+
+        /**
+         * Backup Destinations
+         */
+        Route::post('backup-destinations', [BackupDestinationController::class, 'store'])->middleware('ability:create-backup-destinations');
+        Route::put('backup-destinations/{id}', [BackupDestinationController::class, 'update'])->middleware('ability:update-backup-destinations');
+        Route::delete('backup-destinations/{id}', [BackupDestinationController::class, 'destroy'])->middleware('ability:delete-backup-destinations');
+
+        /**
+         * Tags
+         */
+        Route::post('tags', [TagController::class, 'store'])->middleware('ability:manage-tags');
+        Route::put('tags/{id}', [TagController::class, 'update'])->middleware('ability:manage-tags');
+        Route::delete('tags/{id}', [TagController::class, 'destroy'])->middleware('ability:manage-tags');
+
+        /**
+         * Remote Servers
+         */
+        Route::post('remote-servers', [RemoteServerController::class, 'store'])->middleware('ability:create-remote-servers');
+        Route::put('remote-servers/{id}', [RemoteServerController::class, 'update'])->middleware('ability:update-remote-servers');
+        Route::delete('remote-servers/{id}', [RemoteServerController::class, 'destroy'])->middleware('ability:delete-remote-servers');
+
+        /**
+         * Notification Streams
+         */
+        Route::post('notification-streams', [NotificationStreamController::class, 'store'])->middleware('ability:create-notification-streams');
+        Route::put('notification-streams/{id}', [NotificationStreamController::class, 'update'])->middleware('ability:update-notification-streams');
+        Route::delete('notification-streams/{id}', [NotificationStreamController::class, 'destroy'])->middleware('ability:delete-notification-streams');
+
+        /**
+         * Backup Task Logs
+         */
+        Route::delete('backup-task-logs/{id}', [BackupTaskLogController::class, 'destroy'])->middleware('ability:delete-backup-tasks');
+    });
+
+    /**
+     * Backup Task Execution
+     *
+     * This route is for running backup tasks.
+     * Rate limiting is implemented within the controller itself.
+     */
+    Route::post('backup-tasks/{id}/run', RunBackupTaskController::class)
+        ->middleware(['ability:run-backup-tasks']);
 });
diff --git a/tests/Feature/BackupDestinations/Api/BackupDestinationsAPITest.php b/tests/Feature/BackupDestinations/Api/BackupDestinationsAPITest.php
@@ -272,7 +272,7 @@
     $response = $this->getJson("/api/backup-destinations/{$destination->id}");
 
     $response->assertForbidden()
-        ->assertJson(['message' => 'Invalid ability provided.']);
+        ->assertJson(['message' => 'Access denied due to insufficient permissions. Required token ability scopes: view-backup-destinations']);
 });
 
 test("it returns 403 for user trying to view another user's backup destination", function (): void {

diff --git a/tests/Feature/BackupTasks/Api/BackupRunTaskAPITest.php b/tests/Feature/BackupTasks/Api/BackupRunTaskAPITest.php
@@ -43,7 +43,7 @@
     $response = $this->postJson("/api/backup-tasks/{$backupTask->id}/run");
 
     $response->assertStatus(403)
-        ->assertJson(['message' => 'Invalid ability provided.']);
+        ->assertJson(['message' => 'Access denied due to insufficient permissions. Required token ability scopes: run-backup-tasks']);
 });
 
 test('user cannot run a backup task belonging to another user', function (): void {

diff --git a/tests/Feature/BackupTasks/Api/BackupTaskLatestLogAPITest.php b/tests/Feature/BackupTasks/Api/BackupTaskLatestLogAPITest.php
@@ -40,7 +40,7 @@
     $response = $this->getJson("/api/backup-tasks/{$this->backupTask->id}/latest-log");
 
     $response->assertForbidden()
-        ->assertJson(['message' => 'Invalid ability provided.']);
+        ->assertJson(['message' => 'Access denied due to insufficient permissions. Required token ability scopes: view-backup-tasks']);
 });
 
 test('returns not found for non-existent backup task', function (): void {