Skip to content

Commit aac7d33

Browse files
skjnldsvskjnldsv
authored
Merge pull request #45542 from nextcloud/backport/44664/stable28
[stable28] fix(dav): Rate limit address book creation
2 parents 4c72856 + aafab69 commit aac7d33

File tree

6 files changed

+240
-0
lines changed

6 files changed

+240
-0
lines changed

‎apps/dav/appinfo/v1/carddav.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
use OCA\DAV\AppInfo\PluginManager;
3333
use OCA\DAV\CardDAV\AddressBookRoot;
3434
use OCA\DAV\CardDAV\CardDavBackend;
35+
use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin;
3536
use OCA\DAV\Connector\LegacyDAVACL;
3637
use OCA\DAV\Connector\Sabre\Auth;
3738
use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin;
@@ -103,6 +104,7 @@
103104
\OC::$server->get(LoggerInterface::class)
104105
)));
105106
$server->addPlugin(new ExceptionLoggerPlugin('carddav', \OC::$server->get(LoggerInterface::class)));
107+
$server->addPlugin(\OCP\Server::get(CardDavRateLimitingPlugin::class));
106108

107109
// And off we go!
108110
$server->exec();

‎apps/dav/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@
133133
'OCA\\DAV\\CardDAV\\MultiGetExportPlugin' => $baseDir . '/../lib/CardDAV/MultiGetExportPlugin.php',
134134
'OCA\\DAV\\CardDAV\\PhotoCache' => $baseDir . '/../lib/CardDAV/PhotoCache.php',
135135
'OCA\\DAV\\CardDAV\\Plugin' => $baseDir . '/../lib/CardDAV/Plugin.php',
136+
'OCA\\DAV\\CardDAV\\Security\\CardDavRateLimitingPlugin' => $baseDir . '/../lib/CardDAV/Security/CardDavRateLimitingPlugin.php',
136137
'OCA\\DAV\\CardDAV\\SyncService' => $baseDir . '/../lib/CardDAV/SyncService.php',
137138
'OCA\\DAV\\CardDAV\\SystemAddressbook' => $baseDir . '/../lib/CardDAV/SystemAddressbook.php',
138139
'OCA\\DAV\\CardDAV\\UserAddressBooks' => $baseDir . '/../lib/CardDAV/UserAddressBooks.php',

‎apps/dav/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ class ComposerStaticInitDAV
148148
'OCA\\DAV\\CardDAV\\MultiGetExportPlugin' => __DIR__ . '/..' . '/../lib/CardDAV/MultiGetExportPlugin.php',
149149
'OCA\\DAV\\CardDAV\\PhotoCache' => __DIR__ . '/..' . '/../lib/CardDAV/PhotoCache.php',
150150
'OCA\\DAV\\CardDAV\\Plugin' => __DIR__ . '/..' . '/../lib/CardDAV/Plugin.php',
151+
'OCA\\DAV\\CardDAV\\Security\\CardDavRateLimitingPlugin' => __DIR__ . '/..' . '/../lib/CardDAV/Security/CardDavRateLimitingPlugin.php',
151152
'OCA\\DAV\\CardDAV\\SyncService' => __DIR__ . '/..' . '/../lib/CardDAV/SyncService.php',
152153
'OCA\\DAV\\CardDAV\\SystemAddressbook' => __DIR__ . '/..' . '/../lib/CardDAV/SystemAddressbook.php',
153154
'OCA\\DAV\\CardDAV\\UserAddressBooks' => __DIR__ . '/..' . '/../lib/CardDAV/UserAddressBooks.php',

‎apps/dav/lib/CardDAV/Security/CardDavRateLimitingPlugin.php‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\CardDAV\Security;
11+
12+
use OC\Security\RateLimiting\Exception\RateLimitExceededException;
13+
use OC\Security\RateLimiting\Limiter;
14+
use OCA\DAV\CardDAV\CardDavBackend;
15+
use OCA\DAV\Connector\Sabre\Exception\TooManyRequests;
16+
use OCP\IConfig;
17+
use OCP\IUserManager;
18+
use Psr\Log\LoggerInterface;
19+
use Sabre\DAV;
20+
use Sabre\DAV\Exception\Forbidden;
21+
use Sabre\DAV\ServerPlugin;
22+
use function count;
23+
use function explode;
24+
25+
class CardDavRateLimitingPlugin extends ServerPlugin {
26+
private ?string $userId;
27+
28+
public function __construct(private Limiter $limiter,
29+
private IUserManager $userManager,
30+
private CardDavBackend $cardDavBackend,
31+
private LoggerInterface $logger,
32+
private IConfig $config,
33+
?string $userId) {
34+
$this->limiter = $limiter;
35+
$this->userManager = $userManager;
36+
$this->cardDavBackend = $cardDavBackend;
37+
$this->config = $config;
38+
$this->logger = $logger;
39+
$this->userId = $userId;
40+
}
41+
42+
public function initialize(DAV\Server $server): void {
43+
$server->on('beforeBind', [$this, 'beforeBind'], 1);
44+
}
45+
46+
public function beforeBind(string $path): void {
47+
if ($this->userId === null) {
48+
// We only care about authenticated users here
49+
return;
50+
}
51+
$user = $this->userManager->get($this->userId);
52+
if ($user === null) {
53+
// We only care about authenticated users here
54+
return;
55+
}
56+
57+
$pathParts = explode('/', $path);
58+
if (count($pathParts) === 4 && $pathParts[0] === 'addressbooks') {
59+
// Path looks like addressbooks/users/username/addressbooksname so a new addressbook is created
60+
try {
61+
$this->limiter->registerUserRequest(
62+
'carddav-create-address-book',
63+
(int) $this->config->getAppValue('dav', 'rateLimitAddressBookCreation', '10'),
64+
(int) $this->config->getAppValue('dav', 'rateLimitPeriodAddressBookCreation', '3600'),
65+
$user
66+
);
67+
} catch (RateLimitExceededException $e) {
68+
throw new TooManyRequests('Too many addressbooks created', 0, $e);
69+
}
70+
71+
$addressBookLimit = (int) $this->config->getAppValue('dav', 'maximumAdressbooks', '10');
72+
if ($addressBookLimit === -1) {
73+
return;
74+
}
75+
$numAddressbooks = $this->cardDavBackend->getAddressBooksForUserCount('principals/users/' . $user->getUID());
76+
77+
if ($numAddressbooks >= $addressBookLimit) {
78+
$this->logger->warning('Maximum number of address books reached', [
79+
'addressbooks' => $numAddressbooks,
80+
'addressBookLimit' => $addressBookLimit,
81+
]);
82+
throw new Forbidden('AddressBook limit reached', 0);
83+
}
84+
}
85+
}
86+
87+
}

‎apps/dav/lib/Server.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
use OCA\DAV\CardDAV\ImageExportPlugin;
4545
use OCA\DAV\CardDAV\MultiGetExportPlugin;
4646
use OCA\DAV\CardDAV\PhotoCache;
47+
use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin;
4748
use OCA\DAV\Comments\CommentsPlugin;
4849
use OCA\DAV\Connector\Sabre\AnonymousOptionsPlugin;
4950
use OCA\DAV\Connector\Sabre\Auth;
@@ -210,6 +211,8 @@ public function __construct(IRequest $request, string $baseUri) {
210211
\OC::$server->getAppDataDir('dav-photocache'),
211212
$logger)
212213
));
214+
215+
$this->server->addPlugin(\OCP\Server::get(CardDavRateLimitingPlugin::class));
213216
}
214217

215218
// system tags plugins

‎apps/dav/tests/unit/CardDAV/Security/CardDavRateLimitingPluginTest.php‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\Tests\unit\CardDAV\Security;
11+
12+
use OC\Security\RateLimiting\Exception\RateLimitExceededException;
13+
use OC\Security\RateLimiting\Limiter;
14+
use OCA\DAV\CardDAV\CardDavBackend;
15+
use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin;
16+
use OCA\DAV\Connector\Sabre\Exception\TooManyRequests;
17+
use OCP\IConfig;
18+
use OCP\IUser;
19+
use OCP\IUserManager;
20+
use PHPUnit\Framework\MockObject\MockObject;
21+
use Psr\Log\LoggerInterface;
22+
use Sabre\DAV\Exception\Forbidden;
23+
use Test\TestCase;
24+
25+
class CardDavRateLimitingPluginTest extends TestCase {
26+
27+
private Limiter|MockObject $limiter;
28+
private CardDavBackend|MockObject $cardDavBackend;
29+
private IUserManager|MockObject $userManager;
30+
private LoggerInterface|MockObject $logger;
31+
private IConfig|MockObject $config;
32+
private string $userId = 'user123';
33+
private CardDavRateLimitingPlugin $plugin;
34+
35+
protected function setUp(): void {
36+
parent::setUp();
37+
38+
$this->limiter = $this->createMock(Limiter::class);
39+
$this->userManager = $this->createMock(IUserManager::class);
40+
$this->cardDavBackend = $this->createMock(CardDavBackend::class);
41+
$this->logger = $this->createMock(LoggerInterface::class);
42+
$this->config = $this->createMock(IConfig::class);
43+
$this->plugin = new CardDavRateLimitingPlugin(
44+
$this->limiter,
45+
$this->userManager,
46+
$this->cardDavBackend,
47+
$this->logger,
48+
$this->config,
49+
$this->userId,
50+
);
51+
}
52+
53+
public function testNoUserObject(): void {
54+
$this->limiter->expects(self::never())
55+
->method('registerUserRequest');
56+
57+
$this->plugin->beforeBind('addressbooks/users/foo/addressbookname');
58+
}
59+
60+
public function testUnrelated(): void {
61+
$user = $this->createMock(IUser::class);
62+
$this->userManager->expects(self::once())
63+
->method('get')
64+
->with($this->userId)
65+
->willReturn($user);
66+
$this->limiter->expects(self::never())
67+
->method('registerUserRequest');
68+
69+
$this->plugin->beforeBind('foo/bar');
70+
}
71+
72+
public function testRegisterAddressBookrCreation(): void {
73+
$user = $this->createMock(IUser::class);
74+
$this->userManager->expects(self::once())
75+
->method('get')
76+
->with($this->userId)
77+
->willReturn($user);
78+
$this->config
79+
->method('getAppValue')
80+
->with('dav')
81+
->willReturnArgument(2);
82+
$this->limiter->expects(self::once())
83+
->method('registerUserRequest')
84+
->with(
85+
'carddav-create-address-book',
86+
10,
87+
3600,
88+
$user,
89+
);
90+
91+
$this->plugin->beforeBind('addressbooks/users/foo/addressbookname');
92+
}
93+
94+
public function testAddressBookCreationRateLimitExceeded(): void {
95+
$user = $this->createMock(IUser::class);
96+
$this->userManager->expects(self::once())
97+
->method('get')
98+
->with($this->userId)
99+
->willReturn($user);
100+
$this->config
101+
->method('getAppValue')
102+
->with('dav')
103+
->willReturnArgument(2);
104+
$this->limiter->expects(self::once())
105+
->method('registerUserRequest')
106+
->with(
107+
'carddav-create-address-book',
108+
10,
109+
3600,
110+
$user,
111+
)
112+
->willThrowException(new RateLimitExceededException());
113+
$this->expectException(TooManyRequests::class);
114+
115+
$this->plugin->beforeBind('addressbooks/users/foo/addressbookname');
116+
}
117+
118+
public function testAddressBookLimitReached(): void {
119+
$user = $this->createMock(IUser::class);
120+
$this->userManager->expects(self::once())
121+
->method('get')
122+
->with($this->userId)
123+
->willReturn($user);
124+
$user->method('getUID')->willReturn('user123');
125+
$this->config
126+
->method('getAppValue')
127+
->with('dav')
128+
->willReturnArgument(2);
129+
$this->limiter->expects(self::once())
130+
->method('registerUserRequest')
131+
->with(
132+
'carddav-create-address-book',
133+
10,
134+
3600,
135+
$user,
136+
);
137+
$this->cardDavBackend->expects(self::once())
138+
->method('getAddressBooksForUserCount')
139+
->with('principals/users/user123')
140+
->willReturn(11);
141+
$this->expectException(Forbidden::class);
142+
143+
$this->plugin->beforeBind('addressbooks/users/foo/addressbookname');
144+
}
145+
146+
}

0 commit comments

Comments
 (0)