diff --git a/apps/member/tables.py b/apps/member/tables.py index a9999c11..c8a510ff 100644 --- a/apps/member/tables.py +++ b/apps/member/tables.py @@ -61,7 +61,7 @@ class MembershipTable(tables.Table): def render_club(self, value): s = value.name - if PermissionBackend().has_perm(get_current_authenticated_user(), "member.view_club", value): + if PermissionBackend.check_perm(get_current_authenticated_user(), "member.view_club", value): s = format_html("{name}", url=reverse_lazy('member:club_detail', kwargs={"pk": value.pk}), name=s) @@ -86,7 +86,7 @@ class MembershipTable(tables.Table): date_end=datetime.now().date(), fee=0, ) - if PermissionBackend().has_perm(get_current_authenticated_user(), + if PermissionBackend.check_perm(get_current_authenticated_user(), "member:add_membership", empty_membership): # If the user has right t = format_html(t + ' {text}', url=reverse_lazy('member:club_renew_membership', @@ -96,7 +96,7 @@ class MembershipTable(tables.Table): def render_roles(self, record): roles = record.roles.all() s = ", ".join(str(role) for role in roles) - if PermissionBackend().has_perm(get_current_authenticated_user(), "member.change_membership_roles", record): + if PermissionBackend.check_perm(get_current_authenticated_user(), "member.change_membership_roles", record): s = format_html("" + s + "") return s diff --git a/apps/member/views.py b/apps/member/views.py index 96550e14..f695002f 100644 --- a/apps/member/views.py +++ b/apps/member/views.py @@ -299,7 +299,7 @@ class ClubDetailView(ProtectQuerysetMixin, LoginRequiredMixin, DetailView): context = super().get_context_data(**kwargs) club = context["club"] - if PermissionBackend().has_perm(self.request.user, "member.change_club_membership_start", club): + if PermissionBackend.check_perm(self.request.user, "member.change_club_membership_start", club): club.update_membership_dates() club_transactions = Transaction.objects.all().filter(Q(source=club.note) | Q(destination=club.note))\ diff --git a/apps/permission/backends.py b/apps/permission/backends.py index 4cc0dd9e..4fb7b577 100644 --- a/apps/permission/backends.py +++ b/apps/permission/backends.py @@ -114,8 +114,16 @@ class PermissionBackend(ModelBackend): query = query | perm.query return query + @staticmethod @memoize - def has_perm(self, user_obj, perm, obj=None): + def check_perm(user_obj, perm, obj=None): + """ + Check is the given user has the permission over a given object. + The result is then memoized. + Exception: for add permissions, since the object is not hashable since it doesn't have any + primary key, the result is not memoized. Moreover, the right could change + (e.g. for a transaction, the balance of the user could change) + """ if user_obj is None or isinstance(user_obj, AnonymousUser): return False @@ -134,10 +142,13 @@ class PermissionBackend(ModelBackend): perm_field = perm[2] if len(perm) == 3 else None 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)): + for permission in PermissionBackend.permissions(user_obj, ct, perm_type)): return True return False + def has_perm(self, user_obj, perm, obj=None): + return PermissionBackend.check_perm(user_obj, perm, obj) + def has_module_perms(self, user_obj, app_label): return False diff --git a/apps/permission/decorators.py b/apps/permission/decorators.py index cb8649fd..f144935a 100644 --- a/apps/permission/decorators.py +++ b/apps/permission/decorators.py @@ -49,7 +49,10 @@ def memoize(f): # lru_cache makes the job of memoization # We store only the 512 latest data per session. It has to be enough. sess_funs[sess_key] = lru_cache(512)(f) - return sess_funs[sess_key](*args, **kwargs) + try: + return sess_funs[sess_key](*args, **kwargs) + except TypeError: # For add permissions, objects are not hashable (not yet created). Don't memoize this case. + return f(*args, **kwargs) func.func_name = f.__name__ diff --git a/apps/permission/permissions.py b/apps/permission/permissions.py index 7097085f..40321567 100644 --- a/apps/permission/permissions.py +++ b/apps/permission/permissions.py @@ -44,7 +44,7 @@ class StrongDjangoObjectPermissions(DjangoObjectPermissions): perms = self.get_required_object_permissions(request.method, model_cls) # if not user.has_perms(perms, obj): - if not all(PermissionBackend().has_perm(user, perm, obj) for perm in perms): + if not all(PermissionBackend.check_perm(user, perm, obj) for perm in perms): # 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. diff --git a/apps/permission/signals.py b/apps/permission/signals.py index 4ee63ae3..bf54b72f 100644 --- a/apps/permission/signals.py +++ b/apps/permission/signals.py @@ -44,7 +44,7 @@ def pre_save_object(sender, instance, **kwargs): # 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): + if PermissionBackend.check_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 @@ -56,11 +56,11 @@ def pre_save_object(sender, instance, **kwargs): # 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): + if not PermissionBackend.check_perm(user, app_label + ".change_" + model_name + "_" + field_name, instance): raise PermissionDenied else: # We check if the user has right to add the object - has_perm = PermissionBackend().has_perm(user, app_label + ".add_" + model_name, instance) + has_perm = PermissionBackend.check_perm(user, app_label + ".add_" + model_name, instance) if not has_perm: raise PermissionDenied @@ -87,5 +87,5 @@ def pre_delete_object(instance, **kwargs): 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): + if not PermissionBackend.check_perm(user, app_label + ".delete_" + model_name, instance): raise PermissionDenied diff --git a/apps/permission/templatetags/perms.py b/apps/permission/templatetags/perms.py index 4f61fcfe..a89c7f49 100644 --- a/apps/permission/templatetags/perms.py +++ b/apps/permission/templatetags/perms.py @@ -54,7 +54,7 @@ def model_list(model_name, t="view"): def has_perm(perm, obj): - return PermissionBackend().has_perm(get_current_authenticated_user(), perm, obj) + return PermissionBackend.check_perm(get_current_authenticated_user(), perm, obj) def can_create_transaction(): @@ -77,7 +77,7 @@ def can_create_transaction(): amount=0, reason="Check permissions", ) - session["can_create_transaction"] = PermissionBackend().has_perm(user, "note.add_transaction", empty_transaction) + session["can_create_transaction"] = PermissionBackend.check_perm(user, "note.add_transaction", empty_transaction) return session.get("can_create_transaction") == 1