diff --git a/app/Http/Controllers/Api/DeliveryController.php b/app/Http/Controllers/Api/DeliveryController.php index 71116d6..dd454c3 100644 --- a/app/Http/Controllers/Api/DeliveryController.php +++ b/app/Http/Controllers/Api/DeliveryController.php @@ -16,6 +16,8 @@ class DeliveryController extends Controller */ public function index(Request $request): JsonResponse { + $this->authorizeAbility($request, 'read'); + $validator = Validator::make($request->all(), [ 'status' => ['nullable', 'string', Rule::in(['pending', 'retrying', 'success', 'failed'])], 'from_date' => ['nullable', 'date'], @@ -70,6 +72,8 @@ public function index(Request $request): JsonResponse */ public function show(Request $request, Delivery $delivery): JsonResponse { + $this->authorizeAbility($request, 'read'); + // Load the event with trashed included: a soft-deleted parent event // must not make an existing delivery's ownership check (or its // history) silently disappear. @@ -88,6 +92,8 @@ public function show(Request $request, Delivery $delivery): JsonResponse */ public function stats(Request $request): JsonResponse { + $this->authorizeAbility($request, 'read'); + $baseQuery = Delivery::query() ->whereHas('event', function ($q) use ($request) { $q->withTrashed()->where('user_id', $request->user()->id); diff --git a/app/Http/Controllers/Api/EndpointController.php b/app/Http/Controllers/Api/EndpointController.php index f8050ac..36c03dd 100644 --- a/app/Http/Controllers/Api/EndpointController.php +++ b/app/Http/Controllers/Api/EndpointController.php @@ -17,6 +17,8 @@ class EndpointController extends Controller */ public function index(Request $request): JsonResponse { + $this->authorizeAbility($request, 'read'); + $endpoints = $request->user()->endpoints() ->with('events') ->paginate(15); @@ -29,6 +31,8 @@ public function index(Request $request): JsonResponse */ public function store(Request $request): JsonResponse { + $this->authorizeAbility($request, 'create'); + $validator = Validator::make($request->all(), [ 'name' => 'required|string|max:255', 'url' => ['required', 'url', 'max:2048', new SafeWebhookUrl()], @@ -58,6 +62,8 @@ public function store(Request $request): JsonResponse */ public function show(Request $request, Endpoint $endpoint): JsonResponse { + $this->authorizeAbility($request, 'read'); + if ($endpoint->user_id !== $request->user()->id) { return response()->json(['message' => 'Not Found'], 404); } @@ -72,6 +78,8 @@ public function show(Request $request, Endpoint $endpoint): JsonResponse */ public function update(Request $request, Endpoint $endpoint): JsonResponse { + $this->authorizeAbility($request, 'update'); + if ($endpoint->user_id !== $request->user()->id) { return response()->json(['message' => 'Not Found'], 404); } @@ -100,6 +108,8 @@ public function update(Request $request, Endpoint $endpoint): JsonResponse */ public function regenerateSecret(Request $request, Endpoint $endpoint): JsonResponse { + $this->authorizeAbility($request, 'update'); + if ($endpoint->user_id !== $request->user()->id) { return response()->json(['message' => 'Not Found'], 404); } @@ -117,6 +127,8 @@ public function regenerateSecret(Request $request, Endpoint $endpoint): JsonResp */ public function destroy(Request $request, Endpoint $endpoint): JsonResponse { + $this->authorizeAbility($request, 'delete'); + if ($endpoint->user_id !== $request->user()->id) { return response()->json(['message' => 'Not Found'], 404); } diff --git a/app/Http/Controllers/Api/EventController.php b/app/Http/Controllers/Api/EventController.php index 9ac1619..62d7aa3 100644 --- a/app/Http/Controllers/Api/EventController.php +++ b/app/Http/Controllers/Api/EventController.php @@ -17,6 +17,8 @@ class EventController extends Controller */ public function index(Request $request): JsonResponse { + $this->authorizeAbility($request, 'read'); + $events = $request->user()->events() ->with(['endpoints', 'deliveries' => function ($query) { $query->latest()->limit(5); @@ -31,6 +33,8 @@ public function index(Request $request): JsonResponse */ public function store(Request $request): JsonResponse { + $this->authorizeAbility($request, 'create'); + $validator = Validator::make($request->all(), [ 'name' => [ 'required', 'string', 'max:255', @@ -71,6 +75,8 @@ public function store(Request $request): JsonResponse */ public function show(Request $request, Event $event): JsonResponse { + $this->authorizeAbility($request, 'read'); + if ($event->user_id !== $request->user()->id) { return response()->json(['message' => 'Not Found'], 404); } @@ -88,6 +94,8 @@ public function show(Request $request, Event $event): JsonResponse */ public function update(Request $request, Event $event): JsonResponse { + $this->authorizeAbility($request, 'update'); + if ($event->user_id !== $request->user()->id) { return response()->json(['message' => 'Not Found'], 404); } @@ -132,6 +140,8 @@ public function update(Request $request, Event $event): JsonResponse */ public function destroy(Request $request, Event $event): JsonResponse { + $this->authorizeAbility($request, 'delete'); + if ($event->user_id !== $request->user()->id) { return response()->json(['message' => 'Not Found'], 404); } diff --git a/app/Http/Controllers/Api/WebhookController.php b/app/Http/Controllers/Api/WebhookController.php index e4a0e1f..cc01c13 100644 --- a/app/Http/Controllers/Api/WebhookController.php +++ b/app/Http/Controllers/Api/WebhookController.php @@ -18,6 +18,8 @@ class WebhookController extends Controller */ public function trigger(Request $request, string $eventName): JsonResponse { + $this->authorizeAbility($request, 'create'); + $validator = Validator::make($request->all(), [ 'payload' => ['required', 'array', new WebhookPayloadSize()], ]); @@ -128,6 +130,8 @@ private function dispatchDelivery(Delivery $delivery): void */ public function retryDelivery(Request $request, Delivery $delivery): JsonResponse { + $this->authorizeAbility($request, 'update'); + // Load the event with trashed included: a soft-deleted parent event // must not turn this ownership check into a crash on a null relation. $event = $delivery->event()->withTrashed()->first(); diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 8677cd5..aede116 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -2,7 +2,29 @@ namespace App\Http\Controllers; +use Illuminate\Http\Request; + abstract class Controller { - // + /** + * Ensure the current request's Sanctum access token is scoped to the + * given ability before proceeding. + * + * Session-authenticated requests (the Inertia/web UI, and tests using + * actingAs()) carry Sanctum's TransientToken, whose can() always + * returns true, so this only restricts requests actually made with a + * personal access token that was scoped to a limited set of + * abilities. Without this check, a token restricted to "read" in the + * API Tokens UI silently retained full create/update/delete access, + * since `auth:sanctum` alone only verifies the token is valid, not + * what it's scoped to do. + */ + protected function authorizeAbility(Request $request, string $ability): void + { + abort_unless( + $request->user()->tokenCan($ability), + 403, + "This action requires the \"{$ability}\" API token ability." + ); + } } diff --git a/tests/Feature/ApiEventNameUniquenessScopeTest.php b/tests/Feature/ApiEventNameUniquenessScopeTest.php index 6ac15b9..9e755ec 100644 --- a/tests/Feature/ApiEventNameUniquenessScopeTest.php +++ b/tests/Feature/ApiEventNameUniquenessScopeTest.php @@ -5,6 +5,7 @@ use App\Models\Event; use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; +use Laravel\Sanctum\Sanctum; use Tests\TestCase; class ApiEventNameUniquenessScopeTest extends TestCase @@ -16,12 +17,12 @@ public function test_two_users_can_create_events_with_the_same_name_via_api(): v $userA = User::factory()->withPersonalTeam()->create(); $userB = User::factory()->withPersonalTeam()->create(); - $this->actingAs($userA) - ->postJson('/api/v1/events', ['name' => 'order.created']) + Sanctum::actingAs($userA, ['*']); + $this->postJson('/api/v1/events', ['name' => 'order.created']) ->assertStatus(201); - $this->actingAs($userB) - ->postJson('/api/v1/events', ['name' => 'order.created']) + Sanctum::actingAs($userB, ['*']); + $this->postJson('/api/v1/events', ['name' => 'order.created']) ->assertStatus(201); $this->assertDatabaseHas('events', ['user_id' => $userA->id, 'name' => 'order.created']); @@ -32,12 +33,12 @@ public function test_same_user_cannot_create_duplicate_event_name_via_api(): voi { $user = User::factory()->withPersonalTeam()->create(); - $this->actingAs($user) - ->postJson('/api/v1/events', ['name' => 'order.created']) + Sanctum::actingAs($user, ['*']); + $this->postJson('/api/v1/events', ['name' => 'order.created']) ->assertStatus(201); - $this->actingAs($user) - ->postJson('/api/v1/events', ['name' => 'order.created']) + Sanctum::actingAs($user, ['*']); + $this->postJson('/api/v1/events', ['name' => 'order.created']) ->assertStatus(422) ->assertJsonValidationErrors('name'); @@ -52,8 +53,8 @@ public function test_user_can_rename_event_to_a_name_used_by_another_user_via_ap Event::factory()->for($userA)->create(['name' => 'order.created']); $eventB = Event::factory()->for($userB)->create(['name' => 'order.updated']); - $this->actingAs($userB) - ->putJson("/api/v1/events/{$eventB->id}", ['name' => 'order.created']) + Sanctum::actingAs($userB, ['*']); + $this->putJson("/api/v1/events/{$eventB->id}", ['name' => 'order.created']) ->assertStatus(200); $this->assertDatabaseHas('events', ['id' => $eventB->id, 'name' => 'order.created']); @@ -66,8 +67,8 @@ public function test_user_cannot_rename_event_to_a_name_they_already_use_via_api Event::factory()->for($user)->create(['name' => 'order.created']); $second = Event::factory()->for($user)->create(['name' => 'order.updated']); - $this->actingAs($user) - ->putJson("/api/v1/events/{$second->id}", ['name' => 'order.created']) + Sanctum::actingAs($user, ['*']); + $this->putJson("/api/v1/events/{$second->id}", ['name' => 'order.created']) ->assertStatus(422) ->assertJsonValidationErrors('name'); } diff --git a/tests/Feature/ApiTokenAbilityEnforcementTest.php b/tests/Feature/ApiTokenAbilityEnforcementTest.php new file mode 100644 index 0000000..701547d --- /dev/null +++ b/tests/Feature/ApiTokenAbilityEnforcementTest.php @@ -0,0 +1,256 @@ +for($user)->create(['is_active' => true]); + } + + private function eventFor(User $user): Event + { + return Event::factory()->for($user)->create(); + } + + private function deliveryFor(User $user): Delivery + { + $event = $this->eventFor($user); + $endpoint = $this->endpointFor($user); + + return Delivery::factory()->create([ + 'event_id' => $event->id, + 'endpoint_id' => $endpoint->id, + ]); + } + + // -- Endpoints --------------------------------------------------------- + + public function test_read_only_token_cannot_create_endpoint(): void + { + $user = User::factory()->withPersonalTeam()->create(); + Sanctum::actingAs($user, ['read']); + + $response = $this->postJson('/api/v1/endpoints', [ + 'name' => 'Hijacked', + 'url' => 'https://example.com/webhook', + ]); + + $response->assertStatus(403); + $this->assertDatabaseMissing('endpoints', ['name' => 'Hijacked']); + } + + public function test_read_only_token_cannot_update_endpoint(): void + { + $user = User::factory()->withPersonalTeam()->create(); + $endpoint = $this->endpointFor($user); + Sanctum::actingAs($user, ['read']); + + $response = $this->putJson("/api/v1/endpoints/{$endpoint->id}", ['name' => 'Hijacked']); + + $response->assertStatus(403); + $this->assertDatabaseHas('endpoints', ['id' => $endpoint->id, 'name' => $endpoint->name]); + } + + public function test_read_only_token_cannot_delete_endpoint(): void + { + $user = User::factory()->withPersonalTeam()->create(); + $endpoint = $this->endpointFor($user); + Sanctum::actingAs($user, ['read']); + + $response = $this->deleteJson("/api/v1/endpoints/{$endpoint->id}"); + + $response->assertStatus(403); + $this->assertDatabaseHas('endpoints', ['id' => $endpoint->id, 'deleted_at' => null]); + } + + public function test_read_only_token_cannot_regenerate_endpoint_secret(): void + { + $user = User::factory()->withPersonalTeam()->create(); + $endpoint = $this->endpointFor($user); + $originalSecret = $endpoint->secret_key; + Sanctum::actingAs($user, ['read']); + + $response = $this->postJson("/api/v1/endpoints/{$endpoint->id}/regenerate-secret"); + + $response->assertStatus(403); + $this->assertSame($originalSecret, $endpoint->fresh()->secret_key); + } + + public function test_read_ability_token_can_list_and_view_endpoints(): void + { + $user = User::factory()->withPersonalTeam()->create(); + $endpoint = $this->endpointFor($user); + Sanctum::actingAs($user, ['read']); + + $this->getJson('/api/v1/endpoints')->assertOk(); + $this->getJson("/api/v1/endpoints/{$endpoint->id}")->assertOk(); + } + + public function test_create_ability_token_can_create_endpoint(): void + { + $user = User::factory()->withPersonalTeam()->create(); + Sanctum::actingAs($user, ['create']); + + $response = $this->postJson('/api/v1/endpoints', [ + 'name' => 'Allowed', + 'url' => 'https://example.com/webhook', + ]); + + $response->assertStatus(201); + $this->assertDatabaseHas('endpoints', ['name' => 'Allowed', 'user_id' => $user->id]); + } + + public function test_token_without_read_ability_cannot_list_endpoints(): void + { + $user = User::factory()->withPersonalTeam()->create(); + Sanctum::actingAs($user, ['create', 'update', 'delete']); + + $response = $this->getJson('/api/v1/endpoints'); + + $response->assertStatus(403); + } + + // -- Events -------------------------------------------------------------- + + public function test_read_only_token_cannot_create_event(): void + { + $user = User::factory()->withPersonalTeam()->create(); + Sanctum::actingAs($user, ['read']); + + $response = $this->postJson('/api/v1/events', ['name' => 'hijacked.event']); + + $response->assertStatus(403); + $this->assertDatabaseMissing('events', ['name' => 'hijacked.event']); + } + + public function test_read_only_token_cannot_update_event(): void + { + $user = User::factory()->withPersonalTeam()->create(); + $event = $this->eventFor($user); + Sanctum::actingAs($user, ['read']); + + $response = $this->putJson("/api/v1/events/{$event->id}", ['name' => 'hijacked.event']); + + $response->assertStatus(403); + } + + public function test_read_only_token_cannot_delete_event(): void + { + $user = User::factory()->withPersonalTeam()->create(); + $event = $this->eventFor($user); + Sanctum::actingAs($user, ['read']); + + $response = $this->deleteJson("/api/v1/events/{$event->id}"); + + $response->assertStatus(403); + $this->assertDatabaseHas('events', ['id' => $event->id, 'deleted_at' => null]); + } + + // -- Deliveries ------------------------------------------------------------ + + public function test_token_without_read_ability_cannot_list_deliveries(): void + { + $user = User::factory()->withPersonalTeam()->create(); + $this->deliveryFor($user); + Sanctum::actingAs($user, ['create']); + + $response = $this->getJson('/api/v1/deliveries'); + + $response->assertStatus(403); + } + + public function test_token_without_read_ability_cannot_view_delivery_stats(): void + { + $user = User::factory()->withPersonalTeam()->create(); + Sanctum::actingAs($user, ['create']); + + $response = $this->getJson('/api/v1/deliveries/stats'); + + $response->assertStatus(403); + } + + // -- Webhook trigger / retry ------------------------------------------------ + + public function test_read_only_token_cannot_trigger_webhook(): void + { + Http::fake(); + + $user = User::factory()->withPersonalTeam()->create(); + $event = $this->eventFor($user); + $endpoint = $this->endpointFor($user); + $event->endpoints()->attach($endpoint); + Sanctum::actingAs($user, ['read']); + + $response = $this->postJson('/api/v1/webhooks/trigger/'.$event->name, [ + 'payload' => ['foo' => 'bar'], + ]); + + $response->assertStatus(403); + $this->assertDatabaseMissing('deliveries', ['event_id' => $event->id]); + } + + public function test_create_ability_token_can_trigger_webhook(): void + { + Http::fake(); + + $user = User::factory()->withPersonalTeam()->create(); + $event = $this->eventFor($user); + $endpoint = $this->endpointFor($user); + $event->endpoints()->attach($endpoint); + Sanctum::actingAs($user, ['create']); + + $response = $this->postJson('/api/v1/webhooks/trigger/'.$event->name, [ + 'payload' => ['foo' => 'bar'], + ]); + + $response->assertStatus(200); + $this->assertDatabaseHas('deliveries', ['event_id' => $event->id]); + } + + public function test_read_only_token_cannot_retry_delivery(): void + { + $user = User::factory()->withPersonalTeam()->create(); + $delivery = $this->deliveryFor($user); + $delivery->update(['status' => 'failed']); + Sanctum::actingAs($user, ['read']); + + $response = $this->postJson("/api/v1/deliveries/{$delivery->id}/retry"); + + $response->assertStatus(403); + $this->assertSame('failed', $delivery->fresh()->status); + } + + // -- Session-authenticated (web UI) requests are unaffected ----------------- + + public function test_session_authenticated_request_is_not_restricted_by_token_abilities(): void + { + $user = User::factory()->withPersonalTeam()->create(); + + $response = $this->actingAs($user)->postJson('/api/v1/endpoints', [ + 'name' => 'From the dashboard', + 'url' => 'https://example.com/webhook', + ]); + + $response->assertStatus(201); + } +} diff --git a/tests/Feature/DeliveryRetryRateLimitTest.php b/tests/Feature/DeliveryRetryRateLimitTest.php index a4116af..cfc9f61 100644 --- a/tests/Feature/DeliveryRetryRateLimitTest.php +++ b/tests/Feature/DeliveryRetryRateLimitTest.php @@ -8,6 +8,7 @@ use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Http; +use Laravel\Sanctum\Sanctum; use Tests\TestCase; class DeliveryRetryRateLimitTest extends TestCase @@ -33,19 +34,18 @@ public function test_v1_api_delivery_retry_is_rate_limited(): void config(['webhooks.rate_limit' => 2]); $user = User::factory()->withPersonalTeam()->create(); + Sanctum::actingAs($user, ['*']); for ($i = 0; $i < 2; $i++) { $delivery = $this->createFailedDelivery($user); - $this->actingAs($user) - ->postJson("/api/v1/deliveries/{$delivery->id}/retry") + $this->postJson("/api/v1/deliveries/{$delivery->id}/retry") ->assertOk(); } $delivery = $this->createFailedDelivery($user); - $this->actingAs($user) - ->postJson("/api/v1/deliveries/{$delivery->id}/retry") + $this->postJson("/api/v1/deliveries/{$delivery->id}/retry") ->assertStatus(429); } @@ -55,19 +55,18 @@ public function test_deprecated_api_delivery_retry_is_rate_limited(): void config(['webhooks.rate_limit' => 2]); $user = User::factory()->withPersonalTeam()->create(); + Sanctum::actingAs($user, ['*']); for ($i = 0; $i < 2; $i++) { $delivery = $this->createFailedDelivery($user); - $this->actingAs($user) - ->postJson("/api/deliveries/{$delivery->id}/retry") + $this->postJson("/api/deliveries/{$delivery->id}/retry") ->assertOk(); } $delivery = $this->createFailedDelivery($user); - $this->actingAs($user) - ->postJson("/api/deliveries/{$delivery->id}/retry") + $this->postJson("/api/deliveries/{$delivery->id}/retry") ->assertStatus(429); } @@ -83,16 +82,16 @@ public function test_delivery_retry_rate_limit_is_scoped_per_user(): void $deliveryA2 = $this->createFailedDelivery($userA); $deliveryB = $this->createFailedDelivery($userB); - $this->actingAs($userA) - ->postJson("/api/v1/deliveries/{$deliveryA1->id}/retry") + Sanctum::actingAs($userA, ['*']); + $this->postJson("/api/v1/deliveries/{$deliveryA1->id}/retry") ->assertOk(); - $this->actingAs($userA) - ->postJson("/api/v1/deliveries/{$deliveryA2->id}/retry") + Sanctum::actingAs($userA, ['*']); + $this->postJson("/api/v1/deliveries/{$deliveryA2->id}/retry") ->assertStatus(429); - $this->actingAs($userB) - ->postJson("/api/v1/deliveries/{$deliveryB->id}/retry") + Sanctum::actingAs($userB, ['*']); + $this->postJson("/api/v1/deliveries/{$deliveryB->id}/retry") ->assertOk(); } }