Skip to content

Commit 970410d

Browse files
Merge pull request #56382 from nextcloud/backport/56346/stable32
[stable32] feat(rate-limit): Allow overwriting the rate limit
2 parents ced2896 + 826fe1a commit 970410d

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
@@ -463,6 +463,30 @@
463463
*/
464464
'ratelimit.protection.enabled' => true,
465465

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

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
@@ -20,11 +20,13 @@
2020
use OCP\AppFramework\Http\DataResponse;
2121
use OCP\AppFramework\Http\TemplateResponse;
2222
use OCP\IAppConfig;
23+
use OCP\IConfig;
2324
use OCP\IRequest;
2425
use OCP\ISession;
2526
use OCP\IUser;
2627
use OCP\IUserSession;
2728
use PHPUnit\Framework\MockObject\MockObject;
29+
use Psr\Log\LoggerInterface;
2830
use Test\TestCase;
2931

3032
class TestRateLimitController extends Controller {
@@ -64,7 +66,9 @@ class RateLimitingMiddlewareTest extends TestCase {
6466
private Limiter|MockObject $limiter;
6567
private ISession|MockObject $session;
6668
private IAppConfig|MockObject $appConfig;
69+
private IConfig|MockObject $serverConfig;
6770
private BruteforceAllowList|MockObject $bruteForceAllowList;
71+
private LoggerInterface|MockObject $logger;
6872
private RateLimitingMiddleware $rateLimitingMiddleware;
6973

7074
protected function setUp(): void {
@@ -76,7 +80,9 @@ protected function setUp(): void {
7680
$this->limiter = $this->createMock(Limiter::class);
7781
$this->session = $this->createMock(ISession::class);
7882
$this->appConfig = $this->createMock(IAppConfig::class);
83+
$this->serverConfig = $this->createMock(IConfig::class);
7984
$this->bruteForceAllowList = $this->createMock(BruteforceAllowList::class);
85+
$this->logger = $this->createMock(LoggerInterface::class);
8086

8187
$this->rateLimitingMiddleware = new RateLimitingMiddleware(
8288
$this->request,
@@ -85,7 +91,9 @@ protected function setUp(): void {
8591
$this->limiter,
8692
$this->session,
8793
$this->appConfig,
94+
$this->serverConfig,
8895
$this->bruteForceAllowList,
96+
$this->logger
8997
);
9098
}
9199

0 commit comments

Comments
 (0)