Empêcher les membres d’une structure de s’y maintenir via des invitations #5430
Conversation
francoisfreitag
added
the
no-changelog
|
En attente de #5429 |
|
Est-ce qu'il serait ok pour vous d'ajouter dans cette PR l'invalidation des invitations dans l'ensemble des scenarios metiers ? On peut s'en parler rapidement si vous voulez pour faire le tour mais je crains que le self invitation ne soit pas suffisant. Comme ca je m'aligne avec la proposition de Xav sur le traitement en deux temps :
|
|
Je pense qu’il serait effectivement utile de s’en parler quelques minutes. Je viens d’envoyer une invitation pour demain matin 9h30. |
francoisfreitag
force-pushed
the
ff/YWH-PGM12163-6
branch
2 times, most recently
from
dfda1c6 to
634caf2
Compare
francoisfreitag
force-pushed
the
ff/YWH-PGM12163-6
branch
4 times, most recently
from
90f99cf to
8bf922d
Compare
francoisfreitag
force-pushed
the
ff/YWH-PGM12163-6
branch
8 times, most recently
from
a137384 to
c3edf06
Compare
francoisfreitag
force-pushed
the
ff/YWH-PGM12163-6
branch
from
c3edf06 to
e4c96cf
Compare
| def expire_invitations(self, user): | ||
| expired = self.invitations.pending().filter(Q(email=user.email) | Q(sender=user)).update(validity_days=0) | ||
| logger.info( | ||
| "Expired %(expired)d invitations to %(org_model)s %(org_id)d for user_id=%(user_id)d.", |
There was a problem hiding this comment.
J'aurais bien vu des appels différents (et des logs différents) pour les deux différents cas expire_invitations_to_user/expire_invitations_from_user.
There was a problem hiding this comment.
(soit 2 méthodes, soit expire_invitations(*, from_user=None, to_user=None) avec assert (from_user is None) != (to_user is None) (ou équivalent)
There was a problem hiding this comment.
Pour quelle raison ?
Lorsqu’on retire un utilisateur d’une structure, on veut bien retirer les invitations pour cet utilisateur et les invitations qu’il a émises. Lors de l’ajout ou de l’activation, je m’attends à ce qu’il n’y ait aucune invitation émise par l’utilisateur, donc les expirer ne me semble pas poser de problème.
Ce serait pour les logs ?
There was a problem hiding this comment.
Oui, c'est principalement pour les logs.
Mais si on rajoute un if pour les logs on pourrait également le mettre pour la requête SQL.
Et enfin pour la compréhension je trouve cela plus clair pour comprendre ce que le code fait.
expire_invitations(from_user=user) permet de savoir tout de suite qu'on désactive les invitations envoyées.
There was a problem hiding this comment.
C’est qu’on va devoir appeler de pair expire_invitations(from_user=user) et expire_invitations(to_user=user) au moins au moment du retrait d’un membre, et probablement à l’ajout pour la compatibilité avec l’ancien code qui n’expirait pas les invitations envoyées par l’utilisateur à la désactivation.
There was a problem hiding this comment.
On pourrait mais a priori si l'utilisateur vient d'accepter l'invitation il n'a pas pu envoyer d'invitations ? 🤔
Après ça ne fait qu'une petite requête SQL et un log indiquant qu'on a rien désactivé avec le bénéfice d'enlever une option à la méthode donc ce n'est pas dramatique mais je trouve cela un peu bizarre 🤷♂️
There was a problem hiding this comment.
S’il est réactivé ?
Jusqu’à maintenant, on n’expirait pas les invitations envoyées lorsqu’un utilisateur quittait une structure. Donc on a potentiellement en base des invitations valides envoyées par un utilisateur désactivé.
There was a problem hiding this comment.
Une autre solution serait effectivement une migration pour les attraper et les corriger.
There was a problem hiding this comment.
Yup, mais finalement la solution ceinture-bretelle me va bien avec 2 requêtes et 2 logs.
Et on pourrait également rajouter des pinces à linge en vérifiant à l'acceptation d'une invitation que le sender est bien encore membre actif de l'organisation 😅
There was a problem hiding this comment.
J’ai remis la solution ceinture bretelle, normalement on devrait être tranquilles sans les pinces à linge.
francoisfreitag
force-pushed
the
ff/YWH-PGM12163-6
branch
3 times, most recently
from
75318d8 to
9d0c1e3
Compare
Je me suis appliqué 😇 |
francoisfreitag
force-pushed
the
ff/YWH-PGM12163-6
branch
from
9d0c1e3 to
6865a1d
Compare
Company and PrescriberOrganization all use `invitations`. A uniform related name allows writing `self.invitations` in `OrganizationAbstract`.
Facilitate future access to timezone.now() in tests.
Ensures a member is not reactivated as an admin.
Explain how to manage their organization to new administrators.
An email is now always sent when an administrator joins a structure.
francoisfreitag
force-pushed
the
ff/YWH-PGM12163-6
branch
from
6865a1d to
593476a
Compare
🤔 Pourquoi ?
Sécurité.
🏝️ Comment tester
https://yeswehack.com/vulnerability-center/reports/310822