diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php
index 4e27a3f574229..b5b0c8ec728f9 100644
--- a/apps/dav/composer/composer/autoload_classmap.php
+++ b/apps/dav/composer/composer/autoload_classmap.php
@@ -264,6 +264,7 @@
'OCA\\DAV\\DAV\\GroupPrincipalBackend' => $baseDir . '/../lib/DAV/GroupPrincipalBackend.php',
'OCA\\DAV\\DAV\\PublicAuth' => $baseDir . '/../lib/DAV/PublicAuth.php',
'OCA\\DAV\\DAV\\RemoteUserPrincipalBackend' => $baseDir . '/../lib/DAV/RemoteUserPrincipalBackend.php',
+ 'OCA\\DAV\\DAV\\Security\\RateLimiting' => $baseDir . '/../lib/DAV/Security/RateLimiting.php',
'OCA\\DAV\\DAV\\Sharing\\Backend' => $baseDir . '/../lib/DAV/Sharing/Backend.php',
'OCA\\DAV\\DAV\\Sharing\\IShareable' => $baseDir . '/../lib/DAV/Sharing/IShareable.php',
'OCA\\DAV\\DAV\\Sharing\\Plugin' => $baseDir . '/../lib/DAV/Sharing/Plugin.php',
diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php
index 2f57fe9b48b73..2237d54610ba2 100644
--- a/apps/dav/composer/composer/autoload_static.php
+++ b/apps/dav/composer/composer/autoload_static.php
@@ -279,6 +279,7 @@ class ComposerStaticInitDAV
'OCA\\DAV\\DAV\\GroupPrincipalBackend' => __DIR__ . '/..' . '/../lib/DAV/GroupPrincipalBackend.php',
'OCA\\DAV\\DAV\\PublicAuth' => __DIR__ . '/..' . '/../lib/DAV/PublicAuth.php',
'OCA\\DAV\\DAV\\RemoteUserPrincipalBackend' => __DIR__ . '/..' . '/../lib/DAV/RemoteUserPrincipalBackend.php',
+ 'OCA\\DAV\\DAV\\Security\\RateLimiting' => __DIR__ . '/..' . '/../lib/DAV/Security/RateLimiting.php',
'OCA\\DAV\\DAV\\Sharing\\Backend' => __DIR__ . '/..' . '/../lib/DAV/Sharing/Backend.php',
'OCA\\DAV\\DAV\\Sharing\\IShareable' => __DIR__ . '/..' . '/../lib/DAV/Sharing/IShareable.php',
'OCA\\DAV\\DAV\\Sharing\\Plugin' => __DIR__ . '/..' . '/../lib/DAV/Sharing/Plugin.php',
diff --git a/apps/dav/lib/DAV/Security/RateLimiting.php b/apps/dav/lib/DAV/Security/RateLimiting.php
new file mode 100644
index 0000000000000..cdee6dbd37fd8
--- /dev/null
+++ b/apps/dav/lib/DAV/Security/RateLimiting.php
@@ -0,0 +1,46 @@
+userSession->getUser();
+ if ($user === null) {
+ return;
+ }
+
+ $identifier = 'share-addressbook-or-calendar';
+ $userLimit = $this->config->getValueInt('dav', 'rateLimitShareAddressbookOrCalendar', 20);
+ $userPeriod = $this->config->getValueInt('dav', 'rateLimitPeriodShareAddressbookOrCalendar', 3600);
+
+ try {
+ $this->limiter->registerUserRequest($identifier, $userLimit, $userPeriod, $user);
+ } catch (IRateLimitExceededException $e) {
+ throw new TooManyRequests('Too many addressbook or calendar share requests', 0, $e);
+ }
+ }
+}
diff --git a/apps/dav/lib/DAV/Sharing/Plugin.php b/apps/dav/lib/DAV/Sharing/Plugin.php
index 82b000bc8ce61..9c5c002b42b53 100644
--- a/apps/dav/lib/DAV/Sharing/Plugin.php
+++ b/apps/dav/lib/DAV/Sharing/Plugin.php
@@ -10,11 +10,13 @@
use OCA\DAV\CalDAV\CalDavBackend;
use OCA\DAV\CalDAV\CalendarHome;
use OCA\DAV\Connector\Sabre\Auth;
+use OCA\DAV\DAV\Security\RateLimiting;
use OCA\DAV\DAV\Sharing\Xml\Invite;
use OCA\DAV\DAV\Sharing\Xml\ShareRequest;
use OCP\AppFramework\Http;
use OCP\IConfig;
use OCP\IRequest;
+use Sabre\DAV\Exception\BadRequest;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\ICollection;
use Sabre\DAV\INode;
@@ -28,17 +30,11 @@ class Plugin extends ServerPlugin {
public const NS_OWNCLOUD = 'http://owncloud.org/ns';
public const NS_NEXTCLOUD = 'http://nextcloud.com/ns';
- /**
- * Plugin constructor.
- *
- * @param Auth $auth
- * @param IRequest $request
- * @param IConfig $config
- */
public function __construct(
private Auth $auth,
private IRequest $request,
private IConfig $config,
+ private RateLimiting $rateLimiting,
) {
}
@@ -136,6 +132,9 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
// calendar.
case '{' . self::NS_OWNCLOUD . '}share':
+ $this->rateLimiting->check();
+ $this->validateShareRequest($message);
+
// We can only deal with IShareableCalendar objects
if (!$node instanceof IShareable) {
return;
@@ -170,6 +169,23 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
}
}
+ private function validateShareRequest($shareRequest): void {
+ if (!$shareRequest instanceof ShareRequest) {
+ // @FIXME: Replace switch-case in httpPost with instanceof ShareRequest
+ throw new BadRequest('The given request is not valid');
+ }
+
+ $elements = (count($shareRequest->set) + count($shareRequest->remove));
+
+ if ($elements === 0) {
+ throw new BadRequest(ShareRequest::ELEMENT_SHARE . ' needs at least one set or remove element');
+ }
+
+ if ($elements > 10) {
+ throw new BadRequest(ShareRequest::ELEMENT_SHARE . ' is limited to 10 set or remove elements');
+ }
+ }
+
private function preloadCollection(PropFind $propFind, ICollection $collection): void {
if (!$collection instanceof CalendarHome || $propFind->getDepth() !== 1) {
return;
diff --git a/apps/dav/lib/DAV/Sharing/Xml/ShareRequest.php b/apps/dav/lib/DAV/Sharing/Xml/ShareRequest.php
index aefb39c5701de..fad8e52283895 100644
--- a/apps/dav/lib/DAV/Sharing/Xml/ShareRequest.php
+++ b/apps/dav/lib/DAV/Sharing/Xml/ShareRequest.php
@@ -12,6 +12,8 @@
use Sabre\Xml\XmlDeserializable;
class ShareRequest implements XmlDeserializable {
+ public const ELEMENT_SHARE = '{' . Plugin::NS_OWNCLOUD . '}share';
+
/**
* Constructor
*
@@ -33,6 +35,10 @@ public static function xmlDeserialize(Reader $reader) {
$set = [];
$remove = [];
+ if ($elements === null) {
+ return new self($set, $remove);
+ }
+
foreach ($elements as $elem) {
switch ($elem['name']) {
diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php
index 2cf2f575596fc..51c0b3b6ccdd3 100644
--- a/apps/dav/lib/Server.php
+++ b/apps/dav/lib/Server.php
@@ -55,6 +55,7 @@
use OCA\DAV\Connector\Sabre\ZipFolderPlugin;
use OCA\DAV\DAV\CustomPropertiesBackend;
use OCA\DAV\DAV\PublicAuth;
+use OCA\DAV\DAV\Security\RateLimiting;
use OCA\DAV\DAV\ViewOnlyPlugin;
use OCA\DAV\Db\PropertyMapper;
use OCA\DAV\Events\SabrePluginAddEvent;
@@ -198,7 +199,7 @@ public function __construct(
// calendar plugins
if ($this->requestIsForSubtree(['calendars', 'public-calendars', 'system-calendars', 'principals'])) {
- $this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OCP\Server::get(IRequest::class), \OCP\Server::get(IConfig::class)));
+ $this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OCP\Server::get(IRequest::class), \OCP\Server::get(IConfig::class), \OCP\Server::get(RateLimiting::class)));
$this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin());
$this->server->addPlugin(new ICSExportPlugin(\OCP\Server::get(IConfig::class), $logger));
$this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin(\OCP\Server::get(IConfig::class), \OCP\Server::get(LoggerInterface::class), \OCP\Server::get(DefaultCalendarValidator::class)));
@@ -221,7 +222,7 @@ public function __construct(
// addressbook plugins
if ($this->requestIsForSubtree(['addressbooks', 'principals'])) {
- $this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OCP\Server::get(IRequest::class), \OCP\Server::get(IConfig::class)));
+ $this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OCP\Server::get(IRequest::class), \OCP\Server::get(IConfig::class), \OCP\Server::get(RateLimiting::class)));
$this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin());
$this->server->addPlugin(new VCFExportPlugin());
$this->server->addPlugin(new MultiGetExportPlugin());
diff --git a/apps/dav/tests/unit/CardDAV/Sharing/PluginTest.php b/apps/dav/tests/unit/CardDAV/Sharing/PluginTest.php
deleted file mode 100644
index 1e934a69a5385..0000000000000
--- a/apps/dav/tests/unit/CardDAV/Sharing/PluginTest.php
+++ /dev/null
@@ -1,62 +0,0 @@
-createMock(Auth::class);
- $authBackend->method('isDavAuthenticated')
- ->willReturn(true);
- $request = $this->createMock(IRequest::class);
- $config = $this->createMock(IConfig::class);
- $this->plugin = new Plugin($authBackend, $request, $config);
-
- $root = new SimpleCollection('root');
- $this->server = new \Sabre\DAV\Server($root);
- $this->book = $this->createMock(IShareable::class);
- $this->book->method('getName')
- ->willReturn('addressbook1.vcf');
- $root->addChild($this->book);
- $this->plugin->initialize($this->server);
- }
-
- public function testSharing(): void {
- $this->book->expects($this->once())->method('updateShares')->with([[
- 'href' => 'principal:principals/admin',
- 'commonName' => null,
- 'summary' => null,
- 'readOnly' => false
- ]], ['mailto:wilfredo@example.com']);
-
- // setup request
- $request = new Request('POST', 'addressbook1.vcf');
- $request->addHeader('Content-Type', 'application/xml');
- $request->setBody('principal:principals/admin mailto:wilfredo@example.com');
- $response = new Response();
- $this->plugin->httpPost($request, $response);
- }
-}
diff --git a/apps/dav/tests/unit/DAV/Security/RateLimitingTest.php b/apps/dav/tests/unit/DAV/Security/RateLimitingTest.php
new file mode 100644
index 0000000000000..69b39957319bb
--- /dev/null
+++ b/apps/dav/tests/unit/DAV/Security/RateLimitingTest.php
@@ -0,0 +1,99 @@
+userSession = $this->createMock(IUserSession::class);
+ $this->config = $this->createMock(IAppConfig::class);
+ $this->limiter = $this->createMock(ILimiter::class);
+
+ $this->rateLimiting = new RateLimiting(
+ $this->userSession,
+ $this->config,
+ $this->limiter,
+ );
+ }
+
+ public function testNoUserObject(): void {
+ $this->userSession->expects($this->once())
+ ->method('getUser')
+ ->willReturn(null);
+ $this->limiter->expects($this->never())
+ ->method('registerUserRequest');
+
+ $this->rateLimiting->check();
+ }
+
+ public function testRegisterShareRequest(): void {
+ $user = $this->createMock(IUser::class);
+ $this->userSession->expects($this->once())
+ ->method('getUser')
+ ->willReturn($user);
+ $this->config->method('getValueInt')
+ ->willReturnCallback(static function (string $app, string $key, int $default): int {
+ return match ($key) {
+ 'rateLimitShareAddressbookOrCalendar' => 7,
+ 'rateLimitPeriodShareAddressbookOrCalendar' => 600,
+ default => $default,
+ };
+ });
+ $this->limiter->expects($this->once())
+ ->method('registerUserRequest')
+ ->with(
+ 'share-addressbook-or-calendar',
+ 7,
+ 600,
+ $user,
+ );
+
+ $this->rateLimiting->check();
+ }
+
+ public function testShareRequestRateLimitExceeded(): void {
+ $user = $this->createMock(IUser::class);
+ $this->userSession->expects($this->once())
+ ->method('getUser')
+ ->willReturn($user);
+ $this->config->method('getValueInt')
+ ->willReturnArgument(2);
+ $this->limiter->expects($this->once())
+ ->method('registerUserRequest')
+ ->with(
+ 'share-addressbook-or-calendar',
+ 20,
+ 3600,
+ $user,
+ )
+ ->willThrowException($this->createMock(IRateLimitExceededException::class));
+
+ $this->expectException(TooManyRequests::class);
+
+ $this->rateLimiting->check();
+ }
+}
diff --git a/apps/dav/tests/unit/DAV/Sharing/PluginTest.php b/apps/dav/tests/unit/DAV/Sharing/PluginTest.php
index 7a88f7cc5ddde..5f4b9f00b748a 100644
--- a/apps/dav/tests/unit/DAV/Sharing/PluginTest.php
+++ b/apps/dav/tests/unit/DAV/Sharing/PluginTest.php
@@ -9,11 +9,13 @@
namespace OCA\DAV\Tests\unit\DAV\Sharing;
use OCA\DAV\Connector\Sabre\Auth;
+use OCA\DAV\DAV\Security\RateLimiting;
use OCA\DAV\DAV\Sharing\IShareable;
use OCA\DAV\DAV\Sharing\Plugin;
use OCP\IConfig;
use OCP\IRequest;
use PHPUnit\Framework\MockObject\MockObject;
+use Sabre\DAV\Exception\BadRequest;
use Sabre\DAV\Server;
use Sabre\DAV\SimpleCollection;
use Sabre\HTTP\Request;
@@ -24,6 +26,7 @@ class PluginTest extends TestCase {
private Plugin $plugin;
private Server $server;
private IShareable&MockObject $book;
+ private RateLimiting&MockObject $rateLimiting;
protected function setUp(): void {
parent::setUp();
@@ -33,11 +36,11 @@ protected function setUp(): void {
$request = $this->createMock(IRequest::class);
$config = $this->createMock(IConfig::class);
- $this->plugin = new Plugin($authBackend, $request, $config);
+ $this->rateLimiting = $this->createMock(RateLimiting::class);
+ $this->plugin = new Plugin($authBackend, $request, $config, $this->rateLimiting);
$root = new SimpleCollection('root');
- $this->server = new \Sabre\DAV\Server($root);
- /** @var SimpleCollection $node */
+ $this->server = new Server($root);
$this->book = $this->createMock(IShareable::class);
$this->book->method('getName')->willReturn('addressbook1.vcf');
$root->addChild($this->book);
@@ -45,18 +48,64 @@ protected function setUp(): void {
}
public function testSharing(): void {
- $this->book->expects($this->once())->method('updateShares')->with([[
- 'href' => 'principal:principals/admin',
- 'commonName' => null,
- 'summary' => null,
- 'readOnly' => false
- ]], ['mailto:wilfredo@example.com']);
-
- // setup request
+ $this->rateLimiting->expects(self::once())
+ ->method('check');
+ $this->book->expects(self::once())
+ ->method('updateShares')
+ ->with([[
+ 'href' => 'principal:principals/admin',
+ 'commonName' => null,
+ 'summary' => null,
+ 'readOnly' => false,
+ ]], ['mailto:wilfredo@example.com']);
+ $body = 'principal:principals/admin mailto:wilfredo@example.com';
+
+ $this->executeRequest($body);
+ }
+
+ public function testEmptyShareRequestIsRejected(): void {
+ $this->rateLimiting->expects(self::once())
+ ->method('check');
+ $this->book->expects(self::never())
+ ->method('updateShares');
+ $this->expectException(BadRequest::class);
+ $this->expectExceptionMessage('{http://owncloud.org/ns}share needs at least one set or remove element');
+ $body = '';
+
+ $this->executeRequest($body);
+ }
+
+ public function testShareRequestWithTooManyElementsIsRejected(): void {
+ $this->rateLimiting->expects(self::once())
+ ->method('check');
+ $this->book->expects(self::never())
+ ->method('updateShares');
+ $this->expectException(BadRequest::class);
+ $this->expectExceptionMessage('{http://owncloud.org/ns}share is limited to 10 set or remove elements');
+ $body = ''
+ . 'principal:principals/user1'
+ . 'principal:principals/user2'
+ . 'principal:principals/user3'
+ . 'principal:principals/user4'
+ . 'principal:principals/user5'
+ . 'principal:principals/user6'
+ . 'principal:principals/user7'
+ . 'principal:principals/user8'
+ . 'principal:principals/user9'
+ . 'principal:principals/user10'
+ . 'principal:principals/user11'
+ . 'principal:principals/user12'
+ . '';
+
+ $this->executeRequest($body);
+ }
+
+ private function executeRequest(string $body): void {
$request = new Request('POST', 'addressbook1.vcf');
$request->addHeader('Content-Type', 'application/xml');
- $request->setBody('principal:principals/admin mailto:wilfredo@example.com');
+ $request->setBody($body);
$response = new Response();
+
$this->plugin->httpPost($request, $response);
}
}