Skip to content

Commit fe548e1

Browse files
committed
fix(token): Implement locking for token refresh
Signed-off-by: Git'Fellow <[email protected]>
1 parent ebfbae0 commit fe548e1

File tree

1 file changed

+56
-18
lines changed

1 file changed

+56
-18
lines changed

lib/Service/TokenService.php

Lines changed: 56 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
use OCP\ISession;
3333
use OCP\IURLGenerator;
3434
use OCP\IUserSession;
35+
use OCP\Lock\ILockingProvider;
36+
use OCP\Lock\LockedException;
3537
use OCP\PreConditionNotMetException;
3638
use OCP\Security\ICrypto;
3739
use OCP\Session\Exceptions\SessionNotAvailableException;
@@ -45,6 +47,7 @@
4547
class TokenService {
4648

4749
private const SESSION_TOKEN_KEY = Application::APP_ID . '-user-token';
50+
private const REFRESH_LOCK_KEY = Application::APP_ID . '-lock-key';
4851

4952
private IClient $client;
5053

@@ -63,8 +66,8 @@ public function __construct(
6366
private IAppManager $appManager,
6467
private DiscoveryService $discoveryService,
6568
private ProviderMapper $providerMapper,
69+
private ILockingProvider $lockingProvider,
6670
) {
67-
6871
}
6972

7073
public function storeToken(array $tokenData): Token {
@@ -192,18 +195,56 @@ public function reauthenticate(int $providerId) {
192195
* @throws MultipleObjectsReturnedException
193196
*/
194197
public function refresh(Token $token): Token {
195-
$oidcProvider = $this->providerMapper->getProvider($token->getProviderId());
196-
$discovery = $this->discoveryService->obtainDiscovery($oidcProvider);
198+
$lockKey = self::REFRESH_LOCK_KEY . '_' . $this->session->getId();
199+
200+
// Retry loop to acquire lock with timeout
201+
$maxRetries = 50; // 5 seconds total (50 × 100ms)
202+
$retryCount = 0;
203+
$lockAcquired = false;
204+
205+
while (!$lockAcquired && $retryCount < $maxRetries) {
206+
try {
207+
$this->lockingProvider->acquireLock($lockKey, ILockingProvider::LOCK_EXCLUSIVE);
208+
$lockAcquired = true;
209+
$this->logger->debug('[TokenService] Acquired lock for token refresh');
210+
} catch (LockedException $e) {
211+
// Another request is refreshing, wait and retry
212+
$retryCount++;
213+
if ($retryCount >= $maxRetries) {
214+
$this->logger->warning('[TokenService] Failed to acquire lock after retries, returning old token');
215+
return $token;
216+
}
217+
usleep(100000); // 100ms between retries
218+
}
219+
}
197220

198221
try {
222+
// Double-check: the token might have been refreshed:
223+
// * while we were waiting for the lock (another request held it)
224+
// * OR in another process between the moment this process checked
225+
// the token expiration and the moment it attempted to acquire the lock
226+
$sessionData = $this->session->get(self::SESSION_TOKEN_KEY);
227+
if ($sessionData) {
228+
$currentToken = new Token(json_decode($sessionData, true, 512, JSON_THROW_ON_ERROR));
229+
if (!$currentToken->isExpired()) {
230+
$this->logger->debug('[TokenService] Token already refreshed by another request');
231+
return $currentToken;
232+
}
233+
}
234+
235+
// Token still expired, proceed with refresh
236+
$oidcProvider = $this->providerMapper->getProvider($token->getProviderId());
237+
$discovery = $this->discoveryService->obtainDiscovery($oidcProvider);
238+
199239
$clientSecret = $oidcProvider->getClientSecret();
200240
if ($clientSecret !== '') {
201241
try {
202242
$clientSecret = $this->crypto->decrypt($clientSecret);
203243
} catch (\Exception $e) {
204-
$this->logger->error('[TokenService] Failed to decrypt oidc client secret to refresh the token');
244+
$this->logger->error('[TokenService] Failed to decrypt oidc client secret');
205245
}
206246
}
247+
207248
$this->logger->debug('[TokenService] Refreshing the token: ' . $discovery['token_endpoint']);
208249
$body = $this->clientService->post(
209250
$discovery['token_endpoint'],
@@ -214,25 +255,24 @@ public function refresh(Token $token): Token {
214255
'refresh_token' => $token->getRefreshToken(),
215256
]
216257
);
217-
$this->logger->debug('[TokenService] Token refresh request params', [
218-
'client_id' => $oidcProvider->getClientId(),
219-
// 'client_secret' => $clientSecret,
220-
'grant_type' => 'refresh_token',
221-
// 'refresh_token' => $token->getRefreshToken(),
222-
]);
223258

224259
$bodyArray = json_decode(trim($body), true, 512, JSON_THROW_ON_ERROR);
225260
$this->logger->debug('[TokenService] ---- Refresh token success');
226261
return $this->storeToken(
227-
array_merge(
228-
$bodyArray,
229-
['provider_id' => $token->getProviderId()],
230-
)
262+
array_merge($bodyArray, ['provider_id' => $token->getProviderId()])
231263
);
232264
} catch (\Exception $e) {
233-
$this->logger->error('[TokenService] Failed to refresh token ', ['exception' => $e]);
234-
// Failed to refresh, return old token which will be retried or otherwise timeout if expired
265+
$this->logger->error('[TokenService] Failed to refresh token', ['exception' => $e]);
235266
return $token;
267+
} finally {
268+
if ($lockAcquired) {
269+
try {
270+
$this->lockingProvider->releaseLock($lockKey, ILockingProvider::LOCK_EXCLUSIVE);
271+
$this->logger->debug('[TokenService] Released lock for token refresh');
272+
} catch (\Exception $e) {
273+
$this->logger->error('[TokenService] Failed to release lock', ['exception' => $e]);
274+
}
275+
}
236276
}
237277
}
238278

@@ -316,9 +356,7 @@ public function getExchangedToken(string $targetAudience, array $extraScopes = [
316356
);
317357
$this->logger->debug('[TokenService] Token exchange request params', [
318358
'client_id' => $oidcProvider->getClientId(),
319-
// 'client_secret' => $clientSecret,
320359
'grant_type' => 'urn:ietf:params:oauth:grant-type:token-exchange',
321-
// 'subject_token' => $loginToken->getAccessToken(),
322360
'subject_token_type' => 'urn:ietf:params:oauth:token-type:access_token',
323361
'requested_token_type' => 'urn:ietf:params:oauth:token-type:refresh_token',
324362
'audience' => $targetAudience,

0 commit comments

Comments
 (0)