fix(dav): Abort requests with 429 instead of waiting

Signed-off-by: Joas Schilling <coding@schilljs.com>

diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php
index db7de8c9ac6d3ff8326f69633053b9fbe136af1d..ab7d3e719282d09ffd2610f00cd6d1d23ea86994 100644 (file)
--- a/apps/dav/composer/composer/autoload_classmap.php
+++ b/apps/dav/composer/composer/autoload_classmap.php
@@ -165,6 +165,7 @@ return array(
     'OCA\\DAV\\Connector\\Sabre\\Exception\\Forbidden' => $baseDir . '/../lib/Connector/Sabre/Exception/Forbidden.php',
     'OCA\\DAV\\Connector\\Sabre\\Exception\\InvalidPath' => $baseDir . '/../lib/Connector/Sabre/Exception/InvalidPath.php',
     'OCA\\DAV\\Connector\\Sabre\\Exception\\PasswordLoginForbidden' => $baseDir . '/../lib/Connector/Sabre/Exception/PasswordLoginForbidden.php',
+    'OCA\\DAV\\Connector\\Sabre\\Exception\\TooManyRequests' => $baseDir . '/../lib/Connector/Sabre/Exception/TooManyRequests.php',
     'OCA\\DAV\\Connector\\Sabre\\Exception\\UnsupportedMediaType' => $baseDir . '/../lib/Connector/Sabre/Exception/UnsupportedMediaType.php',
     'OCA\\DAV\\Connector\\Sabre\\FakeLockerPlugin' => $baseDir . '/../lib/Connector/Sabre/FakeLockerPlugin.php',
     'OCA\\DAV\\Connector\\Sabre\\File' => $baseDir . '/../lib/Connector/Sabre/File.php',

diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php
index c29d93d72a833086f03006bb15fbc4d9142ef72b..e0e1f86bdbb3eaa66b6854360875e3878d0cb00a 100644 (file)
--- a/apps/dav/composer/composer/autoload_static.php
+++ b/apps/dav/composer/composer/autoload_static.php
@@ -180,6 +180,7 @@ class ComposerStaticInitDAV
         'OCA\\DAV\\Connector\\Sabre\\Exception\\Forbidden' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Exception/Forbidden.php',
         'OCA\\DAV\\Connector\\Sabre\\Exception\\InvalidPath' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Exception/InvalidPath.php',
         'OCA\\DAV\\Connector\\Sabre\\Exception\\PasswordLoginForbidden' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Exception/PasswordLoginForbidden.php',
+        'OCA\\DAV\\Connector\\Sabre\\Exception\\TooManyRequests' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Exception/TooManyRequests.php',
         'OCA\\DAV\\Connector\\Sabre\\Exception\\UnsupportedMediaType' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Exception/UnsupportedMediaType.php',
         'OCA\\DAV\\Connector\\Sabre\\FakeLockerPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/FakeLockerPlugin.php',
         'OCA\\DAV\\Connector\\Sabre\\File' => __DIR__ . '/..' . '/../lib/Connector/Sabre/File.php',

diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php
index 69821c63c211e4a207c99e96bb395317b28dd23a..69e3946bb1152a320a5fe8abefe2a1bfa5a29dac 100644 (file)
--- a/apps/dav/lib/Connector/Sabre/Auth.php
+++ b/apps/dav/lib/Connector/Sabre/Auth.php
@@ -37,10 +37,13 @@ use Exception;
 use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
 use OC\Authentication\TwoFactorAuth\Manager;
 use OC\Security\Bruteforce\Throttler;
+use OC\User\LoginException;
 use OC\User\Session;
 use OCA\DAV\Connector\Sabre\Exception\PasswordLoginForbidden;
+use OCA\DAV\Connector\Sabre\Exception\TooManyRequests;
 use OCP\IRequest;
 use OCP\ISession;
+use OCP\Security\Bruteforce\MaxDelayReached;
 use Psr\Log\LoggerInterface;
 use Sabre\DAV\Auth\Backend\AbstractBasic;
 use Sabre\DAV\Exception\NotAuthenticated;
@@ -119,6 +122,9 @@ class Auth extends AbstractBasic {
                        } catch (PasswordLoginForbiddenException $ex) {
                                $this->session->close();
                                throw new PasswordLoginForbidden();
+                       } catch (MaxDelayReached $ex) {
+                               $this->session->close();
+                               throw new TooManyRequests();
                        }
                }
        }

diff --git a/apps/dav/lib/Connector/Sabre/Exception/TooManyRequests.php b/apps/dav/lib/Connector/Sabre/Exception/TooManyRequests.php
new file mode 100644 (file)
index 0000000..1110797
--- /dev/null
+++ b/apps/dav/lib/Connector/Sabre/Exception/TooManyRequests.php
@@ -0,0 +1,55 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\DAV\Connector\Sabre\Exception;
+
+use DOMElement;
+use Sabre\DAV\Exception\NotAuthenticated;
+use Sabre\DAV\Server;
+
+class TooManyRequests extends NotAuthenticated {
+       public const NS_OWNCLOUD = 'http://owncloud.org/ns';
+
+       public function getHTTPCode() {
+               return 429;
+       }
+
+       /**
+        * This method allows the exception to include additional information
+        * into the WebDAV error response
+        *
+        * @param Server $server
+        * @param DOMElement $errorNode
+        * @return void
+        */
+       public function serialize(Server $server, DOMElement $errorNode) {
+
+               // set ownCloud namespace
+               $errorNode->setAttribute('xmlns:o', self::NS_OWNCLOUD);
+
+               $error = $errorNode->ownerDocument->createElementNS('o:', 'o:hint', 'too many requests');
+               $errorNode->appendChild($error);
+       }
+}

diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php
index 078539e548932488db50d34ed62d4bffda85e130..6273945ec432447613dcaa01140faffdb4226e18 100644 (file)
--- a/lib/private/User/Session.php
+++ b/lib/private/User/Session.php
@@ -428,7 +428,7 @@ class Session implements IUserSession, Emitter {
                                                                IRequest $request,
                                                                OC\Security\Bruteforce\Throttler $throttler) {
                $remoteAddress = $request->getRemoteAddress();
-               $currentDelay = $throttler->sleepDelay($remoteAddress, 'login');
+               $currentDelay = $throttler->sleepDelayOrThrowOnMax($remoteAddress, 'login');
 
                if ($this->manager instanceof PublicEmitter) {
                        $this->manager->emit('\OC\User', 'preLogin', [$user, $password]);
@@ -479,7 +479,7 @@ class Session implements IUserSession, Emitter {
                $this->dispatcher->dispatchTyped(new OC\Authentication\Events\LoginFailed($user, $password));
 
                if ($currentDelay === 0) {
-                       $throttler->sleepDelay($remoteAddress, 'login');
+                       $throttler->sleepDelayOrThrowOnMax($remoteAddress, 'login');
                }
        }
 

diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php
index e0afb5330d9adbec18fd039a35c75f08dc456116..2af48d8330c6ff0e69a2f6d56c5666a7ecace17c 100644 (file)
--- a/tests/lib/User/SessionTest.php
+++ b/tests/lib/User/SessionTest.php
@@ -362,7 +362,7 @@ class SessionTest extends \Test\TestCase {
                        ->willReturn('192.168.0.1');
                $this->throttler
                        ->expects($this->once())
-                       ->method('sleepDelay')
+                       ->method('sleepDelayOrThrowOnMax')
                        ->with('192.168.0.1');
                $this->throttler
                        ->expects($this->any())
@@ -427,7 +427,7 @@ class SessionTest extends \Test\TestCase {
                        ->willReturn('192.168.0.1');
                $this->throttler
                        ->expects($this->once())
-                       ->method('sleepDelay')
+                       ->method('sleepDelayOrThrowOnMax')
                        ->with('192.168.0.1');
                $this->throttler
                        ->expects($this->any())
@@ -472,7 +472,7 @@ class SessionTest extends \Test\TestCase {
                        ->willReturn('192.168.0.1');
                $this->throttler
                        ->expects($this->once())
-                       ->method('sleepDelay')
+                       ->method('sleepDelayOrThrowOnMax')
                        ->with('192.168.0.1');
                $this->throttler
                        ->expects($this->any())
@@ -1085,7 +1085,7 @@ class SessionTest extends \Test\TestCase {
                        ->willReturn('192.168.0.1');
                $this->throttler
                        ->expects($this->exactly(2))
-                       ->method('sleepDelay')
+                       ->method('sleepDelayOrThrowOnMax')
                        ->with('192.168.0.1');
                $this->throttler
                        ->expects($this->any())
@@ -1135,7 +1135,7 @@ class SessionTest extends \Test\TestCase {
                        ->willReturn('192.168.0.1');
                $this->throttler
                        ->expects($this->exactly(2))
-                       ->method('sleepDelay')
+                       ->method('sleepDelayOrThrowOnMax')
                        ->with('192.168.0.1');
                $this->throttler
                        ->expects($this->any())