From 6b8236e0b1086a480d83e3c24bbc4e8b95ae4950 Mon Sep 17 00:00:00 2001 From: Oliver <20591763+Cryptoom@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:56:23 +0300 Subject: [PATCH 1/2] feat(media): per-platform media pivot + workspace-scoped media id validation Adds media_post_platform (uuid PK, unique on media_id+post_platform_id, cascadeOnDelete on both FKs) and PostPlatform::media()/scopedMediaItems(), the single place publishers and validators will read per-platform media selection from once the publisher rollout (A3) lands. An empty pivot set for a platform means "all of the post's media", today's behavior, so nothing else needs a special case. Combines this with the media-id validation fix identified in TPX-03 (A0b): posts.media accepted a client-supplied id/path with no check that it resolved to a real medias row owned by the caller's workspace, a cross-tenant IDOR, plus an unhandled 500 on a non-UUID id. Both request classes for create/update (web + API) now validate via PostMediaRules::assertHostedMediaExists. Ported the 11 tests from PR #8 (TPX-03 reference branch) plus 8 new tests for the pivot/relation. Co-Authored-By: Claude Sonnet 5 --- .../Requests/Api/Post/StorePostRequest.php | 12 ++ .../Requests/Api/Post/UpdatePostRequest.php | 8 + .../Requests/App/Post/StorePostRequest.php | 15 +- .../Requests/App/Post/UpdatePostRequest.php | 8 + app/Models/MediaPostPlatform.php | 28 ++++ app/Models/PostPlatform.php | 40 +++++ app/Support/PostMediaRules.php | 68 ++++++++ ...20000_create_media_post_platform_table.php | 44 +++++ tests/Feature/Api/PostApiTest.php | 7 +- tests/Feature/Api/PostMediaApiTest.php | 47 +++++- .../Api/PostMediaExistsValidationTest.php | 158 ++++++++++++++++++ .../PostMediaAltTextValidationTest.php | 25 ++- .../PostMediaExistsValidationWebTest.php | 110 ++++++++++++ tests/Feature/PostPlatformMediaTest.php | 153 +++++++++++++++++ tests/Feature/UpdatePostRequestTest.php | 9 +- 15 files changed, 718 insertions(+), 14 deletions(-) create mode 100644 app/Models/MediaPostPlatform.php create mode 100644 database/migrations/2026_09_17_120000_create_media_post_platform_table.php create mode 100644 tests/Feature/Api/PostMediaExistsValidationTest.php create mode 100644 tests/Feature/PostMediaExistsValidationWebTest.php create mode 100644 tests/Feature/PostPlatformMediaTest.php diff --git a/app/Http/Requests/Api/Post/StorePostRequest.php b/app/Http/Requests/Api/Post/StorePostRequest.php index 8f19cb3ba..dda1c680b 100644 --- a/app/Http/Requests/Api/Post/StorePostRequest.php +++ b/app/Http/Requests/Api/Post/StorePostRequest.php @@ -14,6 +14,7 @@ use Illuminate\Foundation\Http\FormRequest; use Illuminate\Support\Collection; use Illuminate\Validation\Rule; +use Illuminate\Validation\Validator; class StorePostRequest extends FormRequest { @@ -61,6 +62,17 @@ public function rules(): array ]; } + public function withValidator(Validator $validator): void + { + $validator->after(function (Validator $validator): void { + PostMediaRules::assertHostedMediaExists( + $validator, + $this->user()->currentWorkspace, + (array) $this->input('media', []), + ); + }); + } + /** * @return array */ diff --git a/app/Http/Requests/Api/Post/UpdatePostRequest.php b/app/Http/Requests/Api/Post/UpdatePostRequest.php index 2796b80f4..6e80f6cf0 100644 --- a/app/Http/Requests/Api/Post/UpdatePostRequest.php +++ b/app/Http/Requests/Api/Post/UpdatePostRequest.php @@ -82,6 +82,14 @@ public function attributes(): array public function withValidator(Validator $validator): void { + $validator->after(function (Validator $validator): void { + PostMediaRules::assertHostedMediaExists( + $validator, + $this->user()->currentWorkspace, + (array) $this->input('media', []), + ); + }); + $validator->after(function (Validator $validator): void { if (! in_array($this->input('status'), [Status::Scheduled->value, Status::Publishing->value], true)) { return; diff --git a/app/Http/Requests/App/Post/StorePostRequest.php b/app/Http/Requests/App/Post/StorePostRequest.php index c564ec583..eec7189de 100644 --- a/app/Http/Requests/App/Post/StorePostRequest.php +++ b/app/Http/Requests/App/Post/StorePostRequest.php @@ -4,7 +4,9 @@ namespace App\Http\Requests\App\Post; +use App\Support\PostMediaRules; use Illuminate\Foundation\Http\FormRequest; +use Illuminate\Validation\Validator; class StorePostRequest extends FormRequest { @@ -20,7 +22,18 @@ public function rules(): array { return [ 'date' => ['nullable', 'date_format:Y-m-d'], - 'media' => ['nullable', 'array'], + ...PostMediaRules::rules(hosted: true), ]; } + + public function withValidator(Validator $validator): void + { + $validator->after(function (Validator $validator): void { + PostMediaRules::assertHostedMediaExists( + $validator, + $this->user()->currentWorkspace, + (array) $this->input('media', []), + ); + }); + } } diff --git a/app/Http/Requests/App/Post/UpdatePostRequest.php b/app/Http/Requests/App/Post/UpdatePostRequest.php index 83647284d..64a40b2a7 100644 --- a/app/Http/Requests/App/Post/UpdatePostRequest.php +++ b/app/Http/Requests/App/Post/UpdatePostRequest.php @@ -79,6 +79,14 @@ public function attributes(): array public function withValidator(Validator $validator): void { + $validator->after(function (Validator $validator): void { + PostMediaRules::assertHostedMediaExists( + $validator, + $this->user()->currentWorkspace, + (array) $this->input('media', []), + ); + }); + $validator->after(function (Validator $validator): void { if (! $this->isPublishingOrScheduling()) { return; diff --git a/app/Models/MediaPostPlatform.php b/app/Models/MediaPostPlatform.php new file mode 100644 index 000000000..511512173 --- /dev/null +++ b/app/Models/MediaPostPlatform.php @@ -0,0 +1,28 @@ +using(self::class)`). The + * plain `attach()` path used without `using()` bypasses model events entirely + * and would leave `id` null. + */ +class MediaPostPlatform extends Pivot +{ + use HasUuids; + + protected $table = 'media_post_platform'; +} diff --git a/app/Models/PostPlatform.php b/app/Models/PostPlatform.php index a7316211d..28b4e977c 100644 --- a/app/Models/PostPlatform.php +++ b/app/Models/PostPlatform.php @@ -4,6 +4,7 @@ namespace App\Models; +use App\Dto\MediaItem; use App\Enums\PostPlatform\ContentType; use App\Enums\PostPlatform\Status; use App\Enums\SocialAccount\Platform as SocialPlatform; @@ -13,6 +14,8 @@ use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; +use Illuminate\Database\Eloquent\Relations\BelongsToMany; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Storage; class PostPlatform extends Model @@ -63,6 +66,43 @@ public function socialAccount(): BelongsTo return $this->belongsTo(SocialAccount::class); } + /** + * The subset of the post's media items scoped to this platform. Empty + * unless a caller has explicitly narrowed which media this platform + * publishes (per-platform media selection). + */ + public function media(): BelongsToMany + { + return $this->belongsToMany(Media::class, 'media_post_platform') + ->using(MediaPostPlatform::class) + ->withTimestamps(); + } + + /** + * The media items this platform actually publishes. An empty pivot set + * for this platform means "no per-platform selection was made", which is + * today's behaviour: fall back to every media item on the post. This is + * the one place that decides that, so every publisher and validator + * reads media through this helper instead of `$postPlatform->post->mediaItems` + * directly, or the two would silently disagree once a selection exists. + * + * @return Collection + */ + public function scopedMediaItems(): Collection + { + $selectedIds = $this->media()->pluck('medias.id'); + + $allMediaItems = $this->post->mediaItems; + + if ($selectedIds->isEmpty()) { + return $allMediaItems; + } + + return $allMediaItems->filter( + fn (MediaItem $item) => $selectedIds->contains($item->id) + )->values(); + } + /** * Only platforms still enabled for publishing — disabled ones are * excluded from PublishPost, so anything else that mirrors publish diff --git a/app/Support/PostMediaRules.php b/app/Support/PostMediaRules.php index 4d192778f..001aa1699 100644 --- a/app/Support/PostMediaRules.php +++ b/app/Support/PostMediaRules.php @@ -5,8 +5,13 @@ namespace App\Support; use App\Enums\Media\Source; +use App\Models\Media; +use App\Models\Workspace; use Closure; +use Illuminate\Database\Eloquent\Relations\Relation; +use Illuminate\Support\Str; use Illuminate\Validation\Rule; +use Illuminate\Validation\Validator; /** * Single source of truth for inline post `media` validation, shared by the post @@ -62,4 +67,67 @@ public static function rules(bool $hosted): array 'media.*.source_meta' => ['sometimes', 'nullable', 'array'], ]; } + + /** + * Reject inline media items that claim to already be hosted (`id` and/or + * `path` set) but don't resolve to a real `medias` row owned by this + * workspace. `rules()` above only checks shape (id is a non-empty string). + * It never confirms the id exists, so a client could otherwise write an + * arbitrary id/path pair straight into `posts.media`, including another + * workspace's real asset (IDOR) or a path nothing backs at all. Mirrors + * the lookup `FindWorkspaceAsset` already uses for `attach-existing-asset`. + * + * A bare `url` with no `id`/`path` is left alone: that's the API-only + * "please download this external URL" case (`PostMediaRules::rules` + * with `hosted: false`), and `HostInlineMedia`/`MediaAttacher` handle it + * by fetching the URL and creating a fresh, workspace-owned `Media` row + * before anything is persisted. There's no pre-existing id to check yet. + * + * @param array> $media + */ + public static function assertHostedMediaExists(Validator $validator, Workspace $workspace, array $media): void + { + foreach ($media as $index => $item) { + $id = data_get($item, 'id'); + $path = data_get($item, 'path'); + + if (blank($id)) { + if (filled($path)) { + $validator->errors()->add( + "media.{$index}.id", + 'The media id field is required when path is present.', + ); + } + + // Blank id, blank path: a bare external url for HostInlineMedia to fetch. Nothing to verify yet. + continue; + } + + if ($validator->errors()->has("media.{$index}.id")) { + // A shape rule (e.g. "must be a string") already failed for this item. + continue; + } + + // A non-UUID id can never match a real medias row (id is a UUID + // primary key), and Postgres rejects it as an invalid uuid literal + // before the query even runs, an unhandled 500 instead of a + // graceful 422. Fail the same way a real, absent id would. + if (! Str::isUuid((string) $id)) { + $validator->errors()->add("media.{$index}.id", 'Media not found.'); + + continue; + } + + $exists = Media::query() + ->where('mediable_type', Relation::getMorphAlias(Workspace::class)) + ->where('mediable_id', $workspace->id) + ->where('collection', 'assets') + ->whereKey($id) + ->exists(); + + if (! $exists) { + $validator->errors()->add("media.{$index}.id", 'Media not found.'); + } + } + } } diff --git a/database/migrations/2026_09_17_120000_create_media_post_platform_table.php b/database/migrations/2026_09_17_120000_create_media_post_platform_table.php new file mode 100644 index 000000000..1c1453a6a --- /dev/null +++ b/database/migrations/2026_09_17_120000_create_media_post_platform_table.php @@ -0,0 +1,44 @@ +uuid('id')->primary(); + $table->uuid('media_id'); + $table->uuid('post_platform_id'); + $table->timestamps(); + + $table->foreign('media_id')->references('id')->on('medias')->cascadeOnDelete(); + $table->foreign('post_platform_id')->references('id')->on('post_platforms')->cascadeOnDelete(); + + $table->unique(['media_id', 'post_platform_id']); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::dropIfExists('media_post_platform'); + } +}; diff --git a/tests/Feature/Api/PostApiTest.php b/tests/Feature/Api/PostApiTest.php index a52ee1069..b30c15361 100644 --- a/tests/Feature/Api/PostApiTest.php +++ b/tests/Feature/Api/PostApiTest.php @@ -7,6 +7,7 @@ use App\Enums\PostPlatform\ContentType; use App\Enums\SocialAccount\Platform; use App\Jobs\PublishPost; +use App\Models\Media; use App\Models\Post; use App\Models\PostPlatform; use App\Models\SocialAccount; @@ -101,10 +102,14 @@ it('creates a post with content, media, and labels', function () { $label = WorkspaceLabel::factory()->create(['workspace_id' => $this->workspace->id]); + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + ]); $payload = [ 'content' => 'Hello from the API', - 'media' => [['id' => 'media-1', 'path' => 'media/foo.jpg', 'url' => 'https://example.com/foo.jpg', 'type' => 'image']], + 'media' => [['id' => $asset->id, 'path' => $asset->path, 'url' => 'https://example.com/'.$asset->path, 'type' => 'image']], 'platforms' => [ ['social_account_id' => $this->socialAccount->id, 'content_type' => 'linkedin_post'], ], diff --git a/tests/Feature/Api/PostMediaApiTest.php b/tests/Feature/Api/PostMediaApiTest.php index caa4cdb6a..7edf64720 100644 --- a/tests/Feature/Api/PostMediaApiTest.php +++ b/tests/Feature/Api/PostMediaApiTest.php @@ -490,6 +490,12 @@ it('keeps an already-hosted item and a freshly-hosted url in order', function () { $this->socialAccount->update(['is_active' => true]); + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + 'path' => 'assets/already.jpg', + ]); + Http::fake([ '93.184.216.34/external.jpg' => Http::response( file_get_contents(__DIR__.'/../../fixtures/1x1.png'), @@ -502,7 +508,7 @@ ->postJson(route('api.posts.store'), [ 'content' => 'Mixed media post', 'media' => [ - ['id' => 'hosted-1', 'path' => 'assets/already.jpg', 'url' => 'https://cdn.trypost.test/assets/already.jpg', 'type' => 'image'], + ['id' => $asset->id, 'path' => $asset->path, 'url' => 'https://cdn.trypost.test/assets/already.jpg', 'type' => 'image'], ['url' => 'https://93.184.216.34/external.jpg'], ], 'platforms' => [ @@ -517,19 +523,25 @@ ->and(data_get($media, '0.path'))->toBe('assets/already.jpg') ->and(data_get($media, '1.url'))->not->toContain('93.184.216.34') ->and(data_get($media, '1.path'))->not->toBeNull(); - // Only the external URL is hosted; the passed-through item creates no new row. - expect(Media::where('mediable_id', $this->workspace->id)->count())->toBe(1); + // The pre-existing asset is reused (no duplicate row); only the external url is newly hosted. + expect(Media::where('mediable_id', $this->workspace->id)->count())->toBe(2); }); it('passes already-hosted media through on create without downloading', function () { $this->socialAccount->update(['is_active' => true]); Http::preventStrayRequests(); + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + 'path' => 'assets/foo.jpg', + ]); + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) ->postJson(route('api.posts.store'), [ 'content' => 'Hosted media post', 'media' => [[ - 'id' => 'media-1', + 'id' => $asset->id, 'path' => 'assets/foo.jpg', 'url' => 'https://cdn.trypost.test/assets/foo.jpg', 'type' => 'image', @@ -541,7 +553,8 @@ ->assertCreated(); expect(data_get(Post::where('content', 'Hosted media post')->firstOrFail()->media, '0.path'))->toBe('assets/foo.jpg'); - expect(Media::where('mediable_id', $this->workspace->id)->count())->toBe(0); + // The pre-existing asset is reused; nothing new is downloaded or hosted. + expect(Media::where('mediable_id', $this->workspace->id)->count())->toBe(1); }); it('downloads and hosts an external media url when updating a post', function () { @@ -587,11 +600,17 @@ $this->socialAccount->update(['is_active' => true]); Http::preventStrayRequests(); + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + 'path' => 'assets/foo.jpg', + ]); + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) ->postJson(route('api.posts.store'), [ 'content' => 'Alt text post', 'media' => [[ - 'id' => 'media-1', + 'id' => $asset->id, 'path' => 'assets/foo.jpg', 'url' => 'https://cdn.trypost.test/assets/foo.jpg', 'type' => 'image', @@ -609,11 +628,17 @@ }); it('accepts and persists media alt text on update', function () { + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + 'path' => 'assets/foo.jpg', + ]); + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) ->putJson(route('api.posts.update', $this->post), [ 'status' => 'draft', 'media' => [[ - 'id' => 'media-1', + 'id' => $asset->id, 'path' => 'assets/foo.jpg', 'url' => 'https://cdn.trypost.test/assets/foo.jpg', 'type' => 'image', @@ -626,11 +651,17 @@ }); it('preserves every media meta key on update, not just alt_text', function () { + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + 'path' => 'assets/foo.jpg', + ]); + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) ->putJson(route('api.posts.update', $this->post), [ 'status' => 'draft', 'media' => [[ - 'id' => 'media-1', + 'id' => $asset->id, 'path' => 'assets/foo.jpg', 'url' => 'https://cdn.trypost.test/assets/foo.jpg', 'type' => 'image', diff --git a/tests/Feature/Api/PostMediaExistsValidationTest.php b/tests/Feature/Api/PostMediaExistsValidationTest.php new file mode 100644 index 000000000..a3020bf9e --- /dev/null +++ b/tests/Feature/Api/PostMediaExistsValidationTest.php @@ -0,0 +1,158 @@ +user = $result['user']; + $this->workspace = $result['workspace']; + $this->plainToken = $result['plain_token']; + + $this->socialAccount = SocialAccount::factory()->create([ + 'workspace_id' => $this->workspace->id, + 'platform' => Platform::LinkedIn, + ]); +}); + +it('rejects creating a post with a fabricated media id/path (no matching medias row)', function () { + $payload = [ + 'content' => 'Fabricated media id', + 'media' => [['id' => 'media-1', 'path' => 'media/foo.jpg', 'url' => 'https://example.com/foo.jpg', 'type' => 'image']], + 'platforms' => [ + ['social_account_id' => $this->socialAccount->id, 'content_type' => 'linkedin_post'], + ], + ]; + + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) + ->postJson(route('api.posts.store'), $payload) + ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) + ->assertJsonValidationErrors(['media.0.id']); + + expect(Post::where('workspace_id', $this->workspace->id)->count())->toBe(0); +}); + +it('creates a post when media id references a real asset owned by the workspace', function () { + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + ]); + + $payload = [ + 'content' => 'Real media id', + 'media' => [['id' => $asset->id, 'path' => $asset->path, 'url' => 'https://example.com/'.$asset->path, 'type' => 'image']], + 'platforms' => [ + ['social_account_id' => $this->socialAccount->id, 'content_type' => 'linkedin_post'], + ], + ]; + + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) + ->postJson(route('api.posts.store'), $payload) + ->assertCreated(); + + $post = Post::where('workspace_id', $this->workspace->id)->first(); + expect($post->media)->toHaveCount(1) + ->and(data_get($post->media, '0.id'))->toBe($asset->id); +}); + +it('rejects creating a post with another workspace\'s real media id (cross-tenant IDOR)', function () { + $other = Workspace::factory()->create(); + $foreignAsset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $other->id, + ]); + + $payload = [ + 'content' => 'Cross-tenant media id', + 'media' => [['id' => $foreignAsset->id, 'path' => $foreignAsset->path, 'url' => $foreignAsset->url, 'type' => 'image']], + 'platforms' => [ + ['social_account_id' => $this->socialAccount->id, 'content_type' => 'linkedin_post'], + ], + ]; + + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) + ->postJson(route('api.posts.store'), $payload) + ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) + ->assertJsonValidationErrors(['media.0.id']); + + expect(Post::where('workspace_id', $this->workspace->id)->count())->toBe(0); +}); + +it('still allows a bare external url with no id/path (API download-and-host path)', function () { + Http::fake([ + 'example.com/photo.png' => Http::response( + file_get_contents(__DIR__.'/../../fixtures/1x1.png'), + 200, + ['Content-Type' => 'image/png'], + ), + ]); + Storage::fake(); + + $payload = [ + 'content' => 'Fresh external url', + 'media' => [['url' => 'https://example.com/photo.png']], + 'platforms' => [ + ['social_account_id' => $this->socialAccount->id, 'content_type' => 'linkedin_post'], + ], + ]; + + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) + ->postJson(route('api.posts.store'), $payload) + ->assertCreated(); + + $post = Post::where('workspace_id', $this->workspace->id)->first(); + expect($post->media)->toHaveCount(1); + + $storedId = data_get($post->media, '0.id'); + expect(Media::query()->whereKey($storedId)->exists())->toBeTrue(); +}); + +it('rejects updating a post with a fabricated media id/path', function () { + $post = Post::factory()->create([ + 'workspace_id' => $this->workspace->id, + 'user_id' => $this->user->id, + ]); + + $payload = [ + 'status' => 'draft', + 'media' => [['id' => 'forged-id', 'path' => 'media/forged.jpg', 'url' => 'https://example.com/forged.jpg', 'type' => 'image']], + ]; + + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) + ->putJson(route('api.posts.update', $post), $payload) + ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) + ->assertJsonValidationErrors(['media.0.id']); + + expect($post->fresh()->media)->toBe([]); +}); + +it('rejects a media item that sends a path without a resolvable id', function () { + $payload = [ + 'content' => 'Path without id', + 'media' => [['path' => 'media/orphan.jpg', 'url' => 'https://example.com/orphan.jpg', 'type' => 'image']], + 'platforms' => [ + ['social_account_id' => $this->socialAccount->id, 'content_type' => 'linkedin_post'], + ], + ]; + + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) + ->postJson(route('api.posts.store'), $payload) + ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) + ->assertJsonValidationErrors(['media.0.id']); +}); diff --git a/tests/Feature/PostMediaAltTextValidationTest.php b/tests/Feature/PostMediaAltTextValidationTest.php index 9fcb6b8b2..1c3be8572 100644 --- a/tests/Feature/PostMediaAltTextValidationTest.php +++ b/tests/Feature/PostMediaAltTextValidationTest.php @@ -3,6 +3,7 @@ declare(strict_types=1); use App\Enums\UserWorkspace\Role; +use App\Models\Media; use App\Models\Post; use App\Models\User; use App\Models\Workspace; @@ -18,11 +19,17 @@ 'user_id' => $user->id, ]); + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $workspace->id, + 'path' => 'uploads/x.jpg', + ]); + $response = $this->actingAs($user)->put(route('app.posts.update', $post), [ 'status' => 'draft', 'content' => 'hi', 'media' => [[ - 'id' => 'm1', 'path' => 'uploads/x.jpg', 'url' => 'https://cdn.test/x.jpg', + 'id' => $asset->id, 'path' => 'uploads/x.jpg', 'url' => 'https://cdn.test/x.jpg', 'meta' => ['alt_text' => 'a golden retriever on a beach'], ]], ]); @@ -42,11 +49,17 @@ 'user_id' => $user->id, ]); + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $workspace->id, + 'path' => 'uploads/x.jpg', + ]); + $response = $this->actingAs($user)->put(route('app.posts.update', $post), [ 'status' => 'draft', 'content' => 'hi', 'media' => [[ - 'id' => 'm1', 'path' => 'uploads/x.jpg', 'url' => 'https://cdn.test/x.jpg', 'type' => 'image', + 'id' => $asset->id, 'path' => 'uploads/x.jpg', 'url' => 'https://cdn.test/x.jpg', 'type' => 'image', 'meta' => [ 'width' => 1080, 'height' => 1350, @@ -102,13 +115,19 @@ 'user_id' => $user->id, ]); + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $workspace->id, + 'path' => 'uploads/x.jpg', + ]); + $altText = str_repeat('a', 2000); $response = $this->actingAs($user)->put(route('app.posts.update', $post), [ 'status' => 'draft', 'content' => 'hi', 'media' => [[ - 'id' => 'm1', 'path' => 'uploads/x.jpg', 'url' => 'https://cdn.test/x.jpg', + 'id' => $asset->id, 'path' => 'uploads/x.jpg', 'url' => 'https://cdn.test/x.jpg', 'meta' => ['alt_text' => $altText], ]], ]); diff --git a/tests/Feature/PostMediaExistsValidationWebTest.php b/tests/Feature/PostMediaExistsValidationWebTest.php new file mode 100644 index 000000000..d82ab5ab8 --- /dev/null +++ b/tests/Feature/PostMediaExistsValidationWebTest.php @@ -0,0 +1,110 @@ + ['nullable', 'array']), + * and App\Http\Requests\App\Post\UpdatePostRequest validated shape only (id/path required + * strings) with no check that the id actually resolves to a medias row owned by the + * workspace. Both now run App\Support\PostMediaRules::assertHostedMediaExists. + */ +beforeEach(function () { + $this->user = User::factory()->create([]); + $this->workspace = Workspace::factory()->create(['user_id' => $this->user->id]); + $this->workspace->members()->attach($this->user->id, ['role' => Role::Member->value]); + $this->user->update(['current_workspace_id' => $this->workspace->id]); + + $this->socialAccount = SocialAccount::factory()->create([ + 'workspace_id' => $this->workspace->id, + 'platform' => Platform::LinkedIn, + ]); +}); + +test('store post rejects a fabricated media id/path (no matching medias row)', function () { + $response = $this->actingAs($this->user)->post(route('app.posts.store'), [ + 'media' => [['id' => 'media-1', 'path' => 'media/foo.jpg', 'url' => 'https://example.com/foo.jpg', 'type' => 'image']], + ]); + + $response->assertSessionHasErrors(['media.0.id']); + expect(Post::where('workspace_id', $this->workspace->id)->count())->toBe(0); +}); + +test('store post accepts a media id that resolves to a real workspace asset', function () { + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + ]); + + $response = $this->actingAs($this->user)->post(route('app.posts.store'), [ + 'media' => [['id' => $asset->id, 'path' => $asset->path, 'url' => 'https://example.com/'.$asset->path, 'type' => 'image']], + ]); + + $response->assertSessionDoesntHaveErrors(); + $response->assertRedirect(); + + $post = Post::where('workspace_id', $this->workspace->id)->first(); + expect($post)->not->toBeNull(); + expect($post->media)->toHaveCount(1) + ->and(data_get($post->media, '0.id'))->toBe($asset->id); +}); + +test('store post rejects another workspace\'s real media id (cross-tenant IDOR)', function () { + $other = Workspace::factory()->create(); + $foreignAsset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $other->id, + ]); + + $response = $this->actingAs($this->user)->post(route('app.posts.store'), [ + 'media' => [['id' => $foreignAsset->id, 'path' => $foreignAsset->path, 'url' => 'https://example.com/'.$foreignAsset->path, 'type' => 'image']], + ]); + + $response->assertSessionHasErrors(['media.0.id']); + expect(Post::where('workspace_id', $this->workspace->id)->count())->toBe(0); +}); + +test('update post rejects a fabricated media id/path', function () { + $post = Post::factory()->create([ + 'workspace_id' => $this->workspace->id, + 'user_id' => $this->user->id, + ]); + + $response = $this->actingAs($this->user)->put(route('app.posts.update', $post), [ + 'status' => 'draft', + 'media' => [['id' => 'forged-id', 'path' => 'media/forged.jpg', 'url' => 'https://example.com/forged.jpg', 'type' => 'image']], + ]); + + $response->assertSessionHasErrors(['media.0.id']); + expect($post->fresh()->media)->toBe([]); +}); + +test('update post accepts a media id that resolves to a real workspace asset', function () { + $post = Post::factory()->create([ + 'workspace_id' => $this->workspace->id, + 'user_id' => $this->user->id, + ]); + + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + ]); + + $response = $this->actingAs($this->user)->put(route('app.posts.update', $post), [ + 'status' => 'draft', + 'media' => [['id' => $asset->id, 'path' => $asset->path, 'url' => 'https://example.com/'.$asset->path, 'type' => 'image']], + ]); + + $response->assertSessionDoesntHaveErrors(); + expect($post->fresh()->media)->toHaveCount(1) + ->and(data_get($post->fresh()->media, '0.id'))->toBe($asset->id); +}); diff --git a/tests/Feature/PostPlatformMediaTest.php b/tests/Feature/PostPlatformMediaTest.php new file mode 100644 index 000000000..a1c5736e6 --- /dev/null +++ b/tests/Feature/PostPlatformMediaTest.php @@ -0,0 +1,153 @@ +workspace = Workspace::factory()->create(); + $this->socialAccount = SocialAccount::factory()->create([ + 'workspace_id' => $this->workspace->id, + 'platform' => Platform::LinkedIn, + ]); +}); + +function makePostWithMedia(Workspace $workspace, SocialAccount $socialAccount, array $mediaItems): array +{ + $media = collect($mediaItems)->map(fn () => Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $workspace->id, + ])); + + $post = Post::factory()->create([ + 'workspace_id' => $workspace->id, + 'media' => $media->map(fn (Media $m) => [ + 'id' => $m->id, + 'path' => $m->path, + 'url' => $m->url, + 'type' => $m->type->value, + ])->all(), + ]); + + $postPlatform = PostPlatform::factory()->create([ + 'post_id' => $post->id, + 'social_account_id' => $socialAccount->id, + ]); + + return [$post, $postPlatform, $media]; +} + +it('scopedMediaItems returns every post media item when no pivot rows exist for the platform', function () { + [$post, $postPlatform, $media] = makePostWithMedia($this->workspace, $this->socialAccount, [1, 2, 3]); + + $scoped = $postPlatform->scopedMediaItems(); + + expect($scoped)->toHaveCount(3) + ->and($scoped->pluck('id')->sort()->values()->all()) + ->toBe($media->pluck('id')->sort()->values()->all()); +}); + +it('scopedMediaItems returns only the pivoted media items when a selection exists', function () { + [$post, $postPlatform, $media] = makePostWithMedia($this->workspace, $this->socialAccount, [1, 2, 3]); + + $postPlatform->media()->attach($media->first()->id); + + $scoped = $postPlatform->scopedMediaItems(); + + expect($scoped)->toHaveCount(1) + ->and($scoped->first()->id)->toBe($media->first()->id); +}); + +it('scopedMediaItems can select more than one media item for a platform', function () { + [$post, $postPlatform, $media] = makePostWithMedia($this->workspace, $this->socialAccount, [1, 2, 3]); + + $postPlatform->media()->attach($media->take(2)->pluck('id')); + + $scoped = $postPlatform->scopedMediaItems(); + + expect($scoped)->toHaveCount(2) + ->and($scoped->pluck('id')->sort()->values()->all()) + ->toBe($media->take(2)->pluck('id')->sort()->values()->all()); +}); + +it('allows the same media item to be selected for two platforms of the same post', function () { + $media = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + ]); + + $post = Post::factory()->create([ + 'workspace_id' => $this->workspace->id, + 'media' => [['id' => $media->id, 'path' => $media->path, 'url' => $media->url, 'type' => $media->type->value]], + ]); + + $otherAccount = SocialAccount::factory()->create([ + 'workspace_id' => $this->workspace->id, + 'platform' => Platform::X, + ]); + + $platformA = PostPlatform::factory()->create(['post_id' => $post->id, 'social_account_id' => $this->socialAccount->id]); + $platformB = PostPlatform::factory()->create(['post_id' => $post->id, 'social_account_id' => $otherAccount->id]); + + $platformA->media()->attach($media->id); + $platformB->media()->attach($media->id); + + expect($platformA->media()->count())->toBe(1) + ->and($platformB->media()->count())->toBe(1); +}); + +it('generates a uuid primary key on the pivot row via HasUuids', function () { + [$post, $postPlatform, $media] = makePostWithMedia($this->workspace, $this->socialAccount, [1]); + + $postPlatform->media()->attach($media->first()->id); + + $pivotRow = MediaPostPlatform::query()->firstOrFail(); + + expect($pivotRow->id)->not->toBeNull() + ->and(Str::isUuid($pivotRow->id))->toBeTrue(); +}); + +it('enforces a unique constraint on media_id and post_platform_id', function () { + [$post, $postPlatform, $media] = makePostWithMedia($this->workspace, $this->socialAccount, [1]); + + $postPlatform->media()->attach($media->first()->id); + + expect(fn () => $postPlatform->media()->attach($media->first()->id)) + ->toThrow(QueryException::class); +}); + +it('cascades delete when the media item is removed', function () { + [$post, $postPlatform, $media] = makePostWithMedia($this->workspace, $this->socialAccount, [1]); + + $postPlatform->media()->attach($media->first()->id); + expect(MediaPostPlatform::query()->count())->toBe(1); + + $media->first()->delete(); + + expect(MediaPostPlatform::query()->count())->toBe(0); +}); + +it('cascades delete when the post platform is removed', function () { + [$post, $postPlatform, $media] = makePostWithMedia($this->workspace, $this->socialAccount, [1]); + + $postPlatform->media()->attach($media->first()->id); + expect(MediaPostPlatform::query()->count())->toBe(1); + + $postPlatform->delete(); + + expect(MediaPostPlatform::query()->count())->toBe(0); +}); diff --git a/tests/Feature/UpdatePostRequestTest.php b/tests/Feature/UpdatePostRequestTest.php index 113af6dde..8c96b7b37 100644 --- a/tests/Feature/UpdatePostRequestTest.php +++ b/tests/Feature/UpdatePostRequestTest.php @@ -6,6 +6,7 @@ use App\Enums\PostPlatform\ContentType; use App\Enums\SocialAccount\Platform; use App\Enums\UserWorkspace\Role; +use App\Models\Media; use App\Models\Post; use App\Models\PostPlatform; use App\Models\SocialAccount; @@ -625,9 +626,15 @@ }); test('draft save accepts media source metadata for ai regeneration', function () { + $asset = Media::factory()->assets()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + 'path' => 'ai-images/generated.webp', + ]); + $payload = [ [ - 'id' => 'media-ai-keep-meta', + 'id' => $asset->id, 'path' => 'ai-images/generated.webp', 'url' => 'https://example.com/ai-images/generated.webp', 'type' => 'image', From 495cac7654f59fdb6cad58cd5bec5355c280cca1 Mon Sep 17 00:00:00 2001 From: Oliver <20591763+Cryptoom@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:02:31 +0300 Subject: [PATCH 2/2] fix(media): tighten pivot docblock rationale, add collection-boundary test Review round 1 feedback (2 independent code-reviewer passes): the composite-key pivot post_workspace_label has the identical many-to-many shape and works fine with a composite PK, so "no natural single key" overstated the uuid-PK justification. Rewrote both docblocks around the actual reason (row addressability for debugging), the unique index still does the uniqueness enforcement. Also closes a minor coverage gap: no test asserted the collection === 'assets' boundary in assertHostedMediaExists (a real workspace-owned medias row in the wrong collection, e.g. a logo, must still 422). Co-Authored-By: Claude Sonnet 5 --- app/Models/MediaPostPlatform.php | 9 ++++---- ...20000_create_media_post_platform_table.php | 9 ++++---- .../Api/PostMediaExistsValidationTest.php | 22 +++++++++++++++++++ 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/app/Models/MediaPostPlatform.php b/app/Models/MediaPostPlatform.php index 511512173..cf210ab9c 100644 --- a/app/Models/MediaPostPlatform.php +++ b/app/Models/MediaPostPlatform.php @@ -10,10 +10,11 @@ /** * Pivot for `PostPlatform::media()`: which of a post's media items apply to a * given platform. A dedicated `id` primary key (via HasUuids) instead of the - * plain composite-key pivot used elsewhere (see `post_workspace_label`), - * because the same media item can be attached to more than one platform of - * the same post, so there is no natural single unique key here, and an - * explicit id makes a row directly addressable when debugging. + * plain composite-key pivot used elsewhere (see `post_workspace_label`) is + * deliberate: it makes an individual row directly addressable for debugging + * (single id in a log line, `firstOrFail()` by id) instead of needing both + * foreign keys every time. The `(media_id, post_platform_id)` uniqueness is + * still enforced separately by the migration's own unique index. * * HasUuids generates the id in the model's `creating` event, which only fires * for a custom pivot class (`belongsToMany(...)->using(self::class)`). The diff --git a/database/migrations/2026_09_17_120000_create_media_post_platform_table.php b/database/migrations/2026_09_17_120000_create_media_post_platform_table.php index 1c1453a6a..78aad1767 100644 --- a/database/migrations/2026_09_17_120000_create_media_post_platform_table.php +++ b/database/migrations/2026_09_17_120000_create_media_post_platform_table.php @@ -14,10 +14,11 @@ * an empty pivot set for a platform means "all of the post's media", * today's behaviour, so no special-casing is needed anywhere reading it. * - * A dedicated `id` primary key (instead of a plain composite-key pivot) - * is deliberate: the same media item can be attached to more than one - * platform of the same post, so there is no natural single-column key, - * and an explicit id makes this row directly addressable when debugging. + * A dedicated `id` primary key (instead of the plain composite-key pivot + * used by `post_workspace_label`) is deliberate: it makes an individual + * row directly addressable for debugging (single id in a log line, + * `firstOrFail()` by id) rather than needing both foreign keys every + * time. Uniqueness is still enforced separately below. */ public function up(): void { diff --git a/tests/Feature/Api/PostMediaExistsValidationTest.php b/tests/Feature/Api/PostMediaExistsValidationTest.php index a3020bf9e..a16cfbafc 100644 --- a/tests/Feature/Api/PostMediaExistsValidationTest.php +++ b/tests/Feature/Api/PostMediaExistsValidationTest.php @@ -156,3 +156,25 @@ ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) ->assertJsonValidationErrors(['media.0.id']); }); + +it('rejects a real media id from the workspace that belongs to a different collection', function () { + $logo = Media::factory()->logo()->create([ + 'mediable_type' => (new Workspace)->getMorphClass(), + 'mediable_id' => $this->workspace->id, + ]); + + $payload = [ + 'content' => 'Wrong collection', + 'media' => [['id' => $logo->id, 'path' => $logo->path, 'url' => $logo->url, 'type' => 'image']], + 'platforms' => [ + ['social_account_id' => $this->socialAccount->id, 'content_type' => 'linkedin_post'], + ], + ]; + + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) + ->postJson(route('api.posts.store'), $payload) + ->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY) + ->assertJsonValidationErrors(['media.0.id']); + + expect(Post::where('workspace_id', $this->workspace->id)->count())->toBe(0); +});