Skip to content

Commit 65e0bc7

Browse files
AltahrimAltahrim
committed
feat(perf): add cache for authtoken lookup
Signed-off-by: Benjamin Gaussorgues <[email protected]>
1 parent 9f7d574 commit 65e0bc7

File tree

4 files changed

+89
-77
lines changed

4 files changed

+89
-77
lines changed

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@ public function __construct(IDBConnection $db) {
4242

4343
/**
4444
* Invalidate (delete) a given token
45-
*
46-
* @param string $token
4745
*/
4846
public function invalidate(string $token) {
4947
/* @var $qb IQueryBuilder */
@@ -141,14 +139,15 @@ public function getTokenByUser(string $uid): array {
141139
return $entities;
142140
}
143141

144-
public function deleteById(string $uid, int $id) {
142+
public function getTokenByUserAndId(string $uid, int $id): ?string {
145143
/* @var $qb IQueryBuilder */
146144
$qb = $this->db->getQueryBuilder();
147-
$qb->delete($this->tableName)
145+
$qb->select('token')
146+
->from($this->tableName)
148147
->where($qb->expr()->eq('id', $qb->createNamedParameter($id)))
149148
->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($uid)))
150149
->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT)));
151-
$qb->execute();
150+
return $qb->executeQuery()->fetchOne() ?: null;
152151
}
153152

154153
/**

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

Lines changed: 71 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@
2929
*/
3030
namespace OC\Authentication\Token;
3131

32+
use OCP\ICache;
33+
use OCP\ICacheFactory;
3234
use OC\Authentication\Exceptions\ExpiredTokenException;
3335
use OC\Authentication\Exceptions\InvalidTokenException;
3436
use OC\Authentication\Exceptions\TokenPasswordExpiredException;
3537
use OC\Authentication\Exceptions\PasswordlessTokenException;
3638
use OC\Authentication\Exceptions\WipeTokenException;
3739
use OCP\AppFramework\Db\TTransactional;
38-
use OCP\Cache\CappedMemoryCache;
3940
use OCP\AppFramework\Db\DoesNotExistException;
4041
use OCP\AppFramework\Utility\ITimeFactory;
4142
use OCP\IConfig;
@@ -47,6 +48,8 @@
4748

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

5154
use TTransactional;
5255

@@ -67,26 +70,28 @@ class PublicKeyTokenProvider implements IProvider {
6770
/** @var ITimeFactory */
6871
private $time;
6972

70-
/** @var CappedMemoryCache */
73+
/** @var ICache */
7174
private $cache;
7275

73-
private IHasher $hasher;
76+
/** @var IHasher */
77+
private $hasher;
7478

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

89-
$this->cache = new CappedMemoryCache();
94+
$this->cache = $cacheFactory->createLocal('authtoken_');
9095
$this->hasher = $hasher;
9196
}
9297

@@ -128,7 +133,7 @@ public function generateToken(string $token,
128133
}
129134

130135
// Add the token to the cache
131-
$this->cache[$dbToken->getToken()] = $dbToken;
136+
$this->cacheToken($dbToken);
132137

133138
return $dbToken;
134139
}
@@ -156,43 +161,56 @@ public function getToken(string $tokenId): IToken {
156161
}
157162

158163
$tokenHash = $this->hashToken($tokenId);
164+
if ($token = $this->getTokenFromCache($tokenHash)) {
165+
$this->checkToken($token);
166+
return $token;
167+
}
159168

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

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

186-
if ($token->getType() === IToken::WIPE_TOKEN) {
187-
throw new WipeTokenException($token);
188-
}
184+
return $token;
185+
}
189186

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

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

198216
public function getTokenById(int $tokenId): IToken {
@@ -202,6 +220,12 @@ public function getTokenById(int $tokenId): IToken {
202220
throw new InvalidTokenException("Token with ID $tokenId does not exist: " . $ex->getMessage(), 0, $ex);
203221
}
204222

223+
$this->checkToken($token);
224+
225+
return $token;
226+
}
227+
228+
private function checkToken($token): void {
205229
if ((int)$token->getExpires() !== 0 && $token->getExpires() < $this->time->getTime()) {
206230
throw new ExpiredTokenException($token);
207231
}
@@ -214,13 +238,9 @@ public function getTokenById(int $tokenId): IToken {
214238
//The password is invalid we should throw an TokenPasswordExpiredException
215239
throw new TokenPasswordExpiredException($token);
216240
}
217-
218-
return $token;
219241
}
220242

221243
public function renewSessionToken(string $oldSessionId, string $sessionId): IToken {
222-
$this->cache->clear();
223-
224244
return $this->atomic(function () use ($oldSessionId, $sessionId) {
225245
$token = $this->getToken($oldSessionId);
226246

@@ -242,29 +262,32 @@ public function renewSessionToken(string $oldSessionId, string $sessionId): ITok
242262
IToken::TEMPORARY_TOKEN,
243263
$token->getRemember()
244264
);
265+
$this->cacheToken($newToken);
245266

267+
$this->cacheInvalidHash($token->getToken());
246268
$this->mapper->delete($token);
247269

248270
return $newToken;
249271
}, $this->db);
250272
}
251273

252274
public function invalidateToken(string $token) {
253-
$this->cache->clear();
254-
275+
$tokenHash = $this->hashToken($token);
255276
$this->mapper->invalidate($this->hashToken($token));
256277
$this->mapper->invalidate($this->hashTokenWithEmptySecret($token));
278+
$this->cacheInvalidHash($tokenHash);
257279
}
258280

259281
public function invalidateTokenById(string $uid, int $id) {
260-
$this->cache->clear();
261-
262-
$this->mapper->deleteById($uid, $id);
282+
$token = $this->mapper->getTokenById($id);
283+
if ($token->getUID() !== $uid) {
284+
return;
285+
}
286+
$this->mapper->invalidate($token->getToken());
287+
$this->cacheInvalidHash($token->getToken());
263288
}
264289

265290
public function invalidateOldTokens() {
266-
$this->cache->clear();
267-
268291
$olderThan = $this->time->getTime() - $this->config->getSystemValueInt('session_lifetime', 60 * 60 * 24);
269292
$this->logger->debug('Invalidating session tokens older than ' . date('c', $olderThan), ['app' => 'cron']);
270293
$this->mapper->invalidateOld($olderThan, IToken::DO_NOT_REMEMBER);
@@ -274,17 +297,14 @@ public function invalidateOldTokens() {
274297
}
275298

276299
public function updateToken(IToken $token) {
277-
$this->cache->clear();
278-
279300
if (!($token instanceof PublicKeyToken)) {
280301
throw new InvalidTokenException("Invalid token type");
281302
}
282303
$this->mapper->update($token);
304+
$this->cacheToken($token);
283305
}
284306

285307
public function updateTokenActivity(IToken $token) {
286-
$this->cache->clear();
287-
288308
if (!($token instanceof PublicKeyToken)) {
289309
throw new InvalidTokenException("Invalid token type");
290310
}
@@ -297,6 +317,7 @@ public function updateTokenActivity(IToken $token) {
297317
if ($token->getLastActivity() < ($now - $activityInterval)) {
298318
$token->setLastActivity($now);
299319
$this->mapper->updateActivity($token, $now);
320+
$this->cacheToken($token);
300321
}
301322
}
302323

@@ -321,8 +342,6 @@ public function getPassword(IToken $savedToken, string $tokenId): string {
321342
}
322343

323344
public function setPassword(IToken $token, string $tokenId, string $password) {
324-
$this->cache->clear();
325-
326345
if (!($token instanceof PublicKeyToken)) {
327346
throw new InvalidTokenException("Invalid token type");
328347
}
@@ -348,8 +367,6 @@ private function hashPassword(string $password): string {
348367
}
349368

350369
public function rotate(IToken $token, string $oldTokenId, string $newTokenId): IToken {
351-
$this->cache->clear();
352-
353370
if (!($token instanceof PublicKeyToken)) {
354371
throw new InvalidTokenException("Invalid token type");
355372
}
@@ -473,19 +490,16 @@ private function newToken(string $token,
473490
}
474491

475492
public function markPasswordInvalid(IToken $token, string $tokenId) {
476-
$this->cache->clear();
477-
478493
if (!($token instanceof PublicKeyToken)) {
479494
throw new InvalidTokenException("Invalid token type");
480495
}
481496

482497
$token->setPasswordInvalid(true);
483498
$this->mapper->update($token);
499+
$this->cacheToken($token);
484500
}
485501

486502
public function updatePasswords(string $uid, string $password) {
487-
$this->cache->clear();
488-
489503
// prevent setting an empty pw as result of pw-less-login
490504
if ($password === '' || !$this->config->getSystemValueBool('auth.storeCryptedPassword', true)) {
491505
return;

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

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ public function testGetTokenByUserNotFound() {
227227
$this->assertCount(0, $this->mapper->getTokenByUser('user1000'));
228228
}
229229

230-
public function testDeleteById() {
230+
public function testGetById() {
231231
/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */
232232
$user = $this->createMock(IUser::class);
233233
$qb = $this->dbConnection->getQueryBuilder();
@@ -237,17 +237,8 @@ public function testDeleteById() {
237237
$result = $qb->execute();
238238
$id = $result->fetch()['id'];
239239

240-
$this->mapper->deleteById('user1', (int)$id);
241-
$this->assertEquals(3, $this->getNumberOfTokens());
242-
}
243-
244-
public function testDeleteByIdWrongUser() {
245-
/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */
246-
$user = $this->createMock(IUser::class);
247-
$id = 33;
248-
249-
$this->mapper->deleteById('user1000', $id);
250-
$this->assertEquals(4, $this->getNumberOfTokens());
240+
$token = $this->mapper->getTokenById((int)$id);
241+
$this->assertEquals('user1', $token->getUID());
251242
}
252243

253244
public function testDeleteByName() {

0 commit comments

Comments
 (0)