From: Joas Schilling <coding@schilljs.com>
Date: Wed, 8 Mar 2023 11:08:53 +0000 (+0100)
Subject: Add a debug message when throttling without defining
X-Git-Tag: v27.0.0beta1~318^2
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=2b4986167975355238d982ba8579e0cccf6bf5aa;p=nextcloud-server.git

Add a debug message when throttling without defining

Signed-off-by: Joas Schilling <coding@schilljs.com>
---

diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php
index 9b202f07fbf..9a9740b7bcc 100644
--- a/lib/private/AppFramework/DependencyInjection/DIContainer.php
+++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php
@@ -292,7 +292,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
 				new OC\AppFramework\Middleware\Security\BruteForceMiddleware(
 					$c->get(IControllerMethodReflector::class),
 					$c->get(OC\Security\Bruteforce\Throttler::class),
-					$c->get(IRequest::class)
+					$c->get(IRequest::class),
+					$c->get(LoggerInterface::class)
 				)
 			);
 			$dispatcher->registerMiddleware(
diff --git a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php
index ed43befd121..ba8c7f45b49 100644
--- a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php
@@ -40,6 +40,7 @@ use OCP\AppFramework\OCS\OCSException;
 use OCP\AppFramework\OCSController;
 use OCP\IRequest;
 use OCP\Security\Bruteforce\MaxDelayReached;
+use Psr\Log\LoggerInterface;
 use ReflectionMethod;
 
 /**
@@ -50,16 +51,12 @@ use ReflectionMethod;
  * @package OC\AppFramework\Middleware\Security
  */
 class BruteForceMiddleware extends Middleware {
-	private ControllerMethodReflector $reflector;
-	private Throttler $throttler;
-	private IRequest $request;
-
-	public function __construct(ControllerMethodReflector $controllerMethodReflector,
-								Throttler $throttler,
-								IRequest $request) {
-		$this->reflector = $controllerMethodReflector;
-		$this->throttler = $throttler;
-		$this->request = $request;
+	public function __construct(
+		protected ControllerMethodReflector $reflector,
+		protected Throttler $throttler,
+		protected IRequest $request,
+		protected LoggerInterface $logger,
+	) {
 	}
 
 	/**
@@ -116,6 +113,8 @@ class BruteForceMiddleware extends Middleware {
 							$this->throttler->registerAttempt($action, $ip, $metaData);
 						}
 					}
+				} else {
+					$this->logger->debug('Response for ' . get_class($controller) . '::' . $methodName . ' got bruteforce throttled but has no annotation nor attribute defined.');
 				}
 			}
 		}
diff --git a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php
index b3dff10f6bc..388b2fbf534 100644
--- a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php
@@ -29,6 +29,7 @@ use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\Attribute\BruteForceProtection;
 use OCP\AppFramework\Http\Response;
 use OCP\IRequest;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class TestController extends Controller {
@@ -58,6 +59,8 @@ class BruteForceMiddlewareTest extends TestCase {
 	private $throttler;
 	/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */
 	private $request;
+	/** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
+	private $logger;
 	private BruteForceMiddleware $bruteForceMiddleware;
 
 	protected function setUp(): void {
@@ -66,15 +69,17 @@ class BruteForceMiddlewareTest extends TestCase {
 		$this->reflector = new ControllerMethodReflector();
 		$this->throttler = $this->createMock(Throttler::class);
 		$this->request = $this->createMock(IRequest::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 
 		$this->bruteForceMiddleware = new BruteForceMiddleware(
 			$this->reflector,
 			$this->throttler,
-			$this->request
+			$this->request,
+			$this->logger,
 		);
 	}
 
-	public function testBeforeControllerWithAnnotation() {
+	public function testBeforeControllerWithAnnotation(): void {
 		$this->request
 			->expects($this->once())
 			->method('getRemoteAddress')
@@ -122,7 +127,7 @@ class BruteForceMiddlewareTest extends TestCase {
 		$this->bruteForceMiddleware->beforeController($controller, 'multipleAttributes');
 	}
 
-	public function testBeforeControllerWithoutAnnotation() {
+	public function testBeforeControllerWithoutAnnotation(): void {
 		$this->request
 			->expects($this->never())
 			->method('getRemoteAddress');
@@ -135,7 +140,7 @@ class BruteForceMiddlewareTest extends TestCase {
 		$this->bruteForceMiddleware->beforeController($controller, 'testMethodWithoutAnnotation');
 	}
 
-	public function testAfterControllerWithAnnotationAndThrottledRequest() {
+	public function testAfterControllerWithAnnotationAndThrottledRequest(): void {
 		/** @var Response|\PHPUnit\Framework\MockObject\MockObject $response */
 		$response = $this->createMock(Response::class);
 		$response
@@ -164,7 +169,7 @@ class BruteForceMiddlewareTest extends TestCase {
 		$this->bruteForceMiddleware->afterController($controller, 'testMethodWithAnnotation', $response);
 	}
 
-	public function testAfterControllerWithAnnotationAndNotThrottledRequest() {
+	public function testAfterControllerWithAnnotationAndNotThrottledRequest(): void {
 		/** @var Response|\PHPUnit\Framework\MockObject\MockObject $response */
 		$response = $this->createMock(Response::class);
 		$response
@@ -282,7 +287,7 @@ class BruteForceMiddlewareTest extends TestCase {
 		$this->bruteForceMiddleware->afterController($controller, 'multipleAttributes', $response);
 	}
 
-	public function testAfterControllerWithoutAnnotation() {
+	public function testAfterControllerWithoutAnnotation(): void {
 		$this->request
 			->expects($this->never())
 			->method('getRemoteAddress');
@@ -296,4 +301,26 @@ class BruteForceMiddlewareTest extends TestCase {
 		$response = $this->createMock(Response::class);
 		$this->bruteForceMiddleware->afterController($controller, 'testMethodWithoutAnnotation', $response);
 	}
+
+	public function testAfterControllerWithThrottledResponseButUnhandled(): void {
+		$this->request
+			->expects($this->never())
+			->method('getRemoteAddress');
+		$this->throttler
+			->expects($this->never())
+			->method('sleepDelay');
+
+		$controller = new TestController('test', $this->request);
+		$this->reflector->reflect($controller, 'testMethodWithoutAnnotation');
+		/** @var Response|\PHPUnit\Framework\MockObject\MockObject $response */
+		$response = $this->createMock(Response::class);
+		$response->method('isThrottled')
+			->willReturn(true);
+
+		$this->logger->expects($this->once())
+			->method('debug')
+			->with('Response for Test\AppFramework\Middleware\Security\TestController::testMethodWithoutAnnotation got bruteforce throttled but has no annotation nor attribute defined.');
+
+		$this->bruteForceMiddleware->afterController($controller, 'testMethodWithoutAnnotation', $response);
+	}
 }