* security(auth): per-IP rate limit on auth endpoints + generalize last-admin guard Closes #45 and #151. #45 — every auth endpoint was unthrottled (login, magic-link, token, bootstrap), leaving us open to password brute-force and SMTP email-bombing. Wires slowapi (new dep) into the middleware chain with per-route limits: 10/min on login + token, 5/min on send-link, 3/min on bootstrap. Returns 429 with Retry-After: 60 once exceeded. Per-IP key respects the leftmost X-Forwarded-For hop (Caddy in front of the app strips client-supplied XFF). Operator escape hatch: AGNES_AUTH_RATELIMIT_ENABLED=0. Test suite disables the limiter via autouse conftest fixture so existing auth tests that hammer endpoints in tight loops are unaffected. #151 — DELETE /api/admin/users/{id}/memberships/{group_id} and the mirror DELETE /api/admin/groups/{group_id}/members/{user_id} only guarded against self-removal as last admin. Generalizes to refuse removing anyone from the seeded Admin group when they are the only remaining active admin (mirrors the existing count_admins(active_only=True) <= 1 check on delete_user / update_user). Recovery from zero admins requires direct DB access, so this closes a path where a scheduler/bootstrap actor that bypasses normal admin checks could otherwise empty the group. * security(auth): throttle remaining email-bombing + token-confirm endpoints Address code-review gap on PR #165 — the first commit covered /send-link but missed two endpoints with the IDENTICAL email-bombing surface: - POST /auth/password/reset — sends reset mail, anti-enum response - POST /auth/password/setup/request — sends setup mail, anti-enum response Both now share the 5/min limit with /send-link. Also add 10/min to the token-confirm surfaces — high-entropy tokens but partial leaks via logs / referer have surfaced before, and unbounded guess rate would let an attacker exhaust the keyspace adjacent to a leaked prefix: - POST /auth/email/verify - GET /auth/email/verify — closes the click-through bypass - POST /auth/password/reset/confirm - POST /auth/password/setup/confirm Doc fix: rate_limit.py module docstring + CHANGELOG entry no longer claim "disable without a redeploy" (misleading). The Limiter constructor freezes `enabled` from env at import time, matching every other Agnes env knob — operators set the flag and bounce the container. Tests: 4 new cases in test_auth_rate_limit.py covering /reset, /setup/request, /reset/confirm, GET /verify. Full suite: 2583 passed, 32 skipped, 0 failed. * security(auth): throttle JSON /auth/password/setup — closes form-throttle bypass Second code-review pass on PR #165 caught a fifth gap: POST /auth/password/setup (JSON variant, kept for backward compat) consumes the same setup_token as the web form /setup/confirm but was unthrottled — an attacker brute-forcing the token just switches from the form path to the JSON path and resumes at unbounded RPS. Apply the same 10/min limit and signature shape used on /setup/confirm. Also extend CHANGELOG note about the JSON-variant bypass for future operators reading the security entry. Test: 1 new case (test_password_setup_json_rate_limited_after_10_requests), 9 rate-limit tests + 28 password-flow tests + 41 auth-provider tests pass, no regressions. * chore(release): cut 0.30.1 — auth security hardening (rate limit + last-admin guard)
101 lines
3.9 KiB
Python
101 lines
3.9 KiB
Python
"""Per-IP rate limiting for auth endpoints (#45).
|
|
|
|
Why: every auth endpoint was unthrottled before this module — `grep -r
|
|
"slowapi\\|limiter\\|throttle"` returned zero hits in app/. That left
|
|
``/auth/password/login`` and ``/auth/token`` open to password brute-force
|
|
and ``/auth/email/send-link`` open to SMTP/SendGrid email-bombing
|
|
(attacker loops with random recipients and burns through quota).
|
|
|
|
How: slowapi installs a starlette middleware that rejects with 429 when
|
|
the per-route ``@limiter.limit("N/period")`` decorator is exceeded. The
|
|
key is the client IP, taken from the leftmost X-Forwarded-For hop (Caddy
|
|
in front of the app strips client-supplied XFF and sets its own — same
|
|
trust model as ``app.auth.dependencies._client_ip``).
|
|
|
|
Operator override: set ``AGNES_AUTH_RATELIMIT_ENABLED=0`` and restart
|
|
the process (no image rebuild needed — flip the env in the compose
|
|
``.env`` / systemd unit and bounce the container). The value is read at
|
|
process start because the slowapi ``Limiter`` constructor freezes
|
|
``enabled`` at import; that limitation is fine in practice because
|
|
Agnes's other env knobs already require a process restart to take
|
|
effect (see ``.env_overlay`` loader in ``app/main.py`` for the same
|
|
shape — file-based overlay merged at startup, no live reload).
|
|
|
|
The test suite flips ``limiter.enabled`` directly via an autouse
|
|
conftest fixture (no restart required because tests share a process)
|
|
and re-enables only inside the dedicated rate-limit test, so
|
|
generous-but-finite limits don't bleed into other test files that
|
|
hammer auth endpoints in tight loops.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
|
|
from slowapi import Limiter
|
|
from slowapi.errors import RateLimitExceeded
|
|
from slowapi.middleware import SlowAPIMiddleware
|
|
from slowapi.util import get_remote_address
|
|
from starlette.requests import Request
|
|
from starlette.responses import JSONResponse
|
|
|
|
|
|
def _client_ip_key(request: Request) -> str:
|
|
"""IP key, preferring leftmost X-Forwarded-For hop.
|
|
|
|
Mirrors ``app.auth.dependencies._client_ip`` — same Caddy-in-front
|
|
trust model. If the app is ever exposed directly to the internet
|
|
without a proxy, the XFF header becomes client-settable and an
|
|
attacker can rotate the per-IP bucket trivially. Document that
|
|
deployment shape in the runbook before flipping it on.
|
|
"""
|
|
xff = request.headers.get("x-forwarded-for")
|
|
if xff:
|
|
ip = xff.split(",", 1)[0].strip()
|
|
if ip:
|
|
return ip
|
|
return get_remote_address(request)
|
|
|
|
|
|
def _enabled_default() -> bool:
|
|
return os.environ.get("AGNES_AUTH_RATELIMIT_ENABLED", "1").lower() not in (
|
|
"0", "false", "no", "off",
|
|
)
|
|
|
|
|
|
# Module-level singleton — slowapi binds storage at construction and the
|
|
# decorators capture this exact instance at import time. Tests toggle
|
|
# ``limiter.enabled`` and call ``limiter.reset()`` between cases.
|
|
#
|
|
# headers_enabled is intentionally OFF: when on, slowapi injects
|
|
# X-RateLimit-* headers via a per-handler response parameter, which forces
|
|
# every decorated endpoint to add ``response: Response`` even on the happy
|
|
# path. The protection here is the 429 with Retry-After (still emitted by
|
|
# the exception handler below) — the diagnostic headers on success
|
|
# responses are not worth the API-shape churn across 5 endpoints.
|
|
limiter = Limiter(
|
|
key_func=_client_ip_key,
|
|
enabled=_enabled_default(),
|
|
headers_enabled=False,
|
|
default_limits=[],
|
|
)
|
|
|
|
|
|
async def _rate_limit_exceeded_handler(request: Request, exc: RateLimitExceeded) -> JSONResponse:
|
|
"""Match Agnes's existing JSON error shape (``{"detail": "..."}``)
|
|
instead of slowapi's text/plain default — keeps the CLI / web error
|
|
parser uniform across all 4xx responses.
|
|
"""
|
|
return JSONResponse(
|
|
{"detail": f"Too many requests — {exc.detail}"},
|
|
status_code=429,
|
|
headers={"Retry-After": "60"},
|
|
)
|
|
|
|
|
|
__all__ = [
|
|
"limiter",
|
|
"RateLimitExceeded",
|
|
"SlowAPIMiddleware",
|
|
"_rate_limit_exceeded_handler",
|
|
]
|