Skip to content

Commit

Permalink
Merge pull request #46116 from nextcloud/backport/45858/stable24
Browse files Browse the repository at this point in the history
[stable24] fix(dav): Rate limit address book creation
  • Loading branch information
nickvergessen authored Jun 27, 2024
2 parents d1ac8fd + 1d915ba commit afa2bbb
Show file tree
Hide file tree
Showing 6 changed files with 273 additions and 0 deletions.
1 change: 1 addition & 0 deletions apps/dav/appinfo/v1/carddav.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
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;
Expand Down
1 change: 1 addition & 0 deletions apps/dav/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@
'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',
Expand Down
1 change: 1 addition & 0 deletions apps/dav/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,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',
Expand Down
108 changes: 108 additions & 0 deletions apps/dav/lib/CardDAV/Security/CardDavRateLimitingPlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<?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;

/** @var Limiter */
private $limiter;
/** @var IUserManager */
private $userManager;
/** @var CardDavBackend */
private $cardDavBackend;
/** @var LoggerInterface */
private $logger;
/** @var IConfig */
private $config;

/**
* CardDavRateLimitingPlugin constructor.
*
* @param Limiter $limiter
* @param IUserManager $userManager
* @param CardDavBackend $cardDavBackend
* @param LoggerInterface $logger
* @param IConfig $config
* @param string|null $userId
*/
public function __construct(Limiter $limiter,
IUserManager $userManager,
CardDavBackend $cardDavBackend,
LoggerInterface $logger,
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);
}
}
}

}
3 changes: 3 additions & 0 deletions apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
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;
Expand Down Expand Up @@ -201,6 +202,8 @@ public function __construct(IRequest $request, string $baseUri) {
\OC::$server->getAppDataDir('dav-photocache'),
\OC::$server->getLogger())
));

$this->server->addPlugin(\OC::$server->get(CardDavRateLimitingPlugin::class));
}

// system tags plugins
Expand Down
159 changes: 159 additions & 0 deletions apps/dav/tests/unit/CardDAV/Security/CardDavRateLimitingPluginTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
<?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 {

/** @var Limiter|MockObject */
private $limiter;

/** @var CardDavBackend|MockObject */
private $cardDavBackend;

/** @var IUserManager|MockObject */
private $userManager;

/** @var LoggerInterface|MockObject */
private $logger;

/** @var IConfig|MockObject */
private $config;

/** @var string */
private $userId = 'user123';

/** @var CardDavRateLimitingPlugin */
private $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');
}

}

0 comments on commit afa2bbb

Please sign in to comment.