From d4f97affc1a0ecfaacfbdc26aab820cad1650a06 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 6 Sep 2021 16:31:01 +0200 Subject: Add database ratelimiting backend In case no distributed memory cache is specified this adds a database backend for ratelimit purposes. Signed-off-by: Lukas Reschke --- core/Migrations/Version23000Date20210906132259.php | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 core/Migrations/Version23000Date20210906132259.php (limited to 'core') diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php new file mode 100644 index 00000000000..b0eb7bc4dcc --- /dev/null +++ b/core/Migrations/Version23000Date20210906132259.php @@ -0,0 +1,47 @@ +hasTable(self::TABLE_NAME); + + if(!$hasTable) + { + $table = $schema->createTable(self::TABLE_NAME); + $table->addColumn('hash', Types::STRING, [ + 'notnull' => true, + 'length' => 128, + ]); + $table->addColumn('timestamp', 'datetime', [ + 'notnull' => true, + ]); + $table->addIndex(['hash'], 'ratelimit_hash_idx'); + $table->addIndex(['timestamp'], 'ratelimit_timestamp_idx'); + } + + return $schema; + } +} -- cgit v1.2.3 From 378cc922c429524b872e83c7b3842eb86bc4b770 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 6 Sep 2021 17:31:36 +0200 Subject: Adjust logic to store period instead of current timestamp Signed-off-by: Lukas Reschke --- core/Migrations/Version23000Date20210906132259.php | 4 +- lib/composer/composer/autoload_classmap.php | 6 - lib/composer/composer/autoload_psr4.php | 1 - lib/composer/composer/autoload_static.php | 14 -- lib/composer/composer/installed.json | 62 +-------- lib/composer/composer/installed.php | 15 +-- .../RateLimiting/Backend/DatabaseBackend.php | 37 +++-- .../Security/RateLimiting/Backend/IBackend.php | 6 +- .../Security/RateLimiting/Backend/MemoryCache.php | 128 ------------------ .../RateLimiting/Backend/MemoryCacheBackend.php | 126 +++++++++++++++++ lib/private/Security/RateLimiting/Limiter.php | 11 +- lib/private/Server.php | 2 +- .../Backend/MemoryCacheBackendTest.php | 149 +++++++++++++++++++++ .../RateLimiting/Backend/MemoryCacheTest.php | 149 --------------------- tests/lib/Security/RateLimiting/LimiterTest.php | 13 -- 15 files changed, 305 insertions(+), 418 deletions(-) delete mode 100644 lib/private/Security/RateLimiting/Backend/MemoryCache.php create mode 100644 lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php create mode 100644 tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php delete mode 100644 tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php (limited to 'core') diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php index b0eb7bc4dcc..c4e0c42075c 100644 --- a/core/Migrations/Version23000Date20210906132259.php +++ b/core/Migrations/Version23000Date20210906132259.php @@ -35,11 +35,11 @@ class Version23000Date20210906132259 extends SimpleMigrationStep { 'notnull' => true, 'length' => 128, ]); - $table->addColumn('timestamp', 'datetime', [ + $table->addColumn('delete_after', 'datetime', [ 'notnull' => true, ]); $table->addIndex(['hash'], 'ratelimit_hash_idx'); - $table->addIndex(['timestamp'], 'ratelimit_timestamp_idx'); + $table->addIndex(['delete_after'], 'ratelimit_delete_after_idx'); } return $schema; diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index ca0607a5d35..ea1473f27dc 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -6,10 +6,6 @@ $vendorDir = dirname(dirname(__FILE__)); $baseDir = dirname(dirname($vendorDir)); return array( - 'Bamarni\\Composer\\Bin\\BinCommand' => $vendorDir . '/bamarni/composer-bin-plugin/src/BinCommand.php', - 'Bamarni\\Composer\\Bin\\CommandProvider' => $vendorDir . '/bamarni/composer-bin-plugin/src/CommandProvider.php', - 'Bamarni\\Composer\\Bin\\Config' => $vendorDir . '/bamarni/composer-bin-plugin/src/Config.php', - 'Bamarni\\Composer\\Bin\\Plugin' => $vendorDir . '/bamarni/composer-bin-plugin/src/Plugin.php', 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', 'OCP\\Accounts\\IAccount' => $baseDir . '/lib/public/Accounts/IAccount.php', 'OCP\\Accounts\\IAccountManager' => $baseDir . '/lib/public/Accounts/IAccountManager.php', @@ -973,7 +969,6 @@ return array( 'OC\\Core\\Migrations\\Version21000Date20210309185126' => $baseDir . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version21000Date20210309185127' => $baseDir . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => $baseDir . '/core/Migrations/Version22000Date20210216080825.php', - 'OC\\Core\\Migrations\\Version23000Date20210906132259' => $baseDir . '/core/Migrations/Version23000Date20210906132259.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', @@ -1370,7 +1365,6 @@ return array( 'OC\\Security\\IdentityProof\\Manager' => $baseDir . '/lib/private/Security/IdentityProof/Manager.php', 'OC\\Security\\IdentityProof\\Signer' => $baseDir . '/lib/private/Security/IdentityProof/Signer.php', 'OC\\Security\\Normalizer\\IpAddress' => $baseDir . '/lib/private/Security/Normalizer/IpAddress.php', - 'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php', 'OC\\Security\\RateLimiting\\Backend\\IBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/IBackend.php', 'OC\\Security\\RateLimiting\\Backend\\MemoryCache' => $baseDir . '/lib/private/Security/RateLimiting/Backend/MemoryCache.php', 'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => $baseDir . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php', diff --git a/lib/composer/composer/autoload_psr4.php b/lib/composer/composer/autoload_psr4.php index fe0f6a5fed2..b641d9c6a03 100644 --- a/lib/composer/composer/autoload_psr4.php +++ b/lib/composer/composer/autoload_psr4.php @@ -9,6 +9,5 @@ return array( 'OC\\Core\\' => array($baseDir . '/core'), 'OC\\' => array($baseDir . '/lib/private'), 'OCP\\' => array($baseDir . '/lib/public'), - 'Bamarni\\Composer\\Bin\\' => array($vendorDir . '/bamarni/composer-bin-plugin/src'), '' => array($baseDir . '/lib/private/legacy'), ); diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 98633252c40..93208f6ff15 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -13,10 +13,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\' => 3, 'OCP\\' => 4, ), - 'B' => - array ( - 'Bamarni\\Composer\\Bin\\' => 21, - ), ); public static $prefixDirsPsr4 = array ( @@ -32,10 +28,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c array ( 0 => __DIR__ . '/../../..' . '/lib/public', ), - 'Bamarni\\Composer\\Bin\\' => - array ( - 0 => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src', - ), ); public static $fallbackDirsPsr4 = array ( @@ -43,10 +35,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c ); public static $classMap = array ( - 'Bamarni\\Composer\\Bin\\BinCommand' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/BinCommand.php', - 'Bamarni\\Composer\\Bin\\CommandProvider' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/CommandProvider.php', - 'Bamarni\\Composer\\Bin\\Config' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/Config.php', - 'Bamarni\\Composer\\Bin\\Plugin' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/Plugin.php', 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', 'OCP\\Accounts\\IAccount' => __DIR__ . '/../../..' . '/lib/public/Accounts/IAccount.php', 'OCP\\Accounts\\IAccountManager' => __DIR__ . '/../../..' . '/lib/public/Accounts/IAccountManager.php', @@ -1010,7 +998,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version21000Date20210309185126' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version21000Date20210309185127' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => __DIR__ . '/../../..' . '/core/Migrations/Version22000Date20210216080825.php', - 'OC\\Core\\Migrations\\Version23000Date20210906132259' => __DIR__ . '/../../..' . '/core/Migrations/Version23000Date20210906132259.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', @@ -1407,7 +1394,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Security\\IdentityProof\\Manager' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Manager.php', 'OC\\Security\\IdentityProof\\Signer' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Signer.php', 'OC\\Security\\Normalizer\\IpAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Normalizer/IpAddress.php', - 'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php', 'OC\\Security\\RateLimiting\\Backend\\IBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/IBackend.php', 'OC\\Security\\RateLimiting\\Backend\\MemoryCache' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/MemoryCache.php', 'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php', diff --git a/lib/composer/composer/installed.json b/lib/composer/composer/installed.json index 9c4acdfa483..f20a6c47c6d 100644 --- a/lib/composer/composer/installed.json +++ b/lib/composer/composer/installed.json @@ -1,61 +1,5 @@ { - "packages": [ - { - "name": "bamarni/composer-bin-plugin", - "version": "1.4.1", - "version_normalized": "1.4.1.0", - "source": { - "type": "git", - "url": "https://github.com/bamarni/composer-bin-plugin.git", - "reference": "9329fb0fbe29e0e1b2db8f4639a193e4f5406225" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/bamarni/composer-bin-plugin/zipball/9329fb0fbe29e0e1b2db8f4639a193e4f5406225", - "reference": "9329fb0fbe29e0e1b2db8f4639a193e4f5406225", - "shasum": "" - }, - "require": { - "composer-plugin-api": "^1.0 || ^2.0", - "php": "^5.5.9 || ^7.0 || ^8.0" - }, - "require-dev": { - "composer/composer": "^1.0 || ^2.0", - "symfony/console": "^2.5 || ^3.0 || ^4.0" - }, - "time": "2020-05-03T08:27:20+00:00", - "type": "composer-plugin", - "extra": { - "class": "Bamarni\\Composer\\Bin\\Plugin" - }, - "installation-source": "dist", - "autoload": { - "psr-4": { - "Bamarni\\Composer\\Bin\\": "src" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "description": "No conflicts for your bin dependencies", - "keywords": [ - "composer", - "conflict", - "dependency", - "executable", - "isolation", - "tool" - ], - "support": { - "issues": "https://github.com/bamarni/composer-bin-plugin/issues", - "source": "https://github.com/bamarni/composer-bin-plugin/tree/master" - }, - "install-path": "../bamarni/composer-bin-plugin" - } - ], - "dev": true, - "dev-package-names": [ - "bamarni/composer-bin-plugin" - ] + "packages": [], + "dev": false, + "dev-package-names": [] } diff --git a/lib/composer/composer/installed.php b/lib/composer/composer/installed.php index 583c0c32593..b811738fbd0 100644 --- a/lib/composer/composer/installed.php +++ b/lib/composer/composer/installed.php @@ -5,9 +5,9 @@ 'type' => 'library', 'install_path' => __DIR__ . '/../../../', 'aliases' => array(), - 'reference' => '33a0b75c83a1c56fa84b98d3a07a26b5c4932b65', + 'reference' => '66144c300395458ff38b86e50cd92174443cd85e', 'name' => '__root__', - 'dev' => true, + 'dev' => false, ), 'versions' => array( '__root__' => array( @@ -16,17 +16,8 @@ 'type' => 'library', 'install_path' => __DIR__ . '/../../../', 'aliases' => array(), - 'reference' => '33a0b75c83a1c56fa84b98d3a07a26b5c4932b65', + 'reference' => '66144c300395458ff38b86e50cd92174443cd85e', 'dev_requirement' => false, ), - 'bamarni/composer-bin-plugin' => array( - 'pretty_version' => '1.4.1', - 'version' => '1.4.1.0', - 'type' => 'composer-plugin', - 'install_path' => __DIR__ . '/../bamarni/composer-bin-plugin', - 'aliases' => array(), - 'reference' => '9329fb0fbe29e0e1b2db8f4639a193e4f5406225', - 'dev_requirement' => true, - ), ), ); diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php index a7f2a7b4b62..3b7382d747e 100644 --- a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -71,21 +71,28 @@ class DatabaseBackend implements IBackend { * @throws \OCP\DB\Exception */ private function getExistingAttemptCount( - string $identifier, - int $seconds + string $identifier ): int { + $currentTime = $this->timeFactory->getDateTime(); + $qb = $this->dbConnection->getQueryBuilder(); - $notOlderThan = $this->timeFactory->getDateTime()->sub(new \DateInterval("PT{$seconds}S")); + $qb->delete(self::TABLE_NAME) + ->where( + $qb->expr()->lte('delete_after', $qb->createParameter('currentTime')) + ) + ->setParameter('currentTime', $currentTime, 'datetime') + ->executeStatement(); + $qb = $this->dbConnection->getQueryBuilder(); $qb->selectAlias($qb->createFunction('COUNT(*)'), 'count') ->from(self::TABLE_NAME) ->where( $qb->expr()->eq('hash', $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR)) ) ->andWhere( - $qb->expr()->gte('timestamp', $qb->createParameter('notOlderThan')) + $qb->expr()->gte('delete_after', $qb->createParameter('currentTime')) ) - ->setParameter('notOlderThan', $notOlderThan, 'datetime'); + ->setParameter('currentTime', $currentTime, 'datetime'); $cursor = $qb->executeQuery(); $row = $cursor->fetch(); @@ -98,10 +105,9 @@ class DatabaseBackend implements IBackend { * {@inheritDoc} */ public function getAttempts(string $methodIdentifier, - string $userIdentifier, - int $seconds): int { + string $userIdentifier): int { $identifier = $this->hash($methodIdentifier, $userIdentifier); - return $this->getExistingAttemptCount($identifier, $seconds); + return $this->getExistingAttemptCount($identifier); } /** @@ -111,25 +117,14 @@ class DatabaseBackend implements IBackend { string $userIdentifier, int $period) { $identifier = $this->hash($methodIdentifier, $userIdentifier); - $currentTime = $this->timeFactory->getDateTime(); - $notOlderThan = $this->timeFactory->getDateTime('@' . $period); + $deleteAfter = $this->timeFactory->getDateTime()->add(new \DateInterval("PT{$period}S")); $qb = $this->dbConnection->getQueryBuilder(); - $qb->delete(self::TABLE_NAME) - ->where( - $qb->expr()->eq('hash', $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR)) - ) - ->andWhere( - $qb->expr()->lt('timestamp', $qb->createParameter('notOlderThan')) - ) - ->setParameter('notOlderThan', $notOlderThan, 'datetime') - ->executeStatement(); - $qb->insert(self::TABLE_NAME) ->values([ 'hash' => $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR), - 'timestamp' => $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE), + 'delete_after' => $qb->createNamedParameter($deleteAfter, IQueryBuilder::PARAM_DATE), ]) ->executeStatement(); } diff --git a/lib/private/Security/RateLimiting/Backend/IBackend.php b/lib/private/Security/RateLimiting/Backend/IBackend.php index d87f53311b2..835a97993da 100644 --- a/lib/private/Security/RateLimiting/Backend/IBackend.php +++ b/lib/private/Security/RateLimiting/Backend/IBackend.php @@ -35,16 +35,14 @@ namespace OC\Security\RateLimiting\Backend; */ interface IBackend { /** - * Gets the amount of attempts within the last specified seconds + * Gets the amount of attempts for the specified method * * @param string $methodIdentifier Identifier for the method * @param string $userIdentifier Identifier for the user - * @param int $seconds Seconds to look back at * @return int */ public function getAttempts(string $methodIdentifier, - string $userIdentifier, - int $seconds): int; + string $userIdentifier): int; /** * Registers an attempt diff --git a/lib/private/Security/RateLimiting/Backend/MemoryCache.php b/lib/private/Security/RateLimiting/Backend/MemoryCache.php deleted file mode 100644 index 0dab25e4048..00000000000 --- a/lib/private/Security/RateLimiting/Backend/MemoryCache.php +++ /dev/null @@ -1,128 +0,0 @@ - - * - * @author Christoph Wurst - * @author Lukas Reschke - * @author Morris Jobke - * @author Roeland Jago Douma - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * - */ -namespace OC\Security\RateLimiting\Backend; - -use OCP\AppFramework\Utility\ITimeFactory; -use OCP\ICache; -use OCP\ICacheFactory; - -/** - * Class MemoryCache uses the configured distributed memory cache for storing - * rate limiting data. - * - * @package OC\Security\RateLimiting\Backend - */ -class MemoryCache implements IBackend { - /** @var ICache */ - private $cache; - /** @var ITimeFactory */ - private $timeFactory; - - /** - * @param ICacheFactory $cacheFactory - * @param ITimeFactory $timeFactory - */ - public function __construct(ICacheFactory $cacheFactory, - ITimeFactory $timeFactory) { - $this->cache = $cacheFactory->createDistributed(__CLASS__); - $this->timeFactory = $timeFactory; - } - - /** - * @param string $methodIdentifier - * @param string $userIdentifier - * @return string - */ - private function hash(string $methodIdentifier, - string $userIdentifier): string { - return hash('sha512', $methodIdentifier . $userIdentifier); - } - - /** - * @param string $identifier - * @return array - */ - private function getExistingAttempts(string $identifier): array { - $cachedAttempts = $this->cache->get($identifier); - if ($cachedAttempts === null) { - return []; - } - - $cachedAttempts = json_decode($cachedAttempts, true); - if (\is_array($cachedAttempts)) { - return $cachedAttempts; - } - - return []; - } - - /** - * {@inheritDoc} - */ - public function getAttempts(string $methodIdentifier, - string $userIdentifier, - int $seconds): int { - $identifier = $this->hash($methodIdentifier, $userIdentifier); - $existingAttempts = $this->getExistingAttempts($identifier); - - $count = 0; - $currentTime = $this->timeFactory->getTime(); - /** @var array $existingAttempts */ - foreach ($existingAttempts as $attempt) { - if (($attempt + $seconds) > $currentTime) { - $count++; - } - } - - return $count; - } - - /** - * {@inheritDoc} - */ - public function registerAttempt(string $methodIdentifier, - string $userIdentifier, - int $period) { - $identifier = $this->hash($methodIdentifier, $userIdentifier); - $existingAttempts = $this->getExistingAttempts($identifier); - $currentTime = $this->timeFactory->getTime(); - - // Unset all attempts older than $period - foreach ($existingAttempts as $key => $attempt) { - if (($attempt + $period) < $currentTime) { - unset($existingAttempts[$key]); - } - } - $existingAttempts = array_values($existingAttempts); - - // Store the new attempt - $existingAttempts[] = (string)$currentTime; - $this->cache->set($identifier, json_encode($existingAttempts)); - } -} diff --git a/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php b/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php new file mode 100644 index 00000000000..f4880fb239c --- /dev/null +++ b/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php @@ -0,0 +1,126 @@ + + * + * @author Christoph Wurst + * @author Lukas Reschke + * @author Morris Jobke + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OC\Security\RateLimiting\Backend; + +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\ICache; +use OCP\ICacheFactory; + +/** + * Class MemoryCacheBackend uses the configured distributed memory cache for storing + * rate limiting data. + * + * @package OC\Security\RateLimiting\Backend + */ +class MemoryCacheBackend implements IBackend { + /** @var ICache */ + private $cache; + /** @var ITimeFactory */ + private $timeFactory; + + /** + * @param ICacheFactory $cacheFactory + * @param ITimeFactory $timeFactory + */ + public function __construct(ICacheFactory $cacheFactory, + ITimeFactory $timeFactory) { + $this->cache = $cacheFactory->createDistributed(__CLASS__); + $this->timeFactory = $timeFactory; + } + + /** + * @param string $methodIdentifier + * @param string $userIdentifier + * @return string + */ + private function hash(string $methodIdentifier, + string $userIdentifier): string { + return hash('sha512', $methodIdentifier . $userIdentifier); + } + + /** + * @param string $identifier + * @return array + */ + private function getExistingAttempts(string $identifier): array { + $cachedAttempts = $this->cache->get($identifier); + if ($cachedAttempts === null) { + return []; + } + + $cachedAttempts = json_decode($cachedAttempts, true); + if (\is_array($cachedAttempts)) { + return $cachedAttempts; + } + + return []; + } + + /** + * {@inheritDoc} + */ + public function getAttempts(string $methodIdentifier, + string $userIdentifier): int { + $identifier = $this->hash($methodIdentifier, $userIdentifier); + $existingAttempts = $this->getExistingAttempts($identifier); + + $count = 0; + $currentTime = $this->timeFactory->getTime(); + foreach ($existingAttempts as $expirationTime) { + if ($expirationTime > $currentTime) { + $count++; + } + } + + return $count; + } + + /** + * {@inheritDoc} + */ + public function registerAttempt(string $methodIdentifier, + string $userIdentifier, + int $period) { + $identifier = $this->hash($methodIdentifier, $userIdentifier); + $existingAttempts = $this->getExistingAttempts($identifier); + $currentTime = $this->timeFactory->getTime(); + + // Unset all attempts that are already expired + foreach ($existingAttempts as $key => $expirationTime) { + if ($expirationTime < $currentTime) { + unset($existingAttempts[$key]); + } + } + $existingAttempts = array_values($existingAttempts); + + // Store the new attempt + $existingAttempts[] = (string)($currentTime + $period); + $this->cache->set($identifier, json_encode($existingAttempts)); + } +} diff --git a/lib/private/Security/RateLimiting/Limiter.php b/lib/private/Security/RateLimiting/Limiter.php index ede72e887fc..1c14fce2a55 100644 --- a/lib/private/Security/RateLimiting/Limiter.php +++ b/lib/private/Security/RateLimiting/Limiter.php @@ -35,17 +35,12 @@ use OCP\IUser; class Limiter { /** @var IBackend */ private $backend; - /** @var ITimeFactory */ - private $timeFactory; /** - * @param ITimeFactory $timeFactory * @param IBackend $backend */ - public function __construct(ITimeFactory $timeFactory, - IBackend $backend) { + public function __construct(IBackend $backend) { $this->backend = $backend; - $this->timeFactory = $timeFactory; } /** @@ -59,12 +54,12 @@ class Limiter { string $userIdentifier, int $period, int $limit): void { - $existingAttempts = $this->backend->getAttempts($methodIdentifier, $userIdentifier, $period); + $existingAttempts = $this->backend->getAttempts($methodIdentifier, $userIdentifier); if ($existingAttempts >= $limit) { throw new RateLimitExceededException(); } - $this->backend->registerAttempt($methodIdentifier, $userIdentifier, $this->timeFactory->getTime()); + $this->backend->registerAttempt($methodIdentifier, $userIdentifier, $period); } /** diff --git a/lib/private/Server.php b/lib/private/Server.php index 83a3c905f94..a70b25d90af 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -787,7 +787,7 @@ class Server extends ServerContainer implements IServerContainer { $this->registerService(\OC\Security\RateLimiting\Backend\IBackend::class, function ($c) { $cacheFactory = $c->get(ICacheFactory::class); if ($cacheFactory->isAvailable()) { - $backend = new \OC\Security\RateLimiting\Backend\MemoryCache( + $backend = new \OC\Security\RateLimiting\Backend\MemoryCacheBackend( $this->get(ICacheFactory::class), new \OC\AppFramework\Utility\TimeFactory() ); diff --git a/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php b/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php new file mode 100644 index 00000000000..8bbf88e8a8e --- /dev/null +++ b/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php @@ -0,0 +1,149 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace Test\Security\RateLimiting\Backend; + +use OC\Security\RateLimiting\Backend\MemoryCacheBackend; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\ICache; +use OCP\ICacheFactory; +use Test\TestCase; + +class MemoryCacheBackendTest extends TestCase { + /** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */ + private $cacheFactory; + /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ + private $timeFactory; + /** @var ICache|\PHPUnit\Framework\MockObject\MockObject */ + private $cache; + /** @var MemoryCacheBackend */ + private $memoryCache; + + protected function setUp(): void { + parent::setUp(); + + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->cache = $this->createMock(ICache::class); + + $this->cacheFactory + ->expects($this->once()) + ->method('createDistributed') + ->with('OC\Security\RateLimiting\Backend\MemoryCacheBackend') + ->willReturn($this->cache); + + $this->memoryCache = new MemoryCacheBackend( + $this->cacheFactory, + $this->timeFactory + ); + } + + public function testGetAttemptsWithNoAttemptsBefore() { + $this->cache + ->expects($this->once()) + ->method('get') + ->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b') + ->willReturn(null); + + $this->assertSame(0, $this->memoryCache->getAttempts('Method', 'User')); + } + + public function testGetAttempts() { + $this->timeFactory + ->expects($this->once()) + ->method('getTime') + ->willReturn(210); + $this->cache + ->expects($this->once()) + ->method('get') + ->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b') + ->willReturn(json_encode([ + '1', + '2', + '87', + '123', + '123', + '124', + ])); + + $this->assertSame(3, $this->memoryCache->getAttempts('Method', 'User')); + } + + public function testRegisterAttemptWithNoAttemptsBefore() { + $this->timeFactory + ->expects($this->once()) + ->method('getTime') + ->willReturn(123); + + $this->cache + ->expects($this->once()) + ->method('get') + ->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b') + ->willReturn(null); + $this->cache + ->expects($this->once()) + ->method('set') + ->with( + 'eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b', + json_encode(['123']) + ); + + $this->memoryCache->registerAttempt('Method', 'User', 100); + } + + public function testRegisterAttempt() { + $this->timeFactory + ->expects($this->once()) + ->method('getTime') + ->willReturn(129); + + $this->cache + ->expects($this->once()) + ->method('get') + ->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b') + ->willReturn(json_encode([ + '1', + '2', + '87', + '123', + '123', + '124', + ])); + $this->cache + ->expects($this->once()) + ->method('set') + ->with( + 'eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b', + json_encode([ + '87', + '123', + '123', + '124', + '129', + ]) + ); + + $this->memoryCache->registerAttempt('Method', 'User', 100); + } +} diff --git a/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php b/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php deleted file mode 100644 index ff58bd5c09e..00000000000 --- a/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php +++ /dev/null @@ -1,149 +0,0 @@ - - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * - */ - -namespace Test\Security\RateLimiting\Backend; - -use OC\Security\RateLimiting\Backend\MemoryCache; -use OCP\AppFramework\Utility\ITimeFactory; -use OCP\ICache; -use OCP\ICacheFactory; -use Test\TestCase; - -class MemoryCacheTest extends TestCase { - /** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */ - private $cacheFactory; - /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ - private $timeFactory; - /** @var ICache|\PHPUnit\Framework\MockObject\MockObject */ - private $cache; - /** @var MemoryCache */ - private $memoryCache; - - protected function setUp(): void { - parent::setUp(); - - $this->cacheFactory = $this->createMock(ICacheFactory::class); - $this->timeFactory = $this->createMock(ITimeFactory::class); - $this->cache = $this->createMock(ICache::class); - - $this->cacheFactory - ->expects($this->once()) - ->method('createDistributed') - ->with('OC\Security\RateLimiting\Backend\MemoryCache') - ->willReturn($this->cache); - - $this->memoryCache = new MemoryCache( - $this->cacheFactory, - $this->timeFactory - ); - } - - public function testGetAttemptsWithNoAttemptsBefore() { - $this->cache - ->expects($this->once()) - ->method('get') - ->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b') - ->willReturn(null); - - $this->assertSame(0, $this->memoryCache->getAttempts('Method', 'User', 123)); - } - - public function testGetAttempts() { - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(210); - $this->cache - ->expects($this->once()) - ->method('get') - ->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b') - ->willReturn(json_encode([ - '1', - '2', - '87', - '123', - '123', - '124', - ])); - - $this->assertSame(3, $this->memoryCache->getAttempts('Method', 'User', 123)); - } - - public function testRegisterAttemptWithNoAttemptsBefore() { - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(123); - - $this->cache - ->expects($this->once()) - ->method('get') - ->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b') - ->willReturn(null); - $this->cache - ->expects($this->once()) - ->method('set') - ->with( - 'eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b', - json_encode(['123']) - ); - - $this->memoryCache->registerAttempt('Method', 'User', 100); - } - - public function testRegisterAttempt() { - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(129); - - $this->cache - ->expects($this->once()) - ->method('get') - ->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b') - ->willReturn(json_encode([ - '1', - '2', - '87', - '123', - '123', - '124', - ])); - $this->cache - ->expects($this->once()) - ->method('set') - ->with( - 'eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b', - json_encode([ - '87', - '123', - '123', - '124', - '129', - ]) - ); - - $this->memoryCache->registerAttempt('Method', 'User', 100); - } -} diff --git a/tests/lib/Security/RateLimiting/LimiterTest.php b/tests/lib/Security/RateLimiting/LimiterTest.php index 8b3509a4790..31c877cbda3 100644 --- a/tests/lib/Security/RateLimiting/LimiterTest.php +++ b/tests/lib/Security/RateLimiting/LimiterTest.php @@ -26,13 +26,10 @@ namespace Test\Security\RateLimiting; use OC\Security\RateLimiting\Backend\IBackend; use OC\Security\RateLimiting\Limiter; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUser; use Test\TestCase; class LimiterTest extends TestCase { - /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ - private $timeFactory; /** @var IBackend|\PHPUnit\Framework\MockObject\MockObject */ private $backend; /** @var Limiter */ @@ -41,11 +38,9 @@ class LimiterTest extends TestCase { protected function setUp(): void { parent::setUp(); - $this->timeFactory = $this->createMock(ITimeFactory::class); $this->backend = $this->createMock(IBackend::class); $this->limiter = new Limiter( - $this->timeFactory, $this->backend ); } @@ -69,10 +64,6 @@ class LimiterTest extends TestCase { } public function testRegisterAnonRequestSuccess() { - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(2000); $this->backend ->expects($this->once()) ->method('getAttempts') @@ -126,10 +117,6 @@ class LimiterTest extends TestCase { ->method('getUID') ->willReturn('MyUid'); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(2000); $this->backend ->expects($this->once()) ->method('getAttempts') -- cgit v1.2.3 From a915372c56724815e3e0a63c8c77e60e7170b325 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 6 Sep 2021 17:50:23 +0200 Subject: phpcs Signed-off-by: Lukas Reschke --- core/Migrations/Version23000Date20210906132259.php | 3 +-- lib/private/Security/RateLimiting/Limiter.php | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) (limited to 'core') diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php index c4e0c42075c..37935201918 100644 --- a/core/Migrations/Version23000Date20210906132259.php +++ b/core/Migrations/Version23000Date20210906132259.php @@ -28,8 +28,7 @@ class Version23000Date20210906132259 extends SimpleMigrationStep { $hasTable = $schema->hasTable(self::TABLE_NAME); - if(!$hasTable) - { + if (!$hasTable) { $table = $schema->createTable(self::TABLE_NAME); $table->addColumn('hash', Types::STRING, [ 'notnull' => true, diff --git a/lib/private/Security/RateLimiting/Limiter.php b/lib/private/Security/RateLimiting/Limiter.php index 1c14fce2a55..91657452d99 100644 --- a/lib/private/Security/RateLimiting/Limiter.php +++ b/lib/private/Security/RateLimiting/Limiter.php @@ -29,7 +29,6 @@ namespace OC\Security\RateLimiting; use OC\Security\Normalizer\IpAddress; use OC\Security\RateLimiting\Backend\IBackend; use OC\Security\RateLimiting\Exception\RateLimitExceededException; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUser; class Limiter { -- cgit v1.2.3 From 471167019c7c9071fa815b2c92691f65370a43b9 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 7 Sep 2021 18:03:34 +0200 Subject: Implement PR review feedback Signed-off-by: Lukas Reschke --- core/Migrations/Version23000Date20210906132259.php | 3 --- lib/private/Security/RateLimiting/Backend/DatabaseBackend.php | 5 ----- lib/private/Security/RateLimiting/Backend/IBackend.php | 2 +- 3 files changed, 1 insertion(+), 9 deletions(-) (limited to 'core') diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php index 37935201918..b4568ab069c 100644 --- a/core/Migrations/Version23000Date20210906132259.php +++ b/core/Migrations/Version23000Date20210906132259.php @@ -10,9 +10,6 @@ use OCP\DB\Types; use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; -/** - * Auto-generated migration step: Please modify to your needs! - */ class Version23000Date20210906132259 extends SimpleMigrationStep { private const TABLE_NAME = 'ratelimit_entries'; diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php index 3b7382d747e..2ba6dc73c47 100644 --- a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -29,11 +29,6 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; -/** - * Class DatabaseBackend uses the database for storing rate limiting data. - * - * @package OC\Security\RateLimiting\Backend - */ class DatabaseBackend implements IBackend { private const TABLE_NAME = 'ratelimit_entries'; diff --git a/lib/private/Security/RateLimiting/Backend/IBackend.php b/lib/private/Security/RateLimiting/Backend/IBackend.php index 835a97993da..960bfd2d159 100644 --- a/lib/private/Security/RateLimiting/Backend/IBackend.php +++ b/lib/private/Security/RateLimiting/Backend/IBackend.php @@ -35,7 +35,7 @@ namespace OC\Security\RateLimiting\Backend; */ interface IBackend { /** - * Gets the amount of attempts for the specified method + * Gets the number of attempts for the specified method * * @param string $methodIdentifier Identifier for the method * @param string $userIdentifier Identifier for the user -- cgit v1.2.3 From 358eaba7dd45e5c5dbce44011bd7eadd88fe8534 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 13 Sep 2021 10:43:01 +0200 Subject: Apply suggestions from code review Signed-off-by: Lukas Reschke Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com> --- core/Migrations/Version23000Date20210906132259.php | 5 +++-- .../Security/RateLimiting/Backend/DatabaseBackend.php | 12 +++++------- 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'core') diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php index b4568ab069c..e65846f40b9 100644 --- a/core/Migrations/Version23000Date20210906132259.php +++ b/core/Migrations/Version23000Date20210906132259.php @@ -31,13 +31,14 @@ class Version23000Date20210906132259 extends SimpleMigrationStep { 'notnull' => true, 'length' => 128, ]); - $table->addColumn('delete_after', 'datetime', [ + $table->addColumn('delete_after', Types::DATETIME, [ 'notnull' => true, ]); $table->addIndex(['hash'], 'ratelimit_hash_idx'); $table->addIndex(['delete_after'], 'ratelimit_delete_after_idx'); + return $schema; } - return $schema; + return null; } } diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php index 2ba6dc73c47..5f579bfef85 100644 --- a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -73,21 +73,19 @@ class DatabaseBackend implements IBackend { $qb = $this->dbConnection->getQueryBuilder(); $qb->delete(self::TABLE_NAME) ->where( - $qb->expr()->lte('delete_after', $qb->createParameter('currentTime')) - ) - ->setParameter('currentTime', $currentTime, 'datetime') + $qb->expr()->lte('delete_after', $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE)) + ); ->executeStatement(); $qb = $this->dbConnection->getQueryBuilder(); - $qb->selectAlias($qb->createFunction('COUNT(*)'), 'count') + $qb->select($qb->func()->count()) ->from(self::TABLE_NAME) ->where( $qb->expr()->eq('hash', $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR)) ) ->andWhere( - $qb->expr()->gte('delete_after', $qb->createParameter('currentTime')) - ) - ->setParameter('currentTime', $currentTime, 'datetime'); + $qb->expr()->gte('delete_after', $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE)) + ); $cursor = $qb->executeQuery(); $row = $cursor->fetch(); -- cgit v1.2.3 From 474a5b55d39e621c12947b50ea6c8febfceed7d7 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 13 Sep 2021 11:01:35 +0200 Subject: Implement review feedback Signed-off-by: Lukas Reschke --- core/Migrations/Version23000Date20210906132259.php | 4 ++-- lib/private/Security/RateLimiting/Backend/DatabaseBackend.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'core') diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php index e65846f40b9..26d18edc8f1 100644 --- a/core/Migrations/Version23000Date20210906132259.php +++ b/core/Migrations/Version23000Date20210906132259.php @@ -34,8 +34,8 @@ class Version23000Date20210906132259 extends SimpleMigrationStep { $table->addColumn('delete_after', Types::DATETIME, [ 'notnull' => true, ]); - $table->addIndex(['hash'], 'ratelimit_hash_idx'); - $table->addIndex(['delete_after'], 'ratelimit_delete_after_idx'); + $table->addIndex(['hash'], 'ratelimit_hash'); + $table->addIndex(['delete_after'], 'ratelimit_delete_after'); return $schema; } diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php index 5f579bfef85..1415b5c4131 100644 --- a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -74,7 +74,7 @@ class DatabaseBackend implements IBackend { $qb->delete(self::TABLE_NAME) ->where( $qb->expr()->lte('delete_after', $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE)) - ); + ) ->executeStatement(); $qb = $this->dbConnection->getQueryBuilder(); @@ -88,10 +88,10 @@ class DatabaseBackend implements IBackend { ); $cursor = $qb->executeQuery(); - $row = $cursor->fetch(); + $row = $cursor->fetchOne(); $cursor->closeCursor(); - return (int)$row['count']; + return (int)$row; } /** -- cgit v1.2.3