daenamnu/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/all-in-one.git synced 2026-05-21 10:50:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: address code review feedback on rate limiter

Browse Source 
- Add error message body to GetTryLogin 429 response for consistency
- Improve atomicity: use apcu_add(0)+apcu_inc instead of conditional add
- Add is_numeric guard before int cast in isLimitReached

Agent-Logs-Url: https://github.com/nextcloud/all-in-one/sessions/1ce6e415-a2b7-478c-955f-72f582c157c4

Co-authored-by: szaimen <42591237+szaimen@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2026-05-12 10:12:08 +00:00
committed by GitHub
parent a466cd4284
commit 8008d6e2f5
2 changed files with 7 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
php/src/Auth/RateLimiter.php
View File

@@ -13,7 +13,7 @@ class RateLimiter {
*/ */
public function isLimitReached(string $ip): bool { public function isLimitReached(string $ip): bool {
$attempts = apcu_fetch($this->getKey($ip)); $attempts = apcu_fetch($this->getKey($ip));
return $attempts !== false && (int)$attempts >= self::MAX_ATTEMPTS; return $attempts !== false && is_numeric($attempts) && (int)$attempts >= self::MAX_ATTEMPTS;
} }
/** /**
@@ -23,12 +23,12 @@ class RateLimiter {
*/ */
public function recordFailedAttempt(string $ip): void { public function recordFailedAttempt(string $ip): void {
$key = $this->getKey($ip); $key = $this->getKey($ip);
// apcu_add only stores when the key does not yet exist. // Initialize the key if it does not yet exist (apcu_add is a no-op when key exists).
// If it already exists (returns false), we increment the existing counter. // apcu_inc is atomic, so using add-then-increment gives a consistent count even
if (!apcu_add($key, 1, self::WINDOW_SECONDS)) { // under concurrent requests.
apcu_add($key, 0, self::WINDOW_SECONDS);
apcu_inc($key); apcu_inc($key);
} }
}
/** /**
* Clears the failed-attempt counter for the given IP, e.g. after a * Clears the failed-attempt counter for the given IP, e.g. after a

1
php/src/Controller/LoginController.php
View File

@@ -53,6 +53,7 @@ readonly class LoginController {
$ip = (string)($request->getServerParams()['REMOTE_ADDR'] ?? ''); $ip = (string)($request->getServerParams()['REMOTE_ADDR'] ?? '');
if ($this->rateLimiter->isLimitReached($ip)) { if ($this->rateLimiter->isLimitReached($ip)) {
$response->getBody()->write("Too many failed login attempts. Please try again later.");
return $response return $response
->withHeader('Retry-After', (string)RateLimiter::WINDOW_SECONDS) ->withHeader('Retry-After', (string)RateLimiter::WINDOW_SECONDS)
->withStatus(429); ->withStatus(429);