From 124588d4a64f518aef9270002e72c3604ddb3077 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 16 Aug 2023 17:40:38 +0200 Subject: [PATCH] fix: Make bypass function public API Signed-off-by: Joas Schilling --- core/Command/Security/BruteforceAttempts.php | 9 ++------- lib/private/Security/Bruteforce/Capabilities.php | 5 +++-- lib/private/Security/Bruteforce/Throttler.php | 10 +++++----- lib/public/Security/Bruteforce/IThrottler.php | 10 ++++++++++ tests/lib/Security/Bruteforce/CapabilitiesTest.php | 8 ++++---- tests/lib/Security/Bruteforce/ThrottlerTest.php | 2 +- 6 files changed, 25 insertions(+), 19 deletions(-) diff --git a/core/Command/Security/BruteforceAttempts.php b/core/Command/Security/BruteforceAttempts.php index 9cbf446958d..9237078339d 100644 --- a/core/Command/Security/BruteforceAttempts.php +++ b/core/Command/Security/BruteforceAttempts.php @@ -25,21 +25,16 @@ declare(strict_types=1); namespace OC\Core\Command\Security; use OC\Core\Command\Base; -use OC\Security\Bruteforce\Throttler; use OCP\Security\Bruteforce\IThrottler; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; class BruteforceAttempts extends Base { - /** @var Throttler */ - protected IThrottler $throttler; - public function __construct( - IThrottler $throttler, + protected IThrottler $throttler, ) { parent::__construct(); - $this->throttler = $throttler; } protected function configure(): void { @@ -69,7 +64,7 @@ class BruteforceAttempts extends Base { } $data = [ - 'allow-listed' => $this->throttler->isIPWhitelisted($ip), + 'bypass-listed' => $this->throttler->isBypassListed($ip), 'attempts' => $this->throttler->getAttempts( $ip, (string) $input->getArgument('action'), diff --git a/lib/private/Security/Bruteforce/Capabilities.php b/lib/private/Security/Bruteforce/Capabilities.php index 4eada3d05f5..b50eea0b7af 100644 --- a/lib/private/Security/Bruteforce/Capabilities.php +++ b/lib/private/Security/Bruteforce/Capabilities.php @@ -32,11 +32,12 @@ namespace OC\Security\Bruteforce; use OCP\Capabilities\IPublicCapability; use OCP\Capabilities\IInitialStateExcludedCapability; use OCP\IRequest; +use OCP\Security\Bruteforce\IThrottler; class Capabilities implements IPublicCapability, IInitialStateExcludedCapability { public function __construct( private IRequest $request, - private Throttler $throttler, + private IThrottler $throttler, ) { } @@ -47,7 +48,7 @@ class Capabilities implements IPublicCapability, IInitialStateExcludedCapability return [ 'bruteforce' => [ 'delay' => $this->throttler->getDelay($this->request->getRemoteAddress()), - 'allow-listed' => $this->throttler->isIPWhitelisted($this->request->getRemoteAddress()), + 'allow-listed' => $this->throttler->isBypassListed($this->request->getRemoteAddress()), ], ]; } diff --git a/lib/private/Security/Bruteforce/Throttler.php b/lib/private/Security/Bruteforce/Throttler.php index 5c4f4d320b1..2803373e8ba 100644 --- a/lib/private/Security/Bruteforce/Throttler.php +++ b/lib/private/Security/Bruteforce/Throttler.php @@ -80,7 +80,7 @@ class Throttler implements IThrottler { } $ipAddress = new IpAddress($ip); - if ($this->isIPWhitelisted((string)$ipAddress)) { + if ($this->isBypassListed((string)$ipAddress)) { return; } @@ -110,7 +110,7 @@ class Throttler implements IThrottler { * @param string $ip * @return bool */ - public function isIPWhitelisted(string $ip): bool { + public function isBypassListed(string $ip): bool { if (isset($this->ipIsWhitelisted[$ip])) { return $this->ipIsWhitelisted[$ip]; } @@ -200,7 +200,7 @@ class Throttler implements IThrottler { } $ipAddress = new IpAddress($ip); - if ($this->isIPWhitelisted((string)$ipAddress)) { + if ($this->isBypassListed((string)$ipAddress)) { return 0; } @@ -245,7 +245,7 @@ class Throttler implements IThrottler { } $ipAddress = new IpAddress($ip); - if ($this->isIPWhitelisted((string)$ipAddress)) { + if ($this->isBypassListed((string)$ipAddress)) { return; } @@ -268,7 +268,7 @@ class Throttler implements IThrottler { } $ipAddress = new IpAddress($ip); - if ($this->isIPWhitelisted((string)$ipAddress)) { + if ($this->isBypassListed((string)$ipAddress)) { return; } diff --git a/lib/public/Security/Bruteforce/IThrottler.php b/lib/public/Security/Bruteforce/IThrottler.php index 03c8c56a23c..620a53fd354 100644 --- a/lib/public/Security/Bruteforce/IThrottler.php +++ b/lib/public/Security/Bruteforce/IThrottler.php @@ -66,6 +66,16 @@ interface IThrottler { */ public function registerAttempt(string $action, string $ip, array $metadata = []): void; + + /** + * Check if the IP is allowed to bypass the brute force protection + * + * @param string $ip + * @return bool + * @since 28.0.0 + */ + public function isBypassListed(string $ip): bool; + /** * Get the throttling delay (in milliseconds) * diff --git a/tests/lib/Security/Bruteforce/CapabilitiesTest.php b/tests/lib/Security/Bruteforce/CapabilitiesTest.php index d3463d307c0..266acdcb285 100644 --- a/tests/lib/Security/Bruteforce/CapabilitiesTest.php +++ b/tests/lib/Security/Bruteforce/CapabilitiesTest.php @@ -25,8 +25,8 @@ declare(strict_types=1); namespace Test\Security\Bruteforce; use OC\Security\Bruteforce\Capabilities; -use OC\Security\Bruteforce\Throttler; use OCP\IRequest; +use OCP\Security\Bruteforce\IThrottler; use Test\TestCase; class CapabilitiesTest extends TestCase { @@ -36,7 +36,7 @@ class CapabilitiesTest extends TestCase { /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */ private $request; - /** @var Throttler|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IThrottler|\PHPUnit\Framework\MockObject\MockObject */ private $throttler; protected function setUp(): void { @@ -44,7 +44,7 @@ class CapabilitiesTest extends TestCase { $this->request = $this->createMock(IRequest::class); - $this->throttler = $this->createMock(Throttler::class); + $this->throttler = $this->createMock(IThrottler::class); $this->capabilities = new Capabilities( $this->request, @@ -59,7 +59,7 @@ class CapabilitiesTest extends TestCase { ->willReturn(42); $this->throttler->expects($this->atLeastOnce()) - ->method('isIPWhitelisted') + ->method('isBypassListed') ->with('10.10.10.10') ->willReturn(true); diff --git a/tests/lib/Security/Bruteforce/ThrottlerTest.php b/tests/lib/Security/Bruteforce/ThrottlerTest.php index e7fd12645e1..e368a0912b1 100644 --- a/tests/lib/Security/Bruteforce/ThrottlerTest.php +++ b/tests/lib/Security/Bruteforce/ThrottlerTest.php @@ -185,7 +185,7 @@ class ThrottlerTest extends TestCase { $this->assertSame( ($enabled === false) ? true : $isWhiteListed, - self::invokePrivate($this->throttler, 'isIPWhitelisted', [$ip]) + self::invokePrivate($this->throttler, 'isBypassListed', [$ip]) ); } -- 2.39.5