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); +});