Merge pull request #39254 from nextcloud/backport/38046/stable24

[stable24] fix(dav): Abort requests with 429 instead of waiting
This commit is contained in:
Joas Schilling 2023-07-11 06:43:10 +02:00 committed by GitHub
commit e65b577b4f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 73 additions and 18 deletions

View file

@ -20,7 +20,7 @@ jobs:
uses: shivammathur/setup-php@master
with:
php-version: 7.4
extensions: ctype,curl,dom,fileinfo,gd,intl,json,mbstring,openssl,pdo_sqlite,posix,sqlite,xml,zip
extensions: apcu,ctype,curl,dom,fileinfo,ftp,gd,intl,json,ldap,mbstring,openssl,pdo_sqlite,posix,sqlite,xml,zip
coverage: none
- name: Composer install
run: composer i
@ -49,7 +49,7 @@ jobs:
uses: shivammathur/setup-php@master
with:
php-version: 7.4
extensions: ctype,curl,dom,fileinfo,gd,intl,json,mbstring,openssl,pdo_sqlite,posix,sqlite,xml,zip
extensions: apcu,ctype,curl,dom,fileinfo,ftp,gd,intl,json,ldap,mbstring,openssl,pdo_sqlite,posix,sqlite,xml,zip
coverage: none
- name: Composer install
run: composer i

View file

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

View file

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

View file

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

View file

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

View file

@ -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]);
@ -458,15 +458,7 @@ class Session implements IUserSession, Emitter {
}
$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;
}
}
@ -488,7 +480,7 @@ class Session implements IUserSession, Emitter {
$this->dispatcher->dispatchTyped(new OC\Authentication\Events\LoginFailed($user));
if ($currentDelay === 0) {
$throttler->sleepDelay($remoteAddress, 'login');
$throttler->sleepDelayOrThrowOnMax($remoteAddress, 'login');
}
}

View file

@ -360,7 +360,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())
@ -425,7 +425,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())
@ -470,7 +470,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())
@ -1083,7 +1083,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())
@ -1133,7 +1133,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())