From 551091f6501388b1962b60a438a98bdf29c3aa4a Mon Sep 17 00:00:00 2001 From: imperosol Date: Sat, 11 Jan 2025 02:22:12 +0100 Subject: [PATCH] add `PermissionOrAuthorRequiredMixin` --- com/models.py | 13 +++++++-- com/views.py | 23 ++++++++------- core/auth/mixins.py | 68 +++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 18 deletions(-) diff --git a/com/models.py b/com/models.py index 85c2b63d..1219410a 100644 --- a/com/models.py +++ b/com/models.py @@ -68,7 +68,10 @@ class NewsQuerySet(models.QuerySet): """ if user.has_perm("com.view_unmoderated_news"): return self - return self.filter(Q(is_moderated=True) | Q(author_id=user.id)) + q_filter = Q(is_moderated=True) + if user.is_authenticated: + q_filter |= Q(author_id=user.id) + return self.filter(q_filter) class News(models.Model): @@ -149,8 +152,12 @@ class News(models.Model): self.author_id == user.id or user.has_perm("com.change_news") ) - def can_be_viewed_by(self, user): - return self.is_moderated or user.has_perm("com.view_unmoderated_news") + def can_be_viewed_by(self, user: User): + return ( + self.is_moderated + or user.has_perm("com.view_unmoderated_news") + or (user.is_authenticated and self.author_id == user.id) + ) def news_notification_callback(notif): diff --git a/com/views.py b/com/views.py index 179c2bed..7d29dd62 100644 --- a/com/views.py +++ b/com/views.py @@ -44,7 +44,11 @@ from club.models import Club, Mailing from com.calendar import IcsCalendar from com.forms import NewsDateForm, NewsForm, PosterForm from com.models import News, NewsDate, Poster, Screen, Sith, Weekmail, WeekmailArticle -from core.auth.mixins import CanEditPropMixin, CanViewMixin +from core.auth.mixins import ( + CanEditPropMixin, + CanViewMixin, + PermissionOrAuthorRequiredMixin, +) from core.models import User from core.views.mixins import QuickNotifMixin, TabedViewMixin from core.views.widgets.markdown import MarkdownInput @@ -162,24 +166,19 @@ class NewsCreateView(PermissionRequiredMixin, CreateView): return init -class NewsUpdateView(UpdateView): +class NewsUpdateView(PermissionOrAuthorRequiredMixin, UpdateView): model = News form_class = NewsForm template_name = "com/news_edit.jinja" pk_url_kwarg = "news_id" - - def dispatch(self, request, *args, **kwargs): - if ( - not request.user.has_perm("com.edit_news") - and self.get_object().author != request.user - ): - raise PermissionDenied - return super().dispatch(request, *args, **kwargs) + permission_required = "com.edit_news" def form_valid(self, form): self.object = form.save() IcsCalendar.make_internal() - return super().form_valid(form) + # Don't call `super().form_valid()`, + # because it would trigger a second call to `form.save()` + return HttpResponseRedirect(self.get_success_url()) def get_date_form_kwargs(self) -> dict[str, Any]: """Get initial data for NewsDateForm""" @@ -202,7 +201,7 @@ class NewsUpdateView(UpdateView): } -class NewsDeleteView(PermissionRequiredMixin, DeleteView): +class NewsDeleteView(PermissionOrAuthorRequiredMixin, DeleteView): model = News pk_url_kwarg = "news_id" template_name = "core/delete_confirm.jinja" diff --git a/core/auth/mixins.py b/core/auth/mixins.py index b25397b0..794021d5 100644 --- a/core/auth/mixins.py +++ b/core/auth/mixins.py @@ -23,14 +23,17 @@ # import types -from typing import Any +from typing import TYPE_CHECKING, Any, LiteralString -from django.contrib.auth.mixins import AccessMixin -from django.core.exceptions import PermissionDenied +from django.contrib.auth.mixins import AccessMixin, PermissionRequiredMixin +from django.core.exceptions import ImproperlyConfigured, PermissionDenied from django.views.generic.base import View from core.models import User +if TYPE_CHECKING: + from django.db.models import Model + def can_edit_prop(obj: Any, user: User) -> bool: """Can the user edit the properties of the object. @@ -210,3 +213,62 @@ class SubscriberMixin(AccessMixin): if not request.user.is_subscribed: return self.handle_no_permission() return super().dispatch(request, *args, **kwargs) + + +class PermissionOrAuthorRequiredMixin(PermissionRequiredMixin): + """Require that the user has the required perm or is the object author. + + This mixin can be used in combination with `DetailView`, + or another base class that implements the `get_object` method. + + Example: + In the following code, a user will be able + to edit news if he has the `com.change_news` permission + or if he tries to edit his own news : + + ```python + class NewsEditView(PermissionOrAuthorRequiredMixin, DetailView): + model = News + author_field = "author" + permission_required = "com.change_news" + ``` + + This is more or less equivalent to : + + ```python + class NewsEditView(PermissionOrAuthorRequiredMixin, DetailView): + model = News + + def dispatch(self, request, *args, **kwargs): + self.object = self.get_object() + if not ( + user.has_perm("com.change_news") + or self.object.author == request.user + ): + raise PermissionDenied + return super().dispatch(request, *args, **kwargs) + ``` + """ + + author_field: LiteralString = "author" + + def has_permission(self): + if not hasattr(self, "get_object"): + raise ImproperlyConfigured( + f"{self.__class__.__name__} is missing the " + "get_object attribute. " + f"Define {self.__class__.__name__}.get_object, " + "or inherit from a class that implement it (like DetailView)" + ) + if super().has_permission(): + return True + if self.request.user.is_anonymous: + return False + obj: Model = self.get_object() + if not self.author_field.endswith("_id"): + # getting the related model could trigger a db query + # so we will rather get the foreign value than + # the object itself. + self.author_field += "_id" + author_id = getattr(obj, self.author_field, None) + return author_id == self.request.user.id