Permissions : Création de groupes pour l'administration de GPS et Pilotage #5309
Conversation
rsebille
force-pushed
the
rsebille/more-groups-and-perms
branch
from
00dbe84 to
aa208be
Compare
rsebille
changed the title
rsebille
force-pushed
the
rsebille/more-groups-and-perms
branch
from
aa208be to
3e13ecd
Compare
| gps_models.FollowUpGroup: PERMS_ALL, | ||
| gps_models.FollowUpGroupMembership: PERMS_ALL, | ||
| users_models.User: PERMS_ADD | PERMS_HIJACK, | ||
| users_models.JobSeekerProfile: PERMS_EDIT, |
There was a problem hiding this comment.
Pourquoi permettre à GPS de mettre à jour les informations de profil ?
There was a problem hiding this comment.
Car je me suis dit que si GPS a besoin d'ajouter des bénéficiaires donc des comptes candidats alors la modification du profil était aussi nécessaire, c'est quelque chose qui a été demandé la semaine dernière par Camille car LJ est en congés et vu qu'il est dans itou-admin bah il a toujours pû 😅.
C'est pour ça que j'ai mis la team GPS pour la relecture, car je sais pas du tout ce qui est OK ou pas 😏.
There was a problem hiding this comment.
Je comprends mieux.
L'idée me met plutôt mal à l'aise car on risque de se retrouver avec des utilisateurs des emplois qui n'ont rien à voir avec le service. Nos règles de gestion peuvent aussi évoluer sans que GPS ne soit au courant. Il y a un risque de données incohérentes. Bon, après, le risque est faible car les cas concernés seront marginaux.
Pour garder une trace des User et UserProfile créés par GPS, je propose que le created_by soit renseigné automatiquement en indiquant le compte de Louis-Jean ou de Camille. Qu'en penses-tu ?
There was a problem hiding this comment.
Traité pour User() dans cette PR : #5321
Pour les jobSeekerProfile() ce n'est pas nécessaire car leur création est lié à la création d'un User() quand celui-ci est un candidat, il n'y a d'ailleurs pas de champs created_by sur le modèle, et de toute façon les permissions n'autorisent justement pas la création de ce type d'objet.
| @@ -283,6 +283,7 @@ class Meta(AbstractUser.Meta): | |||
| ), | |||
| ), | |||
| ] | |||
| permissions = [("hijack_user", "Can impersonate (hijack) other accounts")] | |||
There was a problem hiding this comment.
Excellente idée ! Je suppose donc qu'une autre PR viendra mettre à jour la permission des utilisateurs concernés puis supprimer HIJACK_ALLOWED_USER_EMAILS de itou-secrets ?
There was a problem hiding this comment.
La mise à jour des permissions ne sera pas faite dans une PR pour ne pas faire fuiter les emails mais effectivement une PR suivra pour supprimer HIJACK_ALLOWED_USER_EMAILS sauf si quelqu'un trouve un cas qui demanderais de garder cette liste blanche plutôt que d'utiliser la permission Django.
There was a problem hiding this comment.
👌 Si le cas existe, en tous cas, il ne me saute pas aux yeux.
itou/utils/perms/user.py
Outdated
| @@ -20,7 +20,9 @@ def has_hijack_perm(*, hijacker, hijacked): | |||
| return True | |||
|
|
|||
| # Only whitelisted staff members can hijack other accounts | |||
| if hijacker.is_staff and hijacker.email.lower() in settings.HIJACK_ALLOWED_USER_EMAILS: | |||
| if hijacker.is_staff and ( | |||
There was a problem hiding this comment.
Je pense que tu aurais pu directement retirer le settings, et ajouter la permission dans la foulée.
La probabilité que tu gènes quelqu'un pendant ces quelques minutes est faible, non ?
There was a problem hiding this comment.
Je l'ai pas tant fait pour gêner ou pas que j'avais un doute de si on pouvais encore avoir besoin du settings, et vu que j'accumule les PR ouvertes je voulais pas en "bloquer" une avec une action synchrone, mais je vais rajouter le commit et la passer cette semaine vu que j'ai eu des tampons ;)
Also extract into its own test file instead of the generic one.
rsebille
force-pushed
the
rsebille/more-groups-and-perms
branch
from
3e13ecd to
2257bf6
Compare
🤔 Pourquoi ?
Idées directrices (long terme)
Besoin immédiat
Ne pas avoir de droits personnels sans donner accès à tout le reste via le groupe
itou-adminSimplifier la gestion manuelle de
HIJACK_ALLOWED_USER_EMAILSqui au final contient les emails des personnes concernés par ces 2 groupes