Development #33944
Deux rôles peuvent avoir le même libellé
0%
Description
Sur une instance mono-collectivité (en multicollectivité c'est impossible, on a une erreur "Ce nom n'est pas unique pour cette collectivité") il est possible de créer un rôle qui a le même libellé qu'un rôle déjà existant.
Exemple sur https://connexion-atolcd.test.entrouvert.org/manage/roles/
où deux rôles "Agent" ont pu être créés :
https://connexion-atolcd.test.entrouvert.org/manage/roles/17/
https://connexion-atolcd.test.entrouvert.org/manage/roles/31/
Fichiers
Demandes liées
Révisions associées
a2_rbac: add partial unique index on Role's name (#33944)
Historique
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
- Fichier 0002-a2_rbac-add-partial-unique-index-on-Role-s-name-3394.patch 0002-a2_rbac-add-partial-unique-index-on-Role-s-name-3394.patch ajouté
- Fichier 0001-manager-always-check-role-s-name-uniqueness-33944.patch 0001-manager-always-check-role-s-name-uniqueness-33944.patch ajouté
- Tracker changé de Support à Development
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Corrigé au niveau du modèle, j'ai viré le code ad-hoc sur le formulaire et j'ai corrigé HideOUColumnMixin qui doit poser l'ou sur le modèle dans clean() et pas save() si on veut que les clean() des modèles fonctionnent (et dans l'esprit Django c'est bien dans clean() qu'on doit finir d'initialiser une instance, pas ModelForm.save()).
J'ai regardé du coté de la prod si on avait des doublons de ce genre, j'en ai trouvé deux qu'on devrait pouvoir corriger d'ici que ça passe en prod :
bdauvergne@authentic:/tmp$ cat check_role_s_name_uniqueness.py
from django.db import connection
from django.db.models import Count
from authentic2.a2_rbac.models import Role
roles = Role.objects.filter(admin_scope_ct__isnull=True).values('ou', 'name').annotate(total=Count('id')).filter(total__gt=1).values_list('ou__name', 'name')
if roles:
print connection.tenant
for ou, name in roles:
print '-', ou, name
bdauvergne@authentic:/tmp$ authentic2-multitenant-manage tenant_command runscript --all-tenants ./check_role_s_name_uniqueness.py
connexion_mesdemarches_cca_bzh
- Collectivité par défaut Agent traitant DSI
connexion_nancy_fr
- Nancy Finances TH
idem en recette :
bdauvergne@authentic:/tmp$ authentic2-multitenant-manage tenant_command runscript --all-tenants ./check_role_s_name_uniqueness.py connexion_atolcd_test_entrouvert_org - Collectivité par défaut Agent connexion_nancy_test_entrouvert_org - Collectivité par défaut Parking Abonnement Beaupré - Collectivité par défaut Secrétariat Elu 38 connexion_strasbourg_test_entrouvert_org - Strasbourg Formation – Gestionnaire déchets
Je peux aussi très bien pousser le premier commit et attendre un moment proprice pour le second.
Mis à jour par Paul Marillonnet il y a presque 5 ans
diff --git a/src/authentic2/a2_rbac/models.py b/src/authentic2/a2_rbac/models.py
index a379eaf0..390d2b24 100644
--- a/src/authentic2/a2_rbac/models.py
+++ b/src/authentic2/a2_rbac/models.py
@@ -215,12 +215,11 @@ class Role(RoleAbstractBase):
def clean(self):
super(Role, self).clean()
- if not self.service and not self.admin_scope_ct_id:
- if not self.id and self.__class__.objects.filter(
- name=self.name, ou=self.ou):
- raise ValidationError(
- {'name': _('This name is not unique over this '
- 'organizational unit.')})
+ qs = self.__class__.objects.filter(name=self.name, ou=self.ou)
+ if self.pk:
+ qs = qs.exclude(pk=self.pk)
+ if qs.exists():
+ raise ValidationError({'name': _('Name already used')})
Puisque c'est d'une contrainte implicite d'unicité sur plusieurs champs du modèle dont il est question ici, je pense qu'on peut aussi renommer cette méthode validate_unique pour coller davantage au processus de validation de modèle de Django.
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
- Fichier 0002-a2_rbac-add-partial-unique-index-on-Role-s-name-3394.patch 0002-a2_rbac-add-partial-unique-index-on-Role-s-name-3394.patch ajouté
- Fichier 0001-manager-always-check-role-s-name-uniqueness-33944.patch 0001-manager-always-check-role-s-name-uniqueness-33944.patch ajouté
Au passage je corrige une absence d'appel à super.clean() (quand on surcharge clean on doit toujours finir par un super.clean()).
Mis à jour par Paul Marillonnet il y a presque 5 ans
- Statut changé de Solution proposée à Solution validée
D'ac.
Mis à jour par Benjamin Dauvergne il y a presque 5 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit 1c6fd8886a5bc295581401aaf77afffffaf1f867
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Thu Jun 13 23:23:02 2019 +0200
a2_rbac: add partial unique index on Role's name (#33944)
commit df6cebea4575e677d5515f53377e0dea44a8f4bf
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Thu Jun 13 22:30:00 2019 +0200
manager: always check role's name uniqueness (#33944)Mis à jour par Frédéric Péters il y a presque 5 ans
- Statut changé de Résolu (à déployer) à Solution déployée
Mis à jour par Benjamin Dauvergne il y a environ 2 ans
- Lié à Support #33166: contrôler le libellé des rôles pour éviter la création de doublons ajouté
manager: always check role's name uniqueness (#33944)