diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cd8779..24e90a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,26 @@ Versioning starts at `0.1.0` when sprint 0 lands. ## [Unreleased] +### Sprint 1 — backend follow-up fixes (`feature/backend-auth-wiring`) + +- **Global JSON error envelope** — register `@app.errorhandler(HTTPException)` + so every aborted request now flows through the same + `{ "error": "", "message": "", "details"? }` JSON shape that + `docs/api.md` documented but only `api_error()` honoured before. 422 + responses surface the Pydantic per-field list under `details` so the + frontend can map errors back to form fields. New stable codes: + `bad_request`, `not_found`, `method_not_allowed`, `validation_error`, + `forbidden`, `internal_error`, etc. (see updated `docs/api.md`). +- **`strict_slashes=False`** on the URL map — `/api/v1/engagements` and + `/api/v1/engagements/` match the same handler. Removes the 308 redirect + that some browsers drop the session cookie through. +- **5 new integration tests** covering both slash variants, 422 + `validation_error` envelope shape (incl. `details`), unknown-route 404, + and 400 on a non-JSON body. +- **`docs/api.md`** rewritten: full error code table, 422 `details` + example, trailing-slash policy, dropped trailing slashes from all + endpoint headings. + ### Sprint 1 — backend auth wiring (`feature/backend-auth-wiring`) - **`POST /api/v1/auth/login`** — local-credentials login. Body `{username, diff --git a/backend/src/mimic/app.py b/backend/src/mimic/app.py index a518824..4c8ccb9 100644 --- a/backend/src/mimic/app.py +++ b/backend/src/mimic/app.py @@ -3,9 +3,11 @@ from __future__ import annotations from datetime import timedelta +from typing import cast from flask import Flask, jsonify from flask.typing import ResponseReturnValue +from werkzeug.exceptions import HTTPException from mimic.api import register_blueprints from mimic.auth.identity import load_user @@ -13,12 +15,32 @@ from mimic.config import Settings, get_settings from mimic.extensions import db, login_manager, migrate, socketio from mimic.logging import configure_logging +# HTTP status → stable snake_case error code surfaced in the JSON envelope. +# Anything not listed falls back to "http_error". +_HTTP_ERROR_CODES: dict[int, str] = { + 400: "bad_request", + 401: "not_authenticated", + 403: "forbidden", + 404: "not_found", + 405: "method_not_allowed", + 409: "conflict", + 415: "unsupported_media_type", + 422: "validation_error", + 429: "rate_limited", + 500: "internal_error", + 503: "service_unavailable", +} + def create_app(settings: Settings | None = None) -> Flask: settings = settings or get_settings() configure_logging(settings.log_level, as_json=settings.log_json) app = Flask(__name__) + # `strict_slashes=False` means routes match with or without the trailing + # slash. Cross-origin clients keep their session cookie either way (a + # 308 redirect could drop it on some browsers). + app.url_map.strict_slashes = False app.config.update( SECRET_KEY=settings.secret_key.get_secret_value(), SQLALCHEMY_DATABASE_URI=settings.database_url, @@ -43,6 +65,38 @@ def create_app(settings: Settings | None = None) -> Flask: 401, ) + @app.errorhandler(HTTPException) + def _json_http_error(exc: HTTPException) -> ResponseReturnValue: + """Serialize every aborted request as the uniform JSON envelope. + + `flask.abort()` defaults to a Werkzeug HTML page; without this handler + the contract documented in docs/api.md would only hold for 401s. + """ + status = exc.code or 500 + # The Werkzeug type stub pins `description` to `str | None`, but + # `flask.abort(..., description=)` legally smuggles richer + # payloads through (we use this for Pydantic `errors()` on 422). Cast + # to `object` so the runtime type-narrowing below is type-checked. + description = cast(object, exc.description) + message: str + details: object | None = None + if isinstance(description, str): + message = description + elif isinstance(description, list | dict): + # Pydantic `exc.errors()` flows through `abort(422, description=...)` + # as a list; keep it under `details` so the client can map per-field. + message = "request failed" + details = description + else: + message = exc.name or "request failed" + body: dict[str, object] = { + "error": _HTTP_ERROR_CODES.get(status, "http_error"), + "message": message, + } + if details is not None: + body["details"] = details + return jsonify(body), status + socketio.init_app( app, cors_allowed_origins=settings.cors_origins or "*", diff --git a/backend/tests/integration/test_auth_engagement_e2e.py b/backend/tests/integration/test_auth_engagement_e2e.py index eb0b980..74bcd2f 100644 --- a/backend/tests/integration/test_auth_engagement_e2e.py +++ b/backend/tests/integration/test_auth_engagement_e2e.py @@ -129,3 +129,74 @@ def test_logout_without_session_returns_401(app, client) -> None: response = client.post("/api/v1/auth/logout") assert response.status_code == 401 assert response.get_json() == {"error": "not_authenticated", "message": "no active session"} + + +def test_engagements_route_accepts_both_slash_variants(app, client) -> None: + """strict_slashes=False: `/engagements` and `/engagements/` both match + the same handler — no 308 redirect that would drop the session cookie + on some browsers (frontend fix request).""" + from mimic.extensions import db # noqa: PLC0415 + + with app.app_context(): + _seed_rt_lead(db, email="dual@example.org", password="dual-slash-1") + client.post( + "/api/v1/auth/login", + json={"username": "dual@example.org", "password": "dual-slash-1"}, + ) + + no_slash = client.get("/api/v1/engagements") + with_slash = client.get("/api/v1/engagements/") + assert no_slash.status_code == 200 + assert with_slash.status_code == 200 + assert no_slash.get_json() == with_slash.get_json() + + +def test_engagement_create_validation_error_returns_uniform_envelope(app, client) -> None: + """Pydantic 422 must flow through the global handler with `details` + carrying the per-field error list (frontend uses it for form mapping).""" + from mimic.extensions import db # noqa: PLC0415 + + with app.app_context(): + _seed_rt_lead(db, email="form@example.org", password="form-secret-1") + client.post( + "/api/v1/auth/login", + json={"username": "form@example.org", "password": "form-secret-1"}, + ) + + response = client.post("/api/v1/engagements", json={"description": "no client name"}) + assert response.status_code == 422 + body = response.get_json() + assert body["error"] == "validation_error" + assert body["message"] == "request failed" + assert isinstance(body["details"], list) + locs = [tuple(entry["loc"]) for entry in body["details"]] + assert ("client_name",) in locs + + +def test_unknown_route_returns_uniform_404(app, client) -> None: + response = client.get("/api/v1/does-not-exist") + assert response.status_code == 404 + body = response.get_json() + assert body["error"] == "not_found" + assert "message" in body + + +def test_bad_json_body_returns_uniform_400(app, client) -> None: + from mimic.extensions import db # noqa: PLC0415 + + with app.app_context(): + _seed_rt_lead(db, email="raw@example.org", password="raw-secret-1") + client.post( + "/api/v1/auth/login", + json={"username": "raw@example.org", "password": "raw-secret-1"}, + ) + + response = client.post( + "/api/v1/engagements", + data="not-json", + content_type="application/json", + ) + assert response.status_code == 400 + body = response.get_json() + assert body["error"] == "bad_request" + assert body["message"] == "JSON body required" diff --git a/docs/api.md b/docs/api.md index c11569a..73b2f31 100644 --- a/docs/api.md +++ b/docs/api.md @@ -8,26 +8,59 @@ same origin). ## Conventions -- **Base URL**: `/api/v1/`. +- **Base URL**: `/api/v1`. +- **Trailing slash**: routes accept the URL with **or without** a trailing + slash. The app is configured with `strict_slashes=False`, so Werkzeug + never issues a 308 redirect (which can drop the session cookie on some + browsers). Use whichever form your client prefers; `docs/api.md` writes + the no-slash form. - **Auth transport**: Flask session cookie (`HttpOnly`, `SameSite=Lax`, - `Secure` in production). The browser must send `credentials: "include"` on - every request. + `Secure` in production). The browser must send `credentials: "include"` + on every request. - **Content negotiation**: requests and responses use `application/json`. -- **Error envelope**: failures return `{"error": "", "message": ""}` - with the appropriate HTTP status. Codes are stable identifiers (snake_case), - messages are human-readable but not localized. +- **Error envelope**: **every** failure returns the same shape, served by a + global `HTTPException` handler: + ```json + { "error": "", "message": "", "details": ... } + ``` + `details` is only present for `422` (Pydantic per-field error list). Codes + are stable identifiers; messages are human-readable but not localized. -| Status | Use | -|--------|-----| -| 200 | OK | -| 201 | Resource created | -| 204 | OK, no body | -| 400 | Malformed request (e.g. missing JSON body) | -| 401 | `not_authenticated` or `invalid_credentials` | -| 403 | `forbidden` — authenticated but missing permission | -| 404 | Resource not found (also returned for tenant-scope denials, see below) | -| 422 | Pydantic validation error | -| 500 | Internal — opaque, no leak | +| Status | `error` code | Use | +|--------|--------------|-----| +| 200 | — | OK | +| 201 | — | Resource created | +| 204 | — | OK, no body | +| 400 | `bad_request` | Malformed request (e.g. missing JSON body) | +| 401 | `not_authenticated` or `invalid_credentials` | Anonymous / bad creds | +| 403 | `forbidden` | Authenticated but missing permission | +| 404 | `not_found` | Resource not found (also tenant-scope denials, see below) | +| 405 | `method_not_allowed` | Method not allowed for that route | +| 415 | `unsupported_media_type` | Wrong `Content-Type` on a body | +| 422 | `validation_error` | Pydantic — see `details` | +| 429 | `rate_limited` | Reserved for future limiter | +| 500 | `internal_error` | Opaque — no leak | + +### 422 `details` shape + +`details` is the raw Pydantic `errors()` list — one entry per failed field: + +```json +{ + "error": "validation_error", + "message": "request failed", + "details": [ + { + "type": "missing", + "loc": ["client_name"], + "msg": "Field required", + "input": { "description": "no client_name" } + } + ] +} +``` + +Use `details[i].loc` to map the error back to a form field. ### Tenant scope leak prevention (MA6 — F11) @@ -92,7 +125,7 @@ calls this at boot to rehydrate the application state. ## Engagements -### `GET /api/v1/engagements/` +### `GET /api/v1/engagements` Lists engagements visible to the caller (`engagement.read` permission). @@ -115,12 +148,12 @@ Response — `200`: ] ``` -### `GET /api/v1/engagements/` +### `GET /api/v1/engagements` Same payload shape as the list element. Returns 404 if the engagement does not exist or the caller is not a member (MA6). -### `POST /api/v1/engagements/` +### `POST /api/v1/engagements` Creates an engagement (`engagement.create` permission). @@ -153,6 +186,6 @@ authority. ``` 2. `POST /api/v1/auth/login` with the credentials — receive the user payload plus the session cookie. -3. `POST /api/v1/engagements/` with a body — receive the engagement. -4. `GET /api/v1/engagements/` — see the new engagement in the list. +3. `POST /api/v1/engagements` with a body — receive the engagement. +4. `GET /api/v1/engagements` — see the new engagement in the list. 5. `POST /api/v1/auth/logout` — session cleared.