Produits Entr'ouvert » w.c.s.

Ce patch ne traite pas le cas des agents apparaissant dans l'historique (cas pas traité actuellement non plus, donc il n'y a pas de régression).

Patch s'appliquant toujours sur master.

Discussion par ailleurs sur le fait de créer des utilisateurs dans w.c.s. et pas dans authentic…

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

Discussion par ailleurs sur le fait de créer des utilisateurs dans w.c.s. et pas dans authentic…

Ce ticket est intéressant sans cette discussion; en gros je ne vois pas le fait de finir cette discussion comme étant bloquant pour intégrer ce patch.

Ce que fait ce patch c'est de combiner nos obligations légales qu'on ignore pour l'instant avec les besoins fonctionnels de w.c.s., i.e. le fait que les formdata qui perdent l'utilisateur associés produisent une perte de fonctionnalité importante, au niveau du traitement du formdef lui même mais aussi au niveau du traitement global de différentes demandes de l'usager (sans parler des workflows qui nécessitent la présence des variables form_user_name_identifiers_0 pour ne serait-ce que fonctionner).

Ce ticket est intéressant sans cette discussion; en gros je ne vois pas le fait de finir cette discussion comme étant bloquant pour intégrer ce patch.

C'est parce que je me mets à imaginer des critères supplémentaires à prendre en compte, pour éviter des suppressions malheureuses. (genre quelqu'un au guichet, on commence la saisie backoffice de sa demande, on la garde en brouillon pour la reprendre le lendemain parce qu'il y a quelques documents à scanner à inclure dedans, paf le lendemain le compte n'est plus là).

Je laisse 90 jours, c'est dit dans la description, ça devrait aller.

Je laisse 90 jours, c'est dit dans la description, ça devrait aller.

« l'utilisateur n'est lié à qu'à des formulaires en statut terminal dont la dernière action remonte à plus de 90 jours ».

Mais dans 0002-users-add-cronjob-to-delete-users je ne vois pas apparaitre cette notion de "dernière action remonte à plus de 90 jours". Et de là je comprends "« utilisateur n'est (lié qu'à des formulaires en statut terminal) ET (dont la dernière action remonte à plus de 90 jours) ». Et si w.c.s. s'occupe du premier terme (formulaires en statut terminal), ce serait "autre chose" qui assurerait que le message de déprovisionning n'arriverait pas avant 90 jours.

Et aussi, 0002 ne fait pas "formulaires en statut terminal" mais "pas de formulaire du tout", cf la redéfinition du critère en premier commentaire, « l'utilisateur n'est lié à aucun formulaire ».

_~~

Bref, tout un historique à s/me remettre en tête.

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

Je laisse 90 jours, c'est dit dans la description, ça devrait aller.

Oui c'est vrai finalement j'ai choisi de partir sur pas de formulaire du tout, je l'avais oublié, ce qui est mieux parce que :
1. ça ne met pas de durée particulière en dur qu'il faudrait venir configuré, ces questions restent au niveau du paramétrage des workflows,
2. ça évite de se poser la question de ce qui est terminal ou pas,
3. ça oblige à traiter le problème du nettoyage des brouillons (en vrai je n'y pensais pas du tout mais ta remarque me permet d'y penser) si ce n'est pas déjà le cas.

Je vais faire tourner ça en local un peu.

Sur la forme, je ne trouve pas "users: add cronjob to delete users (#24430)" terrible, c'est dommage d'avoir ainsi du code devant faire la distinction sql/pickle. Peut-être plutôt, taper une méthode neutre, genre dans la classe Publisher, enregistrée via register_cronjobs, qui, elle, pourra faire appel à publisher.user_class et appeler du code situé dans la bonne classe (code qui pourrait juste être un get_active_ids, il me semble).

postgresql 10.5 et :

File "/home/fred/src/eo/wcs/wcs/sql.py", line 339, in f
    return func(*args, **kwargs)
File "/home/fred/src/eo/wcs/wcs/sql.py", line 592, in do_user_table
    cur.execute('ALTER TABLE %s ADD COLUMN deleted DEFAULT(FALSE)' % table_name)
psycopg2.ProgrammingError: syntax error at or near "DEFAULT" 
LINE 1: ALTER TABLE users ADD COLUMN deleted DEFAULT(FALSE)
                                             ^

Migration corrigée. Est-ce qu'il y aurait un test pour tester les migrations ?

cf tests/test_sql.py, il y a des test_migration_XX_blah qui donnent une idée.

Tant que la démarche n'a pas été anonymisée le compte restera.

Oui dans w.c.s. mais pas dans a2 et donc plus de lien avec jsondatastore si c'est basé sur le NameID; je ne vois vraiment pas de solution à part de mettre l'expiration des comptes à 2 ans pour tout le monde, puis 3 ans quand on trouvera un truc à faire tous les 2 ans...

Mais côté a2 il y aura eu email à l'usager, une fois, deux fois (?), pour lui dire de venir cliquer s'il ne voulait pas que le compte soit supprimé. Non ?

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

Mais côté a2 il y aura eu email à l'usager, une fois, deux fois (?), pour lui dire de venir cliquer s'il ne voulait pas que le compte soit supprimé. Non ?

Oui tout à fait, si c'est jugé suffisant on est bon, sauf que si les gens oublie et bien adieu les données.

Peut-on envisager de gérer cela avec des attributs sur des rôles ?
Sinon, pour ce type de cas très particulier, on peut aussi envisager ne pas fermer une demande (qui reste donc "active") pendant longtemps.

Stéphane Laget a écrit :

Peut-on envisager de gérer cela avec des attributs sur des rôles ?

On a pas d'attribut sur les rôles, on aura un attribut sur l'OU mais c'est la même pour tous les usagers. On authentifie des usagers pas des associations; l'usager n'aura qu'à retrouver son association dans l'annuaire des associations (s'il n'a pas cliquer dans les mails de relance) et retrouvera ses données (qui sont publiques de toute façon); c'est pour cela que j'indiquais je ne sais plus quand qu'il fallait séparer dans le jsondatastore, les associations (en utilisant leur RNA comme clé dans le jsondatastore) et la liste des associations d'un utilisateur (qui utilise le NameID comme clé).

Sinon, pour ce type de cas très particulier, on peut aussi envisager ne pas fermer une demande (qui reste donc "active") pendant longtemps.

Oui, ça n'aura aucun effet, on parle des utilisateurs là pas des demandes. On doit supprimer les utilisateurs en ligne même si les demandes ne sont pas fermées.

À Grenoble on pourra peut-être mettre 2 ans d'expiration mais à Lyon ce sera 6 mois au niveau de GLC, rien de plus, donc aucune donnée liée à l'utilisateur ne pourra survivre plus longtemps (sauf à la stocker par rapport à un autre identifiant plus pérenne: identifiant métier, email, etc..).

Benjamin Dauvergne a écrit :

Sinon, pour ce type de cas très particulier, on peut aussi envisager ne pas fermer une demande (qui reste donc "active") pendant longtemps.

Oui, ça n'aura aucun effet, on parle des utilisateurs là pas des demandes. On doit supprimer les utilisateurs en ligne même si les demandes ne sont pas fermées.

Je ne comprends pas.
Le sujet du ticket c'est "Supprimer les utilisateurs quand ils n'ont plus de liens avec un compte et plus de formulaires dans un statut non terminal"
Si un utilisateur a toujours des demandes en cours (non terminées, non anonymisées), on ne doit pas supprimer le compte !
Peut-être à Lyon si ça les amuse, mais ce fonctionnement ne peut pas être généralisé sur toutes les instances, cela n'a pas de sens.

À Grenoble on pourra peut-être mettre 2 ans d'expiration mais à Lyon ce sera 6 mois au niveau de GLC, rien de plus, donc aucune donnée liée à l'utilisateur ne pourra survivre plus longtemps (sauf à la stocker par rapport à un autre identifiant plus pérenne: identifiant métier, email, etc..).

Effectivement ce délai d'expiration doit clairement être un paramètre pour chaque instance.

Donc si mon compte est lié à une demande non anonymisée, il ne risque pas d'être effacé (même si j'ai raté les demandes de confirmation - idéalement je ne devrais même pas en recevoir, mon compte étant lié à une demande) ?
C'est la seule chose qui intéresse Stef je pense (en tout cas ça marche pour les cas d'usage asso, pour lesquels l'anonymisation n'est pas nécessaire).

Pour les demandes de citoyens, on pousse effectivement pour que l'anonymisation soit faite comme il faut, ça permet la suppression des comptes inutilisés. Le fait de conserver des comptes citoyens inusités en dehors du contexte asso me semble acceptable, en tout cas tant qu'on a pas de vraie gestion des comptes personnes morales.

Donc si mon compte est lié à une demande non anonymisée, il ne risque pas d'être effacé (même si j'ai raté les demandes de confirmation - idéalement je ne devrais même pas en recevoir, mon compte étant lié à une demande) ?

Ce ticket concerne w.c.s.; l'utilisateur dans w.c.s. ne sera pas supprimé; mais côté authentic, il pourra l'être, ce qu'écrit Benjamin :

On doit supprimer les utilisateurs en ligne même si les demandes ne sont pas fermées.

S'il y a des mécaniques à définir pour empêcher ça, elles doivent être discutées, mais idéalement pas dans ce ticket w.c.s. Je vais faire un message sur la liste pour reprendre tout ça.

Stéphane Laget a écrit :

Benjamin Dauvergne a écrit :

Sinon, pour ce type de cas très particulier, on peut aussi envisager ne pas fermer une demande (qui reste donc "active") pendant longtemps.

Oui, ça n'aura aucun effet, on parle des utilisateurs là pas des demandes. On doit supprimer les utilisateurs en ligne même si les demandes ne sont pas fermées.

Je ne comprends pas.
Le sujet du ticket c'est "Supprimer les utilisateurs quand ils n'ont plus de liens avec un compte et plus de formulaires dans un statut non terminal"
Si un utilisateur a toujours des demandes en cours (non terminées, non anonymisées), on ne doit pas supprimer le compte !
Peut-être à Lyon si ça les amuse, mais ce fonctionnement ne peut pas être généralisé sur toutes les instances, cela n'a pas de sens.

C'est sans fin en terme de dev, si l'IdP doit interroger toutes les applications de la plate-forme pour savoir si oui ou non elle considère que ce compte est inactif et depuis quand, il faut mettre une barrière quelque part. Dire on conserve les comptes 2 ans sans inactivité me paraissait bien suffisant; si au bout de 2 ans sans inactivité il y a des demandes ouvertes, et bien elles pourront toujours être traitée mais la personne ne pourra plus venir les voir en ligne (et encore, il y a le code de suivi).

(discussion sur la liste)

Avant de merger vérifier numéro de migration avec #22651.

Rebasé.

C'est peut-être parce que ce ticket est trop long mais là c'est compliqué d'être sûr de ce qu'on veut, du coup je trouve que le code gagnerait en commentaires, et peut-être aussi à revoir le nommage, si je comprends bien get_to_delete_ids c'est en fait get_unreferenced_ids ?

Il y aurait alors à faire attention il y a désormais des assignations de fonctions à des personnes, ça se trouve dans workflow_roles, ex: {'_receiver': '_user:123'}.

+            deleted_ids = deleted_ids & {user_id for user_id, in cur.fetchall()}

Détail perso mais je suis une vieille personne et cette ligne m'oblige encore à aller chercher dans la doc vérifier la signification de l'opérateur &, je préfère la forme explicite, deleted_ids.intersection(...).

La partie wcs/users.py peut être zappée on est nécessairement en SQL désormais.

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

C'est peut-être parce que ce ticket est trop long mais là c'est compliqué d'être sûr de ce qu'on veut, du coup je trouve que le code gagnerait en commentaires, et peut-être aussi à revoir le nommage, si je comprends bien get_to_delete_ids c'est en fait get_unreferenced_ids ?

À découper en plusieurs fonctions/ensembles ce serait plutôt get_to_delete_ids = get_marked_as_deleted_ids & get_unreferenced_ids. Je trouve que le get_to_delete_ids manifeste bien l'objectif de la fonction, je peux bien sûr la découper plus ou la renommer si ça te parait toujours obscure.

Il y aurait alors à faire attention il y a désormais des assignations de fonctions à des personnes, ça se trouve dans workflow_roles, ex: {'_receiver': '_user:123'}.

[...]

Ok pris en compte.

Détail perso mais je suis une vieille personne et cette ligne m'oblige encore à aller chercher dans la doc vérifier la signification de l'opérateur &, je préfère la forme explicite, deleted_ids.intersection(...).

Ok.

La partie wcs/users.py peut être zappée on est nécessairement en SQL désormais.

Ok retiré.

Branche à jour avec tout ça.

si ça te parait toujours obscur

Oui ça m'est toujours obscur; j'essaie de mettre le doigt sur le truc, c'est peut-être qu'il y a deleted_ids qui commence en étant tous les utilisateurs supprimés puis qui se réduit petit à petit mais ne contient donc plus "tous les utilisateurs supprimés". Peut-être que ça serait plus clair pour moi d'avoir d'avoir des phases explicites :

• 1/ la liste des utilisateurs supprimés
• 2/ l'accumulation d'une liste indépendante, avec les utilisateurs référencés
• 3/ l'intersection pour représenter les utilisateurs à retirer de la table

Truc pas clair lié c'est "# keep only user.id not present in form/card_data.user_id", c'est cette accumulation de delete / keep / not present qui fait pour moi un truc bizarre.

# once a day delete users without any formdata

s/users without any formdata/unreferenced users/

            # referenced in form/card data.workflow_roles_array
            sql_statement = '''SELECT CAST(SUBSTRING(workflow_role.workflow_role FROM 7) AS INTEGER)
                                 FROM %(table)s AS data, UNNEST(data.workflow_roles_array) AS workflow_role
                                 WHERE SUBSTRING(workflow_role.workflow_role FROM 1 FOR 6) = '_user:' ''' % {

il pourrait y avoir un commentaire expliquant la requête, si je la comprends bien quelque chose comme "users will be referenced as "_user:<user id>" entries in workflow_roles_array, filter on values starting with "_user:" (FROM 1 FOR 6) and extract the id part (FROM 7).

Pardon j'ai raté un dernier truc,

        # once a day delete unreferenced users
        cls.register_cronjob(CronJob(cls.clean_deleted_users, name='clean_deleted_users', minutes=[0]))

c'est toutes les heures, là.

Merci. Je ne pousserai que lorsque le boulot sur keepalive sera terminé et poussé pour ne pas avoir de suppression de compte pour des comptes ayant des demandes (pour le client qui demande ce fonctionnement).

Rebasé, je pousse quand c'est vert.

Automatic expiration