Merge pull request #30 from trypostit/fix/proactive-token-refresh
fix(social): proactive token refresh actually refreshes (not just verifies)
This commit is contained in:
commit
6c2c555bd0
5 changed files with 102 additions and 13 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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 () {
|
||||
|
|
|
|||
69
tests/Feature/Jobs/RefreshSocialTokenTest.php
Normal file
69
tests/Feature/Jobs/RefreshSocialTokenTest.php
Normal file
|
|
@ -0,0 +1,69 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use App\Enums\SocialAccount\Status;
|
||||
use App\Exceptions\TokenExpiredException;
|
||||
use App\Jobs\RefreshSocialToken;
|
||||
use App\Jobs\SendNotification;
|
||||
use App\Models\SocialAccount;
|
||||
use App\Models\User;
|
||||
use App\Models\Workspace;
|
||||
use App\Services\Social\ConnectionVerifier;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
use Illuminate\Support\Facades\Queue;
|
||||
|
||||
beforeEach(function () {
|
||||
$this->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);
|
||||
});
|
||||
Loading…
Reference in a new issue