Files
school_compare/backend/tests/test_rate_limit_key.py
T

147 lines
5.7 KiB
Python
Raw Normal View History

"""The rate-limit bucket must be the caller, not the proxy in front of them.
`get_remote_address` reads request.client.host. In staging and production the
backend has no published ports and its only caller is the Next proxy, so that
host is the Next container — one bucket for every browser user on the site.
Measured before this fix: 70 concurrent requests, 60 served and 10 refused.
"""
from starlette.datastructures import Headers
from backend.app import client_key
class _Req:
"""Enough of a Request for the key function: headers and a client host."""
def __init__(self, headers: dict, host: str = "10.0.0.9"):
self.headers = Headers(headers)
self.client = type("C", (), {"host": host})()
self.scope = {"type": "http", "client": (host, 0),
"headers": [(k.lower().encode(), v.encode())
for k, v in headers.items()]}
def test_cloudflare_header_wins():
# Cloudflare sets CF-Connecting-IP and overwrites any client-supplied
# value, so it is trustworthy in a way a parsed XFF chain is not.
assert client_key(_Req({"cf-connecting-ip": "203.0.113.7"})) == "203.0.113.7"
def test_forwarded_for_is_the_fallback_and_takes_the_first_entry():
# Left-most is the original client; everything after it is proxies.
assert client_key(
_Req({"x-forwarded-for": "203.0.113.7, 10.0.0.2"})) == "203.0.113.7"
def test_remote_address_is_the_last_resort():
assert client_key(_Req({}, host="10.0.0.9")) == "10.0.0.9"
def test_cloudflare_header_beats_forwarded_for():
key = client_key(_Req({"cf-connecting-ip": "203.0.113.7",
"x-forwarded-for": "198.51.100.1"}))
assert key == "203.0.113.7"
def test_two_callers_get_two_buckets():
# The whole point: one user exhausting their limit must not refuse another.
a = client_key(_Req({"cf-connecting-ip": "203.0.113.7"}))
b = client_key(_Req({"cf-connecting-ip": "203.0.113.8"}))
assert a != b
def test_whitespace_is_stripped():
# "a, b" split on comma leaves a leading space on every entry but the
# first; an unstripped key silently creates a second bucket per client.
assert client_key(_Req({"x-forwarded-for": " 203.0.113.7 ,10.0.0.2"})) \
== "203.0.113.7"
# ---------------------------------------------------------------------------
# The ceiling that header rotation cannot raise.
# ---------------------------------------------------------------------------
import pytest
from fastapi.testclient import TestClient
@pytest.fixture()
def api(monkeypatch):
from backend import app as app_module
from backend.config import settings
monkeypatch.setattr(settings, "global_rate_limit_per_minute", 5)
monkeypatch.setattr(app_module, "_global_window", None)
return TestClient(app_module.app, raise_server_exceptions=False)
def _ceiling_req(path: str, host: str):
"""Enough of a Request for exempt_from_ceiling: a path and a peer host."""
return type("R", (), {
"url": type("U", (), {"path": path})(),
"client": type("C", (), {"host": host})(),
})()
def _get(client, path="/api/flags", cf=None):
headers = {"cf-connecting-ip": cf} if cf else {}
return client.get(path, headers=headers)
def test_rotating_the_cloudflare_header_cannot_buy_unlimited_requests(api):
"""The attack the per-client keying opened up.
client_key trusts CF-Connecting-IP, and nothing in this process can tell an
edge-set header from an attacker-set one — that distinction can only be
made at Cloudflare, with Authenticated Origin Pulls or an origin firewall.
A caller reaching the origin directly can therefore mint a fresh
rate-limit bucket per request and evade per-client limits entirely.
Per-client fairness is still the right default; this is the backstop that
bounds what evading it can achieve. Without it, correct keying would be a
net regression against abuse compared with the shared bucket it replaced.
"""
codes = [_get(api, cf=f"203.0.113.{i}").status_code for i in range(8)]
assert codes.count(200) == 5
assert codes.count(429) == 3
def test_the_ceiling_says_which_limit_was_hit(api):
# Distinguishable from slowapi's per-client 429, or an operator reading
# logs cannot tell "one noisy client" from "the origin is saturated".
for i in range(5):
_get(api, cf=f"203.0.113.{i}")
refused = _get(api, cf="203.0.113.99")
assert refused.status_code == 429
assert "capacity" in refused.json()["detail"].lower()
assert refused.headers.get("retry-after")
def test_traffic_below_the_ceiling_is_untouched(api):
codes = [_get(api, cf=f"203.0.113.{i}").status_code for i in range(5)]
assert codes == [200] * 5
def test_the_container_healthcheck_is_exempt(api):
"""The healthcheck runs `curl http://localhost:80/api/data-info` inside the
container. If the ceiling could starve it, saturation would fail the
healthcheck, restart the container, and turn a load spike into an outage
loop — the ceiling has to protect the origin, not kill it.
"""
from backend.app import exempt_from_ceiling
assert exempt_from_ceiling(_ceiling_req("/api/data-info", "127.0.0.1"))
assert exempt_from_ceiling(_ceiling_req("/api/data-info", "::1"))
# Everyone else is counted.
assert not exempt_from_ceiling(_ceiling_req("/api/data-info", "10.0.0.9"))
def test_the_ceiling_ignores_non_api_paths():
# Sitemaps and robots.txt are served by this app too, and a crawler
# fetching them must not be refused because the API is busy.
from backend.app import exempt_from_ceiling
assert exempt_from_ceiling(_ceiling_req("/sitemap.xml", "10.0.0.9"))
assert exempt_from_ceiling(_ceiling_req("/robots.txt", "10.0.0.9"))