test(backend): classe les routes du contrat et dérive les listes d'autorisation
`ROUTES_A_ROLE` était recopiée dans `test_openapi.py`, et deux de ses entrées portaient
`{id}` là où le contrat expose `{user_id}`. Elles ne correspondaient donc à aucune
opération, et `test_every_role_guarded_route_documents_the_role_refusal` passait au vert
sans rien vérifier sur `PATCH /users/{user_id}` ni sur sa réinitialisation de mot de passe :
11 des 13 routes gardées étaient réellement couvertes.
`tests/api/acces.py` porte désormais la classification des 24 routes du contrat en quatre
ensembles, dont la table `ROLE_MINIMUM`, et `test_every_declared_route_is_classified` refuse
aussi bien une route non classée qu'une entrée qui ne correspond plus à rien. C'est ce que
`docs/architecture/20-backend.md` annonçait comme impossible : « ces deux listes sont
maintenues à la main, pas dérivées ».
Au passage, `chemin_concret()` substitue les trois gabarits du contrat et non plus le seul
`{user_id}`, ce qui est sans effet sur le refus anonyme mais nécessaire à un appel qui doit
aboutir.
This commit is contained in:
@@ -0,0 +1,86 @@
|
|||||||
|
# Pourquoi : classification unique des routes du contrat, lue par test_route_protection.py,
|
||||||
|
# test_openapi.py et test_matrice_acces.py. Trois listes séparées dérivaient auparavant chacune
|
||||||
|
# de leur côté, et deux entrées de ROUTES_A_ROLE ne correspondaient plus à aucune route sans que
|
||||||
|
# rien ne le signale.
|
||||||
|
# Piège : les trois ensembles doivent rester disjoints et couvrir tout le schéma. C'est
|
||||||
|
# `test_every_declared_route_is_classified` qui le vérifie, pas la relecture.
|
||||||
|
|
||||||
|
from typing import Final
|
||||||
|
|
||||||
|
from app.core.roles import Role
|
||||||
|
|
||||||
|
Route = tuple[str, str]
|
||||||
|
|
||||||
|
ROUTES_PUBLIQUES: Final[frozenset[Route]] = frozenset(
|
||||||
|
{
|
||||||
|
("GET", "/api/v1/health/live"),
|
||||||
|
("GET", "/api/v1/health/ready"),
|
||||||
|
("POST", "/api/v1/auth/login"),
|
||||||
|
# Sans cookie, la déconnexion ne fait rien et répond 204 : elle est idempotente.
|
||||||
|
("POST", "/api/v1/auth/logout"),
|
||||||
|
("POST", "/api/v1/auth/forgot-password"),
|
||||||
|
# Protégée par le jeton dans le corps de la requête, pas par un `Principal` : aucune
|
||||||
|
# authentification préalable ne s'applique, c'est la validité du jeton qui tranche.
|
||||||
|
("POST", "/api/v1/auth/reset-password"),
|
||||||
|
# Même raison : lecture seule, protégée par le jeton passé en paramètre, pas par un
|
||||||
|
# `Principal`. Le jeton est un secret de 256 bits, non brute-forçable.
|
||||||
|
("GET", "/api/v1/auth/reset-password/validate"),
|
||||||
|
("GET", "/metrics"),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
# Le cookie opaque porte seul l'autorisation : sans lui la route rend 401, mais aucun `Principal`
|
||||||
|
# n'est construit et `require_role` n'entre jamais en jeu.
|
||||||
|
ROUTE_COOKIE: Final[frozenset[Route]] = frozenset({("POST", "/api/v1/auth/refresh")})
|
||||||
|
|
||||||
|
# Authentifiées par `CurrentPrincipalDep` nu, donc hors de `require_role` et, avec lui, hors du
|
||||||
|
# refus `password_change_required`. Volontaire pour `/auth/password`, qui est la sortie de l'état
|
||||||
|
# provisoire ; subi pour `/auth/logout-all`, cf. test_matrice_acces.py.
|
||||||
|
ROUTES_SANS_ROLE: Final[frozenset[Route]] = frozenset(
|
||||||
|
{
|
||||||
|
("GET", "/api/v1/auth/me"),
|
||||||
|
("POST", "/api/v1/auth/password"),
|
||||||
|
("POST", "/api/v1/auth/logout-all"),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
ROLE_MINIMUM: Final[dict[Route, Role]] = {
|
||||||
|
("GET", "/api/v1/sites"): Role.LECTEUR,
|
||||||
|
("GET", "/api/v1/sites/{site_id}"): Role.LECTEUR,
|
||||||
|
("GET", "/api/v1/sites/{site_id}/current"): Role.LECTEUR,
|
||||||
|
("GET", "/api/v1/alerts"): Role.LECTEUR,
|
||||||
|
("GET", "/api/v1/recommendations"): Role.LECTEUR,
|
||||||
|
("GET", "/api/v1/recommendations/{recommendation_id}"): Role.LECTEUR,
|
||||||
|
("GET", "/api/v1/stats/summary"): Role.LECTEUR,
|
||||||
|
("GET", "/api/v1/readings"): Role.LECTEUR,
|
||||||
|
("GET", "/api/v1/sensors/status"): Role.ADMIN,
|
||||||
|
("GET", "/api/v1/users"): Role.ADMIN,
|
||||||
|
("POST", "/api/v1/users"): Role.ADMIN,
|
||||||
|
("PATCH", "/api/v1/users/{user_id}"): Role.ADMIN,
|
||||||
|
("POST", "/api/v1/users/{user_id}/password-reset"): Role.ADMIN,
|
||||||
|
}
|
||||||
|
|
||||||
|
# Piège : `{recommendation_id}` est typé `int` et `{user_id}` est un UUID. Une substitution
|
||||||
|
# uniforme par une chaîne quelconque rendrait 422 avant d'atteindre la garde de rôle, et le test
|
||||||
|
# passerait en prouvant autre chose que ce qu'il annonce.
|
||||||
|
SUBSTITUTIONS: Final[dict[str, str]] = {
|
||||||
|
"{user_id}": "00000000-0000-0000-0000-000000000000",
|
||||||
|
"{site_id}": "site-absent-du-jeu-de-donnees",
|
||||||
|
"{recommendation_id}": "999999999",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def chemin_concret(chemin: str) -> str:
|
||||||
|
for gabarit, valeur in SUBSTITUTIONS.items():
|
||||||
|
chemin = chemin.replace(gabarit, valeur)
|
||||||
|
return chemin
|
||||||
|
|
||||||
|
|
||||||
|
def routes_du_schema(schema: dict[str, object]) -> list[Route]:
|
||||||
|
chemins: dict[str, dict[str, object]] = schema["paths"] # type: ignore[assignment]
|
||||||
|
return [
|
||||||
|
(methode.upper(), chemin)
|
||||||
|
for chemin, operations in chemins.items()
|
||||||
|
for methode in operations
|
||||||
|
if methode.upper() in {"GET", "POST", "PATCH", "PUT", "DELETE"}
|
||||||
|
]
|
||||||
@@ -8,6 +8,7 @@ from typing import Any
|
|||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from app import cli
|
from app import cli
|
||||||
|
from tests.api.acces import ROLE_MINIMUM
|
||||||
|
|
||||||
METHODES = {"get", "post", "patch", "put", "delete"}
|
METHODES = {"get", "post", "patch", "put", "delete"}
|
||||||
|
|
||||||
@@ -24,21 +25,11 @@ ORIGINE_VERIFIEE = {
|
|||||||
|
|
||||||
# Toute route derrière `require_role` (LecteurDep, OperateurDep, AdminDep) peut rendre 403 pour
|
# Toute route derrière `require_role` (LecteurDep, OperateurDep, AdminDep) peut rendre 403 pour
|
||||||
# `password_change_required`, pas seulement les routes `admin`.
|
# `password_change_required`, pas seulement les routes `admin`.
|
||||||
ROUTES_A_ROLE = {
|
# Piège : cette liste était recopiée ici, et deux de ses entrées portaient `{id}` là où le contrat
|
||||||
("GET", "/api/v1/users"),
|
# expose `{user_id}`. Elles ne correspondaient donc à aucune opération, et le test ci-dessous
|
||||||
("POST", "/api/v1/users"),
|
# passait au vert sans rien vérifier sur ces deux routes. Elle est maintenant dérivée, et
|
||||||
("PATCH", "/api/v1/users/{id}"),
|
# `test_every_declared_route_is_classified` interdit l'entrée morte.
|
||||||
("POST", "/api/v1/users/{id}/password-reset"),
|
ROUTES_A_ROLE = frozenset(ROLE_MINIMUM)
|
||||||
("GET", "/api/v1/sites"),
|
|
||||||
("GET", "/api/v1/sites/{site_id}"),
|
|
||||||
("GET", "/api/v1/sites/{site_id}/current"),
|
|
||||||
("GET", "/api/v1/alerts"),
|
|
||||||
("GET", "/api/v1/recommendations"),
|
|
||||||
("GET", "/api/v1/recommendations/{recommendation_id}"),
|
|
||||||
("GET", "/api/v1/stats/summary"),
|
|
||||||
("GET", "/api/v1/readings"),
|
|
||||||
("GET", "/api/v1/sensors/status"),
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture(scope="module")
|
@pytest.fixture(scope="module")
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
# Ce test est le garde-fou de l'autorisation : rendre une route publique oblige à modifier
|
# Ce test est le garde-fou de l'autorisation : rendre une route publique oblige à modifier
|
||||||
# `ROUTES_PUBLIQUES` ci-dessous, ce qui apparaît en clair dans la diff d'une pull request et
|
# `ROUTES_PUBLIQUES` dans `tests/api/acces.py`, ce qui apparaît en clair dans la diff d'une pull
|
||||||
# demande une justification au relecteur.
|
# request et demande une justification au relecteur.
|
||||||
# Pourquoi : il interroge réellement chaque route sans jeton au lieu d'inspecter l'arbre de
|
# Pourquoi : il interroge réellement chaque route sans jeton au lieu d'inspecter l'arbre de
|
||||||
# dépendances. L'arbre n'est accessible que par l'API privée de FastAPI, et surtout une route
|
# dépendances. L'arbre n'est accessible que par l'API privée de FastAPI, et surtout une route
|
||||||
# peut porter la bonne dépendance tout en répondant quand même.
|
# peut porter la bonne dépendance tout en répondant quand même.
|
||||||
@@ -11,58 +11,64 @@ import pytest
|
|||||||
from fastapi import FastAPI
|
from fastapi import FastAPI
|
||||||
from httpx import AsyncClient
|
from httpx import AsyncClient
|
||||||
|
|
||||||
ROUTES_PUBLIQUES = frozenset(
|
from tests.api.acces import (
|
||||||
{
|
ROLE_MINIMUM,
|
||||||
("GET", "/api/v1/health/live"),
|
ROUTE_COOKIE,
|
||||||
("GET", "/api/v1/health/ready"),
|
ROUTES_PUBLIQUES,
|
||||||
("POST", "/api/v1/auth/login"),
|
ROUTES_SANS_ROLE,
|
||||||
# Sans cookie, la déconnexion ne fait rien et répond 204 : elle est idempotente.
|
Route,
|
||||||
("POST", "/api/v1/auth/logout"),
|
chemin_concret,
|
||||||
("POST", "/api/v1/auth/forgot-password"),
|
routes_du_schema,
|
||||||
# Protégée par le jeton dans le corps de la requête, pas par un `Principal` : aucune
|
|
||||||
# authentification préalable ne s'applique, c'est la validité du jeton qui tranche.
|
|
||||||
("POST", "/api/v1/auth/reset-password"),
|
|
||||||
# Même raison : lecture seule, protégée par le jeton passé en paramètre, pas par un
|
|
||||||
# `Principal`. Le jeton est un secret de 256 bits, non brute-forçable.
|
|
||||||
("GET", "/api/v1/auth/reset-password/validate"),
|
|
||||||
("GET", "/metrics"),
|
|
||||||
}
|
|
||||||
)
|
)
|
||||||
|
|
||||||
VALEURS_DE_SUBSTITUTION = "00000000-0000-0000-0000-000000000000"
|
|
||||||
STATUTS_DE_REFUS = {401, 403}
|
STATUTS_DE_REFUS = {401, 403}
|
||||||
|
HORS_SCHEMA = {("GET", "/metrics")}
|
||||||
|
|
||||||
|
|
||||||
def routes_declarees(app: FastAPI) -> list[tuple[str, str]]:
|
def routes_declarees(app: FastAPI) -> list[Route]:
|
||||||
schema: dict[str, Any] = app.openapi()
|
schema: dict[str, Any] = app.openapi()
|
||||||
return [
|
return routes_du_schema(schema)
|
||||||
(methode.upper(), chemin)
|
|
||||||
for chemin, operations in schema["paths"].items()
|
|
||||||
for methode in operations
|
|
||||||
if methode.upper() in {"GET", "POST", "PATCH", "PUT", "DELETE"}
|
|
||||||
]
|
|
||||||
|
|
||||||
|
|
||||||
def routes_protegees(app: FastAPI) -> list[tuple[str, str]]:
|
def routes_protegees(app: FastAPI) -> list[Route]:
|
||||||
return [route for route in routes_declarees(app) if route not in ROUTES_PUBLIQUES]
|
return [route for route in routes_declarees(app) if route not in ROUTES_PUBLIQUES]
|
||||||
|
|
||||||
|
|
||||||
def test_the_public_allow_list_has_no_stale_entry(app: FastAPI) -> None:
|
def test_the_public_allow_list_has_no_stale_entry(app: FastAPI) -> None:
|
||||||
declarees = set(routes_declarees(app)) | {("GET", "/metrics")}
|
declarees = set(routes_declarees(app)) | HORS_SCHEMA
|
||||||
|
|
||||||
inconnues = ROUTES_PUBLIQUES - declarees
|
inconnues = ROUTES_PUBLIQUES - declarees
|
||||||
|
|
||||||
assert inconnues == set()
|
assert inconnues == set()
|
||||||
|
|
||||||
|
|
||||||
|
# Sans lui, une route ajoutée sans être classée n'est vue par aucun test de rôle : elle hérite
|
||||||
|
# du seul contrôle anonyme, et une garde posée au mauvais niveau passe inaperçue.
|
||||||
|
def test_every_declared_route_is_classified(app: FastAPI) -> None:
|
||||||
|
classees = ROUTES_PUBLIQUES | ROUTE_COOKIE | ROUTES_SANS_ROLE | set(ROLE_MINIMUM)
|
||||||
|
|
||||||
|
non_classees = set(routes_declarees(app)) - classees
|
||||||
|
fantomes = classees - set(routes_declarees(app)) - HORS_SCHEMA
|
||||||
|
|
||||||
|
assert non_classees == set(), "classer la route dans tests/api/acces.py"
|
||||||
|
assert fantomes == set(), "entrée morte : la route n'existe plus sous ce chemin"
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_four_classes_of_routes_stay_disjoint() -> None:
|
||||||
|
classes = [ROUTES_PUBLIQUES, ROUTE_COOKIE, ROUTES_SANS_ROLE, frozenset(ROLE_MINIMUM)]
|
||||||
|
|
||||||
|
for rang, classe in enumerate(classes):
|
||||||
|
for autre in classes[rang + 1 :]:
|
||||||
|
assert classe & autre == frozenset()
|
||||||
|
|
||||||
|
|
||||||
async def test_every_route_rejects_an_anonymous_caller_unless_explicitly_public(
|
async def test_every_route_rejects_an_anonymous_caller_unless_explicitly_public(
|
||||||
app: FastAPI, client: AsyncClient
|
app: FastAPI, client: AsyncClient
|
||||||
) -> None:
|
) -> None:
|
||||||
ouvertes: list[tuple[str, str, int]] = []
|
ouvertes: list[tuple[str, str, int]] = []
|
||||||
|
|
||||||
for methode, chemin in routes_protegees(app):
|
for methode, chemin in routes_protegees(app):
|
||||||
concret = chemin.replace("{user_id}", VALEURS_DE_SUBSTITUTION)
|
response = await client.request(methode, chemin_concret(chemin), json={})
|
||||||
response = await client.request(methode, concret, json={})
|
|
||||||
if response.status_code not in STATUTS_DE_REFUS:
|
if response.status_code not in STATUTS_DE_REFUS:
|
||||||
ouvertes.append((methode, chemin, response.status_code))
|
ouvertes.append((methode, chemin, response.status_code))
|
||||||
|
|
||||||
@@ -95,4 +101,5 @@ async def test_the_documentation_routes_are_public_by_design(
|
|||||||
app: FastAPI, client: AsyncClient, chemin: str
|
app: FastAPI, client: AsyncClient, chemin: str
|
||||||
) -> None:
|
) -> None:
|
||||||
response = await client.get(chemin)
|
response = await client.get(chemin)
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
|
|||||||
Reference in New Issue
Block a user