Merge pull request #38046 from nextcloud/bugfix/noid/abort-dav-requests-with-429

fix(dav): Also throw in basic auth requests

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',

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',

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

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

@@ -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');
 		}
 	}
 

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())