fix: enforce Sanctum API token abilities on Api controllers - #219
Merged
Conversation
auth:sanctum only verified that a token was valid, not what it was scoped to do. A personal access token restricted to "read" in the API Tokens UI still had full create/update/delete access, because no controller ever called tokenCan()/checked the token's abilities. Add an authorizeAbility() helper on the base Controller and call it at the top of every action in EndpointController, EventController, DeliveryController, and WebhookController, mapping each action to the create/read/update/delete abilities already offered by the API Tokens UI. Session-authenticated requests (the dashboard, and tests using actingAs()) are unaffected, since Sanctum wraps them in a TransientToken that allows every ability. Also switch ApiEventNameUniquenessScopeTest and DeliveryRetryRateLimitTest to Sanctum::actingAs() where a test authenticates as a second user against a Sanctum-guarded route within the same method: auth:sanctum's guard caches its resolved user for the rest of the test process once it authenticates, so a later plain actingAs() call for a different user bypasses Sanctum's own token-wrapping and was incidentally relying on the missing ability check to go unnoticed. Fixes #53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was broken
Jetstream's API-token feature (
Features::api()) was recently enabled (#218), so users can now create personal access tokens and restrict them to specific abilities (create,read,update,delete) via the API Tokens UI. However,auth:sanctumonly verifies that a token is valid — it never checks what the token is scoped to do. None ofEndpointController,EventController,DeliveryController, orWebhookController(underapp/Http/Controllers/Api) calledtokenCan()or used anyability:middleware.This meant a token a user deliberately restricted to "read" (e.g. handed to a third-party integration) silently retained full create/update/delete access anyway — it could create/update/delete endpoints and events, trigger arbitrary webhook deliveries, and read all delivery data, regardless of what abilities were selected in the UI.
What changed
authorizeAbility()helper on the baseApp\Http\Controllers\Controllerthat aborts with a 403 unless$request->user()->tokenCan($ability).index/show/stats→readstore→createupdate/regenerateSecret→updatedestroy→deleteWebhookController::trigger→create(creates new deliveries)WebhookController::retryDelivery→update(mutates an existing delivery)Session-authenticated requests (the Inertia dashboard, and tests using
actingAs()) are unaffected: Sanctum wraps session-authenticated users in aTransientToken, whosecan()always returnstrue, so this only restricts requests actually made with a real, ability-scoped personal access token.tests/Feature/ApiTokenAbilityEnforcementTest.phpcovering that a read-only token is rejected on every mutating endpoint/event/delivery/webhook action, that a token with the right ability succeeds, and that session-authenticated requests are unaffected.ApiEventNameUniquenessScopeTestandDeliveryRetryRateLimitTestto useSanctum::actingAs()for the specific cases where a test authenticates as a second user against a Sanctum-guarded route within the same test method.auth:sanctum's guard caches its resolved (and TransientToken-wrapped) user for the rest of the test process once it first authenticates, so a later plainactingAs()call for a different user object bypassed that wrapping — previously invisible because nothing checked abilities, now surfaced by this fix. This is Sanctum's own documented pattern for this exact test scenario.Testing
vendor/bin/pint --dirty— cleanphp artisan test— full suite passes (240 passed, 4 pre-existing skips, 0 failures)Fixes #53