vignesh/trypost
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Recover from a lost-rotation race on the post-refresh verify too

Browse source 
refreshThenVerify only guarded refreshToken(); the verify that followed was
outside the try, so the sub-commit window where a lock-skipped refresh reloads a
not-yet-persisted token surfaced as a 401 there and falsely disconnected a
still-usable single-use-refresh_token account (X/LinkedIn). The refresh and the
verify that follows it now share one recovery: on a TokenExpiredException,
reload and, if a concurrent refresh has since persisted a fresh access_token,
verify with it instead of giving up.
This commit is contained in:
Paulo Castellano 2026-07-03 11:39:48 -03:00
parent 3d3150bd20
commit ba8068f517
2 changed files with 36 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
app/Services/Social/ConnectionVerifier.php
View file

@ -47,11 +47,13 @@ public function verify(SocialAccount $account): bool
/**
* Refresh the token, then verify with the new one.
*
* If the refresh is rejected (4xx) but a concurrent refresh has already
* rotated the account and persisted a fresh access_token, reload and
* verify with that token instead of giving up — X (and other providers
* that single-use their refresh_token) otherwise disconnect a still-usable
* account whenever two refreshes race and one loses the rotation.
* If either the refresh is rejected (4xx) or the verify that follows it hits
* a token a concurrent refresh has already rotated — including the sub-commit
* window where a lock-skipped refresh reloads a not-yet-persisted token —
* reload and, when another process has since persisted a fresh access_token,
* verify with that instead of giving up. X (and other providers that
* single-use their refresh_token) would otherwise disconnect a still-usable
* account whenever two refreshes race and one loses.
*
* @throws TokenExpiredException
* @throws PlatformUnavailableException
@ -62,6 +64,8 @@ private function refreshThenVerify(SocialAccount $account, ?TokenExpiredExceptio
try {
$this->refreshToken($account);
return $this->callVerifyEndpoint($account);
} catch (TokenExpiredException $e) {
$account->refresh();
@ -71,8 +75,6 @@ private function refreshThenVerify(SocialAccount $account, ?TokenExpiredExceptio
throw $original ?? $e;
}
return $this->callVerifyEndpoint($account);
}
/**

27
tests/Feature/Services/Social/ConnectionVerifierTest.php
View file

@ -481,6 +481,33 @@
&& $request->header('Authorization')[0] === 'Bearer fresh-token');
});
test('does not disconnect when the verify after a refresh 401s once but a fresh token is available', function () {
Http::fake([
// Refresh succeeds and rotates the token...
config('trypost.platforms.x.api').'/oauth2/token' => Http::response([
'access_token' => 'refreshed-token',
'refresh_token' => 'new-refresh',
'expires_in' => 7200,
], 200),
// ...but the verify that follows 401s once (the sub-commit window where a
// lock-skipped refresh reloads a not-yet-persisted token) before
// succeeding on the reload + retry.
config('trypost.platforms.x.api').'/users/me' => Http::sequence()
->push(['error' => 'unauthorized'], 401)
->push(['data' => ['id' => '123']], 200),
]);
$account = SocialAccount::factory()->x()->create([
'status' => Status::Connected,
'access_token' => 'stale-token',
'refresh_token' => 'old-refresh',
'token_expires_at' => now()->subHour(),
]);
expect((new ConnectionVerifier)->verify($account))->toBeTrue();
expect($account->fresh()->status)->toBe(Status::Connected);
});
test('4xx during refresh keeps raising TokenExpiredException', function () {
Http::fake([
config('trypost.platforms.x.api').'/oauth2/token' => Http::response(['error' => 'invalid_grant'], 400),