Development #20695

Avoir sur les objets un journal des modifications et sur les utilisateurs, en plus, un journal des actions

Ajouté par Mikaël Ates il y a 12 mois. Mis à jour il y a 6 jours.

Statut:En coursDébut:14 déc. 2017
Priorité:HautEchéance:
Assigné à:Paul Marillonnet% réalisé:

0%

Catégorie:-
Version cible:-
Patch proposed:Non

Description

Fonctionnellement ce qui a été fait pour les utilisateurs dans le CUT.

0001-WIP-user-actions-and-modifications-journals-20695.patch Magnifier (8 ko) Paul Marillonnet, 25 oct. 2018 16:39

0001-WIP-add-journal-application-20695.patch Magnifier (16,1 ko) Paul Marillonnet, 08 nov. 2018 11:11

0001-WIP-add-journal-application-20695.patch Magnifier (10,1 ko) Paul Marillonnet, 16 nov. 2018 19:10

0001-WIP-add-journal-application-20695.patch Magnifier (12,1 ko) Paul Marillonnet, 20 nov. 2018 20:29

0001-WIP-add-journal-application-20695.patch Magnifier (27,4 ko) Paul Marillonnet, 21 nov. 2018 18:08

0001-WIP-add-journal-application-20695.patch Magnifier (27,7 ko) Paul Marillonnet, 21 nov. 2018 19:15

0001-WIP-add-journal-application-20695.patch Magnifier (30,6 ko) Paul Marillonnet, 22 nov. 2018 13:06

0001-WIP-add-journal-application-20695.patch Magnifier (33,2 ko) Paul Marillonnet, 22 nov. 2018 21:09

0001-WIP-add-journal-application-20695.patch Magnifier (33,9 ko) Paul Marillonnet, 26 nov. 2018 17:04

0001-WIP-add-journal-application-20695.patch Magnifier (34,8 ko) Paul Marillonnet, 26 nov. 2018 17:32

0001-WIP-add-journal-application-20695.patch Magnifier (36,8 ko) Paul Marillonnet, 27 nov. 2018 15:07

0002-WIP-add-timewindow-filter-for-profile-journal-20695.patch Magnifier (3,42 ko) Paul Marillonnet, 27 nov. 2018 17:53

0001-WIP-add-journal-application-20695.patch Magnifier (40 ko) Paul Marillonnet, 04 déc. 2018 11:37

0001-WIP-add-journal-application-20695.patch Magnifier (40,1 ko) Paul Marillonnet, 05 déc. 2018 17:15


Demandes liées

Lié à Authentic 2 - Development #23494: Écran reprenant les connexions d'un utilisateur Nouveau 29 avr. 2018

Historique

#1 Mis à jour par Mikaël Ates il y a 12 mois

  • Sujet changé de Avoir sur les objets un journal des modification et sur les utilisateurs, en plus, un journal des actions à Avoir sur les objets un journal des modifications et sur les utilisateurs, en plus, un journal des actions

#2 Mis à jour par Mikaël Ates il y a 11 mois

  • Priorité changé de Normal à Haut

#3 Mis à jour par Paul Marillonnet il y a 3 mois

#4 Mis à jour par Paul Marillonnet il y a 3 mois

  • Assigné à mis à Paul Marillonnet

#5 Mis à jour par Paul Marillonnet il y a 3 mois

Donc oui, proche (fonctionnellement au moins) du journal d'actions pour un utilisateur ('/manage/users/<id>/actions-journal/'), donc le code est ici :
https://git.entrouvert.org/authentic2-cut.git/tree/src/authentic2_cut/models.py#n14

#7 Mis à jour par Paul Marillonnet il y a environ 2 mois

Un simple patch qui introduit les deux vues dans le backoffice.

Je vais voir comment introduire les hooks qui viennent créer les entrées de ces deux journaux.

#8 Mis à jour par Paul Marillonnet il y a environ 2 mois

  • Statut changé de Solution proposée à En cours
  • Patch proposed changé de Oui à Non

#9 Mis à jour par Benjamin Dauvergne il y a environ 2 mois

Je vais faire mon casse pied mais je ne souhaite pas partir sur l'exacte réplique de ce qui est fait sur le CUT, pour plusieurs raisons:
  • le CUT ne permet pas de lier une ligne de log à plusieurs objets c'est soit l'acteur et c'est un User soit le sujet et c'est un User aussi
  • le CUT ne permet pas d'ajouter plus d'informations à la ligne de log

Je souhaiterai partie d'un modèle de donnée un peu plus riche de cette forme, dans une application authentic2_journal:

class Line(models.Model):
    timestamp = datetime <- indexé
    kind = positiveinteger (un code, sso, login, registration, etc..) <- indexé
    message = text
    extra = text (du JSON ou null)

class Reference(models.Model):
    timestamp = datetime <- indexé
    line = FK(Line) <- indexé
    representation = Text(max_length=64)  # permet de conserver une idée de l'objet même s'il est supprimé
    target = GFK(target_ct, target_id)
    target_ct = FK(ContentType) <- indexés
    target_id = integer          | ensemble

La répétition du timestamp doit permettre de filtrer directement sur Reference dans les vues de Journal, ça devrait aller plus vite, sur des millions de ligne j'ai eu le problème sur docbow, la jointure coûte cher.

Ensuite au niveau IHM avoir une vue un peu générique réutilisable sur les différents objets (en changeant get_queryset, on filtre pour les évènements d'un utilisateur, d'un service ou d'un rôle) qui utilise de la pagination par date et non par index pour que ça aille vite et du chargement au scroll (en arrivant en base de page on charge les nouvelles lignes). Vu utilisable en BO et FO.

Aussi j'aimerai qu'une ligne de log soit représentée ainsi

24/10/2018 SSO de Benjamin Dauvergne vers demarches.publik.fr +

Il faut que les messages soient courts, évite d'être redondant (c'est pas évident on est bien obligé de répéter le nom de l'utilisateur dans le message, même quand est sur sa vue de log personnelle, idem pour les services), mais restent intelligibles.

Dans la barre de droite on doit voir un champ date intitulé "Avant le [....]" et un moyen de filtrer par grands groupes d'évènements (en gros les évènements d'authentification et les évènements de modifications des données (personnelles ou autre, genre changement des membres d'un rôle).

En appuyant sur le plus on déplie la ligne et en ajax on récupère des informations supplémentaires:
  • les objets liés
  • un rendu du champ extra (dépendra du kind)
24/10/2018 SSO de Benjamin Dauvergne à demarches.publik.fr +
[User Benjamin Dauvergne] [Service demarches.publik.fr]
Protocole: OIDC Prompt:none etc.. des détails

J'aimerai qu'on puisse organiser les kind sous un forme d'arbre, je m'explique:

KINDS = {
  _('Authentication'): {
      "members": {
        _('Register'): 1,
        _('Login'): 2
        _('SSO'): 3,
        _('Logout'): 4,
      },
  },
  _('Modifications'): {
        _('Create'): 5,
        _('Modify'): 6,
        _('Modify members'): 7 <-- uniquement pour les rôles,
        _('Delete'): 8,
  }
}

Ce n'est pas forcément la forme que ça doit prendre parce qu'en fonction du Kind il faut aussi déduire le rendu du champ extra, et donc ça passera peut-être par des classes.

Au niveau API ça doit s'utiliser simplement:

journal.log.sso(user=request.user, service=oidc_client)
Je suis pour éviter une méthode générale log() où on va se retrouver avec des messages et des usages différents un peu partout, il faut cadrer:
  • log.sso()
  • log.login()
  • log.logout()
  • log.register()
  • etc...

#10 Mis à jour par Benjamin Dauvergne il y a environ 2 mois

À déterminer aussi si on log en anglais, en français, un mix, si on se fait chier à traduire (c'est un calvaire à faire).

#11 Mis à jour par Mikaël Ates il y a environ 2 mois

My 2 cents :
  • Ces logs vont aussi servir dans un autre ticket pour afficher en FO, à l'usager, le journal de son compte. Ce doit effectivement être intelligible et traduisible.
  • Admettons qu'on fasse un jour un module de notification des modifications des utilisateurs, ou de n'importe quel objet, peut-être peut-on envisager que ce module tire les événements de ces logs. Et donc ma seule remarque c'est que ce que tu souhaites avec les kinds permettrait bien cela.

#12 Mis à jour par Benjamin Dauvergne il y a environ 2 mois

Alors si on part sur un interface traduisible il faut abandonner l'idée d'avoir des logs utilisables directement (i.e. les message stockés en texte dans la table), on supprime la ligne message, de la colonne kind on trouve la classe kind correspondante, classe qui saura formatter le message en fonction des données dans extra que je renommerai bien content. Ça rend quasi impossible une recherche full text sur les messages mais on peut vraisemblablement faire de la recherche fulltext sur extra, genre si on a

kind extra
registration-request {"email": ""}

un recherche full text sur extra renverra bien la ligne, ça me parait bien suffisant.

Ça peut être bien ici d'enumérer les évènements déjà loggé coté cut pour définir les classes de kind dont on a besoin (on peut renommer kind en event d'ailleurs, histoire de coller au jargon déjà en place).

#13 Mis à jour par Paul Marillonnet il y a environ un mois

Un peu de pseudo-code (avec carrément des parties manquantes), mais pour donner une idée de la direction dans laquelle je pars.
(C'est donc bien d'une nouvelle application authentic2_journal qu'il s'agit.)

Sur ma feuille de route, maintenant, et dans l'ordre :
- faire fonctionner les deux vues du backoffice (pas de pagination ni de chargement au scroll ajax pour l'instant)
- insérer la logique de remplissage des journaux dans les vues des différentes applications a2
- avoir une API qui fonctionne (donc pour préparer le scrolling ajax)
- logique de permissions pour la consultation des journaux, dans le backoffice et dans l'API DRF
- modification des templates pour gérer le scrolling et les différentes options de tri tels que demandés par Benjamin ici.

Est-ce que je me trompe d'approche ? Est-ce que j'oublie quelque chose ?

#14 Mis à jour par Benjamin Dauvergne il y a environ un mois

Paul Marillonnet a écrit :

Sur ma feuille de route, maintenant, et dans l'ordre :
- faire fonctionner les deux vues du backoffice (pas de pagination ni de chargement au scroll ajax pour l'instant)

Ok.

- insérer la logique de remplissage des journaux dans les vues des différentes applications a2

Ok.

- avoir une API qui fonctionne (donc pour préparer le scrolling ajax)

Je mettrai ça de coté tant que tu n'attaques pas la visu ajax.

- logique de permissions pour la consultation des journaux, dans le backoffice et dans l'API DRF

Idem.

- modification des templates pour gérer le scrolling et les différentes options de tri tels que demandés par Benjamin ici.

Mettons cela pour un autre ticket, terminons les éléments de base, avoir les modèles, la définition des types de log et les méthode pour logger insérer les points de logs dans le code existant.

Par contre à la lecture je ne comprends pas pourquoi tu hérites de models.Model sur des tas de classe qui ne sont clairement pas des modèles.

#15 Mis à jour par Paul Marillonnet il y a 25 jours

Un autre patch WIP pour mon propre suivi de l'affaire.

#16 Mis à jour par Paul Marillonnet il y a 21 jours

Et donc oui j'aime bien l'idée de représenter la structuration des logs sous forme d'héritages de classe.
Je le représente en tant que modèles Django pour pouvoir manipuler une FK depuis le modèle Reference, mais on peut sans doute s'en passer, je vais voir.

#17 Mis à jour par Thomas Noël il y a 20 jours

Juste un truc, à regarder le code rapidement : sans doute faut-il penser à conserver les journaux d'un utilisateur qui a été détruit (justement pour savoir quand il l'a été, éventuellement pourquoi). Ca se traduit souvent par une relation lâche dans la base, ie pas une foreignkey, et j'ai l'impression ici que c'est ce qui est fait (GenericForeignKey) ?

(aussi, sur la forme, les "coding: utf-8" ne servent à rien car on code en ascii (ne pas le mettre ça permet de justement vérifier que y'a pas de caractères bizarres qui traînent dans le code) ; en revanche il manque les quelques lignes sur l'agpl mais bon, dans authentic ça se pratique pas vraiment)

#18 Mis à jour par Benjamin Dauvergne il y a 20 jours

Thomas Noël a écrit :

Juste un truc, à regarder le code rapidement : sans doute faut-il penser à conserver les journaux d'un utilisateur qui a été détruit (justement pour savoir quand il l'a été, éventuellement pourquoi). Ca se traduit souvent par une relation lâche dans la base, ie pas une foreignkey, et j'ai l'impression ici que c'est ce qui est fait (GenericForeignKey) ?

Dans mon idée plutôt que de surcharger le message d'informations (genre "sso de Benjamin Dauvergne sur portail citoyen"), l'idée était de mettre des informations dans le champs JSON extra, généralement inutilisée, en cas de suppression de l'objet User et donc d'exception ObjectDoesNotExist en cas d'accès à la GenericForeignKey alors on pourra utiliser "extra" pour redonner un contexte au message (on le type d'object via le content_type, la primary key via object_id et le nom dans extra["user_fullname"] par exemple).

(aussi, sur la forme, les "coding: utf-8" ne servent à rien car on code en ascii (ne pas le mettre ça permet de justement vérifier que y'a pas de caractères bizarres qui traînent dans le code) ; en revanche il manque les quelques lignes sur l'agpl mais bon, dans authentic ça se pratique pas vraiment)

On peut prendre de bonnes habitudes à l'occasion d'une nouvelle application.

#19 Mis à jour par Paul Marillonnet il y a 20 jours

Voilà, une version avec des champs de contexte (Line.extra et Reference.representation), pour la persistance de l'information même lorsque les objets référencés sont détruits.
J'ai ajouté les entêtes AGPL dans le code.
Les quelques tests présents pour l'instant passent.

Prochaine étape pointée par Mikaël : offrir à l'usager une vue de ses actions dans le FO (sans doute un view servie sur /accounts/journal/)
→ Avis au(x) relecteurice(s) : Est-ce qu'on fait un ticket à part pour cette feature purement FO ? Ou bien je peux mettre cette vue dans ce patch aussi ?

#20 Mis à jour par Paul Marillonnet il y a 20 jours

Une très légère modification sur utils.log_event plus proche, je crois, de ce qu'on cherche à avoir.

#21 Mis à jour par Paul Marillonnet il y a 19 jours

Avec des tests sur les vues FO (/accounts/journal/) et BO (/manager/users/<pk>/journal/).
L'internationalisation ne va pas, je corrige ça asap.

#22 Mis à jour par Paul Marillonnet il y a 19 jours

  • Statut changé de Solution proposée à En cours
  • Patch proposed changé de Oui à Non

#23 Mis à jour par Paul Marillonnet il y a 18 jours

Avec une i18n plus correcte, je crois.
Je vais écrire les tests s'assurant du bon fonctionnement de la localisation.

Ensuite, pas forcément dans l'ordre :
- placer les instructions de génération des logs au bon endroit dans les vues A2
- placer les URLs, les vues et les templates des journaux dans l'application principale A2 (seule la logique générique et interne à la gestion de ces journaux resterait dans l'appli authentic2_journal)
- remplacer les fonctions dans authentic2_journal.event_logger par des méthodes d'une classe, pour se laisser la possibilité future de redéfinir le comportement de génération des log (par héritage de la classe et redéfinition des méthodes concernées).

#24 Mis à jour par Paul Marillonnet il y a 15 jours

Paul Marillonnet a écrit :

- remplacer les fonctions dans authentic2_journal.event_logger par des méthodes d'une classe, pour se laisser la possibilité future de redéfinir le comportement de génération des log (par héritage de la classe et redéfinition des méthodes concernées).

Ça donnerait quelque chose comme ça.
Je serais aussi pour la définition d'un paramètre A2_JOURNAL_EVENT_LOGGER, définissant la classe à utiliser, et une fonction authentic2_journal.utils.get_event_logger_class() renvoyant la valeur définie pour ce paramètre.

#25 Mis à jour par Paul Marillonnet il y a 15 jours

Paul Marillonnet a écrit :

Je serais aussi pour la définition d'un paramètre A2_JOURNAL_EVENT_LOGGER, définissant la classe à utiliser, et une fonction authentic2_journal.utils.get_event_logger_class() renvoyant la valeur définie pour ce paramètre.

Comme ça je pense.

#26 Mis à jour par Paul Marillonnet il y a 14 jours

Paul Marillonnet a écrit :

- placer les URLs, les vues et les templates des journaux dans l'application principale A2 (seule la logique générique et interne à la gestion de ces journaux resterait dans l'appli authentic2_journal)

Voilà.
Même si on ne fait pas, dans ce ticket, du chargement dynamique lors du défilement ('infinite scrolling'), je voudrais ajouter de la pagination sur la liste des événements, en plus d'un filtrage sur leur horodatage.

#27 Mis à jour par Frédéric Péters il y a 14 jours

chargement dynamique lors du défilement ('infinite scrolling')

Référence perdue mais sur ce sujet, il faut arrêter de charger lorsque le scroll touche le bas de page, il est mieux d'être explicite, genre un bouton "charger les 10 événéments suivants" (qui les ajoute à la page actuelle, oui).

#28 Mis à jour par Paul Marillonnet il y a 14 jours

Frédéric Péters a écrit :

chargement dynamique lors du défilement ('infinite scrolling')

Référence perdue mais sur ce sujet, il faut arrêter de charger lorsque le scroll touche le bas de page, il est mieux d'être explicite, genre un bouton "charger les 10 événéments suivants" (qui les ajoute à la page actuelle, oui).

Ok, je comprends. Si je fais un ticket pour l'ajout dynamique d'entrée à la liste des événements, je partirai là-dessus.

#29 Mis à jour par Paul Marillonnet il y a 14 jours

Paul Marillonnet a écrit :

en plus d'un filtrage sur leur horodatage.

Ma vision des choses pour y parvenir :
- faire en sorte que la vue de journal accepte deux paramètres de after et before dans la query string.
- rendre dans la vue deux champs DateTime sortis de tout formulaire, possiblement rendus à l'aide du DateTimeWidget d'A2, et faire en sorte que leur utilisation par l'usager recharge la page avec les after et before paramétrés en conséquence.

Je me plante ?

#30 Mis à jour par Paul Marillonnet il y a 14 jours

Paul Marillonnet a écrit :

Je me plante ?

Finalement j'ai fait comme ça (patch WIP 0002 qui s'applique sur le dernier patch WIP posé ici).
J'écris les tests maintenant.

#31 Mis à jour par Paul Marillonnet il y a 14 jours

  • Patch proposed changé de Oui à Non
  • Statut changé de Solution proposée à En cours

#32 Mis à jour par Benjamin Dauvergne il y a 14 jours

  • L'absence de classes pour représenter les "kind" me chagrine (ça me parait essentiel pour avancer ensuite, même si ça fait juste le formatage bateau qu'il y a là pour l'instant)
  • Il ne faut pas récupérer les erreurs lors des log_event() (si ça plante là c'est bien embêtant en fait).
  • La gestion de la pagination est à faire coté vue générique (toutes les vues de log voudront cela) via un système de curseur:
    • on définit en général un identifiant unique ordonnable, exemple (timestamp, id), on trie via .order_by('timestamp', 'id') ou .order_by('-timestamp', '-id'), pour chaque page on génère un curseur de début et de fin (en fait le timestamp,id du premier et dernier élément affiché, si on clique sur précédent on fait (pseudo-code) ?before=cursor(<first_row>) sur suivant ?after=cursor(<first-row>), sur un filtrage par date on générera un ?after=<ladate>,0
    • le filtrage pour un ?after=timestamp,id sera
      .order_by('timestamp', 'id').filter(Q(timestamp__gt=timestamp)|(Q(timestamp=timestamp, id__gt=id))[:page_size]
    • le filtrage pour un ?before=timestamp,id sera
      .order_by('-timestamp', '-id').filter(Q(timestamp__lt=timestamp)|(Q(timestamp=timestamp, id__lt=id))[:page_size]
      , et on prendra soin de faire un reversed sur les données récupérées avant leur affichage (ou en tout cas de les parcourir à l'envers parce qu'on va du plus récent au plus ancien avec ce ordre et qu'on affiche je suppose dans l'ordre chronologique)
    • sur un clique sur précédent/suivant il faut virer le filtrage par date (ré-initialiser le champ qui sert à cela)
    • en entrée sur la vue on devrait faire un ?before=<now> + 1h,0 pour pointer sur les derniers évènements
    • à mon avis il faut prévoir tout de suite un affichage anti-chronologique possible, parce que pour la vue des derniers évènements d'un utilisateur c'est ce qu'on voudra (pour un objet de config par contre non, genre l'historique d'un OIDCProvider ou LibertyProvider)

#33 Mis à jour par Paul Marillonnet il y a 12 jours

Benjamin Dauvergne a écrit :

  • L'absence de classes pour représenter les "kind" me chagrine (ça me parait essentiel pour avancer ensuite, même si ça fait juste le formatage bateau qu'il y a là pour l'instant)

Je ne suis pas sûr de comprendre ce que tu as en tête.
En l'état, la journalisation s'appuie sur un modèle EventKind, dont les objets sont retrouvés au moment de la création des logs (création qui elle même est prise en charge pas une classe EventLogger, ajustable par un paramètre de setting A2_JOURNAL_EVENT_LOGGER).
Je passe à côté de quelque chose visiblement, mais je ne vois pas, pour l'instant, en quoi le modèle actuel fige la gestion du journal, et ne permet pas d'avancer par la suite.

#34 Mis à jour par Benjamin Dauvergne il y a 12 jours

Je suis pour virer app_settings, il n'y a rien à configurer.

Ce que j'imagine en pseudo-pseudo code


_kind_registry = ()

class KindMetaclass(type):
     def __init__(self, name, bases, d):
         super(KindMetaclass, self).__init__(name, bases, d)
         if name != "BaseKind":
             assert set(d.keys()) >= set(['name', 'template_names'])
             assert d['name'] not in _kind_registry
            _kind_registry[d['name']] = self

class BaseKind(object):
     __metaclass__ = KindMetaclass
     long_template_names = []

     def format(self, line):
         return select_template(self.template_names).render(Context(line.extra_json))

     def long_format(self, line):
         if not self.long_template_names:
             return ''
         return select_template(self.long_template_names).render(Context(line.extra_json))

     def log_real(self, **kwargs):
         Line.objects.create(kind=get_kind_model(self.name), extra=json.dumps(kwargs))

# le modèle Kind n'a pas grande utilité, il diminue juste la taille de la
# colonne Kind dans Line (un entier au d'un varchar), mais impose une
# jointure (petite) en plus, ça pourrait être pratique d'ajouter un .select_related('kind')
# implicite le ModelManager de Line.objects

@GlobalCache
def get_kind_model(name):
    return Kind.objects.get(name=name)    

class SSOKind(Kind):
     name = 'sso'
     template_names = ['authentic2_journal/sso.html']
     # dans sso.html 
     # {% blocktrans %}sso on {{service}}{% endblocktrans %}
     def log(self, user, service):
         # get_kind() should be cached, like ContentType are, because they never vary
         return self.log_real(user_id=user.id, service_id=service.id, user='%s' % user, service='%s' % service)

...

class Kind(models.Model):
...
    @property
    def concrete(self):
         return _kind_registry[self.name]

...

class Line(models.Model):
...
    def format(self):
        return self.kind.concrete.format(self)

    def long_format(self):
        return self.kind.concrete.long_format(self)

class Logger(self):
    def __init__(self, kind):
        self.kind = _kind_registry[kind] 

    def __call__(self, **kwargs):
        return self.kind.log(**kwargs)

class LoggerHub(object):
    def __getattr__(self, kind_name):
        return Logger(kind_name)

logger = LoggerHub()
# s'utilise ainsi
# logger.sso(user=user, service=service)

Si on souhaite ajouter de nouveaux comportements au kind ou les faire varier en fonction du type de kind ça nous donne tout l'espace pour le faire.

#35 Mis à jour par Paul Marillonnet il y a 7 jours

Ok, compris, merci pour le bout de pseudo-code, j'ai inclus ça dans mon patch, avec toujours les quelques mêmes tests.
Je passe à la pagination.

#36 Mis à jour par Paul Marillonnet il y a 6 jours

Un début de pagination, mais pour laquelle le code de chargement ajax du contenu de la page (au lieu du rafraichissement complet de celle-ci) ne fonctionne pas. Je n'ai toujours pas compris pourquoi.
Le code en question est ici : http://git.entrouvert.org/authentic.git/tree/src/authentic2/manager/static/authentic2/manager/js/manager.js#n108

Ça me chiffonne. Je continue de chercher.

Formats disponibles : Atom PDF