feat(security): P0 lockdown - admin-managed users, registration removed, rate limit, webhook secret, headers, email normalization #11

Merged
abiba-bot merged 3 commits from fm/denya-onecare-p0-deploy-pr-10-registrati-d7 into main 2026-09-08 17:38:21 +00:00
18 changed files with 1495 additions and 190 deletions
+11
View File
@@ -13,3 +13,14 @@ WHATSAPP_PHONE_NUMBER_ID=
WHATSAPP_ACCESS_TOKEN= WHATSAPP_ACCESS_TOKEN=
WHATSAPP_VERIFY_TOKEN= WHATSAPP_VERIFY_TOKEN=
META_GRAPH_BASE=https://graph.facebook.com/v18.0 META_GRAPH_BASE=https://graph.facebook.com/v18.0
# ── Webhook auth (P0) ──────────────────────────────────
# Shared secret for inbound WhatsApp webhook POSTs (X-Webhook-Secret header).
# FAIL-CLOSED: when unset/empty, every webhook message is rejected (403).
# Generate with: openssl rand -hex 32
WHATSAPP_WEBHOOK_SECRET=
# ── Login rate limiting (P0) ────────────────────────────
# ~5 failed login attempts per 15 minutes per IP+email → HTTP 429
LOGIN_RATE_LIMIT_MAX_ATTEMPTS=5
LOGIN_RATE_LIMIT_WINDOW_SECONDS=900
+31 -8
View File
@@ -47,14 +47,24 @@ Users, units, and categories are auto-seeded on first startup via lifespan hook:
| Method | Path | Auth | Description | | Method | Path | Auth | Description |
|--------|------|------|-------------| |--------|------|------|-------------|
| GET | `/health` | No | Health check | | GET | `/health` | No | Health check |
| POST | `/api/auth/register` | No | Create user | | POST | `/api/auth/login` | No | Get JWT tokens (rate-limited ~5 fails/15min/IP+email → 429) |
| POST | `/api/auth/login` | No | Get JWT tokens |
| POST | `/api/auth/refresh` | Token | Refresh tokens | | POST | `/api/auth/refresh` | Token | Refresh tokens |
| GET | `/api/auth/me` | Bearer | Current user | | GET | `/api/auth/me` | Bearer | Current user |
| GET | `/api/auth/admin-only` | Admin/Jerome, Admin/Wahab | RBAC demo endpoint | | GET | `/api/auth/admin-only` | Admin/Jerome, Admin/Wahab | RBAC demo endpoint |
| GET | `/api/auth/users` | Bearer | List users (id, name, role) for the assign-technician picker | | GET | `/api/auth/users` | Bearer | List users (id, name, role) for the assign-technician picker |
| POST | `/api/whatsapp/mock` | No | Mock WhatsApp | | POST | `/api/auth/users` | Admin/Jerome, Admin/Wahab | Admin creates a user (forced canonical role; unknown roles → 422) |
| GET | `/api/whatsapp/mock-log` | No | Recent mock submissions | | PATCH | `/api/auth/users/{id}` | Admin/Jerome, Admin/Wahab | Role change / deactivate (self-modification → 400) |
| DELETE | `/api/auth/users/{id}` | Admin/Jerome, Admin/Wahab | Delete user (409 if referenced by tickets/timeline/escalations) |
**Self-registration is removed** — `POST /api/auth/register` 404s and there is no
sign-up UI; users are created/managed by admins only (P0 hardening batch).
### WhatsApp
| Method | Path | Auth | Description |
|--------|------|------|-------------|
| GET | `/api/whatsapp/webhook` | No | Meta handshake (`hub.verify_token`, constant-time; mismatch → 403) |
| POST | `/api/whatsapp/webhook` | `X-Webhook-Secret` header | Inbound message → ticket + log. Fail-closed: 403 when `WHATSAPP_WEBHOOK_SECRET` is unset or the header doesn't match |
| GET | `/api/whatsapp/mock-log` | Bearer | Recent webhook submissions (debug; auth required) |
### Pages (Sprint 3) — Jinja2 templates at `app/templates/` ### Pages (Sprint 3) — Jinja2 templates at `app/templates/`
| Method | Path | Auth | Description | | Method | Path | Auth | Description |
@@ -73,7 +83,7 @@ Frontend: Alpine.js (CDN) + Tailwind CSS (CDN). Auth state in localStorage. Role
| Method | Path | Auth | Description | | Method | Path | Auth | Description |
|--------|------|------|-------------| |--------|------|------|-------------|
| POST | `/api/tickets` | Bearer | Create ticket (auto-number PAV-YYYY-NNNNN) | | POST | `/api/tickets` | Bearer | Create ticket (auto-number PAV-YYYY-NNNNN) |
| GET | `/api/tickets` | No | List tickets (filter: status, priority, property, building, unit_id, category_id, assigned_to, date_from, date_to) | | GET | `/api/tickets` | No | List tickets (filter: status, priority, property, building, unit_id, category_id, assigned_to, date_from, date_to; paginate with `page`/`page_size` or `limit` alias — hard cap 200, both given → 422) |
| GET | `/api/tickets/{id}` | No | Get ticket detail with timeline, photos, SLA status, nested unit/category, phone | | GET | `/api/tickets/{id}` | No | Get ticket detail with timeline, photos, SLA status, nested unit/category, phone |
| GET | `/api/tickets/{id}/transitions` | No | Valid next statuses for the ticket's current status (drives the detail-page status picker) | | GET | `/api/tickets/{id}/transitions` | No | Valid next statuses for the ticket's current status (drives the detail-page status picker) |
| PATCH | `/api/tickets/{id}` | Bearer | Update ticket (validates status transitions; assigning a technician auto-advances New/Logged/Triage to Assigned) | | PATCH | `/api/tickets/{id}` | Bearer | Update ticket (validates status transitions; assigning a technician auto-advances New/Logged/Triage to Assigned) |
@@ -88,9 +98,22 @@ Frontend: Alpine.js (CDN) + Tailwind CSS (CDN). Auth state in localStorage. Role
## Auth ## Auth
- JWT access (30min) + refresh (7d) tokens - JWT access (30min) + refresh (7d) tokens
- Roles: CS Rep, CS Manager, FM Dispatcher, Admin/Jerome, Admin/Wahab, Tech, CEO, Director - **Unified role model** lives in `app/core/roles.py` (`CANONICAL_ROLES`,
- Use `require_roles("Admin/Jerome", "Admin/Wahab")` dependency for RBAC `ADMIN_ROLES` = Admin/Jerome + Admin/Wahab, `ROLE_ALIASES` for legacy
- `sub` claim holds string user ID nickname roles like `technician`/`cs`/`fm`/`ceo`). Canonical stored roles:
CS Rep, CS Manager, FM Dispatcher, Admin/Jerome, Admin/Wahab, Tech, CEO, Director.
- 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
old open register) fail closed at login/JWT and can never be recreated via the
API (422). Startup self-heals (lifespan in `app/main.py`, helpers in
`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
- Security headers middleware in `app/main.py`: X-Frame-Options DENY +
nosniff on everything, CSP on HTML pages, HSTS when `X-Forwarded-Proto: https`
## Ticket System (Sprint 2) ## Ticket System (Sprint 2)
-1
View File
@@ -1 +0,0 @@
AGENTS.md
+2
View File
@@ -0,0 +1,2 @@
<!-- Points Claude at AGENTS.md via import; edit AGENTS.md, not this file. -->
@AGENTS.md
+53 -40
View File
@@ -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.
- Emails are normalized (strip + lowercase) on every write path; a startup
self-heal lowercases legacy mixed-case rows so pre-P0 accounts can't be locked
out of login. Unified role model lives in `app/core/roles.py`.
- **Regression tests:** `tests/test_p0_auth_admin_batch.py`.
- 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 (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 default of the least-privilege role** and rate-limiting — never able to mint
admin/FM roles. admin/FM roles.
- **Acceptance:** a raw, unauthenticated register call can no longer mint an
`Admin/*` or `Director` account.
### 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
+7
View File
@@ -36,6 +36,13 @@ class Settings(BaseSettings):
WHATSAPP_ACCESS_TOKEN: str = "" WHATSAPP_ACCESS_TOKEN: str = ""
WHATSAPP_VERIFY_TOKEN: str = "" WHATSAPP_VERIFY_TOKEN: str = ""
META_GRAPH_BASE: str = "https://graph.facebook.com/v18.0" META_GRAPH_BASE: str = "https://graph.facebook.com/v18.0"
# Shared secret for inbound webhook POSTs (header ``X-Webhook-Secret``).
# Fail-closed: when unset/empty the webhook rejects every message.
WHATSAPP_WEBHOOK_SECRET: str = ""
# ── Login rate limiting ──────────────────────────────────────────
LOGIN_RATE_LIMIT_MAX_ATTEMPTS: int = 5
LOGIN_RATE_LIMIT_WINDOW_SECONDS: int = 15 * 60
# ── Paths ──────────────────────────────────────────────────────── # ── Paths ────────────────────────────────────────────────────────
BASE_DIR: Path = Path(__file__).resolve().parent.parent.parent BASE_DIR: Path = Path(__file__).resolve().parent.parent.parent
+82
View File
@@ -0,0 +1,82 @@
"""Dependency-light login rate limiter.
Brute-force protection for ``POST /api/auth/login``: a sliding window of
failed attempts keyed by ``ip|email``. Defaults to ~5 failures / 15 minutes
(env-tunable via ``LOGIN_RATE_LIMIT_MAX_ATTEMPTS`` /
``LOGIN_RATE_LIMIT_WINDOW_SECONDS``).
In-process storage is intentional: the app currently runs a single uvicorn
worker, and keeping the limiter dependency-light avoids pulling slowapi in
for one endpoint. ``_now`` is a module-level hook so tests can fast-forward
the clock.
"""
from __future__ import annotations
import time
from collections import defaultdict, deque
from fastapi import HTTPException, status
from app.core.config import settings
def _now() -> float:
"""Wall-clock epoch seconds; overridable in tests via monkeypatch."""
return time.time()
class LoginRateLimiter:
"""Sliding-window failure limiter keyed by ``ip|email``."""
def __init__(self, max_attempts: int = 5, window_seconds: int = 15 * 60) -> None:
self.max_attempts = max(max_attempts, 1)
self.window_seconds = max(window_seconds, 1)
self._failures: defaultdict[str, deque[float]] = defaultdict(deque)
def key(self, ip: str, email: str) -> str:
return f"{ip}|{email.strip().lower()}"
def _prune(self, key: str, now: float | None = None) -> None:
now = now if now is not None else _now()
window_start = now - self.window_seconds
bucket = self._failures.get(key)
if bucket is None:
return
while bucket and bucket[0] <= window_start:
bucket.popleft()
if not bucket:
self._failures.pop(key, None)
def failure_count(self, key: str) -> int:
self._prune(key)
return len(self._failures.get(key, ()))
def is_blocked(self, key: str) -> bool:
return self.failure_count(key) >= self.max_attempts
def record_failure(self, key: str) -> None:
self._failures[key].append(_now())
self._prune(key)
def clear(self, key: str) -> None:
self._failures.pop(key, None)
def reset(self) -> None:
self._failures.clear()
def check_or_raise(self, key: str) -> None:
"""Raise HTTP 429 when the key has exhausted its attempts."""
if self.is_blocked(key):
raise HTTPException(
status_code=status.HTTP_429_TOO_MANY_REQUESTS,
detail="Too many failed login attempts. Try again later.",
)
# Shared instance — module import is safe because the app fails closed at
# boot (app/core/config.py) before any request can reach the login route.
login_rate_limiter = LoginRateLimiter(
max_attempts=settings.LOGIN_RATE_LIMIT_MAX_ATTEMPTS,
window_seconds=settings.LOGIN_RATE_LIMIT_WINDOW_SECONDS,
)
+99
View File
@@ -0,0 +1,99 @@
"""Unified role model — single source of truth for user roles.
HARDENING (P0 batch): every role string used by seeds, RBAC checks, admin
user management, login, and JWT validation derives from this module so the
system can never silently drift between role vocabularies.
Canonical roles are the human-readable taxonomy the whole product already
uses (PRD §4, ``app/services/seed.py``, the frontend nav in base.html):
Admin/Jerome, Admin/Wahab, CS Rep, CS Manager, FM Dispatcher,
Tech, CEO, Director
Legacy databases created under the pre-P0 open-registration builds can carry
lowercase/nickname role strings (``technician``, ``cs``, ``fm``, ``ceo``,
``admin``, ``superadmin`` …). ``ROLE_ALIASES`` maps the *unambiguous*
nicknames onto a canonical role so startup normalization (see
``app/services/seed.py::normalize_legacy_user_roles``) can converge the data.
``admin`` / ``superadmin`` are deliberately NOT aliased: they are
identity-ambiguous (they cannot be attributed to Jerome or Wahab) and were
mintable by anyone during the open-registration window, so they are treated
as unknown and fail closed — the operator must remediate those rows manually
(role cleanup on the live DB is owned by the deployer).
"""
from __future__ import annotations
# ── Canonical taxonomy ────────────────────────────────────────────────
# Order is cosmetic; membership is what matters.
CANONICAL_ROLES: tuple[str, ...] = (
"Admin/Jerome",
"Admin/Wahab",
"CS Rep",
"CS Manager",
"FM Dispatcher",
"Tech",
"CEO",
"Director",
)
# Roles that pass admin gates (ticket DELETE, user management, …).
ADMIN_ROLES: tuple[str, ...] = ("Admin/Jerome", "Admin/Wahab")
# Roles shown to the frontend nav/assignment helpers as "technician" pool.
TECHNICIAN_ROLE = "Tech"
# ── Legacy alias → canonical mapping (case-insensitive) ───────────────
# Keys are lowercased. Unambiguous nicknames from legacy/early seeds and the
# brief's role model ("technician/cs/fm/ceo") converge onto canonical roles.
ROLE_ALIASES: dict[str, str] = {
"technician": TECHNICIAN_ROLE,
"tech": TECHNICIAN_ROLE,
"cs": "CS Rep",
"cs rep": "CS Rep",
"cs representative": "CS Rep",
"cs manager": "CS Manager",
"fm": "FM Dispatcher",
"fm dispatcher": "FM Dispatcher",
"ceo": "CEO",
"director": "Director",
}
# Aliases that are explicitly NOT auto-mapped (identity-ambiguous and/or
# mintable by the old open register). They stay unknown → denied at login
# and JWT validation until an operator remediates the row.
_BLOCKED_LEGACY_ROLES = frozenset({"admin", "superadmin", "administrator"})
def normalize_role(role: str | None) -> str | None:
"""Return the canonical role for *role*, or ``None`` when unrecognised.
``Admin/Jerome`` → ``Admin/Jerome``; ``technician`` → ``Tech``;
``superadmin`` → ``None`` (unknown; caller must fail closed).
"""
if not role:
return None
stripped = role.strip()
if stripped in CANONICAL_ROLES:
return stripped
return ROLE_ALIASES.get(stripped.lower())
def is_known_role(role: str | None) -> bool:
"""True when *role* is canonical or maps to a canonical role."""
return normalize_role(role) is not None
def is_admin_role(role: str | None) -> bool:
"""True when *role* is one of the canonical administrator roles."""
return normalize_role(role) in ADMIN_ROLES
def is_blocked_legacy_role(role: str | None) -> bool:
"""True for legacy ``admin``/``superadmin`` rows that need remediation.
Such rows are not canonical, are not auto-mapped, and must not pass any
authorization gate; an operator should reassign or remove them.
"""
return bool(role) and role.strip().lower() in _BLOCKED_LEGACY_ROLES
+9
View File
@@ -15,6 +15,7 @@ from sqlalchemy.ext.asyncio import AsyncSession
from app.core.config import settings from app.core.config import settings
from app.core.database import get_db from app.core.database import get_db
from app.core.roles import is_known_role
from app.models.user import User from app.models.user import User
bearer_scheme = HTTPBearer(auto_error=False) bearer_scheme = HTTPBearer(auto_error=False)
@@ -90,6 +91,14 @@ async def get_current_user(
user = result.scalar_one_or_none() user = result.scalar_one_or_none()
if user is None or not user.active: if user is None or not user.active:
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found or inactive") raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found or inactive")
# Unified role model: reject rows whose stored role is not canonical or a
# known legacy alias. Legacy junk roles (e.g. lowercase ``admin``) must
# fail closed here so they can never ride an access token into the app.
if not is_known_role(user.role):
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Account role is not recognised; contact an administrator",
)
return user return user
+73 -1
View File
@@ -14,7 +14,13 @@ from sqlalchemy import text
from app.core.config import settings from app.core.config import settings
from app.core.database import Base, async_session_factory, engine from app.core.database import Base, async_session_factory, engine
from app.routers import auth, health, pages, tickets, whatsapp from app.routers import auth, health, pages, tickets, whatsapp
from app.services.seed import seed_categories, seed_units, seed_users from app.services.seed import (
normalize_legacy_user_emails,
normalize_legacy_user_roles,
seed_categories,
seed_units,
seed_users,
)
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@@ -67,6 +73,10 @@ async def lifespan(app: FastAPI):
await ensure_legacy_schema(conn) await ensure_legacy_schema(conn)
async with async_session_factory() as session: async with async_session_factory() as session:
await seed_users(session) await seed_users(session)
# P0 role-model unification: converge legacy nickname roles (e.g.
# ``technician``/``cs``/``fm``) onto the canonical taxonomy at startup.
await normalize_legacy_user_roles(session)
await normalize_legacy_user_emails(session)
await session.commit() await session.commit()
await seed_units(session, json_path=str(settings.BASE_DIR / "apartment_mapping.json")) await seed_units(session, json_path=str(settings.BASE_DIR / "apartment_mapping.json"))
await session.commit() await session.commit()
@@ -83,6 +93,68 @@ app = FastAPI(
lifespan=lifespan, lifespan=lifespan,
) )
# ── Security headers (P0 batch) ──────────────────────────────────────
class SecurityHeadersMiddleware:
"""Set hardening headers on every HTTP response.
* ``X-Frame-Options: DENY`` and ``X-Content-Type-Options: nosniff`` on
all responses;
* CSP on HTML pages (login + app pages; the Alpine.js/Tailwind CDNs need
the CDN hosts + inline script/style for this demo);
* ``Strict-Transport-Security`` only when TLS terminates (https scheme
or ``X-Forwarded-Proto: https`` from the reverse proxy).
"""
HSTS = "max-age=31536000; includeSubDomains"
CSP = (
"default-src 'self'; "
"script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://cdn.tailwindcss.com; "
"style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://cdn.tailwindcss.com; "
"img-src 'self' data: blob:; "
"font-src 'self' data:; "
"connect-src 'self'; "
"frame-ancestors 'none'; "
"base-uri 'self'; "
"form-action 'self'; "
"object-src 'none'"
)
def __init__(self, app):
self.app = app
async def __call__(self, scope, receive, send):
if scope["type"] != "http":
await self.app(scope, receive, send)
return
is_tls = scope.get("scheme") == "https"
for name, value in scope.get("headers") or []:
if name.lower() == b"x-forwarded-proto":
first = value.decode("latin-1").split(",", 1)[0].strip().lower()
if first == "https":
is_tls = True
async def send_wrapper(message):
if message["type"] == "http.response.start":
headers = list(message.get("headers") or [])
content_type = next(
(v for k, v in headers if k.lower() == b"content-type"), b""
)
if content_type.startswith(b"text/html"):
headers.append((b"content-security-policy", self.CSP.encode()))
headers.append((b"x-frame-options", b"DENY"))
headers.append((b"x-content-type-options", b"nosniff"))
if is_tls:
headers.append((b"strict-transport-security", self.HSTS.encode()))
message["headers"] = headers
await send(message)
await self.app(scope, receive, send_wrapper)
app.add_middleware(SecurityHeadersMiddleware)
# ── CORS (HARDENING.md P0.2 — explicit origin allow-list, never "*") ── # ── CORS (HARDENING.md P0.2 — explicit origin allow-list, never "*") ──
_origins = [o.strip() for o in settings.CORS_ORIGINS.split(",") if o.strip()] _origins = [o.strip() for o in settings.CORS_ORIGINS.split(",") if o.strip()]
if "*" in _origins or not _origins: if "*" in _origins or not _origins:
+74 -27
View File
@@ -1,20 +1,27 @@
"""Authentication router — register, login, refresh, me.""" """Authentication router — login, refresh, me, and admin user management.
Self-registration was removed (P0 hardening): users are created/managed by
admins only via ``POST/PATCH/DELETE /api/auth/users``.
"""
from __future__ import annotations from __future__ import annotations
from typing import Annotated from typing import Annotated
from fastapi import APIRouter, Depends from fastapi import APIRouter, Depends, HTTPException, Request, status
from sqlalchemy import select from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.ext.asyncio import AsyncSession
from app.core.database import get_db from app.core.database import get_db
from app.core.ratelimit import login_rate_limiter
from app.core.roles import ADMIN_ROLES
from app.core.security import get_current_user, require_roles from app.core.security import get_current_user, require_roles
from app.models.user import User from app.models.user import User
from app.schemas.auth import ( from app.schemas.auth import (
AdminCreateUserRequest,
AdminUpdateUserRequest,
LoginRequest, LoginRequest,
RefreshRequest, RefreshRequest,
RegisterRequest,
TokenResponse, TokenResponse,
UserOut, UserOut,
) )
@@ -22,36 +29,29 @@ from app.services import auth as auth_service
router = APIRouter(prefix="/api/auth", tags=["auth"]) router = APIRouter(prefix="/api/auth", tags=["auth"])
_require_admin = require_roles(*ADMIN_ROLES)
@router.get("/users", response_model=list[UserOut])
async def list_users(
db: Annotated[AsyncSession, Depends(get_db)],
current_user: Annotated[User, Depends(get_current_user)],
) -> list[User]:
"""List users (id, name, role) for assignment pickers.
Previously the frontend hard-coded technician ids/names in detail.html;
this endpoint makes the assign dropdown data-driven so a seed change never
silently breaks technician assignment.
"""
result = await db.execute(select(User).order_by(User.full_name))
return list(result.scalars().all())
@router.post("/register", response_model=UserOut, status_code=201)
async def register(
body: RegisterRequest,
db: Annotated[AsyncSession, Depends(get_db)],
) -> User:
return await auth_service.register(db, body)
# ── Public authN ─────────────────────────────────────────────────────
@router.post("/login", response_model=TokenResponse) @router.post("/login", response_model=TokenResponse)
async def login( async def login(
request: Request,
body: LoginRequest, body: LoginRequest,
db: Annotated[AsyncSession, Depends(get_db)], db: Annotated[AsyncSession, Depends(get_db)],
) -> TokenResponse: ) -> TokenResponse:
"""Log in. Brute-force limited to ~5 failures / 15 min per IP+email (429)."""
client_ip = request.client.host if request.client else "unknown"
key = login_rate_limiter.key(client_ip, body.email)
login_rate_limiter.check_or_raise(key)
try:
access, refresh, _user = await auth_service.login(db, body.email, body.password) access, refresh, _user = await auth_service.login(db, body.email, body.password)
except HTTPException as exc:
# Count only real auth failures toward the limit; success resets it.
if exc.status_code == status.HTTP_401_UNAUTHORIZED:
login_rate_limiter.record_failure(key)
raise
login_rate_limiter.clear(key) # successful login resets the failure window
return TokenResponse(access_token=access, refresh_token=refresh) return TokenResponse(access_token=access, refresh_token=refresh)
@@ -71,7 +71,54 @@ async def me(current_user: Annotated[User, Depends(get_current_user)]) -> User:
@router.get("/admin-only", response_model=UserOut) @router.get("/admin-only", response_model=UserOut)
async def admin_only( async def admin_only(
current_user: Annotated[User, Depends(require_roles("Admin/Jerome", "Admin/Wahab"))], current_user: Annotated[User, Depends(_require_admin)],
) -> User: ) -> User:
"""Example RBAC-protected endpoint — only Admins can access.""" """Example RBAC-protected endpoint — only canonical Admins can access."""
return current_user return current_user
# ── User directory ───────────────────────────────────────────────────
@router.get("/users", response_model=list[UserOut])
async def list_users(
db: Annotated[AsyncSession, Depends(get_db)],
current_user: Annotated[User, Depends(get_current_user)],
) -> list[User]:
"""List users (id, name, role) for authenticated assignment pickers."""
result = await db.execute(select(User).order_by(User.full_name))
return list(result.scalars().all())
# ── Admin user management ────────────────────────────────────────────
@router.post("/users", response_model=UserOut, status_code=status.HTTP_201_CREATED)
async def create_user(
body: AdminCreateUserRequest,
db: Annotated[AsyncSession, Depends(get_db)],
current_user: Annotated[User, Depends(_require_admin)],
) -> User:
"""Admin-only: create a user with a forced canonical role.
The client cannot self-register or pick an arbitrary role — unknown roles
(e.g. ``admin``, ``superadmin``) are rejected with 422.
"""
return await auth_service.create_user(db, body)
@router.patch("/users/{user_id}", response_model=UserOut)
async def update_user(
user_id: int,
body: AdminUpdateUserRequest,
db: Annotated[AsyncSession, Depends(get_db)],
current_user: Annotated[User, Depends(_require_admin)],
) -> User:
"""Admin-only: change a user's role and/or deactivate the account."""
return await auth_service.update_user(db, current_user, user_id, body)
@router.delete("/users/{user_id}", status_code=status.HTTP_204_NO_CONTENT)
async def delete_user(
user_id: int,
db: Annotated[AsyncSession, Depends(get_db)],
current_user: Annotated[User, Depends(_require_admin)],
) -> None:
"""Admin-only: delete a user account (guarded; see auth_service)."""
await auth_service.delete_user(db, current_user, user_id)
+26 -5
View File
@@ -13,6 +13,7 @@ from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.ext.asyncio import AsyncSession
from app.core.config import settings from app.core.config import settings
from app.core.database import get_db from app.core.database import get_db
from app.core.roles import ADMIN_ROLES
from app.core.security import get_current_user, require_roles from app.core.security import get_current_user, require_roles
from app.models.category import Category from app.models.category import Category
from app.models.ticket import Ticket, TicketPhoto from app.models.ticket import Ticket, TicketPhoto
@@ -37,6 +38,13 @@ logger = logging.getLogger(__name__)
router = APIRouter(prefix="/api/tickets", tags=["tickets"]) router = APIRouter(prefix="/api/tickets", tags=["tickets"])
_require_admin = require_roles(*ADMIN_ROLES)
# Pagination contract: sane defaults and a hard page-size cap so list
# responses never balloon into truncation territory (P0 batch).
DEFAULT_PAGE_SIZE = 50
MAX_PAGE_SIZE = 200
# Ensure uploads directory exists # Ensure uploads directory exists
UPLOADS_DIR = settings.BASE_DIR / "uploads" UPLOADS_DIR = settings.BASE_DIR / "uploads"
UPLOADS_DIR.mkdir(parents=True, exist_ok=True) UPLOADS_DIR.mkdir(parents=True, exist_ok=True)
@@ -197,7 +205,10 @@ async def create_ticket(
async def list_tickets( async def list_tickets(
db: Annotated[AsyncSession, Depends(get_db)], db: Annotated[AsyncSession, Depends(get_db)],
page: int = Query(1, ge=1), page: int = Query(1, ge=1),
page_size: int = Query(50, ge=1, le=200), page_size: int | None = Query(None, ge=1, le=MAX_PAGE_SIZE),
limit: int | None = Query(
None, ge=1, le=MAX_PAGE_SIZE, description="Alias for page_size (also capped)"
),
status: str | None = Query(None), status: str | None = Query(None),
priority: str | None = Query(None), priority: str | None = Query(None),
property: str | None = Query(None), property: str | None = Query(None),
@@ -208,7 +219,17 @@ async def list_tickets(
date_from: datetime | None = Query(None), date_from: datetime | None = Query(None),
date_to: datetime | None = Query(None), date_to: datetime | None = Query(None),
) -> TicketListResponse: ) -> TicketListResponse:
"""List tickets with optional filtering and pagination.""" """List tickets with optional filtering and pagination.
Both ``page_size`` and its alias ``limit`` are capped at MAX_PAGE_SIZE;
supplying both is an error. Neither defaults to DEFAULT_PAGE_SIZE.
"""
if page_size is not None and limit is not None and page_size != limit:
raise HTTPException(
status_code=422, # ``status`` query param shadows fastapi.status in this scope
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)
tickets, total = await ticket_service.list_tickets( tickets, total = await ticket_service.list_tickets(
db, db,
status_filter=status, status_filter=status,
@@ -221,10 +242,10 @@ async def list_tickets(
date_from=date_from, date_from=date_from,
date_to=date_to, date_to=date_to,
page=page, page=page,
page_size=page_size, page_size=effective_page_size,
) )
items = [TicketBrief.model_validate(t) for t in tickets] items = [TicketBrief.model_validate(t) for t in tickets]
return TicketListResponse(items=items, total=total, page=page, page_size=page_size) return TicketListResponse(items=items, total=total, page=page, page_size=effective_page_size)
@router.get("/{ticket_id}/transitions") @router.get("/{ticket_id}/transitions")
@@ -270,7 +291,7 @@ async def update_ticket(
async def delete_ticket( async def delete_ticket(
ticket_id: int, ticket_id: int,
db: Annotated[AsyncSession, Depends(get_db)], db: Annotated[AsyncSession, Depends(get_db)],
current_user: Annotated[User, Depends(require_roles("Admin/Jerome", "Admin/Wahab"))], current_user: Annotated[User, Depends(_require_admin)],
) -> None: ) -> None:
"""Delete a ticket and its children (timeline, photos, escalations). """Delete a ticket and its children (timeline, photos, escalations).
+75 -20
View File
@@ -1,18 +1,31 @@
"""WhatsApp webhook handler — Meta Graph API integration.""" """WhatsApp webhook handler — Meta Graph API integration.
P0 hardening:
* ``POST /api/whatsapp/webhook`` requires the ``X-Webhook-Secret`` header to
match ``WHATSAPP_WEBHOOK_SECRET``. Fail-closed: when the env var is unset
every message is rejected (same posture as the SECRET_KEY guard).
* ``GET /api/whatsapp/webhook`` (Meta handshake) validates ``hub.verify_token``
with a constant-time compare and returns 403 on mismatch.
* ``GET /api/whatsapp/mock-log`` now requires authentication (was a public
debug endpoint that could 500 and leak stack traces without auth).
"""
from __future__ import annotations from __future__ import annotations
import logging import logging
import secrets
from datetime import datetime, timezone from datetime import datetime, timezone
from typing import Annotated from typing import Annotated
import httpx import httpx
from fastapi import APIRouter, Depends, Query from fastapi import APIRouter, Depends, Header, HTTPException, Query, status
from sqlalchemy import select from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.ext.asyncio import AsyncSession
from app.core.config import settings from app.core.config import settings
from app.core.database import get_db from app.core.database import get_db
from app.core.security import get_current_user
from app.models.user import User
from app.models.whatsapp_log import WhatsAppLog from app.models.whatsapp_log import WhatsAppLog
from app.schemas.whatsapp import ( from app.schemas.whatsapp import (
MetaWebhookRequest, MetaWebhookRequest,
@@ -62,24 +75,55 @@ async def send_whatsapp_reply(
return WhatsAppReplyResponse(success=False, message=str(exc)) return WhatsAppReplyResponse(success=False, message=str(exc))
# ── Webhook auth (fail-closed) ──────────────────────────────────────
async def require_webhook_secret(
x_webhook_secret: Annotated[str | None, Header(alias="X-Webhook-Secret")] = None,
) -> None:
"""Reject webhook messages unless X-Webhook-Secret matches the env secret.
Reads ``settings.WHATSAPP_WEBHOOK_SECRET`` at request time so the value
can be injected per-deployment. Empty/unset env ⇒ reject everything.
"""
expected = settings.WHATSAPP_WEBHOOK_SECRET
if not expected:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Webhook disabled: WHATSAPP_WEBHOOK_SECRET is not configured",
)
if not x_webhook_secret or not secrets.compare_digest(x_webhook_secret, expected):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid webhook secret",
)
# ── Webhook endpoint ──────────────────────────────────────────────── # ── Webhook endpoint ────────────────────────────────────────────────
@router.post("/webhook") @router.get("/webhook")
async def whatsapp_webhook( async def whatsapp_webhook_verify(
mode: str | None = Query(None, alias="hub.mode"),
verify_token: str | None = Query(None, alias="hub.verify_token"),
challenge: str | None = Query(None, alias="hub.challenge"),
) -> WebhookVerificationResponse | dict:
"""Meta webhook handshake (GET): echo the challenge when the token matches."""
if mode != "subscribe" or not challenge:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail="Missing hub.mode / hub.challenge",
)
expected = settings.WHATSAPP_VERIFY_TOKEN
if not expected or not secrets.compare_digest(verify_token or "", expected):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Verify token mismatch",
)
return WebhookVerificationResponse(challenge=challenge)
async def _process_entries(
body: MetaWebhookRequest, body: MetaWebhookRequest,
db: Annotated[AsyncSession, Depends(get_db)], db: AsyncSession,
hub_verify_token: str | None = Query(None, alias="hub.verify_token"),
mode: str | None = Query(None),
hub_challenge: str | None = Query(None),
) -> dict: ) -> dict:
"""Handle incoming WhatsApp webhook from Meta.""" """Create tickets + logs for inbound messages. Shared by the POST handler."""
# ── Verification GET request (Meta sends this on webhook setup) ──
if mode and hub_challenge:
if hub_verify_token != settings.WHATSAPP_VERIFY_TOKEN:
return {"error": "Verify token mismatch"}
return WebhookVerificationResponse(challenge=hub_challenge).model_dump()
# ── Process inbound messages ────────────────────────────────────
if not body.entry: if not body.entry:
return {"status": "no entry"} return {"status": "no entry"}
@@ -153,13 +197,24 @@ async def whatsapp_webhook(
return {"status": "no messages"} return {"status": "no messages"}
# ── Legacy debug endpoint ────────────────────────────────────────── @router.post("/webhook")
async def whatsapp_webhook(
body: MetaWebhookRequest,
db: Annotated[AsyncSession, Depends(get_db)],
_auth: None = Depends(require_webhook_secret),
) -> dict:
"""Process an inbound WhatsApp message (Meta POST). Requires webhook secret."""
return await _process_entries(body, db)
# ── Debug endpoint (auth required) ──────────────────────────────────
@router.get("/mock-log", response_model=list[MockWhatsAppLogEntry]) @router.get("/mock-log", response_model=list[MockWhatsAppLogEntry])
async def mock_whatsapp_log( async def mock_whatsapp_log(
db: Annotated[AsyncSession, Depends(get_db)], db: Annotated[AsyncSession, Depends(get_db)],
limit: int = 50, current_user: Annotated[User, Depends(get_current_user)],
limit: int = Query(50, ge=1, le=200),
) -> list[MockWhatsAppLogEntry]: ) -> list[MockWhatsAppLogEntry]:
"""Return recent WhatsApp webhook submissions for debugging.""" """Return recent WhatsApp webhook submissions (authenticated only)."""
result = await db.execute( result = await db.execute(
select(WhatsAppLog).order_by(WhatsAppLog.received_at.desc()).limit(limit) select(WhatsAppLog).order_by(WhatsAppLog.received_at.desc()).limit(limit)
) )
+66 -10
View File
@@ -2,18 +2,27 @@
from __future__ import annotations from __future__ import annotations
from pydantic import BaseModel from pydantic import BaseModel, Field, field_validator, model_validator
from app.core.roles import CANONICAL_ROLES
class RegisterRequest(BaseModel): def _validate_canonical_role(value: str | None) -> str | None:
email: str """Reject any role that is not part of the unified canonical taxonomy.
password: str
full_name: str The role vocabulary is closed: admin user-management must only ever mint
phone: str | None = None canonical roles (see app/core/roles.py). Legacy/unknown strings
# HARDENING.md P0.3: role is NOT client-controllable. Self-registration (``admin``, ``superadmin``, ``technician``, …) are rejected here so junk
# always creates the least-privilege role; privileged roles are assigned roles can never be (re)created through the API.
# by an admin directly in the DB (or a future admin-gated endpoint). """
role: str = "CS Rep" # kept for backward compat; ignored by the service if value is None:
return None
role = value.strip()
if role not in CANONICAL_ROLES:
raise ValueError(
f"Unknown role '{value}'. Allowed roles: {', '.join(CANONICAL_ROLES)}"
)
return role
class LoginRequest(BaseModel): class LoginRequest(BaseModel):
@@ -40,3 +49,50 @@ class UserOut(BaseModel):
active: bool active: bool
model_config = {"from_attributes": True} model_config = {"from_attributes": True}
# ── Admin user management (self-registration is removed) ──────────────
class AdminCreateUserRequest(BaseModel):
"""Admin-created user. The role is mandatory and must be canonical.
``role`` is deliberately NOT optional and has no default — an admin must
state the intended role explicitly; the server never infers one.
"""
email: str = Field(min_length=1)
password: str = Field(min_length=8, description="Minimum 8 characters")
full_name: str = Field(min_length=1)
phone: str | None = None
role: str
@field_validator("role")
@classmethod
def _role_canonical(cls, value: str) -> str:
return _validate_canonical_role(value) # type: ignore[return-value]
@field_validator("email")
@classmethod
def _lower_email(cls, value: str) -> str:
return value.strip().lower()
class AdminUpdateUserRequest(BaseModel):
"""Admin edits to an existing user: role change and/or deactivation.
At least one field must be present. ``active=False`` deactivates the
account (login and token refresh then fail closed).
"""
role: str | None = None
active: bool | None = None
@field_validator("role")
@classmethod
def _role_canonical(cls, value: str | None) -> str | None:
return _validate_canonical_role(value)
@model_validator(mode="after")
def _at_least_one_field(self) -> AdminUpdateUserRequest:
if self.role is None and self.active is None:
raise ValueError("Provide at least one of 'role' or 'active'")
return self
+146 -40
View File
@@ -1,11 +1,16 @@
"""Authentication service — register, login, refresh.""" """Authentication service — login, refresh, and admin user management.
Self-registration was removed (HARDENING/P0 batch): users are created and
managed exclusively by admins through the admin user-management endpoints.
"""
from __future__ import annotations from __future__ import annotations
from fastapi import HTTPException, status from fastapi import HTTPException, status
from sqlalchemy import select from sqlalchemy import func, select
from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.ext.asyncio import AsyncSession
from app.core.roles import is_known_role, normalize_role
from app.core.security import ( from app.core.security import (
create_access_token, create_access_token,
create_refresh_token, create_refresh_token,
@@ -13,47 +18,35 @@ from app.core.security import (
hash_password, hash_password,
verify_password, verify_password,
) )
from app.models.ticket import Escalation, Ticket, TicketTimeline
from app.models.user import User from app.models.user import User
from app.schemas.auth import RegisterRequest from app.schemas.auth import AdminCreateUserRequest, AdminUpdateUserRequest
# HARDENING.md P0.3 — least-privilege default for self-registered users. _UNAUTHORIZED = status.HTTP_401_UNAUTHORIZED
_SELF_REGISTER_ROLE = "CS Rep"
async def register(db: AsyncSession, body: RegisterRequest) -> User:
"""Create a new user. Raises 409 if email already exists.
HARDENING.md P0.3: unauthenticated self-registration must never mint a
privileged role. The client-supplied ``role`` field is IGNORED — new
self-registered users always land on the least-privilege role.
Admins assign elevated roles directly (DB seed / admin endpoint).
"""
result = await db.execute(select(User).where(User.email == body.email))
if result.scalar_one_or_none():
raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered")
user = User(
email=body.email,
password_hash=hash_password(body.password),
full_name=body.full_name,
phone=body.phone,
role=_SELF_REGISTER_ROLE,
)
db.add(user)
await db.flush()
await db.refresh(user)
return user
# ── AuthN ────────────────────────────────────────────────────────────
async def login(db: AsyncSession, email: str, password: str) -> tuple[str, str, User]: async def login(db: AsyncSession, email: str, password: str) -> tuple[str, str, User]:
"""Authenticate and return (access_token, refresh_token, user).""" """Authenticate and return (access_token, refresh_token, user).
result = await db.execute(select(User).where(User.email == email))
user = result.scalar_one_or_none()
if user is None or not verify_password(password, user.password_hash):
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid email or password")
if not user.active:
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Account is inactive")
Fails closed (401) for bad credentials, inactive accounts, and any user
whose stored role is not part of the unified role model.
"""
result = await db.execute(
select(User)
.where(func.lower(User.email) == email.strip().lower())
.order_by(User.id)
)
user = result.scalars().first()
if user is None or not verify_password(password, user.password_hash):
raise HTTPException(status_code=_UNAUTHORIZED, detail="Invalid email or password")
if not user.active:
raise HTTPException(status_code=_UNAUTHORIZED, detail="Account is inactive")
if not is_known_role(user.role):
raise HTTPException(
status_code=_UNAUTHORIZED,
detail="Account role is not recognised; contact an administrator",
)
access_token = create_access_token({"sub": str(user.id)}) access_token = create_access_token({"sub": str(user.id)})
refresh_token = create_refresh_token({"sub": str(user.id)}) refresh_token = create_refresh_token({"sub": str(user.id)})
return access_token, refresh_token, user return access_token, refresh_token, user
@@ -64,18 +57,131 @@ async def refresh_access_token(db: AsyncSession, token: str) -> tuple[str, str]:
try: try:
payload = decode_token(token) payload = decode_token(token)
if payload.get("type") != "refresh": if payload.get("type") != "refresh":
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token type") raise HTTPException(status_code=_UNAUTHORIZED, detail="Invalid token type")
except HTTPException: except HTTPException:
raise raise
except Exception: except Exception:
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid refresh token") raise HTTPException(status_code=_UNAUTHORIZED, detail="Invalid refresh token")
user_id: int = int(payload["sub"]) user_id: int = int(payload["sub"])
result = await db.execute(select(User).where(User.id == user_id)) result = await db.execute(select(User).where(User.id == user_id))
user = result.scalar_one_or_none() user = result.scalar_one_or_none()
if user is None or not user.active: if user is None or not user.active:
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found or inactive") raise HTTPException(status_code=_UNAUTHORIZED, detail="User not found or inactive")
if not is_known_role(user.role):
raise HTTPException(
status_code=_UNAUTHORIZED,
detail="Account role is not recognised; contact an administrator",
)
new_access = create_access_token({"sub": str(user.id)}) new_access = create_access_token({"sub": str(user.id)})
new_refresh = create_refresh_token({"sub": str(user.id)}) new_refresh = create_refresh_token({"sub": str(user.id)})
return new_access, new_refresh return new_access, new_refresh
# ── Admin user management ────────────────────────────────────────────
async def _get_user_or_404(db: AsyncSession, user_id: int) -> User:
result = await db.execute(select(User).where(User.id == user_id))
user = result.scalar_one_or_none()
if user is None:
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User not found")
return user
async def create_user(db: AsyncSession, body: AdminCreateUserRequest) -> User:
"""Admin-created user with an explicit, canonical role. 409 on duplicate email."""
email = body.email.strip().lower()
result = await db.execute(select(User).where(func.lower(User.email) == email))
if result.scalar_one_or_none():
raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered")
# Defense in depth: schema already guarantees a canonical role.
role = normalize_role(body.role)
if role is None:
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Unknown role")
user = User(
email=email,
password_hash=hash_password(body.password),
full_name=body.full_name.strip(),
phone=body.phone,
role=role,
)
db.add(user)
await db.flush()
await db.refresh(user)
return user
async def update_user(
db: AsyncSession,
actor: User,
user_id: int,
body: AdminUpdateUserRequest,
) -> User:
"""Admin role-change / deactivation for an existing user.
Guards:
* an admin cannot modify their own account through the API (self-lockout);
* role changes are limited to the canonical taxonomy.
The ≥1-active-admin invariant holds structurally: only admins can demote
admins, and no admin can demote/deactivate themselves, so at least one
canonical admin always remains.
"""
user = await _get_user_or_404(db, user_id)
if actor.id == user.id:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail="Admins cannot change their own role or active state through the API",
)
new_role = normalize_role(body.role) if body.role is not None else None
new_active = body.active
if new_role is not None:
user.role = new_role
if new_active is not None:
user.active = new_active
await db.flush()
await db.refresh(user)
return user
async def delete_user(db: AsyncSession, actor: User, user_id: int) -> None:
"""Admin deletes a user account (hard delete).
Guards:
* an admin cannot delete their own account (self-guard also keeps the
≥1-active-admin invariant: admins can never remove themselves);
* users referenced by tickets / timeline / escalations are kept (409) so
historical data never dangles — reassign or deactivate instead.
"""
user = await _get_user_or_404(db, user_id)
if actor.id == user.id:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail="Admins cannot delete their own account through the API",
)
referenced = False
for clause in (
select(func.count(Ticket.id)).where(Ticket.assigned_to == user_id),
select(func.count(TicketTimeline.id)).where(TicketTimeline.user_id == user_id),
select(func.count(Escalation.id)).where(Escalation.escalated_to == user_id),
):
count = (await db.execute(clause)).scalar() or 0
if count:
referenced = True
break
if referenced:
raise HTTPException(
status_code=status.HTTP_409_CONFLICT,
detail="User has related tickets, timeline entries, or escalations; "
"reassign or deactivate instead of deleting",
)
await db.delete(user)
await db.flush()
+83
View File
@@ -13,6 +13,7 @@ from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.ext.asyncio import AsyncSession
from app.core.security import hash_password from app.core.security import hash_password
from app.core.roles import is_blocked_legacy_role, normalize_role
from app.models.unit import Unit from app.models.unit import Unit
from app.models.user import User from app.models.user import User
from app.models.category import Category from app.models.category import Category
@@ -41,6 +42,88 @@ SEED_USERS_DATA = [
] ]
async def normalize_legacy_user_roles(db: AsyncSession) -> int:
"""Converge legacy role strings onto the unified canonical taxonomy.
Databases built before the P0 role-model batch can hold nickname roles
(``technician``, ``cs``, ``fm``, ``ceo`` …) minted by the old open
self-registration. Unambiguous aliases are rewritten to their canonical
role so RBAC keeps working. Ambiguous/unknown roles (e.g. lowercase
``admin``/``superadmin``) are NOT auto-mapped — they fail closed at login
and JWT validation until an operator remediates the row.
Returns the number of rows rewritten. Idempotent.
"""
result = await db.execute(select(User))
changed = 0
for user in result.scalars().all():
canonical = normalize_role(user.role)
if canonical and canonical != user.role:
logger.info("Normalizing legacy role %r → %r for %s", user.role, canonical, user.email)
user.role = canonical
changed += 1
elif canonical is None and not is_blocked_legacy_role(user.role):
logger.warning(
"User %s has unrecognized role %r; login will be denied until fixed",
user.email,
user.role,
)
if changed:
await db.flush()
return changed
async def normalize_legacy_user_emails(db: AsyncSession) -> int:
"""Lowercase stored user emails to match the normalized login lookup.
Databases built before the P0 batch can hold mixed-case emails (the old
open self-registration stored them verbatim) while login now compares on
the lowercase form, so such rows would otherwise be silently locked out.
Rewrites each stored email to its stripped/lowercase form. When two rows
share an email that differs only in case, only the lowest-id row becomes
canonical (any others keep their stored value and are logged as a warning)
so the unique constraint is never violated. Idempotent; returns the
number of rows rewritten.
"""
result = await db.execute(select(User).order_by(User.id))
users = list(result.scalars().all())
groups: dict[str, list[User]] = {}
for user in users:
groups.setdefault(user.email.strip().lower(), []).append(user)
changed = 0
for normalized, members in groups.items():
if len(members) == 1:
user = members[0]
if user.email != normalized:
logger.info(
"Normalizing legacy email %r → %r for user %d", user.email, normalized, user.id
)
user.email = normalized
changed += 1
continue
if any(user.email == normalized for user in members):
losers = [u for u in members if u.email != normalized]
else:
winner = members[0]
logger.info(
"Normalizing legacy email %r → %r for user %d", winner.email, normalized, winner.id
)
winner.email = normalized
changed += 1
losers = members[1:]
for loser in losers:
logger.warning(
"Cannot normalize email %r for user %d: another account already holds "
"that normalized email; keeping the stored value",
normalized,
loser.id,
)
if changed:
await db.flush()
return changed
async def seed_users(db: AsyncSession, default_password: str = "denya123") -> list[User]: async def seed_users(db: AsyncSession, default_password: str = "denya123") -> list[User]:
"""Insert seed users if they don't already exist.""" """Insert seed users if they don't already exist."""
hashed = hash_password(default_password) hashed = hash_password(default_password)
+11 -1
View File
@@ -17,17 +17,27 @@ 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 # noqa: E402
import pytest_asyncio # noqa: E402 (DATABASE_URL must be set before app imports) 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
from app.core.ratelimit import login_rate_limiter # noqa: E402
from app.main import app # noqa: E402 from app.main import app # noqa: E402
from app.models.ticket import Ticket # noqa: E402 from app.models.ticket import Ticket # noqa: E402
from app.services.seed import seed_categories, seed_units, seed_users # noqa: E402 from app.services.seed import seed_categories, seed_units, seed_users # noqa: E402
@pytest.fixture
def _reset_login_rate_limiter():
"""Isolate login rate-limit state between tests (shared in-process store)."""
login_rate_limiter.reset()
yield
login_rate_limiter.reset()
@pytest_asyncio.fixture @pytest_asyncio.fixture
async def client(): async def client(_reset_login_rate_limiter):
"""Async test client with a fresh, seeded database per test.""" """Async test client with a fresh, seeded database per test."""
async with engine.begin() as conn: async with engine.begin() as conn:
await conn.run_sync(Base.metadata.create_all) await conn.run_sync(Base.metadata.create_all)
+610
View File
@@ -0,0 +1,610 @@
"""P0 security batch — admin user management, unified role model, login rate
limiting, WhatsApp webhook secret, mock-log auth, security headers, pagination.
Each item in the P0 hardening batch has an executable behavioral test here
(plus the register-removal tests living in test_p0_hardening.py).
"""
from __future__ import annotations
import pytest
from httpx import AsyncClient
from app.core.config import settings
from app.core.database import async_session_factory
from app.core.security import hash_password
from app.models.user import User
from app.services.seed import normalize_legacy_user_emails, normalize_legacy_user_roles
pytestmark = pytest.mark.asyncio
# ── helpers ───────────────────────────────────────────────────────────
async def _login(client, email="wahab@denya.com", password="denya123") -> str:
resp = await client.post("/api/auth/login", json={"email": email, "password": password})
assert resp.status_code == 200, resp.text
return resp.json()["access_token"]
def _auth(token: str) -> dict[str, str]:
return {"Authorization": f"Bearer {token}"}
async def _insert_user(email: str, role: str, *, active: bool = True) -> int:
"""Insert a user row directly (bypasses API role validation) — used to
simulate legacy bootstrap/registration rows in the DB."""
async with async_session_factory() as session:
user = User(
email=email,
password_hash=hash_password("denya123"),
full_name=f"Legacy {email}",
role=role,
active=active,
)
session.add(user)
await session.commit()
return user.id
async def _normalize_roles() -> None:
async with async_session_factory() as session:
await normalize_legacy_user_roles(session)
await session.commit()
async def _normalize_emails() -> None:
async with async_session_factory() as session:
await normalize_legacy_user_emails(session)
await session.commit()
async def _create_ticket(client, token: str, **overrides) -> dict:
payload = {
"unit_id": 2,
"category_id": 3,
"priority": "medium",
"reporter": "P0 Batch Test",
"reported_via": "walk-in",
"description": "p0 batch ticket",
**overrides,
}
resp = await client.post("/api/tickets", json=payload, headers=_auth(token))
assert resp.status_code == 201, resp.text
return resp.json()
# ── Item 2/3: admin user management ───────────────────────────────────
async def test_create_user_requires_admin(client: AsyncClient):
token = await _login(client, email="bella@denya.com") # CS Rep
resp = await client.post(
"/api/auth/users",
json={"email": "x@example.com", "password": "password1", "full_name": "X", "role": "CS Rep"},
headers=_auth(token),
)
assert resp.status_code == 403
async def test_create_user_requires_auth(client: AsyncClient):
resp = await client.post(
"/api/auth/users",
json={"email": "x@example.com", "password": "password1", "full_name": "X", "role": "CS Rep"},
)
assert resp.status_code == 401
async def test_admin_creates_user_with_forced_role(client: AsyncClient):
token = await _login(client) # Admin/Wahab
resp = await client.post(
"/api/auth/users",
json={"email": "New.Tech@Example.com", "password": "password1", "full_name": "New Tech", "role": "Tech"},
headers=_auth(token),
)
assert resp.status_code == 201, resp.text
created = resp.json()
assert created["role"] == "Tech"
assert created["active"] is True
assert created["email"] == "new.tech@example.com" # normalized lower-case
# The new user can actually log in with their forced role.
login = await client.post(
"/api/auth/login", json={"email": "new.tech@example.com", "password": "password1"}
)
assert login.status_code == 200
me = await client.get("/api/auth/me", headers=_auth(login.json()["access_token"]))
assert me.json()["role"] == "Tech"
async def test_admin_create_user_rejects_unknown_roles(client: AsyncClient):
"""Unified role model: junk/legacy roles cannot be minted at creation."""
token = await _login(client)
for role in ("admin", "superadmin", "technician", "root"):
resp = await client.post(
"/api/auth/users",
json={"email": f"{role.strip().lower()}@example.com", "password": "password1", "full_name": "X", "role": role},
headers=_auth(token),
)
assert resp.status_code == 422, (role, resp.text)
async def test_admin_create_user_duplicate_email_conflict(client: AsyncClient):
token = await _login(client)
payload = {"email": "dupe2@example.com", "password": "password1", "full_name": "D", "role": "CS Rep"}
assert (await client.post("/api/auth/users", json=payload, headers=_auth(token))).status_code == 201
resp = await client.post("/api/auth/users", json=payload, headers=_auth(token))
assert resp.status_code == 409
async def test_patch_user_role_change(client: AsyncClient):
token = await _login(client)
users = (await client.get("/api/auth/users", headers=_auth(token))).json()
tech = next(u for u in users if u["role"] == "Tech")
resp = await client.patch(
f"/api/auth/users/{tech['id']}",
json={"role": "CS Rep"},
headers=_auth(token),
)
assert resp.status_code == 200, resp.text
assert resp.json()["role"] == "CS Rep"
assert resp.json()["active"] is True
async def test_patch_user_rejects_unknown_role(client: AsyncClient):
token = await _login(client)
users = (await client.get("/api/auth/users", headers=_auth(token))).json()
tech = next(u for u in users if u["role"] == "Tech")
resp = await client.patch(
f"/api/auth/users/{tech['id']}",
json={"role": "superadmin"},
headers=_auth(token),
)
assert resp.status_code == 422, resp.text
async def test_patch_deactivate_blocks_login(client: AsyncClient):
token = await _login(client)
users = (await client.get("/api/auth/users", headers=_auth(token))).json()
tech = next(u for u in users if u["role"] == "Tech")
resp = await client.patch(
f"/api/auth/users/{tech['id']}",
json={"active": False},
headers=_auth(token),
)
assert resp.status_code == 200
assert resp.json()["active"] is False
login = await client.post(
"/api/auth/login", json={"email": tech["email"], "password": "denya123"}
)
assert login.status_code == 401
async def test_admin_cannot_modify_own_account(client: AsyncClient):
token = await _login(client) # wahab
me = (await client.get("/api/auth/me", headers=_auth(token))).json()
resp = await client.patch(
f"/api/auth/users/{me['id']}", json={"active": False}, headers=_auth(token)
)
assert resp.status_code == 400
async def test_admin_can_demote_other_admin_but_not_self(client: AsyncClient):
"""An admin can manage the other admin seat, but self-removal stays blocked,
so at least one canonical admin always remains (structural invariant)."""
token = await _login(client) # wahab
users = (await client.get("/api/auth/users", headers=_auth(token))).json()
jerome = next(u for u in users if u["role"] == "Admin/Jerome")
wahab = next(u for u in users if u["role"] == "Admin/Wahab")
# Demote the OTHER admin → allowed, wahab is still the acting admin.
resp = await client.patch(
f"/api/auth/users/{jerome['id']}", json={"role": "CEO"}, headers=_auth(token)
)
assert resp.status_code == 200, resp.text
# Demoting/deactivating yourself is always rejected.
resp = await client.patch(
f"/api/auth/users/{wahab['id']}", json={"active": False}, headers=_auth(token)
)
assert resp.status_code == 400
async def test_delete_user_admin_only_and_works(client: AsyncClient):
admin_token = await _login(client)
users = (await client.get("/api/auth/users", headers=_auth(admin_token))).json()
tech = next(u for u in users if u["role"] == "Tech")
# Non-admin cannot delete.
cs_token = await _login(client, email="bella@denya.com")
resp = await client.delete(f"/api/auth/users/{tech['id']}", headers=_auth(cs_token))
assert resp.status_code == 403
# Admin deletes → 204, user gone, login fails.
resp = await client.delete(f"/api/auth/users/{tech['id']}", headers=_auth(admin_token))
assert resp.status_code == 204
login = await client.post(
"/api/auth/login", json={"email": tech["email"], "password": "denya123"}
)
assert login.status_code == 401
async def test_delete_user_referenced_by_ticket_is_409(client: AsyncClient):
token = await _login(client)
users = (await client.get("/api/auth/users", headers=_auth(token))).json()
tech = next(u for u in users if u["role"] == "Tech")
ticket = await _create_ticket(client, token)
resp = await client.patch(
f"/api/tickets/{ticket['id']}",
json={"assigned_to": tech["id"]},
headers=_auth(token),
)
assert resp.status_code == 200, resp.text
resp = await client.delete(f"/api/auth/users/{tech['id']}", headers=_auth(token))
assert resp.status_code == 409
assert "related" in resp.json()["detail"].lower()
async def test_admin_cannot_delete_self(client: AsyncClient):
token = await _login(client)
me = (await client.get("/api/auth/me", headers=_auth(token))).json()
resp = await client.delete(f"/api/auth/users/{me['id']}", headers=_auth(token))
assert resp.status_code == 400
# ── Item 3: unified role model — legacy rows & JWT validation ─────────
async def test_legacy_alias_role_normalized_at_startup(client: AsyncClient):
"""A legacy 'technician' row is mapped onto canonical 'Tech' at startup."""
await _insert_user("legacy-tech@example.com", "technician")
await _normalize_roles() # what lifespan does each boot
login = await client.post(
"/api/auth/login", json={"email": "legacy-tech@example.com", "password": "denya123"}
)
assert login.status_code == 200, login.text
me = await client.get("/api/auth/me", headers=_auth(login.json()["access_token"]))
assert me.json()["role"] == "Tech"
async def test_login_rejects_unknown_legacy_role_fail_closed(client: AsyncClient):
"""Lowercase 'superadmin' rows (mintable by the old open register) cannot
log in — fail closed, never granted admin powers."""
await _insert_user("legacy-admin@example.com", "superadmin")
# NOTE: no normalize call — the row is exactly what the live DB holds today.
login = await client.post(
"/api/auth/login", json={"email": "legacy-admin@example.com", "password": "denya123"}
)
assert login.status_code == 401
assert "role" in login.json()["detail"].lower()
async def test_jwt_validation_rejects_unknown_role(client: AsyncClient):
"""A token for a user whose row later becomes junk-role must fail closed."""
token = await _login(client) # wahab is a canonical admin at token time
me = (await client.get("/api/auth/me", headers=_auth(token))).json()
# Simulate a legacy DB row flip to a non-canonical role.
async with async_session_factory() as session:
from sqlalchemy import select
user = (await session.execute(select(User).where(User.id == me["id"]))).scalar_one()
user.role = "admin"
await session.commit()
resp = await client.get("/api/auth/me", headers=_auth(token))
assert resp.status_code == 401
async def test_normalize_does_not_map_ambiguous_admin_alias(client: AsyncClient):
"""normalize_legacy_user_roles leaves identity-ambiguous 'admin' rows for
operator remediation instead of guessing a canonical admin."""
await _insert_user("legacy-admin2@example.com", "admin")
await _normalize_roles()
async with async_session_factory() as session:
from sqlalchemy import select
user = (
await session.execute(select(User).where(User.email == "legacy-admin2@example.com"))
).scalar_one()
assert user.role == "admin"
# ── P0 email normalization (legacy mixed-case rows) ───────────────────
async def test_legacy_mixed_case_email_migrated_and_authenticates(client: AsyncClient):
"""A legacy row whose email was stored verbatim in mixed case (the old open
register) is lowercased by the startup self-heal and still authenticates."""
await _insert_user("DemoUser@Example.com", "Tech")
await _normalize_emails() # what lifespan does each boot
async with async_session_factory() as session:
from sqlalchemy import select
user = (
await session.execute(select(User).where(User.email == "demouser@example.com"))
).scalar_one()
assert user.email == "demouser@example.com"
for variant in ("demouser@example.com", "DemoUser@Example.com"):
resp = await client.post(
"/api/auth/login", json={"email": variant, "password": "denya123"}
)
assert resp.status_code == 200, resp.text
async def test_login_matches_legacy_mixed_case_email_before_migration(client: AsyncClient):
"""Login compares on the normalized form, so an un-migrated mixed-case row
is still matched by its lowercase login (no hard dependency on the
self-heal having run)."""
await _insert_user("DemoUser@Example.com", "Tech")
resp = await client.post(
"/api/auth/login", json={"email": "demouser@example.com", "password": "denya123"}
)
assert resp.status_code == 200, resp.text
async def test_legacy_email_normalization_is_idempotent(client: AsyncClient):
"""The startup self-heal rewrites once and no-ops on subsequent boots."""
await _insert_user("DemoUser@Example.com", "Tech")
async with async_session_factory() as session:
first = await normalize_legacy_user_emails(session)
await session.commit()
async with async_session_factory() as session:
second = await normalize_legacy_user_emails(session)
await session.commit()
assert first == 1
assert second == 0
async def test_create_user_rejects_case_variant_of_legacy_email(client: AsyncClient):
"""The admin create-user duplicate check compares on the normalized form:
creating a case-variant of a legacy mixed-case row returns 409, not 201."""
await _insert_user("DemoUser@Example.com", "Tech")
token = await _login(client)
resp = await client.post(
"/api/auth/users",
json={
"email": "demouser@example.com",
"password": "password1",
"full_name": "X",
"role": "Tech",
},
headers=_auth(token),
)
assert resp.status_code == 409, resp.text
async def test_admin_only_rbac_gate(client: AsyncClient):
"""Canonical admins pass /api/auth/admin-only; everyone else 403."""
wahab = await _login(client)
assert (await client.get("/api/auth/admin-only", headers=_auth(wahab))).status_code == 200
bella = await _login(client, email="bella@denya.com")
assert (await client.get("/api/auth/admin-only", headers=_auth(bella))).status_code == 403
# ── Item 4: login rate limiting ───────────────────────────────────────
async def test_login_rate_limited_after_five_failures(client: AsyncClient):
email, password = "rate-limited@example.com", "denya123"
# Make sure the account exists with a valid password.
token = await _login(client)
await client.post(
"/api/auth/users",
json={"email": email, "password": password, "full_name": "Rate", "role": "CS Rep"},
headers=_auth(token),
)
for _ in range(5):
resp = await client.post(
"/api/auth/login", json={"email": email, "password": "wrong-password"}
)
assert resp.status_code == 401
# 6th attempt — even with the CORRECT password — is throttled.
resp = await client.post(
"/api/auth/login", json={"email": email, "password": password}
)
assert resp.status_code == 429, resp.text
async def test_rate_limit_is_per_email(client: AsyncClient):
"""Failures for one account never lock out another account."""
token = await _login(client)
for email in ("victim@example.com", "other@example.com"):
await client.post(
"/api/auth/users",
json={"email": email, "password": "password1", "full_name": "U", "role": "CS Rep"},
headers=_auth(token),
)
for _ in range(6):
resp = await client.post(
"/api/auth/login", json={"email": "victim@example.com", "password": "bad"}
)
assert resp.status_code in (401, 429)
# Unaffected account still logs in fine.
resp = await client.post(
"/api/auth/login", json={"email": "other@example.com", "password": "password1"}
)
assert resp.status_code == 200, resp.text
async def test_rate_limit_window_expires(client: AsyncClient, monkeypatch):
"""After the 15-minute window passes, the account can log in again."""
import time as _time
import app.core.ratelimit as ratelimit_mod
email, password = "window@example.com", "password1"
token = await _login(client)
await client.post(
"/api/auth/users",
json={"email": email, "password": password, "full_name": "W", "role": "CS Rep"},
headers=_auth(token),
)
# Pin the limiter clock so the window can be fast-forwarded deterministically.
clock = {"now": _time.time()}
monkeypatch.setattr(ratelimit_mod, "_now", lambda: clock["now"])
for _ in range(5):
await client.post("/api/auth/login", json={"email": email, "password": "bad"})
blocked = await client.post("/api/auth/login", json={"email": email, "password": password})
assert blocked.status_code == 429, blocked.text
clock["now"] += settings.LOGIN_RATE_LIMIT_WINDOW_SECONDS + 1
resp = await client.post("/api/auth/login", json={"email": email, "password": password})
assert resp.status_code == 200, resp.text
# ── Item 5: WhatsApp webhook secret (fail closed) ─────────────────────
WEBHOOK_BODY = {
"object": "whatsapp_business_account",
"entry": [
{
"id": "1",
"changes": [
{
"id": "wamid.1",
"message": {"from": "+233000000000", "id": "wamid.1", "text": {"text": "AC leaking"}},
}
],
}
],
}
async def test_webhook_fail_closed_when_env_unset(client: AsyncClient):
"""WHATSAPP_WEBHOOK_SECRET unset ⇒ every message rejected (403)."""
assert settings.WHATSAPP_WEBHOOK_SECRET == "" # test env default is unset
resp = await client.post("/api/whatsapp/webhook", json=WEBHOOK_BODY)
assert resp.status_code == 403
assert "not configured" in resp.json()["detail"].lower()
async def test_webhook_rejects_missing_or_wrong_secret(client: AsyncClient, monkeypatch):
monkeypatch.setattr(settings, "WHATSAPP_WEBHOOK_SECRET", "test-webhook-secret")
resp = await client.post("/api/whatsapp/webhook", json=WEBHOOK_BODY)
assert resp.status_code == 403
resp = await client.post(
"/api/whatsapp/webhook", json=WEBHOOK_BODY, headers={"X-Webhook-Secret": "wrong"}
)
assert resp.status_code == 403
async def test_webhook_accepts_valid_secret_and_creates_ticket(client: AsyncClient, monkeypatch):
from app.routers import whatsapp as whatsapp_router
async def _fake_reply(to_phone: str, text: str):
from app.schemas.whatsapp import WhatsAppReplyResponse
return WhatsAppReplyResponse(success=True, message="sent")
monkeypatch.setattr(settings, "WHATSAPP_WEBHOOK_SECRET", "test-webhook-secret")
monkeypatch.setattr(whatsapp_router, "send_whatsapp_reply", _fake_reply)
resp = await client.post(
"/api/whatsapp/webhook",
json=WEBHOOK_BODY,
headers={"X-Webhook-Secret": "test-webhook-secret"},
)
assert resp.status_code == 200, resp.text
data = resp.json()
assert data["status"] == "processed"
assert data["ticket_number"].startswith("PAV-")
# The message is logged and visible to an authenticated mock-log caller.
token = await _login(client)
log = await client.get("/api/whatsapp/mock-log", headers=_auth(token))
assert log.status_code == 200
assert len(log.json()) == 1
assert log.json()[0]["ticket_number"] == data["ticket_number"]
async def test_webhook_verify_token_mismatch_403(client: AsyncClient, monkeypatch):
monkeypatch.setattr(settings, "WHATSAPP_VERIFY_TOKEN", "verify-me")
resp = await client.get(
"/api/whatsapp/webhook",
params={"hub.mode": "subscribe", "hub.verify_token": "nope", "hub.challenge": "1234"},
)
assert resp.status_code == 403
async def test_webhook_verify_token_match_returns_challenge(client: AsyncClient, monkeypatch):
monkeypatch.setattr(settings, "WHATSAPP_VERIFY_TOKEN", "verify-me")
resp = await client.get(
"/api/whatsapp/webhook",
params={"hub.mode": "subscribe", "hub.verify_token": "verify-me", "hub.challenge": "1234"},
)
assert resp.status_code == 200
assert resp.json() == {"challenge": "1234"}
# ── Item 6: mock-log requires auth ────────────────────────────────────
async def test_mock_log_requires_auth(client: AsyncClient):
resp = await client.get("/api/whatsapp/mock-log")
assert resp.status_code == 401, resp.text
token = await _login(client)
resp = await client.get("/api/whatsapp/mock-log", headers=_auth(token))
assert resp.status_code == 200
assert resp.json() == []
# ── Item 7: security headers ──────────────────────────────────────────
async def test_security_headers_on_html_page(client: AsyncClient):
resp = await client.get("/login")
assert resp.status_code == 200
assert resp.headers["x-frame-options"] == "DENY"
assert resp.headers["x-content-type-options"] == "nosniff"
assert "content-security-policy" in resp.headers
assert "default-src 'self'" in resp.headers["content-security-policy"]
async def test_csp_only_on_html_not_json_api(client: AsyncClient):
resp = await client.get("/api/tickets")
assert resp.status_code == 200
assert "content-type" in resp.headers and resp.headers["content-type"].startswith("application/json")
assert "content-security-policy" not in resp.headers
# Frame/type hardening headers apply everywhere.
assert resp.headers["x-frame-options"] == "DENY"
assert resp.headers["x-content-type-options"] == "nosniff"
async def test_hsts_only_when_tls_terminates(client: AsyncClient):
plain = await client.get("/login")
assert "strict-transport-security" not in plain.headers
tls = await client.get("/login", headers={"X-Forwarded-Proto": "https"})
assert tls.headers["strict-transport-security"] == "max-age=31536000; includeSubDomains"
# ── Item 8: pagination (page/limit) and sane max page size ────────────
async def test_limit_alias_over_cap_rejected(client: AsyncClient, seed_tickets):
await seed_tickets(10)
resp = await client.get("/api/tickets", params={"limit": 500})
assert resp.status_code == 422
async def test_limit_alias_paginates(client: AsyncClient, seed_tickets):
await seed_tickets(14)
resp = await client.get("/api/tickets", params={"page": 2, "limit": 5})
assert resp.status_code == 200
data = resp.json()
assert data["total"] == 14
assert len(data["items"]) == 5
assert data["page_size"] == 5
assert data["page"] == 2
async def test_page_beyond_last_returns_empty_with_total(client: AsyncClient, seed_tickets):
await seed_tickets(7)
resp = await client.get("/api/tickets", params={"page": 999, "page_size": 10})
assert resp.status_code == 200
data = resp.json()
assert data["items"] == []
assert data["total"] == 7
async def test_page_size_and_limit_conflict_is_422(client: AsyncClient, seed_tickets):
await seed_tickets(3)
resp = await client.get("/api/tickets", params={"page_size": 10, "limit": 20})
assert resp.status_code == 422
+29 -29
View File
@@ -1,8 +1,8 @@
"""P0 hardening regression tests (HARDENING.md P0.1 / P0.2 / P0.3). """P0 hardening regression tests (HARDENING.md P0.1 / P0.2 / P0.3).
Covers: Covers:
- P0.3: self-registration CANNOT mint a privileged role (role field ignored) - P0.3: self-registration is REMOVED — POST /api/auth/register returns 404 and
- P0.3: duplicate email still 409s no public path can mint a user at all (users are admin-created only).
- P0.1: app fails to import/boot with placeholder or missing SECRET_KEY - P0.1: app fails to import/boot with placeholder or missing SECRET_KEY
- P0.2: app fails to boot with CORS_ORIGINS="*" - P0.2: app fails to boot with CORS_ORIGINS="*"
""" """
@@ -15,18 +15,17 @@ import sys
from pathlib import Path from pathlib import Path
import pytest import pytest
from httpx import ASGITransport, AsyncClient from httpx import AsyncClient
REPO_ROOT = Path(__file__).resolve().parent.parent REPO_ROOT = Path(__file__).resolve().parent.parent
pytestmark = pytest.mark.asyncio pytestmark = pytest.mark.asyncio
# ── P0.3: self-registration is removed entirely ───────────────────────
# ── P0.3: registration role-escalation ──────────────────────────────── async def test_register_endpoint_is_removed(client: AsyncClient):
"""A raw unauthenticated register call must 404 — no public signup path."""
async def test_register_cannot_mint_admin_role(client: AsyncClient):
"""A raw unauthenticated register call must NOT be able to mint Admin/*."""
resp = await client.post( resp = await client.post(
"/api/auth/register", "/api/auth/register",
json={ json={
@@ -36,41 +35,42 @@ async def test_register_cannot_mint_admin_role(client: AsyncClient):
"role": "Admin/Jerome", "role": "Admin/Jerome",
}, },
) )
assert resp.status_code == 201, resp.text assert resp.status_code == 404, resp.text
created = resp.json()
assert created["role"] == "CS Rep", (
f"self-registration minted privileged role: {created['role']}"
)
async def test_register_role_wahab_also_blocked(client: AsyncClient): async def test_register_endpoint_removed_regardless_of_role(client: AsyncClient):
"""Attempts to mint privileged (or any) roles via register all 404."""
for role in ("Admin/Jerome", "Admin/Wahab", "admin", "superadmin", "CS Rep"):
resp = await client.post( resp = await client.post(
"/api/auth/register", "/api/auth/register",
json={ json={
"email": "attacker2@example.com", "email": f"attacker-{role.lower().replace('/', '-')}@example.com",
"password": "Sup3rSecret!", "password": "Sup3rSecret!",
"full_name": "Attacker Two", "full_name": "Attacker",
"role": "Admin/Wahab", "role": role,
}, },
) )
assert resp.status_code == 201 assert resp.status_code == 404, (role, resp.text)
assert resp.json()["role"] == "CS Rep"
async def test_register_duplicate_email_conflict(client: AsyncClient): async def test_register_does_not_create_user(client: AsyncClient):
payload = { """No user row is ever created through the removed register endpoint."""
"email": "dupe@example.com", await client.post(
"/api/auth/register",
json={
"email": "ghost@example.com",
"password": "Sup3rSecret!", "password": "Sup3rSecret!",
"full_name": "Dupe", "full_name": "Ghost",
} },
r1 = await client.post("/api/auth/register", json=payload) )
assert r1.status_code == 201 # The ghost account must not be able to log in.
r2 = await client.post("/api/auth/register", json=payload) resp = await client.post(
assert r2.status_code == 409 "/api/auth/login",
json={"email": "ghost@example.com", "password": "Sup3rSecret!"},
)
assert resp.status_code == 401, resp.text
# ── P0.1 / P0.2: fail-closed boot validation ──────────────────────────
def _boot_with_env(env_overrides: dict[str, str]) -> subprocess.CompletedProcess: def _boot_with_env(env_overrides: dict[str, str]) -> subprocess.CompletedProcess:
"""Try importing app.main in a subprocess with the given env; the import """Try importing app.main in a subprocess with the given env; the import
must fail (non-zero) when fail-closed validation trips.""" must fail (non-zero) when fail-closed validation trips."""