Skip to content

Commit 5e684f2

Browse files
AltahrimAltahrim
committed
feat(perf): add cache for authtoken lookup
Signed-off-by: Benjamin Gaussorgues <[email protected]>
1 parent 6f95feb commit 5e684f2

File tree

4 files changed

+96
-81
lines changed

4 files changed

+96
-81
lines changed

‎lib/private/Authentication/Token/PublicKeyTokenMapper.php‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@
2727
*/
2828
namespace OC\Authentication\Token;
2929

30+
use Generator;
3031
use OCP\AppFramework\Db\DoesNotExistException;
3132
use OCP\AppFramework\Db\QBMapper;
33+
use OCP\Authentication\Token\IToken;
3234
use OCP\DB\QueryBuilder\IQueryBuilder;
3335
use OCP\IDBConnection;
3436

@@ -42,8 +44,6 @@ public function __construct(IDBConnection $db) {
4244

4345
/**
4446
* Invalidate (delete) a given token
45-
*
46-
* @param string $token
4747
*/
4848
public function invalidate(string $token) {
4949
/* @var $qb IQueryBuilder */
@@ -69,7 +69,10 @@ public function invalidateOld(int $olderThan, int $remember = IToken::DO_NOT_REM
6969
->execute();
7070
}
7171

72-
public function invalidateLastUsedBefore(string $uid, int $before): int {
72+
/**
73+
* @return Generator<string> Tokens
74+
*/
75+
public function invalidateLastUsedBefore(string $uid, int $before): Generator {
7376
$qb = $this->db->getQueryBuilder();
7477
$qb->delete($this->tableName)
7578
->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid)))
@@ -150,14 +153,15 @@ public function getTokenByUser(string $uid): array {
150153
return $entities;
151154
}
152155

153-
public function deleteById(string $uid, int $id) {
156+
public function getTokenByUserAndId(string $uid, int $id): ?string {
154157
/* @var $qb IQueryBuilder */
155158
$qb = $this->db->getQueryBuilder();
156-
$qb->delete($this->tableName)
159+
$qb->select('token')
160+
->from($this->tableName)
157161
->where($qb->expr()->eq('id', $qb->createNamedParameter($id)))
158162
->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($uid)))
159163
->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT)));
160-
$qb->execute();
164+
return $qb->executeQuery()->fetchOne() ?: null;
161165
}
162166

163167
/**

‎lib/private/Authentication/Token/PublicKeyTokenProvider.php‎

Lines changed: 73 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@
3838
use OCP\AppFramework\Db\TTransactional;
3939
use OCP\AppFramework\Utility\ITimeFactory;
4040
use OCP\Authentication\Token\IToken as OCPIToken;
41-
use OCP\Cache\CappedMemoryCache;
41+
use OCP\ICache;
42+
use OCP\ICacheFactory;
4243
use OCP\IConfig;
4344
use OCP\IDBConnection;
4445
use OCP\IUserManager;
@@ -48,6 +49,8 @@
4849

4950
class PublicKeyTokenProvider implements IProvider {
5051
public const TOKEN_MIN_LENGTH = 22;
52+
/** Token cache TTL in seconds */
53+
private const TOKEN_CACHE_TTL = 10;
5154

5255
use TTransactional;
5356

@@ -68,26 +71,30 @@ class PublicKeyTokenProvider implements IProvider {
6871
/** @var ITimeFactory */
6972
private $time;
7073

71-
/** @var CappedMemoryCache */
74+
/** @var ICache */
7275
private $cache;
7376

74-
private IHasher $hasher;
77+
/** @var IHasher */
78+
private $hasher;
7579

7680
public function __construct(PublicKeyTokenMapper $mapper,
7781
ICrypto $crypto,
7882
IConfig $config,
7983
IDBConnection $db,
8084
LoggerInterface $logger,
8185
ITimeFactory $time,
82-
IHasher $hasher) {
86+
IHasher $hasher,
87+
ICacheFactory $cacheFactory) {
8388
$this->mapper = $mapper;
8489
$this->crypto = $crypto;
8590
$this->config = $config;
8691
$this->db = $db;
8792
$this->logger = $logger;
8893
$this->time = $time;
8994

90-
$this->cache = new CappedMemoryCache();
95+
$this->cache = $cacheFactory->isLocalCacheAvailable()
96+
? $cacheFactory->createLocal('authtoken_')
97+
: $cacheFactory->createInMemory();
9198
$this->hasher = $hasher;
9299
}
93100

@@ -129,7 +136,7 @@ public function generateToken(string $token,
129136
}
130137

131138
// Add the token to the cache
132-
$this->cache[$dbToken->getToken()] = $dbToken;
139+
$this->cacheToken($dbToken);
133140

134141
return $dbToken;
135142
}
@@ -157,43 +164,56 @@ public function getToken(string $tokenId): OCPIToken {
157164
}
158165

159166
$tokenHash = $this->hashToken($tokenId);
167+
if ($token = $this->getTokenFromCache($tokenHash)) {
168+
$this->checkToken($token);
169+
return $token;
170+
}
160171

161-
if (isset($this->cache[$tokenHash])) {
162-
if ($this->cache[$tokenHash] instanceof DoesNotExistException) {
163-
$ex = $this->cache[$tokenHash];
164-
throw new InvalidTokenException("Token does not exist: " . $ex->getMessage(), 0, $ex);
165-
}
166-
$token = $this->cache[$tokenHash];
167-
} else {
172+
try {
173+
$token = $this->mapper->getToken($tokenHash);
174+
$this->cacheToken($token);
175+
} catch (DoesNotExistException $ex) {
168176
try {
169-
$token = $this->mapper->getToken($tokenHash);
170-
$this->cache[$token->getToken()] = $token;
171-
} catch (DoesNotExistException $ex) {
172-
try {
173-
$token = $this->mapper->getToken($this->hashTokenWithEmptySecret($tokenId));
174-
$this->cache[$token->getToken()] = $token;
175-
$this->rotate($token, $tokenId, $tokenId);
176-
} catch (DoesNotExistException $ex2) {
177-
$this->cache[$tokenHash] = $ex2;
178-
throw new InvalidTokenException("Token does not exist: " . $ex->getMessage(), 0, $ex);
179-
}
177+
$token = $this->mapper->getToken($this->hashTokenWithEmptySecret($tokenId));
178+
$this->rotate($token, $tokenId, $tokenId);
179+
} catch (DoesNotExistException) {
180+
$this->cacheInvalidHash($tokenHash);
181+
throw new InvalidTokenException("Token does not exist: " . $ex->getMessage(), 0, $ex);
180182
}
181183
}
182184

183-
if ((int)$token->getExpires() !== 0 && $token->getExpires() < $this->time->getTime()) {
184-
throw new ExpiredTokenException($token);
185-
}
185+
$this->checkToken($token);
186186

187-
if ($token->getType() === OCPIToken::WIPE_TOKEN) {
188-
throw new WipeTokenException($token);
189-
}
187+
return $token;
188+
}
190189

191-
if ($token->getPasswordInvalid() === true) {
192-
//The password is invalid we should throw an TokenPasswordExpiredException
193-
throw new TokenPasswordExpiredException($token);
190+
/**
191+
* @throws InvalidTokenException when token doesn't exist
192+
*/
193+
private function getTokenFromCache(string $tokenHash): ?PublicKeyToken {
194+
$serializedToken = $this->cache->get($tokenHash);
195+
if (null === $serializedToken) {
196+
if ($this->cache->hasKey($tokenHash)) {
197+
throw new InvalidTokenException('Token does not exist: ' . $tokenHash);
198+
}
199+
200+
return null;
194201
}
195202

196-
return $token;
203+
$token = unserialize($serializedToken, [
204+
'allowed_classes' => [PublicKeyToken::class],
205+
]);
206+
207+
return $token instanceof PublicKeyToken ? $token : null;
208+
}
209+
210+
private function cacheToken(PublicKeyToken $token): void {
211+
$this->cache->set($token->getToken(), serialize($token), self::TOKEN_CACHE_TTL);
212+
}
213+
214+
private function cacheInvalidHash(string $tokenHash) {
215+
// Invalid entries can be kept longer in cache since it’s unlikely to reuse them
216+
$this->cache->set($tokenHash, null, self::TOKEN_CACHE_TTL * 2);
197217
}
198218

199219
public function getTokenById(int $tokenId): OCPIToken {
@@ -203,6 +223,12 @@ public function getTokenById(int $tokenId): OCPIToken {
203223
throw new InvalidTokenException("Token with ID $tokenId does not exist: " . $ex->getMessage(), 0, $ex);
204224
}
205225

226+
$this->checkToken($token);
227+
228+
return $token;
229+
}
230+
231+
private function checkToken($token): void {
206232
if ((int)$token->getExpires() !== 0 && $token->getExpires() < $this->time->getTime()) {
207233
throw new ExpiredTokenException($token);
208234
}
@@ -215,13 +241,9 @@ public function getTokenById(int $tokenId): OCPIToken {
215241
//The password is invalid we should throw an TokenPasswordExpiredException
216242
throw new TokenPasswordExpiredException($token);
217243
}
218-
219-
return $token;
220244
}
221245

222246
public function renewSessionToken(string $oldSessionId, string $sessionId): OCPIToken {
223-
$this->cache->clear();
224-
225247
return $this->atomic(function () use ($oldSessionId, $sessionId) {
226248
$token = $this->getToken($oldSessionId);
227249

@@ -243,29 +265,33 @@ public function renewSessionToken(string $oldSessionId, string $sessionId): OCPI
243265
OCPIToken::TEMPORARY_TOKEN,
244266
$token->getRemember()
245267
);
268+
$this->cacheToken($newToken);
246269

270+
$this->cacheInvalidHash($token->getToken());
247271
$this->mapper->delete($token);
248272

249273
return $newToken;
250274
}, $this->db);
251275
}
252276

253277
public function invalidateToken(string $token) {
254-
$this->cache->clear();
255-
278+
$tokenHash = $this->hashToken($token);
256279
$this->mapper->invalidate($this->hashToken($token));
257280
$this->mapper->invalidate($this->hashTokenWithEmptySecret($token));
281+
$this->cacheInvalidHash($tokenHash);
258282
}
259283

260284
public function invalidateTokenById(string $uid, int $id) {
261-
$this->cache->clear();
285+
$token = $this->mapper->getTokenById($id);
286+
if ($token->getUID() !== $uid) {
287+
return;
288+
}
289+
$this->mapper->invalidate($token->getToken());
290+
$this->cacheInvalidHash($token->getToken());
262291

263-
$this->mapper->deleteById($uid, $id);
264292
}
265293

266294
public function invalidateOldTokens() {
267-
$this->cache->clear();
268-
269295
$olderThan = $this->time->getTime() - $this->config->getSystemValueInt('session_lifetime', 60 * 60 * 24);
270296
$this->logger->debug('Invalidating session tokens older than ' . date('c', $olderThan), ['app' => 'cron']);
271297
$this->mapper->invalidateOld($olderThan, OCPIToken::DO_NOT_REMEMBER);
@@ -275,23 +301,18 @@ public function invalidateOldTokens() {
275301
}
276302

277303
public function invalidateLastUsedBefore(string $uid, int $before): void {
278-
$this->cache->clear();
279-
280304
$this->mapper->invalidateLastUsedBefore($uid, $before);
281305
}
282306

283307
public function updateToken(OCPIToken $token) {
284-
$this->cache->clear();
285-
286308
if (!($token instanceof PublicKeyToken)) {
287309
throw new InvalidTokenException("Invalid token type");
288310
}
289311
$this->mapper->update($token);
312+
$this->cacheToken($token);
290313
}
291314

292315
public function updateTokenActivity(OCPIToken $token) {
293-
$this->cache->clear();
294-
295316
if (!($token instanceof PublicKeyToken)) {
296317
throw new InvalidTokenException("Invalid token type");
297318
}
@@ -304,6 +325,7 @@ public function updateTokenActivity(OCPIToken $token) {
304325
if ($token->getLastActivity() < ($now - $activityInterval)) {
305326
$token->setLastActivity($now);
306327
$this->mapper->updateActivity($token, $now);
328+
$this->cacheToken($token);
307329
}
308330
}
309331

@@ -328,8 +350,6 @@ public function getPassword(OCPIToken $savedToken, string $tokenId): string {
328350
}
329351

330352
public function setPassword(OCPIToken $token, string $tokenId, string $password) {
331-
$this->cache->clear();
332-
333353
if (!($token instanceof PublicKeyToken)) {
334354
throw new InvalidTokenException("Invalid token type");
335355
}
@@ -355,8 +375,6 @@ private function hashPassword(string $password): string {
355375
}
356376

357377
public function rotate(OCPIToken $token, string $oldTokenId, string $newTokenId): OCPIToken {
358-
$this->cache->clear();
359-
360378
if (!($token instanceof PublicKeyToken)) {
361379
throw new InvalidTokenException("Invalid token type");
362380
}
@@ -480,19 +498,16 @@ private function newToken(string $token,
480498
}
481499

482500
public function markPasswordInvalid(OCPIToken $token, string $tokenId) {
483-
$this->cache->clear();
484-
485501
if (!($token instanceof PublicKeyToken)) {
486502
throw new InvalidTokenException("Invalid token type");
487503
}
488504

489505
$token->setPasswordInvalid(true);
490506
$this->mapper->update($token);
507+
$this->cacheToken($token);
491508
}
492509

493510
public function updatePasswords(string $uid, string $password) {
494-
$this->cache->clear();
495-
496511
// prevent setting an empty pw as result of pw-less-login
497512
if ($password === '' || !$this->config->getSystemValueBool('auth.storeCryptedPassword', true)) {
498513
return;

‎tests/lib/Authentication/Token/PublicKeyTokenMapperTest.php‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@
2626
namespace Test\Authentication\Token;
2727

2828
use OC;
29-
use OC\Authentication\Token\IToken;
3029
use OC\Authentication\Token\PublicKeyToken;
3130
use OC\Authentication\Token\PublicKeyTokenMapper;
31+
use OCP\Authentication\Token\IToken;
3232
use OCP\DB\QueryBuilder\IQueryBuilder;
3333
use OCP\IDBConnection;
3434
use OCP\IUser;
@@ -249,7 +249,7 @@ public function testGetTokenByUserNotFound() {
249249
$this->assertCount(0, $this->mapper->getTokenByUser('user1000'));
250250
}
251251

252-
public function testDeleteById() {
252+
public function testGetById() {
253253
/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */
254254
$user = $this->createMock(IUser::class);
255255
$qb = $this->dbConnection->getQueryBuilder();
@@ -259,17 +259,8 @@ public function testDeleteById() {
259259
$result = $qb->execute();
260260
$id = $result->fetch()['id'];
261261

262-
$this->mapper->deleteById('user1', (int)$id);
263-
$this->assertEquals(4, $this->getNumberOfTokens());
264-
}
265-
266-
public function testDeleteByIdWrongUser() {
267-
/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */
268-
$user = $this->createMock(IUser::class);
269-
$id = 33;
270-
271-
$this->mapper->deleteById('user1000', $id);
272-
$this->assertEquals(5, $this->getNumberOfTokens());
262+
$token = $this->mapper->getTokenById((int)$id);
263+
$this->assertEquals('user1', $token->getUID());
273264
}
274265

275266
public function testDeleteByName() {

0 commit comments

Comments
 (0)