Development #42086
L'opération d'auto-administration des rôles devrait être MANAGE_MEMBERS_OP pas CHANGE_OP
0%
Description
Les seuls qui doivent avoir CHANGE_OP sont les administrateurs de tous les rôles (soit globalement soit au niveau de chaque OU) et ceci via ADMIN_OP.
Fichiers
Demandes liées
Révisions associées
Historique
Mis à jour par Benjamin Dauvergne il y a environ 4 ans
- Lié à Development #20513: Ajouter une permission explicite pour gérer les membres d'un rôle ajouté
Mis à jour par Valentin Deniaud il y a presque 4 ans
- Fichier 0001-wip.patch 0001-wip.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Je veux bien un avis sur l'approche, assez simple au demeurant, basée sur la description du ticket qui dit qu'une permission CHANGE d'un rôle sur lui même n'a jamais lieu d'exister.
Mis à jour par Benjamin Dauvergne il y a presque 4 ans
Ça m'a l'air ok.
---
new_perm = Permission.objects.filter(operation=new_op, target_ct=ct, target_id=role.pk).first()
Pas nécessaire si tu mets ou__isnull=True, à cause de l'index d'unicité, donc un get_or_create() suffira, je pense plus propre d'enlever l'ancienne et d'en créer une nouvelle, les permissions sont des objets partagés.
Mis à jour par Valentin Deniaud il y a presque 4 ans
Benjamin Dauvergne a écrit :
je pense plus propre d'enlever l'ancienne et d'en créer une nouvelle, les permissions sont des objets partagés.
Mais est-ce que role1 a le droit d'avoir CHANGE_OP sur role2 ? Le titre du ticket dit que peut-être et la description que non.
Aussi, un truc que j'aimerais bien comprendre, a2_rbac/models.py :
306 self.permissions.through.objects.get_or_create(role=self, permission=self_perm)
J'ai trouvé la doc, https://docs.djangoproject.com/en/1.11/ref/models/fields/#django.db.models.ManyToManyField.through
Mais elle ne me dit pas quel intérêt il y a à faire ça dans ce cas là, ni la différence par rapport à un
self.permissions.add(self_perm).Mis à jour par Benjamin Dauvergne il y a presque 4 ans
Valentin Deniaud a écrit :
Benjamin Dauvergne a écrit :
je pense plus propre d'enlever l'ancienne et d'en créer une nouvelle, les permissions sont des objets partagés.
Mais est-ce que role1 a le droit d'avoir CHANGE_OP sur role2 ? Le titre du ticket dit que peut-être et la description que non.
Je vais simplifier alors peut-être : role.get_admin_role() ne doit pas avoir la permission CHANGE_OP sur role.
Aussi, un truc que j'aimerais bien comprendre, a2_rbac/models.py :
[...]
J'ai trouvé la doc, https://docs.djangoproject.com/en/1.11/ref/models/fields/#django.db.models.ManyToManyField.through
Mais elle ne me dit pas quel intérêt il y a à faire ça dans ce cas là, ni la différence par rapport à unself.permissions.add(self_perm).
Il n'y a pas de différence.
Mis à jour par Valentin Deniaud il y a presque 4 ans
- Fichier 0001-a2_rbac-change-self-admin-permission-to-manage_membe.patch 0001-a2_rbac-change-self-admin-permission-to-manage_membe.patch ajouté
Benjamin Dauvergne a écrit :
Je vais simplifier alors peut-être : role.get_admin_role() ne doit pas avoir la permission CHANGE_OP sur role.
Oui ok ma question n'avait pas trop de sens, on retombe sur un cas déjà traité.
Et du coup je me demande si la migration ne pourrait pas juste être un Permission.objects.filter(operation=change_op, target_ct=role_ct).update(operation=manage_members_op) (mais dans le doute je reste sur mon approche en ciblant spécifiquement les rôles auto-administrés).
Pas nécessaire si tu mets ou__isnull=True, à cause de l'index d'unicité, donc un get_or_create() suffira, je pense plus propre d'enlever l'ancienne et d'en créer une nouvelle, les permissions sont des objets partagés.
Un scénario où ça va casser (#42179 inside), c'est si la permission existe déjà avec l'ou. Dans ce cas, crash dans has_self_administration car la migration a créé la même permission sans ou, du coup le get sur (op=manage_members, target=role) renvoie deux permissions.
Donc soit attendre le fix qui fait que get_admin_role ne met plus l'ou de la permission + réparation des permissions existantes, soit modifier has_self_administration. Je pars sur cette deuxième option.
Mis à jour par Benjamin Dauvergne il y a plus de 3 ans
- Statut changé de Solution proposée à Résolu (à déployer)
commit 3e197664ae314fd02bef8ab434af8cd04d249d30
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Mon Apr 27 15:03:07 2020 +0200
a2_rbac: change self admin permission to manage_members (#42086)Mis à jour par Frédéric Péters il y a plus de 3 ans
- Statut changé de Résolu (à déployer) à Solution déployée
a2_rbac: change self admin permission to manage_members (#42086)