Produits Entr'ouvert » Authentic 2

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

• 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

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).

• les objets liés
• un rendu du champ extra (dépendra du kind)

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)

• log.sso()
• log.login()
• log.logout()
• log.register()
• etc...

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

• 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.

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

un recherche full text toto@example.com 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).

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.

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)

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.

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).

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.

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 ?

• 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)

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.

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.

Dans le pad eo-dev de cette semaine, Frédéric écrivait :

s'il y a juste besoin de date je suggérerais désormais d'utiliser input type=date (cf chrono)

Je pense qu'on a aussi besoin de sélectionner l'heure (si jamais par exemple un veut filtrer les logs sur une journée particulièrement chargée en actions / modifications pour un usager), auquel cas un input type="date" ne serait pas suffisant, non ?

Encore dans le pad eo-dev de cette semaine, Benjamin écrivait :

moz propose du code assez simple pour couvrir tous les cas

Je crois que tu parles de cette documentation mozilla, non ?

https://developer.mozilla.org/fr/docs/Web/HTML/Element/input/datetime-local

Et du coup en effet, c'est coompatible Chrome, Edge et Opera, mais pas Firefox…

Qu'est-ce qu'on fait ? Est-ce qu'on utilise le DateTimePicker d'A2 ?

Je pense qu'on a aussi besoin de sélectionner l'heure (si jamais par exemple un veut filtrer les logs sur une journée particulièrement chargée en actions / modifications pour un usager), auquel cas un input type="date" ne serait pas suffisant, non ?

Faut se calmer, on parle des actions d'un utilisateur. Et à étendre l'idée à un journal de log couvrant toute la plateforme (mais on ne veut pas ça dès ce ticket, je pense), même, je serais en fait pour rien du tout, et s'il fallait quelque chose juste un champ recherche, comme dans passerelle (et s'il détecte une date il filtre sur la date et s'il détecte date/heure sur date/heure).

Benjamin écrivait :

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.

Suivre Benjamin, ne te laisse pas guider par mes remarques.

Mozilla propose un code pas compatible firefox ? :) Ah mon avis il y a incompréhension, mais de toute façon je signalais ça de manière un peu anecdotique ça ne me choque pas que sur un navigateur ancien on soit obligé de taper « 10/12/2019 20:30 » à la main, donc si tu veux commencer à déprécier bootstrap-datetimepicker et si tout le monde est ok avec ça.
J'ai pas étudié le taux de pénétration, et donc sur https://caniuse.com/#feat=input-datetime je vois qu'on perd juste safari desktop et que sur firefox on a pas datetime-local ce qui est gênant.

On pourrait simplement utiliser un https://docs.djangoproject.com/fr/2.1/ref/forms/fields/#splitdatetimefield avec un champs date et un champs time séparés, c'est bien supporté partout; enfin non pas sous edge, bon ben faut choisir entre mal supporté firefox ou mal supporté edge, ou alors garder bootstrap-datetimepicker.

Oui c'est vrai on divague un peu, la priorité c'est la pagination (la vue servira pour l'instant qu'à des objets distincts ayant peu d'historique en majorité), le filtrage on pourra revenir dessus.

Benjamin Dauvergne a écrit :

Oui c'est vrai on divague un peu, la priorité c'est la pagination (la vue servira pour l'instant qu'à des objets distincts ayant peu d'historique en majorité), le filtrage on pourra revenir dessus.

Ok très bien, ça me va. J'ai donc un début de pagination mais, comme mentionné plus tôt (#20695-36), je me prends les pieds dans la compatibilité avec le bout de code ajax pour le rafraichissement du contenu du tableau lors du changement de page. Je continue chercher.

J'ai aussi une drôle de différence de comportement entre le backend postgres (qui fonctionne) et le backend sqlite (qui lui m'envoie une sqlite3.InterfaceError à l'appel des fonctions de log. Je vais aussi chercher pourquoi, une fois le problème JS résolu.

Paul Marillonnet a écrit :

je me prends les pieds dans la compatibilité avec le bout de code ajax pour le rafraichissement du contenu du tableau lors du changement de page. Je continue chercher.

C'était ça : #28991.

Un autre bogue bloquant, #29144, patch à appliquer à moins de n'utiliser que django-jsonfield. À voir donc.

Paul Marillonnet a écrit :

Un autre bogue bloquant, #29144, patch à appliquer à moins de n'utiliser que django-jsonfield. À voir donc.

Quel bug ? Où ça ? Tu ne rapportes rien ici.

Pardon, manque d'information.
Je reproduis le bogue reporté dans https://code.djangoproject.com/ticket/27675, à savoir un champ django.contrib.postgres.fields.jsonb.JSONField qui devient une str au moment de la lecture de la valeur du l'attribut. En l'occurrence l'attribut est Line.extra_json.
Lorsque je déprécie l'utilisation de django-jsonfield (seulement utilisé dans le modèle du module d'authentification OIDC) au profit de ce champ natif JSONField, ce bogue ne survient plus.
Est-ce que tu veux que je fournisse une procédure pour reproduire le bogue ?

Surtout je ne veux pas qu'on abandonne django-jsonfield, on a trop d'utilisateurs Django 1.8 encore, ensuite le jwkset d'OIDCProvider a une utilité, stocker la clé publique d'un IdP OIDC, donc utilise django-jsonfield ici.

Je vais ajouter un dernier patch, pour l'insertion des instructions de log dans les vues A2 déjà existantes.

Et donc, je voyais bien la suite comme ça.
On est bien d'accord ?
Ou on peut viser plus concis, comme définir un décorateur de méthode (voire de classe), pour éviter à chaque fois

def foo(self, ...)
    bar = super(Whatever, self).foo(...)
    logger.some_action(...)
    return bar

Paul Marillonnet a écrit :

Je vais ajouter un dernier patch, pour l'insertion des instructions de log dans les vues A2 déjà existantes.

Et donc, je voyais bien la suite comme ça.
On est bien d'accord ?
Ou on peut viser plus concis, comme définir un décorateur de méthode (voire de classe), pour éviter à chaque fois
[...]
?

Je suis un peu gêné que ça s'appelle logger, je préfèrerai qu'on reste sur une dénomination journal pour pas mélanger avec les loggers Python (et pour pas multiplier les codes ce serait bien que ça log aussi vers logging pour pas avoir des tonnes de code du genre:

journal.sso(user=user, service=service)
logger.info('sso de %s sur %s, user, service)

journal.sso(user=user, service=service)

Là ça passe encore mais je trouve journal.update(self.request.user, self.object) plutôt obscur (dans 0005), déjà oui, comme dans l'exemple, nommer les arguments.

Mais aussi je trouverais plus lisible d'avoir journal.record_update, et record_creation, et record_deletion. (ça fait vraiment bizarre pour moi d'avoir une méthode qui soit un verbe mais qu'en fait le verbe ne s'applique pas à l'objet, e.g. journal.delete(...) ça donne trop l'impression d'une suppression d'un truc du journal).