H1 (critical): reorder send_support_message — get_or_create_thread now runs AFTER a successful Telegram send, not before. Prod runs a single uvicorn process with no --workers on a sync SQLAlchemy engine sharing one event loop; holding a row-lock across the Telegram call (which can legally take minutes on 429/5xx worker-grade retries) risked freezing the entire API on a second concurrent request from the same user. send_message now also accepts explicit timeout/max_retries so the interactive endpoint uses a bounded budget instead of inheriting the long-polling worker's retry policy. M1: web_support_messages and tg_support_messages both gain a support_chat_id column (188 migration for the already-applied tg_support_messages table; 187 edited in place since it hasn't shipped yet). bridge._handle_group_reply now resolves BOTH the Telegram and web candidate under the CURRENT support_chat_id and refuses delivery loudly (logger.error) if both match, instead of silently preferring the Telegram path — closing a cross-table misroute risk that would surface if the support group is ever recreated. M2: a media reply to a web-mirrored message (including photos with a caption) is now refused in full with an explicit notice back in the topic, instead of silently delivering just the caption text or logging a WARNING nobody sees. M3: list_support_messages/get_support_unread/mark_support_read switched from async def to def — their bodies are pure sync psycopg calls; running them as async def executed blocking DB work directly on the event loop. M5: list_messages now takes a bounded LIMIT (last N, chronological) so a long-lived thread doesn't return its entire history on every poll. L1-L4: warn when Telegram doesn't return a message_id (routing dead-end), rate limit is peeked before send and only recorded on success (failed attempts no longer burn the budget), SlidingWindowLimiter gained the same empty-bucket cleanup as RateLimitMiddleware, and _require_username now strips whitespace so a proxy-injected space can't fork a second thread. L5: removed 187 from _manifest_applied.txt — the deploy pipeline tracks applied migrations via _schema_migrations, not this file, and keeping an unmerged migration name out of it preserves the option to rename before merge without tripping the "can't rename applied migrations" test.
169 lines
8.2 KiB
Python
169 lines
8.2 KiB
Python
"""Tests for RateLimitMiddleware — per-user / per-IP sliding window (#655, #2213).
|
||
|
||
Тестируем middleware в изоляции на минимальном FastAPI-приложении, чтобы не
|
||
тянуть тяжёлый app.main (DB / scheduler / sentry). Лимит/окно/множитель читаются
|
||
из settings на каждый dispatch → monkeypatch'им их в маленькие значения.
|
||
|
||
#2213: аутентифицированный трафик больше НЕ освобождается целиком — он лимитируется
|
||
per-user с щедрым порогом (rate_limit × multiplier). Анонимный ключ = client IP,
|
||
взятый из RIGHTMOST элемента X-Forwarded-For (ровно один доверенный прокси Caddy),
|
||
поэтому подделка leftmost XFF не меняет ключ.
|
||
"""
|
||
|
||
import os
|
||
|
||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||
|
||
import pytest
|
||
from fastapi import FastAPI
|
||
from fastapi.testclient import TestClient
|
||
|
||
from app.core import config
|
||
from app.core.ratelimit import RateLimitMiddleware, SlidingWindowLimiter
|
||
|
||
|
||
@pytest.fixture
|
||
def client(monkeypatch):
|
||
"""Минимальное приложение: анонимный лимит 3 / 60 с, auth-множитель ×2 (→6)."""
|
||
monkeypatch.setattr(config.settings, "rate_limit", 3)
|
||
monkeypatch.setattr(config.settings, "rate_limit_window_s", 60.0)
|
||
monkeypatch.setattr(config.settings, "rate_limit_authenticated_multiplier", 2)
|
||
|
||
app = FastAPI()
|
||
app.add_middleware(RateLimitMiddleware)
|
||
|
||
@app.get("/api/v1/ping")
|
||
def ping() -> dict[str, bool]:
|
||
return {"ok": True}
|
||
|
||
return TestClient(app)
|
||
|
||
|
||
def test_anonymous_throttled_past_limit(client):
|
||
# Первые rate_limit запросов проходят, следующий — 429.
|
||
for _ in range(3):
|
||
assert client.get("/api/v1/ping").status_code == 200
|
||
resp = client.get("/api/v1/ping")
|
||
assert resp.status_code == 429
|
||
assert resp.json() == {"detail": "Слишком много запросов. Попробуйте позже."}
|
||
assert "Retry-After" in resp.headers
|
||
|
||
|
||
def test_authenticated_user_is_limited_per_user(client):
|
||
# #2213: authenticated НЕ освобождается — лимит = rate_limit × multiplier = 6.
|
||
headers = {"X-Authenticated-User": "alice"}
|
||
for _ in range(6):
|
||
assert client.get("/api/v1/ping", headers=headers).status_code == 200
|
||
# 7-й запрос того же юзера — 429.
|
||
assert client.get("/api/v1/ping", headers=headers).status_code == 429
|
||
|
||
|
||
def test_authenticated_limit_higher_than_anonymous(client):
|
||
# Auth-порог (6) строго выше анонимного (3): 4-й запрос юзера ещё проходит,
|
||
# тогда как анонимный на 4-м уже режется (см. test_anonymous_throttled_past_limit).
|
||
headers = {"X-Authenticated-User": "alice"}
|
||
for _ in range(4):
|
||
assert client.get("/api/v1/ping", headers=headers).status_code == 200
|
||
|
||
|
||
def test_per_user_key_isolation(client):
|
||
# Разные юзеры — независимые корзины: alice упирается, bob не затронут.
|
||
alice = {"X-Authenticated-User": "alice"}
|
||
bob = {"X-Authenticated-User": "bob"}
|
||
for _ in range(6):
|
||
assert client.get("/api/v1/ping", headers=alice).status_code == 200
|
||
assert client.get("/api/v1/ping", headers=alice).status_code == 429
|
||
# bob со своим ключом проходит свободно.
|
||
assert client.get("/api/v1/ping", headers=bob).status_code == 200
|
||
|
||
|
||
def test_empty_auth_header_does_not_bypass(client):
|
||
# Пустой заголовок не должен no-op'ить лимит (header present но falsy) → per-IP.
|
||
headers = {"X-Authenticated-User": ""}
|
||
for _ in range(3):
|
||
assert client.get("/api/v1/ping", headers=headers).status_code == 200
|
||
assert client.get("/api/v1/ping", headers=headers).status_code == 429
|
||
|
||
|
||
def test_leftmost_xff_does_not_affect_anon_key(client):
|
||
# Клиент подделывает leftmost XFF, но rightmost (добавленный Caddy) одинаков →
|
||
# один ключ ip:9.9.9.9 → лимит общий, подделкой его не обойти.
|
||
spoofed = [
|
||
{"X-Forwarded-For": "1.1.1.1, 9.9.9.9"},
|
||
{"X-Forwarded-For": "2.2.2.2, 9.9.9.9"},
|
||
{"X-Forwarded-For": "3.3.3.3, 9.9.9.9"},
|
||
]
|
||
for h in spoofed:
|
||
assert client.get("/api/v1/ping", headers=h).status_code == 200
|
||
# 4-й (снова другой leftmost, тот же rightmost) — уже за лимитом.
|
||
resp = client.get("/api/v1/ping", headers={"X-Forwarded-For": "4.4.4.4, 9.9.9.9"})
|
||
assert resp.status_code == 429
|
||
|
||
|
||
def test_anonymous_key_from_rightmost_xff(client):
|
||
# Разный rightmost = разные реальные клиенты = независимые корзины.
|
||
for _ in range(3):
|
||
assert (
|
||
client.get("/api/v1/ping", headers={"X-Forwarded-For": "8.8.8.8, 1.1.1.1"}).status_code
|
||
== 200
|
||
)
|
||
# Другой rightmost — своя корзина, не затронут лимитом первого.
|
||
assert (
|
||
client.get("/api/v1/ping", headers={"X-Forwarded-For": "8.8.8.8, 2.2.2.2"}).status_code
|
||
== 200
|
||
)
|
||
|
||
|
||
# ── SlidingWindowLimiter (#tgsupport-web) — reusable narrower per-feature limit ──
|
||
|
||
|
||
def test_sliding_window_limiter_retry_after_does_not_record():
|
||
"""`.retry_after()` — non-destructive peek: не расходует бюджет сам по себе
|
||
(review L3 — вызывающая сторона решает, когда `.record()`)."""
|
||
limiter = SlidingWindowLimiter(limit=1, window_s=60.0)
|
||
assert limiter.retry_after("alice") is None
|
||
# Повторный peek БЕЗ record() — бюджет не тронут, всё ещё None.
|
||
assert limiter.retry_after("alice") is None
|
||
|
||
|
||
def test_sliding_window_limiter_record_then_retry_after_blocks():
|
||
limiter = SlidingWindowLimiter(limit=1, window_s=60.0)
|
||
assert limiter.retry_after("alice") is None
|
||
limiter.record("alice")
|
||
retry_after = limiter.retry_after("alice")
|
||
assert retry_after is not None
|
||
assert retry_after > 0
|
||
|
||
|
||
def test_sliding_window_limiter_check_combines_peek_and_record():
|
||
"""`.check()` — комбинированный (обратной совместимости ради) peek+record."""
|
||
limiter = SlidingWindowLimiter(limit=1, window_s=60.0)
|
||
assert limiter.check("alice") is None # первый — проходит и сразу учитывается
|
||
assert limiter.check("alice") is not None # второй — уже за лимитом
|
||
|
||
|
||
def test_sliding_window_limiter_per_key_isolation():
|
||
limiter = SlidingWindowLimiter(limit=1, window_s=60.0)
|
||
limiter.record("alice")
|
||
assert limiter.retry_after("alice") is not None
|
||
assert limiter.retry_after("bob") is None # свой ключ — не задет alice
|
||
|
||
|
||
def test_sliding_window_limiter_prunes_empty_buckets_past_threshold():
|
||
"""review L2: пустые корзины чистятся при накоплении >10000 ключей (тот же
|
||
паттерн, что `RateLimitMiddleware.dispatch`) — не бесконечная утечка памяти.
|
||
|
||
`retry_after()`-peek на новом ключе создаёт ПУСТУЮ корзину как побочный эффект
|
||
`defaultdict` (даже если вызывающая сторона так и не позвала `.record()` —
|
||
напр. запрос отклонён по другой причине выше по стеку). Это главный источник
|
||
"мусорных" пустых корзин, которые cleanup обязан подбирать.
|
||
"""
|
||
limiter = SlidingWindowLimiter(limit=1000, window_s=60.0)
|
||
for i in range(10001):
|
||
limiter.retry_after(f"user-{i}")
|
||
assert len(limiter._hits) == 10001
|
||
|
||
# Следующий record() создаёт СВОЮ (непустую) корзину и заодно подчищает
|
||
# все чужие пустые — тот же порог (>10000), что и RateLimitMiddleware.
|
||
limiter.record("trigger-cleanup")
|
||
assert len(limiter._hits) < 10001
|