From 67d1d9f7b72f11a58e947909ccc198284afe2c43 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Wed, 18 Sep 2019 14:26:42 +0200 Subject: [PATCH 01/34] Added permission app --- apps/member/backends.py | 33 +++++++++++ apps/member/models.py | 22 +++++++ apps/permission/__init__.py | 0 apps/permission/admin.py | 3 + apps/permission/apps.py | 5 ++ apps/permission/models.py | 112 ++++++++++++++++++++++++++++++++++++ apps/permission/tests.py | 3 + apps/permission/views.py | 3 + note_kfet/settings.py | 1 + 9 files changed, 182 insertions(+) create mode 100644 apps/member/backends.py create mode 100644 apps/permission/__init__.py create mode 100644 apps/permission/admin.py create mode 100644 apps/permission/apps.py create mode 100644 apps/permission/models.py create mode 100644 apps/permission/tests.py create mode 100644 apps/permission/views.py diff --git a/apps/member/backends.py b/apps/member/backends.py new file mode 100644 index 00000000..0b2edad8 --- /dev/null +++ b/apps/member/backends.py @@ -0,0 +1,33 @@ +from django.contribs.contenttype.models import ContentType +from member.models import Club, Membership, RolePermissions + + +class PermissionBackend(object): + supports_object_permissions = True + supports_anonymous_user = False + supports_inactive_user = False + + def authenticate(self, username, password): + return None + + def permissions(self, user, obj): + for membership in user.memberships.all(): + if not membership.valid() or membership.role is None: + continue + for permission in RolePermissions.objects.get(role=membership.role).permissions.objects.all(): + permission = permission.about(user=user, club=membership.club) + yield permission + + def has_perm(self, user_obj, perm, obj=None): + if obj is None: + return False + perm = perm.split('_') + perm_type = perm[1] + perm_field = perm[2] if len(perm) == 3 else None + return any(permission.applies(obj, perm_type, perm_field) for obj in self.permissions(user_obj, obj)) + + def get_all_permissions(self, user_obj, obj=None): + if obj is None: + return [] + else: + return list(self.permissions(user_obj, obj)) diff --git a/apps/member/models.py b/apps/member/models.py index 70f8ccf7..7eacdc60 100644 --- a/apps/member/models.py +++ b/apps/member/models.py @@ -2,6 +2,8 @@ # Copyright (C) 2018-2019 by BDE ENS Paris-Saclay # SPDX-License-Identifier: GPL-3.0-or-later +import datetime + from django.conf import settings from django.db import models from django.db.models.signals import post_save @@ -9,6 +11,7 @@ from django.dispatch import receiver from django.utils.translation import gettext_lazy as _ from django.urls import reverse + class Profile(models.Model): """ An user profile @@ -51,6 +54,7 @@ class Profile(models.Model): def get_absolute_url(self): return reverse('user_detail',args=(self.pk,)) + class Club(models.Model): """ A student club @@ -141,11 +145,29 @@ class Membership(models.Model): verbose_name=_('fee'), ) + def valid(self): + return self.date_start <= datetime.datetime.now() < self.date_end + class Meta: verbose_name = _('membership') verbose_name_plural = _('memberships') +class RolePermissions(models.Model): + """ + Permissions associated with a Role + """ + role = models.ForeignKey( + Role, + on_delete=models.PROTECT, + related_name='+', + verbose_name=_('role'), + ) + permissions = models.ManyToManyField( + 'permission.Permission' + ) + + # @receiver(post_save, sender=settings.AUTH_USER_MODEL) # def save_user_profile(instance, created, **_kwargs): # """ diff --git a/apps/permission/__init__.py b/apps/permission/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/apps/permission/admin.py b/apps/permission/admin.py new file mode 100644 index 00000000..8c38f3f3 --- /dev/null +++ b/apps/permission/admin.py @@ -0,0 +1,3 @@ +from django.contrib import admin + +# Register your models here. diff --git a/apps/permission/apps.py b/apps/permission/apps.py new file mode 100644 index 00000000..0f46ef08 --- /dev/null +++ b/apps/permission/apps.py @@ -0,0 +1,5 @@ +from django.apps import AppConfig + + +class PermissionConfig(AppConfig): + name = 'permission' diff --git a/apps/permission/models.py b/apps/permission/models.py new file mode 100644 index 00000000..b7cc8845 --- /dev/null +++ b/apps/permission/models.py @@ -0,0 +1,112 @@ +import json + +from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ValidationError +from django.db import models +from django.db.models import Q +from django.utils.translation import gettext_lazy as _ + + +class InstancedPermission: + + def __init__(self, model, permission, type, field): + self.model = model + self.permission = permission + self.type = type + self.field = field + + def applies(self, obj, permission_type, field_name=None): + if ContentType.objects.get_for_model(obj) != self.model: + # The permission does not apply to the object + return False + if self.permission is None: + if permission_type == self.type: + if field_name is not None: + return field_name == self.field + else: + return True + else: + return False + elif isinstance(self.permission, dict): + for field in self.permission: + value = getattr(obj, field) + if isinstance(value, models.Model): + value = value.pk + if value != self.permission[field]: + return False + elif isinstance(self.permission, type(obj.pk)): + if obj.pk != self.permission: + return False + if permission_type == self.type: + if field_name: + return field_name == self.field + else: + return True + return False + + def __repr__(self): + if self.field: + return _("Can {type} {model}.{field} in {permission}").format(type=self.type, model=self.model, field=self.field, permission=self.permission) + else: + return _("Can {type} {model} in {permission}").format(type=self.type, model=self.model, permission=self.permission) + + +class Permission(models.Model): + + PERMISSION_TYPES = [ + ('C', 'add'), + ('R', 'view'), + ('U', 'change'), + ('D', 'delete') + ] + + model = models.ForeignKey(ContentType, on_delete=models.CASCADE, related_name='+') + + permission = models.TextField() + + type = models.CharField(max_length=15, choices=PERMISSION_TYPES) + + field = models.CharField(max_length=255, blank=True) + + class Meta: + unique_together = ('model', 'permission', 'type', 'field') + + def clean(self): + if self.field and self.type not in {'R', 'U'}: + raise ValidationError(_("Specifying field applies only to view and change permission types.")) + + def save(self): + self.full_clean() + super().save() + + def _about(_self, _permission, **kwargs): + if _permission[0] == 'all': + return None + elif _permission[0] == 'pk': + if _permission[1] in kwargs: + return kwargs[_permission[1]].pk + else: + return None + elif _permission[0] == 'filter': + return {field: _self._about(_permission[1][field], **kwargs) for field in _permission[1]} + else: + return _permission + + def about(self, **kwargs): + permission = json.loads(self.permission) + permission = self._about(permission, **kwargs) + return InstancedPermission(self.model, permission, self.type, self.field) + + def __str__(self): + if self.field: + return _("Can {type} {model}.{field} in {permission}").format(type=self.type, model=self.model, field=self.field, permission=self.permission) + else: + return _("Can {type} {model} in {permission}").format(type=self.type, model=self.model, permission=self.permission) + + +class UserPermission(models.Model): + + user = models.ForeignKey('auth.User', on_delete=models.CASCADE) + + permission = models.ForeignKey(Permission, on_delete=models.CASCADE) + diff --git a/apps/permission/tests.py b/apps/permission/tests.py new file mode 100644 index 00000000..7ce503c2 --- /dev/null +++ b/apps/permission/tests.py @@ -0,0 +1,3 @@ +from django.test import TestCase + +# Create your tests here. diff --git a/apps/permission/views.py b/apps/permission/views.py new file mode 100644 index 00000000..91ea44a2 --- /dev/null +++ b/apps/permission/views.py @@ -0,0 +1,3 @@ +from django.shortcuts import render + +# Create your views here. diff --git a/note_kfet/settings.py b/note_kfet/settings.py index cfe09f7b..3cd3b717 100644 --- a/note_kfet/settings.py +++ b/note_kfet/settings.py @@ -56,6 +56,7 @@ INSTALLED_APPS = [ 'activity', 'member', 'note', + 'permission' ] MIDDLEWARE = [ From 2a4ab0975353a3c9bb4ba82149c6768cff3c06c1 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Wed, 18 Sep 2019 16:39:37 +0200 Subject: [PATCH 02/34] [permission] Use full names for permission types --- apps/permission/models.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/permission/models.py b/apps/permission/models.py index b7cc8845..73000cbd 100644 --- a/apps/permission/models.py +++ b/apps/permission/models.py @@ -54,10 +54,10 @@ class InstancedPermission: class Permission(models.Model): PERMISSION_TYPES = [ - ('C', 'add'), - ('R', 'view'), - ('U', 'change'), - ('D', 'delete') + ('add', 'add'), + ('view', 'view'), + ('change', 'change'), + ('delete', 'delete') ] model = models.ForeignKey(ContentType, on_delete=models.CASCADE, related_name='+') @@ -72,7 +72,7 @@ class Permission(models.Model): unique_together = ('model', 'permission', 'type', 'field') def clean(self): - if self.field and self.type not in {'R', 'U'}: + if self.field and self.type not in {'view', 'change'}: raise ValidationError(_("Specifying field applies only to view and change permission types.")) def save(self): From d826dc9d2076fcc032eafaf9cbe80f38a5c8f2f1 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Wed, 18 Sep 2019 16:40:21 +0200 Subject: [PATCH 03/34] [permission] Permission admin view --- apps/permission/admin.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/permission/admin.py b/apps/permission/admin.py index 8c38f3f3..4594468d 100644 --- a/apps/permission/admin.py +++ b/apps/permission/admin.py @@ -1,3 +1,11 @@ from django.contrib import admin -# Register your models here. +from .models import Permission + + +@admin.register(Permission) +class PermissionAdmin(admin.ModelAdmin): + """ + Admin customisation for Permission + """ + list_display = ('type', 'model', 'field', 'permission') From 1ac63cbed1981005e4bf2b6a0518177b41b81598 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Wed, 18 Sep 2019 16:41:01 +0200 Subject: [PATCH 04/34] [member] Handle unlimited memberships --- apps/member/models.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/member/models.py b/apps/member/models.py index 2e84dc75..d1c3dea2 100644 --- a/apps/member/models.py +++ b/apps/member/models.py @@ -146,7 +146,10 @@ class Membership(models.Model): ) def valid(self): - return self.date_start <= datetime.datetime.now() < self.date_end + if self.date_end is not None: + return self.date_start <= datetime.datetime.now() < self.date_end + else: + return self.date_start <= datetime.datetime.now() class Meta: verbose_name = _('membership') From 3766a1905df1abff26affb74cae5ecd41e194446 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Wed, 18 Sep 2019 16:44:04 +0200 Subject: [PATCH 05/34] [permission] track migrations directory --- apps/permission/migrations/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 apps/permission/migrations/__init__.py diff --git a/apps/permission/migrations/__init__.py b/apps/permission/migrations/__init__.py new file mode 100644 index 00000000..e69de29b From 94c3a994470efc9f8d58b428a2ccda5caca0b5b9 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Sun, 9 Feb 2020 17:35:15 +0100 Subject: [PATCH 06/34] [permission] Rewrite with comments --- apps/permission/models.py | 87 +++++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 32 deletions(-) diff --git a/apps/permission/models.py b/apps/permission/models.py index 73000cbd..b75496ab 100644 --- a/apps/permission/models.py +++ b/apps/permission/models.py @@ -1,4 +1,6 @@ +import functools import json +import operator from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError @@ -9,15 +11,19 @@ from django.utils.translation import gettext_lazy as _ class InstancedPermission: - def __init__(self, model, permission, type, field): + def __init__(self, model, query, type, field): self.model = model - self.permission = permission + self.query = query self.type = type self.field = field def applies(self, obj, permission_type, field_name=None): + """ + Returns True if the permission applies to + the field `field_name` object `obj` + """ if ContentType.objects.get_for_model(obj) != self.model: - # The permission does not apply to the object + # The permission does not apply to the model return False if self.permission is None: if permission_type == self.type: @@ -27,22 +33,10 @@ class InstancedPermission: return True else: return False - elif isinstance(self.permission, dict): - for field in self.permission: - value = getattr(obj, field) - if isinstance(value, models.Model): - value = value.pk - if value != self.permission[field]: - return False - elif isinstance(self.permission, type(obj.pk)): - if obj.pk != self.permission: - return False - if permission_type == self.type: - if field_name: - return field_name == self.field - else: - return True - return False + elif obj in self.model.objects.get(self.query): + return True + else: + return False def __repr__(self): if self.field: @@ -62,11 +56,24 @@ class Permission(models.Model): model = models.ForeignKey(ContentType, on_delete=models.CASCADE, related_name='+') + # A json encoded Q object with the following grammar + # permission -> [] | {} (the empty permission representing all objects) + # permission -> ['AND', permission, …] + # -> ['OR', permission, …] + # -> ['NOT', permission] + # permission -> {key: value, …} + # key -> string + # value -> int | string | bool | null + # -> [parameter] + # + # Examples: + # Q(is_admin=True) := {'is_admin': ['TYPE', 'bool', 'True']} + # ~Q(is_admin=True) := ['NOT', {'is_admin': ['TYPE', 'bool', 'True']}] permission = models.TextField() - type = models.CharField(max_length=15, choices=PERMISSION_TYPES) + type = models.CharField(max_length=16, choices=PERMISSION_TYPES) - field = models.CharField(max_length=255, blank=True) + field = models.CharField(max_length=256, blank=True) class Meta: unique_together = ('model', 'permission', 'type', 'field') @@ -80,22 +87,38 @@ class Permission(models.Model): super().save() def _about(_self, _permission, **kwargs): - if _permission[0] == 'all': + self = _self + permission = _permission + if len(permission) == 0: + # The permission is either [] or {} and + # applies to all objects of the model + # to represent this we return None return None - elif _permission[0] == 'pk': - if _permission[1] in kwargs: - return kwargs[_permission[1]].pk - else: - return None - elif _permission[0] == 'filter': - return {field: _self._about(_permission[1][field], **kwargs) for field in _permission[1]} + if isinstance(permission, list): + if permission[0] == 'AND': + return functools.reduce(operator.and_, [self._about(permission, **kwargs) for permission in permission[1:]]) + elif permission[0] == 'OR': + return functools.reduce(operator.or_, [self._about(permission, **kwargs) for permission in permission[1:]]) + elif permission[0] == 'NOT': + return ~self._about(permission[1], **kwargs) + elif isinstance(permission, dict): + q_kwargs = {} + for key in permission: + value = permission[key] + if isinstance(value, list): + # It is a parameter we query its primary key + q_kwargs[key] = kwargs[value[0]].pk + else: + q_kwargs[key] = value + return Q(**q_kwargs) else: - return _permission + # TODO: find a better way to crash here + raise Exception("Permission {} is wrong".format(self.permission)) def about(self, **kwargs): permission = json.loads(self.permission) - permission = self._about(permission, **kwargs) - return InstancedPermission(self.model, permission, self.type, self.field) + query = self._about(permission, **kwargs) + return InstancedPermission(self.model, query, self.type, self.field) def __str__(self): if self.field: From 72955ae2d6e58901775a43707716e2141dd23eb7 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Sun, 9 Feb 2020 18:14:36 +0100 Subject: [PATCH 07/34] [permission] Renamed Permission.permission and added description field --- apps/permission/models.py | 68 +++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/apps/permission/models.py b/apps/permission/models.py index b75496ab..5c016806 100644 --- a/apps/permission/models.py +++ b/apps/permission/models.py @@ -57,26 +57,28 @@ class Permission(models.Model): model = models.ForeignKey(ContentType, on_delete=models.CASCADE, related_name='+') # A json encoded Q object with the following grammar - # permission -> [] | {} (the empty permission representing all objects) - # permission -> ['AND', permission, …] - # -> ['OR', permission, …] - # -> ['NOT', permission] - # permission -> {key: value, …} - # key -> string - # value -> int | string | bool | null - # -> [parameter] + # query -> [] | {} (the empty query representing all objects) + # query -> ['AND', query, …] + # -> ['OR', query, …] + # -> ['NOT', query] + # query -> {key: value, …} + # key -> string + # value -> int | string | bool | null + # -> [parameter] # # Examples: # Q(is_admin=True) := {'is_admin': ['TYPE', 'bool', 'True']} # ~Q(is_admin=True) := ['NOT', {'is_admin': ['TYPE', 'bool', 'True']}] - permission = models.TextField() + query = models.TextField() - type = models.CharField(max_length=16, choices=PERMISSION_TYPES) + type = models.CharField(max_length=15, choices=PERMISSION_TYPES) - field = models.CharField(max_length=256, blank=True) + field = models.CharField(max_length=255, blank=True) + + description = models.CharField(max_length=255, blank=True) class Meta: - unique_together = ('model', 'permission', 'type', 'field') + unique_together = ('model', 'query', 'type', 'field') def clean(self): if self.field and self.type not in {'view', 'change'}: @@ -86,25 +88,25 @@ class Permission(models.Model): self.full_clean() super().save() - def _about(_self, _permission, **kwargs): + def _about(_self, _query, **kwargs): self = _self - permission = _permission - if len(permission) == 0: - # The permission is either [] or {} and + query = _query + if len(query) == 0: + # The query is either [] or {} and # applies to all objects of the model # to represent this we return None return None - if isinstance(permission, list): - if permission[0] == 'AND': - return functools.reduce(operator.and_, [self._about(permission, **kwargs) for permission in permission[1:]]) - elif permission[0] == 'OR': - return functools.reduce(operator.or_, [self._about(permission, **kwargs) for permission in permission[1:]]) - elif permission[0] == 'NOT': - return ~self._about(permission[1], **kwargs) - elif isinstance(permission, dict): + if isinstance(query, list): + if query[0] == 'AND': + return functools.reduce(operator.and_, [self._about(query, **kwargs) for query in query[1:]]) + elif query[0] == 'OR': + return functools.reduce(operator.or_, [self._about(query, **kwargs) for query in query[1:]]) + elif query[0] == 'NOT': + return ~self._about(query[1], **kwargs) + elif isinstance(query, dict): q_kwargs = {} - for key in permission: - value = permission[key] + for key in query: + value = query[key] if isinstance(value, list): # It is a parameter we query its primary key q_kwargs[key] = kwargs[value[0]].pk @@ -113,18 +115,22 @@ class Permission(models.Model): return Q(**q_kwargs) else: # TODO: find a better way to crash here - raise Exception("Permission {} is wrong".format(self.permission)) + raise Exception("query {} is wrong".format(self.query)) def about(self, **kwargs): - permission = json.loads(self.permission) - query = self._about(permission, **kwargs) + """ + Return an InstancedPermission with the parameters + replaced by their values and the query interpreted + """ + query = json.loads(self.query) + query = self._about(query, **kwargs) return InstancedPermission(self.model, query, self.type, self.field) def __str__(self): if self.field: - return _("Can {type} {model}.{field} in {permission}").format(type=self.type, model=self.model, field=self.field, permission=self.permission) + return _("Can {type} {model}.{field} in {query}").format(type=self.type, model=self.model, field=self.field, query=self.query) else: - return _("Can {type} {model} in {permission}").format(type=self.type, model=self.model, permission=self.permission) + return _("Can {type} {model} in {query}").format(type=self.type, model=self.model, query=self.query) class UserPermission(models.Model): From 2b49effebbf3e09bc0cfb2bd64a7a8f56cebc309 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Sun, 9 Feb 2020 18:30:37 +0100 Subject: [PATCH 08/34] [permission] Update admin --- apps/permission/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/permission/admin.py b/apps/permission/admin.py index 4594468d..e93de0c5 100644 --- a/apps/permission/admin.py +++ b/apps/permission/admin.py @@ -8,4 +8,4 @@ class PermissionAdmin(admin.ModelAdmin): """ Admin customisation for Permission """ - list_display = ('type', 'model', 'field', 'permission') + list_display = ('type', 'model', 'field', 'description') From 982a5ae0099eca14d58dcb2cb0f9d50d88c10934 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Thu, 13 Feb 2020 15:59:19 +0100 Subject: [PATCH 09/34] [permission] Add F object support --- apps/permission/models.py | 46 ++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/apps/permission/models.py b/apps/permission/models.py index 5c016806..2ca17e4c 100644 --- a/apps/permission/models.py +++ b/apps/permission/models.py @@ -5,7 +5,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 Q +from django.db.models import F, Q from django.utils.translation import gettext_lazy as _ @@ -58,13 +58,20 @@ class Permission(models.Model): # A json encoded Q object with the following grammar # query -> [] | {} (the empty query representing all objects) - # query -> ['AND', query, …] - # -> ['OR', query, …] - # -> ['NOT', query] - # query -> {key: value, …} - # key -> string - # value -> int | string | bool | null - # -> [parameter] + # query -> ['AND', query, …] AND multiple queries + # | ['OR', query, …] OR multiple queries + # | ['NOT', query] Opposite of query + # query -> {key: value, …} A list of fields and values of a Q object + # key -> string A field name + # value -> int | string | bool | null Literal values + # | [parameter] A parameter + # | {'F': oper} An F object + # oper -> [string] A parameter + # | ['ADD', oper, …] Sum multiple F objects or literal + # | ['SUB', oper, oper] Substract two F objects or literal + # | ['MUL', oper, …] Multiply F objects or literals + # | int | string | bool | null Literal values + # | ['F', string] A field # # Examples: # Q(is_admin=True) := {'is_admin': ['TYPE', 'bool', 'True']} @@ -88,6 +95,26 @@ class Permission(models.Model): self.full_clean() super().save() + @staticmethod + def compute_f(_oper, **kwargs): + oper = _oper + if isinstance(oper, list): + if len(oper) == 1: + return kwargs[oper[0]].pk + elif len(oper) >= 2: + if oper[0] == 'ADD': + return functools.reduce(operator.add, [compute_f(oper, **kwargs) for oper in oper[1:]]) + elif oper[0] == 'SUB': + return compute_f(oper[1], **kwargs) - compute_f(oper[2], **kwargs) + elif oper[0] == 'MUL': + return functools.reduce(operator.mul, [compute_f(oper, **kwargs) for oper in oper[1:]]) + elif oper[0] == 'F': + return F(oper[1]) + else: + return oper + # TODO: find a better way to crash here + raise Exception("F is wrong") + def _about(_self, _query, **kwargs): self = _self query = _query @@ -110,6 +137,9 @@ class Permission(models.Model): if isinstance(value, list): # It is a parameter we query its primary key q_kwargs[key] = kwargs[value[0]].pk + elif isinstance(value, dict): + # It is an F object + q_kwargs[key] = compute_f(query['F'], **kwargs) else: q_kwargs[key] = value return Q(**q_kwargs) From 8a9ad0a6e50ef24c3cc2c1519c2be0f0d955e19c Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Sat, 7 Mar 2020 09:30:22 +0100 Subject: [PATCH 10/34] [permission] Handle add rights --- apps/permission/models.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/apps/permission/models.py b/apps/permission/models.py index 2ca17e4c..2fcb23cb 100644 --- a/apps/permission/models.py +++ b/apps/permission/models.py @@ -22,6 +22,9 @@ class InstancedPermission: Returns True if the permission applies to the field `field_name` object `obj` """ + if self.type == 'add': + if permission_type == self.type: + return self.query(obj) if ContentType.objects.get_for_model(obj) != self.model: # The permission does not apply to the model return False @@ -118,6 +121,9 @@ class Permission(models.Model): def _about(_self, _query, **kwargs): self = _self query = _query + 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 @@ -147,6 +153,38 @@ 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): + self = _self + 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(query, **kwargs)(obj) for query in query[1:]]) + elif query[0] == 'OR': + return lambda obj: functools.reduce(operator.or_, [self._about_add(query, **kwargs)(obj) for query 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] = kwargs[value[0]].pk + elif isinstance(value, dict): + # It is an F object + q_kwargs[key] = compute_f(query['F'], **kwargs) + else: + q_kwargs[key] = value + def func(obj): + nonlocal q_kwargs + for arg in q_kwargs: + if getattr(obj, arg) != q_kwargs(arg): + return False + return True + return func + def about(self, **kwargs): """ Return an InstancedPermission with the parameters From 5df1f42f435a7ff0c4e895d06c7de6e45a3baeb0 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Sat, 7 Mar 2020 10:48:38 +0100 Subject: [PATCH 11/34] [permission] Syntax error --- apps/permission/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/permission/models.py b/apps/permission/models.py index 2fcb23cb..000fe69f 100644 --- a/apps/permission/models.py +++ b/apps/permission/models.py @@ -121,7 +121,7 @@ class Permission(models.Model): def _about(_self, _query, **kwargs): self = _self query = _query - if self.type == 'add'): + if self.type == 'add': # Handle add permission differently return self._about_add(query, **kwargs) if len(query) == 0: From 9d61e217e9994eb3d60d0c53c65af0f294601f84 Mon Sep 17 00:00:00 2001 From: Benjamin Graillot Date: Sat, 7 Mar 2020 11:21:19 +0100 Subject: [PATCH 12/34] [permission] Only split permission up to 3 --- apps/member/backends.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/member/backends.py b/apps/member/backends.py index 0b2edad8..9ef9706f 100644 --- a/apps/member/backends.py +++ b/apps/member/backends.py @@ -21,7 +21,7 @@ class PermissionBackend(object): def has_perm(self, user_obj, perm, obj=None): if obj is None: return False - perm = perm.split('_') + perm = perm.split('_', 3) perm_type = perm[1] perm_field = perm[2] if len(perm) == 3 else None return any(permission.applies(obj, perm_type, perm_field) for obj in self.permissions(user_obj, obj)) From 30ce17b644c238183f5e0904c1df621dc209d323 Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Sat, 7 Mar 2020 13:12:17 +0100 Subject: [PATCH 13/34] Update a lot of things --- apps/logs/signals.py | 4 +++ apps/member/admin.py | 3 +- apps/member/backends.py | 36 +++++++++++--------- apps/member/models.py | 4 +-- apps/permission/admin.py | 3 ++ apps/permission/apps.py | 3 ++ apps/permission/models.py | 70 ++++++++++++++++++-------------------- apps/permission/tests.py | 3 ++ apps/permission/views.py | 3 ++ note_kfet/settings/base.py | 7 ++-- requirements.txt | 1 - 11 files changed, 75 insertions(+), 62 deletions(-) diff --git a/apps/logs/signals.py b/apps/logs/signals.py index 55e0f041..13194e5b 100644 --- a/apps/logs/signals.py +++ b/apps/logs/signals.py @@ -78,6 +78,10 @@ def save_object(sender, instance, **kwargs): user, ip = get_user_and_ip(sender) + from django.contrib.auth.models import AnonymousUser + if isinstance(user, AnonymousUser): + user = None + if user is not None and instance._meta.label_lower == "auth.user" and previous: # Don't save last login modifications if instance.last_login != previous.last_login: diff --git a/apps/member/admin.py b/apps/member/admin.py index fb107377..70b00459 100644 --- a/apps/member/admin.py +++ b/apps/member/admin.py @@ -6,7 +6,7 @@ from django.contrib.auth.admin import UserAdmin from django.contrib.auth.models import User from .forms import ProfileForm -from .models import Club, Membership, Profile, Role +from .models import Club, Membership, Profile, Role, RolePermissions class ProfileInline(admin.StackedInline): @@ -40,3 +40,4 @@ admin.site.register(User, CustomUserAdmin) admin.site.register(Club) admin.site.register(Membership) admin.site.register(Role) +admin.site.register(RolePermissions) diff --git a/apps/member/backends.py b/apps/member/backends.py index 9ef9706f..db227cdb 100644 --- a/apps/member/backends.py +++ b/apps/member/backends.py @@ -1,33 +1,37 @@ -from django.contribs.contenttype.models import ContentType +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + from member.models import Club, Membership, RolePermissions +from django.contrib.auth.backends import ModelBackend -class PermissionBackend(object): +class PermissionBackend(ModelBackend): supports_object_permissions = True supports_anonymous_user = False supports_inactive_user = False - def authenticate(self, username, password): - return None - - def permissions(self, user, obj): - for membership in user.memberships.all(): - if not membership.valid() or membership.role is None: + def permissions(self, user): + for membership in Membership.objects.filter(user=user).all(): + if not membership.valid() or membership.roles is None: continue - for permission in RolePermissions.objects.get(role=membership.role).permissions.objects.all(): - permission = permission.about(user=user, club=membership.club) - yield permission + 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) + yield permission def has_perm(self, user_obj, perm, obj=None): + if user_obj.is_superuser: + return True + if obj is None: return False perm = perm.split('_', 3) perm_type = perm[1] perm_field = perm[2] if len(perm) == 3 else None - return any(permission.applies(obj, perm_type, perm_field) for obj in self.permissions(user_obj, obj)) + return any(permission.applies(obj, perm_type, perm_field) for permission in self.permissions(user_obj)) + + def has_module_perms(self, user_obj, app_label): + return False def get_all_permissions(self, user_obj, obj=None): - if obj is None: - return [] - else: - return list(self.permissions(user_obj, obj)) + return list(self.permissions(user_obj)) diff --git a/apps/member/models.py b/apps/member/models.py index c90ab15c..1ca82af0 100644 --- a/apps/member/models.py +++ b/apps/member/models.py @@ -154,9 +154,9 @@ class Membership(models.Model): def valid(self): if self.date_end is not None: - return self.date_start <= datetime.datetime.now() < self.date_end + return self.date_start.toordinal() <= datetime.datetime.now().toordinal() < self.date_end.toordinal() else: - return self.date_start <= datetime.datetime.now() + return self.date_start.toordinal() <= datetime.datetime.now().toordinal() class Meta: verbose_name = _('membership') diff --git a/apps/permission/admin.py b/apps/permission/admin.py index e93de0c5..f7a9b4b5 100644 --- a/apps/permission/admin.py +++ b/apps/permission/admin.py @@ -1,3 +1,6 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-lateré + from django.contrib import admin from .models import Permission diff --git a/apps/permission/apps.py b/apps/permission/apps.py index 0f46ef08..c9c912a5 100644 --- a/apps/permission/apps.py +++ b/apps/permission/apps.py @@ -1,3 +1,6 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + from django.apps import AppConfig diff --git a/apps/permission/models.py b/apps/permission/models.py index 000fe69f..9584f59f 100644 --- a/apps/permission/models.py +++ b/apps/permission/models.py @@ -1,3 +1,6 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + import functools import json import operator @@ -24,28 +27,25 @@ class InstancedPermission: """ if self.type == 'add': if permission_type == self.type: - return self.query(obj) + return obj in self.model.modelclass().objects.get(self.query) if ContentType.objects.get_for_model(obj) != self.model: # The permission does not apply to the model return False - if self.permission is None: - if permission_type == self.type: - if field_name is not None: - return field_name == self.field - else: - return True - else: + if permission_type == self.type: + if field_name and field_name != self.field: return False - elif obj in self.model.objects.get(self.query): - return True + return obj in self.model.model_class().objects.filter(self.query).all() else: return False def __repr__(self): if self.field: - return _("Can {type} {model}.{field} in {permission}").format(type=self.type, model=self.model, field=self.field, permission=self.permission) + return _("Can {type} {model}.{field} in {query}").format(type=self.type, model=self.model, field=self.field, query=self.query) else: - return _("Can {type} {model} in {permission}").format(type=self.type, model=self.model, permission=self.permission) + return _("Can {type} {model} in {query}").format(type=self.type, model=self.model, query=self.query) + + def __str__(self): + return self.__repr__() class Permission(models.Model): @@ -61,24 +61,24 @@ class Permission(models.Model): # A json encoded Q object with the following grammar # query -> [] | {} (the empty query representing all objects) - # query -> ['AND', query, …] AND multiple queries - # | ['OR', query, …] OR multiple queries - # | ['NOT', query] Opposite of query + # query -> ["AND", query, …] AND multiple queries + # | ["OR", query, …] OR multiple queries + # | ["NOT", query] Opposite of query # query -> {key: value, …} A list of fields and values of a Q object # key -> string A field name # value -> int | string | bool | null Literal values # | [parameter] A parameter - # | {'F': oper} An F object + # | {"F": oper} An F object # oper -> [string] A parameter - # | ['ADD', oper, …] Sum multiple F objects or literal - # | ['SUB', oper, oper] Substract two F objects or literal - # | ['MUL', oper, …] Multiply F objects or literals + # | ["ADD", oper, …] Sum multiple F objects or literal + # | ["SUB", oper, oper] Substract two F objects or literal + # | ["MUL", oper, …] Multiply F objects or literals # | int | string | bool | null Literal values - # | ['F', string] A field + # | ["F", string] A field # # Examples: - # Q(is_admin=True) := {'is_admin': ['TYPE', 'bool', 'True']} - # ~Q(is_admin=True) := ['NOT', {'is_admin': ['TYPE', 'bool', 'True']}] + # Q(is_superuser=True) := {"is_superuser": true} + # ~Q(is_superuser=True) := ["NOT", {"is_superuser": true}] query = models.TextField() type = models.CharField(max_length=15, choices=PERMISSION_TYPES) @@ -94,23 +94,22 @@ class Permission(models.Model): if self.field and self.type not in {'view', 'change'}: raise ValidationError(_("Specifying field applies only to view and change permission types.")) - def save(self): + def save(self, **kwargs): self.full_clean() super().save() @staticmethod - def compute_f(_oper, **kwargs): - oper = _oper + def compute_f(oper, **kwargs): if isinstance(oper, list): if len(oper) == 1: return kwargs[oper[0]].pk elif len(oper) >= 2: if oper[0] == 'ADD': - return functools.reduce(operator.add, [compute_f(oper, **kwargs) for oper in oper[1:]]) + return functools.reduce(operator.add, [Permission.compute_f(oper, **kwargs) for oper in oper[1:]]) elif oper[0] == 'SUB': - return compute_f(oper[1], **kwargs) - compute_f(oper[2], **kwargs) + return Permission.compute_f(oper[1], **kwargs) - Permission.compute_f(oper[2], **kwargs) elif oper[0] == 'MUL': - return functools.reduce(operator.mul, [compute_f(oper, **kwargs) for oper in oper[1:]]) + return functools.reduce(operator.mul, [Permission.compute_f(oper, **kwargs) for oper in oper[1:]]) elif oper[0] == 'F': return F(oper[1]) else: @@ -118,9 +117,7 @@ class Permission(models.Model): # TODO: find a better way to crash here raise Exception("F is wrong") - def _about(_self, _query, **kwargs): - self = _self - query = _query + def _about(self, query, **kwargs): if self.type == 'add': # Handle add permission differently return self._about_add(query, **kwargs) @@ -145,7 +142,7 @@ class Permission(models.Model): q_kwargs[key] = kwargs[value[0]].pk elif isinstance(value, dict): # It is an F object - q_kwargs[key] = compute_f(query['F'], **kwargs) + q_kwargs[key] = Permission.compute_f(query['F'], **kwargs) else: q_kwargs[key] = value return Q(**q_kwargs) @@ -153,16 +150,15 @@ 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): - self = _self + 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(query, **kwargs)(obj) for query in query[1:]]) + 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(query, **kwargs)(obj) for query in query[1:]]) + 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): @@ -174,7 +170,7 @@ class Permission(models.Model): q_kwargs[key] = kwargs[value[0]].pk elif isinstance(value, dict): # It is an F object - q_kwargs[key] = compute_f(query['F'], **kwargs) + q_kwargs[key] = Permission.compute_f(query['F'], **kwargs) else: q_kwargs[key] = value def func(obj): diff --git a/apps/permission/tests.py b/apps/permission/tests.py index 7ce503c2..b5d5752e 100644 --- a/apps/permission/tests.py +++ b/apps/permission/tests.py @@ -1,3 +1,6 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + from django.test import TestCase # Create your tests here. diff --git a/apps/permission/views.py b/apps/permission/views.py index 91ea44a2..8d81fd33 100644 --- a/apps/permission/views.py +++ b/apps/permission/views.py @@ -1,3 +1,6 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + from django.shortcuts import render # Create your views here. diff --git a/note_kfet/settings/base.py b/note_kfet/settings/base.py index 63b7ff24..20937fac 100644 --- a/note_kfet/settings/base.py +++ b/note_kfet/settings/base.py @@ -37,7 +37,6 @@ INSTALLED_APPS = [ # External apps 'polymorphic', - 'guardian', 'reversion', 'crispy_forms', 'django_tables2', @@ -134,8 +133,8 @@ PASSWORD_HASHERS = [ # Django Guardian object permissions AUTHENTICATION_BACKENDS = ( - 'django.contrib.auth.backends.ModelBackend', # this is default - 'guardian.backends.ObjectPermissionBackend', + #'django.contrib.auth.backends.ModelBackend', # this is default + 'member.backends.PermissionBackend', 'cas.backends.CASBackend', ) @@ -153,8 +152,6 @@ REST_FRAMEWORK = { ANONYMOUS_USER_NAME = None # Disable guardian anonymous user -GUARDIAN_GET_CONTENT_TYPE = 'polymorphic.contrib.guardian.get_polymorphic_base_content_type' - # Internationalization # https://docs.djangoproject.com/en/2.2/topics/i18n/ diff --git a/requirements.txt b/requirements.txt index 244690bc..9a5eaa22 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,7 +9,6 @@ django-cas-server==1.1.0 django-crispy-forms==1.7.2 django-extensions==2.1.9 django-filter==2.2.0 -django-guardian==2.1.0 django-polymorphic==2.0.3 djangorestframework==3.9.0 django-rest-polymorphic==0.1.8 From 057f42fdb67195d5cd434924bb083074eaf35a99 Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Wed, 18 Mar 2020 14:42:35 +0100 Subject: [PATCH 14/34] Handle permissions (and it seems working!) --- apps/activity/api/views.py | 9 ++-- apps/api/urls.py | 12 +++-- apps/api/viewsets.py | 26 +++++++++++ apps/logs/api/views.py | 4 +- apps/member/api/views.py | 10 ++-- apps/member/backends.py | 57 ++++++++++++++++++++--- apps/member/views.py | 1 - apps/note/api/serializers.py | 6 +++ apps/note/api/views.py | 24 +++++----- apps/permission/__init__.py | 4 ++ apps/permission/api/__init__.py | 0 apps/permission/api/serializers.py | 17 +++++++ apps/permission/api/urls.py | 11 +++++ apps/permission/api/views.py | 20 ++++++++ apps/permission/apps.py | 7 +++ apps/permission/models.py | 70 +++++++++++++++++++--------- apps/permission/permissions.py | 58 +++++++++++++++++++++++ apps/permission/signals.py | 75 ++++++++++++++++++++++++++++++ note_kfet/settings/base.py | 3 +- 19 files changed, 357 insertions(+), 57 deletions(-) create mode 100644 apps/api/viewsets.py create mode 100644 apps/permission/api/__init__.py create mode 100644 apps/permission/api/serializers.py create mode 100644 apps/permission/api/urls.py create mode 100644 apps/permission/api/views.py create mode 100644 apps/permission/permissions.py create mode 100644 apps/permission/signals.py diff --git a/apps/activity/api/views.py b/apps/activity/api/views.py index 4ee2194d..651560fd 100644 --- a/apps/activity/api/views.py +++ b/apps/activity/api/views.py @@ -1,14 +1,15 @@ # Copyright (C) 2018-2020 by BDE ENS Paris-Saclay # SPDX-License-Identifier: GPL-3.0-or-later + from django_filters.rest_framework import DjangoFilterBackend -from rest_framework import viewsets from rest_framework.filters import SearchFilter +from api.viewsets import ReadProtectedModelViewSet from .serializers import ActivityTypeSerializer, ActivitySerializer, GuestSerializer from ..models import ActivityType, Activity, Guest -class ActivityTypeViewSet(viewsets.ModelViewSet): +class ActivityTypeViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `ActivityType` objects, serialize it to JSON with the given serializer, @@ -20,7 +21,7 @@ class ActivityTypeViewSet(viewsets.ModelViewSet): filterset_fields = ['name', 'can_invite', ] -class ActivityViewSet(viewsets.ModelViewSet): +class ActivityViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Activity` objects, serialize it to JSON with the given serializer, @@ -32,7 +33,7 @@ class ActivityViewSet(viewsets.ModelViewSet): filterset_fields = ['name', 'description', 'activity_type', ] -class GuestViewSet(viewsets.ModelViewSet): +class GuestViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Guest` objects, serialize it to JSON with the given serializer, diff --git a/apps/api/urls.py b/apps/api/urls.py index 95ed5f99..40e6c572 100644 --- a/apps/api/urls.py +++ b/apps/api/urls.py @@ -5,12 +5,16 @@ from django.conf.urls import url, include from django.contrib.auth.models import User from django.contrib.contenttypes.models import ContentType from django_filters.rest_framework import DjangoFilterBackend -from rest_framework import routers, serializers, viewsets +from rest_framework import routers, serializers from rest_framework.filters import SearchFilter +from rest_framework.viewsets import ReadOnlyModelViewSet + from activity.api.urls import register_activity_urls +from api.viewsets import ReadProtectedModelViewSet from member.api.urls import register_members_urls from note.api.urls import register_note_urls from logs.api.urls import register_logs_urls +from permission.api.urls import register_permission_urls class UserSerializer(serializers.ModelSerializer): @@ -39,7 +43,7 @@ class ContentTypeSerializer(serializers.ModelSerializer): fields = '__all__' -class UserViewSet(viewsets.ModelViewSet): +class UserViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `User` objects, serialize it to JSON with the given serializer, @@ -52,7 +56,8 @@ class UserViewSet(viewsets.ModelViewSet): search_fields = ['$username', '$first_name', '$last_name', ] -class ContentTypeViewSet(viewsets.ReadOnlyModelViewSet): +# This ViewSet is the only one that is accessible from all authenticated users! +class ContentTypeViewSet(ReadOnlyModelViewSet): """ REST API View set. The djangorestframework plugin will get all `User` objects, serialize it to JSON with the given serializer, @@ -70,6 +75,7 @@ router.register('user', UserViewSet) register_members_urls(router, 'members') register_activity_urls(router, 'activity') register_note_urls(router, 'note') +register_permission_urls(router, 'permission') register_logs_urls(router, 'logs') app_name = 'api' diff --git a/apps/api/viewsets.py b/apps/api/viewsets.py new file mode 100644 index 00000000..cb32b09e --- /dev/null +++ b/apps/api/viewsets.py @@ -0,0 +1,26 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + +from django.contrib.contenttypes.models import ContentType +from member.backends import PermissionBackend +from rest_framework import viewsets + + +class ReadProtectedModelViewSet(viewsets.ModelViewSet): + """ + Protect a ModelViewSet by filtering the objects that the user cannot see. + """ + + def get_queryset(self): + model = ContentType.objects.get_for_model(self.serializer_class.Meta.model) + return super().get_queryset().filter(PermissionBackend().filter_queryset(self.request.user, model, "view")) + + +class ReadOnlyProtectedModelViewSet(viewsets.ReadOnlyModelViewSet): + """ + Protect a ReadOnlyModelViewSet by filtering the objects that the user cannot see. + """ + + def get_queryset(self): + model = ContentType.objects.get_for_model(self.serializer_class.Meta.model) + return super().get_queryset().filter(PermissionBackend().filter_queryset(self.request.user, model, "view")) diff --git a/apps/logs/api/views.py b/apps/logs/api/views.py index 2c47b7a2..6bd4f721 100644 --- a/apps/logs/api/views.py +++ b/apps/logs/api/views.py @@ -2,14 +2,14 @@ # SPDX-License-Identifier: GPL-3.0-or-later from django_filters.rest_framework import DjangoFilterBackend -from rest_framework import viewsets from rest_framework.filters import OrderingFilter +from api.viewsets import ReadOnlyProtectedModelViewSet from .serializers import ChangelogSerializer from ..models import Changelog -class ChangelogViewSet(viewsets.ReadOnlyModelViewSet): +class ChangelogViewSet(ReadOnlyProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Changelog` objects, serialize it to JSON with the given serializer, diff --git a/apps/member/api/views.py b/apps/member/api/views.py index c85df903..b4715cae 100644 --- a/apps/member/api/views.py +++ b/apps/member/api/views.py @@ -1,14 +1,14 @@ # Copyright (C) 2018-2020 by BDE ENS Paris-Saclay # SPDX-License-Identifier: GPL-3.0-or-later -from rest_framework import viewsets from rest_framework.filters import SearchFilter +from api.viewsets import ReadProtectedModelViewSet from .serializers import ProfileSerializer, ClubSerializer, RoleSerializer, MembershipSerializer from ..models import Profile, Club, Role, Membership -class ProfileViewSet(viewsets.ModelViewSet): +class ProfileViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Profile` objects, serialize it to JSON with the given serializer, @@ -18,7 +18,7 @@ class ProfileViewSet(viewsets.ModelViewSet): serializer_class = ProfileSerializer -class ClubViewSet(viewsets.ModelViewSet): +class ClubViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Club` objects, serialize it to JSON with the given serializer, @@ -30,7 +30,7 @@ class ClubViewSet(viewsets.ModelViewSet): search_fields = ['$name', ] -class RoleViewSet(viewsets.ModelViewSet): +class RoleViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Role` objects, serialize it to JSON with the given serializer, @@ -42,7 +42,7 @@ class RoleViewSet(viewsets.ModelViewSet): search_fields = ['$name', ] -class MembershipViewSet(viewsets.ModelViewSet): +class MembershipViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Membership` objects, serialize it to JSON with the given serializer, diff --git a/apps/member/backends.py b/apps/member/backends.py index db227cdb..3fdbd8d1 100644 --- a/apps/member/backends.py +++ b/apps/member/backends.py @@ -1,7 +1,12 @@ # Copyright (C) 2018-2020 by BDE ENS Paris-Saclay # SPDX-License-Identifier: GPL-3.0-or-later -from member.models import Club, Membership, RolePermissions +from django.contrib.auth.models import User +from django.core.exceptions import PermissionDenied +from django.db.models import Q, F + +from note.models import Note, NoteUser, NoteClub, NoteSpecial +from .models import Membership, RolePermissions, Club from django.contrib.auth.backends import ModelBackend @@ -14,21 +19,61 @@ class PermissionBackend(ModelBackend): 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) + 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 + ) yield permission + def filter_queryset(self, user, model, type, field=None): + """ + Filter a queryset by considering the permissions of a given user. + :param user: The owner of the permissions that are fetched + :param model: The concerned model of the queryset + :param type: The type of modification (view, add, change, delete) + :param field: The field of the model to test, if concerned + :return: A query that corresponds to the filter to give to a queryset + """ + + if user.is_superuser: + # Superusers have all rights + return Q() + + # Never satisfied + query = Q(pk=-1) + for perm in self.permissions(user): + if field and field != perm.field: + continue + if perm.model != model or perm.type != type: + continue + query = query | perm.query + return query + def has_perm(self, user_obj, perm, obj=None): if user_obj.is_superuser: return True if obj is None: - return False - perm = perm.split('_', 3) - perm_type = perm[1] + return True + + perm = perm.split('.')[-1].split('_', 2) + perm_type = perm[0] perm_field = perm[2] if len(perm) == 3 else None - return any(permission.applies(obj, perm_type, perm_field) for permission in self.permissions(user_obj)) + if any(permission.applies(obj, perm_type, perm_field) for permission in self.permissions(user_obj)): + return True + return False def has_module_perms(self, user_obj, app_label): return False diff --git a/apps/member/views.py b/apps/member/views.py index dacfde33..2213f37d 100644 --- a/apps/member/views.py +++ b/apps/member/views.py @@ -203,7 +203,6 @@ class DeleteAliasView(LoginRequiredMixin, DeleteView): return HttpResponseRedirect(self.get_success_url()) def get_success_url(self): - print(self.request) return reverse_lazy('member:user_alias', kwargs={'pk': self.object.note.user.pk}) def get(self, request, *args, **kwargs): diff --git a/apps/note/api/serializers.py b/apps/note/api/serializers.py index 85f500ed..02311de1 100644 --- a/apps/note/api/serializers.py +++ b/apps/note/api/serializers.py @@ -88,6 +88,9 @@ class NotePolymorphicSerializer(PolymorphicSerializer): NoteSpecial: NoteSpecialSerializer } + class Meta: + model = Note + class TemplateCategorySerializer(serializers.ModelSerializer): """ @@ -162,3 +165,6 @@ class TransactionPolymorphicSerializer(PolymorphicSerializer): MembershipTransaction: MembershipTransactionSerializer, SpecialTransaction: SpecialTransactionSerializer, } + + class Meta: + model = Transaction diff --git a/apps/note/api/views.py b/apps/note/api/views.py index 29c79bd8..6a3bb41e 100644 --- a/apps/note/api/views.py +++ b/apps/note/api/views.py @@ -3,9 +3,9 @@ from django.db.models import Q from django_filters.rest_framework import DjangoFilterBackend -from rest_framework import viewsets from rest_framework.filters import OrderingFilter, SearchFilter +from api.viewsets import ReadProtectedModelViewSet from .serializers import NoteSerializer, NotePolymorphicSerializer, NoteClubSerializer, NoteSpecialSerializer, \ NoteUserSerializer, AliasSerializer, \ TemplateCategorySerializer, TransactionTemplateSerializer, TransactionPolymorphicSerializer @@ -13,7 +13,7 @@ from ..models.notes import Note, NoteClub, NoteSpecial, NoteUser, Alias from ..models.transactions import TransactionTemplate, Transaction, TemplateCategory -class NoteViewSet(viewsets.ModelViewSet): +class NoteViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Note` objects, serialize it to JSON with the given serializer, @@ -23,7 +23,7 @@ class NoteViewSet(viewsets.ModelViewSet): serializer_class = NoteSerializer -class NoteClubViewSet(viewsets.ModelViewSet): +class NoteClubViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `NoteClub` objects, serialize it to JSON with the given serializer, @@ -33,7 +33,7 @@ class NoteClubViewSet(viewsets.ModelViewSet): serializer_class = NoteClubSerializer -class NoteSpecialViewSet(viewsets.ModelViewSet): +class NoteSpecialViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `NoteSpecial` objects, serialize it to JSON with the given serializer, @@ -43,7 +43,7 @@ class NoteSpecialViewSet(viewsets.ModelViewSet): serializer_class = NoteSpecialSerializer -class NoteUserViewSet(viewsets.ModelViewSet): +class NoteUserViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `NoteUser` objects, serialize it to JSON with the given serializer, @@ -53,7 +53,7 @@ class NoteUserViewSet(viewsets.ModelViewSet): serializer_class = NoteUserSerializer -class NotePolymorphicViewSet(viewsets.ModelViewSet): +class NotePolymorphicViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Note` objects (with polymorhism), serialize it to JSON with the given serializer, @@ -70,7 +70,7 @@ class NotePolymorphicViewSet(viewsets.ModelViewSet): Parse query and apply filters. :return: The filtered set of requested notes """ - queryset = Note.objects.all() + queryset = super().get_queryset() alias = self.request.query_params.get("alias", ".*") queryset = queryset.filter( @@ -92,7 +92,7 @@ class NotePolymorphicViewSet(viewsets.ModelViewSet): return queryset.distinct() -class AliasViewSet(viewsets.ModelViewSet): +class AliasViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Alias` objects, serialize it to JSON with the given serializer, @@ -110,7 +110,7 @@ class AliasViewSet(viewsets.ModelViewSet): :return: The filtered set of requested aliases """ - queryset = Alias.objects.all() + queryset = super().get_queryset() alias = self.request.query_params.get("alias", ".*") queryset = queryset.filter( @@ -138,7 +138,7 @@ class AliasViewSet(viewsets.ModelViewSet): return queryset -class TemplateCategoryViewSet(viewsets.ModelViewSet): +class TemplateCategoryViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `TemplateCategory` objects, serialize it to JSON with the given serializer, @@ -150,7 +150,7 @@ class TemplateCategoryViewSet(viewsets.ModelViewSet): search_fields = ['$name', ] -class TransactionTemplateViewSet(viewsets.ModelViewSet): +class TransactionTemplateViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `TransactionTemplate` objects, serialize it to JSON with the given serializer, @@ -162,7 +162,7 @@ class TransactionTemplateViewSet(viewsets.ModelViewSet): filterset_fields = ['name', 'amount', 'display', 'category', ] -class TransactionViewSet(viewsets.ModelViewSet): +class TransactionViewSet(ReadProtectedModelViewSet): """ REST API View set. The djangorestframework plugin will get all `Transaction` objects, serialize it to JSON with the given serializer, diff --git a/apps/permission/__init__.py b/apps/permission/__init__.py index e69de29b..4e3eb6bc 100644 --- a/apps/permission/__init__.py +++ b/apps/permission/__init__.py @@ -0,0 +1,4 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + +default_app_config = 'permission.apps.PermissionConfig' diff --git a/apps/permission/api/__init__.py b/apps/permission/api/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/apps/permission/api/serializers.py b/apps/permission/api/serializers.py new file mode 100644 index 00000000..0a52f4fe --- /dev/null +++ b/apps/permission/api/serializers.py @@ -0,0 +1,17 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + +from rest_framework import serializers + +from ..models import Permission + + +class PermissionSerializer(serializers.ModelSerializer): + """ + REST API Serializer for Permission types. + The djangorestframework plugin will analyse the model `Permission` and parse all fields in the API. + """ + + class Meta: + model = Permission + fields = '__all__' diff --git a/apps/permission/api/urls.py b/apps/permission/api/urls.py new file mode 100644 index 00000000..d50344ea --- /dev/null +++ b/apps/permission/api/urls.py @@ -0,0 +1,11 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + +from .views import PermissionViewSet + + +def register_permission_urls(router, path): + """ + Configure router for permission REST API. + """ + router.register(path, PermissionViewSet) diff --git a/apps/permission/api/views.py b/apps/permission/api/views.py new file mode 100644 index 00000000..6087c83e --- /dev/null +++ b/apps/permission/api/views.py @@ -0,0 +1,20 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + +from django_filters.rest_framework import DjangoFilterBackend + +from api.viewsets import ReadOnlyProtectedModelViewSet +from .serializers import PermissionSerializer +from ..models import Permission + + +class PermissionViewSet(ReadOnlyProtectedModelViewSet): + """ + REST API View set. + The djangorestframework plugin will get all `Changelog` objects, serialize it to JSON with the given serializer, + then render it on /api/logs/ + """ + queryset = Permission.objects.all() + serializer_class = PermissionSerializer + filter_backends = [DjangoFilterBackend] + filterset_fields = ['model', 'type', ] diff --git a/apps/permission/apps.py b/apps/permission/apps.py index c9c912a5..c0caa41b 100644 --- a/apps/permission/apps.py +++ b/apps/permission/apps.py @@ -2,7 +2,14 @@ # SPDX-License-Identifier: GPL-3.0-or-later from django.apps import AppConfig +from django.db.models.signals import pre_save, pre_delete class PermissionConfig(AppConfig): name = 'permission' + + def ready(self): + # noinspection PyUnresolvedReferences + from . import signals + pre_save.connect(signals.pre_save_object) + pre_delete.connect(signals.pre_delete_object) diff --git a/apps/permission/models.py b/apps/permission/models.py index 9584f59f..b90fcfb9 100644 --- a/apps/permission/models.py +++ b/apps/permission/models.py @@ -27,12 +27,13 @@ class InstancedPermission: """ if self.type == 'add': if permission_type == self.type: - return obj in self.model.modelclass().objects.get(self.query) + return self.query(obj) + if ContentType.objects.get_for_model(obj) != self.model: # The permission does not apply to the model return False if permission_type == self.type: - if field_name and field_name != self.field: + if self.field and field_name != self.field: return False return obj in self.model.model_class().objects.filter(self.query).all() else: @@ -91,6 +92,7 @@ class Permission(models.Model): unique_together = ('model', 'query', 'type', 'field') def clean(self): + self.query = json.dumps(json.loads(self.query)) if self.field and self.type not in {'view', 'change'}: raise ValidationError(_("Specifying field applies only to view and change permission types.")) @@ -101,21 +103,45 @@ class Permission(models.Model): @staticmethod def compute_f(oper, **kwargs): if isinstance(oper, list): - if len(oper) == 1: - return kwargs[oper[0]].pk - elif len(oper) >= 2: - if oper[0] == 'ADD': - return functools.reduce(operator.add, [Permission.compute_f(oper, **kwargs) for oper in oper[1:]]) - elif oper[0] == 'SUB': - return Permission.compute_f(oper[1], **kwargs) - Permission.compute_f(oper[2], **kwargs) - elif oper[0] == 'MUL': - return functools.reduce(operator.mul, [Permission.compute_f(oper, **kwargs) for oper in oper[1:]]) - elif oper[0] == 'F': - return F(oper[1]) + if oper[0] == 'ADD': + return functools.reduce(operator.add, [Permission.compute_f(oper, **kwargs) for oper in oper[1:]]) + elif oper[0] == 'SUB': + return Permission.compute_f(oper[1], **kwargs) - Permission.compute_f(oper[2], **kwargs) + elif oper[0] == 'MUL': + return functools.reduce(operator.mul, [Permission.compute_f(oper, **kwargs) for oper in oper[1:]]) + elif oper[0] == 'F': + return F(oper[1]) + else: + field = kwargs[oper[0]] + for i in range(1, len(oper)): + field = getattr(field, oper[i]) + return field else: return oper - # TODO: find a better way to crash here - raise Exception("F is wrong") + + @staticmethod + def compute_param(value, **kwargs): + if not isinstance(value, list): + return value + + field = kwargs[value[0]] + for i in range(1, len(value)): + if isinstance(value[i], list): + field = getattr(field, value[i][0]) + params = [] + call_kwargs = {} + for j in range(1, len(value[i])): + param = Permission.compute_param(value[i][j], **kwargs) + if isinstance(param, dict): + for key in param: + val = Permission.compute_param(param[key], **kwargs) + call_kwargs[key] = val + else: + params.append(param) + field = field(*params, **call_kwargs) + else: + field = getattr(field, value[i]) + return field def _about(self, query, **kwargs): if self.type == 'add': @@ -124,8 +150,8 @@ class Permission(models.Model): if len(query) == 0: # The query is either [] or {} and # applies to all objects of the model - # to represent this we return None - return None + # to represent this we return a trivial request + return Q(pk=F("pk")) if isinstance(query, list): if query[0] == 'AND': return functools.reduce(operator.and_, [self._about(query, **kwargs) for query in query[1:]]) @@ -138,11 +164,11 @@ class Permission(models.Model): for key in query: value = query[key] if isinstance(value, list): - # It is a parameter we query its primary key - q_kwargs[key] = kwargs[value[0]].pk + # It is a parameter we query its return value + 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) + q_kwargs[key] = Permission.compute_f(value['F'], **kwargs) else: q_kwargs[key] = value return Q(**q_kwargs) @@ -167,7 +193,7 @@ class Permission(models.Model): value = query[key] if isinstance(value, list): # It is a parameter we query its primary key - q_kwargs[key] = kwargs[value[0]].pk + 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) @@ -176,7 +202,7 @@ class Permission(models.Model): def func(obj): nonlocal q_kwargs for arg in q_kwargs: - if getattr(obj, arg) != q_kwargs(arg): + if getattr(obj, arg) != q_kwargs[arg]: return False return True return func diff --git a/apps/permission/permissions.py b/apps/permission/permissions.py new file mode 100644 index 00000000..1cbae474 --- /dev/null +++ b/apps/permission/permissions.py @@ -0,0 +1,58 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + +from rest_framework.permissions import DjangoObjectPermissions + +SAFE_METHODS = ('HEAD', 'OPTIONS', ) + + +class StrongDjangoObjectPermissions(DjangoObjectPermissions): + perms_map = { + 'GET': ['%(app_label)s.view_%(model_name)s'], + 'OPTIONS': [], + 'HEAD': [], + 'POST': ['%(app_label)s.add_%(model_name)s'], + 'PUT': ['%(app_label)s.change_%(model_name)s'], + 'PATCH': ['%(app_label)s.change_%(model_name)s'], + 'DELETE': ['%(app_label)s.delete_%(model_name)s'], + } + + def get_required_object_permissions(self, method, model_cls): + kwargs = { + 'app_label': model_cls._meta.app_label, + 'model_name': model_cls._meta.model_name + } + + if method not in self.perms_map: + from rest_framework import exceptions + raise exceptions.MethodNotAllowed(method) + + return [perm % kwargs for perm in self.perms_map[method]] + + def has_object_permission(self, request, view, obj): + # authentication checks have already executed via has_permission + queryset = self._queryset(view) + model_cls = queryset.model + user = request.user + + perms = self.get_required_object_permissions(request.method, model_cls) + + if not user.has_perms(perms, obj): + # If the user does not have permissions we need to determine if + # they have read permissions to see 403, or not, and simply see + # a 404 response. + from django.http import Http404 + + if request.method in SAFE_METHODS: + # Read permissions already checked and failed, no need + # to make another lookup. + raise Http404 + + read_perms = self.get_required_object_permissions('GET', model_cls) + if not user.has_perms(read_perms, obj): + raise Http404 + + # Has read permissions. + return False + + return True diff --git a/apps/permission/signals.py b/apps/permission/signals.py new file mode 100644 index 00000000..a051482e --- /dev/null +++ b/apps/permission/signals.py @@ -0,0 +1,75 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + +from django.core.exceptions import PermissionDenied +from logs.middlewares import get_current_authenticated_user + + +EXCLUDED = [ + 'cas_server.proxygrantingticket', + 'cas_server.proxyticket', + 'cas_server.serviceticket', + 'cas_server.user', + 'cas_server.userattributes', + 'contenttypes.contenttype', + 'logs.changelog', + 'migrations.migration', + 'sessions.session', +] + + +def pre_save_object(sender, instance, **kwargs): + """ + Before a model get saved, we check the permissions + """ + # noinspection PyProtectedMember + if instance._meta.label_lower in EXCLUDED: + return + + user = get_current_authenticated_user() + if user is None: + # Action performed on shell is always granted + return + + qs = sender.objects.filter(pk=instance.pk).all() + model_name_full = instance._meta.label_lower.split(".") + app_label = model_name_full[0] + model_name = model_name_full[1] + + if qs.exists(): + if user.has_perm(app_label + ".change_" + model_name, instance): + return + + 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 old_value == new_value: + continue + if not user.has_perm(app_label + ".change_" + model_name + "_" + field_name, instance): + raise PermissionDenied + else: + if not user.has_perm(app_label + ".add_" + model_name, instance): + raise PermissionDenied + + +def pre_delete_object(sender, instance, **kwargs): + """ + Before a model get deleted, we check the permissions + """ + # noinspection PyProtectedMember + if instance._meta.label_lower in EXCLUDED: + return + + user = get_current_authenticated_user() + if user is None: + # Action performed on shell is always granted + return + + model_name_full = instance._meta.label_lower.split(".") + app_label = model_name_full[0] + model_name = model_name_full[1] + + if not user.has_perm(app_label + ".delete_" + model_name, instance): + raise PermissionDenied diff --git a/note_kfet/settings/base.py b/note_kfet/settings/base.py index 29ff49c5..800c798e 100644 --- a/note_kfet/settings/base.py +++ b/note_kfet/settings/base.py @@ -139,8 +139,7 @@ REST_FRAMEWORK = { # Use Django's standard `django.contrib.auth` permissions, # or allow read-only access for unauthenticated users. 'DEFAULT_PERMISSION_CLASSES': [ - # TODO Maybe replace it with our custom permissions system - 'rest_framework.permissions.DjangoModelPermissionsOrAnonReadOnly' + 'permission.permissions.StrongDjangoObjectPermissions', ], 'DEFAULT_AUTHENTICATION_CLASSES': [ 'rest_framework.authentication.SessionAuthentication', From e461d70b1431e4e2e7ea858f68248731e1008c15 Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Wed, 18 Mar 2020 15:49:52 +0100 Subject: [PATCH 15/34] Improve add permissions --- apps/note/models/transactions.py | 7 ++++--- apps/permission/models.py | 20 ++++++++++++++++---- apps/permission/signals.py | 4 ++++ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/apps/note/models/transactions.py b/apps/note/models/transactions.py index 86c00737..ee890c9d 100644 --- a/apps/note/models/transactions.py +++ b/apps/note/models/transactions.py @@ -129,14 +129,13 @@ class Transaction(PolymorphicModel): models.Index(fields=['destination']), ] - def save(self, *args, **kwargs): + def post_save(self, *args, **kwargs): """ When saving, also transfer money between two notes """ if self.source.pk == self.destination.pk: # When source == destination, no money is transfered - super().save(*args, **kwargs) return created = self.pk is None @@ -152,10 +151,12 @@ class Transaction(PolymorphicModel): self.source.balance -= to_transfer self.destination.balance += to_transfer + # We save first the transaction, in case of the user has no right to transfer money + super().save(*args, **kwargs) + # Save notes self.source.save() self.destination.save() - super().save(*args, **kwargs) @property def total(self): diff --git a/apps/permission/models.py b/apps/permission/models.py index b90fcfb9..ead3f721 100644 --- a/apps/permission/models.py +++ b/apps/permission/models.py @@ -25,13 +25,14 @@ class InstancedPermission: Returns True if the permission applies to the field `field_name` object `obj` """ + if ContentType.objects.get_for_model(obj) != self.model: + # The permission does not apply to the model + return False + if self.type == 'add': if permission_type == self.type: return self.query(obj) - if ContentType.objects.get_for_model(obj) != self.model: - # The permission does not apply to the model - return False if permission_type == self.type: if self.field and field_name != self.field: return False @@ -202,7 +203,18 @@ class Permission(models.Model): def func(obj): nonlocal q_kwargs for arg in q_kwargs: - if getattr(obj, arg) != q_kwargs[arg]: + 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 diff --git a/apps/permission/signals.py b/apps/permission/signals.py index a051482e..e93c1666 100644 --- a/apps/permission/signals.py +++ b/apps/permission/signals.py @@ -14,6 +14,10 @@ EXCLUDED = [ 'contenttypes.contenttype', 'logs.changelog', 'migrations.migration', + 'note.note', + 'note.noteuser', + 'note.noteclub', + 'note.notespecial', 'sessions.session', ] From 730d37c62062f2eb7d832430e099cd76c013d94b Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Thu, 19 Mar 2020 02:26:06 +0100 Subject: [PATCH 16/34] Protect views from viewing if the user has no right to view an object --- apps/api/viewsets.py | 4 +-- apps/member/backends.py | 18 ++++++---- apps/member/views.py | 19 +++++++++-- apps/note/api/views.py | 5 +-- apps/note/models/transactions.py | 3 +- apps/note/views.py | 20 +++++++---- apps/permission/templatetags/__init__.py | 0 apps/permission/templatetags/perms.py | 42 ++++++++++++++++++++++++ templates/base.html | 40 +++++++++++++--------- 9 files changed, 116 insertions(+), 35 deletions(-) create mode 100644 apps/permission/templatetags/__init__.py create mode 100644 apps/permission/templatetags/perms.py diff --git a/apps/api/viewsets.py b/apps/api/viewsets.py index cb32b09e..6c5a207a 100644 --- a/apps/api/viewsets.py +++ b/apps/api/viewsets.py @@ -13,7 +13,7 @@ class ReadProtectedModelViewSet(viewsets.ModelViewSet): def get_queryset(self): model = ContentType.objects.get_for_model(self.serializer_class.Meta.model) - return super().get_queryset().filter(PermissionBackend().filter_queryset(self.request.user, model, "view")) + return super().get_queryset().filter(PermissionBackend.filter_queryset(self.request.user, model, "view")) class ReadOnlyProtectedModelViewSet(viewsets.ReadOnlyModelViewSet): @@ -23,4 +23,4 @@ class ReadOnlyProtectedModelViewSet(viewsets.ReadOnlyModelViewSet): def get_queryset(self): model = ContentType.objects.get_for_model(self.serializer_class.Meta.model) - return super().get_queryset().filter(PermissionBackend().filter_queryset(self.request.user, model, "view")) + return super().get_queryset().filter(PermissionBackend.filter_queryset(self.request.user, model, "view")) diff --git a/apps/member/backends.py b/apps/member/backends.py index 3fdbd8d1..f0b4e8f2 100644 --- a/apps/member/backends.py +++ b/apps/member/backends.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: GPL-3.0-or-later from django.contrib.auth.models import User +from django.contrib.contenttypes.models import ContentType from django.core.exceptions import PermissionDenied from django.db.models import Q, F @@ -15,7 +16,8 @@ class PermissionBackend(ModelBackend): supports_anonymous_user = False supports_inactive_user = False - def permissions(self, user): + @staticmethod + def permissions(user): for membership in Membership.objects.filter(user=user).all(): if not membership.valid() or membership.roles is None: continue @@ -37,12 +39,13 @@ class PermissionBackend(ModelBackend): ) yield permission - def filter_queryset(self, user, model, type, field=None): + @staticmethod + def filter_queryset(user, model, t, field=None): """ Filter a queryset by considering the permissions of a given user. :param user: The owner of the permissions that are fetched :param model: The concerned model of the queryset - :param type: The type of modification (view, add, change, delete) + :param t: The type of modification (view, add, change, delete) :param field: The field of the model to test, if concerned :return: A query that corresponds to the filter to give to a queryset """ @@ -51,12 +54,15 @@ class PermissionBackend(ModelBackend): # Superusers have all rights return Q() + if not isinstance(model, ContentType): + model = ContentType.objects.get_for_model(model) + # Never satisfied query = Q(pk=-1) - for perm in self.permissions(user): - if field and field != perm.field: + for perm in PermissionBackend.permissions(user): + if perm.field and field != perm.field: continue - if perm.model != model or perm.type != type: + if perm.model != model or perm.type != t: continue query = query | perm.query return query diff --git a/apps/member/views.py b/apps/member/views.py index 2213f37d..293ad3a8 100644 --- a/apps/member/views.py +++ b/apps/member/views.py @@ -23,6 +23,7 @@ from note.forms import AliasForm, ImageForm from note.models import Alias, NoteUser from note.models.transactions import Transaction from note.tables import HistoryTable, AliasTable +from .backends import PermissionBackend from .filters import UserFilter, UserFilterFormHelper from .forms import SignUpForm, ProfileForm, ClubForm, MembershipForm, MemberFormSet, FormSetHelper @@ -120,6 +121,9 @@ class UserDetailView(LoginRequiredMixin, DetailView): context_object_name = "user_object" template_name = "member/profile_detail.html" + def get_queryset(self, **kwargs): + return super().get_queryset().filter(PermissionBackend.filter_queryset(self.request.user, User, "view")) + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) user = context['user_object'] @@ -147,7 +151,7 @@ class UserListView(LoginRequiredMixin, SingleTableView): formhelper_class = UserFilterFormHelper def get_queryset(self, **kwargs): - qs = super().get_queryset() + qs = super().get_queryset().filter(PermissionBackend.filter_queryset(self.request.user, User, "view")) self.filter = self.filter_class(self.request.GET, queryset=qs) self.filter.form.helper = self.formhelper_class() return self.filter.qs @@ -296,7 +300,7 @@ class UserAutocomplete(autocomplete.Select2QuerySetView): if not self.request.user.is_authenticated: return User.objects.none() - qs = User.objects.all() + qs = User.objects.filter(PermissionBackend.filter_queryset(self.request.user, User, "view")).all() if self.q: qs = qs.filter(username__regex="^" + self.q) @@ -327,11 +331,17 @@ class ClubListView(LoginRequiredMixin, SingleTableView): model = Club table_class = ClubTable + def get_queryset(self, **kwargs): + return super().get_queryset().filter(PermissionBackend.filter_queryset(self.request.user, Club, "view")) + class ClubDetailView(LoginRequiredMixin, DetailView): model = Club context_object_name = "club" + def get_queryset(self, **kwargs): + return super().get_queryset().filter(PermissionBackend.filter_queryset(self.request.user, Club, "view")) + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) club = context["club"] @@ -350,6 +360,11 @@ class ClubAddMemberView(LoginRequiredMixin, CreateView): form_class = MembershipForm template_name = 'member/add_members.html' + def get_queryset(self, **kwargs): + return super().get_queryset().filter(PermissionBackend.filter_queryset(self.request.user, Membership, "view") + | PermissionBackend.filter_queryset(self.request.user, Membership, + "change")) + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context['formset'] = MemberFormSet() diff --git a/apps/note/api/views.py b/apps/note/api/views.py index 6a3bb41e..a4fe6fc1 100644 --- a/apps/note/api/views.py +++ b/apps/note/api/views.py @@ -6,6 +6,7 @@ from django_filters.rest_framework import DjangoFilterBackend from rest_framework.filters import OrderingFilter, SearchFilter from api.viewsets import ReadProtectedModelViewSet +from member.backends import PermissionBackend from .serializers import NoteSerializer, NotePolymorphicSerializer, NoteClubSerializer, NoteSpecialSerializer, \ NoteUserSerializer, AliasSerializer, \ TemplateCategorySerializer, TransactionTemplateSerializer, TransactionPolymorphicSerializer @@ -70,7 +71,7 @@ class NotePolymorphicViewSet(ReadProtectedModelViewSet): Parse query and apply filters. :return: The filtered set of requested notes """ - queryset = super().get_queryset() + queryset = super().get_queryset().filter(PermissionBackend.filter_queryset(self.request.user, Note, "view")) alias = self.request.query_params.get("alias", ".*") queryset = queryset.filter( @@ -110,7 +111,7 @@ class AliasViewSet(ReadProtectedModelViewSet): :return: The filtered set of requested aliases """ - queryset = super().get_queryset() + queryset = super().get_queryset().filter(PermissionBackend.filter_queryset(self.request.user, Alias, "view")) alias = self.request.query_params.get("alias", ".*") queryset = queryset.filter( diff --git a/apps/note/models/transactions.py b/apps/note/models/transactions.py index ee890c9d..b7c8f092 100644 --- a/apps/note/models/transactions.py +++ b/apps/note/models/transactions.py @@ -129,13 +129,14 @@ class Transaction(PolymorphicModel): models.Index(fields=['destination']), ] - def post_save(self, *args, **kwargs): + def save(self, *args, **kwargs): """ When saving, also transfer money between two notes """ if self.source.pk == self.destination.pk: # When source == destination, no money is transfered + super().save(*args, **kwargs) return created = self.pk is None diff --git a/apps/note/views.py b/apps/note/views.py index 31a79be7..6b2cb372 100644 --- a/apps/note/views.py +++ b/apps/note/views.py @@ -9,6 +9,7 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import CreateView, ListView, UpdateView from django_tables2 import SingleTableView +from member.backends import PermissionBackend from .forms import TransactionTemplateForm from .models import Transaction, TransactionTemplate, Alias, TemplateTransaction, NoteSpecial from .models.transactions import SpecialTransaction @@ -18,16 +19,18 @@ from .tables import HistoryTable class TransactionCreate(LoginRequiredMixin, SingleTableView): """ Show transfer page - - TODO: If user have sufficient rights, they can transfer from an other note """ - queryset = Transaction.objects.order_by("-id").all()[:50] template_name = "note/transaction_form.html" # Transaction history table table_class = HistoryTable table_pagination = {"per_page": 50} + def get_queryset(self): + return Transaction.objects.filter(PermissionBackend + .filter_queryset(self.request.user, Transaction, "view")) \ + .order_by("-id").all()[:50] + def get_context_data(self, **kwargs): """ Add some context variables in template such as page title @@ -117,21 +120,26 @@ class ConsoView(LoginRequiredMixin, SingleTableView): """ Consume """ - queryset = Transaction.objects.order_by("-id").all()[:50] template_name = "note/conso_form.html" # Transaction history table table_class = HistoryTable table_pagination = {"per_page": 50} + def get_queryset(self): + return Transaction.objects.filter(PermissionBackend + .filter_queryset(self.request.user, Transaction, "view")) \ + .order_by("-id").all()[:50] + def get_context_data(self, **kwargs): """ Add some context variables in template such as page title """ context = super().get_context_data(**kwargs) from django.db.models import Count - buttons = TransactionTemplate.objects.filter(display=True) \ - .annotate(clicks=Count('templatetransaction')).order_by('category__name', 'name') + buttons = TransactionTemplate.objects.filter(PermissionBackend() + .filter_queryset(self.request.user, TransactionTemplate, "view")) \ + .filter(display=True).annotate(clicks=Count('templatetransaction')).order_by('category__name', 'name') context['transaction_templates'] = buttons context['most_used'] = buttons.order_by('-clicks', 'name')[:10] context['title'] = _("Consumptions") diff --git a/apps/permission/templatetags/__init__.py b/apps/permission/templatetags/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/apps/permission/templatetags/perms.py b/apps/permission/templatetags/perms.py new file mode 100644 index 00000000..9b5ff93a --- /dev/null +++ b/apps/permission/templatetags/perms.py @@ -0,0 +1,42 @@ +# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay +# SPDX-License-Identifier: GPL-3.0-or-later + +from django.contrib.contenttypes.models import ContentType +from django.template.defaultfilters import stringfilter + +from logs.middlewares import get_current_authenticated_user +from django import template + +from member.backends import PermissionBackend + + +def has_perm(value): + return get_current_authenticated_user().has_perm(value) + + +@stringfilter +def not_empty_model_list(model_name): + user = get_current_authenticated_user() + if user.is_superuser: + return True + spl = model_name.split(".") + ct = ContentType.objects.get(app_label=spl[0], model=spl[1]) + qs = ct.model_class().objects.filter(PermissionBackend.filter_queryset(user, ct, "view")) + return qs.exists() + + +@stringfilter +def not_empty_model_change_list(model_name): + user = get_current_authenticated_user() + if user.is_superuser: + return True + spl = model_name.split(".") + ct = ContentType.objects.get(app_label=spl[0], model=spl[1]) + qs = ct.model_class().objects.filter(PermissionBackend.filter_queryset(user, ct, "change")) + return qs.exists() + + +register = template.Library() +register.filter('has_perm', has_perm) +register.filter('not_empty_model_list', not_empty_model_list) +register.filter('not_empty_model_change_list', not_empty_model_change_list) diff --git a/templates/base.html b/templates/base.html index e6193702..fae86443 100644 --- a/templates/base.html +++ b/templates/base.html @@ -1,4 +1,4 @@ -{% load static i18n pretty_money static getenv %} +{% load static i18n pretty_money static getenv perms %} {% comment %} SPDX-License-Identifier: GPL-3.0-or-later {% endcomment %} @@ -74,21 +74,29 @@ SPDX-License-Identifier: GPL-3.0-or-later @@ -58,47 +60,49 @@ SPDX-License-Identifier: GPL-2.0-or-later -