diff options
3 files changed, 44 insertions, 17 deletions
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); + } } |