diff options
author | Joas Schilling <coding@schilljs.com> | 2025-01-17 15:01:07 +0100 |
---|---|---|
committer | Joas Schilling <coding@schilljs.com> | 2025-01-27 12:46:15 +0100 |
commit | c1655bcde75a125fde9a92877907a10ed25297a3 (patch) | |
tree | a73a8bc0952a468d98dcadbe7455145caf335f59 /tests | |
parent | f5cd0cbd7007e42635914f3acc69d5e85b0b5907 (diff) | |
download | nextcloud-server-c1655bcde75a125fde9a92877907a10ed25297a3.tar.gz nextcloud-server-c1655bcde75a125fde9a92877907a10ed25297a3.zip |
fix(ratelimit): Allow to bypass rate-limit from bruteforce allowlistbugfix/noid/allow-ratelimit-bypass
Signed-off-by: Joas Schilling <coding@schilljs.com>
Diffstat (limited to 'tests')
3 files changed, 171 insertions, 213 deletions
diff --git a/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php index fddca471215..c42baadcb1c 100644 --- a/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php @@ -11,6 +11,7 @@ namespace Test\AppFramework\Middleware\Security; use OC\AppFramework\Middleware\Security\RateLimitingMiddleware; use OC\AppFramework\Utility\ControllerMethodReflector; +use OC\Security\Ip\BruteforceAllowList; use OC\Security\RateLimiting\Exception\RateLimitExceededException; use OC\Security\RateLimiting\Limiter; use OCP\AppFramework\Controller; @@ -18,6 +19,7 @@ use OCP\AppFramework\Http\Attribute\AnonRateLimit; use OCP\AppFramework\Http\Attribute\UserRateLimit; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\IAppConfig; use OCP\IRequest; use OCP\ISession; use OCP\IUser; @@ -61,6 +63,8 @@ class RateLimitingMiddlewareTest extends TestCase { private ControllerMethodReflector $reflector; private Limiter|MockObject $limiter; private ISession|MockObject $session; + private IAppConfig|MockObject $appConfig; + private BruteforceAllowList|MockObject $bruteForceAllowList; private RateLimitingMiddleware $rateLimitingMiddleware; protected function setUp(): void { @@ -71,13 +75,17 @@ class RateLimitingMiddlewareTest extends TestCase { $this->reflector = new ControllerMethodReflector(); $this->limiter = $this->createMock(Limiter::class); $this->session = $this->createMock(ISession::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->bruteForceAllowList = $this->createMock(BruteforceAllowList::class); $this->rateLimitingMiddleware = new RateLimitingMiddleware( $this->request, $this->userSession, $this->reflector, $this->limiter, - $this->session + $this->session, + $this->appConfig, + $this->bruteForceAllowList, ); } diff --git a/tests/lib/Security/Bruteforce/ThrottlerTest.php b/tests/lib/Security/Bruteforce/ThrottlerTest.php deleted file mode 100644 index a94f1849c8a..00000000000 --- a/tests/lib/Security/Bruteforce/ThrottlerTest.php +++ /dev/null @@ -1,212 +0,0 @@ -<?php - -declare(strict_types=1); - -/** - * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -namespace Test\Security\Bruteforce; - -use OC\Security\Bruteforce\Backend\DatabaseBackend; -use OC\Security\Bruteforce\Throttler; -use OCP\AppFramework\Utility\ITimeFactory; -use OCP\IConfig; -use OCP\IDBConnection; -use Psr\Log\LoggerInterface; -use Test\TestCase; - -/** - * Based on the unit tests from Paragonie's Airship CMS - * Ref: https://github.com/paragonie/airship/blob/7e5bad7e3c0fbbf324c11f963fd1f80e59762606/test/unit/Engine/Security/AirBrakeTest.php - * - * @package Test\Security\Bruteforce - */ -class ThrottlerTest extends TestCase { - /** @var Throttler */ - private $throttler; - /** @var IDBConnection */ - private $dbConnection; - /** @var ITimeFactory */ - private $timeFactory; - /** @var LoggerInterface */ - private $logger; - /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ - private $config; - - protected function setUp(): void { - $this->dbConnection = $this->createMock(IDBConnection::class); - $this->timeFactory = $this->createMock(ITimeFactory::class); - $this->logger = $this->createMock(LoggerInterface::class); - $this->config = $this->createMock(IConfig::class); - - $this->throttler = new Throttler( - $this->timeFactory, - $this->logger, - $this->config, - new DatabaseBackend($this->dbConnection) - ); - parent::setUp(); - } - - public function dataIsIPWhitelisted() { - return [ - [ - '10.10.10.10', - [ - 'whitelist_0' => '10.10.10.0/24', - ], - true, - ], - [ - '10.10.10.10', - [ - 'whitelist_0' => '192.168.0.0/16', - ], - false, - ], - [ - '10.10.10.10', - [ - 'whitelist_0' => '192.168.0.0/16', - 'whitelist_1' => '10.10.10.0/24', - ], - true, - ], - [ - '10.10.10.10', - [ - 'whitelist_0' => '10.10.10.11/31', - ], - true, - ], - [ - '10.10.10.10', - [ - 'whitelist_0' => '10.10.10.9/31', - ], - false, - ], - [ - '10.10.10.10', - [ - 'whitelist_0' => '10.10.10.15/29', - ], - true, - ], - [ - 'dead:beef:cafe::1', - [ - 'whitelist_0' => '192.168.0.0/16', - 'whitelist_1' => '10.10.10.0/24', - 'whitelist_2' => 'deaf:beef:cafe:1234::/64' - ], - false, - ], - [ - 'dead:beef:cafe::1', - [ - 'whitelist_0' => '192.168.0.0/16', - 'whitelist_1' => '10.10.10.0/24', - 'whitelist_2' => 'deaf:beef::/64' - ], - false, - ], - [ - 'dead:beef:cafe::1', - [ - 'whitelist_0' => '192.168.0.0/16', - 'whitelist_1' => '10.10.10.0/24', - 'whitelist_2' => 'deaf:cafe::/8' - ], - true, - ], - [ - 'dead:beef:cafe::1111', - [ - 'whitelist_0' => 'dead:beef:cafe::1100/123', - - ], - true, - ], - [ - 'invalid', - [], - false, - ], - ]; - } - - /** - * @param string $ip - * @param string[] $whitelists - * @param bool $isWhiteListed - * @param bool $enabled - */ - private function isIpWhiteListedHelper($ip, - $whitelists, - $isWhiteListed, - $enabled) { - $this->config->method('getAppKeys') - ->with($this->equalTo('bruteForce')) - ->willReturn(array_keys($whitelists)); - $this->config - ->expects($this->once()) - ->method('getSystemValueBool') - ->with('auth.bruteforce.protection.enabled', true) - ->willReturn($enabled); - - $this->config->method('getAppValue') - ->willReturnCallback(function ($app, $key, $default) use ($whitelists) { - if ($app !== 'bruteForce') { - return $default; - } - if (isset($whitelists[$key])) { - return $whitelists[$key]; - } - return $default; - }); - - $this->assertSame( - ($enabled === false) ? true : $isWhiteListed, - self::invokePrivate($this->throttler, 'isBypassListed', [$ip]) - ); - } - - /** - * @dataProvider dataIsIPWhitelisted - * - * @param string $ip - * @param string[] $whitelists - * @param bool $isWhiteListed - */ - public function testIsIpWhiteListedWithEnabledProtection($ip, - $whitelists, - $isWhiteListed): void { - $this->isIpWhiteListedHelper( - $ip, - $whitelists, - $isWhiteListed, - true - ); - } - - /** - * @dataProvider dataIsIPWhitelisted - * - * @param string $ip - * @param string[] $whitelists - * @param bool $isWhiteListed - */ - public function testIsIpWhiteListedWithDisabledProtection($ip, - $whitelists, - $isWhiteListed): void { - $this->isIpWhiteListedHelper( - $ip, - $whitelists, - $isWhiteListed, - false - ); - } -} diff --git a/tests/lib/Security/Ip/BruteforceAllowListTest.php b/tests/lib/Security/Ip/BruteforceAllowListTest.php new file mode 100644 index 00000000000..f7ef9e3df72 --- /dev/null +++ b/tests/lib/Security/Ip/BruteforceAllowListTest.php @@ -0,0 +1,162 @@ +<?php + +declare(strict_types=1); + +/** + * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +namespace Test\Security\Ip; + +use OC\Security\Ip\BruteforceAllowList; +use OC\Security\Ip\Factory; +use OCP\IAppConfig; +use OCP\Security\Ip\IFactory; +use PHPUnit\Framework\MockObject\MockObject; +use Test\TestCase; + +/** + * Based on the unit tests from Paragonie's Airship CMS + * Ref: https://github.com/paragonie/airship/blob/7e5bad7e3c0fbbf324c11f963fd1f80e59762606/test/unit/Engine/Security/AirBrakeTest.php + * + * @package Test\Security\Bruteforce + */ +class BruteforceAllowListTest extends TestCase { + /** @var IAppConfig|MockObject */ + private $appConfig; + /** @var IFactory|MockObject */ + private $factory; + /** @var BruteforceAllowList */ + private $allowList; + + protected function setUp(): void { + parent::setUp(); + + $this->appConfig = $this->createMock(IAppConfig::class); + $this->factory = new Factory(); + + $this->allowList = new BruteforceAllowList( + $this->appConfig, + $this->factory, + ); + } + + public function dataIsBypassListed(): array { + return [ + [ + '10.10.10.10', + [ + 'whitelist_0' => '10.10.10.0/24', + ], + true, + ], + [ + '10.10.10.10', + [ + 'whitelist_0' => '192.168.0.0/16', + ], + false, + ], + [ + '10.10.10.10', + [ + 'whitelist_0' => '192.168.0.0/16', + 'whitelist_1' => '10.10.10.0/24', + ], + true, + ], + [ + '10.10.10.10', + [ + 'whitelist_0' => '10.10.10.11/31', + ], + true, + ], + [ + '10.10.10.10', + [ + 'whitelist_0' => '10.10.10.9/31', + ], + false, + ], + [ + '10.10.10.10', + [ + 'whitelist_0' => '10.10.10.15/29', + ], + true, + ], + [ + 'dead:beef:cafe::1', + [ + 'whitelist_0' => '192.168.0.0/16', + 'whitelist_1' => '10.10.10.0/24', + 'whitelist_2' => 'deaf:beef:cafe:1234::/64' + ], + false, + ], + [ + 'dead:beef:cafe::1', + [ + 'whitelist_0' => '192.168.0.0/16', + 'whitelist_1' => '10.10.10.0/24', + 'whitelist_2' => 'deaf:beef::/64' + ], + false, + ], + [ + 'dead:beef:cafe::1', + [ + 'whitelist_0' => '192.168.0.0/16', + 'whitelist_1' => '10.10.10.0/24', + 'whitelist_2' => 'deaf:cafe::/8' + ], + true, + ], + [ + 'dead:beef:cafe::1111', + [ + 'whitelist_0' => 'dead:beef:cafe::1100/123', + ], + true, + ], + [ + 'invalid', + [], + false, + ], + ]; + } + + /** + * @dataProvider dataIsBypassListed + * + * @param string[] $allowList + */ + public function testIsBypassListed( + string $ip, + array $allowList, + bool $isAllowListed, + ): void { + $this->appConfig->method('getKeys') + ->with($this->equalTo('bruteForce')) + ->willReturn(array_keys($allowList)); + + $this->appConfig->method('getValueString') + ->willReturnCallback(function ($app, $key, $default) use ($allowList) { + if ($app !== 'bruteForce') { + return $default; + } + if (isset($allowList[$key])) { + return $allowList[$key]; + } + return $default; + }); + + $this->assertSame( + $isAllowListed, + $this->allowList->isBypassListed($ip) + ); + } +} |