fix(astrolabe): delete stale tokens when refresh fails
- Delete stored token when refresh callback fails or returns null - Delete stored token when expired with no refresh callback available - Fix test namespaces (Service → OCA\Astrolabe\Tests\Unit\Service) - Update tests to verify token deletion on refresh failure Prevents repeated refresh attempts with stale tokens that will never succeed, improving error handling and reducing unnecessary API calls. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
+10
-2
@@ -191,11 +191,19 @@ class McpTokenStorage {
|
||||
$this->logger->error("Failed to refresh token for user $userId", [
|
||||
'error' => $e->getMessage()
|
||||
]);
|
||||
// Fall through to return null
|
||||
// Delete stale token to prevent repeated refresh attempts
|
||||
$this->deleteUserToken($userId);
|
||||
return null;
|
||||
}
|
||||
|
||||
// Refresh callback returned null or invalid data - delete stale token
|
||||
$this->deleteUserToken($userId);
|
||||
$this->logger->info("Deleted stale token for user $userId after refresh failure");
|
||||
return null;
|
||||
}
|
||||
|
||||
// Token expired and no refresh available
|
||||
// Token expired and no refresh callback available - delete stale token
|
||||
$this->deleteUserToken($userId);
|
||||
$this->logger->info("Token expired for user $userId, no refresh available");
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Service;
|
||||
namespace OCA\Astrolabe\Tests\Unit\Service;
|
||||
|
||||
use OCA\Astrolabe\Service\IdpTokenRefresher;
|
||||
use OCA\Astrolabe\Service\McpServerClient;
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace Service;
|
||||
namespace OCA\Astrolabe\Tests\Unit\Service;
|
||||
|
||||
use OCA\Astrolabe\Service\McpTokenStorage;
|
||||
use OCP\IConfig;
|
||||
@@ -232,7 +232,7 @@ final class McpTokenStorageTest extends TestCase {
|
||||
$this->assertEquals('new-access-token', $result);
|
||||
}
|
||||
|
||||
public function testGetAccessTokenReturnsNullWhenRefreshFails(): void {
|
||||
public function testGetAccessTokenReturnsNullWhenRefreshFailsAndDeletesToken(): void {
|
||||
$userId = 'testuser';
|
||||
$expiredTokenData = [
|
||||
'access_token' => 'expired-access-token',
|
||||
@@ -246,6 +246,11 @@ final class McpTokenStorageTest extends TestCase {
|
||||
$this->crypto->method('decrypt')
|
||||
->willReturn(json_encode($expiredTokenData));
|
||||
|
||||
// Expect stale token to be deleted when refresh fails
|
||||
$this->config->expects($this->once())
|
||||
->method('deleteUserValue')
|
||||
->with($userId, 'astrolabe', 'oauth_tokens');
|
||||
|
||||
// Refresh callback returns null (failure)
|
||||
$refreshCallback = fn (string $refreshToken) => null;
|
||||
|
||||
@@ -254,7 +259,7 @@ final class McpTokenStorageTest extends TestCase {
|
||||
$this->assertNull($result);
|
||||
}
|
||||
|
||||
public function testGetAccessTokenReturnsNullWhenExpiredAndNoCallback(): void {
|
||||
public function testGetAccessTokenReturnsNullWhenExpiredAndNoCallbackAndDeletesToken(): void {
|
||||
$userId = 'testuser';
|
||||
$expiredTokenData = [
|
||||
'access_token' => 'expired-access-token',
|
||||
@@ -268,6 +273,11 @@ final class McpTokenStorageTest extends TestCase {
|
||||
$this->crypto->method('decrypt')
|
||||
->willReturn(json_encode($expiredTokenData));
|
||||
|
||||
// Expect stale token to be deleted when expired with no callback
|
||||
$this->config->expects($this->once())
|
||||
->method('deleteUserValue')
|
||||
->with($userId, 'astrolabe', 'oauth_tokens');
|
||||
|
||||
// No refresh callback provided
|
||||
$result = $this->storage->getAccessToken($userId, null);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user