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

apps/dav/composer/composer/autoload_classmap.php

@@ -157,6 +157,7 @@
     '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',

apps/dav/composer/composer/autoload_static.php

@@ -172,6 +172,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',

apps/dav/lib/Connector/Sabre/Auth.php

@@ -37,10 +37,13 @@
 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 Sabre\DAV\Auth\Backend\AbstractBasic;
 use Sabre\DAV\Exception\NotAuthenticated;
 use Sabre\DAV\Exception\ServiceUnavailable;
@@ -138,6 +141,9 @@ protected function validateUserPass($username, $password) {
 			} catch (PasswordLoginForbiddenException $ex) {
 				$this->session->close();
 				throw new PasswordLoginForbidden();
+			} catch (MaxDelayReached $ex) {
+				$this->session->close();
+				throw new TooManyRequests();
 			}
 		}
 	}

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);
+	}
+}

lib/private/User/Session.php

@@ -438,7 +438,7 @@ public function logClientIn($user,
 								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]);
@@ -467,15 +467,7 @@ public function logClientIn($user,
 			}
 			$users = $this->manager->getByEmail($user);
 			if (!(\count($users) === 1 && $this->login($users[0]->getUID(), $password))) {
-				$this->logger->warning('Login failed: \'' . $user . '\' (Remote IP: \'' . \OC::$server->getRequest()->getRemoteAddress() . '\')', ['app' => 'core']);
-
-				$throttler->registerAttempt('login', $request->getRemoteAddress(), ['user' => $user]);
-
-				$this->dispatcher->dispatchTyped(new OC\Authentication\Events\LoginFailed($user));
-
-				if ($currentDelay === 0) {
-					$throttler->sleepDelay($request->getRemoteAddress(), 'login');
-				}
+				$this->handleLoginFailed($throttler, $currentDelay, $remoteAddress, $user, $password);
 				return false;
 			}
 		}
@@ -497,7 +489,7 @@ private function handleLoginFailed(Throttler $throttler, int $currentDelay, stri
 		$this->dispatcher->dispatchTyped(new OC\Authentication\Events\LoginFailed($user));
 
 		if ($currentDelay === 0) {
-			$throttler->sleepDelay($remoteAddress, 'login');
+			$throttler->sleepDelayOrThrowOnMax($remoteAddress, 'login');
 		}
 	}
 

tests/lib/User/SessionTest.php

@@ -448,7 +448,7 @@ public function testLogClientInNoTokenPasswordWith2fa() {
 			->willReturn('192.168.0.1');
 		$this->throttler
 			->expects($this->once())
-			->method('sleepDelay')
+			->method('sleepDelayOrThrowOnMax')
 			->with('192.168.0.1');
 		$this->throttler
 			->expects($this->any())
@@ -513,7 +513,7 @@ public function testLogClientInWithTokenPassword() {
 			->willReturn('192.168.0.1');
 		$this->throttler
 			->expects($this->once())
-			->method('sleepDelay')
+			->method('sleepDelayOrThrowOnMax')
 			->with('192.168.0.1');
 		$this->throttler
 			->expects($this->any())
@@ -558,7 +558,7 @@ public function testLogClientInNoTokenPasswordNo2fa() {
 			->willReturn('192.168.0.1');
 		$this->throttler
 			->expects($this->once())
-			->method('sleepDelay')
+			->method('sleepDelayOrThrowOnMax')
 			->with('192.168.0.1');
 		$this->throttler
 			->expects($this->any())
@@ -1273,7 +1273,7 @@ public function testUpdateAuthTokenLastCheck() {
 			->willReturn('192.168.0.1');
 		$this->throttler
 			->expects($this->once())
-			->method('sleepDelay')
+			->method('sleepDelayOrThrowOnMax')
 			->with('192.168.0.1')
 			->willReturn(5);
 		$this->timeFactory
@@ -1323,7 +1323,7 @@ public function testNoUpdateAuthTokenLastCheckRecent() {
 			->willReturn('192.168.0.1');
 		$this->throttler
 			->expects($this->once())
-			->method('sleepDelay')
+			->method('sleepDelayOrThrowOnMax')
 			->with('192.168.0.1')
 			->willReturn(5);
 		$this->timeFactory
@@ -1500,7 +1500,7 @@ public function testLogClientInThrottlerUsername() {
 			->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())
@@ -1550,7 +1550,7 @@ public function testLogClientInThrottlerEmail() {
 			->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())