Merge pull request #52815 from nextcloud/backport/52798/stable31

[stable31] fix: log requests exceeding the rate limiting

Author: kesselb

lib/private/Security/RateLimiting/Limiter.php

@@ -13,10 +13,12 @@
 use OC\Security\RateLimiting\Exception\RateLimitExceededException;
 use OCP\IUser;
 use OCP\Security\RateLimiting\ILimiter;
+use Psr\Log\LoggerInterface;
 
 class Limiter implements ILimiter {
 	public function __construct(
 		private IBackend $backend,
+		private LoggerInterface $logger,
 	) {
 	}
 
@@ -32,6 +34,11 @@ private function register(
 	): void {
 		$existingAttempts = $this->backend->getAttempts($methodIdentifier, $userIdentifier);
 		if ($existingAttempts >= $limit) {
+			$this->logger->info('Request blocked because it exceeds the rate limit [method: {method}, limit: {limit}, period: {period}]', [
+				'method' => $methodIdentifier,
+				'limit' => $limit,
+				'period' => $period,
+			]);
 			throw new RateLimitExceededException();
 		}
 

tests/lib/Security/RateLimiting/LimiterTest.php

@@ -12,21 +12,26 @@
 use OC\Security\RateLimiting\Backend\IBackend;
 use OC\Security\RateLimiting\Limiter;
 use OCP\IUser;
+use OCP\Security\RateLimiting\ILimiter;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class LimiterTest extends TestCase {
-	/** @var IBackend|\PHPUnit\Framework\MockObject\MockObject */
-	private $backend;
-	/** @var Limiter */
-	private $limiter;
+
+	private IBackend&MockObject $backend;
+	private ILimiter $limiter;
+	private LoggerInterface $logger;
 
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->backend = $this->createMock(IBackend::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 
 		$this->limiter = new Limiter(
-			$this->backend
+			$this->backend,
+			$this->logger,
 		);
 	}
 
@@ -43,6 +48,8 @@ public function testRegisterAnonRequestExceeded(): void {
 				'4664f0d9c88dcb7552be47b37bb52ce35977b2e60e1ac13757cf625f31f87050a41f3da064887fa87d49fd042e4c8eb20de8f10464877d3959677ab011b73a47'
 			)
 			->willReturn(101);
+		$this->logger->expects($this->once())
+			->method('info');
 
 		$this->limiter->registerAnonRequest('MyIdentifier', 100, 100, '127.0.0.1');
 	}
@@ -64,6 +71,8 @@ public function testRegisterAnonRequestSuccess(): void {
 				'4664f0d9c88dcb7552be47b37bb52ce35977b2e60e1ac13757cf625f31f87050a41f3da064887fa87d49fd042e4c8eb20de8f10464877d3959677ab011b73a47',
 				100
 			);
+		$this->logger->expects($this->never())
+			->method('info');
 
 		$this->limiter->registerAnonRequest('MyIdentifier', 100, 100, '127.0.0.1');
 	}
@@ -87,6 +96,8 @@ public function testRegisterUserRequestExceeded(): void {
 				'ddb2ec50fa973fd49ecf3d816f677c8095143e944ad10485f30fb3dac85c13a346dace4dae2d0a15af91867320957bfd38a43d9eefbb74fe6919e15119b6d805'
 			)
 			->willReturn(101);
+		$this->logger->expects($this->once())
+			->method('info');
 
 		$this->limiter->registerUserRequest('MyIdentifier', 100, 100, $user);
 	}
@@ -115,6 +126,8 @@ public function testRegisterUserRequestSuccess(): void {
 				'ddb2ec50fa973fd49ecf3d816f677c8095143e944ad10485f30fb3dac85c13a346dace4dae2d0a15af91867320957bfd38a43d9eefbb74fe6919e15119b6d805',
 				100
 			);
+		$this->logger->expects($this->never())
+			->method('info');
 
 		$this->limiter->registerUserRequest('MyIdentifier', 100, 100, $user);
 	}