From d8b6a7dab18afb7e59af9872e1926d2cf9a1b016 Mon Sep 17 00:00:00 2001 From: Hugo Levy-Falk Date: Thu, 30 Apr 2020 19:59:42 +0200 Subject: [PATCH 1/9] Allow ACL to be used for two instances of the same model. --- re2o/acl.py | 17 ++++++++++------- re2o/mixins.py | 7 ++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/re2o/acl.py b/re2o/acl.py index fd149a97..ae193f08 100644 --- a/re2o/acl.py +++ b/re2o/acl.py @@ -47,8 +47,7 @@ def acl_error_message(msg, permissions): message = msg or _("You don't have the right to edit this option.") if groups: return ( - message - + _("You need to be a member of one of these groups: %s.") % groups + message + _("You need to be a member of one of these groups: %s.") % groups ) else: return message + _("No group has the %s permission(s)!") % " or ".join( @@ -124,7 +123,7 @@ ModelC) ``` The view will be called like this: ``` - view(request, instance_of_A, instance_of_b, *args, **kwargs) + view(request, instance_of_A, instance_of_b, *args, **kwargs) ``` where `*args` and `**kwargs` are the original view arguments. """ @@ -153,7 +152,7 @@ ModelC) """The wrapper used for a specific request""" instances = [] - def process_target(target, fields): + def process_target(target, fields, target_id=None): """This function calls the methods on the target and checks for the can_change_`field` method with the given fields. It also stores the instances of models in order to avoid duplicate DB @@ -161,7 +160,7 @@ ModelC) """ if on_instance: try: - target = target.get_instance(*args, **kwargs) + target = target.get_instance(target_id, *args, **kwargs) instances.append(target) except target.DoesNotExist: yield False, _("Nonexistent entry."), [] @@ -175,8 +174,12 @@ ModelC) error_messages = [] warning_messages = [] - for target, fields in group_targets(): - for can, msg, permissions in process_target(target, fields): + for arg_key, (target, fields) in zip(kwargs.keys(), group_targets()): + if on_instance: + target_id = int(kwargs[arg_key]) + else: + target_id = None + for can, msg, permissions in process_target(target, fields, target_id): if not can: error_messages.append(acl_error_message(msg, permissions)) elif msg: diff --git a/re2o/mixins.py b/re2o/mixins.py index 4e42daa1..77382933 100644 --- a/re2o/mixins.py +++ b/re2o/mixins.py @@ -61,8 +61,7 @@ class FormRevMixin(object): ) elif self.changed_data: reversion.set_comment( - "Field(s) edited: %s" - % ", ".join(field for field in self.changed_data) + "Field(s) edited: %s" % ", ".join(field for field in self.changed_data) ) return super(FormRevMixin, self).save(*args, **kwargs) @@ -93,11 +92,9 @@ class AclMixin(object): return str(cls.__module__).split(".")[0].lower() @classmethod - def get_instance(cls, *_args, **kwargs): + def get_instance(cls, object_id, *_args, **kwargs): """Récupère une instance - :param objectid: Instance id à trouver :return: Une instance de la classe évidemment""" - object_id = kwargs.get(cls.get_classname() + "id") return cls.objects.get(pk=object_id) @classmethod From 68ce40a368928ebee7fa514d8a8218ada1d7b20c Mon Sep 17 00:00:00 2001 From: Hugo Levy-Falk Date: Thu, 30 Apr 2020 21:55:44 +0200 Subject: [PATCH 2/9] fix history view. --- logs/views.py | 97 +++++++++++++++++++++++++++------------------------ 1 file changed, 51 insertions(+), 46 deletions(-) diff --git a/logs/views.py b/logs/views.py index 3f9fac5e..cf69ae38 100644 --- a/logs/views.py +++ b/logs/views.py @@ -104,13 +104,10 @@ from .models import ( ActionsSearch, RevisionAction, MachineHistorySearch, - get_history_class + get_history_class, ) -from .forms import ( - ActionsSearchForm, - MachineHistorySearchForm -) +from .forms import ActionsSearchForm, MachineHistorySearchForm @login_required @@ -180,7 +177,9 @@ def stats_logs(request): revisions.object_list = [RevisionAction(r) for r in revisions.object_list] return render(request, "logs/stats_logs.html", {"revisions_list": revisions}) - return render(request, "logs/search_stats_logs.html", {"actions_form": actions_form}) + return render( + request, "logs/search_stats_logs.html", {"actions_form": actions_form} + ) @login_required @@ -305,19 +304,29 @@ def stats_general(request): "email_state_verified_users": [ _("Users with a confirmed email"), User.objects.filter(email_state=User.EMAIL_STATE_VERIFIED).count(), - Adherent.objects.filter(email_state=User.EMAIL_STATE_VERIFIED).count(), + Adherent.objects.filter( + email_state=User.EMAIL_STATE_VERIFIED + ).count(), Club.objects.filter(email_state=User.EMAIL_STATE_VERIFIED).count(), ], "email_state_unverified_users": [ _("Users with an unconfirmed email"), - User.objects.filter(email_state=User.EMAIL_STATE_UNVERIFIED).count(), - Adherent.objects.filter(email_state=User.EMAIL_STATE_UNVERIFIED).count(), - Club.objects.filter(email_state=User.EMAIL_STATE_UNVERIFIED).count(), + User.objects.filter( + email_state=User.EMAIL_STATE_UNVERIFIED + ).count(), + Adherent.objects.filter( + email_state=User.EMAIL_STATE_UNVERIFIED + ).count(), + Club.objects.filter( + email_state=User.EMAIL_STATE_UNVERIFIED + ).count(), ], "email_state_pending_users": [ _("Users pending email confirmation"), User.objects.filter(email_state=User.EMAIL_STATE_PENDING).count(), - Adherent.objects.filter(email_state=User.EMAIL_STATE_PENDING).count(), + Adherent.objects.filter( + email_state=User.EMAIL_STATE_PENDING + ).count(), Club.objects.filter(email_state=User.EMAIL_STATE_PENDING).count(), ], "actives_interfaces": [ @@ -449,32 +458,36 @@ def stats_users(request): de bannissement par user, etc""" stats = { User._meta.verbose_name: { - Machine._meta.verbose_name_plural: User.objects.annotate(num=Count("machine")).order_by("-num")[ - :10 - ], - Facture._meta.verbose_name_plural: User.objects.annotate(num=Count("facture")).order_by("-num")[ - :10 - ], - Ban._meta.verbose_name_plural: User.objects.annotate(num=Count("ban")).order_by("-num")[:10], - Whitelist._meta.verbose_name_plural: User.objects.annotate(num=Count("whitelist")).order_by( - "-num" - )[:10], + Machine._meta.verbose_name_plural: User.objects.annotate( + num=Count("machine") + ).order_by("-num")[:10], + Facture._meta.verbose_name_plural: User.objects.annotate( + num=Count("facture") + ).order_by("-num")[:10], + Ban._meta.verbose_name_plural: User.objects.annotate( + num=Count("ban") + ).order_by("-num")[:10], + Whitelist._meta.verbose_name_plural: User.objects.annotate( + num=Count("whitelist") + ).order_by("-num")[:10], _("rights"): User.objects.annotate(num=Count("groups")).order_by("-num")[ :10 ], }, School._meta.verbose_name: { - User._meta.verbose_name_plural: School.objects.annotate(num=Count("user")).order_by("-num")[:10] + User._meta.verbose_name_plural: School.objects.annotate( + num=Count("user") + ).order_by("-num")[:10] }, Paiement._meta.verbose_name: { - User._meta.verbose_name_plural: Paiement.objects.annotate(num=Count("facture")).order_by("-num")[ - :10 - ] + User._meta.verbose_name_plural: Paiement.objects.annotate( + num=Count("facture") + ).order_by("-num")[:10] }, Banque._meta.verbose_name: { - User._meta.verbose_name_plural: Banque.objects.annotate(num=Count("facture")).order_by("-num")[ - :10 - ] + User._meta.verbose_name_plural: Banque.objects.annotate( + num=Count("facture") + ).order_by("-num")[:10] }, } return render(request, "logs/stats_users.html", {"stats_list": stats}) @@ -505,22 +518,15 @@ def stats_search_machine_history(request): if history_form.is_valid(): history = MachineHistorySearch() events = history.get( - history_form.cleaned_data.get("q", ""), - history_form.cleaned_data + history_form.cleaned_data.get("q", ""), history_form.cleaned_data ) max_result = GeneralOption.get_cached_value("pagination_number") - events = re2o_paginator( - request, - events, - max_result - ) + events = re2o_paginator(request, events, max_result) - return render( - request, - "logs/machine_history.html", - { "events": events }, - ) - return render(request, "logs/search_machine_history.html", {"history_form": history_form}) + return render(request, "logs/machine_history.html", {"events": events},) + return render( + request, "logs/search_machine_history.html", {"history_form": history_form} + ) def get_history_object(request, model, object_name, object_id): @@ -528,9 +534,7 @@ def get_history_object(request, model, object_name, object_id): Handles permissions and DoesNotExist errors """ try: - object_name_id = object_name + "id" - kwargs = {object_name_id: object_id} - instance = model.get_instance(**kwargs) + instance = model.get_instance(object_id) except model.DoesNotExist: instance = None @@ -544,8 +548,9 @@ def get_history_object(request, model, object_name, object_id): messages.error( request, msg or _("You don't have the right to access this menu.") ) - return False, redirect( - reverse("users:profil", kwargs={"userid": str(request.user.id)}) + return ( + False, + redirect(reverse("users:profil", kwargs={"userid": str(request.user.id)})), ) return True, instance From b440519c73a64827018ce26f43e1431c5ba58a58 Mon Sep 17 00:00:00 2001 From: Gabriel Detraz Date: Thu, 30 Apr 2020 22:00:17 +0200 Subject: [PATCH 3/9] Wrong name for basic django permission change --- tickets/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tickets/models.py b/tickets/models.py index 98a006ac..32d8894f 100644 --- a/tickets/models.py +++ b/tickets/models.py @@ -197,13 +197,13 @@ class CommentTicket(AclMixin, models.Model): """ Check that the user has the right to edit the ticket comment or that it is the author""" if ( - not user_request.has_perm("tickets.edit_commentticket") + not user_request.has_perm("tickets.change_commentticket") and (self.parent_ticket.user != user_request or self.parent_ticket.user != self.created_by) ): return ( False, _("You don't have the right to edit other tickets comments than yours."), - ("tickets.edit_commentticket",), + ("tickets.change_commentticket",), ) else: return True, None, None From ace47e1bc774324f5e92989a978a8db923aea25c Mon Sep 17 00:00:00 2001 From: Gabriel Detraz Date: Thu, 30 Apr 2020 22:21:12 +0200 Subject: [PATCH 4/9] More logic permission check in machine --- machines/locale/fr/LC_MESSAGES/django.po | 48 +++++++++++++++++++----- machines/models.py | 26 ++++++------- 2 files changed, 51 insertions(+), 23 deletions(-) diff --git a/machines/locale/fr/LC_MESSAGES/django.po b/machines/locale/fr/LC_MESSAGES/django.po index a5ebf1e1..204b7463 100644 --- a/machines/locale/fr/LC_MESSAGES/django.po +++ b/machines/locale/fr/LC_MESSAGES/django.po @@ -21,7 +21,7 @@ msgid "" msgstr "" "Project-Id-Version: 2.5\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2020-04-24 18:45+0200\n" +"POT-Creation-Date: 2020-04-30 22:19+0200\n" "PO-Revision-Date: 2018-06-23 16:35+0200\n" "Last-Translator: Laouen Fernet \n" "Language-Team: \n" @@ -192,8 +192,7 @@ msgstr "" "Vous avez atteint le nombre maximal d'interfaces que vous pouvez créer vous-" "même (%s)." -#: machines/models.py:189 machines/models.py:1354 machines/models.py:1373 -#: machines/models.py:1476 machines/models.py:1494 +#: machines/models.py:189 msgid "You don't have the right to edit a machine of another user." msgstr "" "Vous n'avez pas le droit de modifier une machine d'un autre utilisateur." @@ -588,9 +587,19 @@ msgstr "" msgid "You don't have the right to edit the machine." msgstr "Vous n'avez pas le droit d'éditer une machine." -#: machines/models.py:1391 machines/models.py:1511 -msgid "You don't have the right to view machines other than yours." -msgstr "Vous n'avez pas le droit de voir d'autres machines que les vôtres." +#: machines/models.py:1354 +msgid "You don't have the right to edit interfaces of another user." +msgstr "" +"Vous n'avez pas le droit de modifier une interface d'un autre utilisateur." + +#: machines/models.py:1373 +msgid "You don't have the right to delete interfaces of another user." +msgstr "" +"Vous n'avez pas le droit de supprimer une interface d'une autre utilisateur." + +#: machines/models.py:1391 +msgid "You don't have the right to view interfaces other than yours." +msgstr "Vous n'avez pas le droit de voir d'autres interfaces que les vôtres." #: machines/models.py:1419 msgid "Can view an IPv6 addresses list object" @@ -612,17 +621,30 @@ msgstr "Listes d'adresses IPv6" msgid "Nonexistent interface." msgstr "Interface inexistante." -#: machines/models.py:1444 machines/models.py:1688 -msgid "You don't have the right to add an alias to a machine of another user." +#: machines/models.py:1444 +msgid "You don't have the right to add ipv6 to a machine of another user." msgstr "" -"Vous n'avez pas le droit d'ajouter un alias à une machine d'un autre " -"utilisateur." +"Vous n'avez pas le droit d'ajouter des ipv6 à une machine d'un autre utilisateur." #: machines/models.py:1457 msgid "You don't have the right to change the SLAAC value of an IPv6 address." msgstr "" "Vous n'avez pas le droit de changer la valeur SLAAC d'une adresse IPv6." +#: machines/models.py:1476 +msgid "You don't have the right to edit ipv6 of a machine of another user." +msgstr "" +"Vous n'avez pas le droit de modifier les ipv6 d'une machine d'un autre utilisateur." + +#: machines/models.py:1494 +msgid "You don't have the right to delete ipv6 of a machine of another user." +msgstr "" +"Vous n'avez pas le droit de supprimer les ipv6 d'une machine d'une autre utilisateur." + +#: machines/models.py:1511 +msgid "You don't have the right to view ipv6 of machines other than yours." +msgstr "Vous n'avez pas le droit de voir les ipv6 d'autres machines que les vôtres." + #: machines/models.py:1543 msgid "A SLAAC IP address is already registered." msgstr "Une adresse IP SLAAC est déjà enregistrée." @@ -677,6 +699,12 @@ msgstr "Le nom de domaine %s contient des caractères interdits." msgid "Invalid extension." msgstr "Extension invalide." +#: machines/models.py:1688 +msgid "You don't have the right to add an alias to a machine of another user." +msgstr "" +"Vous n'avez pas le droit d'ajouter un alias à une machine d'un autre " +"utilisateur." + #: machines/models.py:1704 #, python-format msgid "" diff --git a/machines/models.py b/machines/models.py index 66866e77..f20fb416 100644 --- a/machines/models.py +++ b/machines/models.py @@ -202,14 +202,14 @@ class Machine(RevMixin, FieldPermissionModelMixin, models.Model): can_user, _message, permissions = self.user.can_edit( self.user, user_request, *args, **kwargs ) - if not (user_request.has_perm("machines.change_interface") and can_user): + if not (user_request.has_perm("machines.delete_interface") and can_user): return ( False, _( "You don't have the right to delete a machine" " of another user." ), - ("machines.change_interface",) + permissions, + ("machines.delete_interface",) + permissions, ) return True, None, None @@ -1351,7 +1351,7 @@ class Interface(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): if not (user_request.has_perm("machines.change_interface") and can_user): return ( False, - _("You don't have the right to edit a machine of another" + _("You don't have the right to edit interfaces of another" " user."), ("machines.change_interface",) + permissions, ) @@ -1367,12 +1367,12 @@ class Interface(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): can_user, _message, permissions = self.machine.user.can_edit( user_request, *args, **kwargs ) - if not (user_request.has_perm("machines.change_interface") and can_user): + if not (user_request.has_perm("machines.delete_interface") and can_user): return ( False, - _("You don't have the right to edit a machine of another" + _("You don't have the right to delete interfaces of another" " user."), - ("machines.change_interface",) + permissions, + ("machines.delete_interface",) + permissions, ) return True, None, None @@ -1388,7 +1388,7 @@ class Interface(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): ): return ( False, - _("You don't have the right to view machines other than yours."), + _("You don't have the right to view interfaces other than yours."), ("machines.view_interface",), ) return True, None, None @@ -1441,7 +1441,7 @@ class Ipv6List(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): return ( False, _( - "You don't have the right to add an alias to a" + "You don't have the right to add ipv6 to a" " machine of another user." ), ("machines.add_ipv6list",), @@ -1473,7 +1473,7 @@ class Ipv6List(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): if not (user_request.has_perm("machines.change_ipv6list") and can_user): return ( False, - _("You don't have the right to edit a machine of another user."), + _("You don't have the right to edit ipv6 of a machine of another user."), ("machines.change_ipv6list",), ) return True, None, None @@ -1488,11 +1488,11 @@ class Ipv6List(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): can_user, _message, permissions = self.interface.machine.user.can_edit( user_request, *args, **kwargs ) - if not (user_request.has_perm("machines.change_ipv6list") and can_user): + if not (user_request.has_perm("machines.delete_ipv6list") and can_user): return ( False, - _("You don't have the right to edit a machine of another user."), - ("machines.change_ipv6list",) + permissions, + _("You don't have the right to delete ipv6 of a machine of another user."), + ("machines.delete_ipv6list",) + permissions, ) return True, None, None @@ -1508,7 +1508,7 @@ class Ipv6List(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): ): return ( False, - _("You don't have the right to view machines other than yours."), + _("You don't have the right to view ipv6 of machines other than yours."), ("machines.view_ipv6list",), ) return True, None, None From 7edef7b72263b94e5c4c85735ec37821bb4d9379 Mon Sep 17 00:00:00 2001 From: Hugo Levy-Falk Date: Thu, 30 Apr 2020 22:27:21 +0200 Subject: [PATCH 5/9] Machine inherits AclMixin --- machines/models.py | 40 ++++++++++++++-------------------------- topologie/models.py | 30 ++++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/machines/models.py b/machines/models.py index 66866e77..84e9f157 100644 --- a/machines/models.py +++ b/machines/models.py @@ -62,7 +62,7 @@ from re2o.field_permissions import FieldPermissionModelMixin from re2o.mixins import AclMixin, RevMixin -class Machine(RevMixin, FieldPermissionModelMixin, models.Model): +class Machine(RevMixin, FieldPermissionModelMixin, AclMixin, models.Model): """ Class définissant une machine, object parent user, objets fils interfaces""" @@ -80,14 +80,6 @@ class Machine(RevMixin, FieldPermissionModelMixin, models.Model): verbose_name = _("machine") verbose_name_plural = _("machines") - @classmethod - def get_instance(cls, machineid, *_args, **_kwargs): - """Get the Machine instance with machineid. - :param userid: The id - :return: The user - """ - return cls.objects.get(pk=machineid) - def linked_objects(self): """Return linked objects : machine and domain. Usefull in history display""" @@ -157,8 +149,7 @@ class Machine(RevMixin, FieldPermissionModelMixin, models.Model): if user != user_request: return ( False, - _("You don't have the right to add a machine to another" - " user."), + _("You don't have the right to add a machine to another" " user."), ("machines.add_machine",), ) if user.user_interfaces().count() >= max_lambdauser_interfaces: @@ -186,8 +177,7 @@ class Machine(RevMixin, FieldPermissionModelMixin, models.Model): if not (user_request.has_perm("machines.change_interface") and can_user): return ( False, - _("You don't have the right to edit a machine of another" - " user."), + _("You don't have the right to edit a machine of another" " user."), ("machines.change_interface",) + permissions, ) return True, None, None @@ -225,8 +215,7 @@ class Machine(RevMixin, FieldPermissionModelMixin, models.Model): ): return ( False, - _("You don't have the right to view other machines than" - " yours."), + _("You don't have the right to view other machines than" " yours."), ("machines.view_machine",), ) return True, None, None @@ -558,8 +547,7 @@ class IpType(RevMixin, AclMixin, models.Model): for element in IpType.objects.all().exclude(pk=self.pk): if not self.ip_set.isdisjoint(element.ip_set): raise ValidationError( - _("The specified range is not disjoint from existing" - " ranges.") + _("The specified range is not disjoint from existing" " ranges.") ) # On formate le prefix v6 if self.prefix_v6: @@ -1302,7 +1290,11 @@ class Interface(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): if not ( preferences.models.OptionalMachine.get_cached_value("create_machine") ): - return False, _("You don't have the right to add a machine."), ("machines.add_interface",) + return ( + False, + _("You don't have the right to add a machine."), + ("machines.add_interface",), + ) max_lambdauser_interfaces = preferences.models.OptionalMachine.get_cached_value( "max_lambdauser_interfaces" ) @@ -1351,8 +1343,7 @@ class Interface(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): if not (user_request.has_perm("machines.change_interface") and can_user): return ( False, - _("You don't have the right to edit a machine of another" - " user."), + _("You don't have the right to edit a machine of another" " user."), ("machines.change_interface",) + permissions, ) return True, None, None @@ -1370,8 +1361,7 @@ class Interface(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): if not (user_request.has_perm("machines.change_interface") and can_user): return ( False, - _("You don't have the right to edit a machine of another" - " user."), + _("You don't have the right to edit a machine of another" " user."), ("machines.change_interface",) + permissions, ) return True, None, None @@ -1761,8 +1751,7 @@ class Domain(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): ): return ( False, - _("You don't have the right to view other machines than" - " yours."), + _("You don't have the right to view other machines than" " yours."), ("machines.view_domain",), ) return True, None, None @@ -1979,8 +1968,7 @@ class OuverturePortList(RevMixin, AclMixin, models.Model): class Meta: permissions = ( - ("view_ouvertureportlist", _("Can view a ports opening list" - " object")), + ("view_ouvertureportlist", _("Can view a ports opening list" " object")), ) verbose_name = _("ports opening list") verbose_name_plural = _("ports opening lists") diff --git a/topologie/models.py b/topologie/models.py index 34c390bb..445d70e5 100644 --- a/topologie/models.py +++ b/topologie/models.py @@ -88,7 +88,7 @@ class Stack(AclMixin, RevMixin, models.Model): ) -class AccessPoint(AclMixin, Machine): +class AccessPoint(Machine): """Define a wireless AP. Inherit from machines.interfaces Definition pour une borne wifi , hérite de machines.interfaces @@ -135,6 +135,12 @@ class AccessPoint(AclMixin, Machine): def __str__(self): return str(self.interface_set.first()) + @classmethod + def get_instance(cls, object_id, *_args, **kwargs): + """Récupère une instance + :return: Une instance de la classe évidemment""" + return cls.objects.get(pk=object_id) + class Server(Machine): """ @@ -173,8 +179,14 @@ class Server(Machine): def __str__(self): return str(self.interface_set.first()) + @classmethod + def get_instance(cls, object_id, *_args, **kwargs): + """Récupère une instance + :return: Une instance de la classe évidemment""" + return cls.objects.get(pk=object_id) -class Switch(AclMixin, Machine): + +class Switch(Machine): """ Definition d'un switch. Contient un nombre de ports (number), un emplacement (location), un stack parent (optionnel, stack) et un id de membre dans le stack (stack_member_id) @@ -457,6 +469,12 @@ class Switch(AclMixin, Machine): def __str__(self): return str(self.get_name) + @classmethod + def get_instance(cls, object_id, *_args, **kwargs): + """Récupère une instance + :return: Une instance de la classe évidemment""" + return cls.objects.get(pk=object_id) + class ModelSwitch(AclMixin, RevMixin, models.Model): """Un modèle (au sens constructeur) de switch""" @@ -532,7 +550,9 @@ class ModuleOnSwitch(AclMixin, RevMixin, models.Model): unique_together = ["slot", "switch"] def __str__(self): - return _("On slot %(slot)s of %(switch)s").format(slot=str(self.slot), switch=str(self.switch)) + return _("On slot %(slot)s of %(switch)s").format( + slot=str(self.slot), switch=str(self.switch) + ) class ConstructorSwitch(AclMixin, RevMixin, models.Model): @@ -842,9 +862,7 @@ class PortProfile(AclMixin, RevMixin, models.Model): radius_type = models.CharField( max_length=32, choices=TYPES, - help_text=_( - "Type of RADIUS authentication: inactive, MAC-address or 802.1X." - ), + help_text=_("Type of RADIUS authentication: inactive, MAC-address or 802.1X."), verbose_name=_("RADIUS type"), ) radius_mode = models.CharField( From c4e84417184a393e20414a8b7b60c0e3321a185e Mon Sep 17 00:00:00 2001 From: Hugo Levy-Falk Date: Thu, 30 Apr 2020 22:32:17 +0200 Subject: [PATCH 6/9] Avoid failing when permissions depending on another model are None. --- cotisations/models.py | 16 ++++++++-------- machines/models.py | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/cotisations/models.py b/cotisations/models.py index dcb62408..00978275 100644 --- a/cotisations/models.py +++ b/cotisations/models.py @@ -138,7 +138,7 @@ class Facture(BaseInvoice): abstract = False permissions = ( # TODO : change facture to invoice - ("change_facture_control", _("Can edit the \"controlled\" state")), + ("change_facture_control", _('Can edit the "controlled" state')), ("view_facture", _("Can view an invoice object")), ("change_all_facture", _("Can edit all the previous invoices")), ) @@ -166,7 +166,7 @@ class Facture(BaseInvoice): return ( False, _("You don't have the right to edit this user's invoices."), - ("cotisations.change_all_facture",) + permissions, + ("cotisations.change_all_facture",) + (permissions or ()), ) elif not user_request.has_perm("cotisations.change_all_facture") and ( self.control or not self.valid @@ -198,7 +198,7 @@ class Facture(BaseInvoice): return ( False, _("You don't have the right to delete this user's invoices."), - ("cotisations.change_all_facture",) + permissions, + ("cotisations.change_all_facture",) + (permissions or ()), ) elif not user_request.has_perm("cotisations.change_all_facture") and ( self.control or not self.valid @@ -243,7 +243,7 @@ class Facture(BaseInvoice): can = user_request.has_perm("cotisations.change_facture_control") return ( can, - _("You don't have the right to edit the \"controlled\" state.") + _('You don\'t have the right to edit the "controlled" state.') if not can else None, ("cotisations.change_facture_control",), @@ -297,21 +297,21 @@ class Facture(BaseInvoice): for purchase in self.vente_set.all(): if hasattr(purchase, "cotisation"): cotisation = purchase.cotisation - if cotisation.type_cotisation == 'Connexion': + if cotisation.type_cotisation == "Connexion": cotisation.date_start = date_con date_con += relativedelta( months=(purchase.duration or 0) * purchase.number, days=(purchase.duration_days or 0) * purchase.number, ) cotisation.date_end = date_con - elif cotisation.type_cotisation == 'Adhesion': + elif cotisation.type_cotisation == "Adhesion": cotisation.date_start = date_adh date_adh += relativedelta( months=(purchase.duration or 0) * purchase.number, days=(purchase.duration_days or 0) * purchase.number, ) cotisation.date_end = date_adh - else: # it is assumed that adhesion is required for a connexion + else: # it is assumed that adhesion is required for a connexion date = min(date_adh, date_con) cotisation.date_start = date date_adh += relativedelta( @@ -566,7 +566,7 @@ class Vente(RevMixin, AclMixin, models.Model): return ( False, _("You don't have the right to edit this user's purchases."), - ("cotisations.change_all_facture",) + permissions, + ("cotisations.change_all_facture",) + (permissions or ()), ) elif not user_request.has_perm("cotisations.change_all_vente") and ( self.facture.control or not self.facture.valid diff --git a/machines/models.py b/machines/models.py index 84e9f157..8c1df4c5 100644 --- a/machines/models.py +++ b/machines/models.py @@ -178,7 +178,7 @@ class Machine(RevMixin, FieldPermissionModelMixin, AclMixin, models.Model): return ( False, _("You don't have the right to edit a machine of another" " user."), - ("machines.change_interface",) + permissions, + ("machines.change_interface",) + (permissions or ()), ) return True, None, None @@ -199,7 +199,7 @@ class Machine(RevMixin, FieldPermissionModelMixin, AclMixin, models.Model): "You don't have the right to delete a machine" " of another user." ), - ("machines.change_interface",) + permissions, + ("machines.change_interface",) + (permissions or ()), ) return True, None, None @@ -1344,7 +1344,7 @@ class Interface(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): return ( False, _("You don't have the right to edit a machine of another" " user."), - ("machines.change_interface",) + permissions, + ("machines.change_interface",) + (permissions or ()), ) return True, None, None @@ -1362,7 +1362,7 @@ class Interface(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): return ( False, _("You don't have the right to edit a machine of another" " user."), - ("machines.change_interface",) + permissions, + ("machines.change_interface",) + (permissions or ()), ) return True, None, None @@ -1482,7 +1482,7 @@ class Ipv6List(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): return ( False, _("You don't have the right to edit a machine of another user."), - ("machines.change_ipv6list",) + permissions, + ("machines.change_ipv6list",) + (permissions or ()), ) return True, None, None From bf55bf0fa9f9a8d27c9ecb63f2cd2ba5d6d15dc8 Mon Sep 17 00:00:00 2001 From: Hugo Levy-Falk Date: Thu, 30 Apr 2020 22:36:41 +0200 Subject: [PATCH 7/9] PEP8 is for everyone. --- machines/models.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/machines/models.py b/machines/models.py index f82610f6..0835f38e 100644 --- a/machines/models.py +++ b/machines/models.py @@ -1361,8 +1361,10 @@ class Interface(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): if not (user_request.has_perm("machines.delete_interface") and can_user): return ( False, - _("You don't have the right to delete interfaces of another" - " user."), + _( + "You don't have the right to delete interfaces of another" + " user." + ), ("machines.delete_interface",) + (permissions or ()), ) return True, None, None @@ -1464,7 +1466,9 @@ class Ipv6List(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): if not (user_request.has_perm("machines.change_ipv6list") and can_user): return ( False, - _("You don't have the right to edit ipv6 of a machine of another user."), + _( + "You don't have the right to edit ipv6 of a machine of another user." + ), ("machines.change_ipv6list",), ) return True, None, None @@ -1482,7 +1486,9 @@ class Ipv6List(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): if not (user_request.has_perm("machines.delete_ipv6list") and can_user): return ( False, - _("You don't have the right to delete ipv6 of a machine of another user."), + _( + "You don't have the right to delete ipv6 of a machine of another user." + ), ("machines.delete_ipv6list",) + (permissions or ()), ) return True, None, None @@ -1499,7 +1505,9 @@ class Ipv6List(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): ): return ( False, - _("You don't have the right to view ipv6 of machines other than yours."), + _( + "You don't have the right to view ipv6 of machines other than yours." + ), ("machines.view_ipv6list",), ) return True, None, None From 367e3061453c5888854316f4aa9f811854abf303 Mon Sep 17 00:00:00 2001 From: Hugo Levy-Falk Date: Fri, 1 May 2020 12:23:41 +0200 Subject: [PATCH 8/9] fix acl functions in topologie models inheriting from Machine --- machines/models.py | 1 + topologie/models.py | 69 +++++++++++++++++++----- topologie/templates/topologie/index.html | 2 +- 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/machines/models.py b/machines/models.py index 0835f38e..fa1654fd 100644 --- a/machines/models.py +++ b/machines/models.py @@ -130,6 +130,7 @@ class Machine(RevMixin, FieldPermissionModelMixin, AclMixin, models.Model): :param user_request: Utilisateur qui fait la requête :param userid: id de l'user dont on va créer une machine :return: soit True, soit False avec la raison de l'échec""" + raise ValueError("Now you see me.") try: user = users.models.User.objects.get(pk=userid) except users.models.User.DoesNotExist: diff --git a/topologie/models.py b/topologie/models.py index 445d70e5..a184bde4 100644 --- a/topologie/models.py +++ b/topologie/models.py @@ -135,11 +135,26 @@ class AccessPoint(Machine): def __str__(self): return str(self.interface_set.first()) + # We want to retrieve the default behaviour given by AclMixin rather + # than the one overwritten by Machine. If you are not familiar with + # the behaviour of `super`, please check https://docs.python.org/3/library/functions.html#super + @classmethod - def get_instance(cls, object_id, *_args, **kwargs): - """Récupère une instance - :return: Une instance de la classe évidemment""" - return cls.objects.get(pk=object_id) + def get_instance(cls, *args, **kwargs): + return super(Machine, cls).get_instance(*args, **kwargs) + + @classmethod + def can_create(cls, *args, **kwargs): + return super(Machine, cls).can_create(*args, **kwargs) + + def can_edit(self, *args, **kwargs): + return super(Machine, self).can_edit(*args, **kwargs) + + def can_delete(self, *args, **kwargs): + return super(Machine, self).can_delete(*args, **kwargs) + + def can_view(self, *args, **kwargs): + return super(Machine, self).can_view(*args, **kwargs) class Server(Machine): @@ -179,11 +194,26 @@ class Server(Machine): def __str__(self): return str(self.interface_set.first()) + # We want to retrieve the default behaviour given by AclMixin rather + # than the one overwritten by Machine. If you are not familiar with + # the behaviour of `super`, please check https://docs.python.org/3/library/functions.html#super + @classmethod - def get_instance(cls, object_id, *_args, **kwargs): - """Récupère une instance - :return: Une instance de la classe évidemment""" - return cls.objects.get(pk=object_id) + def get_instance(cls, *args, **kwargs): + return super(Machine, cls).get_instance(*args, **kwargs) + + @classmethod + def can_create(cls, *args, **kwargs): + return super(Machine, cls).can_create(*args, **kwargs) + + def can_edit(self, *args, **kwargs): + return super(Machine, self).can_edit(*args, **kwargs) + + def can_delete(self, *args, **kwargs): + return super(Machine, self).can_delete(*args, **kwargs) + + def can_view(self, *args, **kwargs): + return super(Machine, self).can_view(*args, **kwargs) class Switch(Machine): @@ -469,11 +499,26 @@ class Switch(Machine): def __str__(self): return str(self.get_name) + # We want to retrieve the default behaviour given by AclMixin rather + # than the one overwritten by Machine. If you are not familiar with + # the behaviour of `super`, please check https://docs.python.org/3/library/functions.html#super + @classmethod - def get_instance(cls, object_id, *_args, **kwargs): - """Récupère une instance - :return: Une instance de la classe évidemment""" - return cls.objects.get(pk=object_id) + def get_instance(cls, *args, **kwargs): + return super(Machine, cls).get_instance(*args, **kwargs) + + @classmethod + def can_create(cls, *args, **kwargs): + return super(Machine, cls).can_create(*args, **kwargs) + + def can_edit(self, *args, **kwargs): + return super(Machine, self).can_edit(*args, **kwargs) + + def can_delete(self, *args, **kwargs): + return super(Machine, self).can_delete(*args, **kwargs) + + def can_view(self, *args, **kwargs): + return super(Machine, self).can_view(*args, **kwargs) class ModelSwitch(AclMixin, RevMixin, models.Model): diff --git a/topologie/templates/topologie/index.html b/topologie/templates/topologie/index.html index 4d021779..85424b25 100644 --- a/topologie/templates/topologie/index.html +++ b/topologie/templates/topologie/index.html @@ -56,7 +56,7 @@ function toggle_graph() { - +