diff options
author | John Molakvoæ <skjnldsv@users.noreply.github.com> | 2024-06-07 13:42:33 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-06-07 13:42:33 +0200 |
commit | aac7d33865774e2ccb97d5bacbcfb75a65aff4ca (patch) | |
tree | 8cce0021dbeacd7908f4a6f93eb44ac9020a7ee0 /apps | |
parent | 4c7285623cbc5efb3e81d6700a138717e59e9a5c (diff) | |
parent | aafab6987b00408af2d8e97597f14564a220a5a9 (diff) | |
download | nextcloud-server-aac7d33865774e2ccb97d5bacbcfb75a65aff4ca.tar.gz nextcloud-server-aac7d33865774e2ccb97d5bacbcfb75a65aff4ca.zip |
Merge pull request #45542 from nextcloud/backport/44664/stable28
[stable28] fix(dav): Rate limit address book creation
Diffstat (limited to 'apps')
-rw-r--r-- | apps/dav/appinfo/v1/carddav.php | 2 | ||||
-rw-r--r-- | apps/dav/composer/composer/autoload_classmap.php | 1 | ||||
-rw-r--r-- | apps/dav/composer/composer/autoload_static.php | 1 | ||||
-rw-r--r-- | apps/dav/lib/CardDAV/Security/CardDavRateLimitingPlugin.php | 87 | ||||
-rw-r--r-- | apps/dav/lib/Server.php | 3 | ||||
-rw-r--r-- | apps/dav/tests/unit/CardDAV/Security/CardDavRateLimitingPluginTest.php | 146 |
6 files changed, 240 insertions, 0 deletions
diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php index e7faa9314e2..f11281a00a8 100644 --- a/apps/dav/appinfo/v1/carddav.php +++ b/apps/dav/appinfo/v1/carddav.php @@ -32,6 +32,7 @@ use OC\KnownUser\KnownUserService; use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\CardDAV\AddressBookRoot; use OCA\DAV\CardDAV\CardDavBackend; +use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin; use OCA\DAV\Connector\LegacyDAVACL; use OCA\DAV\Connector\Sabre\Auth; use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin; @@ -103,6 +104,7 @@ $server->addPlugin(new \OCA\DAV\CardDAV\ImageExportPlugin(new \OCA\DAV\CardDAV\P \OC::$server->get(LoggerInterface::class) ))); $server->addPlugin(new ExceptionLoggerPlugin('carddav', \OC::$server->get(LoggerInterface::class))); +$server->addPlugin(\OCP\Server::get(CardDavRateLimitingPlugin::class)); // And off we go! $server->exec(); diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 630dbb32d43..e4695c93f26 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -133,6 +133,7 @@ return array( 'OCA\\DAV\\CardDAV\\MultiGetExportPlugin' => $baseDir . '/../lib/CardDAV/MultiGetExportPlugin.php', 'OCA\\DAV\\CardDAV\\PhotoCache' => $baseDir . '/../lib/CardDAV/PhotoCache.php', 'OCA\\DAV\\CardDAV\\Plugin' => $baseDir . '/../lib/CardDAV/Plugin.php', + 'OCA\\DAV\\CardDAV\\Security\\CardDavRateLimitingPlugin' => $baseDir . '/../lib/CardDAV/Security/CardDavRateLimitingPlugin.php', 'OCA\\DAV\\CardDAV\\SyncService' => $baseDir . '/../lib/CardDAV/SyncService.php', 'OCA\\DAV\\CardDAV\\SystemAddressbook' => $baseDir . '/../lib/CardDAV/SystemAddressbook.php', 'OCA\\DAV\\CardDAV\\UserAddressBooks' => $baseDir . '/../lib/CardDAV/UserAddressBooks.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 627bd3d3e0e..3fa6cbf585e 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -148,6 +148,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\CardDAV\\MultiGetExportPlugin' => __DIR__ . '/..' . '/../lib/CardDAV/MultiGetExportPlugin.php', 'OCA\\DAV\\CardDAV\\PhotoCache' => __DIR__ . '/..' . '/../lib/CardDAV/PhotoCache.php', 'OCA\\DAV\\CardDAV\\Plugin' => __DIR__ . '/..' . '/../lib/CardDAV/Plugin.php', + 'OCA\\DAV\\CardDAV\\Security\\CardDavRateLimitingPlugin' => __DIR__ . '/..' . '/../lib/CardDAV/Security/CardDavRateLimitingPlugin.php', 'OCA\\DAV\\CardDAV\\SyncService' => __DIR__ . '/..' . '/../lib/CardDAV/SyncService.php', 'OCA\\DAV\\CardDAV\\SystemAddressbook' => __DIR__ . '/..' . '/../lib/CardDAV/SystemAddressbook.php', 'OCA\\DAV\\CardDAV\\UserAddressBooks' => __DIR__ . '/..' . '/../lib/CardDAV/UserAddressBooks.php', diff --git a/apps/dav/lib/CardDAV/Security/CardDavRateLimitingPlugin.php b/apps/dav/lib/CardDAV/Security/CardDavRateLimitingPlugin.php new file mode 100644 index 00000000000..672b5ea310f --- /dev/null +++ b/apps/dav/lib/CardDAV/Security/CardDavRateLimitingPlugin.php @@ -0,0 +1,87 @@ +<?php + +declare(strict_types=1); + +/* + * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +namespace OCA\DAV\CardDAV\Security; + +use OC\Security\RateLimiting\Exception\RateLimitExceededException; +use OC\Security\RateLimiting\Limiter; +use OCA\DAV\CardDAV\CardDavBackend; +use OCA\DAV\Connector\Sabre\Exception\TooManyRequests; +use OCP\IConfig; +use OCP\IUserManager; +use Psr\Log\LoggerInterface; +use Sabre\DAV; +use Sabre\DAV\Exception\Forbidden; +use Sabre\DAV\ServerPlugin; +use function count; +use function explode; + +class CardDavRateLimitingPlugin extends ServerPlugin { + private ?string $userId; + + public function __construct(private Limiter $limiter, + private IUserManager $userManager, + private CardDavBackend $cardDavBackend, + private LoggerInterface $logger, + private IConfig $config, + ?string $userId) { + $this->limiter = $limiter; + $this->userManager = $userManager; + $this->cardDavBackend = $cardDavBackend; + $this->config = $config; + $this->logger = $logger; + $this->userId = $userId; + } + + public function initialize(DAV\Server $server): void { + $server->on('beforeBind', [$this, 'beforeBind'], 1); + } + + public function beforeBind(string $path): void { + if ($this->userId === null) { + // We only care about authenticated users here + return; + } + $user = $this->userManager->get($this->userId); + if ($user === null) { + // We only care about authenticated users here + return; + } + + $pathParts = explode('/', $path); + if (count($pathParts) === 4 && $pathParts[0] === 'addressbooks') { + // Path looks like addressbooks/users/username/addressbooksname so a new addressbook is created + try { + $this->limiter->registerUserRequest( + 'carddav-create-address-book', + (int) $this->config->getAppValue('dav', 'rateLimitAddressBookCreation', '10'), + (int) $this->config->getAppValue('dav', 'rateLimitPeriodAddressBookCreation', '3600'), + $user + ); + } catch (RateLimitExceededException $e) { + throw new TooManyRequests('Too many addressbooks created', 0, $e); + } + + $addressBookLimit = (int) $this->config->getAppValue('dav', 'maximumAdressbooks', '10'); + if ($addressBookLimit === -1) { + return; + } + $numAddressbooks = $this->cardDavBackend->getAddressBooksForUserCount('principals/users/' . $user->getUID()); + + if ($numAddressbooks >= $addressBookLimit) { + $this->logger->warning('Maximum number of address books reached', [ + 'addressbooks' => $numAddressbooks, + 'addressBookLimit' => $addressBookLimit, + ]); + throw new Forbidden('AddressBook limit reached', 0); + } + } + } + +} diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index e26b5e97683..e5309b79f03 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -44,6 +44,7 @@ use OCA\DAV\CardDAV\HasPhotoPlugin; use OCA\DAV\CardDAV\ImageExportPlugin; use OCA\DAV\CardDAV\MultiGetExportPlugin; use OCA\DAV\CardDAV\PhotoCache; +use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin; use OCA\DAV\Comments\CommentsPlugin; use OCA\DAV\Connector\Sabre\AnonymousOptionsPlugin; use OCA\DAV\Connector\Sabre\Auth; @@ -210,6 +211,8 @@ class Server { \OC::$server->getAppDataDir('dav-photocache'), $logger) )); + + $this->server->addPlugin(\OCP\Server::get(CardDavRateLimitingPlugin::class)); } // system tags plugins diff --git a/apps/dav/tests/unit/CardDAV/Security/CardDavRateLimitingPluginTest.php b/apps/dav/tests/unit/CardDAV/Security/CardDavRateLimitingPluginTest.php new file mode 100644 index 00000000000..f2c592e6744 --- /dev/null +++ b/apps/dav/tests/unit/CardDAV/Security/CardDavRateLimitingPluginTest.php @@ -0,0 +1,146 @@ +<?php + +declare(strict_types=1); + +/* + * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +namespace OCA\DAV\Tests\unit\CardDAV\Security; + +use OC\Security\RateLimiting\Exception\RateLimitExceededException; +use OC\Security\RateLimiting\Limiter; +use OCA\DAV\CardDAV\CardDavBackend; +use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin; +use OCA\DAV\Connector\Sabre\Exception\TooManyRequests; +use OCP\IConfig; +use OCP\IUser; +use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; +use Sabre\DAV\Exception\Forbidden; +use Test\TestCase; + +class CardDavRateLimitingPluginTest extends TestCase { + + private Limiter|MockObject $limiter; + private CardDavBackend|MockObject $cardDavBackend; + private IUserManager|MockObject $userManager; + private LoggerInterface|MockObject $logger; + private IConfig|MockObject $config; + private string $userId = 'user123'; + private CardDavRateLimitingPlugin $plugin; + + protected function setUp(): void { + parent::setUp(); + + $this->limiter = $this->createMock(Limiter::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->cardDavBackend = $this->createMock(CardDavBackend::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->config = $this->createMock(IConfig::class); + $this->plugin = new CardDavRateLimitingPlugin( + $this->limiter, + $this->userManager, + $this->cardDavBackend, + $this->logger, + $this->config, + $this->userId, + ); + } + + public function testNoUserObject(): void { + $this->limiter->expects(self::never()) + ->method('registerUserRequest'); + + $this->plugin->beforeBind('addressbooks/users/foo/addressbookname'); + } + + public function testUnrelated(): void { + $user = $this->createMock(IUser::class); + $this->userManager->expects(self::once()) + ->method('get') + ->with($this->userId) + ->willReturn($user); + $this->limiter->expects(self::never()) + ->method('registerUserRequest'); + + $this->plugin->beforeBind('foo/bar'); + } + + public function testRegisterAddressBookrCreation(): void { + $user = $this->createMock(IUser::class); + $this->userManager->expects(self::once()) + ->method('get') + ->with($this->userId) + ->willReturn($user); + $this->config + ->method('getAppValue') + ->with('dav') + ->willReturnArgument(2); + $this->limiter->expects(self::once()) + ->method('registerUserRequest') + ->with( + 'carddav-create-address-book', + 10, + 3600, + $user, + ); + + $this->plugin->beforeBind('addressbooks/users/foo/addressbookname'); + } + + public function testAddressBookCreationRateLimitExceeded(): void { + $user = $this->createMock(IUser::class); + $this->userManager->expects(self::once()) + ->method('get') + ->with($this->userId) + ->willReturn($user); + $this->config + ->method('getAppValue') + ->with('dav') + ->willReturnArgument(2); + $this->limiter->expects(self::once()) + ->method('registerUserRequest') + ->with( + 'carddav-create-address-book', + 10, + 3600, + $user, + ) + ->willThrowException(new RateLimitExceededException()); + $this->expectException(TooManyRequests::class); + + $this->plugin->beforeBind('addressbooks/users/foo/addressbookname'); + } + + public function testAddressBookLimitReached(): void { + $user = $this->createMock(IUser::class); + $this->userManager->expects(self::once()) + ->method('get') + ->with($this->userId) + ->willReturn($user); + $user->method('getUID')->willReturn('user123'); + $this->config + ->method('getAppValue') + ->with('dav') + ->willReturnArgument(2); + $this->limiter->expects(self::once()) + ->method('registerUserRequest') + ->with( + 'carddav-create-address-book', + 10, + 3600, + $user, + ); + $this->cardDavBackend->expects(self::once()) + ->method('getAddressBooksForUserCount') + ->with('principals/users/user123') + ->willReturn(11); + $this->expectException(Forbidden::class); + + $this->plugin->beforeBind('addressbooks/users/foo/addressbookname'); + } + +} |