Draft
feat: add proper rate limiting for login endpoints#8081
Conversation
- Add RateLimiter class using APCu for per-IP tracking - Limit failed login attempts to 10 per 15-minute window - Apply rate limiting in TryLogin (POST /api/auth/login) and GetTryLogin (GET /api/auth/getlogin) - Return 429 Too Many Requests with Retry-After header when limit exceeded - Reset counter on successful login - Register RateLimiter in DependencyInjection container 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>
- 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>
Copilot created this pull request from a session on behalf of
szaimen
May 12, 2026 10:13
View session
szaimen
reviewed
May 12, 2026
| } | ||
|
|
||
| public function TryLogin(Request $request, Response $response, array $args) : Response { | ||
| $ip = (string)($request->getServerParams()['REMOTE_ADDR'] ?? ''); |
Collaborator
There was a problem hiding this comment.
I guess this needs some proper thinking if this is enough, for example for the reverse proxy situation. We might need to add a configuration option that allows to configure the trusted proxies range for caddy
szaimen
reviewed
May 12, 2026
| } | ||
|
|
||
| public function GetTryLogin(Request $request, Response $response, array $args) : Response { | ||
| $ip = (string)($request->getServerParams()['REMOTE_ADDR'] ?? ''); |
Collaborator
There was a problem hiding this comment.
I guess this needs some proper thinking if this is enough, for example for the reverse proxy situation. We might need to add a configuration option that allows to configure the trusted proxies range for caddy
This was referenced May 12, 2026
31 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces IP-based rate limiting for the two login endpoints of the AIO master container web interface.
Problem
The previous protection against brute-force attacks was a simple
sleep(5)after each failed attempt. This does not prevent parallel attacks or scripted attempts that simply tolerate the delay.Solution
A new
RateLimiterclass (backed by APCu, which is already a required PHP extension) tracks failed login attempts per source IP:429 Too Many Requestswith aRetry-After: 900header and a clear error messagePOST /api/auth/login(password login) andGET /api/auth/getlogin(token-based login from Nextcloud admin panel) share the same counter per IPThe
sleep(5)delay on failed attempts is kept as an additional cost for individual attempts, complementing the overall rate limit.Changes
php/src/Auth/RateLimiter.phpphp/src/Controller/LoginController.phpRateLimiter; check/record/reset inTryLoginandGetTryLoginphp/src/DependencyInjection.phpRateLimitersingleton in the DI containerImplementation notes
REMOTE_ADDR(the direct peer address as seen by the PHP process)apcu_add(key, 0, TTL)+apcu_inc(key)pattern ensures the TTL is set on first use while keeping the increment atomicis_numericguard added before casting the cached value to prevent unexpected behaviour from corrupted cache entries