From b91957e3df058b5d22b9d2bb7c0e464749e4e22d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 30 Mar 2023 15:02:51 +0200 Subject: [PATCH] fix(dav): Abort requests with 429 instead of waiting Signed-off-by: Joas Schilling --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + apps/dav/lib/Connector/Sabre/Auth.php | 6 ++ .../Sabre/Exception/TooManyRequests.php | 55 +++++++++++++++++++ lib/private/User/Session.php | 4 +- tests/lib/User/SessionTest.php | 10 ++-- 6 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 apps/dav/lib/Connector/Sabre/Exception/TooManyRequests.php diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index db7de8c9ac6..ab7d3e71928 100644 --- 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 c29d93d72a8..e0e1f86bdbb 100644 --- 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 69821c63c21..69e3946bb11 100644 --- 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 index 00000000000..1110797aed4 --- /dev/null +++ b/apps/dav/lib/Connector/Sabre/Exception/TooManyRequests.php @@ -0,0 +1,55 @@ + + * + * @author Joas Schilling + * + * @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 . + * + */ + +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 078539e5489..6273945ec43 100644 --- 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 e0afb5330d9..2af48d8330c 100644 --- 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()) -- 2.39.5