diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index e99463c75efc4..47b12a6f6c31d 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -151,6 +151,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', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 01a5df8c1b8b0..21684f83885e5 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -166,6 +166,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 30a27f672dd7c..77298a68c7241 100644 --- a/apps/dav/lib/Connector/Sabre/Auth.php +++ b/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(); } } } 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 0000000000000..1110797aed4f3 --- /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 7aea219b6089e..d993903d73928 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -47,6 +47,7 @@ use OC\Authentication\Token\IToken; use OC\Hooks\Emitter; use OC\Hooks\PublicEmitter; +use OC\Security\Bruteforce\Throttler; use OC_User; use OC_Util; use OCA\DAV\Connector\Sabre\Auth; @@ -428,7 +429,7 @@ public function completeLogin(IUser $user, array $loginDetails, $regenerateSessi * @param string $user * @param string $password * @param IRequest $request - * @param OC\Security\Bruteforce\Throttler $throttler + * @param Throttler $throttler * @throws LoginException * @throws PasswordLoginForbiddenException * @return boolean @@ -436,8 +437,9 @@ public function completeLogin(IUser $user, array $loginDetails, $regenerateSessi public function logClientIn($user, $password, IRequest $request, - OC\Security\Bruteforce\Throttler $throttler) { - $currentDelay = $throttler->sleepDelay($request->getRemoteAddress(), 'login'); + Throttler $throttler) { + $remoteAddress = $request->getRemoteAddress(); + $currentDelay = $throttler->sleepDelayOrThrowOnMax($remoteAddress, 'login'); if ($this->manager instanceof PublicEmitter) { $this->manager->emit('\OC\User', 'preLogin', [$user, $password]); @@ -461,17 +463,13 @@ public function logClientIn($user, if (!$this->login($user, $password)) { // Failed, maybe the user used their email address + if (!filter_var($user, FILTER_VALIDATE_EMAIL)) { + $this->handleLoginFailed($throttler, $currentDelay, $remoteAddress, $user, $password); + return false; + } $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; } } @@ -486,6 +484,17 @@ public function logClientIn($user, return true; } + private function handleLoginFailed(Throttler $throttler, int $currentDelay, string $remoteAddress, string $user, ?string $password) { + $this->logger->warning("Login failed: '" . $user . "' (Remote IP: '" . $remoteAddress . "')", ['app' => 'core']); + + $throttler->registerAttempt('login', $remoteAddress, ['user' => $user]); + $this->dispatcher->dispatchTyped(new OC\Authentication\Events\LoginFailed($user)); + + if ($currentDelay === 0) { + $throttler->sleepDelayOrThrowOnMax($remoteAddress, 'login'); + } + } + protected function supportsCookies(IRequest $request) { if (!is_null($request->getCookie('cookie_test'))) { return true; @@ -574,11 +583,11 @@ protected function prepareUserLogin($firstTimeLogin, $refreshCsrfToken = true) { * * @todo do not allow basic auth if the user is 2FA enforced * @param IRequest $request - * @param OC\Security\Bruteforce\Throttler $throttler + * @param Throttler $throttler * @return boolean if the login was successful */ public function tryBasicAuthLogin(IRequest $request, - OC\Security\Bruteforce\Throttler $throttler) { + Throttler $throttler) { if (!empty($request->server['PHP_AUTH_USER']) && !empty($request->server['PHP_AUTH_PW'])) { try { if ($this->logClientIn($request->server['PHP_AUTH_USER'], $request->server['PHP_AUTH_PW'], $request, $throttler)) { diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 334c3d9065fa1..2e364e9066cd4 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -9,6 +9,7 @@ namespace Test\User; use OC\AppFramework\Http\Request; +use OC\Authentication\Events\LoginFailed; use OC\Authentication\Token\DefaultTokenMapper; use OC\Authentication\Token\DefaultTokenProvider; use OC\Authentication\Token\IProvider; @@ -447,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()) @@ -512,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()) @@ -557,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()) @@ -1272,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 @@ -1322,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 @@ -1472,4 +1473,100 @@ public function testUpdateTokens() { $this->userSession->updateTokens('uid', 'pass'); } + + public function testLogClientInThrottlerUsername() { + $manager = $this->createMock(Manager::class); + $session = $this->createMock(ISession::class); + $request = $this->createMock(IRequest::class); + + /** @var Session $userSession */ + $userSession = $this->getMockBuilder(Session::class) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser']) + ->getMock(); + + $userSession->expects($this->once()) + ->method('isTokenPassword') + ->willReturn(true); + $userSession->expects($this->once()) + ->method('login') + ->with('john', 'I-AM-AN-PASSWORD') + ->willReturn(false); + + $session->expects($this->never()) + ->method('set'); + $request + ->method('getRemoteAddress') + ->willReturn('192.168.0.1'); + $this->throttler + ->expects($this->exactly(2)) + ->method('sleepDelayOrThrowOnMax') + ->with('192.168.0.1'); + $this->throttler + ->expects($this->any()) + ->method('getDelay') + ->with('192.168.0.1') + ->willReturn(0); + + $this->throttler + ->expects($this->once()) + ->method('registerAttempt') + ->with('login', '192.168.0.1', ['user' => 'john']); + $this->dispatcher + ->expects($this->once()) + ->method('dispatchTyped') + ->with(new LoginFailed('john', 'I-AM-AN-PASSWORD')); + + $this->assertFalse($userSession->logClientIn('john', 'I-AM-AN-PASSWORD', $request, $this->throttler)); + } + + public function testLogClientInThrottlerEmail() { + $manager = $this->createMock(Manager::class); + $session = $this->createMock(ISession::class); + $request = $this->createMock(IRequest::class); + + /** @var Session $userSession */ + $userSession = $this->getMockBuilder(Session::class) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser']) + ->getMock(); + + $userSession->expects($this->once()) + ->method('isTokenPassword') + ->willReturn(true); + $userSession->expects($this->once()) + ->method('login') + ->with('john@foo.bar', 'I-AM-AN-PASSWORD') + ->willReturn(false); + $manager + ->method('getByEmail') + ->with('john@foo.bar') + ->willReturn([]); + + $session->expects($this->never()) + ->method('set'); + $request + ->method('getRemoteAddress') + ->willReturn('192.168.0.1'); + $this->throttler + ->expects($this->exactly(2)) + ->method('sleepDelayOrThrowOnMax') + ->with('192.168.0.1'); + $this->throttler + ->expects($this->any()) + ->method('getDelay') + ->with('192.168.0.1') + ->willReturn(0); + + $this->throttler + ->expects($this->once()) + ->method('registerAttempt') + ->with('login', '192.168.0.1', ['user' => 'john@foo.bar']); + $this->dispatcher + ->expects($this->once()) + ->method('dispatchTyped') + ->with(new LoginFailed('john@foo.bar', 'I-AM-AN-PASSWORD')); + + $this->assertFalse($userSession->logClientIn('john@foo.bar', 'I-AM-AN-PASSWORD', $request, $this->throttler)); + } }