fix(backend): JSON error envelope for every HTTPException + strict_slashes=False
Some checks failed
ci / backend (lint + typecheck + unit tests) (push) Failing after 0s
ci / frontend (lint + typecheck + build + unit tests) (push) Failing after 1s

Two issues spotted by ux-frontend consuming docs/api.md against the actual
code path:

1. `flask.abort(...)` returned the Werkzeug HTML error page for 400/403/404/
   422/etc. — only the 401 paths going through `api_error()` and the
   Flask-Login `unauthorized_handler` honoured the `{error, message}`
   envelope the contract promised. The frontend's `ApiClientError.body`
   parser was forced to fall back to a raw string, and the 422 case
   could not surface Pydantic per-field errors.

   Fix: register `@app.errorhandler(HTTPException)` that serialises every
   `HTTPException` to the same JSON envelope. 422s gain a `details: [...]`
   field holding the Pydantic `errors()` list (`loc` / `msg` / `type`),
   matching the shape now documented in `docs/api.md`.

   A `_HTTP_ERROR_CODES` map maps statuses to stable snake_case codes
   (`bad_request`, `not_found`, `method_not_allowed`,
   `validation_error`, `forbidden`, `internal_error`, ...). Unknown
   statuses fall back to `http_error`.

   `description` is `cast(object, ...)` because the Werkzeug stub pins it
   to `str | None` while `flask.abort(..., description=<list>)` is the
   officially supported way to smuggle a Pydantic errors list to the
   handler.

2. `@bp.get("")` on the engagements blueprint produced `/api/v1/engagements`
   (no slash). Hitting it with a trailing slash issued a 308 redirect,
   and some browsers drop the session cookie across that hop.

   Fix: `app.url_map.strict_slashes = False`. Both forms now match the
   same handler without redirect.

5 new integration tests cover the new envelope shape (422 with details,
unknown 404, malformed-JSON 400) and the dual-slash matching. `docs/api.md`
rewritten to reflect the table of stable codes, the `details` shape, and
the no-trailing-slash convention. `CHANGELOG.md` gains a follow-up entry.

Verification: ruff check / mypy --strict / pytest tests/unit all green
(61 unit + 5 new integration).
This commit is contained in:
knacky
2026-05-23 04:33:23 +02:00
parent dd321c2cd0
commit dd5c508b04
4 changed files with 200 additions and 22 deletions

View File

@@ -5,6 +5,26 @@ Versioning starts at `0.1.0` when sprint 0 lands.
## [Unreleased] ## [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": "<code>", "message": "<human>", "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`) ### Sprint 1 — backend auth wiring (`feature/backend-auth-wiring`)
- **`POST /api/v1/auth/login`** — local-credentials login. Body `{username, - **`POST /api/v1/auth/login`** — local-credentials login. Body `{username,

View File

@@ -3,9 +3,11 @@
from __future__ import annotations from __future__ import annotations
from datetime import timedelta from datetime import timedelta
from typing import cast
from flask import Flask, jsonify from flask import Flask, jsonify
from flask.typing import ResponseReturnValue from flask.typing import ResponseReturnValue
from werkzeug.exceptions import HTTPException
from mimic.api import register_blueprints from mimic.api import register_blueprints
from mimic.auth.identity import load_user 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.extensions import db, login_manager, migrate, socketio
from mimic.logging import configure_logging 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: def create_app(settings: Settings | None = None) -> Flask:
settings = settings or get_settings() settings = settings or get_settings()
configure_logging(settings.log_level, as_json=settings.log_json) configure_logging(settings.log_level, as_json=settings.log_json)
app = Flask(__name__) 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( app.config.update(
SECRET_KEY=settings.secret_key.get_secret_value(), SECRET_KEY=settings.secret_key.get_secret_value(),
SQLALCHEMY_DATABASE_URI=settings.database_url, SQLALCHEMY_DATABASE_URI=settings.database_url,
@@ -43,6 +65,38 @@ def create_app(settings: Settings | None = None) -> Flask:
401, 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=<list|dict>)` 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( socketio.init_app(
app, app,
cors_allowed_origins=settings.cors_origins or "*", cors_allowed_origins=settings.cors_origins or "*",

View File

@@ -129,3 +129,74 @@ def test_logout_without_session_returns_401(app, client) -> None:
response = client.post("/api/v1/auth/logout") response = client.post("/api/v1/auth/logout")
assert response.status_code == 401 assert response.status_code == 401
assert response.get_json() == {"error": "not_authenticated", "message": "no active session"} 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"

View File

@@ -8,26 +8,59 @@ same origin).
## Conventions ## 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`, - **Auth transport**: Flask session cookie (`HttpOnly`, `SameSite=Lax`,
`Secure` in production). The browser must send `credentials: "include"` on `Secure` in production). The browser must send `credentials: "include"`
every request. on every request.
- **Content negotiation**: requests and responses use `application/json`. - **Content negotiation**: requests and responses use `application/json`.
- **Error envelope**: failures return `{"error": "<code>", "message": "<human>"}` - **Error envelope**: **every** failure returns the same shape, served by a
with the appropriate HTTP status. Codes are stable identifiers (snake_case), global `HTTPException` handler:
messages are human-readable but not localized. ```json
{ "error": "<snake_case_code>", "message": "<human>", "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 | | Status | `error` code | Use |
|--------|-----| |--------|--------------|-----|
| 200 | OK | | 200 | — | OK |
| 201 | Resource created | | 201 | — | Resource created |
| 204 | OK, no body | | 204 | — | OK, no body |
| 400 | Malformed request (e.g. missing JSON body) | | 400 | `bad_request` | Malformed request (e.g. missing JSON body) |
| 401 | `not_authenticated` or `invalid_credentials` | | 401 | `not_authenticated` or `invalid_credentials` | Anonymous / bad creds |
| 403 | `forbidden` — authenticated but missing permission | | 403 | `forbidden` | Authenticated but missing permission |
| 404 | Resource not found (also returned for tenant-scope denials, see below) | | 404 | `not_found` | Resource not found (also tenant-scope denials, see below) |
| 422 | Pydantic validation error | | 405 | `method_not_allowed` | Method not allowed for that route |
| 500 | Internal — opaque, no leak | | 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) ### Tenant scope leak prevention (MA6 — F11)
@@ -92,7 +125,7 @@ calls this at boot to rehydrate the application state.
## Engagements ## Engagements
### `GET /api/v1/engagements/` ### `GET /api/v1/engagements`
Lists engagements visible to the caller (`engagement.read` permission). Lists engagements visible to the caller (`engagement.read` permission).
@@ -115,12 +148,12 @@ Response — `200`:
] ]
``` ```
### `GET /api/v1/engagements/<eid>` ### `GET /api/v1/engagements<eid>`
Same payload shape as the list element. Returns 404 if the engagement does not Same payload shape as the list element. Returns 404 if the engagement does not
exist or the caller is not a member (MA6). exist or the caller is not a member (MA6).
### `POST /api/v1/engagements/` ### `POST /api/v1/engagements`
Creates an engagement (`engagement.create` permission). Creates an engagement (`engagement.create` permission).
@@ -153,6 +186,6 @@ authority.
``` ```
2. `POST /api/v1/auth/login` with the credentials — receive the user payload 2. `POST /api/v1/auth/login` with the credentials — receive the user payload
plus the session cookie. plus the session cookie.
3. `POST /api/v1/engagements/` with a body — receive the engagement. 3. `POST /api/v1/engagements` with a body — receive the engagement.
4. `GET /api/v1/engagements/` — see the new engagement in the list. 4. `GET /api/v1/engagements` — see the new engagement in the list.
5. `POST /api/v1/auth/logout` — session cleared. 5. `POST /api/v1/auth/logout` — session cleared.