From f80cb635d3fd7ba5dde313ba3461daba64ed6e02 Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Fri, 20 Mar 2020 00:06:28 +0100 Subject: [PATCH] Optimize permissions, full support add perms, more fixtures --- apps/member/backends.py | 53 +++--- apps/permission/fixtures/initial.json | 257 +++++++++++++++++++++++++- apps/permission/models.py | 60 ++---- apps/permission/signals.py | 31 +++- 4 files changed, 329 insertions(+), 72 deletions(-) diff --git a/apps/member/backends.py b/apps/member/backends.py index e68f6c19..099ef8a8 100644 --- a/apps/member/backends.py +++ b/apps/member/backends.py @@ -7,7 +7,8 @@ from django.db.models import Q, F from note.models import Note, NoteUser, NoteClub, NoteSpecial from note_kfet.middlewares import get_current_session -from .models import Membership, RolePermissions, Club +from permission.models import Permission +from .models import Membership, Club from django.contrib.auth.backends import ModelBackend @@ -17,28 +18,31 @@ class PermissionBackend(ModelBackend): supports_inactive_user = False @staticmethod - def permissions(user): + def permissions(user, model, type): for membership in Membership.objects.filter(user=user).all(): if not membership.valid() or membership.roles is None: continue - for role_permissions in RolePermissions.objects.filter(role=membership.roles).all(): - for permission in role_permissions.permissions.all(): - permission = permission.about( - user=user, - club=membership.club, - User=User, - Club=Club, - Membership=Membership, - Note=Note, - NoteUser=NoteUser, - NoteClub=NoteClub, - NoteSpecial=NoteSpecial, - F=F, - Q=Q - ) - if permission.mask.rank <= get_current_session().get("permission_mask", 0): - yield permission + for permission in Permission.objects.filter( + rolepermissions__role=membership.roles, + model__app_label=model.app_label, # For polymorphic models, we don't filter on model type + type=type + ).all(): + permission = permission.about( + user=user, + club=membership.club, + User=User, + Club=Club, + Membership=Membership, + Note=Note, + NoteUser=NoteUser, + NoteClub=NoteClub, + NoteSpecial=NoteSpecial, + F=F, + Q=Q + ) + if permission.mask.rank <= get_current_session().get("permission_mask", 0): + yield permission @staticmethod def filter_queryset(user, model, t, field=None): @@ -60,10 +64,10 @@ class PermissionBackend(ModelBackend): # Never satisfied query = Q(pk=-1) - for perm in PermissionBackend.permissions(user): + for perm in PermissionBackend.permissions(user, model, t): if perm.field and field != perm.field: continue - if perm.model != model or perm.type != t: + if perm.type != t or perm.model != model: continue query = query | perm.query return query @@ -78,7 +82,9 @@ class PermissionBackend(ModelBackend): perm = perm.split('.')[-1].split('_', 2) perm_type = perm[0] perm_field = perm[2] if len(perm) == 3 else None - if any(permission.applies(obj, perm_type, perm_field) for permission in self.permissions(user_obj)): + ct = ContentType.objects.get_for_model(obj) + if any(permission.applies(obj, perm_type, perm_field) + for permission in self.permissions(user_obj, ct, perm_type)): return True return False @@ -86,4 +92,5 @@ class PermissionBackend(ModelBackend): return False def get_all_permissions(self, user_obj, obj=None): - return list(self.permissions(user_obj)) + ct = ContentType.objects.get_for_model(obj) + return list(self.permissions(user_obj, ct, "view")) diff --git a/apps/permission/fixtures/initial.json b/apps/permission/fixtures/initial.json index d361abe1..500af1d0 100644 --- a/apps/permission/fixtures/initial.json +++ b/apps/permission/fixtures/initial.json @@ -239,6 +239,186 @@ "description": "Update a note balance with a transaction" } }, + { + "model": "permission.permission", + "pk": 19, + "fields": { + "model": 35, + "query": "[\"OR\", {\"pk\": [\"club\", \"note\", \"pk\"]}, {\"pk__in\": [\"NoteUser\", \"objects\", [\"filter\", {\"user__membership__club\": [\"club\"]}], [\"all\"]]}]", + "type": "view", + "mask": 2, + "field": "", + "description": "View notes of club members" + } + }, + { + "model": "permission.permission", + "pk": 20, + "fields": { + "model": 37, + "query": "[\"AND\", [\"OR\", {\"source\": [\"club\", \"note\"]}, {\"destination\": [\"club\", \"note\"]}], {\"amount__lte\": {\"F\": [\"ADD\", [\"F\", \"source__balance\"], 5000]}}]", + "type": "add", + "mask": 2, + "field": "", + "description": "Create transactions with a club" + } + }, + { + "model": "permission.permission", + "pk": 21, + "fields": { + "model": 44, + "query": "[\"AND\", {\"destination\": [\"club\", \"note\"]}, {\"amount__lte\": {\"F\": [\"ADD\", [\"F\", \"source__balance\"], 50]}}]", + "type": "add", + "mask": 2, + "field": "", + "description": "Create transactions from buttons with a club" + } + }, + { + "model": "permission.permission", + "pk": 22, + "fields": { + "model": 29, + "query": "{\"pk\": [\"club\", \"pk\"]}", + "type": "view", + "mask": 1, + "field": "", + "description": "View club infos" + } + }, + { + "model": "permission.permission", + "pk": 23, + "fields": { + "model": 37, + "query": "{}", + "type": "change", + "mask": 1, + "field": "valid", + "description": "Update validation status of a transaction" + } + }, + { + "model": "permission.permission", + "pk": 24, + "fields": { + "model": 37, + "query": "{}", + "type": "view", + "mask": 2, + "field": "", + "description": "View all transactions" + } + }, + { + "model": "permission.permission", + "pk": 25, + "fields": { + "model": 43, + "query": "{}", + "type": "view", + "mask": 2, + "field": "", + "description": "Display credit/debit interface" + } + }, + { + "model": "permission.permission", + "pk": 26, + "fields": { + "model": 43, + "query": "{}", + "type": "add", + "mask": 2, + "field": "", + "description": "Create credit/debit transaction" + } + }, + { + "model": "permission.permission", + "pk": 27, + "fields": { + "model": 36, + "query": "{}", + "type": "view", + "mask": 2, + "field": "", + "description": "View button categories" + } + }, + { + "model": "permission.permission", + "pk": 28, + "fields": { + "model": 36, + "query": "{}", + "type": "change", + "mask": 3, + "field": "", + "description": "Change button category" + } + }, + { + "model": "permission.permission", + "pk": 29, + "fields": { + "model": 36, + "query": "{}", + "type": "add", + "mask": 3, + "field": "", + "description": "Add button category" + } + }, + { + "model": "permission.permission", + "pk": 30, + "fields": { + "model": 38, + "query": "{}", + "type": "view", + "mask": 2, + "field": "", + "description": "View buttons" + } + }, + { + "model": "permission.permission", + "pk": 31, + "fields": { + "model": 38, + "query": "{}", + "type": "add", + "mask": 3, + "field": "", + "description": "Add buttons" + } + }, + { + "model": "permission.permission", + "pk": 32, + "fields": { + "model": 38, + "query": "{}", + "type": "change", + "mask": 3, + "field": "", + "description": "Update buttons" + } + }, + { + "model": "permission.permission", + "pk": 33, + "fields": { + "model": 37, + "query": "{}", + "type": "add", + "mask": 2, + "field": "", + "description": "Create any transaction" + } + }, { "model": "member.role", "pk": 1, @@ -253,6 +433,48 @@ "name": "Adh\u00e9rent Kfet" } }, + { + "model": "member.role", + "pk": 3, + "fields": { + "name": "Pr\u00e9sident\u00b7e BDE" + } + }, + { + "model": "member.role", + "pk": 4, + "fields": { + "name": "Tr\u00e9sorier\u00b7\u00e8re BDE" + } + }, + { + "model": "member.role", + "pk": 5, + "fields": { + "name": "Respo info" + } + }, + { + "model": "member.role", + "pk": 6, + "fields": { + "name": "GC Kfet" + } + }, + { + "model": "member.role", + "pk": 7, + "fields": { + "name": "Pr\u00e9sident\u00b7e de club" + } + }, + { + "model": "member.role", + "pk": 8, + "fields": { + "name": "Tr\u00e9sorier\u00b7\u00e8re de club" + } + }, { "model": "member.rolepermissions", "pk": 1, @@ -295,5 +517,38 @@ 18 ] } + }, + { + "model": "member.rolepermissions", + "pk": 3, + "fields": { + "role": 8, + "permissions": [ + 19, + 20, + 21, + 22 + ] + } + }, + { + "model": "member.rolepermissions", + "pk": 4, + "fields": { + "role": 4, + "permissions": [ + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33 + ] + } } -] +] \ No newline at end of file diff --git a/apps/permission/models.py b/apps/permission/models.py index 7934aa16..f7752517 100644 --- a/apps/permission/models.py +++ b/apps/permission/models.py @@ -8,7 +8,7 @@ import operator from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from django.db import models -from django.db.models import F, Q +from django.db.models import F, Q, Model from django.utils.translation import gettext_lazy as _ @@ -33,7 +33,14 @@ class InstancedPermission: if self.type == 'add': if permission_type == self.type: - return self.query(obj) + # Don't increase indexes + obj.pk = 0 + # Force insertion, no data verification, no trigger + Model.save(obj, force_insert=True) + ret = obj in self.model.model_class().objects.filter(self.query).all() + # Delete testing object + Model.delete(obj) + return ret if permission_type == self.type: if self.field and field_name != self.field: @@ -151,6 +158,10 @@ class Permission(models.Model): field = kwargs[value[0]] for i in range(1, len(value)): if isinstance(value[i], list): + if value[i][0] in kwargs: + field = Permission.compute_param(value[i], **kwargs) + continue + field = getattr(field, value[i][0]) params = [] call_kwargs = {} @@ -168,9 +179,6 @@ class Permission(models.Model): return field def _about(self, query, **kwargs): - if self.type == 'add': - # Handle add permission differently - return self._about_add(query, **kwargs) if len(query) == 0: # The query is either [] or {} and # applies to all objects of the model @@ -200,48 +208,6 @@ class Permission(models.Model): # TODO: find a better way to crash here raise Exception("query {} is wrong".format(self.query)) - def _about_add(self, _query, **kwargs): - query = _query - if len(query) == 0: - return lambda _: True - if isinstance(query, list): - if query[0] == 'AND': - return lambda obj: functools.reduce(operator.and_, [self._about_add(q, **kwargs)(obj) for q in query[1:]]) - elif query[0] == 'OR': - return lambda obj: functools.reduce(operator.or_, [self._about_add(q, **kwargs)(obj) for q in query[1:]]) - elif query[0] == 'NOT': - return lambda obj: not self._about_add(query[1], **kwargs)(obj) - elif isinstance(query, dict): - q_kwargs = {} - for key in query: - value = query[key] - if isinstance(value, list): - # It is a parameter we query its primary key - q_kwargs[key] = Permission.compute_param(value, **kwargs) - elif isinstance(value, dict): - # It is an F object - q_kwargs[key] = Permission.compute_f(query['F'], **kwargs) - else: - q_kwargs[key] = value - def func(obj): - nonlocal q_kwargs - for arg in q_kwargs: - spl = arg.split('__') - value = obj - last = None - for s in spl: - if not hasattr(obj, s): - last = s - break - value = getattr(obj, s) - if last == "lte": # TODO Add more filters - if value > q_kwargs[arg]: - return False - elif value != q_kwargs[arg]: - return False - return True - return func - def about(self, **kwargs): """ Return an InstancedPermission with the parameters diff --git a/apps/permission/signals.py b/apps/permission/signals.py index f18f35e0..a3b6b189 100644 --- a/apps/permission/signals.py +++ b/apps/permission/signals.py @@ -2,7 +2,9 @@ # SPDX-License-Identifier: GPL-3.0-or-later from django.core.exceptions import PermissionDenied +from django.db.models.signals import pre_save, pre_delete, post_save, post_delete +from logs import signals as logs_signals from member.backends import PermissionBackend from note_kfet.middlewares import get_current_authenticated_user @@ -39,20 +41,46 @@ def pre_save_object(sender, instance, **kwargs): model_name = model_name_full[1] if qs.exists(): + # We check if the user can change the model + + # If the user has all right on a model, then OK if PermissionBackend().has_perm(user, app_label + ".change_" + model_name, instance): return + # In the other case, we check if he/she has the right to change one field previous = qs.get() for field in instance._meta.fields: field_name = field.name old_value = getattr(previous, field.name) new_value = getattr(instance, field.name) + # If the field wasn't modified, no need to check the permissions if old_value == new_value: continue if not PermissionBackend().has_perm(user, app_label + ".change_" + model_name + "_" + field_name, instance): raise PermissionDenied else: - if not PermissionBackend().has_perm(user, app_label + ".add_" + model_name, instance): + # We check if the user can add the model + + # While checking permissions, the object will be inserted in the DB, then removed. + # We disable temporary the connectors + pre_save.disconnect(pre_save_object) + pre_delete.disconnect(pre_delete_object) + # We disable also logs connectors + pre_save.disconnect(logs_signals.pre_save_object) + post_save.disconnect(logs_signals.save_object) + post_delete.disconnect(logs_signals.delete_object) + + # We check if the user has right to add the object + has_perm = PermissionBackend().has_perm(user, app_label + ".add_" + model_name, instance) + + # Then we reconnect all + pre_save.connect(pre_save_object) + pre_delete.connect(pre_delete_object) + pre_save.connect(logs_signals.pre_save_object) + post_save.connect(logs_signals.save_object) + post_delete.connect(logs_signals.delete_object) + + if not has_perm: raise PermissionDenied @@ -73,5 +101,6 @@ def pre_delete_object(sender, instance, **kwargs): app_label = model_name_full[0] model_name = model_name_full[1] + # We check if the user has rights to delete the object if not PermissionBackend().has_perm(user, app_label + ".delete_" + model_name, instance): raise PermissionDenied