Skip to content

Commit c111afe

Browse files
committed
fix(dav): Rate limit address book creation
Signed-off-by: Hamza Mahjoubi <hamzamahjoubi221@gmail.com>
1 parent 6fc2c47 commit c111afe

6 files changed

Lines changed: 278 additions & 0 deletions

File tree

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;
@@ -109,6 +110,7 @@
109110
\OC::$server->get(LoggerInterface::class)
110111
)));
111112
$server->addPlugin(new ExceptionLoggerPlugin('carddav', \OC::$server->get(LoggerInterface::class)));
113+
$server->addPlugin(\OCP\Server::get(CardDavRateLimitingPlugin::class));
112114

113115
// And off we go!
114116
$server->exec();

apps/dav/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@
132132
'OCA\\DAV\\CardDAV\\MultiGetExportPlugin' => $baseDir . '/../lib/CardDAV/MultiGetExportPlugin.php',
133133
'OCA\\DAV\\CardDAV\\PhotoCache' => $baseDir . '/../lib/CardDAV/PhotoCache.php',
134134
'OCA\\DAV\\CardDAV\\Plugin' => $baseDir . '/../lib/CardDAV/Plugin.php',
135+
'OCA\\DAV\\CardDAV\\Security\\CardDavRateLimitingPlugin' => $baseDir . '/../lib/CardDAV/Security/CardDavRateLimitingPlugin.php',
135136
'OCA\\DAV\\CardDAV\\Sharing\\Backend' => $baseDir . '/../lib/CardDAV/Sharing/Backend.php',
136137
'OCA\\DAV\\CardDAV\\Sharing\\Service' => $baseDir . '/../lib/CardDAV/Sharing/Service.php',
137138
'OCA\\DAV\\CardDAV\\SyncService' => $baseDir . '/../lib/CardDAV/SyncService.php',

apps/dav/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ class ComposerStaticInitDAV
153153
'OCA\\DAV\\CardDAV\\SystemAddressbook' => __DIR__ . '/..' . '/../lib/CardDAV/SystemAddressbook.php',
154154
'OCA\\DAV\\CardDAV\\UserAddressBooks' => __DIR__ . '/..' . '/../lib/CardDAV/UserAddressBooks.php',
155155
'OCA\\DAV\\CardDAV\\Xml\\Groups' => __DIR__ . '/..' . '/../lib/CardDAV/Xml/Groups.php',
156+
'OCA\\DAV\\CardDAV\\Security\\CardDavRateLimitingPlugin' => __DIR__ . '/..' . '/../lib/CardDAV/Security/CardDavRateLimitingPlugin.php',
156157
'OCA\\DAV\\Command\\CreateAddressBook' => __DIR__ . '/..' . '/../lib/Command/CreateAddressBook.php',
157158
'OCA\\DAV\\Command\\CreateCalendar' => __DIR__ . '/..' . '/../lib/Command/CreateCalendar.php',
158159
'OCA\\DAV\\Command\\DeleteCalendar' => __DIR__ . '/..' . '/../lib/Command/DeleteCalendar.php',
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* @copyright 2023 Christoph Wurst <christoph@winzerhof-wurst.at>
7+
*
8+
* @author 2023 Christoph Wurst <christoph@winzerhof-wurst.at>
9+
*
10+
* @license GNU AGPL version 3 or any later version
11+
*
12+
* This program is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License as
14+
* published by the Free Software Foundation, either version 3 of the
15+
* License, or (at your option) any later version.
16+
*
17+
* This program is distributed in the hope that it will be useful,
18+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
19+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+
* GNU Affero General Public License for more details.
21+
*
22+
* You should have received a copy of the GNU Affero General Public License
23+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
24+
*/
25+
26+
namespace OCA\DAV\CardDAV\Security;
27+
28+
use OC\Security\RateLimiting\Exception\RateLimitExceededException;
29+
use OC\Security\RateLimiting\Limiter;
30+
use OCA\DAV\CardDAV\CardDavBackend;
31+
use OCA\DAV\Connector\Sabre\Exception\TooManyRequests;
32+
use OCP\IAppConfig;
33+
use OCP\IUserManager;
34+
use Psr\Log\LoggerInterface;
35+
use Sabre\DAV;
36+
use Sabre\DAV\Exception\Forbidden;
37+
use Sabre\DAV\ServerPlugin;
38+
use function count;
39+
use function explode;
40+
41+
class CardDavRateLimitingPlugin extends ServerPlugin {
42+
43+
private Limiter $limiter;
44+
private IUserManager $userManager;
45+
private CardDavBackend $cardDavBackend;
46+
private IAppConfig $config;
47+
private LoggerInterface $logger;
48+
private ?string $userId;
49+
50+
public function __construct(Limiter $limiter,
51+
IUserManager $userManager,
52+
CardDavBackend $cardDavBackend,
53+
LoggerInterface $logger,
54+
IAppConfig $config,
55+
?string $userId) {
56+
$this->limiter = $limiter;
57+
$this->userManager = $userManager;
58+
$this->cardDavBackend = $cardDavBackend;
59+
$this->config = $config;
60+
$this->logger = $logger;
61+
$this->userId = $userId;
62+
}
63+
64+
public function initialize(DAV\Server $server): void {
65+
$server->on('beforeBind', [$this, 'beforeBind'], 1);
66+
}
67+
68+
public function beforeBind(string $path): void {
69+
if ($this->userId === null) {
70+
// We only care about authenticated users here
71+
return;
72+
}
73+
$user = $this->userManager->get($this->userId);
74+
if ($user === null) {
75+
// We only care about authenticated users here
76+
return;
77+
}
78+
79+
$pathParts = explode('/', $path);
80+
if (count($pathParts) === 4 && $pathParts[0] === 'addressbooks') {
81+
// Path looks like addressbooks/users/username/addressbooksname so a new addressbook is created
82+
try {
83+
$this->limiter->registerUserRequest(
84+
'carddav-create-address-book',
85+
$this->config->getValueInt('dav', 'rateLimitAddressBookCreation', 10),
86+
$this->config->getValueInt('dav', 'rateLimitPeriodAddressBookCreation', 3600),
87+
$user
88+
);
89+
} catch (RateLimitExceededException $e) {
90+
throw new TooManyRequests('Too many calendars created', 0, $e);
91+
}
92+
93+
$addressBookLimit = $this->config->getValueInt('dav', 'maximumAdressbooks', 10);
94+
if ($addressBookLimit === -1) {
95+
return;
96+
}
97+
$numAddressbooks = $this->cardDavBackend->getAddressBooksForUserCount('principals/users/' . $user->getUID());
98+
99+
if ($numAddressbooks >= $addressBookLimit) {
100+
$this->logger->warning('Maximum number of address books reached', [
101+
'addressbooks' => $numAddressbooks,
102+
'addressBookLimit' => $addressBookLimit,
103+
]);
104+
throw new Forbidden('AddressBook limit reached', 0);
105+
}
106+
}
107+
}
108+
109+
}

apps/dav/lib/Server.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
use OCA\DAV\CardDAV\ImageExportPlugin;
4646
use OCA\DAV\CardDAV\MultiGetExportPlugin;
4747
use OCA\DAV\CardDAV\PhotoCache;
48+
use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin;
4849
use OCA\DAV\Comments\CommentsPlugin;
4950
use OCA\DAV\Connector\Sabre\AnonymousOptionsPlugin;
5051
use OCA\DAV\Connector\Sabre\Auth;
@@ -208,6 +209,8 @@ public function __construct(IRequest $request, string $baseUri) {
208209
\OC::$server->getAppDataDir('dav-photocache'),
209210
$logger)
210211
));
212+
213+
$this->server->addPlugin(\OCP\Server::get(CardDavRateLimitingPlugin::class));
211214
}
212215

213216
// system tags plugins
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* @copyright 2023 Christoph Wurst <christoph@winzerhof-wurst.at>
7+
*
8+
* @author 2023 Christoph Wurst <christoph@winzerhof-wurst.at>
9+
*
10+
* @license GNU AGPL version 3 or any later version
11+
*
12+
* This program is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License as
14+
* published by the Free Software Foundation, either version 3 of the
15+
* License, or (at your option) any later version.
16+
*
17+
* This program is distributed in the hope that it will be useful,
18+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
19+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+
* GNU Affero General Public License for more details.
21+
*
22+
* You should have received a copy of the GNU Affero General Public License
23+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
24+
*/
25+
26+
namespace OCA\DAV\Tests\unit\CardDAV\Security;
27+
28+
use OC\Security\RateLimiting\Exception\RateLimitExceededException;
29+
use OC\Security\RateLimiting\Limiter;
30+
use OCA\DAV\CardDAV\CardDavBackend;
31+
use OCA\DAV\CardDAV\Security\CardDavRateLimitingPlugin;
32+
use OCA\DAV\Connector\Sabre\Exception\TooManyRequests;
33+
use OCP\IAppConfig;
34+
use OCP\IUser;
35+
use OCP\IUserManager;
36+
use PHPUnit\Framework\MockObject\MockObject;
37+
use Psr\Log\LoggerInterface;
38+
use Sabre\DAV\Exception\Forbidden;
39+
use Test\TestCase;
40+
41+
class CardDavRateLimitingPluginTest extends TestCase {
42+
43+
private Limiter|MockObject $limiter;
44+
private CardDavBackend|MockObject $cardDavBackend;
45+
private IUserManager|MockObject $userManager;
46+
private LoggerInterface|MockObject $logger;
47+
private IAppConfig|MockObject $config;
48+
private string $userId = 'user123';
49+
private CardDavRateLimitingPlugin $plugin;
50+
51+
protected function setUp(): void {
52+
parent::setUp();
53+
54+
$this->limiter = $this->createMock(Limiter::class);
55+
$this->userManager = $this->createMock(IUserManager::class);
56+
$this->cardDavBackend = $this->createMock(CardDavBackend::class);
57+
$this->logger = $this->createMock(LoggerInterface::class);
58+
$this->config = $this->createMock(IAppConfig::class);
59+
$this->plugin = new CardDavRateLimitingPlugin(
60+
$this->limiter,
61+
$this->userManager,
62+
$this->cardDavBackend,
63+
$this->logger,
64+
$this->config,
65+
$this->userId,
66+
);
67+
}
68+
69+
public function testNoUserObject(): void {
70+
$this->limiter->expects(self::never())
71+
->method('registerUserRequest');
72+
73+
$this->plugin->beforeBind('addressbooks/users/foo/addressbookname');
74+
}
75+
76+
public function testUnrelated(): void {
77+
$user = $this->createMock(IUser::class);
78+
$this->userManager->expects(self::once())
79+
->method('get')
80+
->with($this->userId)
81+
->willReturn($user);
82+
$this->limiter->expects(self::never())
83+
->method('registerUserRequest');
84+
85+
$this->plugin->beforeBind('foo/bar');
86+
}
87+
88+
public function testRegisterAddressBookrCreation(): void {
89+
$user = $this->createMock(IUser::class);
90+
$this->userManager->expects(self::once())
91+
->method('get')
92+
->with($this->userId)
93+
->willReturn($user);
94+
$this->config
95+
->method('getValueInt')
96+
->with('dav')
97+
->willReturnArgument(2);
98+
$this->limiter->expects(self::once())
99+
->method('registerUserRequest')
100+
->with(
101+
'carddav-create-address-book',
102+
10,
103+
3600,
104+
$user,
105+
);
106+
107+
$this->plugin->beforeBind('addressbooks/users/foo/addressbookname');
108+
}
109+
110+
public function testAddressBookCreationRateLimitExceeded(): void {
111+
$user = $this->createMock(IUser::class);
112+
$this->userManager->expects(self::once())
113+
->method('get')
114+
->with($this->userId)
115+
->willReturn($user);
116+
$this->config
117+
->method('getValueInt')
118+
->with('dav')
119+
->willReturnArgument(2);
120+
$this->limiter->expects(self::once())
121+
->method('registerUserRequest')
122+
->with(
123+
'carddav-create-address-book',
124+
10,
125+
3600,
126+
$user,
127+
)
128+
->willThrowException(new RateLimitExceededException());
129+
$this->expectException(TooManyRequests::class);
130+
131+
$this->plugin->beforeBind('addressbooks/users/foo/addressbookname');
132+
}
133+
134+
public function testAddressBookLimitReached(): void {
135+
$user = $this->createMock(IUser::class);
136+
$this->userManager->expects(self::once())
137+
->method('get')
138+
->with($this->userId)
139+
->willReturn($user);
140+
$user->method('getUID')->willReturn('user123');
141+
$this->config
142+
->method('getValueInt')
143+
->with('dav')
144+
->willReturnArgument(2);
145+
$this->limiter->expects(self::once())
146+
->method('registerUserRequest')
147+
->with(
148+
'carddav-create-address-book',
149+
10,
150+
3600,
151+
$user,
152+
);
153+
$this->cardDavBackend->expects(self::once())
154+
->method('getAddressBooksForUserCount')
155+
->with('principals/users/user123')
156+
->willReturn(11);
157+
$this->expectException(Forbidden::class);
158+
159+
$this->plugin->beforeBind('addressbooks/users/foo/addressbookname');
160+
}
161+
162+
}

0 commit comments

Comments
 (0)