Conversation
Legacy plugins/gitlab folded unmatched members at AccessLevel.DEVELOPER (d67f96a, 1f1fabd) with a documented TODO to revisit after the pre-fine-grained member migration. The server-nestjs port seeds the fallback at AccessLevel.GUEST, silently downgrading existing members whose OIDC groups no longer match any role suffix. Restore the Developer fallback in generateAccessLevelMapping so no-match members keep push and pipeline-trigger rights, matching the legacy behavior until that migration ships. Refs #2682 Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: If911dd76d8eee03970a3bd36207748256a6a6964
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
shikanime
left a comment
shikanime
left a comment
There was a problem hiding this comment.
Verdict : Commentaire
La modification rétablit la parité avec le plugin legacy (repli DEVELOPER, commits d67f96a / 1f1fabd, #2682) : résumé, spécifications et noms de tests sont alignés, aucune règle TS stricte violée. Point positif : la chaîne de preuve (issue, commits de référence, suite 664 tests au vert) rend la justification auditable. Vérification complémentaire effectuée : le seul autre AccessLevel.GUEST du module (gitlab.service.ts:199) est défensif et inatteignable sur ce chemin, la couverture project.members étant totale ; la nit sur l'absence d'assertion de plafond est résolue par l'auteur même de la revue (aucun test requis, changement assumé).
| return highest | ||
| }, null) | ||
| acc.set(membership.user.id, highest ?? AccessLevel.GUEST) | ||
| acc.set(membership.user.id, highest ?? AccessLevel.DEVELOPER) |
There was a problem hiding this comment.
[🔴 Bloquant — conditionnel] Élévation du repli par défaut GUEST → DEVELOPER sur un chemin de synchronisation d'accès : fail-open, en tension avec le moindre privilège. La justification exigée (parité legacy, #2682) figure dans la description de la PR — à confirmer explicitement par un mainteneur avant fusion ; à défaut, viser AccessLevel.REPORTER en repli intermédiaire.
1 participant
Issues liées
Refs #2682
Quel est le comportement actuel ?
generateAccessLevelMapping(apps/server-nestjs/src/modules/gitlab/gitlab.utils.ts) assigneAccessLevel.GUESTà tout membre dont aucun rôle ne correspond à un groupe OIDC. Un membre existant perd ses droits Developer (push, déclenchement de pipeline) sur son groupe GitLab.Quel est le nouveau comportement ?
Le fallback par défaut devient
AccessLevel.DEVELOPER, rétablissant la parité avec le plugin historiqueplugins/gitlab(d67f96a, 1f1fabd) qui porte un TODO documenté pour un éventuel retour à Guest après la migration des membres pré-fine-grained.gitlab.utils.ts: seed du fallbackGUEST→DEVELOPERgitlab.service.spec.ts: les deux cas sans correspondance (ajout et rétrogradation) attendent désormaisDEVELOPERSuite server-nestjs : 664 passed, 69 skipped, exit 0.
Cette PR introduit-elle un breaking change ?
Non — elle restaure le comportement historique du plugin GitLab pour les membres sans rôle correspondant.
Autres informations
PR #2681 portait la même mitigation mais a été fermée sans fusion ; l'issue #2682 a été rouverte avec preuve (le fallback Guest est toujours en place sur
main).