Skip to content

Commit

Permalink
feat(perf): add cache for authtoken lookup
Browse files Browse the repository at this point in the history 
Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>

[skip ci]
  • Loading branch information
@Altahrim @backportbot
Altahrim authored and backportbot[bot] committed Mar 25, 2024
1 parent b9c1383 commit d82f1e8
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 43 deletions.
10 changes: 5 additions & 5 deletions lib/private/Authentication/Token/PublicKeyTokenMapper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Db\QBMapper;
use OCP\Authentication\Token\IToken;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;

Expand All @@ -42,8 +43,6 @@ public function __construct(IDBConnection $db) {

/**
* Invalidate (delete) a given token
*
* @param string $token
*/
public function invalidate(string $token) {
/* @var $qb IQueryBuilder */
Expand Down Expand Up @@ -141,14 +140,15 @@ public function getTokenByUser(string $uid): array {
return $entities;
}

public function deleteById(string $uid, int $id) {
public function getTokenByUserAndId(string $uid, int $id): ?string {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$qb->delete($this->tableName)
$qb->select('token')
->from($this->tableName)
->where($qb->expr()->eq('id', $qb->createNamedParameter($id)))
->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($uid)))
->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(PublicKeyToken::VERSION, IQueryBuilder::PARAM_INT)));
$qb->execute();
return $qb->executeQuery()->fetchOne() ?: null;
}

/**
Expand Down
73 changes: 41 additions & 32 deletions lib/private/Authentication/Token/PublicKeyTokenProvider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@

class PublicKeyTokenProvider implements IProvider {
public const TOKEN_MIN_LENGTH = 22;
/** Token cache TTL in seconds */
private const TOKEN_CACHE_TTL = 10;

use TTransactional;

Expand All @@ -67,10 +69,11 @@ class PublicKeyTokenProvider implements IProvider {
/** @var ITimeFactory */
private $time;

/** @var CappedMemoryCache */
/** @var ICache */
private $cache;

private IHasher $hasher;
/** @var IHasher */
private $hasher;

public function __construct(PublicKeyTokenMapper $mapper,
ICrypto $crypto,
Expand All @@ -86,7 +89,9 @@ public function __construct(PublicKeyTokenMapper $mapper,
$this->logger = $logger;
$this->time = $time;

$this->cache = new CappedMemoryCache();
$this->cache = $cacheFactory->isLocalCacheAvailable()
? $cacheFactory->createLocal('authtoken_')
: $cacheFactory->createInMemory();
$this->hasher = $hasher;
}

Expand Down Expand Up @@ -128,7 +133,7 @@ public function generateToken(string $token,
}

// Add the token to the cache
$this->cache[$dbToken->getToken()] = $dbToken;
$this->cacheToken($dbToken);

return $dbToken;
}
Expand Down Expand Up @@ -156,26 +161,21 @@ public function getToken(string $tokenId): IToken {
}

$tokenHash = $this->hashToken($tokenId);
if ($token = $this->getTokenFromCache($tokenHash)) {
$this->checkToken($token);
return $token;
}

if (isset($this->cache[$tokenHash])) {
if ($this->cache[$tokenHash] instanceof DoesNotExistException) {
$ex = $this->cache[$tokenHash];
throw new InvalidTokenException("Token does not exist: " . $ex->getMessage(), 0, $ex);
}
$token = $this->cache[$tokenHash];
} else {
try {
$token = $this->mapper->getToken($tokenHash);
$this->cacheToken($token);
} catch (DoesNotExistException $ex) {
try {
$token = $this->mapper->getToken($tokenHash);
$this->cache[$token->getToken()] = $token;
} catch (DoesNotExistException $ex) {
try {
$token = $this->mapper->getToken($this->hashTokenWithEmptySecret($tokenId));
$this->cache[$token->getToken()] = $token;
$this->rotate($token, $tokenId, $tokenId);
} catch (DoesNotExistException $ex2) {
$this->cache[$tokenHash] = $ex2;
throw new InvalidTokenException("Token does not exist: " . $ex->getMessage(), 0, $ex);
}
$token = $this->mapper->getToken($this->hashTokenWithEmptySecret($tokenId));
$this->rotate($token, $tokenId, $tokenId);
} catch (DoesNotExistException) {
$this->cacheInvalidHash($tokenHash);
throw new InvalidTokenException("Token does not exist: " . $ex->getMessage(), 0, $ex);
}
}

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

$this->checkToken($token);

return $token;
}

private function checkToken($token): void {
if ((int)$token->getExpires() !== 0 && $token->getExpires() < $this->time->getTime()) {
throw new ExpiredTokenException($token);
}
Expand All @@ -214,8 +220,6 @@ public function getTokenById(int $tokenId): IToken {
//The password is invalid we should throw an TokenPasswordExpiredException
throw new TokenPasswordExpiredException($token);
}

return $token;
}

public function renewSessionToken(string $oldSessionId, string $sessionId): IToken {
Expand All @@ -242,29 +246,33 @@ public function renewSessionToken(string $oldSessionId, string $sessionId): ITok
IToken::TEMPORARY_TOKEN,
$token->getRemember()
);
$this->cacheToken($newToken);

$this->cacheInvalidHash($token->getToken());
$this->mapper->delete($token);

return $newToken;
}, $this->db);
}

public function invalidateToken(string $token) {
$this->cache->clear();

$tokenHash = $this->hashToken($token);
$this->mapper->invalidate($this->hashToken($token));
$this->mapper->invalidate($this->hashTokenWithEmptySecret($token));
$this->cacheInvalidHash($tokenHash);
}

public function invalidateTokenById(string $uid, int $id) {
$this->cache->clear();
$token = $this->mapper->getTokenById($id);
if ($token->getUID() !== $uid) {
return;
}
$this->mapper->invalidate($token->getToken());
$this->cacheInvalidHash($token->getToken());

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

public function invalidateOldTokens() {
$this->cache->clear();

$olderThan = $this->time->getTime() - $this->config->getSystemValueInt('session_lifetime', 60 * 60 * 24);
$this->logger->debug('Invalidating session tokens older than ' . date('c', $olderThan), ['app' => 'cron']);
$this->mapper->invalidateOld($olderThan, IToken::DO_NOT_REMEMBER);
Expand All @@ -280,6 +288,7 @@ public function updateToken(IToken $token) {
throw new InvalidTokenException("Invalid token type");
}
$this->mapper->update($token);
$this->cacheToken($token);
}

public function updateTokenActivity(IToken $token) {
Expand All @@ -297,6 +306,7 @@ public function updateTokenActivity(IToken $token) {
if ($token->getLastActivity() < ($now - $activityInterval)) {
$token->setLastActivity($now);
$this->mapper->updateActivity($token, $now);
$this->cacheToken($token);
}
}

Expand Down Expand Up @@ -481,11 +491,10 @@ public function markPasswordInvalid(IToken $token, string $tokenId) {

$token->setPasswordInvalid(true);
$this->mapper->update($token);
$this->cacheToken($token);
}

public function updatePasswords(string $uid, string $password) {
$this->cache->clear();

// prevent setting an empty pw as result of pw-less-login
if ($password === '' || !$this->config->getSystemValueBool('auth.storeCryptedPassword', true)) {
return;
Expand Down
4 changes: 2 additions & 2 deletions tests/lib/Authentication/Token/PublicKeyTokenMapperTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
namespace Test\Authentication\Token;

use OC;
use OC\Authentication\Token\IToken;
use OC\Authentication\Token\PublicKeyToken;
use OC\Authentication\Token\PublicKeyTokenMapper;
use OCP\Authentication\Token\IToken;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
use OCP\IUser;
Expand Down Expand Up @@ -227,7 +227,7 @@ public function testGetTokenByUserNotFound() {
$this->assertCount(0, $this->mapper->getTokenByUser('user1000'));
}

public function testDeleteById() {
public function testGetById() {
/** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */
$user = $this->createMock(IUser::class);
$qb = $this->dbConnection->getQueryBuilder();
Expand Down
13 changes: 9 additions & 4 deletions tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@
use OC\Authentication\Exceptions\ExpiredTokenException;
use OC\Authentication\Exceptions\InvalidTokenException;
use OC\Authentication\Exceptions\PasswordlessTokenException;
use OC\Authentication\Token\IToken;
use OC\Authentication\Token\PublicKeyToken;
use OC\Authentication\Token\PublicKeyTokenMapper;
use OC\Authentication\Token\PublicKeyTokenProvider;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Authentication\Token\IToken;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Security\ICrypto;
Expand All @@ -57,6 +58,8 @@ class PublicKeyTokenProviderTest extends TestCase {
private $logger;
/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
private $timeFactory;
/** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */
private $cacheFactory;
/** @var int */
private $time;

Expand Down Expand Up @@ -87,6 +90,7 @@ protected function setUp(): void {
$this->time = 1313131;
$this->timeFactory->method('getTime')
->willReturn($this->time);
$this->cacheFactory = $this->createMock(ICacheFactory::class);

$this->tokenProvider = new PublicKeyTokenProvider(
$this->mapper,
Expand All @@ -96,6 +100,7 @@ protected function setUp(): void {
$this->logger,
$this->timeFactory,
$this->hasher,
$this->cacheFactory,
);
}

Expand Down Expand Up @@ -329,12 +334,12 @@ public function testInvalidateToken() {
$this->tokenProvider->invalidateToken('token7');
}

public function testInvaildateTokenById() {
public function testInvalidateTokenById() {
$id = 123;

$this->mapper->expects($this->once())
->method('deleteById')
->with('uid', $id);
->method('getTokenById')
->with($id);

$this->tokenProvider->invalidateTokenById('uid', $id);
}
Expand Down

0 comments on commit d82f1e8

Please sign in to comment.