From 49c77406a0cb5c94ff15d2005ffb1c77426116ab Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Thu, 25 Jun 2026 15:34:03 -0300 Subject: [PATCH] fix(linkedin): enforce carousel cap and fail loudly on processing timeout - publishCarousel now caps images at the platform max so the API/MCP paths can't bypass the 10-image limit the UI enforces - waitForProcessing throws on timeout instead of posting against an asset still being processed; poll attempts/interval extracted to overridable protected methods for testability - scope EnsureHasWorkspace to connect entry points + disconnect only; session-driven OAuth callbacks/selects stay ungated so a missing current workspace can't HTML-redirect the popup instead of closing it cleanly --- .../Social/AbstractLinkedInPublisher.php | 38 +++++-- routes/app.php | 52 +++++---- .../Middleware/EnsureHasWorkspaceTest.php | 13 +++ .../Services/Social/LinkedInPublisherTest.php | 103 ++++++++++++++++++ 4 files changed, 173 insertions(+), 33 deletions(-) diff --git a/app/Services/Social/AbstractLinkedInPublisher.php b/app/Services/Social/AbstractLinkedInPublisher.php index 5d669379..8fc052cc 100644 --- a/app/Services/Social/AbstractLinkedInPublisher.php +++ b/app/Services/Social/AbstractLinkedInPublisher.php @@ -134,11 +134,11 @@ private function publishCarousel(?string $content, $media): array { $images = []; - foreach ($media as $item) { - if (! $item->isImage()) { - continue; - } + $imageItems = collect($media) + ->filter(fn ($item) => $item->isImage()) + ->take($this->platform()->maxImages()); + foreach ($imageItems as $item) { $imageUrn = $this->uploadImage($item); if ($imageUrn) { @@ -447,18 +447,20 @@ private function uploadDocument($mediaItem): ?string /** * Poll an asset until it finishes processing. LinkedIn rejects a post that - * references an asset still being processed, so we wait for AVAILABLE. + * references an asset still being processed, so we wait for AVAILABLE and + * fail loudly if it never gets there — publishing against an unprocessed + * asset would error out at the API anyway, with a far less obvious message. */ - private function waitForProcessing(string $resource, string $assetUrn, string $label, int $maxAttempts = 30): void + private function waitForProcessing(string $resource, string $assetUrn, string $label): void { $encodedUrn = urlencode($assetUrn); - for ($i = 0; $i < $maxAttempts; $i++) { + for ($i = 0; $i < $this->processingMaxAttempts(); $i++) { $response = $this->getHttpClient()->get("{$this->baseUrl()}/rest/{$resource}/{$encodedUrn}"); if ($response->failed()) { Log::warning("{$this->label()} {$label} status check failed", ['attempt' => $i]); - sleep(5); + sleep($this->processingPollSeconds()); continue; } @@ -473,10 +475,26 @@ private function waitForProcessing(string $resource, string $assetUrn, string $l throw new \Exception("{$this->label()} {$label} processing failed"); } - sleep(5); + sleep($this->processingPollSeconds()); } - Log::warning("{$this->label()} {$label} processing timeout, proceeding anyway"); + throw new \Exception("{$this->label()} {$label} processing did not complete in time"); + } + + /** + * How many times to poll an uploaded asset for AVAILABLE before giving up. + */ + protected function processingMaxAttempts(): int + { + return 30; + } + + /** + * Seconds to wait between asset processing status checks. + */ + protected function processingPollSeconds(): int + { + return 5; } private function downloadToTempFile(string $url, string $tempFile): void diff --git a/routes/app.php b/routes/app.php index 84aa8406..f503f8f4 100644 --- a/routes/app.php +++ b/routes/app.php @@ -77,60 +77,66 @@ }); // Social Connect routes -Route::middleware(['auth', EnsureHasWorkspace::class])->group(function () { - Route::get('connect/linkedin', [LinkedInController::class, 'connect'])->name('app.social.linkedin.connect'); +Route::middleware(['auth'])->group(function () { + // Starting a connection reads the user's current workspace, so these require + // one — during onboarding they redirect to workspace creation. Disconnecting + // lives here too (and not behind EnsureAccountReady) so it works before a + // subscription exists; the controller still authorizes workspace ownership. + Route::middleware(EnsureHasWorkspace::class)->group(function () { + Route::get('connect/linkedin', [LinkedInController::class, 'connect'])->name('app.social.linkedin.connect'); + Route::get('connect/x', [XController::class, 'connect'])->name('app.social.x.connect'); + Route::get('connect/tiktok', [TikTokController::class, 'connect'])->name('app.social.tiktok.connect'); + Route::get('connect/youtube', [YouTubeController::class, 'connect'])->name('app.social.youtube.connect'); + Route::get('connect/facebook', [FacebookController::class, 'connect'])->name('app.social.facebook.connect'); + Route::get('connect/instagram', [InstagramController::class, 'connect'])->name('app.social.instagram.connect'); + Route::get('connect/instagram-facebook', [InstagramFacebookController::class, 'connect'])->name('app.social.instagram-facebook.connect'); + Route::get('connect/threads', [ThreadsController::class, 'connect'])->name('app.social.threads.connect'); + Route::get('connect/pinterest', [PinterestController::class, 'connect'])->name('app.social.pinterest.connect'); + Route::get('connect/bluesky', [BlueskyController::class, 'connect'])->name('app.social.bluesky.connect'); + Route::post('connect/bluesky', [BlueskyController::class, 'store'])->name('app.social.bluesky.store'); + Route::get('connect/mastodon', [MastodonController::class, 'connect'])->name('app.social.mastodon.connect'); + Route::post('connect/mastodon', [MastodonController::class, 'authorizeInstance'])->name('app.social.mastodon.authorize'); + Route::post('connect/telegram', [TelegramController::class, 'connect'])->name('app.social.telegram.connect'); + Route::get('connect/discord', [DiscordController::class, 'connect'])->name('app.social.discord.connect'); + + Route::delete('accounts/{account}', [SocialController::class, 'disconnect'])->name('app.accounts.disconnect'); + }); + + // OAuth callbacks and identity selection resolve their workspace from the + // session set when the flow started, then self-close the popup. They run + // without the current-workspace gate so a momentarily missing current + // workspace can't HTML-redirect the popup instead of closing it cleanly. Route::get('accounts/linkedin/callback', [LinkedInController::class, 'callback'])->name('app.social.linkedin.callback'); Route::get('accounts/linkedin/select', [LinkedInController::class, 'selectIdentity'])->name('app.social.linkedin.select-identity'); Route::post('accounts/linkedin/select', [LinkedInController::class, 'select'])->name('app.social.linkedin.select'); - Route::get('connect/x', [XController::class, 'connect'])->name('app.social.x.connect'); Route::get('accounts/x/callback', [XController::class, 'callback'])->name('app.social.x.callback'); - Route::get('connect/tiktok', [TikTokController::class, 'connect'])->name('app.social.tiktok.connect'); Route::get('accounts/tiktok/callback', [TikTokController::class, 'callback'])->name('app.social.tiktok.callback'); - Route::get('connect/youtube', [YouTubeController::class, 'connect'])->name('app.social.youtube.connect'); Route::get('accounts/youtube/callback', [YouTubeController::class, 'callback'])->name('app.social.youtube.callback'); Route::get('accounts/youtube/select', [YouTubeController::class, 'selectChannel'])->name('app.social.youtube.select-channel'); Route::post('accounts/youtube/select', [YouTubeController::class, 'select'])->name('app.social.youtube.select'); - Route::get('connect/facebook', [FacebookController::class, 'connect'])->name('app.social.facebook.connect'); Route::get('accounts/facebook/callback', [FacebookController::class, 'callback'])->name('app.social.facebook.callback'); Route::get('accounts/facebook/select', [FacebookController::class, 'selectPage'])->name('app.social.facebook.select-page'); Route::post('accounts/facebook/select', [FacebookController::class, 'select'])->name('app.social.facebook.select'); - Route::get('connect/instagram', [InstagramController::class, 'connect'])->name('app.social.instagram.connect'); Route::get('accounts/instagram/callback', [InstagramController::class, 'callback'])->name('app.social.instagram.callback'); Route::get('accounts/instagram/select', [InstagramController::class, 'selectAccount'])->name('app.social.instagram.select-account'); Route::post('accounts/instagram/select', [InstagramController::class, 'select'])->name('app.social.instagram.select'); - Route::get('connect/instagram-facebook', [InstagramFacebookController::class, 'connect'])->name('app.social.instagram-facebook.connect'); Route::get('accounts/instagram-facebook/callback', [InstagramFacebookController::class, 'callback'])->name('app.social.instagram-facebook.callback'); Route::get('accounts/instagram-facebook/select-page', [InstagramFacebookController::class, 'selectPage'])->name('app.social.instagram-facebook.select-page'); Route::post('accounts/instagram-facebook/select', [InstagramFacebookController::class, 'select'])->name('app.social.instagram-facebook.select'); - Route::get('connect/threads', [ThreadsController::class, 'connect'])->name('app.social.threads.connect'); Route::get('accounts/threads/callback', [ThreadsController::class, 'callback'])->name('app.social.threads.callback'); - Route::get('connect/pinterest', [PinterestController::class, 'connect'])->name('app.social.pinterest.connect'); Route::get('accounts/pinterest/callback', [PinterestController::class, 'callback'])->name('app.social.pinterest.callback'); - Route::get('connect/bluesky', [BlueskyController::class, 'connect'])->name('app.social.bluesky.connect'); - Route::post('connect/bluesky', [BlueskyController::class, 'store'])->name('app.social.bluesky.store'); - - Route::get('connect/mastodon', [MastodonController::class, 'connect'])->name('app.social.mastodon.connect'); - Route::post('connect/mastodon', [MastodonController::class, 'authorizeInstance'])->name('app.social.mastodon.authorize'); Route::get('accounts/mastodon/callback', [MastodonController::class, 'callback'])->name('app.social.mastodon.callback'); - Route::post('connect/telegram', [TelegramController::class, 'connect'])->name('app.social.telegram.connect'); - - Route::get('connect/discord', [DiscordController::class, 'connect'])->name('app.social.discord.connect'); Route::get('accounts/discord/callback', [DiscordController::class, 'callback'])->name('app.social.discord.callback'); - - // Disconnecting must also work during onboarding (before a subscription - // exists), so it lives here rather than behind EnsureAccountReady — the - // controller still authorizes workspace ownership. - Route::delete('accounts/{account}', [SocialController::class, 'disconnect'])->name('app.accounts.disconnect'); }); // Routes that require active subscription and completed onboarding diff --git a/tests/Feature/Middleware/EnsureHasWorkspaceTest.php b/tests/Feature/Middleware/EnsureHasWorkspaceTest.php index 952a5e52..b7f06bd1 100644 --- a/tests/Feature/Middleware/EnsureHasWorkspaceTest.php +++ b/tests/Feature/Middleware/EnsureHasWorkspaceTest.php @@ -3,6 +3,7 @@ declare(strict_types=1); use App\Models\User; +use Inertia\Testing\AssertableInertia; test('connect routes redirect to workspace creation when there is no current workspace', function () { $user = User::factory()->create(['current_workspace_id' => null]); @@ -21,3 +22,15 @@ ->get(route('app.social.x.connect')) ->assertRedirect(route('app.workspaces.create')); }); + +test('oauth callbacks are not blocked by the workspace gate and self-close the popup', function () { + $user = User::factory()->create(['current_workspace_id' => null]); + + $this->actingAs($user) + ->get(route('app.social.x.callback')) + ->assertOk() + ->assertInertia(fn (AssertableInertia $page) => $page + ->component('accounts/PopupCallback') + ->where('success', false), + ); +}); diff --git a/tests/Feature/Services/Social/LinkedInPublisherTest.php b/tests/Feature/Services/Social/LinkedInPublisherTest.php index 2c26dcbd..f0fe2a3f 100644 --- a/tests/Feature/Services/Social/LinkedInPublisherTest.php +++ b/tests/Feature/Services/Social/LinkedInPublisherTest.php @@ -314,6 +314,60 @@ }); }); +test('linkedin publisher caps a carousel at the platform max images', function () { + $media = []; + for ($i = 1; $i <= 12; $i++) { + $media[] = [ + 'id' => "test-media-{$i}", + 'path' => "media/2026-01/carousel-{$i}.jpg", + 'url' => "https://example.com/media/2026-01/carousel-{$i}.jpg", + 'mime_type' => 'image/jpeg', + 'original_filename' => "carousel-{$i}.jpg", + ]; + } + $this->post->update(['media' => $media]); + + $initCallCount = 0; + + Http::fake(function ($request) use (&$initCallCount) { + $url = $request->url(); + + if (str_contains($url, '/rest/images')) { + $initCallCount++; + + return Http::response([ + 'value' => [ + 'uploadUrl' => "https://www.linkedin.com/dms/upload/v2/pic/carousel/{$initCallCount}", + 'image' => "urn:li:image:CarouselImageUrn{$initCallCount}", + ], + ], 200); + } + + if (str_contains($url, '/dms/upload/v2/pic/carousel/')) { + return Http::response(null, 201); + } + + if (str_contains($url, '/rest/posts')) { + return Http::response(null, 201, ['x-restli-id' => 'urn:li:share:capped']); + } + + return Http::response('fake-image-content', 200); + }); + + $this->publisher->publish($this->postPlatform); + + // Only the first 10 images (LinkedIn::maxImages) are uploaded; the extra 2 are dropped. + expect($initCallCount)->toBe(10); + + Http::assertSent(function ($request) { + if (! str_contains($request->url(), '/rest/posts')) { + return false; + } + + return count(data_get($request->data(), 'content.multiImage.images', [])) === 10; + }); +}); + test('linkedin publisher can publish post with video', function () { $this->post->update([ 'media' => [ @@ -575,6 +629,55 @@ Http::assertNotSent(fn ($request) => str_contains($request->url(), '/rest/posts')); }); +test('linkedin publisher throws and does not post when document processing never completes', function () { + $this->postPlatform->update(['content_type' => ContentType::LinkedInPost]); + $this->post->update([ + 'media' => [[ + 'id' => 'doc-media-1', 'path' => 'media/2026-01/deck.pdf', + 'url' => 'https://example.com/media/2026-01/deck.pdf', + 'mime_type' => 'application/pdf', 'original_filename' => 'deck.pdf', + ]], + ]); + + $uploadUrl = 'https://www.linkedin.com/dms-uploads/document/timeout'; + + Http::fake(function ($request) use ($uploadUrl) { + $url = $request->url(); + + if (str_contains($url, '/rest/documents') && str_contains($url, 'initializeUpload')) { + return Http::response(['value' => ['uploadUrl' => $uploadUrl, 'document' => 'urn:li:document:Timeout']], 200); + } + + if ($url === $uploadUrl) { + return Http::response(null, 201); + } + + if (str_contains($url, '/rest/documents/')) { + return Http::response(['status' => 'PROCESSING'], 200); + } + + return Http::response('fake-pdf-bytes', 200); + }); + + $publisher = new class extends LinkedInPublisher + { + protected function processingMaxAttempts(): int + { + return 2; + } + + protected function processingPollSeconds(): int + { + return 0; + } + }; + + expect(fn () => $publisher->publish($this->postPlatform)) + ->toThrow(Exception::class, 'processing did not complete in time'); + + Http::assertNotSent(fn ($request) => str_contains($request->url(), '/rest/posts')); +}); + test('linkedin publisher throws and does not post when document init fails', function () { $this->postPlatform->update(['content_type' => ContentType::LinkedInPost]); $this->post->update([