From 3ba47ad02a4c1e97d6a5d036c78e913cd456632c Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Tue, 12 May 2026 19:36:35 -0300 Subject: [PATCH] fix(social): proactive token refresh actually refreshes (not just verifies) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three orthogonal fixes that together close the gap where social tokens were silently aging out without ever being refreshed, then dying at the provider when the refresh_token also got revoked. The original failure mode: a user's X token expired because the hourly proactive-refresh cron's smart `verify()` skip-logic kept saying 'token still works, no need to refresh', and once the token actually expired, the cron's WHERE clause excluded it from future runs. By the time anyone noticed, the refresh_token at X was also gone. (C) ConnectionVerifier: rename private `refreshTokenIfNeeded` → public `refreshToken`. Callers that want the smart 'try access_token first' behavior keep using `verify()`. Callers that want a proactive refresh (the cron) call `refreshToken` directly. (B) RefreshExpiringTokens command: drop the `where('token_expires_at', '>', now())` filter. Already-expired tokens now get a last-chance refresh attempt before the refresh_token also dies at the provider. Status filter (`Connected`) still excludes accounts already marked TokenExpired. (D) RefreshSocialToken job: switch from `verify()` to `refreshToken()`, and on `TokenExpiredException` call `markAsTokenExpired` so the user is notified immediately. The lock + transition detection in markAsTokenExpired prevents notification spam if subsequent cron passes also fail. Tests: - 3 new tests for RefreshSocialToken (calls refreshToken not verify, marks TokenExpired on TokenExpiredException, logs warning on other errors) - Updated RefreshExpiringTokens test to assert already-expired tokens are now dispatched (was previously asserted as 'should NOT') --- .../Commands/RefreshExpiringTokens.php | 3 +- app/Jobs/RefreshSocialToken.php | 12 +++- app/Services/Social/ConnectionVerifier.php | 11 +-- .../Commands/RefreshExpiringTokensTest.php | 20 ++++-- tests/Feature/Jobs/RefreshSocialTokenTest.php | 69 +++++++++++++++++++ 5 files changed, 102 insertions(+), 13 deletions(-) create mode 100644 tests/Feature/Jobs/RefreshSocialTokenTest.php diff --git a/app/Console/Commands/RefreshExpiringTokens.php b/app/Console/Commands/RefreshExpiringTokens.php index 170d08f2..200c3063 100644 --- a/app/Console/Commands/RefreshExpiringTokens.php +++ b/app/Console/Commands/RefreshExpiringTokens.php @@ -13,7 +13,7 @@ class RefreshExpiringTokens extends Command { protected $signature = 'social:refresh-expiring-tokens'; - protected $description = 'Proactively refresh tokens expiring in the next 2 hours'; + protected $description = 'Proactively refresh tokens expiring in the next 2 hours (or already expired)'; public function handle(): void { @@ -23,7 +23,6 @@ public function handle(): void ->where('status', Status::Connected) ->whereNotNull('token_expires_at') ->where('token_expires_at', '<=', now()->addHours(2)) - ->where('token_expires_at', '>', now()) ->chunk(50, function ($accounts) use (&$count) { foreach ($accounts as $account) { RefreshSocialToken::dispatch($account); diff --git a/app/Jobs/RefreshSocialToken.php b/app/Jobs/RefreshSocialToken.php index 2162c604..c23a1db0 100644 --- a/app/Jobs/RefreshSocialToken.php +++ b/app/Jobs/RefreshSocialToken.php @@ -4,11 +4,13 @@ namespace App\Jobs; +use App\Exceptions\TokenExpiredException; use App\Models\SocialAccount; use App\Services\Social\ConnectionVerifier; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; use Illuminate\Support\Facades\Log; +use Throwable; class RefreshSocialToken implements ShouldQueue { @@ -21,8 +23,14 @@ public function __construct(public SocialAccount $account) {} public function handle(ConnectionVerifier $verifier): void { try { - $verifier->verify($this->account); - } catch (\Throwable $e) { + $verifier->refreshToken($this->account); + } catch (TokenExpiredException $e) { + // refresh_token rejected by the provider (revoked / rotated / + // expired beyond refresh). Mark the account so the user is + // notified immediately instead of waiting for the next failed + // publish or the daily verify pass. + $this->account->markAsTokenExpired($e->getMessage()); + } catch (Throwable $e) { Log::warning('Proactive token refresh failed', [ 'account_id' => $this->account->id, 'platform' => $this->account->platform->value, diff --git a/app/Services/Social/ConnectionVerifier.php b/app/Services/Social/ConnectionVerifier.php index 742933ca..d65a493d 100644 --- a/app/Services/Social/ConnectionVerifier.php +++ b/app/Services/Social/ConnectionVerifier.php @@ -28,7 +28,7 @@ public function verify(SocialAccount $account): bool // refresh, so proactive refreshes during races cause false-positive // disconnects even though the access_token still works fine. if ($account->is_token_expired) { - $this->refreshTokenIfNeeded($account); + $this->refreshToken($account); return $this->callVerifyEndpoint($account); } @@ -39,7 +39,7 @@ public function verify(SocialAccount $account): bool // Verify returned 401: the access_token is actually invalid. // Refresh and retry once with the new token. try { - $this->refreshTokenIfNeeded($account); + $this->refreshToken($account); } catch (TokenExpiredException) { throw $e; } @@ -69,11 +69,14 @@ private function callVerifyEndpoint(SocialAccount $account): bool } /** - * Refresh token based on platform type. + * Refresh the account's token via the platform-specific OAuth flow. + * Callers that want the smart "try access_token first" behavior should + * use verify() instead. This method always attempts a refresh under + * the per-account lock and throws TokenExpiredException on failure. * * @throws TokenExpiredException if refresh fails */ - private function refreshTokenIfNeeded(SocialAccount $account): void + public function refreshToken(SocialAccount $account): void { $lock = Cache::lock("token_refresh:{$account->id}", 30); diff --git a/tests/Feature/Commands/RefreshExpiringTokensTest.php b/tests/Feature/Commands/RefreshExpiringTokensTest.php index 417070e2..5656d28b 100644 --- a/tests/Feature/Commands/RefreshExpiringTokensTest.php +++ b/tests/Feature/Commands/RefreshExpiringTokensTest.php @@ -9,7 +9,7 @@ use App\Models\Workspace; use Illuminate\Support\Facades\Queue; -test('it dispatches refresh jobs for tokens expiring within 2 hours', function () { +test('it dispatches refresh jobs for tokens expiring within 2 hours or already expired', function () { Queue::fake(); $workspace = Workspace::factory()->create(); @@ -22,7 +22,7 @@ 'token_expires_at' => now()->addHour(), ]); - // Should NOT be refreshed (expires in 5 hours) + // Should NOT be refreshed (expires in 5 hours — outside the proactive window) SocialAccount::factory()->create([ 'workspace_id' => $workspace->id, 'platform' => Platform::Instagram, @@ -30,8 +30,9 @@ 'token_expires_at' => now()->addHours(5), ]); - // Should NOT be refreshed (already expired) - SocialAccount::factory()->create([ + // SHOULD be refreshed (already expired — last-chance attempt before the + // refresh_token also dies at the provider). + $justExpired = SocialAccount::factory()->create([ 'workspace_id' => $workspace->id, 'platform' => Platform::TikTok, 'status' => Status::Connected, @@ -46,11 +47,20 @@ 'token_expires_at' => now()->addHour(), ]); + // Should NOT be refreshed (already token expired — daily verify handles these) + SocialAccount::factory()->create([ + 'workspace_id' => $workspace->id, + 'platform' => Platform::Pinterest, + 'status' => Status::TokenExpired, + 'token_expires_at' => now()->subHour(), + ]); + $this->artisan('social:refresh-expiring-tokens') ->assertSuccessful(); - Queue::assertPushed(RefreshSocialToken::class, 1); + Queue::assertPushed(RefreshSocialToken::class, 2); Queue::assertPushed(RefreshSocialToken::class, fn ($job) => $job->account->id === $expiringSoon->id); + Queue::assertPushed(RefreshSocialToken::class, fn ($job) => $job->account->id === $justExpired->id); }); test('it dispatches nothing when no tokens are expiring', function () { diff --git a/tests/Feature/Jobs/RefreshSocialTokenTest.php b/tests/Feature/Jobs/RefreshSocialTokenTest.php new file mode 100644 index 00000000..41b956c1 --- /dev/null +++ b/tests/Feature/Jobs/RefreshSocialTokenTest.php @@ -0,0 +1,69 @@ +owner = User::factory()->create(); + $this->workspace = Workspace::factory()->create(['user_id' => $this->owner->id]); + $this->account = SocialAccount::factory()->x()->create([ + 'workspace_id' => $this->workspace->id, + 'status' => Status::Connected, + 'username' => 'testuser', + ]); +}); + +test('refresh job calls refreshToken (not verify) on the verifier', function () { + $verifier = mock(ConnectionVerifier::class); + $verifier->shouldReceive('refreshToken')->once()->with( + Mockery::on(fn ($account) => $account->id === $this->account->id) + ); + $verifier->shouldNotReceive('verify'); + app()->instance(ConnectionVerifier::class, $verifier); + + (new RefreshSocialToken($this->account))->handle($verifier); +}); + +test('refresh job marks account as TokenExpired when refresh_token is rejected', function () { + Queue::fake(); + + $verifier = mock(ConnectionVerifier::class); + $verifier->shouldReceive('refreshToken')->once()->andThrow( + new TokenExpiredException('refresh_token revoked') + ); + app()->instance(ConnectionVerifier::class, $verifier); + + (new RefreshSocialToken($this->account))->handle($verifier); + + expect($this->account->fresh()->status)->toBe(Status::TokenExpired); + expect($this->account->fresh()->error_message)->toBe('refresh_token revoked'); + + // Notification dispatched because account transitioned from Connected. + Queue::assertPushed(SendNotification::class); +}); + +test('refresh job logs warning on non-token errors and leaves status alone', function () { + Log::shouldReceive('warning')->once()->withArgs(function ($message, $context) { + return $message === 'Proactive token refresh failed' + && $context['account_id'] === $this->account->id + && $context['error'] === 'network blip'; + }); + + $verifier = mock(ConnectionVerifier::class); + $verifier->shouldReceive('refreshToken')->once()->andThrow(new RuntimeException('network blip')); + app()->instance(ConnectionVerifier::class, $verifier); + + (new RefreshSocialToken($this->account))->handle($verifier); + + expect($this->account->fresh()->status)->toBe(Status::Connected); +});