Skip to content

Commit

Permalink
Merge pull request nextcloud#45545 from nextcloud/backport/44664/stab…
Browse files Browse the repository at this point in the history
…le26
  • Loading branch information
skjnldsv authored Jun 7, 2024
2 parents 7840b4a + 695a1e2 commit afeac8f
Show file tree
Hide file tree
Showing 6 changed files with 240 additions and 0 deletions.
2 changes: 2 additions & 0 deletions apps/dav/appinfo/v1/carddav.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,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 Expand Up @@ -102,6 +103,7 @@
\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();
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 @@ -122,6 +122,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 @@ -137,6 +137,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
87 changes: 87 additions & 0 deletions apps/dav/lib/CardDAV/Security/CardDavRateLimitingPlugin.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
}

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

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

// system tags plugins
Expand Down
146 changes: 146 additions & 0 deletions apps/dav/tests/unit/CardDAV/Security/CardDavRateLimitingPluginTest.php
Original file line number Diff line number Diff line change
@@ -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');
}

}

0 comments on commit afeac8f

Please sign in to comment.