Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: normalization TCOM #13

Merged
merged 27 commits into from
Jan 9, 2025
Merged

Feat: normalization TCOM #13

merged 27 commits into from
Jan 9, 2025

Conversation

juliengrach-cassation
Copy link
Contributor

No description provided.

Copy link

gitguardian bot commented Dec 17, 2024

⚠️ GitGuardian has uncovered 3 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
14657510 Triggered Generic High Entropy Secret bf94e3a .env.dev View secret
14657512 Triggered Generic High Entropy Secret bf94e3a src/.jest/setupEnvVars.ts View secret
15034016 Triggered Generic High Entropy Secret 8054622 src/.jest/setupEnvVars.ts View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@SebCourvoisier
Copy link
Contributor

SebCourvoisier commented Dec 17, 2024

@mauryaland J'aurais besoin d'un regard Data sur src/batch/normalization/services/computeOccultation.ts : construction des categoriesToOmit (j'ai fait certains trucs un peu au pif...) et des additionalTerms (on a discuté des additionalTerms en amont avec Paul, donc ça devrait être bon)

Copy link

@mauryaland mauryaland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2-3 ajustements à faire pour categoriesToOmit

}

if (occultationsComplementaires.personnePhysicoMoraleGeoMorale !== true) {
categoriesToOmit.push(Categories.PERSONNEPHYSIQUE)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Les personnes physico/géomorales ne sont pas des catégories en soit (plus maintenant). La façon dont on les traite côté data est la suivante : si la catégorie personne morale n'est pas dans les catégories à occulter, alors on va chercher dans les personnes morales détectées par le moteur les personnes physiques et les localités.
Ici, rajouter personne physique à categoriesToOmit revient à ne plus occulter la catégorie la plus importante en terme d'éléments de réidentification. 💥


if (occultationsComplementaires.personnePhysicoMoraleGeoMorale !== true) {
categoriesToOmit.push(Categories.PERSONNEPHYSIQUE)
categoriesToOmit.push(Categories.ETABLISSEMENT)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Voir commentaire plus haut mais la mention géomorale ne concerne pas les établissements mais les localités

Comment on lines 53 to 55
categoriesToOmit.push(Categories.ADRESSE)
categoriesToOmit.push(Categories.CADASTRE)
categoriesToOmit.push(Categories.LOCALITE)
Copy link

@mauryaland mauryaland Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je garderais seulement le cadastre. A vérifier mais je ne vois pas pourquoi on retire adresse et localité des catégories à occulter

}

if (occultationsComplementaires.professionnelMagistratGreffier !== true) {
categoriesToOmit.push(Categories.PROFESSIONNELAVOCAT)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

La catégorie professionnel avocat n'est normalement jamais occulté

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oui, c'est tout le problème de cette logique qui repose sur ce qui est censé se faire par défaut (mais qui n'est explicité nulle part) : ça signifie qu'ici on ne doit pas ajouter un truc à categoriesToOmit, mais à additionalTerms...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Car pour les TCOM on ne reçoit pas de bloc d'occultation par exemple, permettant de déterminer des occultations par défaut que l'on "amende" avec des categoriesToOmit. On reçoit ce qu'ils veulent occulter au final (à partir d'indicateurs cochés ou non par défaut dans leur formulaire géré en amont), et en complément des éléments précis à garder ou à conserver (additionalTerms).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok je vois !

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'arrive après la bataille sur ce sujet mais voila ce que je peux ajouter :
avoir ou non la catégorie PROFESSIONNELAVOCAT dans categoriesToOmit ne change rien a la pseudo, la catégorie est considérée comme 'alwaysVisible' dans label donc sera toujours envoyée au moteur pour être identifiée mais ne sera jamais pseudonymisée.

@SebCourvoisier
Copy link
Contributor

Merci je vais regarder tout ça. Cette gymnastique qui consiste à passer d'une liste de ce qu'on veut occulter à une liste de ce qu'on ne veut pas occulter me fait vriller le cerveau à chaque fois...

}

if (occultationsComplementaires.professionnelMagistratGreffier === true) {
additionalTermsRaw.push('#magistratGreffe')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sur l'occultation des magistrat on peut aussi le gérer via categoriesToOmit comme les autres catégories (label gère bien cette catégorie)
En soit ça fonctionne de passer par les additionalTerms mais c'est moins propre
ça donnerait un truc dans le genre

  if (occultationsComplementaires.professionnelMagistratGreffier !== true) {
    categoriesToOmitRaw.push(Categories.PROFESSIONNELMAGISTRATGREFFIER)
  }

@SebCourvoisier SebCourvoisier marked this pull request as ready for review January 9, 2025 10:35
@SebCourvoisier SebCourvoisier merged commit 8fdc8e4 into dev Jan 9, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants