From 06c4fb2d89ecc578557de2d935a234093ab72961 Mon Sep 17 00:00:00 2001 From: Ivan Moroz Date: Wed, 3 Jun 2026 16:46:22 +0300 Subject: [PATCH] CONV-310: Add API ownership guards Co-Authored-By: Claude Sonnet 4.6 --- .../Api/V1/ConversionController.php | 10 ++- .../Controllers/Api/V1/FileController.php | 8 +- app/Support/Api/ApiExceptionMapper.php | 16 ++++ app/Support/Api/ApiOwnershipGuard.php | 27 ++++++ .../Feature/Api/V1/ApiOwnershipGuardTest.php | 82 +++++++++++++++++++ 5 files changed, 134 insertions(+), 9 deletions(-) create mode 100644 app/Support/Api/ApiOwnershipGuard.php create mode 100644 tests/Feature/Api/V1/ApiOwnershipGuardTest.php diff --git a/app/Http/Controllers/Api/V1/ConversionController.php b/app/Http/Controllers/Api/V1/ConversionController.php index 197d48b..d5eb2f8 100644 --- a/app/Http/Controllers/Api/V1/ConversionController.php +++ b/app/Http/Controllers/Api/V1/ConversionController.php @@ -13,6 +13,7 @@ use App\Http\Resources\Api\V1\ConversionResource; use App\Models\ConversionJob; use App\Models\FileRecord; +use App\Support\Api\ApiOwnershipGuard; use App\Support\Conversions\Exceptions\UnsupportedConversionException; use App\Support\Converters\ConverterRegistry; use Illuminate\Http\JsonResponse; @@ -24,13 +25,14 @@ final class ConversionController public function __construct( private readonly ConverterRegistry $registry, private readonly CreditLedger $creditLedger, + private readonly ApiOwnershipGuard $ownershipGuard, ) {} public function estimate(EstimateConversionRequest $request): JsonResponse { $file = FileRecord::findOrFail($request->integer('file_id')); - abort_if($file->user_id !== $request->user()->id, 403, 'You do not own this file.'); + $this->ownershipGuard->ensureFileOwner($request->user(), $file); $converter = $this->registry->find($file->extension, $request->string('target_format')->toString()); @@ -57,7 +59,7 @@ public function store(CreateConversionRequest $request): JsonResponse { $file = FileRecord::findOrFail($request->integer('file_id')); - abort_if($file->user_id !== $request->user()->id, 403, 'You do not own this file.'); + $this->ownershipGuard->ensureFileOwner($request->user(), $file); $job = app(CreateConversionJobAction::class)->handle( user: $request->user(), @@ -73,14 +75,14 @@ public function store(CreateConversionRequest $request): JsonResponse public function show(Request $request, ConversionJob $conversion): JsonResponse { - abort_if($conversion->user_id !== $request->user()->id, 403, 'You do not own this conversion.'); + $this->ownershipGuard->ensureConversionOwner($request->user(), $conversion); return response()->json(['data' => (new ConversionResource($conversion))->resolve()]); } public function download(Request $request, ConversionJob $conversion): mixed { - abort_if($conversion->user_id !== $request->user()->id, 403, 'You do not own this conversion.'); + $this->ownershipGuard->ensureConversionOwner($request->user(), $conversion); if ($conversion->status !== ConversionStatus::Completed) { return response()->json([ diff --git a/app/Http/Controllers/Api/V1/FileController.php b/app/Http/Controllers/Api/V1/FileController.php index 4ffc770..2231991 100644 --- a/app/Http/Controllers/Api/V1/FileController.php +++ b/app/Http/Controllers/Api/V1/FileController.php @@ -9,6 +9,7 @@ use App\Http\Resources\Api\V1\FileResource; use App\Http\Resources\Api\V1\TargetFormatResource; use App\Models\FileRecord; +use App\Support\Api\ApiOwnershipGuard; use App\Support\Converters\ConverterRegistry; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -18,6 +19,7 @@ final class FileController { public function __construct( private readonly ConverterRegistry $registry, + private readonly ApiOwnershipGuard $ownershipGuard, ) {} public function store(UploadFileRequest $request): JsonResponse @@ -34,11 +36,7 @@ public function store(UploadFileRequest $request): JsonResponse public function targets(Request $request, FileRecord $file): JsonResource { - abort_if( - $file->user_id !== $request->user()->id, - 403, - 'You do not own this file.', - ); + $this->ownershipGuard->ensureFileOwner($request->user(), $file); $targets = $this->registry->targetsFor($file->extension); diff --git a/app/Support/Api/ApiExceptionMapper.php b/app/Support/Api/ApiExceptionMapper.php index 7ba48b0..c6ed6bb 100644 --- a/app/Support/Api/ApiExceptionMapper.php +++ b/app/Support/Api/ApiExceptionMapper.php @@ -13,6 +13,7 @@ use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Auth\AuthenticationException; use Illuminate\Http\Exceptions\ThrottleRequestsException; +use Symfony\Component\HttpKernel\Exception\HttpException; use Throwable; final class ApiExceptionMapper @@ -62,6 +63,21 @@ public function map(Throwable $e): ?MappedApiError message: 'Too many requests.', status: 429, ), + $e instanceof HttpException && $e->getStatusCode() === 401 => new MappedApiError( + code: 'unauthorized', + message: 'Unauthenticated.', + status: 401, + ), + $e instanceof HttpException && $e->getStatusCode() === 403 => new MappedApiError( + code: 'forbidden', + message: 'Forbidden.', + status: 403, + ), + $e instanceof HttpException && $e->getStatusCode() === 404 => new MappedApiError( + code: 'not_found', + message: 'Resource not found.', + status: 404, + ), default => null, }; } diff --git a/app/Support/Api/ApiOwnershipGuard.php b/app/Support/Api/ApiOwnershipGuard.php new file mode 100644 index 0000000..85e2e60 --- /dev/null +++ b/app/Support/Api/ApiOwnershipGuard.php @@ -0,0 +1,27 @@ +user_id !== $user->id) { + throw new AuthorizationException('You do not own this file.'); + } + } + + public function ensureConversionOwner(User $user, ConversionJob $conversion): void + { + if ($conversion->user_id !== $user->id) { + throw new AuthorizationException('You do not own this conversion.'); + } + } +} diff --git a/tests/Feature/Api/V1/ApiOwnershipGuardTest.php b/tests/Feature/Api/V1/ApiOwnershipGuardTest.php new file mode 100644 index 0000000..b041fc8 --- /dev/null +++ b/tests/Feature/Api/V1/ApiOwnershipGuardTest.php @@ -0,0 +1,82 @@ +create(['plan' => Plan::Pro]); + $other = User::factory()->create(['plan' => Plan::Pro]); + + $file = FileRecord::factory()->for($owner)->create(['extension' => 'png']); + $token = app(ApiKeyGenerator::class)->create($other, 'Other')->plainToken; + + $this->withToken($token) + ->getJson("/api/v1/files/{$file->id}/targets") + ->assertForbidden() + ->assertJsonPath('error.code', 'forbidden'); +}); + +it('does not allow api user to estimate cost on another users file', function () { + $owner = User::factory()->create(['plan' => Plan::Pro]); + $other = User::factory()->create(['plan' => Plan::Pro]); + + $file = FileRecord::factory()->for($owner)->create(['extension' => 'png']); + $token = app(ApiKeyGenerator::class)->create($other, 'Other')->plainToken; + + $this->withToken($token) + ->postJson('/api/v1/conversions/estimate', [ + 'file_id' => $file->id, + 'target_format' => 'jpg', + 'options' => [], + ]) + ->assertForbidden() + ->assertJsonPath('error.code', 'forbidden'); +}); + +it('does not allow api user to create conversion on another users file', function () { + $owner = User::factory()->create(['plan' => Plan::Pro]); + $other = User::factory()->create(['plan' => Plan::Pro]); + + $file = FileRecord::factory()->for($owner)->create(['extension' => 'png']); + $token = app(ApiKeyGenerator::class)->create($other, 'Other')->plainToken; + + $this->withToken($token) + ->postJson('/api/v1/conversions', [ + 'file_id' => $file->id, + 'target_format' => 'jpg', + 'options' => [], + ]) + ->assertForbidden() + ->assertJsonPath('error.code', 'forbidden'); +}); + +it('does not allow api user to get another users conversion status', function () { + $owner = User::factory()->create(['plan' => Plan::Pro]); + $other = User::factory()->create(['plan' => Plan::Pro]); + + $job = ConversionJob::factory()->for($owner)->queued()->create(); + $token = app(ApiKeyGenerator::class)->create($other, 'Other')->plainToken; + + $this->withToken($token) + ->getJson("/api/v1/conversions/{$job->id}") + ->assertForbidden() + ->assertJsonPath('error.code', 'forbidden'); +}); + +it('does not allow api user to download another users conversion', function () { + $owner = User::factory()->create(['plan' => Plan::Pro]); + $other = User::factory()->create(['plan' => Plan::Pro]); + + $job = ConversionJob::factory()->for($owner)->queued()->create(); + $token = app(ApiKeyGenerator::class)->create($other, 'Other')->plainToken; + + $this->withToken($token) + ->getJson("/api/v1/conversions/{$job->id}/download") + ->assertForbidden() + ->assertJsonPath('error.code', 'forbidden'); +});