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

Ajout de logs lors de la generation de documents #612

Merged
merged 14 commits into from
Aug 12, 2022

Conversation

jusabatier
Copy link
Collaborator

Ce patch permet l'ajout de logs lorsque des documents sont générés.

Pour gérer ça correctement, j'ai du remplacer le système de logs (logback) par un plus récent (log4j2).
Cela a pour conséquence le changement du fichier de log dans le datadir cadastrapp : ce n'est plus logback.xml mais log4J/log4j2.properties

Par defaut, la generation de BP, RP, Demande et Export génèrent des logs dans /tmp/cadastrapp-audit.log
Ce fichier sera archivé chaque jour sous la forme cadastrapp-backup-%d{yyyy-MM-dd}.log.gz.

Dans le fichier log4j2.properties fourni en exemple, il y a une config permettant de générer les logs directement dans des tables en BDD.
Il suffit de la coper dans le fichier de votre datadir puis :

  • renseigner les propriétés en haut de fichier (dbUser,dbPassword,dbConnectionString) et les décommenter
  • commenter la ligne 9 et décommenter la 10 (appenders)
  • décommenter les lignes 31 à 173 et 184 à 187

Les script pour les tables à créer en BDD, à adapter à votre convenance :

CREATE TABLE IF NOT EXISTS public.cadastrapp_bp
(
    username text,
    log_date timestamp with time zone NOT NULL,
    uri text,
    organisation text,
    roles text,
    message text,
    methode text GENERATED ALWAYS AS ( split_part(message,' - ',2) ) STORED,
    demande text GENERATED ALWAYS AS ( split_part(message,' - ',3) ) STORED,
    parcelle text GENERATED ALWAYS AS ( split_part(message,' - ',4) ) STORED,
    proprietaires text GENERATED ALWAYS AS ( split_part(message,' - ',5) ) STORED,
    copro text GENERATED ALWAYS AS ( split_part(message,' - ',6) ) STORED
);
ALTER TABLE public.cadastrapp_bp OWNER TO cadastrapp;

CREATE TABLE IF NOT EXISTS public.cadastrapp_rp
(
    username text,
    log_date timestamp with time zone NOT NULL,
    uri text,
    organisation text,
    roles text,
    message text,
    methode text GENERATED ALWAYS AS ( split_part(message,' - ',2) ) STORED,
    demande text GENERATED ALWAYS AS ( split_part(message,' - ',3) ) STORED,
    ccomunal text GENERATED ALWAYS AS ( split_part(message,' - ',4) ) STORED,
    parcelle text GENERATED ALWAYS AS ( split_part(message,' - ',5) ) STORED,
    minimal text GENERATED ALWAYS AS ( split_part(message,' - ',6) ) STORED,
    format text GENERATED ALWAYS AS ( split_part(message,' - ',7) ) STORED
);
ALTER TABLE public.cadastrapp_rp OWNER TO cadastrapp;

CREATE TABLE IF NOT EXISTS public.cadastrapp_demande
(
    username text,
    log_date timestamp with time zone NOT NULL,
    uri text,
    organisation text,
    roles text,
    message text,
    demande text GENERATED ALWAYS AS ( split_part(message,' - ',2) ) STORED,
    usertype text GENERATED ALWAYS AS ( split_part(message,' - ',3) ) STORED
);
ALTER TABLE public.cadastrapp_demande OWNER TO cadastrapp;

CREATE TABLE IF NOT EXISTS public.cadastrapp_export
(
    username text,
    log_date timestamp with time zone NOT NULL,
    uri text,
    organisation text,
    roles text,
    message text,
    type text GENERATED ALWAYS AS ( split_part(message,' - ',2) ) STORED,
    params text GENERATED ALWAYS AS ( split_part(message,' - ',3) ) STORED
);
ALTER TABLE public.cadastrapp_export OWNER TO cadastrapp;

@jusabatier
Copy link
Collaborator Author

Implémente #613


MDC.put("demmandeId", Long.toString(requestId));
MDC.put("isMinimal", (isMinimal?"true":"false"));
docLogger.info("Demmande - "+requestId+" - "+requestInformation.getUser().getType() );
Copy link
Member

Choose a reason for hiding this comment

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

Demande not Demmande :)

@jusabatier
Copy link
Collaborator Author

Un peu de doc sur les logs générés :

Chaque ligne comporte les mêmes infos que les précédents logs Cadastrapp, à savoir ce layout :

%d %-5p [%c] %X{uri} - $${ctx:sec-username:-nouser} - $${ctx:sec-roles:-norole} - $${ctx:sec-org:-noorg} - %m%n

Ci-dessous, c'est la composition du message qui est détaillée (%m).

Bordereau parcellaires :

Exemple :

Bordereau Parcellaire - {Methode} - {Demande} - {Parcelle} - {WithProps} - {Copro}

  • Methode => Moyen par lequel le document a été généré
  • Demande (ID) => Identifiant de la demande si existe, sinon null
  • Parcelle (ID) => Identifiant de la parcelle représentée
  • WithProps (0/1) => 1 si avec propriétaires, 0 sinon
  • Copro (true/false) => Demande de type Copro

Methodes pour les BP :

  • GenerationDirecte
  • DemandeParcelleId
  • DemandeCCId
  • DemandeInfoParcelle
  • DemandeInfoProp
  • DemandeLot

Relevés de propriété :

Exemple :

Releve de propriete - {Methode} - {Demande} - {CompteCommunal} - {Parcelle} - {Minimal} - {Format}

  • Methode => Moyen par lequel le document a été généré
  • Demande (ID) => Identifiant de la demande si existe, sinon null
  • CompteCommunal (ID) => Identifiant du compte communal
  • Parcelle (ID) => Identifiant de la parcelle exporté si renseignée, sinon null
  • Minimal (true/false) => false si export complet, true si minimal
  • Format (PDF/CSV) => Format d'export

Methodes pour les RP :

  • GenerationDirecte
  • DemandeCCId
  • DemandeParcelleId
  • DemandeCoProCCParcelleId
  • DemandeInfoParcelle
  • DemandeInfoProp
  • DemandeLot

Demandes

Exemple :

Demande - {Demande} - {UserType}

  • Demande (ID) => Identifiant de la demande
  • UserType => Type de demandeur

UserType pour les Demandes :

  • A => Administration
  • P1 => Particulier détendeur des droits
  • P2 => Particulier agissant en qualité de mandataire
  • P3 => Particuliers tier

Exports CSV

Exemple :

Export CSV - {Type} - {Params}

  • Type =>Type d'export demandé
  • Params => Parametres fournis pour l'export

Type d'exports :

  • Parcelles
  • Propriétaires
  • CoPropriétaires
  • Lots
  • ComptesCommunaux

Cette doc pourra éventuellement être ajoutée au Wiki pour les admins si la PR est validée.

@MaelREBOUX

@MaelREBOUX
Copy link
Member

MaelREBOUX commented Nov 24, 2021

Oooooh : c'est Noël en avance ?

Alors j'ai spontanément 2 remarques.

La première : il faut enlever l'attribut CompteCommunal de la table cadastrapp_rp. parce que ça prête le flanc au RGPD. En plus je vois pas ce que ça apport en terme de stats.

La deuxième, je nommerai les tables en préfixant stats_ et en ne faisant pas référence à cadastrapp car on est déjà dans un schéma cadastrapp.

Et une question : ça n'aurait pas été possible de mettre tout dans une seule table ? Quitte à ne pas remplir tous les attributs en fonction de la source de la stat. Pcq ce serait plus simple de manipuler 1 seule table pour en tirer des infos.

(je répond vite : il faudrait que je regarde ça de bcp plus près).

@jusabatier
Copy link
Collaborator Author

La première : il faut enlever l'attribut CompteCommunal de la table cadastrapp_rp. parce que ça prête le flanc au RGPD. En plus je vois pas ce que ça apport en terme de stats

En es-tu sur ? Ce n'est qu'un code, pas directement les noms des personnes. Deplus on le retrouve dans les logs NGINX d'un reverse proxy.
Ce patch ne vise pas uniquement à faire des stats, mais également à garder une trace des extractions qui sont faites des données du logiciels. Ainsi en cas de problème il est possible de retrouver d'où a pu venir une fuite de données.

La deuxième, je nommerai les tables en préfixant stats_ et en ne faisant pas référence à cadastrapp car on est déjà dans un schéma cadastrapp.

Et une question : ça n'aurait pas été possible de mettre tout dans une seule table ? Quitte à ne pas remplir tous les attributs en fonction de la source de la stat. Pcq ce serait plus simple de manipuler 1 seule table pour en tirer des infos.

Comme indiqué, la configuration fournie est un exemple de ce qui est faisable. Les scripts SQL sont faits pour cette conf.
Il est tout à fait possible de modifier celle-ci pour mettre en place une autre structure de BDD.

L'essentiel dans cette PR sont :

  • le passage à log4j qui dispose d'un appender JDBC bien plus performant
  • l'ajout d'un nouveau logger pour les documents

@landryb landryb requested a review from pierrejego November 25, 2021 10:14
@jusabatier
Copy link
Collaborator Author

Après discussion avec notre DPO, il s'avère que le compte communal ne pose pas de souci, car il ne permet pas une identification directe d'une personne.

En revanche le nom d'utilisateur le permet. Il reste possible de le conserver afin de garder une trace de ce qui est fait, dans l'éventualité de fuite de données, mais la rétention ne doit pas excéder 1 mois.
Il serait donc pour rester dans les clous du RGPD nécessaire d'anonymiser les données tous les mois.

Ici je fais ça via un CRON qui une fois par mois convertit tous les username de plus d'un mois en MD5 dans la BDD.

@jusabatier jusabatier force-pushed the patch_logs_documents branch from a43357b to 4e3ec0f Compare August 11, 2022 13:55
@landryb landryb self-requested a review August 11, 2022 14:04
Copy link
Member

@landryb landryb left a comment

Choose a reason for hiding this comment

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

J'ai pu builder la branche, vais tester au runtime.

@MaelREBOUX : une opposition a passer de logback a log4j2 ? implication: il faut updater la config dans le datadir georchestra
@pierrejego : pour moi c'est ok de merger (et ptete de tagguer une 2e RC..)

@landryb
Copy link
Member

landryb commented Aug 11, 2022

pour ce que ca vaut, j'ai testé au runtime, et j'ai bien un fichier séparé /tmp/cadastrapp-audit.log avec une ligne par document généré.

$cut -d '-' -f 6- /tmp/cadastrapp-audit.log 
 CRAIG - Bordereau Parcellaire - GenerationDirecte - null - 6302160000A0163 - 0 - false
 CRAIG - Releve de propriete - GenerationDirecte - null - 630216B00045 - 6302160000A0163 - false - PDF
 CRAIG - Export CSV - Parcelles - [6302160000A0163,6302160000A0355,6302160000A0161,6302160000A0162]

donc encore + 👍

par contre il faut une 2e PR complétant celle ci et intégrant le contenu de #612 (comment) dans la doc officielle, permettant de déchiffrer le log log d'audit.

@landryb
Copy link
Member

landryb commented Aug 11, 2022

@jusabatier un truc qui serait pas mal d'ajouter a la PR c'est des exemples dans log4j.properties pour les différents loggers/niveaux de log comme il y'a dans le logback.xml:

        <logger name="org.georchestra.cadastrapp" level="DEBUG" />
        <logger name="org.springframework" level="INFO" />
        
        <!--  database -->
        <logger name="org.hibernate.SQL" level="INFO" />
        <logger name="org.hibernate.type" level="INFO" />
        <logger name="org.springframework.jdbc.core.JdbcTemplate" level="INFO" />
        
        <!--  services -->
        <logger name="org.apache.cxf" level="INFO" />
        
        <!--  pdf generation -->
        <logger name="org.apache.xmlgraphics" level="INFO" />
        <logger name="org.apache.fop" level="ERROR" />

c'est un truc qu'on perd avec la migration de logback a log4j, c'est possible de faire pareil mais c'est mieux si les lignes sont présentes par défaut.

@MaelREBOUX
Copy link
Member

Bonjour,

Avez-vous prévu un ajout dans la documentation ? Car c'est de la config donc il en faudrait.

je verrais bien ça là : https://docs.georchestra.org/cadastrapp/latest/guide_administrateur/configuration_webapp.html
Une 3e entrée.

@jusabatier
Copy link
Collaborator Author

Je vais en faire dans la matinée, mais j'aurais plus vu ça dans la partie installation en fait.

@landryb landryb merged commit 6bcd3c9 into georchestra:master Aug 12, 2022
@MaelREBOUX MaelREBOUX added this to the v 2.1 milestone Aug 22, 2022
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.

3 participants