Make BruteForceProtection annotation more clever

This makes the new `@BruteForceProtection` annotation more clever and moves the relevant code into it's own middleware.

Basically you can now set `@BruteForceProtection(action=$key)` as annotation and that will make the controller bruteforce protected. However, the difference to before is that you need to call `$responmse->throttle()` to increase the counter. Before the counter was increased every time which leads to all kind of unexpected problems.

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

1 files changed, 1 insertions, 75 deletions

diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
index 164ea48de70..17ac30b8fe4 100644
--- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
@@ -20,8 +20,6 @@
  *
  */
 
-
-
 namespace Test\AppFramework\Middleware\Security;
 
 use OC\AppFramework\Http;
@@ -34,7 +32,6 @@ use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
 use OC\Appframework\Middleware\Security\Exceptions\StrictCookieMissingException;
 use OC\AppFramework\Middleware\Security\SecurityMiddleware;
 use OC\AppFramework\Utility\ControllerMethodReflector;
-use OC\Security\Bruteforce\Throttler;
 use OC\Security\CSP\ContentSecurityPolicy;
 use OC\Security\CSP\ContentSecurityPolicyManager;
 use OC\Security\CSP\ContentSecurityPolicyNonceManager;
@@ -54,7 +51,6 @@ use OCP\ISession;
 use OCP\IURLGenerator;
 use OCP\Security\ISecureRandom;
 
-
 class SecurityMiddlewareTest extends \Test\TestCase {
 
 	/** @var SecurityMiddleware|\PHPUnit_Framework_MockObject_MockObject */
@@ -83,8 +79,6 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 	private $csrfTokenManager;
 	/** @var ContentSecurityPolicyNonceManager|\PHPUnit_Framework_MockObject_MockObject */
 	private $cspNonceManager;
-	/** @var  Throttler|\PHPUnit_Framework_MockObject_MockObject */
-	private $bruteForceThrottler;
 
 	protected function setUp() {
 		parent::setUp();
@@ -99,7 +93,6 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 		$this->contentSecurityPolicyManager = $this->createMock(ContentSecurityPolicyManager::class);
 		$this->csrfTokenManager = $this->createMock(CsrfTokenManager::class);
 		$this->cspNonceManager = $this->createMock(ContentSecurityPolicyNonceManager::class);
-		$this->bruteForceThrottler = $this->getMockBuilder(Throttler::class)->disableOriginalConstructor()->getMock();
 		$this->middleware = $this->getMiddleware(true, true);
 		$this->secException = new SecurityException('hey', false);
 		$this->secAjaxException = new SecurityException('hey', true);
@@ -123,8 +116,7 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 			$isAdminUser,
 			$this->contentSecurityPolicyManager,
 			$this->csrfTokenManager,
-			$this->cspNonceManager,
-			$this->bruteForceThrottler
+			$this->cspNonceManager
 		);
 	}
 
@@ -657,70 +649,4 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 
 		$this->assertEquals($response, $this->middleware->afterController($this->controller, 'test', $response));
 	}
-
-	/**
-	 * @dataProvider dataTestBeforeControllerBruteForce
-	 */
-	public function testBeforeControllerBruteForce($bruteForceProtectionEnabled) {
-		/** @var ControllerMethodReflector|\PHPUnit_Framework_MockObject_MockObject $reader */
-		$reader = $this->getMockBuilder(ControllerMethodReflector::class)->disableOriginalConstructor()->getMock();
-
-		$middleware = new SecurityMiddleware(
-			$this->request,
-			$reader,
-			$this->navigationManager,
-			$this->urlGenerator,
-			$this->logger,
-			$this->session,
-			'files',
-			false,
-			false,
-			$this->contentSecurityPolicyManager,
-			$this->csrfTokenManager,
-			$this->cspNonceManager,
-			$this->bruteForceThrottler
-		);
-
-		$reader->expects($this->any())->method('hasAnnotation')
-			->willReturnCallback(
-				function($annotation) use ($bruteForceProtectionEnabled) {
-
-					switch ($annotation) {
-						case 'BruteForceProtection':
-							return $bruteForceProtectionEnabled;
-						case 'PasswordConfirmationRequired':
-						case 'StrictCookieRequired':
-							return false;
-						case 'PublicPage':
-						case 'NoCSRFRequired':
-							return true;
-					}
-
-					return true;
-			}
-			);
-
-		$reader->expects($this->any())->method('getAnnotationParameter')->willReturn('action');
-		$this->request->expects($this->any())->method('getRemoteAddress')->willReturn('remoteAddress');
-
-		if ($bruteForceProtectionEnabled) {
-			$this->bruteForceThrottler->expects($this->once())->method('sleepDelay')
-				->with('remoteAddress', 'action');
-			$this->bruteForceThrottler->expects($this->once())->method('registerAttempt')
-				->with('action', 'remoteAddress');
-		} else {
-			$this->bruteForceThrottler->expects($this->never())->method('sleepDelay');
-			$this->bruteForceThrottler->expects($this->never())->method('registerAttempt');
-		}
-
-		$middleware->beforeController($this->controller, 'test');
-
-	}
-
-	public function dataTestBeforeControllerBruteForce() {
-		return [
-			[true],
-			[false]
-		];
-	}
 }