refactor(admin-roles): migrate from server by shikanime · Pull Request #2205 · cloud-pi-native/console

shikanime · 2026-06-12T16:58:49Z

Signed-off-by: William Phetsinorath william.phetsinorath-open@interieur.gouv.fr
Change-Id: I941eb6668cf8c98cbd8ea8e8510062216a6a6964## Issues liées

Issues numéro: #2204

Quel est le comportement actuel ?

Quel est le nouveau comportement ?

Cette PR introduit-elle un breaking change ?

Autres informations

github-actions · 2026-06-12T17:02:54Z

🤖 Hey !

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

github-actions · 2026-06-15T12:09:42Z

🤖 Hey !

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

github-actions · 2026-07-21T13:22:55Z

🤖 Hey !

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

github-actions · 2026-07-21T13:25:55Z

🤖 Hey !

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

github-actions · 2026-07-21T13:27:47Z

🤖 Hey !

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

github-actions · 2026-07-21T13:37:34Z

🤖 Hey !

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

github-actions · 2026-07-21T13:38:20Z

🤖 Hey !

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

github-actions · 2026-07-21T13:54:41Z

🤖 Hey !

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

github-actions · 2026-07-21T14:01:24Z

🤖 Hey !

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

github-actions · 2026-07-21T14:09:40Z

🤖 Hey !

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

github-actions · 2026-07-21T14:14:50Z

🤖 Hey !

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

github-actions · 2026-07-21T14:28:05Z

🤖 Hey !

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

github-actions · 2026-07-21T14:36:30Z

🤖 Hey !

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

github-actions · 2026-07-21T14:41:28Z

🤖 Hey !

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

github-actions · 2026-07-21T14:46:28Z

🤖 Hey !

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

github-actions · 2026-07-21T14:47:18Z

🤖 Hey !

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

github-actions · 2026-07-21T14:56:59Z

🤖 Hey !

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

github-actions · 2026-07-21T15:58:18Z

🤖 Hey !

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

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I6e2bf93ddac464c88a82683dc6e6b0846a6a6964 Signed-off-by: Shikanime Deva <william.phetsinorath@shikanime.studio>

github-actions · 2026-07-23T09:29:46Z

🤖 Hey !

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

github-actions · 2026-07-23T09:30:24Z

🤖 Hey !

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

cloud-pi-native-sonarqube · 2026-07-23T09:32:14Z

Quality Gate passed

Issues

Measures

Project ID: cloud-pi-native_console_AYsa46O31ebDtzs-pYyZ

View in SonarQube

shikanime · 2026-07-25T13:21:32Z

Code Review — `refactor(admin-roles): migrate from server` (PR #2205)

Scope note: this review covers the admin-role Fastify→NestJS migration (commits bd426bdcf6, 6fe5a02d01, +617/−23, 9 source files). It was routed here from a task labeled shikanime/push-ktsssqsyoktz, but that branch is PR #2365 (config migration) and contains no admin-role files — the actual code under review is PR #2205. Findings below are against apps/server-nestjs/src/modules/admin-role/*, compared with the Fastify source in apps/server/src/resources/admin-role/.

Verdict: 🚫 REQUEST CHANGES

One blocker and three warnings are functional/parity regressions vs Fastify that should be fixed before this lands.

🔴 Blocker

B1 — adminRole.upsert / adminRole.delete events are emitted with zero subscribers → admin-role provisioning is silently dropped
apps/server-nestjs/src/modules/admin-role/admin-role.service.ts#L44 #L123 #L168

Fastify called hook.adminRole.upsert(id) / hook.adminRole.delete(role) on every mutate (apps/server/src/resources/admin-role/business.ts#L42 #L66 #L94). Those hooks drive the gitlab and keycloak plugin steps upsertAdminRole / deleteAdminRole (plugins/gitlab/src/functions.ts#L238 #L277, plugins/keycloak/src/functions.ts#L246 #L292) — i.e. OIDC group / Keycloak role provisioning on role changes.

NestJS emits this.eventEmitter.emitAsync('adminRole.upsert' | 'adminRole.delete', ...) but there is no @OnEvent('adminRole.upsert' | 'adminRole.delete') listener anywhere (grep across apps/server-nestjs/src returns only the emit sites + the spec). AppEventsService only bridges project.upsert / project.delete (apps/server-nestjs/src/modules/events/app-events.service.ts#L12). So creating/updating/deleting an admin role no longer triggers plugin sync — a silent regression, no error thrown.

Suggested fix (mirror project events): add @OnEvent('adminRole.upsert') / @OnEvent('adminRole.delete') handlers in apps/server-nestjs/src/modules/gitlab/gitlab.service.ts and the keycloak service that invoke their existing upsertAdminRole / deleteAdminRole hook steps via capturePluginResult(...) — exactly like GitlabService.handleUpsert (gitlab.service.ts#L62 #L63). Also add an AdminRoleEventName type + EventLogAction entries in app-events.service.ts (see W2).

🟠 Warnings

W2 — Admin-role mutations are no longer written to the admin action log
admin-role.service.ts (whole) vs apps/server/src/resources/admin-role/business.ts#L43 #L67 #L95

Fastify called addLogs({ action: 'Create/Update/Delete Admin Role', data: hookReply, requestId }) on every mutate. AdminRoleService never calls the log service, and EventLogAction (app-events.service.ts#L21) has no admin-role entries. Audit trail for role changes is lost. Wire the B1 fix through AppEventsService so merged results get logged.

W3 — patch permissions handling diverges from Fastify (truthy vs presence)
admin-role.service.ts#L83

Fastify: permissions: matchingRole.permissions ? BigInt(...) : dbRole.permissions → only overwrites when the string is truthy.
NestJS: permissions: matchingRole.permissions === undefined ? dbRole.permissions : BigInt(matchingRole.permissions) → overwrites whenever the key is present.

A client sending permissions: '0' (valid "no perms" bitmask) is kept by Fastify but reset to 0n by NestJS. Same input, different outcome. Choose one behavior deliberately — prefer replicating Fastify's truthy rule for byte-for-byte parity, or document the presence-based change.

W4 — create returns a single role; Fastify returned the full list (admin-role.service.ts#L56)
Contract createAdminRole response is 200: AdminRoleSchema (singular), so NestJS is contract-correct and Fastify violated it. Low risk — just confirm the client doesn't depend on the array shape.

🟡 Nits

N5 — Redundant AuthModule import admin-role.module.ts#L2 #L10
UserGuard / RequireAdminPermission come from UserPermissionModule, which already imports AuthModule. The direct import is transitively satisfied — drop it.

N6 — Event-emit ordering inconsistent: inside the $transaction for delete (#L168), outside for create (#L44) / patch (#L123)
If B1 is fixed with listeners, emitting inside the transaction makes plugin side-effects (Keycloak/GitLab network calls) run within the DB transaction window. Emit after commit for all three, consistent with create / patch.

N7 — Test coverage is a single happy-path create test admin-role.service.spec.ts
Only create is exercised; list, patch, memberCounts, delete (incl. NotFound path and the position-coherence 400) are untested. Add specs before merge.

Security / standards — clean

No hardcoded secrets, no string-built SQL (all Prisma parameterized), no eval/exec.
Controller input validation is solid: ParseUUIDPipe + ZodValidationPipe; UserGuard + @RequireAdminPermission('ManageRoles') on mutating routes; list intentionally unguarded (matches Fastify).
toAdminRole/select cover all 6 AdminRole model fields; type ?? 'managed' matches legacy.
Lint passes; new spec passes (1/1).

Approval criteria

B1 fixed (admin-role events have real subscribers; plugin provisioning restored) — blocker.
W2 fixed (audit-log writes restored).
W3 resolved with a deliberate, documented behavior (parity preferred).
N7 addressed (specs for list/patch/delete + error paths) — strongly recommended pre-merge.
W4 / N5 / N6 are non-blocking polish.

shikanime self-assigned this Jun 12, 2026

shikanime added technical debt Résoud de la dette technique tech Technical issue labels Jun 12, 2026

shikanime linked an issue Jun 12, 2026 that may be closed by this pull request

[NestJS] Port admin-role module from Fastify server to server-nestjs #2204

Open

5 tasks

github-actions Bot added the built label Jun 12, 2026

shikanime force-pushed the shikanime/push-tlxokqwmmpnv branch from 9732d98 to a6fb16f Compare June 12, 2026 17:08

shikanime changed the base branch from shikanime/push-wtnzonmntzmn to shikanime/push-prlytoyrrvsz June 12, 2026 17:53

shikanime force-pushed the shikanime/push-prlytoyrrvsz branch from 6f5e810 to 863e72d Compare June 15, 2026 09:56

shikanime force-pushed the shikanime/push-tlxokqwmmpnv branch 3 times, most recently from 072e499 to 98d9d04 Compare June 15, 2026 12:05

shikanime force-pushed the shikanime/push-prlytoyrrvsz branch 13 times, most recently from cad00e2 to 2199b21 Compare June 16, 2026 16:44

Base automatically changed from shikanime/push-prlytoyrrvsz to main June 17, 2026 07:26

shikanime force-pushed the shikanime/push-tlxokqwmmpnv branch from 98d9d04 to 4b1836d Compare June 17, 2026 12:25

shikanime changed the base branch from main to shikanime/push-lnpwrpwymovu June 17, 2026 12:26

shikanime force-pushed the shikanime/push-tlxokqwmmpnv branch from 4b1836d to 3cd401d Compare June 17, 2026 12:28

StephaneTrebel self-requested a review July 15, 2026 15:26

StephaneTrebel requested changes Jul 15, 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 removed this from the 9.20.0 milestone Jul 16, 2026

StephaneTrebel removed a link to an issue Jul 16, 2026

[NestJS] Port admin-role module from Fastify server to server-nestjs #2204

Open

5 tasks

shikanime force-pushed the shikanime/push-tlxokqwmmpnv branch from 7ffacd8 to c4ed596 Compare July 21, 2026 13:15

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

View reviewed changes

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

refactor(admin-roles): migrate from server

bd426bd

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I6e2bf93ddac464c88a82683dc6e6b0846a6a6964 Signed-off-by: Shikanime Deva <william.phetsinorath@shikanime.studio>

This was referenced Jul 27, 2026

feat: clarify default config value #2370

Closed

refactor(admin-roles): migrate from server #2371

Open

Uh oh!

Conversation

shikanime commented Jun 12, 2026

Quel est le comportement actuel ?

Quel est le nouveau comportement ?

Cette PR introduit-elle un breaking change ?

Autres informations

Uh oh!

github-actions Bot commented Jun 12, 2026

Uh oh!

github-actions Bot commented Jun 15, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

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!

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

Quality Gate passed

Issues

Measures

Uh oh!

shikanime commented Jul 25, 2026

Code Review — refactor(admin-roles): migrate from server (PR #2205)

Verdict: 🚫 REQUEST CHANGES

🔴 Blocker

🟠 Warnings

🟡 Nits

Security / standards — clean

Approval criteria

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Code Review — `refactor(admin-roles): migrate from server` (PR #2205)