diff options
3 files changed, 46 insertions, 9 deletions
diff --git a/lib/private/Authentication/Token/PublicKeyTokenMapper.php b/lib/private/Authentication/Token/PublicKeyTokenMapper.php index 8aab7daf623..0c532312ace 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenMapper.php +++ b/lib/private/Authentication/Token/PublicKeyTokenMapper.php @@ -190,4 +190,43 @@ class PublicKeyTokenMapper extends QBMapper { return count($data) === 1; } + + /** + * Update the last activity timestamp + * + * In highly concurrent setups it can happen that two parallel processes + * trigger the update at (nearly) the same time. In that special case it's + * not necessary to hit the database with two actual updates. Therefore the + * target last activity is included in the WHERE clause with a few seconds + * of tolerance. + * + * Example: + * - process 1 (P1) reads the token at timestamp 1500 + * - process 1 (P2) reads the token at timestamp 1501 + * - activity update interval is 100 + * + * This means + * + * - P1 will see a last_activity smaller than the current time and update + * the token row + * - If P2 reads after P1 had written, it will see 1600 as last activity + * and the comparison on last_activity won't be truthy. This means no rows + * need to be updated a second time + * - If P2 reads before P1 had written, it will see 1501 as last activity, + * but the comparison on last_activity will still not be truthy and the + * token row is not updated a second time + * + * @param IToken $token + * @param int $now + */ + public function updateActivity(IToken $token, int $now): void { + $qb = $this->db->getQueryBuilder(); + $update = $qb->update($this->getTableName()) + ->set('last_activity', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT)) + ->where( + $qb->expr()->eq('id', $qb->createNamedParameter($token->getId(), IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT), + $qb->expr()->lt('last_activity', $qb->createNamedParameter($now - 15, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT) + ); + $update->executeStatement(); + } } diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index e8149319904..c2af5de8a03 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -221,9 +221,8 @@ class PublicKeyTokenProvider implements IProvider { /** @var PublicKeyToken $token */ $now = $this->time->getTime(); if ($token->getLastActivity() < ($now - $activityInterval)) { - // Update token only once per minute $token->setLastActivity($now); - $this->mapper->update($token); + $this->mapper->updateActivity($token, $now); } } diff --git a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php index f27100b5d78..486660f17c6 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php @@ -100,10 +100,10 @@ class PublicKeyTokenProviderTest extends TestCase { public function testUpdateToken() { $tk = new PublicKeyToken(); - $tk->setLastActivity($this->time - 200); $this->mapper->expects($this->once()) - ->method('update') - ->with($tk); + ->method('updateActivity') + ->with($tk, $this->time); + $tk->setLastActivity($this->time - 200); $this->tokenProvider->updateTokenActivity($tk); @@ -112,16 +112,15 @@ class PublicKeyTokenProviderTest extends TestCase { public function testUpdateTokenDebounce() { $tk = new PublicKeyToken(); - $this->config->method('getSystemValueInt') ->willReturnCallback(function ($value, $default) { return $default; }); - $tk->setLastActivity($this->time - 30); + $this->mapper->expects($this->never()) - ->method('update') - ->with($tk); + ->method('updateActivity') + ->with($tk, $this->time); $this->tokenProvider->updateTokenActivity($tk); } |