Harden login rate limiting

Signed-off-by: Yoshifumi Nakamura <nakamura@riken.jp>

Author: yoshifuminakamura

docs/cx/AUDIT_LOG_SPEC.md

@@ -39,10 +39,10 @@ Common fields:
 | `api_auth_failed` | `failure` | none | Missing or invalid API key. The presented key is never logged. |
 | `ingest_accepted` | `success` | runner id | Result, estimate, PA data, or estimation-input upload accepted. |
 | `api_query_accepted` | `success` | runner id | Authenticated API query accepted. |
-| `rate_limit_exceeded` | `failure` | rate-limit key | API or admin fixed-window limit exceeded. |
+| `rate_limit_exceeded` | `failure` | rate-limit key | Login, API, or admin fixed-window limit exceeded. |
 | `redis_unavailable` | `failure` / `degraded` | none | Redis unavailable for authentication or throttling. |
 | `login_success` | `success` | user email | User completed TOTP login. |
-| `login_failure` | `failure` | user email, when known | Login failed. TOTP code is never logged. |
+| `login_failure` | `failure` | user email, when known | Login failed. TOTP code is never logged. Invalid-code failures include a short-window failed-attempt count for audit context, but accounts are not hard-locked by that counter. |
 | `setup_complete` | `success` | user email | Invitation-based TOTP setup completed. |
 | `setup_failure` | `failure` | user email | TOTP setup verification failed. |
 | `admin_user_invited` | `success` | admin email | Admin created an invitation. |

docs/deploy/hardening-guide.md

@@ -22,24 +22,38 @@ oversized uploads are rejected before they consume worker memory.
 
 ## Rate Limits
 
-API ingest/query routes and admin write routes use Redis-backed fixed-window
-rate limits. Production deployments must keep Redis monitored and available;
-when Redis is required but unavailable, protected operations fail closed with a
-503 response.
+Login POST, API ingest/query routes, and admin write routes use Redis-backed
+fixed-window rate limits. Production deployments must keep Redis monitored and
+available; when Redis is required but unavailable, protected operations fail
+closed with a 503 response.
 
 Default limits:
 
+- Login verification: 20 requests per client source per minute
 - API ingest: 120 requests per runner per minute
 - API query: 60 requests per runner per minute
 - Admin write actions: 20 requests per admin user per minute
 
+Login failures also maintain a short-lived per-email counter for audit and
+alerting context, but the counter does not hard-lock the account. This avoids a
+targeted lockout DoS where an attacker can repeatedly close a known user's
+login window; volume control is enforced by the source-scoped login limit.
+
 ## Reverse Proxy
 
 Run the Flask app behind a reverse proxy that terminates TLS and forwards only
 loopback traffic to the app. Keep `/admin/` and `/auth/` protected by portal
 authentication; `robots.txt` only reduces crawler noise and is not an access
 control mechanism.
 
+`app.py` trusts one reverse proxy hop with Werkzeug `ProxyFix`, so the frontend
+proxy must set `X-Forwarded-For` and `X-Forwarded-Proto`. The configured hop
+count assumes nginx is the only trusted proxy directly in front of Gunicorn. If
+a load balancer or another proxy is inserted before nginx, review the
+`ProxyFix` hop count and nginx header handling before enabling the deployment.
+Do not expose Gunicorn directly to untrusted clients, because forwarded headers
+are trusted only under the single-nginx deployment model.
+
 ## Gunicorn
 
 Run Gunicorn under systemd with explicit worker, bind, timeout, and recycling

docs/guides/developer-reference.md

@@ -220,7 +220,8 @@ For production portal deployments:
 - The legacy `RESULT_SERVER_KEY` variable is still accepted as runner `default` for compatibility, but should be rotated to `RESULT_SERVER_KEYS`.
 - See `docs/deploy/key-management.md` for generation and rotation guidance.
 - `REDIS_URL` must point to a monitored Redis instance; production authentication refuses login when Redis is unavailable.
-- API ingest and query endpoints use Redis-backed rate limits by default; set `RESULT_SERVER_MAX_UPLOAD_MB` and `RESULT_SERVER_MAX_ARCHIVE_MEMBER_MB` when deployment-specific upload limits are needed.
+- Login verification, API ingest/query, and admin write endpoints use Redis-backed rate limits by default; set `RESULT_SERVER_MAX_UPLOAD_MB` and `RESULT_SERVER_MAX_ARCHIVE_MEMBER_MB` when deployment-specific upload limits are needed.
+- Repeated login failures are tracked per email for audit context only; source-scoped Redis rate limits enforce login traffic control without hard-locking a target account.
 - Admin-managed affiliations are only rejected when they contain unsafe path/control characters or the comma delimiter used by the form; set `RESULT_SERVER_ALLOWED_AFFILIATIONS` only when a deployment wants to enforce a fixed comma-separated allowlist.
 - Security-relevant auth, API, and admin actions emit structured `benchkit.audit` events; see `docs/cx/AUDIT_LOG_SPEC.md`.
 - `app_dev.py` is localhost-only, uses ephemeral development secrets when none are provided, and enables the Werkzeug debugger only with `RESULT_SERVER_DEV_DEBUG=1`.

result_server/app.py

@@ -4,6 +4,7 @@
 
 from flask import Flask, jsonify, render_template
 from flask_session import Session
+from werkzeug.middleware.proxy_fix import ProxyFix
 
 from routes.api import api_bp
 from routes.estimated import estimated_bp
@@ -42,6 +43,11 @@ def _configure_session(app, base_dir):
     Session(app)
 
 
+def _configure_proxy_fix(app):
+    """Trust the single nginx reverse proxy in front of production gunicorn."""
+    app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1)
+
+
 def _configure_redis(app, prefix):
     """Attach Redis connection settings and the key prefix to app config."""
     import redis
@@ -123,6 +129,7 @@ def create_app(prefix="", base_dir=None):
         raise ValueError("base_dir must be specified")
 
     app = Flask(__name__, template_folder="templates")
+    _configure_proxy_fix(app)
 
     secret_key = os.environ.get("FLASK_SECRET_KEY")
     if not secret_key:

result_server/app_dev.py

@@ -84,7 +84,7 @@ def _create_stub_totp_manager():
     mod.generate_qr_base64 = lambda s, e, **kw: ""
     mod.verify_code = lambda s, c: True
     mod.check_code_reuse = lambda *a, **kw: False
-    mod.check_rate_limit = lambda *a, **kw: False
+    mod.get_failed_attempt_count = lambda *a, **kw: 0
     mod.record_failed_attempt = lambda *a, **kw: 0
     mod.clear_failed_attempts = lambda *a, **kw: None
     mod.MAX_LOGIN_ATTEMPTS = 5

result_server/routes/auth.py

@@ -14,14 +14,16 @@
     session,
     url_for,
 )
+from werkzeug.exceptions import TooManyRequests
 
 from utils.audit_logging import audit_event
+from utils.rate_limit import enforce_rate_limit
 from utils.totp_manager import (
     check_code_reuse,
-    check_rate_limit,
     clear_failed_attempts,
     generate_qr_base64,
     generate_secret,
+    MAX_LOGIN_ATTEMPTS,
     record_failed_attempt,
     verify_code,
 )
@@ -115,6 +117,11 @@ def _render_setup_page(email, token, secret):
     )
 
 
+def _login_rate_key():
+    """Return the source-scoped login rate-limit key."""
+    return request.remote_addr or "unknown"
+
+
 @auth_bp.route("/login", methods=["GET", "POST"])
 def login():
     """Render the login flow and validate submitted TOTP codes."""
@@ -138,19 +145,17 @@ def login():
         if not email:
             return redirect(url_for("auth.login"))
 
-        # Enforce rate limiting when Redis-backed tracking is available.
         if redis_conn:
-            is_locked, remaining = check_rate_limit(redis_conn, prefix, email)
-            if is_locked:
-                audit_event(
-                    "login_failure",
-                    actor=email,
-                    result="failure",
-                    level=logging.WARNING,
-                    details={"reason": "rate_limited"},
+            try:
+                enforce_rate_limit(
+                    redis_conn=redis_conn,
+                    key_suffix=_login_rate_key(),
+                    max_per_minute=20,
+                    scope="login",
                 )
-                flash(f"Too many failed attempts. Please try again in {remaining} seconds.")
-                return _render_login_totp_step(email)
+            except TooManyRequests:
+                flash("Too many login attempts. Please wait and try again.")
+                return _render_login_totp_step(email), 429
 
         store = get_user_store()
         user = store.get_user(email)
@@ -179,26 +184,25 @@ def login():
             flash("Authentication successful.")
             return redirect(url_for("results.results"))
 
-        # Failed authentication: record or report the attempt.
+        # Failed authentication: record a short-window counter for audit/alerting.
+        details = {"reason": "invalid_totp_or_user"}
+        if redis_conn:
+            attempts = record_failed_attempt(redis_conn, prefix, email)
+            details.update(
+                {
+                    "failed_attempts": attempts,
+                    "failed_attempt_threshold": MAX_LOGIN_ATTEMPTS,
+                    "threshold_exceeded": attempts >= MAX_LOGIN_ATTEMPTS,
+                }
+            )
         audit_event(
             "login_failure",
             actor=email,
             result="failure",
             level=logging.WARNING,
-            details={"reason": "invalid_totp_or_user"},
+            details=details,
         )
-        if redis_conn:
-            attempts = record_failed_attempt(redis_conn, prefix, email)
-            from utils.totp_manager import MAX_LOGIN_ATTEMPTS
-
-            remaining_attempts = MAX_LOGIN_ATTEMPTS - attempts
-            if remaining_attempts > 0:
-                flash(f"Authentication failed. {remaining_attempts} attempts remaining.")
-            else:
-                flash("Too many failed attempts. Your account is temporarily locked.")
-                return _render_login_totp_step(email)
-        else:
-            flash("Authentication failed. Please check your code.")
+        flash("Authentication failed. Please check your code.")
         return _render_login_totp_step(email)
 
     return redirect(url_for("auth.login"))

result_server/tests/test_rate_limit.py

@@ -178,6 +178,23 @@ def test_rate_limit_redis_failure_fails_closed_when_required():
         _cleanup(temp_dirs)
 
 
+def test_rate_limit_missing_redis_fails_closed_when_required():
+    app, temp_dirs = _api_app()
+    app.config["REDIS_CONN"] = None
+    app.config["AUTH_REQUIRES_REDIS"] = True
+    try:
+        with app.test_client() as client:
+            resp = client.post(
+                "/api/ingest/result",
+                data=json.dumps({"code": "first"}),
+                headers={"X-API-Key": API_KEY, "Content-Type": "application/json"},
+            )
+
+        assert resp.status_code == 503
+    finally:
+        _cleanup(temp_dirs)
+
+
 def test_admin_write_rate_limit_returns_429():
     app, temp_dirs = _portal_app()
     try: