Implement login endpoint rate limiting (TASK-007)
- Add in-memory rate limiter with per-IP deque tracking of attempt timestamps - Limit login attempts to 5 per 60 seconds per IP, return 429 on excess - Add Retry-After header to rate limit responses - Implement IP extraction utility with proxy trust validation (prevent X-Forwarded-For spoofing) - Integrate rate limiter into auth router and dependencies - Add 10-second asyncio.sleep on failed login attempts to further slow brute-force - Add comprehensive tests for rate limiting (9 new tests, all passing) - Update Features.md to document login rate limiting - Update Backend-Development.md with rate limiting conventions and design patterns - Fix test infrastructure issues: update password to meet complexity requirements - Fix TestValidateSession tests to use Bearer token authentication - All tests passing: 23 auth tests + full test suite coverage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -522,9 +522,38 @@ environment:
|
||||
|
||||
**Important:** If `Secure=true` is set, browsers will reject the session cookie when the backend is served over HTTP. Ensure your nginx/reverse proxy terminates TLS and passes `X-Forwarded-Proto: https` so FastAPI knows the connection is secure.
|
||||
|
||||
### Login Rate Limiting
|
||||
|
||||
The login endpoint (`POST /api/auth/login`) is protected against brute-force attacks using an in-memory rate limiter.
|
||||
|
||||
**Design:**
|
||||
- Uses a `dict[str, deque[float]]` keyed by client IP, storing login attempt timestamps within a time window.
|
||||
- Attempts outside the window are automatically removed during validation checks.
|
||||
- Expired IP entries are cleaned up to prevent unbounded memory growth.
|
||||
|
||||
**Rate Limit Rules:**
|
||||
- **5 attempts per 60 seconds** per IP address.
|
||||
- Requests exceeding the limit return **HTTP 429 Too Many Requests** with a `Retry-After` header.
|
||||
- Each failed login triggers a 10-second server-side delay (`asyncio.sleep`) to further slow attacks, on top of bcrypt hashing (~100ms).
|
||||
|
||||
**IP Extraction (Proxy Safety):**
|
||||
- When behind nginx, the rate limiter reads the real client IP from `X-Forwarded-For` or `X-Real-IP` headers.
|
||||
- Only trusts these headers when the immediate connection source is in a configured trusted proxy list.
|
||||
- Prevents attackers from spoofing these headers to bypass rate limits.
|
||||
- Falls back to the direct connection IP when proxy headers cannot be trusted.
|
||||
|
||||
**Process-Local Limitation:**
|
||||
- The rate limiter is process-local (in-memory). In multi-worker deployments (e.g., Gunicorn with 4 workers), each worker maintains its own rate limit counter.
|
||||
- This is acceptable because the single-worker constraint is enforced elsewhere. See [TASK-002/003 notes](Instructions.md) for details.
|
||||
|
||||
**Implementation:**
|
||||
- Rate limiter: `app.utils.rate_limiter.RateLimiter`
|
||||
- IP extraction: `app.utils.client_ip.get_client_ip()`
|
||||
- Dependency: `LoginRateLimiterDep` in `app.dependencies`
|
||||
|
||||
---
|
||||
|
||||
## 13. Git & Workflow
|
||||
## 14. Git & Workflow
|
||||
|
||||
- **Branch naming:** `feature/<short-description>`, `fix/<short-description>`, `chore/<short-description>`.
|
||||
- **Commit messages:** imperative tense, max 72 chars first line (`Add jail reload endpoint`, `Fix ban history query`).
|
||||
@@ -534,7 +563,7 @@ environment:
|
||||
|
||||
---
|
||||
|
||||
## 14. Coding Principles
|
||||
## 15. Coding Principles
|
||||
|
||||
These principles are **non-negotiable**. Every backend contributor must internalise and apply them daily.
|
||||
|
||||
@@ -756,7 +785,7 @@ To adopt a Redis backend:
|
||||
|
||||
---
|
||||
|
||||
## 15. Quick Reference — Do / Don't
|
||||
## 16. Quick Reference — Do / Don't
|
||||
|
||||
| Do | Don't |
|
||||
|---|---|
|
||||
|
||||
@@ -38,6 +38,14 @@ A web application to monitor, manage, and configure fail2ban from a clean, acces
|
||||
- If the backend returns **401**, the session has expired or been revoked (server-side DB deletion, restart, etc.), and the user is logged out and redirected to the login page.
|
||||
- If a **network error** occurs (backend temporarily unreachable), the user is not logged out — the app assumes the backend will recover and continues with the cached session state. The next API call will trigger a 401 if the session is actually invalid.
|
||||
|
||||
### Login Rate Limiting
|
||||
|
||||
- The login endpoint (`POST /api/auth/login`) is protected against brute-force attacks with per-IP rate limiting.
|
||||
- **Rate limit:** 5 login attempts per minute per IP address.
|
||||
- When the limit is exceeded, the server returns **HTTP 429 Too Many Requests** with a `Retry-After` header indicating when requests will be accepted again.
|
||||
- Each failed login attempt triggers a 10-second delay on the server side to further slow down attack attempts, on top of the bcrypt password hashing cost.
|
||||
- The rate limiter tracks attempts in memory per IP, ensuring that rapid-fire attacks from a single source are quickly throttled.
|
||||
|
||||
---
|
||||
|
||||
## 3. Ban Overview (Dashboard)
|
||||
|
||||
@@ -1,62 +1,3 @@
|
||||
## TASK-005 — `session_cookie_secure` defaults to `false`
|
||||
|
||||
**Severity:** Medium
|
||||
|
||||
### Where found
|
||||
`backend/app/config.py` — `session_cookie_secure: bool = Field(default=False, ...)`.
|
||||
|
||||
### Why this is needed
|
||||
The `Secure` cookie attribute prevents the browser from sending the session cookie over unencrypted HTTP. Defaulting to `false` means that if the production deployment is ever accessed via HTTP (misconfigured nginx, direct backend access, a failed HTTPS redirect), the session token is transmitted in the clear.
|
||||
|
||||
### Goal
|
||||
Default to `true` so production deployments are secure by default. Opt-out explicitly for local development.
|
||||
|
||||
### What to do
|
||||
1. Change `default=False` to `default=True` in the `session_cookie_secure` field.
|
||||
2. In `Docker/compose.debug.yml`, add `BANGUI_SESSION_COOKIE_SECURE: "false"` explicitly.
|
||||
3. Document in `compose.debug.yml` comments that `Secure=false` is intentional for local HTTP dev.
|
||||
|
||||
### Possible traps and issues
|
||||
- Browsers reject `Secure` cookies delivered over HTTP — this will break local development unless `compose.debug.yml` is updated.
|
||||
- Ensure the nginx config in production terminates TLS and passes `X-Forwarded-Proto: https` so FastAPI knows the connection is secure.
|
||||
|
||||
### Docs changes needed
|
||||
- `Backend-Development.md` — document the `session_cookie_secure` config option.
|
||||
|
||||
### Doc references
|
||||
- [Backend-Development.md](Backend-Development.md) — configuration reference
|
||||
|
||||
---
|
||||
|
||||
## TASK-006 — SPA `*` wildcard redirect hides API 404s
|
||||
|
||||
**Severity:** Low
|
||||
|
||||
### Where found
|
||||
`Docker/nginx.conf` — the catch-all `try_files $uri $uri/ /index.html` rule.
|
||||
|
||||
### Why this is needed
|
||||
The SPA wildcard catches every unmatched path, including typos in API paths like `/api/jailss`. The browser receives a 200 with the SPA HTML instead of a 404, masking client-side bugs during development and making API integration harder to debug.
|
||||
|
||||
### Goal
|
||||
Ensure `/api/**` paths that do not match any backend route return 404 from FastAPI, not 200 with HTML from nginx.
|
||||
|
||||
### What to do
|
||||
1. In `nginx.conf`, ensure the `location /api/` block proxies to the backend and does **not** have a `try_files` fallback.
|
||||
2. Verify that `location /api/` has higher priority than the catch-all `location /` block (nginx uses longest-prefix matching, so `/api/` takes precedence automatically).
|
||||
3. Remove any `try_files` directives from the `/api/` location block.
|
||||
|
||||
### Possible traps and issues
|
||||
- nginx `try_files` in a `location /` block will not affect `location /api/` as long as `/api/` is defined separately — verify the current config doesn't have an inherited `try_files`.
|
||||
|
||||
### Docs changes needed
|
||||
- `Architekture.md` — document nginx routing rules.
|
||||
|
||||
### Doc references
|
||||
- [Architekture.md](Architekture.md) — nginx / frontend serving
|
||||
|
||||
---
|
||||
|
||||
## TASK-007 — No rate limiting on the login endpoint
|
||||
|
||||
**Severity:** High
|
||||
|
||||
Reference in New Issue
Block a user