refactor(user-tokens): migrate from server by shikanime · Pull Request #2279 · cloud-pi-native/console

shikanime · 2026-06-30T09:42:04Z

Issues liées

Issues numéro: #1889

Quel est le comportement actuel ?

Quel est le nouveau comportement ?

Cette PR introduit-elle un breaking change ?

Autres informations

github-actions · 2026-07-01T13:39:37Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-16T08:03:12Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-16T08:47:11Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-16T09:50:54Z

🤖 Hey !

The security scan report for the current pull request is available here.

StephaneTrebel · 2026-07-16T15:50:22Z

Je tente un truc

github-actions · 2026-07-16T16:09:47Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-16T16:10:00Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-21T12:27:05Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-23T10:17:43Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-23T10:18:09Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-23T11:46:00Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-23T12:36:15Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-23T12:46:57Z

🤖 Hey !

The security scan report for the current pull request is available here.

shikanime · 2026-07-27T08:53:03Z

Review: REQUEST CHANGES (posted as comment — GitHub does not allow the author to submit a formal review on their own PR).

Contract regressions in the token migration:

BLOCKER B1 — user-tokens.controller.ts:37 and admin-token.controller.ts:31: @Delete(':tokenId') uses bare @Param('tokenId') with no ParseUUIDPipe/zod pipe. Legacy contract enforced z.string().uuid() (400 on bad UUID); now a non-UUID id reaches Prisma → P2023 → unhandled 500. Fix: @Param('tokenId', ParseUUIDPipe) tokenId: string.

BLOCKER B2 — date parsing path (user-tokens.service.ts / admin-token.service.ts): z.coerce.date().parse() throws a raw ZodError (500) on bad input; legacy returned 400. Map ZodError → BadRequestException or route through ZodValidationPipe.

WARNINGS

Use the existing @RequireUserType('human') decorator instead of the hand-rolled userType !== 'human' checks (3×).
withRevoked === 'true' silently coerces invalid query values to false (legacy rejected them).

NITS

unused verifyTokenHash export; needless async on generateTokenPair; commit scope says user-tokens but half the diff is admin-token; confirm the strangler routing.conf is flipped to the NestJS route (or the new code is dead).

Good: token hash never selected/exposed, authz parity preserved, tests are meaningful (25/25).

github-actions · 2026-07-27T09:01:18Z

🤖 Hey !

The security scan report for the current pull request is available here.

shikanime

Review: #2279 — refactor(user-tokens): migrate from server

Verdict: REQUEST CHANGES (dead crypto export; strong authz parity + tests)

Findings

[blocker] apps/server-nestjs/src/utils/crypto.ts:46 verifyTokenHash is exported but never imported anywhere (verified: zero usages outside its own def + spec). Dead code in a security-sensitive file. Either wire it into the token-login validation path or drop it from this PR.

[warning] admin-token/admin-token.service.ts:58 BigInt(data.permissions) — permissionLevelSchema is z.coerce.string(), so permissions is a string. BigInt("chocapics") throws a raw SyntaxError → 500, not a BadRequestException. Validate with z.coerce.bigint() or wrap in a typed parse. (Latent in the Fastify version too; you're rewriting, so fix it.)

[warning] admin-token.service.ts:39 / user-tokens.service.ts — z.coerce.date().parse("toto") returns Invalid Date (not a ZodError), and isAtLeastTomorrow then yields a confusing "trop courte" message. Add explicit Number.isNaN(date.getTime()) → BadRequestException('Date d\'expiration invalide'). (Aligns with StephaneTrebel's still-open "la date peut aussi être invalide".)

[resolved/non-issue] Items from StephaneTrebel's review:

RequireUserType('human') — the repo has no HUMAN_KIND constant; literal 'human' matches the Prisma enum and existing usage. No change needed.
withRevoked="chocapics" — ZodValidationPipe + CoerceBooleanSchema rejects non-boolean with 400. Already mitigated.

Compliance checklist

Conventional title
Authz preserved: admin routes use RequireAdminPermission; user routes UserGuard + RequireUserType('human') — mirrors Fastify AdminAuthorized / ProjectAuthorized
No secret leakage: hash omitted from all selects/responses
Tests present (service specs + crypto specs + gated e2e)
Remove dead verifyTokenHash export before merge

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: Ib1b5cbbfcd8274912ed9ffb20d60cbed6a6a6964

- Export CoerceBooleanSchema from @cpn-console/shared schemas index so server-nestjs can import it (TS2305 broke the Docker build). - Add apps/server-nestjs to sonar.javascript.lcov.reportPaths; the PR's new code was uncovered and failed the new-code coverage quality gate. Co-Authored-By: Shikanime Deva <william.phetsinorath@shikanime.studio>

Move './_utils.js' before './config.js' to satisfy perfectionist/sort-exports in the shared lint gate. Co-Authored-By: Shikanime Deva <william.phetsinorath@shikanime.studio>

Cover the newly migrated admin-token and user-tokens data-access functions (list/create/revoke, bot-user creation) with mockDeep-based unit tests. Lifts new-code coverage for the SonarQube quality gate alongside the build/runtime fixes already on this branch.

github-actions · 2026-07-28T12:22:47Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T12:24:49Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T12:26:01Z

🤖 Hey !

The security scan report for the current pull request is available here.

The vitest coverage reporter was set to text/json/html but no lcov, so `vitest --coverage` never wrote coverage/lcov.info. Sonar's reportPath for server-nestjs then resolved to nothing and the PR's new code reported 0% coverage, failing the quality gate. Match the sibling apps' ['text','lcov']. Signed-off-by: William Phetsinorath <william.phetsinorath@shikanime.studio> Co-Authored-By: Shikanime Deva <william.phetsinorath@shikanime.studio>

github-actions · 2026-07-28T12:39:56Z

🤖 Hey !

The security scan report for the current pull request is available here.

cloud-pi-native-sonarqube · 2026-07-28T12:41:09Z

Quality Gate failed

11 New Issues (is greater than 10)

Project ID: cloud-pi-native_console_AYsa46O31ebDtzs-pYyZ

View in SonarQube

github-advanced-security AI found potential problems Jun 30, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/modules/admin-token/admin-token.service.ts Fixed

Comment thread apps/server-nestjs/src/modules/user-tokens/user-tokens.service.ts Fixed

shikanime force-pushed the shikanime/push-uotmotuknkvy branch from a9a75b7 to 64a5234 Compare June 30, 2026 10:02

github-advanced-security AI found potential problems Jun 30, 2026

View reviewed changes

shikanime force-pushed the shikanime/push-uotmotuknkvy branch 10 times, most recently from b37e308 to 0331d48 Compare July 1, 2026 09:54

github-code-quality Bot found potential problems Jul 1, 2026

View reviewed changes

Comment thread ...odules/infrastructure/auth/keycloak-secret-provider/keycloak-secret-provider.service.spec.ts Fixed

Comment thread apps/server-nestjs/src/modules/registry/registry-client.service.spec.ts Fixed

shikanime force-pushed the shikanime/push-uotmotuknkvy branch from 0331d48 to c53cb23 Compare July 1, 2026 10:19

github-code-quality Bot found potential problems Jul 1, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts Fixed

Comment thread apps/server-nestjs/src/modules/gitlab/gitlab.service.ts Fixed

shikanime force-pushed the shikanime/push-uotmotuknkvy branch 2 times, most recently from 09f3402 to b667443 Compare July 1, 2026 11:29

github-code-quality Bot found potential problems Jul 1, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/modules/project-roles/project-roles.service.ts Fixed

shikanime force-pushed the shikanime/push-uotmotuknkvy branch 2 times, most recently from 1b31261 to f16a730 Compare July 1, 2026 12:00

shikanime changed the title ~~refactor(server-nestjs): migrate project hook~~ refactor(user-tokens): migrate from server Jul 1, 2026

shikanime force-pushed the shikanime/push-uotmotuknkvy branch 4 times, most recently from cecad1b to 10b4770 Compare July 1, 2026 13:35

github-actions Bot added the built label Jul 1, 2026

shikanime force-pushed the shikanime/push-uotmotuknkvy branch 2 times, most recently from 0eb3e93 to 43d21dc Compare July 1, 2026 13:56

shikanime force-pushed the shikanime/push-uotmotuknkvy branch from 792f711 to 1740e91 Compare July 16, 2026 08:38

StephaneTrebel previously approved these changes Jul 16, 2026

View reviewed changes

shikanime mentioned this pull request Jul 16, 2026

💡 [REQUEST] - PluginManager : factoriser la plomberie commune des plugins (Log, Cron, event, autoSync/Suspend, flags) #2323

Open

5 tasks

shikanime mentioned this pull request Jul 23, 2026

refactor: use HTTP code constant from NestJS instead of static values #2357

Merged

shikanime commented Jul 27, 2026

View reviewed changes

github-code-quality Bot found potential problems Jul 27, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/modules/admin-token/admin-token.service.spec.ts Fixed

Comment thread apps/server-nestjs/src/modules/user-tokens/user-tokens.service.spec.ts Fixed

shikanime added 4 commits July 27, 2026 16:35

refactor(user-tokens): migrate from server

50a17a1

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: Ib1b5cbbfcd8274912ed9ffb20d60cbed6a6a6964

style(shared): sort schemas exports per perfectionist rule

a8de082

Move './_utils.js' before './config.js' to satisfy perfectionist/sort-exports in the shared lint gate. Co-Authored-By: Shikanime Deva <william.phetsinorath@shikanime.studio>

Uh oh!

Conversation

shikanime commented Jun 30, 2026

Issues liées

Quel est le comportement actuel ?

Quel est le nouveau comportement ?

Cette PR introduit-elle un breaking change ?

Autres informations

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 1, 2026

Uh oh!

github-actions Bot commented Jul 16, 2026

Uh oh!

github-actions Bot commented Jul 16, 2026

Uh oh!

github-actions Bot commented Jul 16, 2026

Uh oh!

StephaneTrebel commented Jul 16, 2026

Uh oh!

github-actions Bot commented Jul 16, 2026

Uh oh!

github-actions Bot commented Jul 16, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

shikanime commented Jul 27, 2026

Uh oh!

github-actions Bot commented Jul 27, 2026

Uh oh!

shikanime left a comment

Choose a reason for hiding this comment

Review: #2279 — refactor(user-tokens): migrate from server

Findings

Compliance checklist

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

cloud-pi-native-sonarqube Bot commented Jul 28, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants