diff --git a/backend/app/Controllers/User/Auth/AuthLogoutController.php b/backend/app/Controllers/User/Auth/AuthLogoutController.php index 383bd20be..eac6712b8 100755 --- a/backend/app/Controllers/User/Auth/AuthLogoutController.php +++ b/backend/app/Controllers/User/Auth/AuthLogoutController.php @@ -22,6 +22,7 @@ use App\Helpers\ApiResponse; use OpenApi\Attributes as OA; use App\CloudFlare\CloudFlareRealIP; +use App\Helpers\SessionCookieHelper; use App\Plugins\Events\Events\AuthEvent; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -53,7 +54,7 @@ public function get(Request $request): Response { global $eventManager; if (!isset($_COOKIE['remember_token'])) { - setcookie('remember_token', '', time() - 3600 - 1500 * 120); + SessionCookieHelper::clear(); return ApiResponse::success([], 'Logged out and we did not find a remember_token', 200); } @@ -94,7 +95,7 @@ public function get(Request $request): Response 'ip_address' => CloudFlareRealIP::getRealIP(), ]); } - setcookie('remember_token', '', time() - 3600 - 1500 * 120); + SessionCookieHelper::clear(); return ApiResponse::success([], 'Logged out', 200); } diff --git a/backend/app/Controllers/User/Auth/ForgotPasswordController.php b/backend/app/Controllers/User/Auth/ForgotPasswordController.php index 57f1f5a16..d2f0efd20 100755 --- a/backend/app/Controllers/User/Auth/ForgotPasswordController.php +++ b/backend/app/Controllers/User/Auth/ForgotPasswordController.php @@ -121,6 +121,12 @@ public function put(Request $request): Response return ApiResponse::error('Invalid email address', 'INVALID_EMAIL_ADDRESS'); } + // Generic response message used regardless of whether the account exists. + // Returning a distinct error for unknown emails allows trivial account + // enumeration (see security audit), so we always respond the same way + // and only perform the reset side-effects when the account is real. + $genericSuccess = static fn () => ApiResponse::success(null, 'If an account with that email exists, we have sent a password reset link', 200); + // Login user $userInfo = User::getUserByEmail($data['email']); if ($userInfo == null) { @@ -137,9 +143,11 @@ public function put(Request $request): Response ); } - return ApiResponse::error('Email does not exist', 'EMAIL_DOES_NOT_EXIST'); + // Do not reveal whether the email exists: respond identically to the + // success path instead of returning EMAIL_DOES_NOT_EXIST. + return $genericSuccess(); } - $resetToken = bin2hex(random_bytes(32)); + $resetToken = bin2hex(random_bytes(32)) . '.' . (time() + 3600); if (User::updateUser($userInfo['uuid'], ['mail_verify' => $resetToken])) { // Send reset password email @@ -182,9 +190,11 @@ public function put(Request $request): Response 'ip_address' => CloudFlareRealIP::getRealIP(), ]); - return ApiResponse::success(null, 'We have sent you an email to reset your password', 200); + return $genericSuccess(); } - return ApiResponse::error('Failed to update user', 'FAILED_TO_UPDATE_USER'); + // Even on internal failure, do not leak account existence via a different + // response than the generic one. + return $genericSuccess(); } } diff --git a/backend/app/Controllers/User/Auth/LoginController.php b/backend/app/Controllers/User/Auth/LoginController.php index 0c0fc95f3..71b3d4a2c 100755 --- a/backend/app/Controllers/User/Auth/LoginController.php +++ b/backend/app/Controllers/User/Auth/LoginController.php @@ -27,6 +27,8 @@ use App\Config\ConfigInterface; use App\Helpers\UserDeviceTracker; use App\CloudFlare\CloudFlareRealIP; +use App\Helpers\AccountLockoutHelper; +use App\Helpers\SessionCookieHelper; use App\Plugins\Events\Events\AuthEvent; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -203,7 +205,13 @@ public function put(Request $request): Response ); } - return ApiResponse::error('Invalid username or email address', 'INVALID_USERNAME_OR_EMAIL'); + // Run a dummy password_verify against a fixed bcrypt hash so that the + // response timing for "unknown user" is close to the "wrong password" + // path below, and return the same generic error/code in both cases + // to avoid leaking whether an account exists (account enumeration). + password_verify($data['password'], '$2y$12$hKs6swAiRf/kPjRDC6xEWun.GMew67fz3jytWTurlD/p4Ag7xyCf6'); + + return ApiResponse::error('Invalid username, email address, or password', 'INVALID_CREDENTIALS'); } if ($userInfo['banned'] == 'true') { // Emit login failed event @@ -226,6 +234,20 @@ public function put(Request $request): Response return ApiResponse::error('Account is deleted', 'ACCOUNT_DELETED', 403); } + // Per-account lockout: independent of IP-based rate limiting so that + // rotating IPs (proxy/botnet) cannot be used to brute-force a single + // known account's password. + $lockoutId = 'login:' . $userInfo['uuid']; + $lockoutRemaining = AccountLockoutHelper::getLockoutRemaining($lockoutId); + if ($lockoutRemaining > 0) { + return ApiResponse::error( + 'Too many failed login attempts. Try again in ' . ceil($lockoutRemaining / 60) . ' minute(s).', + 'ACCOUNT_LOCKED', + 429, + ['retry_after' => $lockoutRemaining] + ); + } + // When OIDC has disabled local login, only allow local login for admins (before password check to avoid leaking valid-credential signal) if ($config->getSetting(ConfigInterface::OIDC_DISABLE_LOCAL_LOGIN, 'false') === 'true') { if (!\App\Helpers\PermissionHelper::hasPermission($userInfo['uuid'], \App\Permissions::ADMIN_ROOT)) { @@ -247,9 +269,15 @@ public function put(Request $request): Response ); } - return ApiResponse::error('Invalid password', 'INVALID_PASSWORD'); + AccountLockoutHelper::recordFailure($lockoutId); + + return ApiResponse::error('Invalid username, email address, or password', 'INVALID_CREDENTIALS'); } + // Successful password check: clear any prior failure count for this account. + AccountLockoutHelper::clear($lockoutId); + + $requiresEmailVerification = $config->getSetting(ConfigInterface::REGISTRATION_REQUIRE_EMAIL_VERIFICATION, 'false') === 'true'; $isEmailVerified = !isset($userInfo['mail_verify']) || $userInfo['mail_verify'] === null || trim((string) $userInfo['mail_verify']) === ''; if ($requiresEmailVerification && !$isEmailVerified) { @@ -292,7 +320,7 @@ public function completeLogin(array $userInfo, ?string $redirectTo = null): Resp return ApiResponse::error('Remember token not set', 'REMEMBER_TOKEN_NOT_SET'); } $userInfo['remember_token'] = $token; - setcookie('remember_token', $token, time() + 60 * 60 * 24 * 30, '/'); + SessionCookieHelper::set($token, time() + 60 * 60 * 24 * 30); User::updateUser($userInfo['uuid'], ['last_ip' => CloudFlareRealIP::getRealIP()]); UserDeviceTracker::trackFromGlobals($userInfo); diff --git a/backend/app/Controllers/User/Auth/RegisterController.php b/backend/app/Controllers/User/Auth/RegisterController.php index 56e5b98e4..4f805995a 100755 --- a/backend/app/Controllers/User/Auth/RegisterController.php +++ b/backend/app/Controllers/User/Auth/RegisterController.php @@ -30,6 +30,7 @@ use App\Mail\templates\VerifyEmail; use App\CloudFlare\CloudFlareRealIP; use App\Helpers\EmailDomainValidator; +use App\Helpers\SessionCookieHelper; use App\Plugins\Events\Events\AuthEvent; use App\Helpers\AbuseIPDBRegistrationGuard; use App\Helpers\UserDeviceRegistrationGuard; @@ -345,7 +346,7 @@ public function put(Request $request): Response // Set session/cookie if (isset($createdUser['remember_token'])) { $token = $createdUser['remember_token']; - setcookie('remember_token', $token, time() + 60 * 60 * 24 * 30, '/'); + SessionCookieHelper::set($token, time() + 60 * 60 * 24 * 30); User::updateUser($createdUser['uuid'], ['last_ip' => CloudFlareRealIP::getRealIP()]); // Create login activity (user is automatically logged in) diff --git a/backend/app/Controllers/User/Auth/ResetPasswordController.php b/backend/app/Controllers/User/Auth/ResetPasswordController.php index 90eda99d5..225afd365 100755 --- a/backend/app/Controllers/User/Auth/ResetPasswordController.php +++ b/backend/app/Controllers/User/Auth/ResetPasswordController.php @@ -120,6 +120,29 @@ public function put(Request $request): Response return ApiResponse::error('Looks like the token is invalid or expired or already used', 'INVALID_TOKEN'); } + // Password reset tokens embed an expiry as "." (see + // ForgotPasswordController). Older/foreign tokens without that + // suffix (e.g. an email-verification token that happens to be + // looked up here) are treated as expired/invalid rather than + // accepted indefinitely. + if (!self::isResetTokenValid($data['token'])) { + global $eventManager; + if (isset($eventManager) && $eventManager !== null) { + $eventManager->emit( + AuthEvent::onAuthPasswordResetFailed(), + [ + 'token' => $data['token'], + 'reason' => 'TOKEN_EXPIRED', + 'ip_address' => CloudFlareRealIP::getRealIP(), + ] + ); + } + // Invalidate the stale token so it can't be retried later. + User::updateUser($userInfo['uuid'], ['mail_verify' => null]); + + return ApiResponse::error('Looks like the token is invalid or expired or already used', 'INVALID_TOKEN'); + } + if (User::updateUser($userInfo['uuid'], ['password' => password_hash($data['password'], PASSWORD_BCRYPT), 'remember_token' => User::generateAccountToken()]) && User::updateUser($userInfo['uuid'], ['mail_verify' => null])) { Activity::createActivity([ 'user_uuid' => $userInfo['uuid'], @@ -187,10 +210,37 @@ public function get(Request $request): Response 'token' => $token, ]); } + if (!self::isResetTokenValid($token)) { + return ApiResponse::error('Looks like the token is invalid or expired', 'INVALID_TOKEN', 400, [ + 'token' => $token, + ]); + } return ApiResponse::success(null, 'Token is valid', 200); } catch (\Exception $e) { return ApiResponse::exception('An error occurred: ' . $e->getMessage(), $e->getCode()); } } + + /** + * Reset tokens are generated as "<64 hex chars>." (see + * ForgotPasswordController::put()). Validate both the shape and that the + * embedded expiry has not passed yet. + */ + private static function isResetTokenValid(string $token): bool + { + $parts = explode('.', $token, 2); + if (count($parts) !== 2) { + return false; + } + [$hex, $expiresAt] = $parts; + if (!ctype_xdigit($hex) || strlen($hex) !== 64) { + return false; + } + if (!ctype_digit($expiresAt)) { + return false; + } + + return (int) $expiresAt >= time(); + } } diff --git a/backend/app/Controllers/User/Auth/TwoFactorController.php b/backend/app/Controllers/User/Auth/TwoFactorController.php index 33d969a24..47e4ff59d 100755 --- a/backend/app/Controllers/User/Auth/TwoFactorController.php +++ b/backend/app/Controllers/User/Auth/TwoFactorController.php @@ -25,6 +25,8 @@ use App\Config\ConfigInterface; use PragmaRX\Google2FA\Google2FA; use App\CloudFlare\CloudFlareRealIP; +use App\Helpers\AccountLockoutHelper; +use App\Helpers\SessionCookieHelper; use App\Plugins\Events\Events\AuthEvent; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -266,6 +268,21 @@ public function post(Request $request): Response if (!$userInfo || $userInfo['two_fa_enabled'] !== 'true') { return ApiResponse::error('2FA not enabled', 'two_fa_NOT_ENABLED'); } + + // Per-account lockout for 2FA verification: a 6-digit TOTP code has + // only 1,000,000 possible values, so this must be tighter than the + // login password lockout to prevent brute-forcing it via IP rotation. + $lockoutId = '2fa:' . $userInfo['uuid']; + $lockoutRemaining = AccountLockoutHelper::getLockoutRemaining($lockoutId); + if ($lockoutRemaining > 0) { + return ApiResponse::error( + 'Too many failed 2FA attempts. Try again in ' . ceil($lockoutRemaining / 60) . ' minute(s).', + 'ACCOUNT_LOCKED', + 429, + ['retry_after' => $lockoutRemaining] + ); + } + $google2fa = new Google2FA(); if (!$google2fa->verifyKey($userInfo['two_fa_key'], $data['code'])) { // Emit 2FA failed event @@ -280,9 +297,13 @@ public function post(Request $request): Response ); } + AccountLockoutHelper::recordFailure($lockoutId, maxAttempts: 5, lockoutSeconds: 900); + return ApiResponse::error('Invalid 2FA code', 'INVALID_CODE'); } + AccountLockoutHelper::clear($lockoutId); + // Logging in cancels a pending self-service account deletion request if (\App\Services\User\UserDeletionService::hasPendingDeletion($userInfo)) { \App\Services\User\UserDeletionService::cancelPendingDeletion($userInfo); @@ -297,7 +318,7 @@ public function post(Request $request): Response return ApiResponse::error('Remember token not set', 'REMEMBER_TOKEN_NOT_SET'); } $userInfo['remember_token'] = $token; - setcookie('remember_token', $token, time() + 60 * 60 * 24 * 30, '/'); + SessionCookieHelper::set($token, time() + 60 * 60 * 24 * 30); User::updateUser($userInfo['uuid'], ['last_ip' => CloudFlareRealIP::getRealIP()]); Activity::createActivity([ diff --git a/backend/app/Controllers/User/User/AccountDeletionController.php b/backend/app/Controllers/User/User/AccountDeletionController.php index 106d367f3..68c0c42b5 100644 --- a/backend/app/Controllers/User/User/AccountDeletionController.php +++ b/backend/app/Controllers/User/User/AccountDeletionController.php @@ -24,6 +24,7 @@ use OpenApi\Attributes as OA; use App\Helpers\CaptchaHelper; use App\Config\ConfigInterface; +use App\Helpers\SessionCookieHelper; use PragmaRX\Google2FA\Google2FA; use App\CloudFlare\CloudFlareRealIP; use App\Mail\templates\AccountDeletionOtp; @@ -372,7 +373,7 @@ private function validateCaptchaIfRequired($config, array $data): ?Response private function clearSessionCookie(): void { - setcookie('remember_token', '', time() - 3600, '/'); + SessionCookieHelper::clear(); unset($_COOKIE['remember_token']); } } diff --git a/backend/app/Helpers/AccountLockoutHelper.php b/backend/app/Helpers/AccountLockoutHelper.php new file mode 100644 index 000000000..864b7d1b8 --- /dev/null +++ b/backend/app/Helpers/AccountLockoutHelper.php @@ -0,0 +1,117 @@ +. + */ + +namespace App\Helpers; + +use App\App; + +/** + * Per-account failed authentication tracking, independent of the existing + * per-IP rate limiter. This closes the gap where an attacker can rotate IPs + * (proxy/botnet) to bypass IP-based rate limiting and still brute-force a + * single known account (login password or 2FA code). + * + * Fails open (does not block login) if Redis is unavailable, matching the + * behavior of RateLimitMiddleware, so a Redis outage never locks everyone out. + */ +class AccountLockoutHelper +{ + /** Failed attempts allowed before lockout kicks in. */ + private const MAX_ATTEMPTS = 10; + + /** Lockout duration in seconds once MAX_ATTEMPTS is reached. */ + private const LOCKOUT_SECONDS = 900; // 15 minutes + + /** Window in seconds during which failed attempts are counted. */ + private const ATTEMPT_WINDOW_SECONDS = 900; // 15 minutes + + /** + * Returns the remaining lockout time in seconds, or 0 if the identifier + * (e.g. "login:" or "2fa:") is not currently locked. + */ + public static function getLockoutRemaining(string $identifier): int + { + $redis = self::getRedis(); + if ($redis === null) { + return 0; + } + + $ttl = $redis->ttl(self::lockKey($identifier)); + + return $ttl > 0 ? $ttl : 0; + } + + /** + * Record a failed attempt for the identifier. If this pushes the count + * over $maxAttempts within the attempt window, the account is locked for + * $lockoutSeconds. + */ + public static function recordFailure(string $identifier, ?int $maxAttempts = null, ?int $lockoutSeconds = null): void + { + $redis = self::getRedis(); + if ($redis === null) { + return; + } + + $maxAttempts ??= self::MAX_ATTEMPTS; + $lockoutSeconds ??= self::LOCKOUT_SECONDS; + + $countKey = self::countKey($identifier); + $count = $redis->incr($countKey); + if ($count === 1) { + $redis->expire($countKey, self::ATTEMPT_WINDOW_SECONDS); + } + + if ($count >= $maxAttempts) { + $redis->setex(self::lockKey($identifier), $lockoutSeconds, '1'); + } + } + + /** + * Clear failure tracking for the identifier (call on successful auth). + */ + public static function clear(string $identifier): void + { + $redis = self::getRedis(); + if ($redis === null) { + return; + } + + $redis->del([self::countKey($identifier), self::lockKey($identifier)]); + } + + private static function countKey(string $identifier): string + { + return 'account_lockout:count:' . $identifier; + } + + private static function lockKey(string $identifier): string + { + return 'account_lockout:locked:' . $identifier; + } + + private static function getRedis(): ?\Redis + { + try { + $app = App::getInstance(true); + + return $app->getRedisConnection(); + } catch (\Throwable $e) { + return null; + } + } +} diff --git a/backend/app/Helpers/SessionCookieHelper.php b/backend/app/Helpers/SessionCookieHelper.php new file mode 100644 index 000000000..8b8aa1412 --- /dev/null +++ b/backend/app/Helpers/SessionCookieHelper.php @@ -0,0 +1,75 @@ +. + */ + +namespace App\Helpers; + +/** + * Central place to set/clear the `remember_token` session cookie with secure + * flags. Previously every call site used the legacy 4-argument setcookie() + * signature (name, value, expire, path), which sets none of HttpOnly, + * Secure, or SameSite - leaving the session cookie readable by JavaScript + * (XSS -> session hijack) and sendable cross-site (CSRF exposure). + */ +class SessionCookieHelper +{ + private const COOKIE_NAME = 'remember_token'; + + /** + * Set the remember_token cookie with secure flags. + * + * @param string $token The session token value + * @param int $expire Unix timestamp when the cookie should expire + */ + public static function set(string $token, int $expire): void + { + setcookie(self::COOKIE_NAME, $token, [ + 'expires' => $expire, + 'path' => '/', + 'secure' => self::isHttps(), + 'httponly' => true, + 'samesite' => 'Lax', + ]); + } + + /** + * Clear the remember_token cookie (logout / account deletion / etc). + */ + public static function clear(): void + { + setcookie(self::COOKIE_NAME, '', [ + 'expires' => time() - 3600, + 'path' => '/', + 'secure' => self::isHttps(), + 'httponly' => true, + 'samesite' => 'Lax', + ]); + } + + private static function isHttps(): bool + { + if (!empty($_SERVER['HTTPS']) && strtolower((string) $_SERVER['HTTPS']) !== 'off') { + return true; + } + + // Reverse proxy (nginx/Cloudflare) terminates TLS in front of the app. + if (!empty($_SERVER['HTTP_X_FORWARDED_PROTO']) && strtolower((string) $_SERVER['HTTP_X_FORWARDED_PROTO']) === 'https') { + return true; + } + + return false; + } +}