diff --git a/.env.example b/.env.example index 7b9110f..56ff850 100644 --- a/.env.example +++ b/.env.example @@ -13,3 +13,14 @@ WHATSAPP_PHONE_NUMBER_ID= WHATSAPP_ACCESS_TOKEN= WHATSAPP_VERIFY_TOKEN= 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 diff --git a/AGENTS.md b/AGENTS.md index 59691e5..15fd99c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,14 +47,24 @@ Users, units, and categories are auto-seeded on first startup via lifespan hook: | Method | Path | Auth | Description | |--------|------|------|-------------| | GET | `/health` | No | Health check | -| POST | `/api/auth/register` | No | Create user | -| POST | `/api/auth/login` | No | Get JWT tokens | +| POST | `/api/auth/login` | No | Get JWT tokens (rate-limited ~5 fails/15min/IP+email → 429) | | POST | `/api/auth/refresh` | Token | Refresh tokens | | GET | `/api/auth/me` | Bearer | Current user | | 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 | -| POST | `/api/whatsapp/mock` | No | Mock WhatsApp | -| GET | `/api/whatsapp/mock-log` | No | Recent mock submissions | +| POST | `/api/auth/users` | Admin/Jerome, Admin/Wahab | Admin creates a user (forced canonical role; unknown roles → 422) | +| 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/` | Method | Path | Auth | Description | @@ -73,7 +83,7 @@ Frontend: Alpine.js (CDN) + Tailwind CSS (CDN). Auth state in localStorage. Role | Method | Path | Auth | Description | |--------|------|------|-------------| | 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}/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) | @@ -88,9 +98,22 @@ Frontend: Alpine.js (CDN) + Tailwind CSS (CDN). Auth state in localStorage. Role ## Auth - JWT access (30min) + refresh (7d) tokens -- Roles: CS Rep, CS Manager, FM Dispatcher, Admin/Jerome, Admin/Wahab, Tech, CEO, Director -- Use `require_roles("Admin/Jerome", "Admin/Wahab")` dependency for RBAC -- `sub` claim holds string user ID +- **Unified role model** lives in `app/core/roles.py` (`CANONICAL_ROLES`, + `ADMIN_ROLES` = Admin/Jerome + Admin/Wahab, `ROLE_ALIASES` for legacy + 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) diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 120000 index 47dc3e3..0000000 --- a/CLAUDE.md +++ /dev/null @@ -1 +0,0 @@ -AGENTS.md \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..a9d4d26 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,2 @@ + +@AGENTS.md diff --git a/HARDENING.md b/HARDENING.md index 923aaec..27a7b5c 100644 --- a/HARDENING.md +++ b/HARDENING.md @@ -19,15 +19,18 @@ legacy-schema self-heal. **32 pytest tests pass.** - **Live:** container `denya-onecare` on LXC `scottdenya` (192.168.68.75:8000), image built 2026-08-02, `restart: unless-stopped`. -- **Known demo-only posture (must change):** `SECRET_KEY=change-me-in-production`, - `CORS_ORIGINS=*`, open `/api/auth/register`, SQLite backend, WhatsApp webhook +- **Demo-only posture resolved (PR #10 + P0 batch):** `SECRET_KEY` now fails + 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**. --- ## 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` - 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 @@ -37,7 +40,7 @@ - **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`). -### P0.2 Lock down CORS +### P0.2 Lock down CORS — **DONE (PR #10)** - **Files:** `app/main.py`, `docker-compose.yml` - `CORS_ORIGINS=*` + `allow_credentials=True` is an invalid/unsafe combo (browsers reject `*` with credentials anyway). Replace with an explicit @@ -47,18 +50,21 @@ - **Acceptance:** `settings.CORS_ORIGINS` is a comma-separated explicit list; the middleware builds an allow-list, not `["*"]`. -### P0.3 Gate user registration -- **File:** `app/routers/auth.py` (`POST /api/auth/register`) -- Today anyone on the network can self-register. Decide the model: - - **Recommended:** require an admin-issued invitation token, or restrict - registration to a seed/allowed list, or remove the open route and create - users only via seed/admin. - - If a public self-service resident/tenant signup is genuinely required - (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. -- **Acceptance:** a raw, unauthenticated register call can no longer mint an - `Admin/*` or `Director` account. +### P0.3 Gate user registration — **DONE (P0 batch, 2026-09)** +- Open `POST /api/auth/register` was removed entirely (404) — there is no sign-up UI. +- Users are admin-managed: `POST /api/auth/users` (Admin/Jerome + Admin/Wahab only) + creates users with a **forced canonical role** (unknown roles → 422); + `PATCH /api/auth/users/{id}` changes role / deactivates (self-modification → 400); + `DELETE /api/auth/users/{id}` is guarded (users referenced by + 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 + default of the least-privilege role** and rate-limiting — never able to mint + admin/FM roles. ### P0.4 Reconsider SQLite for the final product - **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), 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` -- The handler exists but no credentials are set. When wiring this week: - - Verify the `hub.verify_token` check is constant-time (compare with - `secrets.compare_digest`). **The GET verification path currently returns - `{"error": ...}` with HTTP 200** — flip to `403` on token mismatch. - - Validate **inbound messages only from Meta** — the webhook MUST authenticate - Meta's request signature (X-Hub-Signature-256 HMAC over the raw body with your - app secret) before processing, otherwise anyone who discovers the endpoint can - forge tickets. This is the single most important WhatsApp hardening item. - - Add per-sender rate limiting / dedupe on `wa_message_id` (webhook retries can - double-create tickets). Create an idempotency guard keyed on `wa_message_id`. - - Never log the raw access token; redact in `send_whatsapp_reply` error paths. -- **Acceptance:** a forged POST without the Meta signature is rejected; duplicate - `wa_message_id` does not create a second ticket; verify-token mismatch returns 403. +- **Done:** the `GET` handshake validates `hub.verify_token` with a constant-time + compare and returns **403 on mismatch**; inbound `POST`s are gated by the + `X-Webhook-Secret` header matching `WHATSAPP_WEBHOOK_SECRET` (fail-closed 403 + when the env var is unset — see `.env.example`); the debug + `GET /api/whatsapp/mock-log` now requires Bearer auth. +- **Still open:** Meta request-signature validation (`X-Hub-Signature-256` HMAC + over the raw body with the app secret — the shared-secret header above is the + interim gate); per-sender rate limiting / dedupe idempotency keyed on + `wa_message_id` (retries can still double-create tickets); redacting the raw + access token in `send_whatsapp_reply` error paths. +- **Acceptance (open items):** a forged POST without the Meta signature is + rejected; duplicate `wa_message_id` does not create a second ticket; + verify-token mismatch returns 403 (done). --- @@ -130,10 +136,13 @@ `X-Content-Type-Options: nosniff`. ### P1.6 API hardening & rate limiting -- Add rate limiting on `POST /api/auth/login` (brute-force) — per-IP/IP+account. -- Consider rate limits on ticket creation (spam / mass-creation). -- Normalize/validate `page_size` (already capped `le=200`) and pagination - tie-breaker (`id DESC` present — good). +- **Done (P0 batch):** `POST /api/auth/login` is rate-limited in-process — + ~5 failures / 15 min per IP+email → 429 (env-tunable `LOGIN_RATE_LIMIT_*`, + a successful login resets the window). **Open:** ticket-creation rate limiting + (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` (git-ignored) and inject at runtime. Never commit. 2. **Webhook handshake:** in Meta dashboard point the webhook URL at - `/api/whatsapp/webhook`. The GET verify path currently echoes - `hub.challenge` when the verify token matches — confirm this works, then apply - P0.5 (403 on mismatch, HMAC signature validation). + `/api/whatsapp/webhook`. The GET verify path echoes `hub.challenge` + when the verify token matches (constant-time compare; mismatch → 403). The + 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 of the raw body with your app secret, compared with `compare_digest`. 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) -- [ ] No default `SECRET_KEY`; app fails closed without a real key -- [ ] CORS is an explicit origin allow-list -- [ ] Self-registration cannot mint privileged roles (or is admin-gated/removed) +- [x] No default `SECRET_KEY`; app fails closed without a real key (PR #10) +- [x] CORS is an explicit origin allow-list (PR #10) +- [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 - [ ] TLS-terminated reverse proxy with real domain; no raw :8000 on WAN -- [ ] WhatsApp webhook: Meta signature validated, verify-token mismatch → 403, - idempotent on `wa_message_id`, real credentials injected at runtime -- [ ] Login/ticket rate limiting in place +- [x] Webhook POST gated by `X-Webhook-Secret` (fail-closed); verify-token + mismatch → 403; `mock-log` requires auth +- [ ] 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 - [ ] RBAC audited per-route; phone data access controlled - [ ] Expanded test suite (auth, WhatsApp, SLA boundary, uploads) — all green diff --git a/app/core/config.py b/app/core/config.py index 5a8cc51..75b530d 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -36,6 +36,13 @@ class Settings(BaseSettings): WHATSAPP_ACCESS_TOKEN: str = "" WHATSAPP_VERIFY_TOKEN: str = "" 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 ──────────────────────────────────────────────────────── BASE_DIR: Path = Path(__file__).resolve().parent.parent.parent diff --git a/app/core/ratelimit.py b/app/core/ratelimit.py new file mode 100644 index 0000000..26737fd --- /dev/null +++ b/app/core/ratelimit.py @@ -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, +) diff --git a/app/core/roles.py b/app/core/roles.py new file mode 100644 index 0000000..99ac37e --- /dev/null +++ b/app/core/roles.py @@ -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 diff --git a/app/core/security.py b/app/core/security.py index 6e0657d..1cb6b9e 100644 --- a/app/core/security.py +++ b/app/core/security.py @@ -15,6 +15,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.core.config import settings from app.core.database import get_db +from app.core.roles import is_known_role from app.models.user import User bearer_scheme = HTTPBearer(auto_error=False) @@ -90,6 +91,14 @@ async def get_current_user( user = result.scalar_one_or_none() if user is None or not user.active: 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 diff --git a/app/main.py b/app/main.py index 18800e7..d8003d9 100644 --- a/app/main.py +++ b/app/main.py @@ -14,7 +14,13 @@ from sqlalchemy import text from app.core.config import settings from app.core.database import Base, async_session_factory, engine 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__) @@ -67,6 +73,10 @@ async def lifespan(app: FastAPI): await ensure_legacy_schema(conn) async with async_session_factory() as 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 seed_units(session, json_path=str(settings.BASE_DIR / "apartment_mapping.json")) await session.commit() @@ -83,6 +93,68 @@ app = FastAPI( 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 "*") ── _origins = [o.strip() for o in settings.CORS_ORIGINS.split(",") if o.strip()] if "*" in _origins or not _origins: diff --git a/app/routers/auth.py b/app/routers/auth.py index 3cf7b2f..0280ab4 100644 --- a/app/routers/auth.py +++ b/app/routers/auth.py @@ -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 typing import Annotated -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, HTTPException, Request, status from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession 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.models.user import User from app.schemas.auth import ( + AdminCreateUserRequest, + AdminUpdateUserRequest, LoginRequest, RefreshRequest, - RegisterRequest, TokenResponse, UserOut, ) @@ -22,36 +29,29 @@ from app.services import auth as auth_service router = APIRouter(prefix="/api/auth", tags=["auth"]) - -@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) +_require_admin = require_roles(*ADMIN_ROLES) +# ── Public authN ───────────────────────────────────────────────────── @router.post("/login", response_model=TokenResponse) async def login( + request: Request, body: LoginRequest, db: Annotated[AsyncSession, Depends(get_db)], ) -> TokenResponse: - access, refresh, _user = await auth_service.login(db, body.email, body.password) + """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) + 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) @@ -71,7 +71,54 @@ async def me(current_user: Annotated[User, Depends(get_current_user)]) -> User: @router.get("/admin-only", response_model=UserOut) async def admin_only( - current_user: Annotated[User, Depends(require_roles("Admin/Jerome", "Admin/Wahab"))], + current_user: Annotated[User, Depends(_require_admin)], ) -> User: - """Example RBAC-protected endpoint — only Admins can access.""" + """Example RBAC-protected endpoint — only canonical Admins can access.""" 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) diff --git a/app/routers/tickets.py b/app/routers/tickets.py index c6405aa..9a60eee 100644 --- a/app/routers/tickets.py +++ b/app/routers/tickets.py @@ -13,6 +13,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from app.core.config import settings 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.models.category import Category from app.models.ticket import Ticket, TicketPhoto @@ -37,6 +38,13 @@ logger = logging.getLogger(__name__) 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 UPLOADS_DIR = settings.BASE_DIR / "uploads" UPLOADS_DIR.mkdir(parents=True, exist_ok=True) @@ -197,7 +205,10 @@ async def create_ticket( async def list_tickets( db: Annotated[AsyncSession, Depends(get_db)], 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), priority: str | None = Query(None), property: str | None = Query(None), @@ -208,7 +219,17 @@ async def list_tickets( date_from: datetime | None = Query(None), date_to: datetime | None = Query(None), ) -> 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( db, status_filter=status, @@ -221,10 +242,10 @@ async def list_tickets( date_from=date_from, date_to=date_to, page=page, - page_size=page_size, + page_size=effective_page_size, ) 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") @@ -270,7 +291,7 @@ async def update_ticket( async def delete_ticket( ticket_id: int, 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: """Delete a ticket and its children (timeline, photos, escalations). diff --git a/app/routers/whatsapp.py b/app/routers/whatsapp.py index 376d3fc..b24d422 100644 --- a/app/routers/whatsapp.py +++ b/app/routers/whatsapp.py @@ -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 import logging +import secrets from datetime import datetime, timezone from typing import Annotated import httpx -from fastapi import APIRouter, Depends, Query +from fastapi import APIRouter, Depends, Header, HTTPException, Query, status from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from app.core.config import settings 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.schemas.whatsapp import ( MetaWebhookRequest, @@ -62,24 +75,55 @@ async def send_whatsapp_reply( 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 ──────────────────────────────────────────────── -@router.post("/webhook") -async def whatsapp_webhook( +@router.get("/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, - db: Annotated[AsyncSession, Depends(get_db)], - hub_verify_token: str | None = Query(None, alias="hub.verify_token"), - mode: str | None = Query(None), - hub_challenge: str | None = Query(None), + db: AsyncSession, ) -> dict: - """Handle incoming WhatsApp webhook from Meta.""" - - # ── 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 ──────────────────────────────────── + """Create tickets + logs for inbound messages. Shared by the POST handler.""" if not body.entry: return {"status": "no entry"} @@ -153,13 +197,24 @@ async def whatsapp_webhook( 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]) async def mock_whatsapp_log( 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]: - """Return recent WhatsApp webhook submissions for debugging.""" + """Return recent WhatsApp webhook submissions (authenticated only).""" result = await db.execute( select(WhatsAppLog).order_by(WhatsAppLog.received_at.desc()).limit(limit) ) diff --git a/app/schemas/auth.py b/app/schemas/auth.py index ac55166..6f2e089 100644 --- a/app/schemas/auth.py +++ b/app/schemas/auth.py @@ -2,18 +2,27 @@ 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): - email: str - password: str - full_name: str - phone: str | None = None - # HARDENING.md P0.3: role is NOT client-controllable. Self-registration - # always creates the least-privilege role; privileged roles are assigned - # 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 +def _validate_canonical_role(value: str | None) -> str | None: + """Reject any role that is not part of the unified canonical taxonomy. + + The role vocabulary is closed: admin user-management must only ever mint + canonical roles (see app/core/roles.py). Legacy/unknown strings + (``admin``, ``superadmin``, ``technician``, …) are rejected here so junk + roles can never be (re)created through the API. + """ + 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): @@ -40,3 +49,50 @@ class UserOut(BaseModel): active: bool 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 diff --git a/app/services/auth.py b/app/services/auth.py index fd82810..a96a8b2 100644 --- a/app/services/auth.py +++ b/app/services/auth.py @@ -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 fastapi import HTTPException, status -from sqlalchemy import select +from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession +from app.core.roles import is_known_role, normalize_role from app.core.security import ( create_access_token, create_refresh_token, @@ -13,47 +18,35 @@ from app.core.security import ( hash_password, verify_password, ) +from app.models.ticket import Escalation, Ticket, TicketTimeline 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. -_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 +_UNAUTHORIZED = status.HTTP_401_UNAUTHORIZED +# ── AuthN ──────────────────────────────────────────────────────────── async def login(db: AsyncSession, email: str, password: str) -> tuple[str, str, 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") + """Authenticate and return (access_token, refresh_token, user). + 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)}) refresh_token = create_refresh_token({"sub": str(user.id)}) return access_token, refresh_token, user @@ -64,18 +57,131 @@ async def refresh_access_token(db: AsyncSession, token: str) -> tuple[str, str]: try: payload = decode_token(token) 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: raise 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"]) result = await db.execute(select(User).where(User.id == user_id)) user = result.scalar_one_or_none() 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_refresh = create_refresh_token({"sub": str(user.id)}) 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() diff --git a/app/services/seed.py b/app/services/seed.py index e8c3397..1f27f68 100644 --- a/app/services/seed.py +++ b/app/services/seed.py @@ -13,6 +13,7 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession 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.user import User 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]: """Insert seed users if they don't already exist.""" hashed = hash_password(default_password) diff --git a/tests/conftest.py b/tests/conftest.py index b8fbdf6..87fd46a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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("CORS_ORIGINS", "http://test") +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 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.models.ticket import Ticket # 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 -async def client(): +async def client(_reset_login_rate_limiter): """Async test client with a fresh, seeded database per test.""" async with engine.begin() as conn: await conn.run_sync(Base.metadata.create_all) diff --git a/tests/test_p0_auth_admin_batch.py b/tests/test_p0_auth_admin_batch.py new file mode 100644 index 0000000..6448362 --- /dev/null +++ b/tests/test_p0_auth_admin_batch.py @@ -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 diff --git a/tests/test_p0_hardening.py b/tests/test_p0_hardening.py index 71d3b0c..3a5ecad 100644 --- a/tests/test_p0_hardening.py +++ b/tests/test_p0_hardening.py @@ -1,8 +1,8 @@ """P0 hardening regression tests (HARDENING.md P0.1 / P0.2 / P0.3). Covers: -- P0.3: self-registration CANNOT mint a privileged role (role field ignored) -- P0.3: duplicate email still 409s +- P0.3: self-registration is REMOVED — POST /api/auth/register returns 404 and + 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.2: app fails to boot with CORS_ORIGINS="*" """ @@ -15,18 +15,17 @@ import sys from pathlib import Path import pytest -from httpx import ASGITransport, AsyncClient +from httpx import AsyncClient REPO_ROOT = Path(__file__).resolve().parent.parent pytestmark = pytest.mark.asyncio +# ── P0.3: self-registration is removed entirely ─────────────────────── -# ── P0.3: registration role-escalation ──────────────────────────────── - -async def test_register_cannot_mint_admin_role(client: AsyncClient): - """A raw unauthenticated register call must NOT be able to mint Admin/*.""" +async def test_register_endpoint_is_removed(client: AsyncClient): + """A raw unauthenticated register call must 404 — no public signup path.""" resp = await client.post( "/api/auth/register", json={ @@ -36,41 +35,42 @@ async def test_register_cannot_mint_admin_role(client: AsyncClient): "role": "Admin/Jerome", }, ) - assert resp.status_code == 201, resp.text - created = resp.json() - assert created["role"] == "CS Rep", ( - f"self-registration minted privileged role: {created['role']}" - ) + assert resp.status_code == 404, resp.text -async def test_register_role_wahab_also_blocked(client: AsyncClient): - resp = await client.post( +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( + "/api/auth/register", + json={ + "email": f"attacker-{role.lower().replace('/', '-')}@example.com", + "password": "Sup3rSecret!", + "full_name": "Attacker", + "role": role, + }, + ) + assert resp.status_code == 404, (role, resp.text) + + +async def test_register_does_not_create_user(client: AsyncClient): + """No user row is ever created through the removed register endpoint.""" + await client.post( "/api/auth/register", json={ - "email": "attacker2@example.com", + "email": "ghost@example.com", "password": "Sup3rSecret!", - "full_name": "Attacker Two", - "role": "Admin/Wahab", + "full_name": "Ghost", }, ) - assert resp.status_code == 201 - assert resp.json()["role"] == "CS Rep" + # The ghost account must not be able to log in. + resp = await client.post( + "/api/auth/login", + json={"email": "ghost@example.com", "password": "Sup3rSecret!"}, + ) + assert resp.status_code == 401, resp.text -async def test_register_duplicate_email_conflict(client: AsyncClient): - payload = { - "email": "dupe@example.com", - "password": "Sup3rSecret!", - "full_name": "Dupe", - } - r1 = await client.post("/api/auth/register", json=payload) - assert r1.status_code == 201 - r2 = await client.post("/api/auth/register", json=payload) - assert r2.status_code == 409 - - -# ── P0.1 / P0.2: fail-closed boot validation ────────────────────────── - def _boot_with_env(env_overrides: dict[str, str]) -> subprocess.CompletedProcess: """Try importing app.main in a subprocess with the given env; the import must fail (non-zero) when fail-closed validation trips."""