Skip to content

Commit 55f6157

Browse files
nickvergessenbackportbot[bot]
authored andcommitted
feat(rate-limit): Allow overwriting the rate limit
Signed-off-by: Joas Schilling <coding@schilljs.com> [skip ci]
1 parent 72dd55e commit 55f6157

File tree

2 files changed

+54
-3
lines changed

2 files changed

+54
-3
lines changed

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: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
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;
@@ -64,7 +65,9 @@ class RateLimitingMiddlewareTest extends TestCase {
6465
private Limiter|MockObject $limiter;
6566
private ISession|MockObject $session;
6667
private IAppConfig|MockObject $appConfig;
68+
private IConfig|MockObject $serverConfig;
6769
private BruteforceAllowList|MockObject $bruteForceAllowList;
70+
private LoggerInterface|MockObject $logger;
6871
private RateLimitingMiddleware $rateLimitingMiddleware;
6972

7073
protected function setUp(): void {
@@ -76,7 +79,9 @@ protected function setUp(): void {
7679
$this->limiter = $this->createMock(Limiter::class);
7780
$this->session = $this->createMock(ISession::class);
7881
$this->appConfig = $this->createMock(IAppConfig::class);
82+
$this->serverConfig = $this->createMock(IConfig::class);
7983
$this->bruteForceAllowList = $this->createMock(BruteforceAllowList::class);
84+
$this->logger = $this->createMock(LoggerInterface::class);
8085

8186
$this->rateLimitingMiddleware = new RateLimitingMiddleware(
8287
$this->request,
@@ -85,7 +90,9 @@ protected function setUp(): void {
8590
$this->limiter,
8691
$this->session,
8792
$this->appConfig,
93+
$this->serverConfig,
8894
$this->bruteForceAllowList,
95+
$this->logger
8996
);
9097
}
9198

0 commit comments

Comments
 (0)