fix: address code review feedback on rate limiter

- 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>

php/src/Auth/RateLimiter.php

@@ -0,0 +1,44 @@
+<?php
+declare(strict_types=1);
+
+namespace AIO\Auth;
+
+class RateLimiter {
+    public const int MAX_ATTEMPTS = 10;
+    public const int WINDOW_SECONDS = 900; // 15 minutes
+
+    /**
+     * Returns true when the IP has exceeded the maximum number of failed login
+     * attempts within the current time window and should be blocked.
+     */
+    public function isLimitReached(string $ip): bool {
+        $attempts = apcu_fetch($this->getKey($ip));
+        return $attempts !== false && is_numeric($attempts) && (int)$attempts >= self::MAX_ATTEMPTS;
+    }
+
+    /**
+     * Records a failed login attempt for the given IP.
+     * Uses a 15-minute sliding window: the first failure starts the window and
+     * subsequent failures within that window are counted together.
+     */
+    public function recordFailedAttempt(string $ip): void {
+        $key = $this->getKey($ip);
+        // Initialize the key if it does not yet exist (apcu_add is a no-op when key exists).
+        // apcu_inc is atomic, so using add-then-increment gives a consistent count even
+        // under concurrent requests.
+        apcu_add($key, 0, self::WINDOW_SECONDS);
+        apcu_inc($key);
+    }
+
+    /**
+     * Clears the failed-attempt counter for the given IP, e.g. after a
+     * successful login.
+     */
+    public function resetAttempts(string $ip): void {
+        apcu_delete($this->getKey($ip));
+    }
+
+    private function getKey(string $ip): string {
+        return 'login_attempts_' . hash('sha256', $ip);
+    }
+}

php/src/Controller/LoginController.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 namespace AIO\Controller;
 
 use AIO\Auth\AuthManager;
+use AIO\Auth\RateLimiter;
 use AIO\Container\Container;
 use AIO\ContainerDefinitionFetcher;
 use AIO\Docker\DockerActionManager;
@@ -14,20 +15,33 @@ readonly class LoginController {
     public function __construct(
         private AuthManager $authManager,
         private DockerActionManager $dockerActionManager,
+        private RateLimiter $rateLimiter,
     ) {
     }
 
     public function TryLogin(Request $request, Response $response, array $args) : Response {
+        $ip = (string)($request->getServerParams()['REMOTE_ADDR'] ?? '');
+
+        if ($this->rateLimiter->isLimitReached($ip)) {
+            $response->getBody()->write("Too many failed login attempts. Please try again later.");
+            return $response
+                ->withHeader('Retry-After', (string)RateLimiter::WINDOW_SECONDS)
+                ->withStatus(429);
+        }
+
         if (!$this->dockerActionManager->isLoginAllowed()) {
             $response->getBody()->write("The login is blocked since Nextcloud is running.");
             return $response->withHeader('Location', '.')->withStatus(422);
         }
         $password = $request->getParsedBody()['password'] ?? '';
         if($this->authManager->CheckCredentials($password)) {
+            $this->rateLimiter->resetAttempts($ip);
             $this->authManager->SetAuthState(true);
             return $response->withHeader('Location', '.')->withStatus(201);
         }
 
+        $this->rateLimiter->recordFailedAttempt($ip);
+
         // Punish failed auth attempts with a delay, as a very simple means against bots.
         sleep(5);
 
@@ -36,12 +50,24 @@ readonly class LoginController {
     }
 
     public function GetTryLogin(Request $request, Response $response, array $args) : Response {
+        $ip = (string)($request->getServerParams()['REMOTE_ADDR'] ?? '');
+
+        if ($this->rateLimiter->isLimitReached($ip)) {
+            $response->getBody()->write("Too many failed login attempts. Please try again later.");
+            return $response
+                ->withHeader('Retry-After', (string)RateLimiter::WINDOW_SECONDS)
+                ->withStatus(429);
+        }
+
         $token = $request->getQueryParams()['token'] ?? '';
         if($this->authManager->CheckToken($token)) {
+            $this->rateLimiter->resetAttempts($ip);
             $this->authManager->SetAuthState(true);
             return $response->withHeader('Location', '../..')->withStatus(302);
         }
 
+        $this->rateLimiter->recordFailedAttempt($ip);
+
         // Punish failed auth attempts with a delay, as a very simple means against bots.
         sleep(5);
 

php/src/DependencyInjection.php

@@ -43,6 +43,10 @@ class DependencyInjection
             \AIO\Auth\AuthManager::class,
             new \AIO\Auth\AuthManager($container->get(\AIO\Data\ConfigurationManager::class))
         );
+        $container->set(
+            \AIO\Auth\RateLimiter::class,
+            new \AIO\Auth\RateLimiter()
+        );
         $container->set(
             \AIO\Data\Setup::class,
             new \AIO\Data\Setup(