Skip to content

Commit b76a3ba

Browse files
kesselbkesselb
committed
fix(dav): Limit share/unshare requests per user
AI-assisted: OpenCode (gpt-5.4) Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
1 parent 6b8c589 commit b76a3ba

7 files changed

Lines changed: 162 additions & 72 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@
268268
'OCA\\DAV\\DAV\\GroupPrincipalBackend' => $baseDir . '/../lib/DAV/GroupPrincipalBackend.php',
269269
'OCA\\DAV\\DAV\\PublicAuth' => $baseDir . '/../lib/DAV/PublicAuth.php',
270270
'OCA\\DAV\\DAV\\RemoteUserPrincipalBackend' => $baseDir . '/../lib/DAV/RemoteUserPrincipalBackend.php',
271+
'OCA\\DAV\\DAV\\Security\\RateLimiting' => $baseDir . '/../lib/DAV/Security/RateLimiting.php',
271272
'OCA\\DAV\\DAV\\Sharing\\Backend' => $baseDir . '/../lib/DAV/Sharing/Backend.php',
272273
'OCA\\DAV\\DAV\\Sharing\\IShareable' => $baseDir . '/../lib/DAV/Sharing/IShareable.php',
273274
'OCA\\DAV\\DAV\\Sharing\\Plugin' => $baseDir . '/../lib/DAV/Sharing/Plugin.php',

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@ class ComposerStaticInitDAV
283283
'OCA\\DAV\\DAV\\GroupPrincipalBackend' => __DIR__ . '/..' . '/../lib/DAV/GroupPrincipalBackend.php',
284284
'OCA\\DAV\\DAV\\PublicAuth' => __DIR__ . '/..' . '/../lib/DAV/PublicAuth.php',
285285
'OCA\\DAV\\DAV\\RemoteUserPrincipalBackend' => __DIR__ . '/..' . '/../lib/DAV/RemoteUserPrincipalBackend.php',
286+
'OCA\\DAV\\DAV\\Security\\RateLimiting' => __DIR__ . '/..' . '/../lib/DAV/Security/RateLimiting.php',
286287
'OCA\\DAV\\DAV\\Sharing\\Backend' => __DIR__ . '/..' . '/../lib/DAV/Sharing/Backend.php',
287288
'OCA\\DAV\\DAV\\Sharing\\IShareable' => __DIR__ . '/..' . '/../lib/DAV/Sharing/IShareable.php',
288289
'OCA\\DAV\\DAV\\Sharing\\Plugin' => __DIR__ . '/..' . '/../lib/DAV/Sharing/Plugin.php',

‎apps/dav/lib/DAV/Security/RateLimiting.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\DAV\Security;
11+
12+
use OCA\DAV\Connector\Sabre\Exception\TooManyRequests;
13+
use OCP\IAppConfig;
14+
use OCP\IUserManager;
15+
use OCP\Security\RateLimiting\ILimiter;
16+
use OCP\Security\RateLimiting\IRateLimitExceededException;
17+
18+
class RateLimiting {
19+
20+
public function __construct(
21+
private readonly ILimiter $limiter,
22+
private readonly IUserManager $userManager,
23+
private readonly IAppConfig $config,
24+
private readonly ?string $userId,
25+
) {
26+
}
27+
28+
/**
29+
* @throws TooManyRequests
30+
*/
31+
public function check(): void {
32+
if ($this->userId === null) {
33+
return;
34+
}
35+
36+
$user = $this->userManager->get($this->userId);
37+
if ($user === null) {
38+
return;
39+
}
40+
41+
$identifier = 'share-addressbook-or-calendar';
42+
$userLimit = $this->config->getValueInt('dav', 'rateLimitShareAddressbookOrCalendar', 20);
43+
$userPeriod = $this->config->getValueInt('dav', 'rateLimitPeriodShareAddressbookOrCalendar', 3600);
44+
45+
try {
46+
$this->limiter->registerUserRequest($identifier, $userLimit, $userPeriod, $user);
47+
} catch (IRateLimitExceededException $e) {
48+
throw new TooManyRequests('Too many addressbook or calendar share requests', 0, $e);
49+
}
50+
}
51+
}

‎apps/dav/lib/DAV/Sharing/Plugin.php‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OCA\DAV\CalDAV\CalDavBackend;
1111
use OCA\DAV\CalDAV\CalendarHome;
1212
use OCA\DAV\Connector\Sabre\Auth;
13+
use OCA\DAV\DAV\Security\RateLimiting;
1314
use OCA\DAV\DAV\Sharing\Xml\Invite;
1415
use OCA\DAV\DAV\Sharing\Xml\ShareRequest;
1516
use OCP\AppFramework\Http;
@@ -29,17 +30,11 @@ class Plugin extends ServerPlugin {
2930
public const NS_OWNCLOUD = 'http://owncloud.org/ns';
3031
public const NS_NEXTCLOUD = 'http://nextcloud.com/ns';
3132

32-
/**
33-
* Plugin constructor.
34-
*
35-
* @param Auth $auth
36-
* @param IRequest $request
37-
* @param IConfig $config
38-
*/
3933
public function __construct(
4034
private Auth $auth,
4135
private IRequest $request,
4236
private IConfig $config,
37+
private RateLimiting $rateLimiting,
4338
) {
4439
}
4540

@@ -137,6 +132,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
137132
// calendar.
138133
case '{' . self::NS_OWNCLOUD . '}share':
139134

135+
$this->rateLimiting->check();
140136
$this->validateShareRequest($message);
141137

142138
// We can only deal with IShareableCalendar objects
@@ -160,7 +156,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
160156
return;
161157
}
162158
}
163-
159+
164160
$node->updateShares($message->set, $message->remove);
165161

166162
$response->setStatus(Http::STATUS_OK);

‎apps/dav/lib/Server.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
use OCA\DAV\Connector\Sabre\ZipFolderPlugin;
5757
use OCA\DAV\DAV\CustomPropertiesBackend;
5858
use OCA\DAV\DAV\PublicAuth;
59+
use OCA\DAV\DAV\Security\RateLimiting;
5960
use OCA\DAV\DAV\ViewOnlyPlugin;
6061
use OCA\DAV\Db\PropertyMapper;
6162
use OCA\DAV\Events\SabrePluginAddEvent;
@@ -200,7 +201,7 @@ public function __construct(
200201

201202
// calendar plugins
202203
if ($this->requestIsForSubtree(['calendars', 'public-calendars', 'system-calendars', 'principals'])) {
203-
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OCP\Server::get(IRequest::class), \OCP\Server::get(IConfig::class)));
204+
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OCP\Server::get(IRequest::class), \OCP\Server::get(IConfig::class), \OCP\Server::get(RateLimiting::class)));
204205
$this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin());
205206
$this->server->addPlugin(new ICSExportPlugin(\OCP\Server::get(IConfig::class), $logger));
206207
$this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin(\OCP\Server::get(IConfig::class), \OCP\Server::get(LoggerInterface::class), \OCP\Server::get(DefaultCalendarValidator::class)));
@@ -223,7 +224,7 @@ public function __construct(
223224

224225
// addressbook plugins
225226
if ($this->requestIsForSubtree(['addressbooks', 'principals'])) {
226-
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OCP\Server::get(IRequest::class), \OCP\Server::get(IConfig::class)));
227+
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OCP\Server::get(IRequest::class), \OCP\Server::get(IConfig::class), \OCP\Server::get(RateLimiting::class)));
227228
$this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin());
228229
$this->server->addPlugin(new VCFExportPlugin());
229230
$this->server->addPlugin(new MultiGetExportPlugin());

‎apps/dav/tests/unit/CardDAV/Sharing/PluginTest.php‎

Lines changed: 0 additions & 62 deletions
This file was deleted.

‎apps/dav/tests/unit/DAV/Security/RateLimitingTest.php‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\Tests\unit\DAV\Security;
11+
12+
use OCA\DAV\Connector\Sabre\Exception\TooManyRequests;
13+
use OCA\DAV\DAV\Security\RateLimiting;
14+
use OCP\IAppConfig;
15+
use OCP\IUser;
16+
use OCP\IUserManager;
17+
use OCP\Security\RateLimiting\ILimiter;
18+
use OCP\Security\RateLimiting\IRateLimitExceededException;
19+
use PHPUnit\Framework\MockObject\MockObject;
20+
use Test\TestCase;
21+
22+
class RateLimitingTest extends TestCase {
23+
private ILimiter&MockObject $limiter;
24+
private IUserManager&MockObject $userManager;
25+
private IAppConfig&MockObject $config;
26+
private RateLimiting $rateLimiting;
27+
private string $userId = 'user123';
28+
29+
protected function setUp(): void {
30+
parent::setUp();
31+
32+
$this->limiter = $this->createMock(ILimiter::class);
33+
$this->userManager = $this->createMock(IUserManager::class);
34+
$this->config = $this->createMock(IAppConfig::class);
35+
$this->rateLimiting = new RateLimiting(
36+
$this->limiter,
37+
$this->userManager,
38+
$this->config,
39+
$this->userId,
40+
);
41+
}
42+
43+
public function testNoUserObject(): void {
44+
$this->userManager->expects(self::once())
45+
->method('get')
46+
->with($this->userId)
47+
->willReturn(null);
48+
$this->limiter->expects(self::never())
49+
->method('registerUserRequest');
50+
51+
$this->rateLimiting->check();
52+
}
53+
54+
public function testRegisterShareRequest(): void {
55+
$user = $this->createMock(IUser::class);
56+
$this->userManager->expects(self::once())
57+
->method('get')
58+
->with($this->userId)
59+
->willReturn($user);
60+
$this->config->method('getValueInt')
61+
->willReturnCallback(static function (string $app, string $key, int $default): int {
62+
return match ($key) {
63+
'rateLimitShareAddressbookOrCalendar' => 7,
64+
'rateLimitPeriodShareAddressbookOrCalendar' => 600,
65+
default => $default,
66+
};
67+
});
68+
$this->limiter->expects(self::once())
69+
->method('registerUserRequest')
70+
->with(
71+
'share-addressbook-or-calendar',
72+
7,
73+
600,
74+
$user,
75+
);
76+
77+
$this->rateLimiting->check();
78+
}
79+
80+
public function testShareRequestRateLimitExceeded(): void {
81+
$user = $this->createMock(IUser::class);
82+
$this->userManager->expects(self::once())
83+
->method('get')
84+
->with($this->userId)
85+
->willReturn($user);
86+
$this->config->method('getValueInt')
87+
->willReturnArgument(2);
88+
$this->limiter->expects(self::once())
89+
->method('registerUserRequest')
90+
->with(
91+
'share-addressbook-or-calendar',
92+
20,
93+
3600,
94+
$user,
95+
)
96+
->willThrowException($this->createMock(IRateLimitExceededException::class));
97+
98+
$this->expectException(TooManyRequests::class);
99+
100+
$this->rateLimiting->check();
101+
}
102+
}

0 commit comments

Comments
 (0)