fix(backend): JSON error envelope for every HTTPException + strict_slashes=False
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:
20
CHANGELOG.md
20
CHANGELOG.md
@@ -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,
|
||||||
|
|||||||
@@ -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 "*",
|
||||||
|
|||||||
@@ -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"
|
||||||
|
|||||||
77
docs/api.md
77
docs/api.md
@@ -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.
|
||||||
|
|||||||
Reference in New Issue
Block a user