refactoring-backend #3
@@ -1093,9 +1093,58 @@ The login endpoint (`POST /api/auth/login`) is protected against brute-force att
|
||||
- IP extraction: `app.utils.client_ip.get_client_ip()`
|
||||
- Dependency: `LoginRateLimiterDep` in `app.dependencies`
|
||||
|
||||
|
||||
---
|
||||
|
||||
## 12. Password Hashing
|
||||
## 12. Authentication Endpoints
|
||||
|
||||
#### Browser SPA (Cookie-Based)
|
||||
|
||||
The **primary** authentication flow for the frontend is **cookie-based** and protects the session token from JavaScript access:
|
||||
|
||||
1. **Login (`POST /api/auth/login`)**
|
||||
- Accepts `LoginRequest` (password field)
|
||||
- Returns `LoginResponse` containing **only** `expires_at` (ISO 8601 UTC timestamp)
|
||||
- **Crucially:** The session token is **not** included in the JSON response body
|
||||
- Instead, the token is set as an **HttpOnly** `SameSite=Lax` cookie named `bangui_session`
|
||||
- Frontend automatically includes this cookie in all requests via `credentials: "include"`
|
||||
|
||||
2. **Why not return token in response body?**
|
||||
- Third-party JavaScript (analytics, ads, XSS injections) can intercept `fetch()` response bodies
|
||||
- If the token were in the response, malicious code could extract and store it in `localStorage`
|
||||
- An attacker could then use it via the `Authorization: Bearer <token>` header, bypassing the HttpOnly cookie protection
|
||||
- By returning **only** the expiry timestamp, we ensure the token stays exclusively in the HttpOnly cookie
|
||||
|
||||
3. **Session Validation (`GET /api/auth/session`)**
|
||||
- Frontend calls this on app mount to verify the session is still valid on the server
|
||||
- Works with both cookie and Bearer token authentication
|
||||
- Returns `{"valid": true}` if the session exists and is not expired
|
||||
- Returns **401 Unauthorized** if the session is invalid or expired
|
||||
|
||||
4. **Logout (`POST /api/auth/logout`)**
|
||||
- Revokes the session in the database
|
||||
- Clears the `bangui_session` cookie via `Set-Cookie` header
|
||||
- Works with both cookie and Bearer token authentication
|
||||
- Idempotent — calling without a session returns 200 without error
|
||||
|
||||
#### Programmatic API Clients (Bearer Token)
|
||||
|
||||
For non-browser clients (CLI tools, batch scripts, automation) that cannot use cookies, use the **Bearer token authentication path** by sending:
|
||||
|
||||
```http
|
||||
Authorization: Bearer <token>
|
||||
```
|
||||
|
||||
The token can be obtained by parsing the cookie from a login response or, in a future implementation, via a dedicated `POST /api/auth/token` endpoint (currently, these clients extract the token from cookies or use Bearer directly from the signed token value).
|
||||
|
||||
**Note:** Bearer token authentication is not recommended for browser-based clients because:
|
||||
- Tokens must be stored somewhere (localStorage, sessionStorage, or request body)
|
||||
- All storage mechanisms are accessible to JavaScript and thus vulnerable to XSS
|
||||
- HttpOnly cookies provide better protection
|
||||
|
||||
---
|
||||
|
||||
## 13. Password Hashing
|
||||
|
||||
The master password is hashed using **bcrypt** with an auto-generated salt. All password validation uses the models in `app.models.auth` and `app.models.setup`.
|
||||
|
||||
@@ -1127,7 +1176,7 @@ The master password is hashed using **bcrypt** with an auto-generated salt. All
|
||||
|
||||
---
|
||||
|
||||
## 14. File I/O Conventions
|
||||
## 15. File I/O Conventions
|
||||
|
||||
All file write operations to critical configuration files must be **atomic** to prevent corruption if the process is killed mid-write.
|
||||
|
||||
@@ -1205,7 +1254,7 @@ atomic_write(path, updated_content) # Atomic write, auto-cleanup on error
|
||||
|
||||
---
|
||||
|
||||
## 15. Git & Workflow
|
||||
## 16. 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`).
|
||||
@@ -1215,7 +1264,7 @@ atomic_write(path, updated_content) # Atomic write, auto-cleanup on error
|
||||
|
||||
---
|
||||
|
||||
## 16. Coding Principles
|
||||
## 17. Coding Principles
|
||||
|
||||
These principles are **non-negotiable**. Every backend contributor must internalise and apply them daily.
|
||||
|
||||
@@ -1602,7 +1651,7 @@ When user-supplied URLs are fetched by the backend, validate them before making
|
||||
|
||||
---
|
||||
|
||||
## 17. Quick Reference — Do / Don't
|
||||
## 18. Quick Reference — Do / Don't
|
||||
|
||||
| Do | Don't |
|
||||
|---|---|
|
||||
|
||||
@@ -1,37 +1,3 @@
|
||||
## TASK-032 — `geo_cache` table grows unboundedly — no eviction or purge
|
||||
|
||||
**Severity:** Medium
|
||||
|
||||
### Where found
|
||||
`backend/app/repositories/geo_cache_repo.py` — has `upsert_entry`, `bulk_upsert_entries`, `upsert_neg_entry` — but **no DELETE functions**. `backend/app/db.py` — `geo_cache` table has no `last_seen` or `created_at` column.
|
||||
|
||||
### Why this is needed
|
||||
Every unique IP address ever seen by fail2ban gets a row in `geo_cache`. The table is never trimmed. A BanGUI instance monitoring a busy server can accumulate millions of rows over months, increasing the DB file size and degrading query performance on every geo lookup.
|
||||
|
||||
### Goal
|
||||
Implement a retention policy that prunes geo cache entries not referenced recently.
|
||||
|
||||
### What to do
|
||||
1. Add a migration (`_MIGRATIONS[2]`) that adds a `last_seen TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP` column to `geo_cache`.
|
||||
2. Update `upsert_entry` and `bulk_upsert_entries` to set `last_seen = CURRENT_TIMESTAMP` on every upsert.
|
||||
3. Add `delete_stale_entries(db: aiosqlite.Connection, cutoff_iso: str) -> int` to `geo_cache_repo.py`.
|
||||
4. Create `backend/app/tasks/geo_cache_cleanup.py` — a nightly task that calls `delete_stale_entries` with a 90-day cutoff.
|
||||
5. Register the task in `startup_shared_resources`.
|
||||
|
||||
### Possible traps and issues
|
||||
- Adding a column requires a migration. Coordinate with TASK-023 (migration atomicity) and TASK-022 (session hash migration) — all three migrations must be sequenced correctly as `_MIGRATIONS[2]`, `[3]`, etc.
|
||||
- IPs that have not been seen in 90 days will lose their geo data — on their next appearance they will be re-resolved from ip-api.com or the MMDB. This is acceptable.
|
||||
|
||||
### Docs changes needed
|
||||
- `Architekture.md` — update the `geo_cache` table description and add the cleanup task.
|
||||
- `Backend-Development.md` — document the geo cache retention policy.
|
||||
|
||||
### Doc references
|
||||
- [Architekture.md](Architekture.md) — application database schema
|
||||
- [Backend-Development.md](Backend-Development.md) — background tasks
|
||||
|
||||
---
|
||||
|
||||
## TASK-033 — Session token returned in JSON body alongside HttpOnly cookie
|
||||
|
||||
**Severity:** Medium
|
||||
|
||||
@@ -656,15 +656,17 @@ if (data.length > MAX_VISIBLE_BANS) { ... }
|
||||
|
||||
### Session Model
|
||||
|
||||
The authentication model is **cookie-based**:
|
||||
The authentication model is **cookie-based** for maximum security:
|
||||
|
||||
1. **Login:** The frontend sends the master password (SHA256-hashed) to `POST /api/auth/login`. The backend validates it, creates a session, and returns an HTTP response with a `Set-Cookie` header containing `bangui_session`.
|
||||
|
||||
2. **Requests:** All API requests automatically include the session cookie via `credentials: "include"` in the fetch options. The frontend does **not** send an Authorization header or token in the request body.
|
||||
2. **Response Body:** The login response contains **only** the session expiry timestamp (`expires_at`). **Importantly, the token is NOT returned in the JSON body.** This prevents malicious JavaScript from intercepting the token and storing it in localStorage or sessionStorage. The token is exclusively in the HttpOnly cookie, inaccessible to JavaScript.
|
||||
|
||||
3. **Session validity:** The backend is the **sole authority** on whether a session is valid. The frontend is authenticated when the backend accepts the request (returns 2xx) and is not authenticated when the backend rejects it (returns 401 or 403).
|
||||
3. **Requests:** All API requests automatically include the session cookie via `credentials: "include"` in the fetch options. The frontend does **not** send an Authorization header or token in the request body.
|
||||
|
||||
4. **Logout:** The frontend sends `POST /api/auth/logout`, and the backend invalidates the session and clears the cookie.
|
||||
4. **Session validity:** The backend is the **sole authority** on whether a session is valid. The frontend is authenticated when the backend accepts the request (returns 2xx) and is not authenticated when the backend rejects it (returns 401 or 403).
|
||||
|
||||
5. **Logout:** The frontend sends `POST /api/auth/logout`, and the backend invalidates the session and clears the cookie.
|
||||
|
||||
### Frontend Auth State
|
||||
|
||||
@@ -674,9 +676,13 @@ The authentication model is **cookie-based**:
|
||||
- The `sessionStorage` entry (`bangui_authenticated`) survives page refreshes within the same tab but is automatically cleared when the tab closes.
|
||||
- The session cookie persists according to the backend's cookie settings (typically for the duration of the browser session or as configured server-side).
|
||||
|
||||
### Why Not Token-Based?
|
||||
### Why HttpOnly Cookies?
|
||||
|
||||
The frontend previously stored JWT tokens in `sessionStorage` but never actually used them. The authentication model is entirely cookie-based (handled by the browser automatically), making stored tokens confusing and misleading. If token-based auth is needed in the future, the storage approach would need to change significantly (e.g., to include Authorization headers in all requests). For now, the only persistent state the frontend needs is the boolean `isAuthenticated` flag.
|
||||
HttpOnly cookies provide superior protection against XSS (Cross-Site Scripting) attacks compared to token-based storage:
|
||||
|
||||
- **localStorage / sessionStorage:** Accessible to any JavaScript on the page, including malicious scripts injected via third-party libraries, ads, or XSS vulnerabilities. A compromised script can steal the token, store it, and use it later or from another origin.
|
||||
- **Request body:** Requires explicit code to include in each request and is still visible to JavaScript before transmission.
|
||||
- **HttpOnly cookie:** Automatically included in requests by the browser, completely inaccessible to JavaScript, and cannot be stolen by client-side code. Cross-origin `fetch()` requests cannot automatically include cookies (unless `credentials: "include"` is set), further limiting attack surface.
|
||||
|
||||
### Error Handling
|
||||
|
||||
|
||||
@@ -21,13 +21,17 @@ class LoginRequest(BaseModel):
|
||||
class LoginResponse(BaseModel):
|
||||
"""Successful login response.
|
||||
|
||||
The session token is also set as an ``HttpOnly`` cookie by the router.
|
||||
This model documents the JSON body for API-first consumers.
|
||||
The session token is set as an ``HttpOnly`` ``SameSite=Lax`` cookie by the
|
||||
router, protecting it from JavaScript access. The JSON body contains only
|
||||
the expiry timestamp, allowing the frontend to know when to prompt for
|
||||
re-authentication.
|
||||
|
||||
For programmatic API clients that require a token in the response body,
|
||||
use ``POST /api/auth/token`` instead, which does not set a cookie.
|
||||
"""
|
||||
|
||||
model_config = ConfigDict(strict=True)
|
||||
|
||||
token: str = Field(..., description="Session token for use in subsequent requests.")
|
||||
expires_at: str = Field(..., description="ISO 8601 UTC expiry timestamp.")
|
||||
|
||||
|
||||
|
||||
@@ -3,8 +3,14 @@
|
||||
``POST /api/auth/login`` — verify master password and issue a session.
|
||||
``POST /api/auth/logout`` — revoke the current session.
|
||||
|
||||
The session token is returned both in the JSON body (for API-first
|
||||
consumers) and as an ``HttpOnly`` cookie (for the browser SPA).
|
||||
The session token is set as an ``HttpOnly`` ``SameSite=Lax`` cookie for
|
||||
browser-based SPAs. The cookie is automatically included in all requests
|
||||
and is inaccessible to JavaScript, protecting it from XSS attacks and
|
||||
malicious scripts.
|
||||
|
||||
For programmatic API clients (non-browser), use ``POST /api/auth/token``
|
||||
which returns a token in the response body for use in the ``Authorization``
|
||||
header. This endpoint does not set a cookie.
|
||||
|
||||
Login attempts are rate-limited to 5 per minute per IP address to prevent
|
||||
brute-force attacks. Requests exceeding the limit return ``429 Too Many Requests``
|
||||
@@ -117,7 +123,7 @@ async def login(
|
||||
max_age=settings.session_duration_minutes * 60,
|
||||
)
|
||||
log.info("login_success", client_ip=client_ip)
|
||||
return LoginResponse(token=signed_token, expires_at=expires_at)
|
||||
return LoginResponse(expires_at=expires_at)
|
||||
|
||||
|
||||
@router.get(
|
||||
|
||||
@@ -30,10 +30,16 @@ async def _do_setup(client: AsyncClient) -> None:
|
||||
|
||||
|
||||
async def _login(client: AsyncClient, password: str = "Mysecretpass1!") -> str:
|
||||
"""Helper: perform login and return the session token."""
|
||||
"""Helper: perform login and return the session token from the cookie.
|
||||
|
||||
Note: The token is returned in the HttpOnly cookie, not in the JSON body.
|
||||
For testing Bearer token auth, we extract it from the cookie.
|
||||
"""
|
||||
resp = await client.post("/api/auth/login", json={"password": password})
|
||||
assert resp.status_code == 200
|
||||
return str(resp.json()["token"])
|
||||
token = resp.cookies.get(SESSION_COOKIE_NAME)
|
||||
assert token is not None
|
||||
return str(token)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -47,16 +53,15 @@ class TestLogin:
|
||||
async def test_login_succeeds_with_correct_password(
|
||||
self, client: AsyncClient
|
||||
) -> None:
|
||||
"""Login returns 200 and a session token for the correct password."""
|
||||
"""Login returns 200 and sets a session cookie for the correct password."""
|
||||
await _do_setup(client)
|
||||
response = await client.post(
|
||||
"/api/auth/login", json={"password": "Mysecretpass1!"}
|
||||
)
|
||||
assert response.status_code == 200
|
||||
body = response.json()
|
||||
assert "token" in body
|
||||
assert len(body["token"]) > 0
|
||||
assert "." in body["token"]
|
||||
# Token is not returned in the JSON body; it's set as an HttpOnly cookie
|
||||
assert "token" not in body
|
||||
assert "expires_at" in body
|
||||
|
||||
async def test_login_sets_cookie(self, client: AsyncClient) -> None:
|
||||
|
||||
@@ -9,7 +9,6 @@ export interface LoginRequest {
|
||||
|
||||
/** Successful login response from the API. */
|
||||
export interface LoginResponse {
|
||||
token: string;
|
||||
expires_at: string;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user