Skip to content

Commit 49324bc

Browse files
Merge pull request #56346 from nextcloud/feat/noid/allow-overwriting-rate-limit
feat(rate-limit): Allow overwriting the rate limit
2 parents b3fd79f + 2b9083a commit 49324bc

File tree

3 files changed

+79
-3
lines changed

3 files changed

+79
-3
lines changed

config/config.sample.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,30 @@
473473
*/
474474
'ratelimit.protection.enabled' => true,
475475

476+
/**
477+
* Overwrite the individual rate limit for a specific route
478+
*
479+
* From time to time it can be necessary to extend the rate limit of a specific route,
480+
* depending on your usage pattern or when you script some actions.
481+
* Instead of completely disabling the rate limit or excluding an IP address from the
482+
* rate limit, the following config allows to overwrite the rate limit duration and period.
483+
*
484+
* The first level key is the name of the route. You can find the route name from a URL
485+
* using the ``occ router:list`` command of your server.
486+
*
487+
* You can also specify different limits for logged-in users with the ``user`` key
488+
* and not-logged-in users with the ``anon`` key. However, if there is no specific ``user`` limit,
489+
* the ``anon`` limit is also applied for logged-in users.
490+
*
491+
* Defaults to empty array ``[]``
492+
*/
493+
'ratelimit_overwrite' => [
494+
'profile.profilepage.index' => [
495+
'user' => ['limit' => 300, 'period' => 3600],
496+
'anon' => ['limit' => 1, 'period' => 300],
497+
]
498+
],
499+
476500
/**
477501
* Size of subnet used to normalize IPv6
478502
*

lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@
2222
use OCP\AppFramework\Http\TemplateResponse;
2323
use OCP\AppFramework\Middleware;
2424
use OCP\IAppConfig;
25+
use OCP\IConfig;
2526
use OCP\IRequest;
2627
use OCP\ISession;
2728
use OCP\IUserSession;
29+
use Psr\Log\LoggerInterface;
2830
use ReflectionMethod;
2931

3032
/**
@@ -56,7 +58,9 @@ public function __construct(
5658
protected Limiter $limiter,
5759
protected ISession $session,
5860
protected IAppConfig $appConfig,
61+
protected IConfig $serverConfig,
5962
protected BruteforceAllowList $bruteForceAllowList,
63+
protected LoggerInterface $logger,
6064
) {
6165
}
6266

@@ -74,7 +78,13 @@ public function beforeController(Controller $controller, string $methodName): vo
7478
}
7579

7680
if ($this->userSession->isLoggedIn()) {
77-
$rateLimit = $this->readLimitFromAnnotationOrAttribute($controller, $methodName, 'UserRateThrottle', UserRateLimit::class);
81+
$rateLimit = $this->readLimitFromAnnotationOrAttribute(
82+
$controller,
83+
$methodName,
84+
'UserRateThrottle',
85+
UserRateLimit::class,
86+
'user',
87+
);
7888

7989
if ($rateLimit !== null) {
8090
if ($this->appConfig->getValueBool('bruteforcesettings', 'apply_allowlist_to_ratelimit')
@@ -94,7 +104,13 @@ public function beforeController(Controller $controller, string $methodName): vo
94104
// If not user specific rate limit is found the Anon rate limit applies!
95105
}
96106

97-
$rateLimit = $this->readLimitFromAnnotationOrAttribute($controller, $methodName, 'AnonRateThrottle', AnonRateLimit::class);
107+
$rateLimit = $this->readLimitFromAnnotationOrAttribute(
108+
$controller,
109+
$methodName,
110+
'AnonRateThrottle',
111+
AnonRateLimit::class,
112+
'anon',
113+
);
98114

99115
if ($rateLimit !== null) {
100116
$this->limiter->registerAnonRequest(
@@ -115,7 +131,35 @@ public function beforeController(Controller $controller, string $methodName): vo
115131
* @param class-string<T> $attributeClass
116132
* @return ?ARateLimit
117133
*/
118-
protected function readLimitFromAnnotationOrAttribute(Controller $controller, string $methodName, string $annotationName, string $attributeClass): ?ARateLimit {
134+
protected function readLimitFromAnnotationOrAttribute(Controller $controller, string $methodName, string $annotationName, string $attributeClass, string $overwriteKey): ?ARateLimit {
135+
$rateLimitOverwrite = $this->serverConfig->getSystemValue('ratelimit_overwrite', []);
136+
if (!empty($rateLimitOverwrite)) {
137+
$controllerRef = new \ReflectionClass($controller);
138+
$appName = $controllerRef->getProperty('appName')->getValue($controller);
139+
$controllerName = substr($controller::class, strrpos($controller::class, '\\') + 1);
140+
$controllerName = substr($controllerName, 0, 0 - strlen('Controller'));
141+
142+
$overwriteConfig = strtolower($appName . '.' . $controllerName . '.' . $methodName);
143+
$rateLimitOverwriteForActionAndType = $rateLimitOverwrite[$overwriteConfig][$overwriteKey] ?? null;
144+
if ($rateLimitOverwriteForActionAndType !== null) {
145+
$isValid = isset($rateLimitOverwriteForActionAndType['limit'], $rateLimitOverwriteForActionAndType['period'])
146+
&& $rateLimitOverwriteForActionAndType['limit'] > 0
147+
&& $rateLimitOverwriteForActionAndType['period'] > 0;
148+
149+
if ($isValid) {
150+
return new $attributeClass(
151+
(int)$rateLimitOverwriteForActionAndType['limit'],
152+
(int)$rateLimitOverwriteForActionAndType['period'],
153+
);
154+
}
155+
156+
$this->logger->warning('Rate limit overwrite on controller "{overwriteConfig}" for "{overwriteKey}" is invalid', [
157+
'overwriteConfig' => $overwriteConfig,
158+
'overwriteKey' => $overwriteKey,
159+
]);
160+
}
161+
}
162+
119163
$annotationLimit = $this->reflector->getAnnotationParameter($annotationName, 'limit');
120164
$annotationPeriod = $this->reflector->getAnnotationParameter($annotationName, 'period');
121165

tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@
1717
use OCP\AppFramework\Http\DataResponse;
1818
use OCP\AppFramework\Http\TemplateResponse;
1919
use OCP\IAppConfig;
20+
use OCP\IConfig;
2021
use OCP\IRequest;
2122
use OCP\ISession;
2223
use OCP\IUser;
2324
use OCP\IUserSession;
2425
use PHPUnit\Framework\MockObject\MockObject;
26+
use Psr\Log\LoggerInterface;
2527
use Test\AppFramework\Middleware\Security\Mock\RateLimitingMiddlewareController;
2628
use Test\TestCase;
2729

@@ -33,7 +35,9 @@ class RateLimitingMiddlewareTest extends TestCase {
3335
private Limiter|MockObject $limiter;
3436
private ISession|MockObject $session;
3537
private IAppConfig|MockObject $appConfig;
38+
private IConfig|MockObject $serverConfig;
3639
private BruteforceAllowList|MockObject $bruteForceAllowList;
40+
private LoggerInterface|MockObject $logger;
3741
private RateLimitingMiddleware $rateLimitingMiddleware;
3842

3943
protected function setUp(): void {
@@ -45,7 +49,9 @@ protected function setUp(): void {
4549
$this->limiter = $this->createMock(Limiter::class);
4650
$this->session = $this->createMock(ISession::class);
4751
$this->appConfig = $this->createMock(IAppConfig::class);
52+
$this->serverConfig = $this->createMock(IConfig::class);
4853
$this->bruteForceAllowList = $this->createMock(BruteforceAllowList::class);
54+
$this->logger = $this->createMock(LoggerInterface::class);
4955

5056
$this->rateLimitingMiddleware = new RateLimitingMiddleware(
5157
$this->request,
@@ -54,7 +60,9 @@ protected function setUp(): void {
5460
$this->limiter,
5561
$this->session,
5662
$this->appConfig,
63+
$this->serverConfig,
5764
$this->bruteForceAllowList,
65+
$this->logger
5866
);
5967
}
6068

0 commit comments

Comments
 (0)