diff --git a/app/Console/Commands/RefreshExpiringTokens.php b/app/Console/Commands/RefreshExpiringTokens.php index 9dc08b661..5e4218a03 100644 --- a/app/Console/Commands/RefreshExpiringTokens.php +++ b/app/Console/Commands/RefreshExpiringTokens.php @@ -18,10 +18,9 @@ class RefreshExpiringTokens extends Command protected $description = 'Proactively refresh social tokens before they expire'; /** - * Rotating refresh_token platforms only need a short lead: verify() won't - * rotate a still-valid token, so we catch them right before or after expiry. - * Extension-model platforms (Instagram/Threads) can't be refreshed once - * expired, so they get a much wider lead to survive queue backlog. + * Rotating platforms get a short lead — a wider window would only rotate + * more often. Instagram and Threads can't be refreshed once expired, so + * theirs is wide enough to survive queue backlog. */ public function handle(): void { @@ -46,6 +45,8 @@ public function handle(): void } }); - $this->info("Dispatched {$count} token refresh jobs."); + // Accounts in the window, not jobs queued: the job is unique per + // account, so a dispatch during a backlog is silently discarded. + $this->info("{$count} accounts due for a token refresh."); } } diff --git a/app/Http/Controllers/App/AnalyticsController.php b/app/Http/Controllers/App/AnalyticsController.php index cac42f7c6..8376afe8f 100644 --- a/app/Http/Controllers/App/AnalyticsController.php +++ b/app/Http/Controllers/App/AnalyticsController.php @@ -5,6 +5,7 @@ namespace App\Http\Controllers\App; use App\Enums\SocialAccount\Platform; +use App\Exceptions\PlatformUnavailableException; use App\Http\Controllers\Controller; use App\Models\SocialAccount; use App\Services\Social\FacebookAnalytics; @@ -16,6 +17,7 @@ use App\Services\Social\TikTokAnalytics; use App\Services\Social\XAnalytics; use App\Services\Social\YouTubeAnalytics; +use Illuminate\Http\Client\ConnectionException; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Carbon; @@ -72,19 +74,37 @@ public function show(Request $request, SocialAccount $account): JsonResponse $since = $request->has('since') ? Carbon::parse($request->input('since')) : null; $until = $request->has('until') ? Carbon::parse($request->input('until')) : null; - $metrics = match ($account->platform) { - Platform::TikTok => app(TikTokAnalytics::class)->getMetrics($account), - Platform::Instagram, Platform::InstagramFacebook => app(InstagramAnalytics::class)->getMetrics($account, $since, $until), - Platform::Threads => app(ThreadsAnalytics::class)->getMetrics($account, $since, $until), - Platform::Facebook => app(FacebookAnalytics::class)->getMetrics($account, $since, $until), - Platform::X => app(XAnalytics::class)->getMetrics($account, $since, $until), - Platform::LinkedInPage => app(LinkedInPageAnalytics::class)->getMetrics($account, $since, $until), - Platform::Pinterest => app(PinterestAnalytics::class)->getMetrics($account, $since, $until), - Platform::YouTube => app(YouTubeAnalytics::class)->getMetrics($account, $since, $until), - Platform::Telegram => app(TelegramAnalytics::class)->getMetrics($account), - default => [], - }; + $metrics = $this->metricsFor($account, $since, $until); return response()->json(['metrics' => $metrics]); } + + /** + * An unreachable platform is not a server error — empty numbers beat a 500 + * on a page the user just opened. Narrow on purpose: catching Throwable + * would render a defect as "this account has no activity". + * + * @return array + */ + private function metricsFor(SocialAccount $account, ?Carbon $since, ?Carbon $until): array + { + try { + return match ($account->platform) { + Platform::TikTok => app(TikTokAnalytics::class)->getMetrics($account), + Platform::Instagram, Platform::InstagramFacebook => app(InstagramAnalytics::class)->getMetrics($account, $since, $until), + Platform::Threads => app(ThreadsAnalytics::class)->getMetrics($account, $since, $until), + Platform::Facebook => app(FacebookAnalytics::class)->getMetrics($account, $since, $until), + Platform::X => app(XAnalytics::class)->getMetrics($account, $since, $until), + Platform::LinkedInPage => app(LinkedInPageAnalytics::class)->getMetrics($account, $since, $until), + Platform::Pinterest => app(PinterestAnalytics::class)->getMetrics($account, $since, $until), + Platform::YouTube => app(YouTubeAnalytics::class)->getMetrics($account, $since, $until), + Platform::Telegram => app(TelegramAnalytics::class)->getMetrics($account), + default => [], + }; + } catch (PlatformUnavailableException|ConnectionException $e) { + report($e); + + return []; + } + } } diff --git a/app/Jobs/RefreshSocialToken.php b/app/Jobs/RefreshSocialToken.php index c3678c96a..c83a85736 100644 --- a/app/Jobs/RefreshSocialToken.php +++ b/app/Jobs/RefreshSocialToken.php @@ -8,29 +8,41 @@ use App\Exceptions\TokenExpiredException; use App\Models\SocialAccount; use App\Services\Social\ConnectionVerifier; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Foundation\Queue\Queueable; +use Illuminate\Http\Client\ConnectionException; use Illuminate\Support\Facades\Log; use Throwable; -class RefreshSocialToken implements ShouldQueue +class RefreshSocialToken implements ShouldBeUnique, ShouldQueue { use Queueable; public int $tries = 1; + // token_expires_at only moves once this job runs, so a backlogged queue + // would stack one job per tick for the same account. + public int $uniqueFor = 900; + public function __construct(public SocialAccount $account) {} + public function uniqueId(): string + { + return $this->account->id; + } + + /** + * Refresh outright rather than verifying first: a refresh replaces the + * access token, leaving nothing for a verify call — billed as a "User: + * Read" on X — to confirm. + */ public function handle(ConnectionVerifier $verifier): void { try { - if ($this->account->platform->extendsAccessTokenOnRefresh()) { - // Instagram/Threads extend the long-lived token itself and - // can't be refreshed once expired, so extend it while it's - // still valid instead of waiting for it to fail. - $verifier->refreshToken($this->account); - } else { - $verifier->verify($this->account); + if ($verifier->refreshToken($this->account)) { + $this->recordVerification(); } } catch (PlatformUnavailableException $e) { Log::warning('Token refresh skipped: platform unavailable', [ @@ -39,6 +51,15 @@ public function handle(ConnectionVerifier $verifier): void 'error' => $e->getMessage(), ]); } catch (TokenExpiredException $e) { + // Instagram and Threads extend their token in place and cannot + // renew it once expired, so a rejected extension means the + // connection is already doomed — say so while reconnecting still + // helps. Elsewhere a rejection often just means we lost a race. + if (! $this->account->platform->extendsAccessTokenOnRefresh() + && $this->accessTokenStillWorks($verifier)) { + return; + } + $this->account->markAsTokenExpired($e->getMessage()); } catch (Throwable $e) { Log::warning('Proactive token refresh failed', [ @@ -48,4 +69,48 @@ public function handle(ConnectionVerifier $verifier): void ]); } } + + /** + * The only place this job reaches the (billed) verify endpoint, and only + * once a refresh has been rejected — which on X and Bluesky usually means + * a concurrent refresh consumed the single-use refresh_token first. + */ + private function accessTokenStillWorks(ConnectionVerifier $verifier): bool + { + try { + // The winner of that race persisted a new pair; ours is stale. + $this->account->refresh(); + + if (! $verifier->verifyAccessToken($this->account)) { + return false; + } + + $this->recordVerification(); + + return true; + } catch (TokenExpiredException) { + return false; + } catch (PlatformUnavailableException|ConnectionException|ModelNotFoundException $e) { + // Only these three earn the benefit of the doubt. Reading any other + // failure as "healthy" leaves the account Connected forever while + // every publish hard-fails. + Log::warning('Access token fallback check failed after a rejected refresh', [ + 'account_id' => $this->account->id, + 'platform' => $this->account->platform->value, + 'error' => $e->getMessage(), + ]); + + return true; + } + } + + /** + * Lets the daily sweep and the pre-publish check skip a verify of their + * own. Not done inside refreshToken(): refreshThenVerify() calls that too + * and can still fail on the verify that follows. + */ + private function recordVerification(): void + { + $this->account->update(['last_verified_at' => now()]); + } } diff --git a/app/Jobs/VerifyWorkspaceConnections.php b/app/Jobs/VerifyWorkspaceConnections.php index 128965d95..00973751a 100644 --- a/app/Jobs/VerifyWorkspaceConnections.php +++ b/app/Jobs/VerifyWorkspaceConnections.php @@ -26,6 +26,12 @@ class VerifyWorkspaceConnections implements ShouldQueue public int $timeout = 120; + // A refresh stamps last_verified_at and replaces the access token, so + // there is nothing left for a billed read to confirm. On short-TTL + // platforms the stamp is never stale here and this sweep stops calling + // verify() entirely — intended, not an oversight. + private const VERIFIED_WITHIN_HOURS = 12; + public function __construct(public Workspace $workspace) {} public function handle(ConnectionVerifier $verifier): void @@ -42,12 +48,11 @@ public function handle(ConnectionVerifier $verifier): void $disconnectedAccounts = collect(); foreach ($accounts as $account) { - if ($this->verifyAccount($verifier, $account)) { - // If was TokenExpired but now verified OK, mark as connected again - if ($account->status === Status::TokenExpired) { - $account->markAsConnected(); - } + if ($this->recentlyProvenValid($account)) { + continue; + } + if ($this->verifyAccount($verifier, $account)) { continue; } @@ -59,10 +64,28 @@ public function handle(ConnectionVerifier $verifier): void } } + /** + * Connected only: verifying a TokenExpired account is how it gets promoted + * back, so a stale stamp would strand one that has recovered. + */ + private function recentlyProvenValid(SocialAccount $account): bool + { + return $account->status === Status::Connected + && $account->last_verified_at !== null + && $account->last_verified_at->isAfter(now()->subHours(self::VERIFIED_WITHIN_HOURS)); + } + private function verifyAccount(ConnectionVerifier $verifier, SocialAccount $account): bool { try { $verifier->verify($account); + $account->update(['last_verified_at' => now()]); + + // Here, not on this method's return value — that is also true + // for "could not check, don't disconnect". + if ($account->status === Status::TokenExpired) { + $account->markAsConnected(); + } return true; } catch (PlatformUnavailableException $e) { diff --git a/app/Services/Social/ConnectionVerifier.php b/app/Services/Social/ConnectionVerifier.php index 954e83298..7c38c91f0 100644 --- a/app/Services/Social/ConnectionVerifier.php +++ b/app/Services/Social/ConnectionVerifier.php @@ -20,11 +20,30 @@ use App\Services\Social\Discord\DiscordClient; use App\Services\Social\Meta\GraphError; use App\Services\Social\Telegram\TelegramApi; +use Illuminate\Http\Client\PendingRequest; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Http; class ConnectionVerifier { + /** + * Read and connect timeouts for a token refresh. Stated explicitly, even + * though they match the client's defaults, so a change to those cannot + * silently break the lock invariant below. Generous on purpose: giving up + * on a request the provider already processed loses a single-use + * refresh_token for good. + */ + public const REFRESH_TIMEOUT_SECONDS = 30; + + public const REFRESH_CONNECT_TIMEOUT_SECONDS = 10; + + /** + * Must exceed the slowest refresh the timeouts above allow — Bluesky's two + * sequential calls — or the lock lapses mid-flight and a second process + * reuses the same single-use refresh_token. Pinned by a test. + */ + public const REFRESH_LOCK_SECONDS = 120; + /** * Verify that a social account connection is still valid. * @@ -36,10 +55,9 @@ public function verify(SocialAccount $account): bool // Hard-expired tokens cannot make API calls — refresh is mandatory. // For tokens that are still valid OR only "expiring soon", try the // verify endpoint FIRST with the current access_token. This avoids - // rotating refresh_tokens unnecessarily — many providers (X v2, - // LinkedIn, etc.) invalidate the previous refresh_token on each - // refresh, so proactive refreshes during races cause false-positive - // disconnects even though the access_token still works fine. + // rotating refresh_tokens unnecessarily — X and Bluesky single-use + // theirs, so refreshing during a race disconnects an account whose + // access_token still works. (LinkedIn returns the same token.) if ($account->is_token_expired) { return $this->refreshThenVerify($account); } @@ -96,6 +114,57 @@ private function refreshThenVerify(SocialAccount $account, ?TokenExpiredExceptio } } + /** + * Pull a token out of a refresh response, refusing to persist a blank one. + * + * TokenRefreshClient classifies on HTTP status alone, so a 200 carrying no + * token would otherwise overwrite a credential that still works. + * + * @param array|null $data + * + * @throws PlatformUnavailableException + */ + private function rotatedTokenFrom(?array $data, string $key, string $current): string + { + $token = data_get($data, $key); + + // Blank, not just missing: data_get()'s own default lets an explicit + // null through and overwrite. + return blank($token) ? $current : (string) $token; + } + + private function tokenFrom(?array $data, Platform $platform, string $key = 'access_token'): string + { + $token = data_get($data, $key); + + if (blank($token)) { + throw new PlatformUnavailableException( + "{$platform->label()} returned a successful refresh with no {$key}." + ); + } + + return (string) $token; + } + + private function refreshHttp(): PendingRequest + { + return Http::timeout(self::REFRESH_TIMEOUT_SECONDS) + ->connectTimeout(self::REFRESH_CONNECT_TIMEOUT_SECONDS); + } + + /** + * Check the stored access token as it is, skipping the refresh-and-retry + * ladder verify() runs — which would re-send a refresh_token the provider + * just rejected, and on Bluesky re-run a rate-limited password re-auth. + * + * @throws TokenExpiredException if the access token itself is rejected + * @throws PlatformUnavailableException if the platform is unreachable + */ + public function verifyAccessToken(SocialAccount $account): bool + { + return $this->callVerifyEndpoint($account); + } + /** * @throws TokenExpiredException */ @@ -124,21 +193,39 @@ private function callVerifyEndpoint(SocialAccount $account): bool * use verify() instead. This method always attempts a refresh under * the per-account lock. * + * @return bool whether a refresh actually ran — false means another + * process held the lock and this call proved nothing. + * * @throws TokenExpiredException if refresh is rejected by the provider (4xx) * @throws PlatformUnavailableException if the platform is unreachable (5xx / network) */ - public function refreshToken(SocialAccount $account): void + public function refreshToken(SocialAccount $account): bool { - $lock = Cache::lock("token_refresh:{$account->id}", 30); + $lock = Cache::lock("token_refresh:{$account->id}", self::REFRESH_LOCK_SECONDS); if (! $lock->get()) { - // Another process is already refreshing this token + // Another process is already refreshing this token. $account->refresh(); - return; + if ($account->is_token_expired) { + // Returning false would hand the caller a token it knows is + // dead; a publisher then posts with it, fails the post and + // disconnects the account. Transient is the truth here. + throw new PlatformUnavailableException( + "A {$account->platform->label()} token refresh is already in progress." + ); + } + + return false; } try { + if (! $account->platform->hasTokenRefreshFlow()) { + // Page tokens, Mastodon and the shared bot tokens have + // nothing per-account to refresh. + return false; + } + match ($account->platform) { Platform::LinkedIn, Platform::LinkedInPage => $this->refreshLinkedInToken($account), Platform::X => $this->refreshXToken($account), @@ -148,10 +235,9 @@ public function refreshToken(SocialAccount $account): void Platform::Pinterest => $this->refreshPinterestToken($account), Platform::Threads => $this->refreshThreadsToken($account), Platform::Instagram => $this->refreshInstagramToken($account), - // Facebook / InstagramFacebook use Page tokens that don't expire. - // Mastodon tokens don't expire either. - default => null, }; + + return true; } finally { $lock->release(); } @@ -163,7 +249,7 @@ private function refreshLinkedInToken(SocialAccount $account): void throw new TokenExpiredException("No refresh token available for {$account->platform->label()} account"); } - $response = TokenRefreshClient::for($account->platform)->send(fn () => Http::asForm() + $response = TokenRefreshClient::for($account->platform)->send(fn () => $this->refreshHttp()->asForm() ->post(config('trypost.platforms.linkedin.oauth_api').'/oauth/v2/accessToken', [ 'grant_type' => 'refresh_token', 'refresh_token' => $account->refresh_token, @@ -174,8 +260,8 @@ private function refreshLinkedInToken(SocialAccount $account): void $data = $response->json(); $account->update([ - 'access_token' => data_get($data, 'access_token'), - 'refresh_token' => data_get($data, 'refresh_token', $account->refresh_token), + 'access_token' => $this->tokenFrom($data, $account->platform), + 'refresh_token' => $this->rotatedTokenFrom($data, 'refresh_token', $account->refresh_token), 'token_expires_at' => data_get($data, 'expires_in') ? now()->addSeconds(data_get($data, 'expires_in')) : null, ]); @@ -188,7 +274,7 @@ private function refreshXToken(SocialAccount $account): void throw new TokenExpiredException('No refresh token available for X account'); } - $response = TokenRefreshClient::for(Platform::X)->send(fn () => Http::asForm() + $response = TokenRefreshClient::for(Platform::X)->send(fn () => $this->refreshHttp()->asForm() ->withBasicAuth(config('services.x.client_id'), config('services.x.client_secret')) ->post(config('trypost.platforms.x.api').'/oauth2/token', [ 'grant_type' => 'refresh_token', @@ -198,8 +284,8 @@ private function refreshXToken(SocialAccount $account): void $data = $response->json(); $account->update([ - 'access_token' => data_get($data, 'access_token'), - 'refresh_token' => data_get($data, 'refresh_token', $account->refresh_token), + 'access_token' => $this->tokenFrom($data, $account->platform), + 'refresh_token' => $this->rotatedTokenFrom($data, 'refresh_token', $account->refresh_token), 'token_expires_at' => now()->addSeconds(data_get($data, 'expires_in', $account->platform->defaultTokenTtlSeconds())), ]); @@ -212,13 +298,13 @@ private function refreshBlueskyToken(SocialAccount $account): void $client = TokenRefreshClient::for(Platform::Bluesky); try { - $response = $client->send(fn () => Http::withToken($account->refresh_token) + $response = $client->send(fn () => $this->refreshHttp()->withToken($account->refresh_token) ->post("{$service}/xrpc/".BlueskyLexicon::REFRESH_SESSION)); $data = $response->json(); $account->update([ - 'access_token' => data_get($data, 'accessJwt'), - 'refresh_token' => data_get($data, 'refreshJwt'), + 'access_token' => $this->tokenFrom($data, $account->platform, 'accessJwt'), + 'refresh_token' => $this->tokenFrom($data, $account->platform, 'refreshJwt'), 'token_expires_at' => now()->addHours(2), ]); @@ -231,15 +317,15 @@ private function refreshBlueskyToken(SocialAccount $account): void if (isset($account->meta['password'])) { try { - $reauth = $client->send(fn () => Http::post("{$service}/xrpc/".BlueskyLexicon::CREATE_SESSION, [ + $reauth = $client->send(fn () => $this->refreshHttp()->post("{$service}/xrpc/".BlueskyLexicon::CREATE_SESSION, [ 'identifier' => $account->meta['identifier'], 'password' => decrypt($account->meta['password']), ])); $data = $reauth->json(); $account->update([ - 'access_token' => data_get($data, 'accessJwt'), - 'refresh_token' => data_get($data, 'refreshJwt'), + 'access_token' => $this->tokenFrom($data, $account->platform, 'accessJwt'), + 'refresh_token' => $this->tokenFrom($data, $account->platform, 'refreshJwt'), 'token_expires_at' => now()->addHours(2), ]); @@ -260,7 +346,7 @@ private function refreshYouTubeToken(SocialAccount $account): void throw new TokenExpiredException('No refresh token available for YouTube account'); } - $response = TokenRefreshClient::for(Platform::YouTube)->send(fn () => Http::asForm() + $response = TokenRefreshClient::for(Platform::YouTube)->send(fn () => $this->refreshHttp()->asForm() ->post(config('trypost.platforms.youtube.oauth_api').'/token', [ 'grant_type' => 'refresh_token', 'refresh_token' => $account->refresh_token, @@ -271,7 +357,7 @@ private function refreshYouTubeToken(SocialAccount $account): void $data = $response->json(); $account->update([ - 'access_token' => data_get($data, 'access_token'), + 'access_token' => $this->tokenFrom($data, $account->platform), 'token_expires_at' => data_get($data, 'expires_in') ? now()->addSeconds(data_get($data, 'expires_in')) : null, ]); @@ -284,7 +370,7 @@ private function refreshTikTokToken(SocialAccount $account): void throw new TokenExpiredException('No refresh token available for TikTok account'); } - $response = TokenRefreshClient::for(Platform::TikTok)->send(fn () => Http::asForm() + $response = TokenRefreshClient::for(Platform::TikTok)->send(fn () => $this->refreshHttp()->asForm() ->post(config('trypost.platforms.tiktok.api').'/oauth/token/', [ 'grant_type' => 'refresh_token', 'refresh_token' => $account->refresh_token, @@ -295,8 +381,8 @@ private function refreshTikTokToken(SocialAccount $account): void $data = $response->json(); $account->update([ - 'access_token' => data_get($data, 'access_token'), - 'refresh_token' => data_get($data, 'refresh_token', $account->refresh_token), + 'access_token' => $this->tokenFrom($data, $account->platform), + 'refresh_token' => $this->rotatedTokenFrom($data, 'refresh_token', $account->refresh_token), 'token_expires_at' => data_get($data, 'expires_in') ? now()->addSeconds(data_get($data, 'expires_in')) : null, ]); @@ -311,7 +397,7 @@ private function refreshPinterestToken(SocialAccount $account): void $credentials = base64_encode(config('services.pinterest.client_id').':'.config('services.pinterest.client_secret')); - $response = TokenRefreshClient::for(Platform::Pinterest)->send(fn () => Http::withHeaders([ + $response = TokenRefreshClient::for(Platform::Pinterest)->send(fn () => $this->refreshHttp()->withHeaders([ 'Authorization' => "Basic {$credentials}", 'Content-Type' => 'application/x-www-form-urlencoded', ])->asForm()->post(config('trypost.platforms.pinterest.api').'/oauth/token', [ @@ -322,8 +408,8 @@ private function refreshPinterestToken(SocialAccount $account): void $data = $response->json(); $account->update([ - 'access_token' => data_get($data, 'access_token'), - 'refresh_token' => data_get($data, 'refresh_token', $account->refresh_token), + 'access_token' => $this->tokenFrom($data, $account->platform), + 'refresh_token' => $this->rotatedTokenFrom($data, 'refresh_token', $account->refresh_token), 'token_expires_at' => data_get($data, 'expires_in') ? now()->addSeconds(data_get($data, 'expires_in')) : null, ]); @@ -334,7 +420,7 @@ private function refreshThreadsToken(SocialAccount $account): void { // Threads uses long-lived tokens that can be refreshed $response = TokenRefreshClient::for(Platform::Threads)->send( - fn () => Http::get(config('trypost.platforms.threads.auth_api').'/refresh_access_token', [ + fn () => $this->refreshHttp()->get(config('trypost.platforms.threads.auth_api').'/refresh_access_token', [ 'grant_type' => 'th_refresh_token', 'access_token' => $account->access_token, ]), @@ -342,7 +428,7 @@ private function refreshThreadsToken(SocialAccount $account): void ); $data = $response->json(); - $newToken = data_get($data, 'access_token'); + $newToken = $this->tokenFrom($data, $account->platform); $account->update([ 'access_token' => $newToken, @@ -356,7 +442,7 @@ private function refreshThreadsToken(SocialAccount $account): void private function refreshInstagramToken(SocialAccount $account): void { $response = TokenRefreshClient::for(Platform::Instagram)->send( - fn () => Http::get(config('trypost.platforms.instagram.auth_api').'/refresh_access_token', [ + fn () => $this->refreshHttp()->get(config('trypost.platforms.instagram.auth_api').'/refresh_access_token', [ 'grant_type' => 'ig_refresh_token', 'access_token' => $account->access_token, ]), @@ -364,7 +450,7 @@ private function refreshInstagramToken(SocialAccount $account): void ); $data = $response->json(); - $newToken = data_get($data, 'access_token'); + $newToken = $this->tokenFrom($data, $account->platform); $account->update([ 'access_token' => $newToken, diff --git a/app/Services/Social/XAnalytics.php b/app/Services/Social/XAnalytics.php index 618a87eff..dee5cc583 100644 --- a/app/Services/Social/XAnalytics.php +++ b/app/Services/Social/XAnalytics.php @@ -16,6 +16,9 @@ class XAnalytics { use HasSocialHttpClient; + /** Each page is billed per Post returned, so this bounds cost as well as time. */ + private const MAX_TIMELINE_PAGES = 5; + private string $baseUrl; private string $accessToken; @@ -52,27 +55,51 @@ private function fetchMetricsFromApi(SocialAccount $account, CarbonInterface $si $this->accessToken = $account->access_token; - // Fetch recent tweets in the period - $tweetIds = $this->fetchTweetIds($account, $since, $until); + [$totals, $tweetCount] = $this->fetchTimelineMetrics($account, $since, $until); - if (empty($tweetIds)) { + if ($tweetCount === 0) { return []; } - // Fetch public_metrics for those tweets - return $this->fetchTweetMetrics($tweetIds); + return [ + ['label' => __('analytics.metrics.impressions'), 'value' => $totals['impression_count']], + ['label' => __('analytics.metrics.likes'), 'value' => $totals['like_count']], + ['label' => __('analytics.metrics.retweets'), 'value' => $totals['retweet_count']], + ['label' => __('analytics.metrics.replies'), 'value' => $totals['reply_count']], + ['label' => __('analytics.metrics.quotes'), 'value' => $totals['quote_count']], + ['label' => __('analytics.metrics.bookmarks'), 'value' => $totals['bookmark_count']], + ]; } - private function fetchTweetIds(SocialAccount $account, CarbonInterface $since, CarbonInterface $until): array + /** + * Walk the timeline, summing public_metrics as the pages come back. + * + * Asked for on the timeline request rather than looked up afterwards from + * /2/tweets: both bill per Post returned, so re-reading the same ids only + * bought a second round-trip. + * + * @return array{0: array, 1: int} totals, and how many Posts fed them + */ + private function fetchTimelineMetrics(SocialAccount $account, CarbonInterface $since, CarbonInterface $until): array { - $ids = []; + $totals = [ + 'impression_count' => 0, + 'like_count' => 0, + 'retweet_count' => 0, + 'reply_count' => 0, + 'quote_count' => 0, + 'bookmark_count' => 0, + ]; + + $tweetCount = 0; $paginationToken = null; - for ($i = 0; $i < 5; $i++) { + for ($page = 0; $page < self::MAX_TIMELINE_PAGES; $page++) { $params = [ 'start_time' => $since->toIso8601ZuluString(), 'end_time' => $until->toIso8601ZuluString(), 'max_results' => 100, + 'tweet.fields' => 'public_metrics', ]; if ($paginationToken) { @@ -90,10 +117,14 @@ private function fetchTweetIds(SocialAccount $account, CarbonInterface $since, C } $data = $response->json(); - $tweets = data_get($data, 'data', []); - foreach ($tweets as $tweet) { - $ids[] = data_get($tweet, 'id'); + foreach (data_get($data, 'data', []) as $tweet) { + $tweetCount++; + $metrics = data_get($tweet, 'public_metrics', []); + + foreach (array_keys($totals) as $metric) { + $totals[$metric] += (int) data_get($metrics, $metric, 0); + } } $paginationToken = data_get($data, 'meta.next_token'); @@ -103,54 +134,7 @@ private function fetchTweetIds(SocialAccount $account, CarbonInterface $since, C } } - return $ids; - } - - private function fetchTweetMetrics(array $tweetIds): array - { - $totals = [ - 'impression_count' => 0, - 'like_count' => 0, - 'retweet_count' => 0, - 'reply_count' => 0, - 'quote_count' => 0, - 'bookmark_count' => 0, - ]; - - // X API allows max 100 IDs per request - foreach (array_chunk($tweetIds, 100) as $chunk) { - $response = $this->getHttpClient() - ->get("{$this->baseUrl}/tweets", [ - 'ids' => implode(',', $chunk), - 'tweet.fields' => 'public_metrics', - ]); - - if ($response->failed()) { - Log::warning('X tweets metrics fetch failed', [ - 'body' => $this->redactResponseBody($response->body()), - ]); - - continue; - } - - $tweets = data_get($response->json(), 'data', []); - - foreach ($tweets as $tweet) { - $metrics = data_get($tweet, 'public_metrics', []); - foreach ($totals as $key => &$total) { - $total += data_get($metrics, $key, 0); - } - } - } - - return [ - ['label' => __('analytics.metrics.impressions'), 'value' => $totals['impression_count']], - ['label' => __('analytics.metrics.likes'), 'value' => $totals['like_count']], - ['label' => __('analytics.metrics.retweets'), 'value' => $totals['retweet_count']], - ['label' => __('analytics.metrics.replies'), 'value' => $totals['reply_count']], - ['label' => __('analytics.metrics.quotes'), 'value' => $totals['quote_count']], - ['label' => __('analytics.metrics.bookmarks'), 'value' => $totals['bookmark_count']], - ]; + return [$totals, $tweetCount]; } public function fetchPostMetrics(PostPlatform $postPlatform): array diff --git a/tests/Feature/AnalyticsResilienceTest.php b/tests/Feature/AnalyticsResilienceTest.php new file mode 100644 index 000000000..a1928d220 --- /dev/null +++ b/tests/Feature/AnalyticsResilienceTest.php @@ -0,0 +1,60 @@ +create(); + $workspace = Workspace::factory()->create(['user_id' => $user->id]); + $user->update(['current_workspace_id' => $workspace->id]); + + $account = SocialAccount::factory()->x()->create([ + 'workspace_id' => $workspace->id, + 'status' => Status::Connected, + 'platform_user_id' => '4242', + // Expired, so the analytics service tries to refresh before reading. + 'token_expires_at' => now()->subMinutes(5), + ]); + + Http::fake(['*' => Http::response(['data' => [], 'meta' => []], 200)]); + + // The scheduled RefreshSocialToken is mid-refresh for this account. + Cache::lock("token_refresh:{$account->id}", 120)->get(); + + $response = $this->actingAs($user)->getJson(route('app.analytics.show', $account)); + + // A transient collision is not a server error. Before the lock started + // reporting itself as transient this returned empty metrics, and a 500 on + // a page the user just opened is a worse answer than no numbers. + $response->assertOk(); + expect($response->json('metrics'))->toBe([]); +}); + +test('a bug in a metrics service is not hidden behind empty numbers', function () { + $user = User::factory()->create(); + $workspace = Workspace::factory()->create(['user_id' => $user->id]); + $user->update(['current_workspace_id' => $workspace->id]); + + $account = SocialAccount::factory()->x()->create([ + 'workspace_id' => $workspace->id, + 'status' => Status::Connected, + 'token_expires_at' => now()->addHours(2), + ]); + + $this->mock(XAnalytics::class) + ->shouldReceive('getMetrics') + ->andThrow(new RuntimeException('a real bug, not the platform being down')); + + // Degrading to [] here would show the user an empty dashboard and leave a + // genuine defect looking like "this account has no activity". + $this->actingAs($user) + ->getJson(route('app.analytics.show', $account)) + ->assertStatus(500); +}); diff --git a/tests/Feature/Commands/RefreshExpiringTokensTest.php b/tests/Feature/Commands/RefreshExpiringTokensTest.php index 957f8f7f6..0eb389bfe 100644 --- a/tests/Feature/Commands/RefreshExpiringTokensTest.php +++ b/tests/Feature/Commands/RefreshExpiringTokensTest.php @@ -127,3 +127,38 @@ Queue::assertNothingPushed(); }); + +test('a backed-up queue cannot stack duplicate refresh jobs for one account', function () { + Queue::fake(); + + SocialAccount::factory()->x()->create([ + 'workspace_id' => Workspace::factory()->create()->id, + 'status' => Status::Connected, + 'token_expires_at' => now()->addMinutes(20), + ]); + + // Two scheduler ticks before the first job got a worker: token_expires_at + // has not moved, so the account is still inside the window. + $this->artisan('social:refresh-expiring-tokens'); + $this->artisan('social:refresh-expiring-tokens'); + + // Each extra job rotates a single-use refresh_token again for nothing, and + // widens the window where a worker death loses the pair. + Queue::assertPushed(RefreshSocialToken::class, 1); +}); + +test('the command reports accounts in the window, not jobs it cannot know landed', function () { + Queue::fake(); + + SocialAccount::factory()->x()->create([ + 'workspace_id' => Workspace::factory()->create()->id, + 'status' => Status::Connected, + 'token_expires_at' => now()->addMinutes(20), + ]); + + // RefreshSocialToken is unique per account, so a second dispatch while the + // first is in flight is silently discarded. dispatch() still returns a + // PendingDispatch either way, so a "dispatched" count would be a guess. + $this->artisan('social:refresh-expiring-tokens') + ->expectsOutput('1 accounts due for a token refresh.'); +}); diff --git a/tests/Feature/Jobs/RefreshSocialTokenTest.php b/tests/Feature/Jobs/RefreshSocialTokenTest.php index 898950c30..373605f41 100644 --- a/tests/Feature/Jobs/RefreshSocialTokenTest.php +++ b/tests/Feature/Jobs/RefreshSocialTokenTest.php @@ -12,6 +12,8 @@ use App\Models\User; use App\Models\Workspace; use App\Services\Social\ConnectionVerifier; +use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Queue; @@ -26,23 +28,22 @@ ]); }); -test('refresh job routes through verify (access-token-first) not refreshToken', function () { +test('refresh job routes through refreshToken, never the billed verify endpoint', function () { $verifier = mock(ConnectionVerifier::class); - $verifier->shouldReceive('verify')->once()->with( + $verifier->shouldReceive('refreshToken')->once()->with( Mockery::on(fn ($account) => $account->id === $this->account->id) - ); - $verifier->shouldNotReceive('refreshToken'); + )->andReturnTrue(); + $verifier->shouldNotReceive('verify'); app()->instance(ConnectionVerifier::class, $verifier); (new RefreshSocialToken($this->account))->handle($verifier); }); -test('proactive refresh does NOT rotate the X refresh token while the access token still works', function () { +test('proactive refresh rotates the X refresh token without disconnecting the account', function () { Http::fake([ - config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200), config('trypost.platforms.x.api').'/oauth2/token' => Http::response([ - 'access_token' => 'should-not-be-used', - 'refresh_token' => 'should-not-be-used', + 'access_token' => 'rotated-access-token', + 'refresh_token' => 'rotated-refresh-token', 'expires_in' => 7200, ], 200), ]); @@ -55,9 +56,9 @@ (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); - Http::assertSent(fn ($request) => str_contains($request->url(), '/users/me')); - Http::assertNotSent(fn ($request) => str_contains($request->url(), '/oauth2/token')); - expect($this->account->fresh()->refresh_token)->toBe('original-refresh-token'); + // X single-uses the refresh_token, so rotating one proactively has to leave + // the account healthy instead of tripping a false-positive disconnect. + expect($this->account->fresh()->refresh_token)->toBe('rotated-refresh-token'); expect($this->account->fresh()->status)->toBe(Status::Connected); }); @@ -133,9 +134,13 @@ Queue::fake(); $verifier = mock(ConnectionVerifier::class); - $verifier->shouldReceive('verify')->once()->andThrow( + $verifier->shouldReceive('refreshToken')->once()->andThrow( new TokenExpiredException('refresh_token revoked') ); + // The access_token is dead too, so there is nothing left to fall back to. + $verifier->shouldReceive('verifyAccessToken')->once()->andThrow( + new TokenExpiredException('X access token is invalid or expired') + ); app()->instance(ConnectionVerifier::class, $verifier); (new RefreshSocialToken($this->account))->handle($verifier); @@ -155,7 +160,7 @@ }); $verifier = mock(ConnectionVerifier::class); - $verifier->shouldReceive('verify')->once()->andThrow(new RuntimeException('network blip')); + $verifier->shouldReceive('refreshToken')->once()->andThrow(new RuntimeException('network blip')); app()->instance(ConnectionVerifier::class, $verifier); (new RefreshSocialToken($this->account))->handle($verifier); @@ -173,13 +178,574 @@ }); $verifier = mock(ConnectionVerifier::class); - $verifier->shouldReceive('verify')->once()->andThrow( + $verifier->shouldReceive('refreshToken')->once()->andThrow( + new PlatformUnavailableException('X API returned 503 during token refresh', 503) + ); + app()->instance(ConnectionVerifier::class, $verifier); + + (new RefreshSocialToken($this->account))->handle($verifier); + + expect($this->account->fresh()->status)->toBe(Status::Connected); + Queue::assertNotPushed(SendNotification::class); +}); + +test('proactive refresh renews a still-valid X token without spending a billed user read', function () { + Http::fake([ + config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200), + config('trypost.platforms.x.api').'/oauth2/token' => Http::response([ + 'access_token' => 'rotated-access-token', + 'refresh_token' => 'rotated-refresh-token', + 'expires_in' => 7200, + ], 200), + ]); + + $this->account->update([ + 'access_token' => 'original-access-token', + 'token_expires_at' => now()->addMinutes(20), + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + // GET /2/users/me is a billed "User: Read" ($0.010). A successful token + // refresh already proves the credential works, so it must not be called. + Http::assertNotSent(fn ($request) => str_contains($request->url(), '/users/me')); + Http::assertSent(fn ($request) => str_contains($request->url(), '/oauth2/token')); + + expect($this->account->fresh()->access_token)->toBe('rotated-access-token'); + expect($this->account->fresh()->token_expires_at->isAfter(now()->addHour()))->toBeTrue(); +}); + +test('a successful refresh stamps last_verified_at so other jobs can skip verifying', function () { + Http::fake([ + config('trypost.platforms.x.api').'/oauth2/token' => Http::response([ + 'access_token' => 'rotated-access-token', + 'refresh_token' => 'rotated-refresh-token', + 'expires_in' => 7200, + ], 200), + ]); + + $this->account->update([ + 'token_expires_at' => now()->addMinutes(20), + 'last_verified_at' => null, + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + expect($this->account->fresh()->last_verified_at)->not->toBeNull(); +}); + +test('a rejected refresh does not disconnect an account whose access token still works', function () { + Queue::fake(); + + Http::fake([ + // X single-uses the refresh_token; a concurrent refresh already burned + // this one, so the provider rejects it — but the access_token is alive. + config('trypost.platforms.x.api').'/oauth2/token' => Http::response([ + 'error' => 'invalid_grant', + 'error_description' => 'Value passed for the token was invalid.', + ], 400), + config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200), + ]); + + $this->account->update([ + 'access_token' => 'still-valid-access-token', + 'refresh_token' => 'already-consumed-by-a-race', + 'token_expires_at' => now()->addMinutes(20), + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + // PublishToSocialPlatform hard-fails posts for a TokenExpired account, so + // disconnecting here would kill posts the access_token could still publish. + expect($this->account->fresh()->status)->toBe(Status::Connected); + Queue::assertNotPushed(SendNotification::class); +}); + +test('an account with no refresh token stays connected while its access token works', function () { + Queue::fake(); + + Http::fake([ + config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200), + ]); + + $this->account->update([ + 'access_token' => 'still-valid-access-token', + 'refresh_token' => null, + 'token_expires_at' => now()->addMinutes(20), + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + expect($this->account->fresh()->status)->toBe(Status::Connected); + Queue::assertNotPushed(SendNotification::class); +}); + +test('a rejected refresh DOES disconnect once the access token is dead too', function () { + Queue::fake(); + + Http::fake([ + config('trypost.platforms.x.api').'/oauth2/token' => Http::response([ + 'error' => 'invalid_grant', + 'error_description' => 'refresh_token revoked', + ], 400), + config('trypost.platforms.x.api').'/users/me' => Http::response([ + 'title' => 'Unauthorized', + 'status' => 401, + ], 401), + ]); + + $this->account->update([ + 'access_token' => 'dead-access-token', + 'refresh_token' => 'revoked-refresh-token', + 'token_expires_at' => now()->addMinutes(20), + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + expect($this->account->fresh()->status)->toBe(Status::TokenExpired); +}); + +test('lock skipped by a concurrent refresh does not record a verification', function () { + Http::fake([config('trypost.platforms.x.api').'/*' => Http::response([], 200)]); + + $this->account->update([ + 'last_verified_at' => null, + 'token_expires_at' => now()->addMinutes(20), + ]); + + Cache::lock("token_refresh:{$this->account->id}", 30)->get(); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + // Nothing was refreshed here, so nothing was proven — stamping would let + // the daily sweep skip an account no one actually checked. + Http::assertNothingSent(); + expect($this->account->fresh()->last_verified_at)->toBeNull(); +}); + +test('a platform with nothing to refresh is never recorded as verified', function () { + $account = SocialAccount::factory()->mastodon()->create([ + 'workspace_id' => $this->workspace->id, + 'status' => Status::Connected, + 'last_verified_at' => null, + ]); + + app(ConnectionVerifier::class)->refreshToken($account); + + expect($account->fresh()->last_verified_at)->toBeNull(); +}); + +test('a refresh whose follow-up verify fails is not recorded as a verification', function () { + Http::fake([ + config('trypost.platforms.x.api').'/oauth2/token' => Http::response([ + 'access_token' => 'fresh-but-rejected', + 'refresh_token' => 'rt-new', + 'expires_in' => 7200, + ], 200), + config('trypost.platforms.x.api').'/users/me' => Http::response(['title' => 'Unauthorized'], 401), + ]); + + $this->account->update([ + 'token_expires_at' => now()->subMinute(), + 'last_verified_at' => null, + ]); + + try { + app(ConnectionVerifier::class)->verify($this->account); + } catch (TokenExpiredException) { + // expected — the refreshed token is rejected too + } + + // The refresh succeeded but the credential was never proven good. Stamping + // here lets both skip-windows wave through an account nobody verified. + expect($this->account->fresh()->last_verified_at)->toBeNull(); +}); + +test('a refresh that returns an empty access token is not recorded as a verification', function () { + // TokenRefreshClient classifies on HTTP status alone and never inspects the + // body, so a 200 carrying an empty token is stored as-is. + Http::fake([ + config('trypost.platforms.x.api').'/oauth2/token' => Http::response([ + 'access_token' => '', + 'refresh_token' => 'rt-new', + 'expires_in' => 7200, + ], 200), + ]); + + $this->account->update([ + 'token_expires_at' => now()->addMinutes(20), + 'last_verified_at' => null, + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + expect($this->account->fresh()->last_verified_at)->toBeNull(); +}); + +test('a rejected refresh is not re-sent before the access token is checked', function () { + Queue::fake(); + + Http::fake([ + config('trypost.platforms.x.api').'/oauth2/token' => Http::response([ + 'error' => 'invalid_grant', + ], 400), + config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200), + ]); + + $this->account->update([ + 'access_token' => 'still-valid-access-token', + 'refresh_token' => 'already-consumed-by-a-race', + 'token_expires_at' => now()->subMinute(), + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + // Going through verify() would re-send the refresh_token the provider just + // rejected — and on Bluesky re-run the rate-limited password re-auth. + $refreshCalls = collect(Http::recorded()) + ->filter(fn ($pair) => str_contains($pair[0]->url(), '/oauth2/token')) + ->count(); + + expect($refreshCalls)->toBe(1); +}); + +test('a refresh lost to a concurrent one falls back to the token that won', function () { + Queue::fake(); + + $api = config('trypost.platforms.x.api'); + Http::fake([ + // Our refresh_token was already consumed by the process that won. + $api.'/oauth2/token' => Http::response(['error' => 'invalid_grant'], 400), + $api.'/users/me' => function ($request) { + $auth = $request->header('Authorization')[0] ?? ''; + + return str_contains($auth, 'winner-access-token') + ? Http::response(['data' => ['id' => '123']], 200) + : Http::response(['title' => 'Unauthorized', 'status' => 401], 401); + }, + ]); + + $this->account->update([ + 'access_token' => 'stale-access-token', + 'refresh_token' => 'stale-refresh-token', + 'token_expires_at' => now()->addMinutes(20), + ]); + + // The winner persisted its new pair while ours was in flight; this + // instance still holds the rotated-away one. Written through a separate + // model so the encrypted casts apply — a raw DB write stores plaintext, + // and reading it back throws DecryptException instead of exercising this. + SocialAccount::find($this->account->id)->update([ + 'access_token' => 'winner-access-token', + 'refresh_token' => 'winner-refresh-token', + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + // The recovery is the point: reload, find the winner's token, verify with + // it. Asserting the call proves we got that far rather than bailing early. + Http::assertSent(fn ($request) => str_contains($request->url(), '/users/me')); + expect($this->account->fresh()->status)->toBe(Status::Connected); + Queue::assertNotPushed(SendNotification::class); +}); + +test('the refresh lock outlives the slowest refresh a provider can make us wait', function () { + // Bluesky refreshes with two sequential calls (refreshSession, then the + // createSession re-auth). If the lock expires first, a second process + // refreshes with the same single-use refresh_token and one of the two is + // rejected. Bounding the calls ourselves keeps that under the lock without + // holding the lock longer, which the publish path also waits on. + $worstCaseSeconds = 2 * (ConnectionVerifier::REFRESH_TIMEOUT_SECONDS + ConnectionVerifier::REFRESH_CONNECT_TIMEOUT_SECONDS); + + expect(ConnectionVerifier::REFRESH_LOCK_SECONDS)->toBeGreaterThan($worstCaseSeconds); +}); + +test('a rejected Instagram extension disconnects loudly instead of waiting for the token to die', function () { + Queue::fake(); + + $account = SocialAccount::factory()->create([ + 'workspace_id' => $this->workspace->id, + 'platform' => Platform::Instagram, + 'status' => Status::Connected, + 'access_token' => 'still-valid-but-unextendable', + 'token_expires_at' => now()->addHours(20), + ]); + + Http::fake([ + config('trypost.platforms.instagram.auth_api').'/refresh_access_token*' => Http::response([ + 'error' => ['message' => 'Invalid OAuth access token', 'type' => 'OAuthException', 'code' => 190], + ], 400), + config('trypost.platforms.instagram.graph_api').'/me*' => Http::response(['id' => '1', 'username' => 'u'], 200), + ]); + + (new RefreshSocialToken($account))->handle(app(ConnectionVerifier::class)); + + // Instagram/Threads tokens cannot be refreshed once expired. Staying + // Connected because the token still reads means the owner is told only + // after it dies — by which point reconnecting is the only option left. + expect($account->fresh()->status)->toBe(Status::TokenExpired); + Queue::assertPushed(SendNotification::class); +}); + +test('the job survives the account being deleted while it is in flight', function () { + Http::fake([ + config('trypost.platforms.x.api').'/oauth2/token' => Http::response(['error' => 'invalid_grant'], 400), + ]); + + $account = $this->account; + $account->update(['token_expires_at' => now()->addMinutes(20)]); + + SocialAccount::whereKey($account->id)->delete(); + + // Guard the repro itself: a delete that silently did nothing would make + // this test pass without ever exercising the path it claims to cover. + expect(SocialAccount::find($account->id))->toBeNull(); + + // tries = 1, so an escaping exception lands the job straight in failed_jobs. + (new RefreshSocialToken($account))->handle(app(ConnectionVerifier::class)); + + // Reaching this line is the point: the refresh ran and the vanished row + // did not escape as a ModelNotFoundException. + Http::assertSent(fn ($request) => str_contains($request->url(), '/oauth2/token')); +}); + +test('a 200 without a token leaves the working credential intact', function () { + Queue::fake(); + + Http::fake([ + config('trypost.platforms.x.api').'/oauth2/token' => Http::response([ + 'access_token' => '', + 'refresh_token' => 'rt-new', + 'expires_in' => 7200, + ], 200), + ]); + + $this->account->update([ + 'access_token' => 'the-token-that-still-works', + 'token_expires_at' => now()->addMinutes(20), + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + // Persisting the empty token would destroy a credential that still works, + // and no amount of after-the-fact detection gets it back. + expect($this->account->fresh()->access_token)->toBe('the-token-that-still-works'); + + // And it must not disconnect: the refresh_token is probably fine, so the + // next tick should retry rather than emailing the owner to reconnect. + expect($this->account->fresh()->status)->toBe(Status::Connected); + Queue::assertNotPushed(SendNotification::class); +}); + +test('refreshToken reports false for a platform with nothing to refresh', function () { + $account = SocialAccount::factory()->mastodon()->create([ + 'workspace_id' => $this->workspace->id, + 'status' => Status::Connected, + ]); + + expect(app(ConnectionVerifier::class)->refreshToken($account))->toBeFalse(); +}); + +test('every platform that claims a refresh flow actually performs one', function () { + $verifier = app(ConnectionVerifier::class); + $checked = 0; + + foreach (Platform::cases() as $platform) { + if (! $platform->hasTokenRefreshFlow()) { + continue; + } + + $checked++; + + $account = SocialAccount::factory()->create([ + 'workspace_id' => Workspace::factory()->create()->id, + 'platform' => $platform, + 'status' => Status::Connected, + 'refresh_token' => 'rt-seed', + 'meta' => ['service' => 'https://bsky.social', 'identifier' => 'a.bsky.social'], + ]); + + Http::fake(['*' => Http::response([ + 'access_token' => 'at', 'refresh_token' => 'rt', 'expires_in' => 3600, + 'accessJwt' => 'j', 'refreshJwt' => 'r', 'id' => '1', 'data' => ['id' => '1'], + ], 200)]); + + try { + $verifier->refreshToken($account); + } catch (UnhandledMatchError $e) { + // refreshToken()'s match has no default arm, so a platform added to + // hasTokenRefreshFlow() without one blows up in production instead + // of falling through quietly. + $this->fail("{$platform->value} claims a refresh flow but refreshToken() has no arm for it"); + } catch (Throwable $e) { + $this->fail("{$platform->value} refresh threw ".$e::class.': '.$e->getMessage()); + } + + // A broken client chain (a renamed helper, a method that no longer + // exists on PendingRequest) raises before anything leaves the process, + // so "no request sent" is the signal that catches it. + expect(Http::recorded()) + ->not->toBeEmpty("{$platform->value} refresh sent no HTTP request at all"); + } + + expect($checked)->toBeGreaterThan(0); +}); + +test('no platform lets a tokenless 200 destroy the credential it already had', function () { + Queue::fake(); + + $verifier = app(ConnectionVerifier::class); + $checked = 0; + + foreach (Platform::cases() as $platform) { + if (! $platform->hasTokenRefreshFlow()) { + continue; + } + + $checked++; + + $account = SocialAccount::factory()->create([ + 'workspace_id' => Workspace::factory()->create()->id, + 'platform' => $platform, + 'status' => Status::Connected, + 'access_token' => 'the-token-that-still-works', + 'refresh_token' => 'rt-seed', + 'meta' => ['service' => 'https://bsky.social', 'identifier' => 'a.bsky.social'], + ]); + + // A 200 carrying no token at all. Every provider reads a different + // field name, so this is the shape none of them can parse. + Http::fake(['*' => Http::response(['expires_in' => 3600], 200)]); + + try { + $verifier->refreshToken($account); + $this->fail("{$platform->value} accepted a 200 with no token in it"); + } catch (PlatformUnavailableException) { + // Correct: nothing is provably dead, so refuse and let the next + // tick retry rather than disconnecting anyone. + } catch (Throwable $e) { + // Without the guard the write reaches the database and trips the + // NOT NULL column, which also poisons the surrounding transaction. + $this->fail("{$platform->value} should refuse a tokenless 200 cleanly, got ".$e::class.': '.$e->getMessage()); + } + + expect($account->fresh()->access_token) + ->toBe('the-token-that-still-works', "{$platform->value} overwrote a working token with nothing"); + } + + Queue::assertNotPushed(SendNotification::class); + expect($checked)->toBeGreaterThan(0); +}); + +test('a platform outage never disconnects, not even once the token has expired', function () { + Queue::fake(); + + $verifier = mock(ConnectionVerifier::class); + $verifier->shouldReceive('refreshToken')->once()->andThrow( + // TokenRefreshClient raises this for 5xx, 429 and connection timeouts. + new PlatformUnavailableException('X API returned 429 during token refresh', 429) + ); + app()->instance(ConnectionVerifier::class, $verifier); + + $this->account->update(['token_expires_at' => now()->subMinutes(5)]); + + (new RefreshSocialToken($this->account))->handle($verifier); + + // A rate limit around expiry is not evidence of anything. Disconnecting + // here emails the owner and hard-fails every scheduled post, and only the + // daily sweep would undo it. + expect($this->account->fresh()->status)->toBe(Status::Connected); + Queue::assertNotPushed(SendNotification::class); +}); + +test('a platform outage on a live token stays quiet and retries', function () { + Queue::fake(); + + $verifier = mock(ConnectionVerifier::class); + $verifier->shouldReceive('refreshToken')->once()->andThrow( new PlatformUnavailableException('X API returned 503 during token refresh', 503) ); app()->instance(ConnectionVerifier::class, $verifier); + $this->account->update(['token_expires_at' => now()->addMinutes(20)]); + (new RefreshSocialToken($this->account))->handle($verifier); expect($this->account->fresh()->status)->toBe(Status::Connected); Queue::assertNotPushed(SendNotification::class); }); + +test('a failure the fallback cannot attribute to the token surfaces instead of passing as healthy', function () { + Queue::fake(); + + $verifier = mock(ConnectionVerifier::class); + $verifier->shouldReceive('refreshToken')->once()->andThrow(new TokenExpiredException('rejected')); + // e.g. a decrypt failure after an APP_KEY rotation, or an unhandled match + // for a platform someone just added. + $verifier->shouldReceive('verifyAccessToken')->once()->andThrow(new RuntimeException('cannot decrypt')); + app()->instance(ConnectionVerifier::class, $verifier); + + // Swallowing this would leave the account Connected forever while every + // publish hard-fails. It has to reach failed_jobs where someone sees it — + // and it must not disconnect users, since an APP_KEY rotation breaks all + // of them at once. + expect(fn () => (new RefreshSocialToken($this->account))->handle($verifier)) + ->toThrow(RuntimeException::class); + + expect($this->account->fresh()->status)->toBe(Status::Connected); + Queue::assertNotPushed(SendNotification::class); +}); + +test('a null refresh_token in a 200 does not wipe the one we already had', function () { + Http::fake([ + config('trypost.platforms.x.api').'/oauth2/token' => Http::response([ + 'access_token' => 'fresh-access-token', + 'refresh_token' => null, + 'expires_in' => 7200, + ], 200), + ]); + + $this->account->update([ + 'refresh_token' => 'the-refresh-token-that-still-works', + 'token_expires_at' => now()->addMinutes(20), + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + // data_get() only falls back when the key is absent, so an explicit null + // overwrites. Losing it means the next tick throws "no refresh token" + // without a single call, and the account dies with the access token. + expect($this->account->fresh()->refresh_token)->toBe('the-refresh-token-that-still-works'); +}); + +test('a refresh already in flight on a dead token is transient, not something to publish through', function () { + $this->account->update(['token_expires_at' => now()->subMinutes(5)]); + + Cache::lock("token_refresh:{$this->account->id}", 120)->get(); + + // Returning false here hands the caller a token it already knows is dead. + // A publisher then posts with it, gets a 401, and PublishToSocialPlatform + // finalises the post as failed and disconnects the account — for a lock + // that a worker death left behind. + expect(fn () => app(ConnectionVerifier::class)->refreshToken($this->account)) + ->toThrow(PlatformUnavailableException::class); +}); + +test('a billed fallback check counts as a verification like any other', function () { + Http::fake([ + config('trypost.platforms.x.api').'/oauth2/token' => Http::response(['error' => 'invalid_grant'], 400), + config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200), + ]); + + $this->account->update([ + 'access_token' => 'still-valid-access-token', + 'token_expires_at' => now()->addMinutes(20), + 'last_verified_at' => null, + ]); + + (new RefreshSocialToken($this->account))->handle(app(ConnectionVerifier::class)); + + // GET /2/users/me is billed and it just proved the token alive. Throwing + // that away means the pre-publish check pays to ask again minutes later. + expect($this->account->fresh()->last_verified_at)->not->toBeNull(); +}); diff --git a/tests/Feature/VerifyWorkspaceConnectionsTest.php b/tests/Feature/VerifyWorkspaceConnectionsTest.php index 199767010..f59ac8e42 100644 --- a/tests/Feature/VerifyWorkspaceConnectionsTest.php +++ b/tests/Feature/VerifyWorkspaceConnectionsTest.php @@ -10,6 +10,7 @@ use App\Models\SocialAccount; use App\Models\Workspace; use App\Services\Social\ConnectionVerifier; +use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Mail; test('job does nothing when workspace has no connected accounts', function () { @@ -156,3 +157,102 @@ Mail::assertNothingSent(); }); + +test('daily sweep skips verifying a connected account a recent refresh already proved valid', function () { + Mail::fake(); + Http::fake([ + config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200), + ]); + + $workspace = Workspace::factory()->create(); + SocialAccount::factory()->x()->create([ + 'workspace_id' => $workspace->id, + 'status' => Status::Connected, + 'last_verified_at' => now()->subHours(2), + ]); + + VerifyWorkspaceConnections::dispatch($workspace); + + // A successful token refresh within the trust window already proved the + // credential — re-reading the profile would only burn a billed User Read. + Http::assertNotSent(fn ($request) => str_contains($request->url(), '/users/me')); + Mail::assertNothingSent(); +}); + +test('daily sweep still verifies a connected account whose last_verified_at is stale', function () { + Mail::fake(); + Http::fake([ + config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200), + ]); + + $workspace = Workspace::factory()->create(); + SocialAccount::factory()->x()->create([ + 'workspace_id' => $workspace->id, + 'status' => Status::Connected, + 'last_verified_at' => now()->subHours(20), + ]); + + VerifyWorkspaceConnections::dispatch($workspace); + + Http::assertSent(fn ($request) => str_contains($request->url(), '/users/me')); +}); + +test('daily sweep still verifies a TokenExpired account despite a fresh last_verified_at', function () { + Mail::fake(); + Http::fake([ + config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200), + ]); + + $workspace = Workspace::factory()->create(); + $account = SocialAccount::factory()->x()->create([ + 'workspace_id' => $workspace->id, + 'status' => Status::TokenExpired, + 'last_verified_at' => now()->subMinutes(5), + ]); + + VerifyWorkspaceConnections::dispatch($workspace); + + // Skipping here would strand a recovered account in TokenExpired forever. + Http::assertSent(fn ($request) => str_contains($request->url(), '/users/me')); + expect($account->fresh()->status)->toBe(Status::Connected); +}); + +test('daily sweep records its own successful verification', function () { + Mail::fake(); + Http::fake([ + config('trypost.platforms.x.api').'/users/me' => Http::response(['data' => ['id' => '123']], 200), + ]); + + $workspace = Workspace::factory()->create(); + $account = SocialAccount::factory()->x()->create([ + 'workspace_id' => $workspace->id, + 'status' => Status::Connected, + 'last_verified_at' => null, + ]); + + VerifyWorkspaceConnections::dispatch($workspace); + + // Otherwise VerifyUpcomingPostConnections burns a fresh call minutes later + // on an account this sweep just confirmed healthy. + expect($account->fresh()->last_verified_at)->not->toBeNull(); +}); + +test('an unreachable platform does not revive an account nobody verified', function () { + Mail::fake(); + + $workspace = Workspace::factory()->create(); + $account = SocialAccount::factory()->x()->create([ + 'workspace_id' => $workspace->id, + 'status' => Status::TokenExpired, + ]); + + $verifier = mock(ConnectionVerifier::class); + $verifier->shouldReceive('verify')->andThrow(new PlatformUnavailableException('X API returned 503', 503)); + app()->instance(ConnectionVerifier::class, $verifier); + + VerifyWorkspaceConnections::dispatch($workspace); + + // "Don't disconnect" is not the same as "verified". Promoting on an + // outage tells the owner their reconnect worked when nothing was checked. + expect($account->fresh()->status)->toBe(Status::TokenExpired); +}); diff --git a/tests/Feature/XAnalyticsTest.php b/tests/Feature/XAnalyticsTest.php new file mode 100644 index 000000000..5c6e31511 --- /dev/null +++ b/tests/Feature/XAnalyticsTest.php @@ -0,0 +1,109 @@ +account = SocialAccount::factory()->x()->create([ + 'workspace_id' => Workspace::factory()->create()->id, + 'platform_user_id' => '4242', + 'token_expires_at' => now()->addHours(2), + ]); + $this->api = config('trypost.platforms.x.api'); +}); + +test('metrics come from the timeline itself instead of a second lookup of the same posts', function () { + Http::fake([ + $this->api.'/users/4242/tweets*' => Http::response([ + 'data' => [ + ['id' => '1', 'public_metrics' => ['impression_count' => 100, 'like_count' => 10, 'retweet_count' => 1, 'reply_count' => 2, 'quote_count' => 3, 'bookmark_count' => 4]], + ['id' => '2', 'public_metrics' => ['impression_count' => 200, 'like_count' => 20, 'retweet_count' => 2, 'reply_count' => 4, 'quote_count' => 6, 'bookmark_count' => 8]], + ], + 'meta' => [], + ], 200), + ]); + + $metrics = app(XAnalytics::class)->getMetrics($this->account); + + // Every post returned is a billed Post read. Re-reading the same ids from + // /2/tweets buys nothing the timeline could not have returned. + Http::assertNotSent(fn ($request) => str_contains($request->url(), '/2/tweets?') + || preg_match('#/tweets\?ids=#', $request->url()) === 1); + + expect(collect($metrics)->firstWhere('label', __('analytics.metrics.impressions'))['value'])->toBe(300); + expect(collect($metrics)->firstWhere('label', __('analytics.metrics.likes'))['value'])->toBe(30); +}); + +test('the timeline request asks for public_metrics', function () { + Http::fake([ + $this->api.'/users/4242/tweets*' => Http::response(['data' => [], 'meta' => []], 200), + ]); + + app(XAnalytics::class)->getMetrics($this->account); + + Http::assertSent(fn ($request) => str_contains($request->url(), 'tweet.fields=public_metrics')); +}); + +test('metrics accumulate across paginated timeline pages', function () { + Http::fake([ + $this->api.'/users/4242/tweets*' => Http::sequence() + ->push([ + 'data' => [['id' => '1', 'public_metrics' => ['impression_count' => 100, 'like_count' => 1, 'retweet_count' => 0, 'reply_count' => 0, 'quote_count' => 0, 'bookmark_count' => 0]]], + 'meta' => ['next_token' => 'page2'], + ], 200) + ->push([ + 'data' => [['id' => '2', 'public_metrics' => ['impression_count' => 50, 'like_count' => 2, 'retweet_count' => 0, 'reply_count' => 0, 'quote_count' => 0, 'bookmark_count' => 0]]], + 'meta' => [], + ], 200), + ]); + + $metrics = app(XAnalytics::class)->getMetrics($this->account); + + expect(collect($metrics)->firstWhere('label', __('analytics.metrics.impressions'))['value'])->toBe(150); + expect(collect($metrics)->firstWhere('label', __('analytics.metrics.likes'))['value'])->toBe(3); +}); + +test('a page that fails mid-pagination keeps the totals collected so far', function () { + Http::fake([ + $this->api.'/users/4242/tweets*' => Http::sequence() + ->push([ + 'data' => [['id' => '1', 'public_metrics' => ['impression_count' => 100, 'like_count' => 5, 'retweet_count' => 0, 'reply_count' => 0, 'quote_count' => 0, 'bookmark_count' => 0]]], + 'meta' => ['next_token' => 'page2'], + ], 200) + ->push(['title' => 'Internal Error'], 500), + ]); + + $metrics = app(XAnalytics::class)->getMetrics($this->account); + + // Partial data beats an exception on a dashboard the user is looking at. + expect(collect($metrics)->firstWhere('label', __('analytics.metrics.impressions'))['value'])->toBe(100); + expect(collect($metrics)->firstWhere('label', __('analytics.metrics.likes'))['value'])->toBe(5); +}); + +test('a post returned without public_metrics counts as zero rather than erroring', function () { + Http::fake([ + $this->api.'/users/4242/tweets*' => Http::response([ + 'data' => [ + ['id' => '1'], + ['id' => '2', 'public_metrics' => ['impression_count' => 7, 'like_count' => 1, 'retweet_count' => 0, 'reply_count' => 0, 'quote_count' => 0, 'bookmark_count' => 0]], + ], + 'meta' => [], + ], 200), + ]); + + $metrics = app(XAnalytics::class)->getMetrics($this->account); + + expect(collect($metrics)->firstWhere('label', __('analytics.metrics.impressions'))['value'])->toBe(7); +}); + +test('an account that posted nothing in the range returns no metrics at all', function () { + Http::fake([ + $this->api.'/users/4242/tweets*' => Http::response(['data' => [], 'meta' => []], 200), + ]); + + expect(app(XAnalytics::class)->getMetrics($this->account))->toBe([]); +});