Merge pull request #4630 from nextcloud/mark-ip-as-whitelisted-if-bruteforce-protection-is-disabled

Mark IP as whitelisted if brute force protection is disabled

2 files changed, 54 insertions, 7 deletions

diff --git a/lib/private/Security/Bruteforce/Throttler.php b/lib/private/Security/Bruteforce/Throttler.php
index b2524b63c63..ee02bc5a1c4 100644
--- a/lib/private/Security/Bruteforce/Throttler.php
+++ b/lib/private/Security/Bruteforce/Throttler.php
@@ -133,6 +133,10 @@ class Throttler {
 	 * @return bool
 	 */
 	private function isIPWhitelisted($ip) {
+		if($this->config->getSystemValue('auth.bruteforce.protection.enabled', true) === false) {
+			return true;
+		}
+
 		$keys = $this->config->getAppKeys('bruteForce');
 		$keys = array_filter($keys, function($key) {
 			$regex = '/^whitelist_/S';
diff --git a/tests/lib/Security/Bruteforce/ThrottlerTest.php b/tests/lib/Security/Bruteforce/ThrottlerTest.php
index 9679d0c1759..dac12a00dcd 100644
--- a/tests/lib/Security/Bruteforce/ThrottlerTest.php
+++ b/tests/lib/Security/Bruteforce/ThrottlerTest.php
@@ -54,19 +54,19 @@ class ThrottlerTest extends TestCase {
 			$this->logger,
 			$this->config
 		);
-		return parent::setUp();
+		parent::setUp();
 	}
 
 	public function testCutoff() {
 		// precisely 31 second shy of 12 hours
-		$cutoff = $this->invokePrivate($this->throttler, 'getCutoff', [43169]);
+		$cutoff = self::invokePrivate($this->throttler, 'getCutoff', [43169]);
 		$this->assertSame(0, $cutoff->y);
 		$this->assertSame(0, $cutoff->m);
 		$this->assertSame(0, $cutoff->d);
 		$this->assertSame(11, $cutoff->h);
 		$this->assertSame(59, $cutoff->i);
 		$this->assertSame(29, $cutoff->s);
-		$cutoff = $this->invokePrivate($this->throttler, 'getCutoff', [86401]);
+		$cutoff = self::invokePrivate($this->throttler, 'getCutoff', [86401]);
 		$this->assertSame(0, $cutoff->y);
 		$this->assertSame(0, $cutoff->m);
 		$this->assertSame(1, $cutoff->d);
@@ -136,16 +136,23 @@ class ThrottlerTest extends TestCase {
 	}
 
 	/**
-	 * @dataProvider dataIsIPWhitelisted
-	 *
 	 * @param string $ip
 	 * @param string[] $whitelists
 	 * @param bool $isWhiteListed
+	 * @param bool $enabled
 	 */
-	public function testIsIPWhitelisted($ip, $whitelists, $isWhiteListed) {
+	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('getSystemValue')
+			->with('auth.bruteforce.protection.enabled', true)
+			->willReturn($enabled);
 
 		$this->config->method('getAppValue')
 			->will($this->returnCallback(function($app, $key, $default) use ($whitelists) {
@@ -159,8 +166,44 @@ class ThrottlerTest extends TestCase {
 			}));
 
 		$this->assertSame(
+			($enabled === false) ? true : $isWhiteListed,
+			self::invokePrivate($this->throttler, 'isIPWhitelisted', [$ip])
+		);
+	}
+
+	/**
+	 * @dataProvider dataIsIPWhitelisted
+	 *
+	 * @param string $ip
+	 * @param string[] $whitelists
+	 * @param bool $isWhiteListed
+	 */
+	public function testIsIpWhiteListedWithEnabledProtection($ip,
+															 $whitelists,
+															 $isWhiteListed) {
+		$this->isIpWhiteListedHelper(
+			$ip,
+			$whitelists,
+			$isWhiteListed,
+			true
+		);
+	}
+
+	/**
+	 * @dataProvider dataIsIPWhitelisted
+	 *
+	 * @param string $ip
+	 * @param string[] $whitelists
+	 * @param bool $isWhiteListed
+	 */
+	public function testIsIpWhiteListedWithDisabledProtection($ip,
+															 $whitelists,
+															 $isWhiteListed) {
+		$this->isIpWhiteListedHelper(
+			$ip,
+			$whitelists,
 			$isWhiteListed,
-			$this->invokePrivate($this->throttler, 'isIPWhitelisted', [$ip])
+			false
 		);
 	}
 }