Skip to content
Prev Previous commit
Next Next commit
fix: Make bypass function public API
Signed-off-by: Joas Schilling <[email protected]>
  • Loading branch information
nickvergessen committed Aug 21, 2023
commit 124588d4a64f518aef9270002e72c3604ddb3077
9 changes: 2 additions & 7 deletions core/Command/Security/BruteforceAttempts.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,16 @@
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 {
Expand Down Expand Up @@ -69,7 +64,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

$data = [
'allow-listed' => $this->throttler->isIPWhitelisted($ip),
'bypass-listed' => $this->throttler->isBypassListed($ip),
'attempts' => $this->throttler->getAttempts(

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OCP\Security\Bruteforce\IThrottler::getAttempts has been marked as deprecated
$ip,
(string) $input->getArgument('action'),
Expand Down
5 changes: 3 additions & 2 deletions lib/private/Security/Bruteforce/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@
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,
) {
}

Expand All @@ -47,7 +48,7 @@ public function getCapabilities(): array {
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()),
],
];
}
Expand Down
10 changes: 5 additions & 5 deletions lib/private/Security/Bruteforce/Throttler.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public function registerAttempt(string $action,
}

$ipAddress = new IpAddress($ip);
if ($this->isIPWhitelisted((string)$ipAddress)) {
if ($this->isBypassListed((string)$ipAddress)) {
return;
}

Expand Down Expand Up @@ -110,7 +110,7 @@ public function registerAttempt(string $action,
* @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];
}
Expand Down Expand Up @@ -200,7 +200,7 @@ public function getAttempts(string $ip, string $action = '', float $maxAgeHours
}

$ipAddress = new IpAddress($ip);
if ($this->isIPWhitelisted((string)$ipAddress)) {
if ($this->isBypassListed((string)$ipAddress)) {
return 0;
}

Expand Down Expand Up @@ -245,7 +245,7 @@ public function resetDelay(string $ip, string $action, array $metadata): void {
}

$ipAddress = new IpAddress($ip);
if ($this->isIPWhitelisted((string)$ipAddress)) {
if ($this->isBypassListed((string)$ipAddress)) {
return;
}

Expand All @@ -268,7 +268,7 @@ public function resetDelayForIP(string $ip): void {
}

$ipAddress = new IpAddress($ip);
if ($this->isIPWhitelisted((string)$ipAddress)) {
if ($this->isBypassListed((string)$ipAddress)) {
return;
}

Expand Down
10 changes: 10 additions & 0 deletions lib/public/Security/Bruteforce/IThrottler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
*
Expand Down
8 changes: 4 additions & 4 deletions tests/lib/Security/Bruteforce/CapabilitiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
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 {
Expand All @@ -36,15 +36,15 @@ 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 {
parent::setUp();

$this->request = $this->createMock(IRequest::class);

$this->throttler = $this->createMock(Throttler::class);
$this->throttler = $this->createMock(IThrottler::class);

$this->capabilities = new Capabilities(
$this->request,
Expand All @@ -59,7 +59,7 @@ public function testGetCapabilities(): void {
->willReturn(42);

$this->throttler->expects($this->atLeastOnce())
->method('isIPWhitelisted')
->method('isBypassListed')
->with('10.10.10.10')
->willReturn(true);

Expand Down
2 changes: 1 addition & 1 deletion tests/lib/Security/Bruteforce/ThrottlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ private function isIpWhiteListedHelper($ip,

$this->assertSame(
($enabled === false) ? true : $isWhiteListed,
self::invokePrivate($this->throttler, 'isIPWhitelisted', [$ip])
self::invokePrivate($this->throttler, 'isBypassListed', [$ip])
);
}

Expand Down