All gates verified. The phase is complete — everything was already implemented by prior task runs; this pass verified each completion criterion end-to-end and found no defects. **Phase 81 final verification report** (tasks 01–03 all present in `complete/`; no code changes needed this pass) - Verified: `app/core/rate_limit.py` (stdlib sliding window, fail-open) + both login routes' 429 pre-check/record/reset wiring + all unit/integration pins present (11th-429, blocked-success-still-429, clean-counter reset, shared counter, autouse `clean_rate_limit` fixture documented). - Live check (task 03): dev server + 11 rapid wrong logins → `401 ×10, 429`; 429 body carries `retry-after: 900` + generic detail; server restarted (per-process counter cleared by design). - `uv run pytest tests/unit/test_rate_limit.py -v --no-cov` → 10 passed - `uv run pytest --cov=app --cov-report=term-missing` → 1652 passed, **TOTAL 99%** (>90%; rate_limit.py 100%, auth.py 100%) - `uv run pytest tests/e2e/test_smoke.py -v --no-cov` (isolation) → 3 passed - `uv run ruff check . && uv run pyright` → All checks passed / 0 errors, 0 warnings - Completion criteria: all met, except commit + phase-dir move — per harness rules I left all changes uncommitted in the working tree (harness commits atomically and moves the phase). - Diff scope: exactly `app/core/rate_limit.py`, `app/api/auth.py`, `tests/unit/test_rate_limit.py`, `tests/integration/test_auth_api.py` + phase files; `pyproject.toml` / `uv.lock` / `frontend/` untouched. - Deviations: none in code; commit/move deferred to harness as instructed. - Next pending phase: `82_security_headers`.
140 lines
5.5 KiB
Python
140 lines
5.5 KiB
Python
"""Sliding-window rate limiting for failed sign-ins (phase 81, audit SEC-03).
|
|
|
|
Audit basis
|
|
-----------
|
|
SEC-03 (security audit 2026-09-07, severity Medium): neither ``POST
|
|
/api/login`` nor ``POST /api/token-auth`` throttled failed attempts —
|
|
the password is compared in constant time (``secrets.compare_digest``),
|
|
so an attacker who can reach ``:8000`` could guess at line rate (audit
|
|
PoC: 10 000 sequential logins, all answered at line rate). This module
|
|
is the fix: a per-client-IP sliding window over FAILED sign-in
|
|
attempts only.
|
|
|
|
Threshold (owner decision A2 — module constants, NOT ``BOR_`` settings:
|
|
a security control with a fixed, tested default and no env surface to
|
|
mistype): ``MAX_FAILURES = 10`` failed attempts per client IP within
|
|
``WINDOW_SECONDS = 900`` (15 min). The window slides — only failures
|
|
within the last ``WINDOW_SECONDS`` count. The IP is blocked while its
|
|
in-window failure count is >= ``MAX_FAILURES``: the 10th failure is
|
|
recorded, and the 11th attempt is the first 429 (the routes' pre-check
|
|
runs BEFORE the attempt).
|
|
|
|
Shared counter
|
|
--------------
|
|
The counter is SHARED by both login routes (``/api/login`` and
|
|
``/api/token-auth``): any auth failure from an IP counts, so a
|
|
token-spraying attack cannot dodge the window by alternating routes,
|
|
and a SUCCESS on either route clears that IP's counter (a legitimate
|
|
owner who fat-fingers twice is not poisoned). The routes do the
|
|
bookkeeping (pre-check / record / reset); this module only knows about
|
|
failure timestamps.
|
|
|
|
Client IP
|
|
---------
|
|
Callers pass ``request.client.host`` — the DIRECT peer address (owner
|
|
decision A4). Assumption locked in the phase: the app is served
|
|
directly in the homelab, with NO reverse proxy in front. If a proxy is
|
|
ever put in front, this module would need ``X-Forwarded-For`` handling
|
|
(out of scope now).
|
|
|
|
Fail-open
|
|
---------
|
|
The limiter is a friction bump, not the security boundary (owner
|
|
decision A1 — in-memory, per-process, stdlib-only; the counter is lost
|
|
on restart, an accepted reset). A limiter bug must NEVER lock the
|
|
owner out: any internal error (a corrupted state entry, a clock
|
|
anomaly) makes ``remaining_wait`` return 0 (allow) and
|
|
``record_failure`` drop the count silently. Over-denial is the one
|
|
outcome this module is forbidden to produce.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import math
|
|
import threading
|
|
import time
|
|
from collections import deque
|
|
|
|
#: An IP is blocked while it has at least this many failed sign-ins in
|
|
#: the window (owner decision A2: 10 attempts per 15 minutes).
|
|
MAX_FAILURES = 10
|
|
#: Sliding-window length in seconds (15 minutes).
|
|
WINDOW_SECONDS = 900
|
|
|
|
#: Failed-attempt timestamps per client IP, oldest first. Module-global
|
|
#: on purpose (A1: per-process state, no new service or table); the
|
|
#: lock keeps the dict honest if the event loop is ever shared.
|
|
_failures: dict[str, deque[float]] = {}
|
|
_lock = threading.Lock()
|
|
|
|
|
|
def _prune(dq: deque[float], now: float) -> None:
|
|
"""Drop failures that have slid out of the window (oldest first).
|
|
|
|
A timestamp exactly ``WINDOW_SECONDS`` old is expired: the window
|
|
holds the last ``WINDOW_SECONDS`` strictly.
|
|
"""
|
|
cutoff = now - WINDOW_SECONDS
|
|
while dq and dq[0] <= cutoff:
|
|
dq.popleft()
|
|
|
|
|
|
def record_failure(client_ip: str) -> None:
|
|
"""Count one failed sign-in attempt from ``client_ip``.
|
|
|
|
Appends ``time.monotonic()`` to the IP's deque and prunes expired
|
|
entries. NEVER raises (fail-open): any internal error — including a
|
|
corrupted, non-deque state entry — drops the count silently instead
|
|
of risk blocking the owner. A corrupt entry is left in place (and
|
|
treated as "not blocked" by ``remaining_wait``) until ``reset`` or a
|
|
process restart clears it.
|
|
"""
|
|
try:
|
|
with _lock:
|
|
now = time.monotonic()
|
|
dq = _failures.get(client_ip)
|
|
if dq is None:
|
|
dq = deque()
|
|
_failures[client_ip] = dq
|
|
elif not isinstance(dq, deque):
|
|
return # corrupt state → drop this count (fail-open)
|
|
dq.append(now)
|
|
_prune(dq, now)
|
|
except Exception:
|
|
return # a limiter bug must never break the login route
|
|
|
|
|
|
def remaining_wait(client_ip: str) -> int:
|
|
"""Seconds until ``client_ip`` may attempt again (0 = allowed now).
|
|
|
|
Prunes expired failures; if fewer than ``MAX_FAILURES`` remain in
|
|
the window the IP is allowed (0). Otherwise returns the WHOLE-window
|
|
wait — ``ceil(WINDOW_SECONDS - (now - oldest))``, clamped to >= 1 —
|
|
i.e. until the oldest counted failure expires (the value the routes
|
|
put in ``Retry-After``; the window, not any single failure's age,
|
|
governs). NEVER raises (fail-open → 0).
|
|
"""
|
|
try:
|
|
with _lock:
|
|
now = time.monotonic()
|
|
dq = _failures.get(client_ip)
|
|
if not isinstance(dq, deque) or not dq:
|
|
return 0 # unknown IP or corrupt state → allow (fail-open)
|
|
_prune(dq, now)
|
|
if len(dq) < MAX_FAILURES:
|
|
return 0 # includes the all-expired (empty) case
|
|
wait = WINDOW_SECONDS - (now - dq[0])
|
|
return max(1, math.ceil(wait))
|
|
except Exception:
|
|
return 0 # a limiter bug must never deny the owner
|
|
|
|
|
|
def reset(client_ip: str) -> None:
|
|
"""Clear ``client_ip``'s failure count (called on a SUCCESS).
|
|
|
|
A successful sign-in on either route unblocks the IP immediately —
|
|
a legitimate owner who fat-fingers twice is not poisoned by their
|
|
own earlier misses. Unknown IPs are a silent no-op.
|
|
"""
|
|
with _lock:
|
|
_failures.pop(client_ip, None)
|