Merge branch 'fix_acl' into 'dev'

Fix acl

See merge request re2o/re2o!555

api/acl.py

@@ -56,7 +56,7 @@ def _create_api_permission():
 _create_api_permission()
 
 
-def can_view(user):
+def can_view(user, *args, **kwargs):
     """Check if an user can view the application.
 
     Args:

cotisations/acl.py

@@ -28,7 +28,7 @@ Here are defined some functions to check acl on the application.
 from django.utils.translation import ugettext as _
 
 
-def can_view(user):
+def can_view(user, *args, **kwargs):
     """Check if an user can view the application.
 
     Args:

logs/acl.py

@@ -28,7 +28,7 @@ Here are defined some functions to check acl on the application.
 from django.utils.translation import ugettext as _
 
 
-def can_view(user):
+def can_view(user, *args, **kwargs):
     """Check if an user can view the application.
 
     Args:
@@ -41,7 +41,6 @@ def can_view(user):
     can = user.has_module_perms("admin")
     return (
         can,
-        None if can else _("You don't have the right to view this"
+        None if can else _("You don't have the right to view this" " application."),
-                           " application."),
+        ("logs",),
-        "admin",
     )

logs/views.py

@@ -98,7 +98,13 @@ from re2o.utils import (
     all_active_interfaces_count,
 )
 from re2o.base import re2o_paginator, SortTable
-from re2o.acl import can_view_all, can_view_app, can_edit_history, can_view
+from re2o.acl import (
+    can_view_all,
+    can_view_app,
+    can_edit_history,
+    can_view,
+    acl_error_message,
+)
 
 from .models import (
     ActionsSearch,
@@ -109,6 +115,8 @@ from .models import (
 
 from .forms import ActionsSearchForm, MachineHistorySearchForm
 
+from .acl import can_view as can_view_logs
+
 
 @login_required
 @can_view_app("logs")
@@ -536,10 +544,11 @@ def get_history_object(request, model, object_name, object_id):
         instance = None
 
     if instance is None:
-        authorized = can_view_app("logs")
+        authorized, msg, permissions = can_view_logs(request.user)
-        msg = None
     else:
-        authorized, msg, _permissions = instance.can_view(request.user)
+        authorized, msg, permissions = instance.can_view(request.user)
+
+    msg = acl_error_message(msg, permissions)
 
     if not authorized:
         messages.error(
@@ -581,7 +590,7 @@ def history(request, application, object_name, object_id):
         raise Http404(_("No model found."))
 
     authorized, instance = get_history_object(request, model, object_name, object_id)
-    if not can_view:
+    if not authorized:
         return instance
 
     history = get_history_class(model)

machines/acl.py

@@ -28,7 +28,7 @@ Here are defined some functions to check acl on the application.
 from django.utils.translation import ugettext as _
 
 
-def can_view(user):
+def can_view(user, *args, **kwargs):
     """Check if an user can view the application.
 
     Args:
@@ -41,7 +41,6 @@ def can_view(user):
     can = user.has_module_perms("machines")
     return (
         can,
-        None if can else _("You don't have the right to view this"
+        None if can else _("You don't have the right to view this" " application."),
-                           " application."),
         ("machines",),
     )

preferences/acl.py

@@ -28,7 +28,7 @@ Here are defined some functions to check acl on the application.
 from django.utils.translation import ugettext as _
 
 
-def can_view(user):
+def can_view(user, *args, **kwargs):
     """Check if an user can view the application.
 
     Args:
@@ -41,7 +41,6 @@ def can_view(user):
     can = user.has_module_perms("preferences")
     return (
         can,
-        None if can else _("You don't have the right to view this"
+        None if can else _("You don't have the right to view this" " application."),
-                           " application."),
         ("preferences",),
     )

re2o/acl.py

@@ -57,6 +57,9 @@ def acl_error_message(msg, permissions):
         )
 
 
+# This is the function of main interest of this file. Almost all the decorators
+# use it, and it is a fairly complicated piece of code. Let me guide you through
+# this ! 🌈😸
 def acl_base_decorator(method_name, *targets, on_instance=True):
     """Base decorator for acl. It checks if the `request.user` has the
     permission by calling model.method_name. If the flag on_instance is True,
@@ -128,22 +131,43 @@ ModelC)
         where `*args` and `**kwargs` are the original view arguments.
     """
 
+    # First we define a utilitary functions. This is what parses the input of
+    #  the decorator. It will group a target (i.e. a model class) with a list
+    # of associated fields (possibly empty).
+
     def group_targets():
         """This generator parses the targets of the decorator, yielding
         2-tuples of (model, [fields]).
         """
         current_target = None
         current_fields = []
+        # We iterate over all the possible target passed in argument of the
+        # decorator. Let's call the `target` variable a target candidate.
+        # We basically want to discriminate the valid targets over the field
+        # names.
         for target in targets:
+            # We enter this conditional block if the current target is not
+            # a string, i.e. if it is not a field name, i.e. it is a model
+            # name.
             if not isinstance(target, str):
+                # if the current target is defined, this means we already
+                # encountered a valid target and we have been storing field
+                # names ever since. This group is ready and we can `yield` it.
                 if current_target:
                     yield (current_target, current_fields)
+                # Then we define the current target and reset its fields.
                 current_target = target
                 current_fields = []
             else:
+                # When we encounter a string, this is not valid target and is
+                # thus a field name. We store it for later.
                 current_fields.append(target)
+        # We need to yield the last pair of target and fields.
         yield (current_target, current_fields)
 
+    # Now to the main topic ! if you are not sure why we need to use a function
+    # `wrapper` inside the `decorator` function, you need to read some
+    #  documentation on decorators !
     def decorator(view):
         """The decorator to use on a specific view
         """
@@ -158,43 +182,74 @@ ModelC)
                 stores the instances of models in order to avoid duplicate DB
                 calls for the view.
                 """
+                # When working on instances, retrieve the associated instance
+                # and store it to pass it to the view.
                 if on_instance:
                     try:
                         target = target.get_instance(target_id, *args, **kwargs)
                         instances.append(target)
                     except target.DoesNotExist:
+                        # A non existing instance is a valid reason to deny
+                        # access to the view.
                         yield False, _("Nonexistent entry."), []
                         return
+                # Now we can actually make the ACL test, using the right ACL
+                # method.
                 if hasattr(target, method_name):
                     can_fct = getattr(target, method_name)
                     yield can_fct(request.user, *args, **kwargs)
+
+                # If working on fields, iterate through the concerned ones
+                # and check that the user can change this field. (this is
+                # the only available ACL for fields)
                 for field in fields:
                     can_change_fct = getattr(target, "can_change_" + field)
                     yield can_change_fct(request.user, *args, **kwargs)
 
+            # Now to the main loop. We are going iterate through the targets
+            # pairs (remember the `group_targets` function) and the keyword
+            # arguments of the view to retrieve the associated model instances
+            # and check that the user making the request is authorized to do it
+            # as well as storing the the associated error and warning messages.
             error_messages = []
             warning_messages = []
-            for arg_key, (target, fields) in zip(kwargs.keys(), group_targets()):
+
+            if on_instance:
+                iterator = zip(kwargs.keys(), group_targets())
+            else:
+                iterator = group_targets()
+
+            for it in iterator:
+                # If the decorator must work on instances, retrieve the
+                # associated instance.
                 if on_instance:
+                    arg_key, (target, fields) = it
                     target_id = int(kwargs[arg_key])
                 else:
+                    target, fields = it
                     target_id = None
+
+                # Store the messages at the right place.
                 for can, msg, permissions in process_target(target, fields, target_id):
                     if not can:
                         error_messages.append(acl_error_message(msg, permissions))
                     elif msg:
                         warning_messages.append(acl_error_message(msg, permissions))
 
+            # Display the warning messages
             if warning_messages:
                 for msg in warning_messages:
                     messages.warning(request, msg)
 
+            # If there is any error message, then the request must be denied.
             if error_messages:
+                # We display the message
                 for msg in error_messages:
                     messages.error(
                         request,
                         msg or _("You don't have the right to access this menu."),
                     )
+                # And redirect the user to the right place.
                 if request.user.id is not None:
                     return redirect(
                         reverse("users:profil", kwargs={"userid": str(request.user.id)})

re2o/templatetags/massive_bootstrap_form.py

@@ -31,6 +31,7 @@ from django import template
 from django.utils.safestring import mark_safe
 from django.forms import TextInput
 from django.forms.widgets import Select
+from django.utils.translation import ugettext_lazy as _
 from bootstrap3.utils import render_tag
 from bootstrap3.forms import render_field
 
@@ -244,6 +245,7 @@ class MBFForm:
                     self.html += mbf_field.render()
 
                 else:
+                    f = field.get_bound_field(self.form, name), self.args, self.kwargs
                     self.html += render_field(
                         field.get_bound_field(self.form, name),
                         *self.args,
@@ -406,7 +408,10 @@ class MBFField:
             self.html += render_field(self.bound, *self.args, **self.kwargs)
 
         self.field.widget = TextInput(
-            attrs={"name": "mbf_" + self.name, "placeholder": self.field.empty_label}
+            attrs={
+                "name": "mbf_" + self.name,
+                "placeholder": getattr(self.field, "empty_label", _("Nothing")),
+            }
         )
         self.replace_input = render_field(self.bound, *self.args, **self.kwargs)
 

re2o/utils.py

@@ -38,7 +38,7 @@ from __future__ import unicode_literals
 
 from django.utils import timezone
 from django.db.models import Q
-from django.contrib.auth.models import Permission
+from django.contrib.auth.models import Permission, Group
 
 from cotisations.models import Cotisation, Facture, Vente
 from machines.models import Interface, Machine
@@ -58,11 +58,20 @@ def get_group_having_permission(*permission_name):
     """
     groups = set()
     for name in permission_name:
-        app_label, codename = name.split(".")
+        if "." in name:
-        permission = Permission.objects.get(
+            app_label, codename = name.split(".")
-            content_type__app_label=app_label, codename=codename
+            permission = Permission.objects.get(
-        )
+                content_type__app_label=app_label, codename=codename
-        groups = groups.union(permission.group_set.all())
+            )
+            groups = groups.union(permission.group_set.all())
+        else:
+            groups = groups.union(
+                Group.objects.filter(
+                    permissions__in=Permission.objects.filter(
+                        content_type__app_label="users"
+                    )
+                ).distinct()
+            )
     return groups
 
 

search/acl.py

@@ -27,7 +27,7 @@ Here are defined some functions to check acl on the application.
 """
 
 
-def can_view(_user):
+def can_view(*args, **kwargs):
     """Check if an user can view the application.
 
     Args:

topologie/acl.py

@@ -28,7 +28,7 @@ Here are defined some functions to check acl on the application.
 from django.utils.translation import ugettext as _
 
 
-def can_view(user):
+def can_view(user, *args, **kwargs):
     """Check if an user can view the application.
 
     Args:

users/acl.py

@@ -28,7 +28,7 @@ Here are defined some functions to check acl on the application.
 from django.utils.translation import ugettext as _
 
 
-def can_view(user):
+def can_view(user, *args, **kwargs):
     """Check if an user can view the application.
 
     Args: