From 8356d0dadc850bc224ed9e34c7187bc8b9419190 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 09:58:46 +0000 Subject: [PATCH] security: address code-review comments on rate-limit and clean-history script Agent-Logs-Url: https://github.com/nextcloud/all-in-one/sessions/f1016d36-0771-46e0-992c-95ce22594414 Co-authored-by: szaimen <42591237+szaimen@users.noreply.github.com> --- php/public/clean-history.js | 7 ++++++- php/src/Controller/LoginController.php | 11 ++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/php/public/clean-history.js b/php/public/clean-history.js index 731fce4a..17bd4ac4 100644 --- a/php/public/clean-history.js +++ b/php/public/clean-history.js @@ -2,5 +2,10 @@ // It replaces the browser's current history entry (stripping the token from the // URL) before navigating to the main AIO page, so the token is never left in // the browser history and cannot be accidentally exposed via the back-button. +// +// The target URL is passed via the script tag's data-target attribute. +// document.currentScript is only available during synchronous script execution +// (not with defer/async), so this script is loaded without those attributes. +const target = document.currentScript.dataset.target; history.replaceState(null, '', location.pathname); -window.location.replace('../../'); +window.location.replace(target); diff --git a/php/src/Controller/LoginController.php b/php/src/Controller/LoginController.php index c40f1553..00a1a2fc 100644 --- a/php/src/Controller/LoginController.php +++ b/php/src/Controller/LoginController.php @@ -34,7 +34,12 @@ readonly class LoginController { } // Per-IP rate limiting: block after MAX_FAILED_ATTEMPTS failures within RATE_LIMIT_WINDOW_SEC. - $ip = $request->getServerParams()['REMOTE_ADDR'] ?? 'unknown'; + $ip = $request->getServerParams()['REMOTE_ADDR'] ?? ''; + if ($ip === '') { + // Refuse to rate-limit against a shared bucket when no IP is available. + $response->getBody()->write("Unable to determine client IP. Login refused."); + return $response->withStatus(403); + } $rateLimitKey = 'login_attempts_' . hash('sha256', $ip); $attempts = (int)(apcu_fetch($rateLimitKey) ?: 0); @@ -42,7 +47,7 @@ readonly class LoginController { // Keep a delay even when blocked so the 429 itself isn't a timing oracle. sleep(5); $response->getBody()->write("Too many failed login attempts. Please try again later."); - return $response->withHeader('Location', '.')->withStatus(429); + return $response->withStatus(429); } $password = $request->getParsedBody()['password'] ?? ''; @@ -73,7 +78,7 @@ readonly class LoginController { $response->getBody()->write( '' . '' . - '
' . + '' . '' . '' );