From 5b71ac3251402adea741ca15386ae655dcb7fd0b Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 27 Jan 2026 22:23:42 +0100 Subject: [PATCH] fix(astrolabe): add locking to prevent token refresh race condition Adds distributed locking using Nextcloud's ILockingProvider to prevent race conditions between background job and on-demand token refresh. Uses double-check locking pattern: 1. Quick check without lock - return immediately if token is valid 2. Acquire exclusive lock if token needs refresh 3. Re-check after lock - another process may have refreshed 4. Refresh only if still needed 5. Graceful degradation on LockedException Changes: - McpTokenStorage: add ILockingProvider, withTokenLock() method - McpTokenStorage: update getAccessToken() with locking pattern - RefreshUserTokens: wrap refresh in withTokenLock(), catch LockedException - Add comprehensive unit tests for locking behavior Co-Authored-By: Claude Opus 4.5 --- .../lib/BackgroundJob/RefreshUserTokens.php | 105 ++++++++---- .../astrolabe/lib/Service/McpTokenStorage.php | 150 ++++++++++++----- .../BackgroundJob/RefreshUserTokensTest.php | 145 ++++++++++++++++ .../unit/Service/McpTokenStorageTest.php | 156 +++++++++++++++++- 4 files changed, 482 insertions(+), 74 deletions(-) diff --git a/third_party/astrolabe/lib/BackgroundJob/RefreshUserTokens.php b/third_party/astrolabe/lib/BackgroundJob/RefreshUserTokens.php index c79add2..a74ec65 100644 --- a/third_party/astrolabe/lib/BackgroundJob/RefreshUserTokens.php +++ b/third_party/astrolabe/lib/BackgroundJob/RefreshUserTokens.php @@ -9,6 +9,7 @@ use OCA\Astrolabe\Service\McpTokenStorage; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJob; use OCP\BackgroundJob\TimedJob; +use OCP\Lock\LockedException; use Psr\Log\LoggerInterface; /** @@ -76,6 +77,10 @@ class RefreshUserTokens extends TimedJob { * Calculates the refresh threshold based on the token's actual lifetime, * refreshing when less than 50% of the lifetime remains. * + * Uses locking to prevent race conditions with on-demand refresh in + * getAccessToken(). If lock cannot be acquired, skips this user since + * on-demand refresh is already handling it. + * * @return string 'refreshed', 'failed', or 'skipped' */ private function refreshUserTokenIfNeeded(string $userId): string { @@ -108,45 +113,81 @@ class RefreshUserTokens extends TimedJob { return 'skipped'; } - // Token is expiring soon, attempt refresh - if (!isset($token['refresh_token'])) { - $this->logger->warning("RefreshUserTokens: User $userId has no refresh token"); - return 'failed'; - } - - $this->logger->debug("RefreshUserTokens: Refreshing token for user $userId (remaining={$timeRemaining}s, threshold={$threshold}s)"); - + // Token is expiring soon, attempt refresh with lock try { - /** @var string $refreshToken */ - $refreshToken = $token['refresh_token']; - $newTokenData = $this->tokenRefresher->refreshAccessToken($refreshToken); + return $this->tokenStorage->withTokenLock($userId, function () use ($userId) { + // Re-check token after acquiring lock (double-check pattern) + // Another process may have refreshed while we waited for lock + $currentToken = $this->tokenStorage->getUserToken($userId); - if ($newTokenData === null) { - $this->logger->warning("RefreshUserTokens: Refresh returned null for user $userId"); - // Don't delete token here - let on-demand refresh handle cleanup - return 'failed'; - } + if ($currentToken === null) { + return 'skipped'; + } - // Calculate new expiration and store issued_at for future calculations - $expiresIn = (int)($newTokenData['expires_in'] ?? self::DEFAULT_TOKEN_LIFETIME_SECONDS); - $now = time(); + // Recalculate threshold with current token data + $currentExpiresAt = (int)($currentToken['expires_at'] ?? 0); + $currentIssuedAt = isset($currentToken['issued_at']) ? (int)$currentToken['issued_at'] : null; + $currentTimeRemaining = $currentExpiresAt - time(); - /** @var string $accessToken */ - $accessToken = $newTokenData['access_token']; - /** @var string $newRefreshToken */ - $newRefreshToken = $newTokenData['refresh_token'] ?? $refreshToken; + if ($currentIssuedAt !== null) { + $currentTokenLifetime = $currentExpiresAt - $currentIssuedAt; + } else { + $currentTokenLifetime = self::DEFAULT_TOKEN_LIFETIME_SECONDS; + } - $this->tokenStorage->storeUserToken( - $userId, - $accessToken, - $newRefreshToken, - $now + $expiresIn, - $now // issued_at - ); + $currentThreshold = max( + (int)($currentTokenLifetime * self::REFRESH_AT_REMAINING_PERCENT), + self::MIN_THRESHOLD_SECONDS + ); - $this->logger->debug("RefreshUserTokens: Successfully refreshed token for user $userId"); - return 'refreshed'; + if ($currentTimeRemaining > $currentThreshold) { + // Token was refreshed by another process while we waited + $this->logger->debug("RefreshUserTokens: Token already refreshed for user $userId while waiting for lock"); + return 'skipped'; + } + // Still needs refresh, proceed + if (!isset($currentToken['refresh_token'])) { + $this->logger->warning("RefreshUserTokens: User $userId has no refresh token"); + return 'failed'; + } + + $this->logger->debug("RefreshUserTokens: Refreshing token for user $userId (remaining={$currentTimeRemaining}s, threshold={$currentThreshold}s)"); + + /** @var string $refreshToken */ + $refreshToken = $currentToken['refresh_token']; + $newTokenData = $this->tokenRefresher->refreshAccessToken($refreshToken); + + if ($newTokenData === null) { + $this->logger->warning("RefreshUserTokens: Refresh returned null for user $userId"); + // Don't delete token here - let on-demand refresh handle cleanup + return 'failed'; + } + + // Calculate new expiration and store issued_at for future calculations + $expiresIn = (int)($newTokenData['expires_in'] ?? self::DEFAULT_TOKEN_LIFETIME_SECONDS); + $now = time(); + + /** @var string $accessToken */ + $accessToken = $newTokenData['access_token']; + /** @var string $newRefreshToken */ + $newRefreshToken = $newTokenData['refresh_token'] ?? $refreshToken; + + $this->tokenStorage->storeUserToken( + $userId, + $accessToken, + $newRefreshToken, + $now + $expiresIn, + $now // issued_at + ); + + $this->logger->debug("RefreshUserTokens: Successfully refreshed token for user $userId"); + return 'refreshed'; + }); + } catch (LockedException $e) { + // Lock held by on-demand refresh - expected, not an error + $this->logger->debug("RefreshUserTokens: Lock held for user $userId, skipping"); + return 'skipped'; } catch (\Exception $e) { $this->logger->error("RefreshUserTokens: Failed to refresh for user $userId: " . $e->getMessage()); return 'failed'; diff --git a/third_party/astrolabe/lib/Service/McpTokenStorage.php b/third_party/astrolabe/lib/Service/McpTokenStorage.php index 6e71eb3..6ceefc6 100644 --- a/third_party/astrolabe/lib/Service/McpTokenStorage.php +++ b/third_party/astrolabe/lib/Service/McpTokenStorage.php @@ -6,6 +6,8 @@ namespace OCA\Astrolabe\Service; use OCP\IConfig; use OCP\IDBConnection; +use OCP\Lock\ILockingProvider; +use OCP\Lock\LockedException; use OCP\Security\ICrypto; use Psr\Log\LoggerInterface; @@ -23,17 +25,20 @@ class McpTokenStorage { private $crypto; private $db; private $logger; + private ILockingProvider $lockingProvider; public function __construct( IConfig $config, ICrypto $crypto, IDBConnection $db, LoggerInterface $logger, + ILockingProvider $lockingProvider, ) { $this->config = $config; $this->crypto = $crypto; $this->db = $db; $this->logger = $logger; + $this->lockingProvider = $lockingProvider; } /** @@ -136,6 +141,38 @@ class McpTokenStorage { return time() >= ($token['expires_at'] - self::TOKEN_EXPIRY_BUFFER_SECONDS); } + /** + * Get the lock path for a user's token refresh operation. + * + * @param string $userId User ID + * @return string Lock path + */ + private function getTokenRefreshLockPath(string $userId): string { + return 'astrolabe/oauth/tokens/' . $userId; + } + + /** + * Execute callback while holding exclusive lock on user's token. + * + * Prevents race conditions between background job and on-demand token refresh. + * + * @template T + * @param string $userId User ID + * @param callable(): T $callback + * @return T + * @throws LockedException If lock cannot be acquired + */ + public function withTokenLock(string $userId, callable $callback): mixed { + $lockPath = $this->getTokenRefreshLockPath($userId); + + $this->lockingProvider->acquireLock($lockPath, ILockingProvider::LOCK_EXCLUSIVE); + try { + return $callback(); + } finally { + $this->lockingProvider->releaseLock($lockPath, ILockingProvider::LOCK_EXCLUSIVE); + } + } + /** * Delete stored tokens for a user. * @@ -195,67 +232,98 @@ class McpTokenStorage { * This is a convenience method that combines token retrieval, * expiration checking, and automatic refresh if needed. * + * Uses double-check locking pattern to prevent race conditions between + * background job and on-demand refresh while minimizing lock contention. + * * @param string $userId User ID * @param callable|null $refreshCallback Callback to refresh token if expired * Should accept (refreshToken) and return new token data * @return string|null Access token, or null if not available */ public function getAccessToken(string $userId, ?callable $refreshCallback = null): ?string { + // Quick check without lock (optimization) $token = $this->getUserToken($userId); if (!$token) { return null; } - // Check if token is expired - if ($this->isExpired($token)) { - // Try to refresh if callback provided - if ($refreshCallback && isset($token['refresh_token'])) { - try { - $newTokenData = $refreshCallback($token['refresh_token']); + // If not expired, return immediately without lock + if (!$this->isExpired($token)) { + return $token['access_token']; + } - if ($newTokenData && isset($newTokenData['access_token'])) { - // Store refreshed token - // Use new refresh token if provided (rotation), otherwise keep old one - $now = time(); - /** @var string $accessToken */ - $accessToken = $newTokenData['access_token']; - /** @var string $refreshToken */ - $refreshToken = $newTokenData['refresh_token'] ?? $token['refresh_token']; - $expiresIn = (int)($newTokenData['expires_in'] ?? 3600); + // Token expired - acquire lock for refresh + try { + /** @var string|null */ + return $this->withTokenLock($userId, function () use ($userId, $refreshCallback): ?string { + // Re-check after acquiring lock (double-check pattern) + // Another process may have refreshed while we waited for the lock + $currentToken = $this->getUserToken($userId); - $this->storeUserToken( - $userId, - $accessToken, - $refreshToken, - $now + $expiresIn, - $now // issued_at for accurate lifetime calculation - ); - - return $accessToken; - } - } catch (\Exception $e) { - $this->logger->error("Failed to refresh token for user $userId", [ - 'error' => $e->getMessage() - ]); - // Delete stale token to prevent repeated refresh attempts - $this->deleteUserToken($userId); + if (!$currentToken) { return null; } - // Refresh callback returned null or invalid data - delete stale token + // Check if another process already refreshed the token + if (!$this->isExpired($currentToken)) { + $this->logger->debug("Token already refreshed for user $userId while waiting for lock"); + return $currentToken['access_token']; + } + + // Still expired, perform refresh + if ($refreshCallback && isset($currentToken['refresh_token'])) { + try { + /** @var string $refreshToken */ + $refreshToken = $currentToken['refresh_token']; + $newTokenData = $refreshCallback($refreshToken); + + if ($newTokenData && isset($newTokenData['access_token'])) { + // Store refreshed token + // Use new refresh token if provided (rotation), otherwise keep old one + $now = time(); + /** @var string $accessToken */ + $accessToken = $newTokenData['access_token']; + /** @var string $newRefreshToken */ + $newRefreshToken = $newTokenData['refresh_token'] ?? $refreshToken; + $expiresIn = (int)($newTokenData['expires_in'] ?? 3600); + + $this->storeUserToken( + $userId, + $accessToken, + $newRefreshToken, + $now + $expiresIn, + $now // issued_at for accurate lifetime calculation + ); + + return $accessToken; + } + } catch (\Exception $e) { + $this->logger->error("Failed to refresh token for user $userId", [ + 'error' => $e->getMessage() + ]); + // 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 callback available - delete stale token $this->deleteUserToken($userId); - $this->logger->info("Deleted stale token for user $userId after refresh failure"); + $this->logger->info("Token expired for user $userId, no refresh available"); return null; - } - - // 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; + }); + } catch (LockedException $e) { + // Could not acquire lock - another process is refreshing + // Return stale token rather than failing - caller can retry if needed + $this->logger->warning("Could not acquire token lock for user $userId, returning stale token"); + return $token['access_token'] ?? null; } - - return $token['access_token']; } /** diff --git a/third_party/astrolabe/tests/unit/BackgroundJob/RefreshUserTokensTest.php b/third_party/astrolabe/tests/unit/BackgroundJob/RefreshUserTokensTest.php index 62bd32a..3ebe654 100644 --- a/third_party/astrolabe/tests/unit/BackgroundJob/RefreshUserTokensTest.php +++ b/third_party/astrolabe/tests/unit/BackgroundJob/RefreshUserTokensTest.php @@ -8,6 +8,7 @@ use OCA\Astrolabe\BackgroundJob\RefreshUserTokens; use OCA\Astrolabe\Service\IdpTokenRefresher; use OCA\Astrolabe\Service\McpTokenStorage; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\Lock\LockedException; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -40,6 +41,15 @@ final class RefreshUserTokensTest extends TestCase { ); } + /** + * Set up default withTokenLock behavior that executes the callback. + * Call this in tests that need the lock to succeed. + */ + private function setupDefaultLockBehavior(): void { + $this->tokenStorage->method('withTokenLock') + ->willReturnCallback(fn ($userId, $callback) => $callback()); + } + // ========================================================================= // Constructor Tests // ========================================================================= @@ -82,6 +92,8 @@ final class RefreshUserTokensTest extends TestCase { } public function testRunWithMultipleUsersAndMixedResults(): void { + $this->setupDefaultLockBehavior(); + $this->tokenStorage->method('getAllUsersWithTokens') ->willReturn(['alice', 'bob', 'charlie']); @@ -170,6 +182,8 @@ final class RefreshUserTokensTest extends TestCase { } public function testRefreshTriggeredWhenTokenNearExpiry(): void { + $this->setupDefaultLockBehavior(); + $now = time(); $this->tokenStorage->method('getUserToken') ->with('testuser') @@ -198,6 +212,8 @@ final class RefreshUserTokensTest extends TestCase { } public function testRefreshFailsWhenNoRefreshToken(): void { + $this->setupDefaultLockBehavior(); + $now = time(); $this->tokenStorage->method('getUserToken') ->with('testuser') @@ -221,6 +237,8 @@ final class RefreshUserTokensTest extends TestCase { } public function testRefreshFailsWhenRefresherReturnsNull(): void { + $this->setupDefaultLockBehavior(); + $now = time(); $this->tokenStorage->method('getUserToken') ->with('testuser') @@ -250,6 +268,8 @@ final class RefreshUserTokensTest extends TestCase { } public function testRefreshUsesIssuedAtForLifetimeCalculation(): void { + $this->setupDefaultLockBehavior(); + $now = time(); // Token with custom lifetime: issued 50 min ago, expires in 10 min (total 60 min) // 10/60 = 16.7% remaining, which is < 50%, so should refresh @@ -276,6 +296,8 @@ final class RefreshUserTokensTest extends TestCase { } public function testRefreshUsesDefaultLifetimeWhenNoIssuedAt(): void { + $this->setupDefaultLockBehavior(); + $now = time(); // Token without issued_at, uses default 3600s lifetime // 300s remaining / 3600s = 8.3% remaining, which is < 50%, so should refresh @@ -302,6 +324,8 @@ final class RefreshUserTokensTest extends TestCase { } public function testRefreshStoresNewTokenWithIssuedAt(): void { + $this->setupDefaultLockBehavior(); + $now = time(); $this->tokenStorage->method('getUserToken') ->with('testuser') @@ -337,6 +361,8 @@ final class RefreshUserTokensTest extends TestCase { } public function testRefreshKeepsOldRefreshTokenIfNotRotated(): void { + $this->setupDefaultLockBehavior(); + $now = time(); $this->tokenStorage->method('getUserToken') ->with('testuser') @@ -373,6 +399,8 @@ final class RefreshUserTokensTest extends TestCase { } public function testRefreshHandlesException(): void { + $this->setupDefaultLockBehavior(); + $now = time(); $this->tokenStorage->method('getUserToken') ->with('testuser') @@ -409,6 +437,123 @@ final class RefreshUserTokensTest extends TestCase { $this->assertEquals('skipped', $result); } + // ========================================================================= + // Locking Tests + // ========================================================================= + + public function testRefreshSkippedWhenLockCannotBeAcquired(): void { + $now = time(); + $this->tokenStorage->method('getUserToken') + ->with('testuser') + ->willReturn([ + 'access_token' => 'expiring-token', + 'refresh_token' => 'refresh-token', + 'expires_at' => $now + 100, // ~100s remaining (< 50% of default) + 'issued_at' => $now - 3500, + ]); + + // Lock acquisition fails (on-demand refresh is holding it) + $this->tokenStorage->expects($this->once()) + ->method('withTokenLock') + ->willThrowException(new LockedException('astrolabe/oauth/tokens/testuser')); + + // Token refresher should NOT be called when lock fails + $this->tokenRefresher->expects($this->never()) + ->method('refreshAccessToken'); + + $this->logger->expects($this->once()) + ->method('debug') + ->with($this->stringContains('Lock held for user testuser')); + + $result = $this->invokeRefreshUserTokenIfNeeded('testuser'); + + $this->assertEquals('skipped', $result); + } + + public function testRefreshUsesLockForTokenRefresh(): void { + $now = time(); + $this->tokenStorage->method('getUserToken') + ->with('testuser') + ->willReturn([ + 'access_token' => 'expiring-token', + 'refresh_token' => 'refresh-token', + 'expires_at' => $now + 100, + 'issued_at' => $now - 3500, + ]); + + // withTokenLock is called and executes the callback + $this->tokenStorage->expects($this->once()) + ->method('withTokenLock') + ->with('testuser', $this->isInstanceOf(\Closure::class)) + ->willReturnCallback(function ($userId, $callback) { + return $callback(); + }); + + $this->tokenRefresher->expects($this->once()) + ->method('refreshAccessToken') + ->with('refresh-token') + ->willReturn([ + 'access_token' => 'new-token', + 'refresh_token' => 'new-refresh-token', + 'expires_in' => 3600, + ]); + + $this->tokenStorage->expects($this->once()) + ->method('storeUserToken'); + + $result = $this->invokeRefreshUserTokenIfNeeded('testuser'); + + $this->assertEquals('refreshed', $result); + } + + public function testRefreshSkippedWhenTokenAlreadyRefreshedWhileWaitingForLock(): void { + $now = time(); + + // First call (before lock): token is expiring + // Calls inside lock callback: token is now fresh + $callCount = 0; + $this->tokenStorage->method('getUserToken') + ->with('testuser') + ->willReturnCallback(function () use (&$callCount, $now) { + $callCount++; + if ($callCount === 1) { + // First check: token is expiring + return [ + 'access_token' => 'expiring-token', + 'refresh_token' => 'refresh-token', + 'expires_at' => $now + 100, + 'issued_at' => $now - 3500, + ]; + } + // Inside lock: token was already refreshed + return [ + 'access_token' => 'already-refreshed-token', + 'refresh_token' => 'new-refresh-token', + 'expires_at' => $now + 3600, // Fresh token + 'issued_at' => $now, + ]; + }); + + // withTokenLock is called and executes the callback + $this->tokenStorage->expects($this->once()) + ->method('withTokenLock') + ->willReturnCallback(function ($userId, $callback) { + return $callback(); + }); + + // Token refresher should NOT be called since token is already fresh + $this->tokenRefresher->expects($this->never()) + ->method('refreshAccessToken'); + + $this->logger->expects($this->once()) + ->method('debug') + ->with($this->stringContains('already refreshed')); + + $result = $this->invokeRefreshUserTokenIfNeeded('testuser'); + + $this->assertEquals('skipped', $result); + } + // ========================================================================= // Helper Methods // ========================================================================= diff --git a/third_party/astrolabe/tests/unit/Service/McpTokenStorageTest.php b/third_party/astrolabe/tests/unit/Service/McpTokenStorageTest.php index 9688516..bb24924 100644 --- a/third_party/astrolabe/tests/unit/Service/McpTokenStorageTest.php +++ b/third_party/astrolabe/tests/unit/Service/McpTokenStorageTest.php @@ -10,6 +10,8 @@ use OCP\DB\QueryBuilder\IExpressionBuilder; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IConfig; use OCP\IDBConnection; +use OCP\Lock\ILockingProvider; +use OCP\Lock\LockedException; use OCP\Security\ICrypto; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -25,6 +27,7 @@ final class McpTokenStorageTest extends TestCase { private ICrypto&MockObject $crypto; private IDBConnection&MockObject $db; private LoggerInterface&MockObject $logger; + private ILockingProvider&MockObject $lockingProvider; private McpTokenStorage $storage; protected function setUp(): void { @@ -34,12 +37,14 @@ final class McpTokenStorageTest extends TestCase { $this->crypto = $this->createMock(ICrypto::class); $this->db = $this->createMock(IDBConnection::class); $this->logger = $this->createMock(LoggerInterface::class); + $this->lockingProvider = $this->createMock(ILockingProvider::class); $this->storage = new McpTokenStorage( $this->config, $this->crypto, $this->db, - $this->logger + $this->logger, + $this->lockingProvider ); } @@ -291,6 +296,155 @@ final class McpTokenStorageTest extends TestCase { $this->assertNull($result); } + // ========================================================================= + // Token Refresh Locking Tests + // ========================================================================= + + public function testGetAccessTokenAcquiresLockWhenRefreshing(): void { + $userId = 'testuser'; + $expiredTokenData = [ + 'access_token' => 'expired-access-token', + 'refresh_token' => 'old-refresh-token', + 'expires_at' => time() - 100, // Expired + ]; + + $newTokenData = [ + 'access_token' => 'new-access-token', + 'refresh_token' => 'new-refresh-token', + 'expires_in' => 3600, + ]; + + $this->config->method('getUserValue') + ->willReturn('encrypted-data'); + + $this->crypto->method('decrypt') + ->willReturn(json_encode($expiredTokenData)); + + $this->crypto->method('encrypt') + ->willReturn('new-encrypted-data'); + + // Verify lock is acquired and released + $this->lockingProvider->expects($this->once()) + ->method('acquireLock') + ->with('astrolabe/oauth/tokens/testuser', ILockingProvider::LOCK_EXCLUSIVE); + + $this->lockingProvider->expects($this->once()) + ->method('releaseLock') + ->with('astrolabe/oauth/tokens/testuser', ILockingProvider::LOCK_EXCLUSIVE); + + $refreshCallback = fn (string $refreshToken) => $newTokenData; + + $result = $this->storage->getAccessToken($userId, $refreshCallback); + + $this->assertEquals('new-access-token', $result); + } + + public function testGetAccessTokenReturnsStaleTokenOnLockedException(): void { + $userId = 'testuser'; + $expiredTokenData = [ + 'access_token' => 'expired-access-token', + 'refresh_token' => 'old-refresh-token', + 'expires_at' => time() - 100, // Expired + ]; + + $this->config->method('getUserValue') + ->willReturn('encrypted-data'); + + $this->crypto->method('decrypt') + ->willReturn(json_encode($expiredTokenData)); + + // Lock acquisition fails + $this->lockingProvider->expects($this->once()) + ->method('acquireLock') + ->willThrowException(new LockedException('astrolabe/oauth/tokens/testuser')); + + // Refresh callback should NOT be called when lock fails + $refreshCallbackCalled = false; + $refreshCallback = function (string $refreshToken) use (&$refreshCallbackCalled) { + $refreshCallbackCalled = true; + return ['access_token' => 'new-token', 'expires_in' => 3600]; + }; + + $result = $this->storage->getAccessToken($userId, $refreshCallback); + + // Should return stale token instead of failing + $this->assertEquals('expired-access-token', $result); + $this->assertFalse($refreshCallbackCalled); + } + + public function testGetAccessTokenSkipsRefreshWhenTokenAlreadyRefreshedWhileWaitingForLock(): void { + $userId = 'testuser'; + $expiredTokenData = [ + 'access_token' => 'expired-access-token', + 'refresh_token' => 'old-refresh-token', + 'expires_at' => time() - 100, // Expired + ]; + + // After lock is acquired, token appears fresh (another process refreshed it) + $freshTokenData = [ + 'access_token' => 'fresh-access-token', + 'refresh_token' => 'fresh-refresh-token', + 'expires_at' => time() + 3600, // Valid for 1 hour + ]; + + $callCount = 0; + $this->config->method('getUserValue') + ->willReturn('encrypted-data'); + + // First call returns expired, subsequent calls return fresh + $this->crypto->method('decrypt') + ->willReturnCallback(function () use (&$callCount, $expiredTokenData, $freshTokenData) { + $callCount++; + return $callCount === 1 + ? json_encode($expiredTokenData) + : json_encode($freshTokenData); + }); + + $this->lockingProvider->expects($this->once()) + ->method('acquireLock'); + + $this->lockingProvider->expects($this->once()) + ->method('releaseLock'); + + // Refresh callback should NOT be called since token is already fresh + $refreshCallbackCalled = false; + $refreshCallback = function (string $refreshToken) use (&$refreshCallbackCalled) { + $refreshCallbackCalled = true; + return ['access_token' => 'new-token', 'expires_in' => 3600]; + }; + + $result = $this->storage->getAccessToken($userId, $refreshCallback); + + $this->assertEquals('fresh-access-token', $result); + $this->assertFalse($refreshCallbackCalled); + } + + public function testGetAccessTokenNoLockRequiredWhenNotExpired(): void { + $userId = 'testuser'; + $validTokenData = [ + 'access_token' => 'valid-access-token', + 'refresh_token' => 'refresh-token', + 'expires_at' => time() + 3600, // Valid for 1 hour + ]; + + $this->config->method('getUserValue') + ->willReturn('encrypted-data'); + + $this->crypto->method('decrypt') + ->willReturn(json_encode($validTokenData)); + + // Lock should NOT be acquired for valid tokens + $this->lockingProvider->expects($this->never()) + ->method('acquireLock'); + + $this->lockingProvider->expects($this->never()) + ->method('releaseLock'); + + $result = $this->storage->getAccessToken($userId); + + $this->assertEquals('valid-access-token', $result); + } + // ========================================================================= // App Password Storage Tests (Multi-User Basic Auth) // =========================================================================