no-mistakes(document): Document P0 auth batch; reconcile HARDENING statuses; lint fixes
This commit is contained in:
@@ -105,8 +105,12 @@ Frontend: Alpine.js (CDN) + Tailwind CSS (CDN). Auth state in localStorage. Role
|
|||||||
- Role checks, admin user creation, login, and JWT validation all derive from the
|
- Role checks, admin user creation, login, and JWT validation all derive from the
|
||||||
role module; unknown/junk roles (e.g. lowercase `admin`/`superadmin` from the
|
role module; unknown/junk roles (e.g. lowercase `admin`/`superadmin` from the
|
||||||
old open register) fail closed at login/JWT and can never be recreated via the
|
old open register) fail closed at login/JWT and can never be recreated via the
|
||||||
API (422). `seed_users` normalizes unambiguous legacy aliases to canonical on
|
API (422). Startup self-heals (lifespan in `app/main.py`, helpers in
|
||||||
every startup (`normalize_legacy_user_roles`).
|
`app/services/seed.py`) converge legacy rows: `normalize_legacy_user_roles`
|
||||||
|
maps unambiguous alias nicknames onto canonical roles, and
|
||||||
|
`normalize_legacy_user_emails` lowercases stored emails — login and the admin
|
||||||
|
create-user duplicate check both compare on the lowercased form, so pre-P0
|
||||||
|
mixed-case emails are never silently locked out.
|
||||||
- Use `require_roles(*ADMIN_ROLES)` for admin gates; `sub` claim holds string user ID
|
- Use `require_roles(*ADMIN_ROLES)` for admin gates; `sub` claim holds string user ID
|
||||||
- Security headers middleware in `app/main.py`: X-Frame-Options DENY +
|
- Security headers middleware in `app/main.py`: X-Frame-Options DENY +
|
||||||
nosniff on everything, CSP on HTML pages, HSTS when `X-Forwarded-Proto: https`
|
nosniff on everything, CSP on HTML pages, HSTS when `X-Forwarded-Proto: https`
|
||||||
|
|||||||
+56
-43
@@ -19,15 +19,18 @@
|
|||||||
legacy-schema self-heal. **32 pytest tests pass.**
|
legacy-schema self-heal. **32 pytest tests pass.**
|
||||||
- **Live:** container `denya-onecare` on LXC `scottdenya` (192.168.68.75:8000),
|
- **Live:** container `denya-onecare` on LXC `scottdenya` (192.168.68.75:8000),
|
||||||
image built 2026-08-02, `restart: unless-stopped`.
|
image built 2026-08-02, `restart: unless-stopped`.
|
||||||
- **Known demo-only posture (must change):** `SECRET_KEY=change-me-in-production`,
|
- **Demo-only posture resolved (PR #10 + P0 batch):** `SECRET_KEY` now fails
|
||||||
`CORS_ORIGINS=*`, open `/api/auth/register`, SQLite backend, WhatsApp webhook
|
closed without a real key, CORS is an explicit origin allow-list, and open
|
||||||
|
self-registration was removed — `POST /api/auth/register` → 404, users are
|
||||||
|
admin-managed only (see AGENTS.md).
|
||||||
|
- **Known demo-only posture (must change):** SQLite backend; WhatsApp webhook
|
||||||
code is present but **no real credentials wired**.
|
code is present but **no real credentials wired**.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 1. P0 — Security blockers (do these FIRST, before any real data)
|
## 1. P0 — Security blockers (do these FIRST, before any real data)
|
||||||
|
|
||||||
### P0.1 Hardcode-safe secrets; never ship the default key
|
### P0.1 Hardcode-safe secrets; never ship the default key — **DONE (PR #10)**
|
||||||
- **Files:** `docker-compose.yml`, `app/core/config.py`
|
- **Files:** `docker-compose.yml`, `app/core/config.py`
|
||||||
- Replace the hardcoded `SECRET_KEY=change-me-in-production` default with a
|
- Replace the hardcoded `SECRET_KEY=change-me-in-production` default with a
|
||||||
fail-closed default: if `SECRET_KEY` is unset/empty or still the well-known
|
fail-closed default: if `SECRET_KEY` is unset/empty or still the well-known
|
||||||
@@ -37,7 +40,7 @@
|
|||||||
- **Acceptance:** starting the app without a real key fails loudly; container env
|
- **Acceptance:** starting the app without a real key fails loudly; container env
|
||||||
contains a strong random key (≥32 bytes, e.g. `openssl rand -hex 32`).
|
contains a strong random key (≥32 bytes, e.g. `openssl rand -hex 32`).
|
||||||
|
|
||||||
### P0.2 Lock down CORS
|
### P0.2 Lock down CORS — **DONE (PR #10)**
|
||||||
- **Files:** `app/main.py`, `docker-compose.yml`
|
- **Files:** `app/main.py`, `docker-compose.yml`
|
||||||
- `CORS_ORIGINS=*` + `allow_credentials=True` is an invalid/unsafe combo
|
- `CORS_ORIGINS=*` + `allow_credentials=True` is an invalid/unsafe combo
|
||||||
(browsers reject `*` with credentials anyway). Replace with an explicit
|
(browsers reject `*` with credentials anyway). Replace with an explicit
|
||||||
@@ -47,18 +50,21 @@
|
|||||||
- **Acceptance:** `settings.CORS_ORIGINS` is a comma-separated explicit list; the
|
- **Acceptance:** `settings.CORS_ORIGINS` is a comma-separated explicit list; the
|
||||||
middleware builds an allow-list, not `["*"]`.
|
middleware builds an allow-list, not `["*"]`.
|
||||||
|
|
||||||
### P0.3 Gate user registration
|
### P0.3 Gate user registration — **DONE (P0 batch, 2026-09)**
|
||||||
- **File:** `app/routers/auth.py` (`POST /api/auth/register`)
|
- Open `POST /api/auth/register` was removed entirely (404) — there is no sign-up UI.
|
||||||
- Today anyone on the network can self-register. Decide the model:
|
- Users are admin-managed: `POST /api/auth/users` (Admin/Jerome + Admin/Wahab only)
|
||||||
- **Recommended:** require an admin-issued invitation token, or restrict
|
creates users with a **forced canonical role** (unknown roles → 422);
|
||||||
registration to a seed/allowed list, or remove the open route and create
|
`PATCH /api/auth/users/{id}` changes role / deactivates (self-modification → 400);
|
||||||
users only via seed/admin.
|
`DELETE /api/auth/users/{id}` is guarded (users referenced by
|
||||||
- If a public self-service resident/tenant signup is genuinely required
|
tickets/timeline/escalations → 409); duplicate email → 409.
|
||||||
(Phase 2 QR/self-service), it must be a SEPARATE endpoint with a **role
|
- Emails are normalized (strip + lowercase) on every write path; a startup
|
||||||
default of the least-privilege role** and rate-limiting — never able to mint
|
self-heal lowercases legacy mixed-case rows so pre-P0 accounts can't be locked
|
||||||
admin/FM roles.
|
out of login. Unified role model lives in `app/core/roles.py`.
|
||||||
- **Acceptance:** a raw, unauthenticated register call can no longer mint an
|
- **Regression tests:** `tests/test_p0_auth_admin_batch.py`.
|
||||||
`Admin/*` or `Director` account.
|
- If a public self-service resident/tenant signup is genuinely required later
|
||||||
|
(Phase 2 QR/self-service), it must be a SEPARATE endpoint with a **role
|
||||||
|
default of the least-privilege role** and rate-limiting — never able to mint
|
||||||
|
admin/FM roles.
|
||||||
|
|
||||||
### P0.4 Reconsider SQLite for the final product
|
### P0.4 Reconsider SQLite for the final product
|
||||||
- **Files:** `docker-compose.yml`, `app/core/database.py`, `app/core/config.py`, PRD §16
|
- **Files:** `docker-compose.yml`, `app/core/database.py`, `app/core/config.py`, PRD §16
|
||||||
@@ -71,21 +77,21 @@
|
|||||||
- **Acceptance:** `pytest` green against Postgres (tests param via conftest),
|
- **Acceptance:** `pytest` green against Postgres (tests param via conftest),
|
||||||
Alembic applies cleanly on a fresh Postgres DB.
|
Alembic applies cleanly on a fresh Postgres DB.
|
||||||
|
|
||||||
### P0.5 WhatsApp webhook auth + hardening (finish wiring, then lock it)
|
### P0.5 WhatsApp webhook auth + hardening — **PARTIAL (P0 batch, 2026-09)**
|
||||||
- **File:** `app/routers/whatsapp.py`
|
- **File:** `app/routers/whatsapp.py`
|
||||||
- The handler exists but no credentials are set. When wiring this week:
|
- **Done:** the `GET` handshake validates `hub.verify_token` with a constant-time
|
||||||
- Verify the `hub.verify_token` check is constant-time (compare with
|
compare and returns **403 on mismatch**; inbound `POST`s are gated by the
|
||||||
`secrets.compare_digest`). **The GET verification path currently returns
|
`X-Webhook-Secret` header matching `WHATSAPP_WEBHOOK_SECRET` (fail-closed 403
|
||||||
`{"error": ...}` with HTTP 200** — flip to `403` on token mismatch.
|
when the env var is unset — see `.env.example`); the debug
|
||||||
- Validate **inbound messages only from Meta** — the webhook MUST authenticate
|
`GET /api/whatsapp/mock-log` now requires Bearer auth.
|
||||||
Meta's request signature (X-Hub-Signature-256 HMAC over the raw body with your
|
- **Still open:** Meta request-signature validation (`X-Hub-Signature-256` HMAC
|
||||||
app secret) before processing, otherwise anyone who discovers the endpoint can
|
over the raw body with the app secret — the shared-secret header above is the
|
||||||
forge tickets. This is the single most important WhatsApp hardening item.
|
interim gate); per-sender rate limiting / dedupe idempotency keyed on
|
||||||
- Add per-sender rate limiting / dedupe on `wa_message_id` (webhook retries can
|
`wa_message_id` (retries can still double-create tickets); redacting the raw
|
||||||
double-create tickets). Create an idempotency guard keyed on `wa_message_id`.
|
access token in `send_whatsapp_reply` error paths.
|
||||||
- Never log the raw access token; redact in `send_whatsapp_reply` error paths.
|
- **Acceptance (open items):** a forged POST without the Meta signature is
|
||||||
- **Acceptance:** a forged POST without the Meta signature is rejected; duplicate
|
rejected; duplicate `wa_message_id` does not create a second ticket;
|
||||||
`wa_message_id` does not create a second ticket; verify-token mismatch returns 403.
|
verify-token mismatch returns 403 (done).
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -130,10 +136,13 @@
|
|||||||
`X-Content-Type-Options: nosniff`.
|
`X-Content-Type-Options: nosniff`.
|
||||||
|
|
||||||
### P1.6 API hardening & rate limiting
|
### P1.6 API hardening & rate limiting
|
||||||
- Add rate limiting on `POST /api/auth/login` (brute-force) — per-IP/IP+account.
|
- **Done (P0 batch):** `POST /api/auth/login` is rate-limited in-process —
|
||||||
- Consider rate limits on ticket creation (spam / mass-creation).
|
~5 failures / 15 min per IP+email → 429 (env-tunable `LOGIN_RATE_LIMIT_*`,
|
||||||
- Normalize/validate `page_size` (already capped `le=200`) and pagination
|
a successful login resets the window). **Open:** ticket-creation rate limiting
|
||||||
tie-breaker (`id DESC` present — good).
|
(spam / mass-creation).
|
||||||
|
- **Done (P0 batch):** ticket-list pagination — `page`/`page_size` (default 50,
|
||||||
|
cap 200), `limit` alias for `page_size` also capped; passing both with
|
||||||
|
different values → 422. Tie-breaker `id DESC` present.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -145,9 +154,9 @@ Assumes Denya provides: **phone number ID, access token, verify token, app secre
|
|||||||
`WHATSAPP_VERIFY_TOKEN`, `WHATSAPP_APP_SECRET`, `META_GRAPH_BASE`) to `.env`
|
`WHATSAPP_VERIFY_TOKEN`, `WHATSAPP_APP_SECRET`, `META_GRAPH_BASE`) to `.env`
|
||||||
(git-ignored) and inject at runtime. Never commit.
|
(git-ignored) and inject at runtime. Never commit.
|
||||||
2. **Webhook handshake:** in Meta dashboard point the webhook URL at
|
2. **Webhook handshake:** in Meta dashboard point the webhook URL at
|
||||||
`<domain>/api/whatsapp/webhook`. The GET verify path currently echoes
|
`<domain>/api/whatsapp/webhook`. The GET verify path echoes `hub.challenge`
|
||||||
`hub.challenge` when the verify token matches — confirm this works, then apply
|
when the verify token matches (constant-time compare; mismatch → 403). The
|
||||||
P0.5 (403 on mismatch, HMAC signature validation).
|
still-open P0.5 work is the Meta signature validation in step 3.
|
||||||
3. **Verify incoming signature** (P0.5) — use `X-Hub-Signature-256` = HMAC-SHA256
|
3. **Verify incoming signature** (P0.5) — use `X-Hub-Signature-256` = HMAC-SHA256
|
||||||
of the raw body with your app secret, compared with `compare_digest`.
|
of the raw body with your app secret, compared with `compare_digest`.
|
||||||
4. **Reply flow:** confirm `send_whatsapp_reply` posts correctly to
|
4. **Reply flow:** confirm `send_whatsapp_reply` posts correctly to
|
||||||
@@ -194,14 +203,18 @@ Assumes Denya provides: **phone number ID, access token, verify token, app secre
|
|||||||
|
|
||||||
## 5. Definition of Done (production-ready)
|
## 5. Definition of Done (production-ready)
|
||||||
|
|
||||||
- [ ] No default `SECRET_KEY`; app fails closed without a real key
|
- [x] No default `SECRET_KEY`; app fails closed without a real key (PR #10)
|
||||||
- [ ] CORS is an explicit origin allow-list
|
- [x] CORS is an explicit origin allow-list (PR #10)
|
||||||
- [ ] Self-registration cannot mint privileged roles (or is admin-gated/removed)
|
- [x] Self-registration removed (register → 404); users are admin-managed only
|
||||||
|
with forced canonical roles (P0 batch)
|
||||||
- [ ] Postgres backend; Alembic applies cleanly on fresh DB; nightly backups
|
- [ ] Postgres backend; Alembic applies cleanly on fresh DB; nightly backups
|
||||||
- [ ] TLS-terminated reverse proxy with real domain; no raw :8000 on WAN
|
- [ ] TLS-terminated reverse proxy with real domain; no raw :8000 on WAN
|
||||||
- [ ] WhatsApp webhook: Meta signature validated, verify-token mismatch → 403,
|
- [x] Webhook POST gated by `X-Webhook-Secret` (fail-closed); verify-token
|
||||||
idempotent on `wa_message_id`, real credentials injected at runtime
|
mismatch → 403; `mock-log` requires auth
|
||||||
- [ ] Login/ticket rate limiting in place
|
- [ ] WhatsApp webhook: Meta `X-Hub-Signature-256` HMAC validated; idempotent on
|
||||||
|
`wa_message_id`; real credentials injected at runtime
|
||||||
|
- [x] Login rate limiting in place (~5 fails / 15 min per IP+email → 429)
|
||||||
|
- [ ] Ticket-creation rate limiting in place
|
||||||
- [ ] Photo uploads size-limited and content-sniffed
|
- [ ] Photo uploads size-limited and content-sniffed
|
||||||
- [ ] RBAC audited per-route; phone data access controlled
|
- [ ] RBAC audited per-route; phone data access controlled
|
||||||
- [ ] Expanded test suite (auth, WhatsApp, SLA boundary, uploads) — all green
|
- [ ] Expanded test suite (auth, WhatsApp, SLA boundary, uploads) — all green
|
||||||
|
|||||||
@@ -226,7 +226,7 @@ async def list_tickets(
|
|||||||
"""
|
"""
|
||||||
if page_size is not None and limit is not None and page_size != limit:
|
if page_size is not None and limit is not None and page_size != limit:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=422, # noqa: PLR2004 — param ``status`` shadows fastapi.status here
|
status_code=422, # ``status`` query param shadows fastapi.status in this scope
|
||||||
detail="Provide either 'page_size' or 'limit', not both",
|
detail="Provide either 'page_size' or 'limit', not both",
|
||||||
)
|
)
|
||||||
effective_page_size = page_size if page_size is not None else (limit or DEFAULT_PAGE_SIZE)
|
effective_page_size = page_size if page_size is not None else (limit or DEFAULT_PAGE_SIZE)
|
||||||
|
|||||||
+1
-1
@@ -92,7 +92,7 @@ class AdminUpdateUserRequest(BaseModel):
|
|||||||
return _validate_canonical_role(value)
|
return _validate_canonical_role(value)
|
||||||
|
|
||||||
@model_validator(mode="after")
|
@model_validator(mode="after")
|
||||||
def _at_least_one_field(self) -> "AdminUpdateUserRequest":
|
def _at_least_one_field(self) -> AdminUpdateUserRequest:
|
||||||
if self.role is None and self.active is None:
|
if self.role is None and self.active is None:
|
||||||
raise ValueError("Provide at least one of 'role' or 'active'")
|
raise ValueError("Provide at least one of 'role' or 'active'")
|
||||||
return self
|
return self
|
||||||
|
|||||||
+1
-1
@@ -17,8 +17,8 @@ os.environ["DATABASE_URL"] = f"sqlite+aiosqlite:///{_TMP_DIR}/test.db"
|
|||||||
os.environ.setdefault("SECRET_KEY", "test-secret-key-not-for-production-0123456789abcdef")
|
os.environ.setdefault("SECRET_KEY", "test-secret-key-not-for-production-0123456789abcdef")
|
||||||
os.environ.setdefault("CORS_ORIGINS", "http://test")
|
os.environ.setdefault("CORS_ORIGINS", "http://test")
|
||||||
|
|
||||||
import pytest_asyncio # noqa: E402 (DATABASE_URL must be set before app imports)
|
|
||||||
import pytest # noqa: E402
|
import pytest # noqa: E402
|
||||||
|
import pytest_asyncio # noqa: E402 (DATABASE_URL must be set before app imports)
|
||||||
from httpx import ASGITransport, AsyncClient # noqa: E402
|
from httpx import ASGITransport, AsyncClient # noqa: E402
|
||||||
|
|
||||||
from app.core.database import Base, async_session_factory, engine # noqa: E402
|
from app.core.database import Base, async_session_factory, engine # noqa: E402
|
||||||
|
|||||||
@@ -429,9 +429,10 @@ async def test_rate_limit_is_per_email(client: AsyncClient):
|
|||||||
|
|
||||||
async def test_rate_limit_window_expires(client: AsyncClient, monkeypatch):
|
async def test_rate_limit_window_expires(client: AsyncClient, monkeypatch):
|
||||||
"""After the 15-minute window passes, the account can log in again."""
|
"""After the 15-minute window passes, the account can log in again."""
|
||||||
import app.core.ratelimit as ratelimit_mod
|
|
||||||
import time as _time
|
import time as _time
|
||||||
|
|
||||||
|
import app.core.ratelimit as ratelimit_mod
|
||||||
|
|
||||||
email, password = "window@example.com", "password1"
|
email, password = "window@example.com", "password1"
|
||||||
token = await _login(client)
|
token = await _login(client)
|
||||||
await client.post(
|
await client.post(
|
||||||
|
|||||||
Reference in New Issue
Block a user