marvin/62-API-REST-brique-authentification #63
No reviewers
Labels
No labels
Compat/Breaking
EC01
EC02
EC03
EC04
EC05
EC06
Kind/BDD
Kind/Back
Kind/Bug
Kind/CICD
Kind/Cloud
Kind/Contenu
Kind/Data
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Front
Kind/IA
Kind/Infra
Kind/Monitoring
Kind/Security
Kind/Testing
Portée/Post-jury
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
ops/alerte
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
g2/enervision!63
Loading…
Reference in a new issue
No description provided.
Delete branch "marvin/62-API-REST-brique-authentification"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Ce que ça change
Toute la brique d'authentification de l'API : création et gestion des comptes (par un administrateur), connexion par mot de passe, session maintenue en cookies httpOnly, rôles, et le durcissement associé. Les jetons sont émis par l'API - JWT d'accès court, jeton de rafraîchissement opaque à rotation. Seul point d'entrée d'authentification du tableau de bord.
Closes #
Preuve
Relecture
Ce qui suit le code
docs/runbooks/mis à jour, un geste d'exploitation a changédocs/adr/complété, une décision structurante a été prisedocs/journal.mdcomplété, un incident a été rencontré.envdu projet back de plus un .env.exemple a été créer- /refresh : fenêtre de grâce (refresh_grace_seconds, 10 s) — un double appel concurrent ne révoque plus la famille ; corrige la déconnexion multi-onglets - PATCH /auth/users/{id} (admin) : désactiver, changer le rôle ou le mot de passe ; révoque les sessions de l'utilisateur ; refuse l'auto-rétrogradation d'un admin - limite de débit du login par (ip, email) au lieu de (ip) - _client_ip ne lit X-Forwarded-For que si trust_proxy, et prend le dernier élément - 0003_auth_indexes.sql : index family_id, index login_ko (ip, email) - longueurs bornées sur email et mot de passe (Credentials, NewUser, UserPatch) 25 tests unitaires, toujours sans base. Refs #38 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>Relecture critique de la brique auth (ticket #62). Le code est soigné : archi en 4 couches nette, rotation + detection de rejeu correcte, argon2id, login en temps constant, doc de conception complete.
ruff,ruff format,mypy --strictet les 25 tests passent en local.Trois bloquants avant merge, puis des points importants et des nits. Detail ligne a ligne en commentaires.
Bloquants
1. Branche en retard sur
develop(mergeable: false). Conflit reel :.gitignoreseulement. A mergerdevelop.2. Le job CI
tests-unitairesechoue a la collecte. Apres merge dedevelop, ce job n'installe querequirements-dev.txt, pas les deps runtime des services ->import jwt/psycopg/argon2echouent avant le premier test (reproduit en venv propre). Defaut de la chaine (#40) que cette PR est la premiere a exposer : le jobqualiteinstalle les deps de service viadeps-services.py, pastests-unitaires. A corriger cote CI, en coordination avec Gabriel.3. Les deux paliers de couverture passent au rouge. Cette PR apporte les premiers tests unitaires -> les mesures s'activent :
services+packages: 40 % vs 70 % exige (--cov-fail-under).../auth/*: 47 % vs 85 % exigerouter.py(116 stmts) etdependencies.py(37) sont a 0 %. Le plan de test du ticket lui-meme n'inclut pas la couche HTTP -> contradiction avec la barriere CI. Soit des testsTestClient(cookies, 401/403/409/429, rotation), soit une decision d'equipe au point du matin (le runbookci.mdinterdit de baisser le seuil en douce).Important
Closes #62manquant dans la description.user_created/user_updatedn'enregistrent pas l'admin auteur, etuser_updatedne dit pas ce qui a change. Cf. commentaire l.231._origin_guardinoperant siALLOWED_ORIGINvide (deploiement meme-origine). Cf.main.pyl.39.(ip, email)->(email)seul siTRUST_PROXY=falsederriere Caddy. Cf.router.pyl.98.min_length=1). Cf.router.pyl.26.Nits
email: str->EmailStr(validation de format).decode_access:options={"require": ["exp"]}. Cf.tokens.pyl.33.refresh: pas deSELECT ... FOR UPDATE, detection de rejeu TOCTOU (compromis assume, merite un commentaire dans le code).0002puis remplace en0003dans la meme PR. Cf.0003.char(64)/role text:char(n)est un anti-idiome PostgreSQL, prefertext.Rien a redire sur : rotation
family_id, plafond durabsolute_exp, condense SHA-256 seul stocke, role verifie en base a chaque requete (JWT non fiable pour l'authz),is_activerecontrole dansget_current_user, fenetre de grace du double-refresh, verrou consultatif sur les migrations.@ -0,0 +4,4 @@create index if not exists auth_sessions_famille on ops.auth_sessions (family_id);-- limite de débit du login par (ip, email)drop index if exists ops.auth_events_echecs_recents;0002creeauth_events_echecs_recentssur(ip, at), et ce fichier le supprime aussitot pour le recreer sur(ip, email, at)-- dans la meme PR. Autant definir directement le bon index dans0002et supprimer cedrop/create. (Sur une base deja migree en preprod on garderait la migration ; ici tout est neuf.)@ -0,0 +23,4 @@class Credentials(BaseModel):email: str = Field(min_length=3, max_length=320)password: str = Field(min_length=1, max_length=1024)min_length=1: un admin peut creer un compte avec un mot de passe d'un caractere. Pour un ticket "durcissement", poser un plancher (8-12 car. min). IdemCredentials(l.31) etUserPatch(l.40).@ -0,0 +95,4 @@agent = request.headers.get("user-agent")if (ipand repo.count_recent_login_failures(conn, ip, email, s.login_failure_window_seconds)Le compteur est indexe
(ip, email). AvecTRUST_PROXY=false(le defaut, y compris dans.env.example) derriere Caddy, toutes les requetes portent l'IP du proxy -> 5 echecs survictime@x.comdepuis n'importe ou verrouillent ce compte 15 min pour tout le monde (DoS par verrouillage). Rendre le defaut prod sur /assertde config au demarrage, ou verrouiller sur l'IP plutot que sur le compte.@ -0,0 +228,4 @@) from Nonerepo.record_event(conn,"user_updated",user_updatedne dit pas ce qui a change : reset de mot de passe, changement de role et desactivation produisent le meme evenement. Et l'user_idenregistre est la cible, pas l'admin auteur -> impossible de dire quel admin a fait l'action. Pour EC04 (journal d'audit), ajouter un champ "action" et l'id de l'acteur. Idemuser_createdl.188.@ -0,0 +30,4 @@def decode_access(token: str, *, secret: str) -> int:"""Renvoie l'id utilisateur. Lève ``jwt.InvalidTokenError`` sur tout écart."""payload = jwt.decode(token, secret, algorithms=[_ALGO])jwt.decodesansoptions={"require": ["exp"]}: un jeton sansexpserait accepte comme non-expirant. Ton encodeur met toujoursexp, donc c'est de la defense en profondeur -- mais peu couteux.@ -0,0 +36,4 @@# et qu'un en-tête Origin est présent (absent = client non-navigateur).allowed = get_settings().allowed_originorigin = request.headers.get("origin")if request.method in _UNSAFE_METHODS and allowed and origin and origin != allowed:_origin_guardne fait rien quandallowed_originest vide -- ce qui sera le cas en prod si Caddy sert le SPA et l'API sur la meme origine. Le ticket #62 exige un controle d'origine sur les requetes mutantes "en plus des cookies SameSite". Ici, en meme-origine, il ne reste queSameSite=Laxsurev_access. Le navigateur envoie pourtantOriginsur les requetes non-sures meme en meme-origine : on pourrait comparer a l'hote de la requete quandOriginest present, ou au minimum documenter que SameSite est la seule barriere dans ce mode et le faire valider.@ -0,0 +11,4 @@## Limites connues- Pas d'auto-service : un utilisateur ne peut pas changer son propre mot de passeA modifier. Ça rentre en conflit avec l'issue #21 https://10.105.200.41/g2/enervision/issues/21
évolution possible vers un system de ticking a faire plus tard
LGTM
New commits pushed, approval review dismissed automatically according to repository settings