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 9b202f07fbf29ca7704a2b31b2d9add61096a8f0..9a9740b7bccc40c61b7c681dd83cbdb6ee142098 100644 (file)
--- 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 ed43befd121120c06e55510e5883b3fe3b705312..ba8c7f45b49c3174efd527433e483044fe9d97a1 100644 (file)
--- 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 b3dff10f6bcc00e5b8b185061c976a97c58127ec..388b2fbf534391a0f27cbab07a28df157f71ccff 100644 (file)
--- 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);
+       }
 }