Projet

Général

Profil

Development #31083

Il est possible de créer des rôles avec un uuid vide

Ajouté par Benjamin Dauvergne il y a environ 5 ans. Mis à jour il y a plus de 4 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Valentin Deniaud
Catégorie:
-
Version cible:
-
Début:
05 mars 2019
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Pas bien, voir #31076.

1. vérifier que le champ est bien en blank=False (ou sans blank=True au moins)
2. ensuite faire le tour des points de créatio en commençant par l'import pour vérifier qu'il y a des checks sur ce point ou au moins que le comportement par défut du save() ou du default le créera
3. compléter les tests existants sur les points de création avec des uuid absent, vide, null, etc..

Fichiers

0001-django_rbac-always-prevent-empty-uuid-31083.patch (828 octets) 0001-django_rbac-always-prevent-empty-uuid-31083.patch Valentin Deniaud, 08 octobre 2019 16:17
0002-data_transfer-fail-importing-on-empty-role-uuid-3108.patch (1,9 ko) 0002-data_transfer-fail-importing-on-empty-role-uuid-3108.patch Valentin Deniaud, 08 octobre 2019 16:17

Demandes liées

Lié à Authentic 2 - Development #31871: ne pas accepter l'import d'un uuid videFermé30 mars 2019

Actions

Révisions associées

Révision 9159c4d7 (diff)
Ajouté par Valentin Deniaud il y a plus de 4 ans

django_rbac: always prevent empty uuid (#31083)

Révision d177f4a2 (diff)
Ajouté par Valentin Deniaud il y a plus de 4 ans

data_transfer: fail importing on empty role uuid (#31083)

Historique

#1

Mis à jour par Benjamin Dauvergne il y a environ 5 ans

  • Description mis à jour (diff)
#3

Mis à jour par Frédéric Péters il y a presque 5 ans

#4

Mis à jour par Valentin Deniaud il y a plus de 4 ans

  • Statut changé de Nouveau à En cours
  • Assigné à mis à Valentin Deniaud

L'uuid est défini dans django_rbac/models.py :

class AbstractBase(models.Model):
    '''Abstract base model for all models having a name and uuid and a
       slug
    '''
    uuid = models.CharField(
        max_length=32,
        verbose_name=_('uuid'),
        unique=True,
        default=utils.get_hex_uuid)

blank=False implicitement donc. (par curiosité j'ai regardé le uuid du model User dans custom_user.py, il a blank=True et null=True, je me demande bien pourquoi)
On a également le default qui ajoute bien un uuid, néanmoins ni l'un ni l'autre ne protègent d'un Role.objects.create(name='hop', uuid='').

Donc la ligne fautive dans data_transfer.py :

173         else:  # Create role
174             self._obj = get_role_model().objects.create(**kwargs)
175             status = 'created'

Le fix le plus direct me paraît être d'ajouter un if not self.uuid: self.uuid=get_hex_uuid() dans le save() du model, et hop plus de problème.
Et dans un second temps ajouter de la validation pour faire échouer l'import d'un fichier avec des uuids vides, mais c'est plus pour faire joli.
ie je trouve que « faire le tour des points de création » c'est pas méga utile par rapport à un fix en amont, d'autant qu'il faudrait le faire pour Role, OrganisationalUnit et Permission.

#5

Mis à jour par Valentin Deniaud il y a plus de 4 ans

#6

Mis à jour par Benjamin Dauvergne il y a plus de 4 ans

Valentin Deniaud a écrit :

L'uuid est défini dans django_rbac/models.py :
[...]
blank=False implicitement donc. (par curiosité j'ai regardé le uuid du model User dans custom_user.py, il a blank=True et null=True, je me demande bien pourquoi)

Hmm ?

122     uuid = models.CharField(
123         _('uuid'),
124         max_length=32,
125         default=utils.get_hex_uuid, editable=False, unique=True)

On a également le default qui ajoute bien un uuid, néanmoins ni l'un ni l'autre ne protègent d'un Role.objects.create(name='hop', uuid='').

Yep.

Donc la ligne fautive dans data_transfer.py :
[...]

Le fix le plus direct me paraît être d'ajouter un if not self.uuid: self.uuid=get_hex_uuid() dans le save() du model, et hop plus de problème.
Et dans un second temps ajouter de la validation pour faire échouer l'import d'un fichier avec des uuids vides, mais c'est plus pour faire joli.
ie je trouve que « faire le tour des points de création » c'est pas méga utile par rapport à un fix en amont, d'autant qu'il faudrait le faire pour Role, OrganisationalUnit et Permission.

Ok.

#7

Mis à jour par Benjamin Dauvergne il y a plus de 4 ans

  • Statut changé de Solution proposée à Solution validée
#8

Mis à jour par Valentin Deniaud il y a plus de 4 ans

Hmm ?

Je sais pas lire, rien de grave.

#9

Mis à jour par Valentin Deniaud il y a plus de 4 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit d177f4a296052550a57fe8b85742fc5cf55b20f9
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Tue Oct 8 16:14:51 2019 +0200

    data_transfer: fail importing on empty role uuid (#31083)

commit 9159c4d70b218d1b6f8333dfbc8f4751ff277dfa
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Tue Oct 8 16:03:19 2019 +0200

    django_rbac: always prevent empty uuid (#31083)
#10

Mis à jour par Frédéric Péters il y a plus de 4 ans

  • Statut changé de Résolu (à déployer) à Solution déployée

Formats disponibles : Atom PDF