Repository navigation
Conversation
a9a75b7 to
64a5234
Compare
b37e308 to
0331d48
Compare
0331d48 to
c53cb23
Compare
09f3402 to
b667443
Compare
1b31261 to
f16a730
Compare
cecad1b to
10b4770
Compare
|
🤖 Hey ! The security scan report for the current pull request is available here. |
0eb3e93 to
43d21dc
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🤖 Hey ! A preview of the application is available at : https://console-pr-2279.dso.cpin-hp.numerique-interieur.fr Please be patient, deployment may take a few minutes. |
shikanime
left a comment
There was a problem hiding this comment.
Verdict : Commentaire
Migration user-tokens aboutie après six semaines d'itérations : hachage paritaire documenté, expiration validée par ExpirationDateSchema désormais réellement branchée sur le contrat de création, et le verifyTokenHash mort signalé en revue a disparu. Les demandes du relecteur sur le schéma de contrat et la bascule nginx sont résolues.
There was a problem hiding this comment.
🟠 important — la branche n’est pas rebasée sur main : son merge-base est 5729b71, alors que main pointe sur b17a159 (elle est donc en retard d’un commit). Merci de rebaser, puis de relancer les contrôles ; après vérification, la migration user-tokens préserve l’authentification humaine, le bornage propriétaire de la suppression, le contrat partagé et l’absence d’exposition du hash.
StephaneTrebel
left a comment
There was a problem hiding this comment.
🟠 important — le commit courant fa4c349 est intitulé « placeholder », ce qui ne respecte pas la convention Conventional Commits du dépôt. Merci de le renommer avec un intitulé conventionnel décrivant la migration (par exemple : refactor(user-tokens): migrate from server), puis de relancer la CI.
Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I241a395b8533631173b04f556b97bf666a6a6964 Signed-off-by: Shikanime Deva <22115108+shikanime@users.noreply.github.com>
shikanime
left a comment
There was a problem hiding this comment.
Revue du head 75cd904d — migration user-tokens propre. Le module NestJS (controller/service/module + specs colocalisées) reproduit la parité legacy : hash SHA-256 sans sel centralisé dans hashToken, suppression silencieuse 204 alignée sur deleteToken legacy (findUnique puis delete), imports de module minimaux (AuthModule, DatabaseModule, UserPermissionModule). Aucun cast as, aucune interaction Vault nouvelle, specs adaptées et non supprimées (le spec dso-token réutilise hashToken au lieu de re-bricoler createHash). Le remplacement de daysAgoFromNow local par daysAgo partagé est un bon réflexe anti-duplication. Verdict : aucun point bloquant ni important ; un seul nit de rédaction du corps de la PR.
Résumé des sévérités : 0 bloquant, 0 important, 1 nit.
|
StephaneTrebel
left a comment
There was a problem hiding this comment.
✨ La migration sépare bien les DTO du stockage, limite les opérations au propriétaire du token et préserve le hash compatible avec le serveur legacy. Le statut non routé est clairement documenté et la CI est verte.

0 New Issues
0 Fixed Issues
0 Accepted Issues
Issues liées
Refs #1889
Quel est le comportement actuel ?
Le module
user-tokens(tokens d'accès utilisateur, génération, révocation, exposition) est servi par l'ancienne application Fastifyapps/server, dansapps/server/src/resources/user/tokens/.Quel est le nouveau comportement ?
Migration du module
user-tokensversapps/server-nestjs:UserTokensController,UserTokensService,UserTokensModule.user-tokens-queries.utils.ts: sélections Prisma typées.user-tokens.utils.ts: génération de token (longueur, expiration 24h) et helpers de révocation.main.module.ts.apps/server/src/resources/user/tokens/.Cette PR introduit-elle un breaking change ?
Non.
Autres informations
Rebasé sur
mainpost-#2371 ;crypto.utils.tsetpackages/shared/src/schemas/index.tssont alignés sur la version fusionnée de #2371 (#2442 fermée sans fusion).Statut bascule : ce module est porté et enregistré, non routé — aucune règle Nginx V1 ne pointe vers NestJS, le fallback legacy reste actif. La migration applicative ne constitue pas une bascule de trafic. La PR de bascule Nginx sera référencée ici dès son ouverture ; le registre #1889 sera mis à jour à la fusion.