Merge pull request #1002 from ae-utbm/perms

Permissions refactor

accounting/api.py

@@ -7,7 +7,7 @@
 
 from accounting.models import ClubAccount, Company
 from accounting.schemas import ClubAccountSchema, CompanySchema
-from core.api_permissions import CanAccessLookup
+from core.auth.api_permissions import CanAccessLookup
 
 
 @api_controller("/lookup", permissions=[CanAccessLookup])

accounting/views.py

@@ -17,6 +17,7 @@
 
 from django import forms
 from django.conf import settings
+from django.contrib.auth.mixins import PermissionRequiredMixin
 from django.core.exceptions import PermissionDenied, ValidationError
 from django.db import transaction
 from django.db.models import Sum
@@ -44,15 +45,15 @@
 )
 from club.models import Club
 from club.widgets.select import AutoCompleteSelectClub
-from core.models import User
-from core.views import (
+from core.auth.mixins import (
     CanCreateMixin,
     CanEditMixin,
     CanEditPropMixin,
     CanViewMixin,
-    TabedViewMixin,
 )
+from core.models import User
 from core.views.forms import SelectDate, SelectFile
+from core.views.mixins import TabedViewMixin
 from core.views.widgets.select import AutoCompleteSelectUser
 from counter.models import Counter, Product, Selling
 
@@ -86,12 +87,13 @@ class SimplifiedAccountingTypeEditView(CanViewMixin, UpdateView):
     template_name = "core/edit.jinja"
 
 
-class SimplifiedAccountingTypeCreateView(CanCreateMixin, CreateView):
+class SimplifiedAccountingTypeCreateView(PermissionRequiredMixin, CreateView):
     """Create an accounting type (for the admins)."""
 
     model = SimplifiedAccountingType
     fields = ["label", "accounting_type"]
     template_name = "core/create.jinja"
+    permission_required = "accounting.add_simplifiedaccountingtype"
 
 
 # Accounting types
@@ -113,12 +115,13 @@ class AccountingTypeEditView(CanViewMixin, UpdateView):
     template_name = "core/edit.jinja"
 
 
-class AccountingTypeCreateView(CanCreateMixin, CreateView):
+class AccountingTypeCreateView(PermissionRequiredMixin, CreateView):
     """Create an accounting type (for the admins)."""
 
     model = AccountingType
     fields = ["code", "label", "movement_type"]
     template_name = "core/create.jinja"
+    permission_required = "accounting.add_accountingtype"
 
 
 # BankAccount views

club/api.py

@@ -7,7 +7,7 @@
 
 from club.models import Club
 from club.schemas import ClubSchema
-from core.api_permissions import CanAccessLookup
+from core.auth.api_permissions import CanAccessLookup
 
 
 @api_controller("/club")

club/views.py

@@ -25,6 +25,7 @@
 import csv
 
 from django.conf import settings
+from django.contrib.auth.mixins import PermissionRequiredMixin
 from django.core.exceptions import NON_FIELD_ERRORS, PermissionDenied, ValidationError
 from django.core.paginator import InvalidPage, Paginator
 from django.db.models import Sum
@@ -49,17 +50,15 @@
     PosterEditBaseView,
     PosterListBaseView,
 )
-from core.models import PageRev
-from core.views import (
+from core.auth.mixins import (
     CanCreateMixin,
     CanEditMixin,
     CanEditPropMixin,
     CanViewMixin,
-    DetailFormView,
-    PageEditViewBase,
-    TabedViewMixin,
-    UserIsRootMixin,
 )
+from core.models import PageRev
+from core.views import DetailFormView, PageEditViewBase
+from core.views.mixins import TabedViewMixin
 from counter.models import Selling
 
 
@@ -474,13 +473,14 @@ class ClubEditPropView(ClubTabsMixin, CanEditPropMixin, UpdateView):
     current_tab = "props"
 
 
-class ClubCreateView(CanCreateMixin, CreateView):
+class ClubCreateView(PermissionRequiredMixin, CreateView):
     """Create a club (for the Sith admin)."""
 
     model = Club
     pk_url_kwarg = "club_id"
     fields = ["name", "unix_name", "parent"]
     template_name = "core/edit.jinja"
+    permission_required = "club.add_club"
 
 
 class MembershipSetOldView(CanEditMixin, DetailView):
@@ -512,12 +512,13 @@ def post(self, request, *args, **kwargs):
         )
 
 
-class MembershipDeleteView(UserIsRootMixin, DeleteView):
+class MembershipDeleteView(PermissionRequiredMixin, DeleteView):
     """Delete a membership (for admins only)."""
 
     model = Membership
     pk_url_kwarg = "membership_id"
     template_name = "core/delete_confirm.jinja"
+    permission_required = "club.delete_membership"
 
     def get_success_url(self):
         return reverse_lazy("core:user_clubs", kwargs={"user_id": self.object.user.id})

com/models.py

@@ -68,7 +68,10 @@ def viewable_by(self, user: User) -> Self:
         """
         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 @@ def can_be_edited_by(self, user: User):
             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):

com/tests/test_views.py

@@ -159,13 +159,13 @@ def test_news_owner(self):
 
     def test_news_viewer(self):
         """Test that moderated news can be viewed by anyone
-        and not moderated news only by com admins.
+        and not moderated news only by com admins and by their author.
         """
-        # by default a news isn't moderated
+        # by default news aren't moderated
         assert self.new.can_be_viewed_by(self.com_admin)
+        assert self.new.can_be_viewed_by(self.author)
         assert not self.new.can_be_viewed_by(self.sli)
         assert not self.new.can_be_viewed_by(self.anonymous)
-        assert not self.new.can_be_viewed_by(self.author)
 
         self.new.is_moderated = True
         self.new.save()

com/views.py

@@ -27,10 +27,7 @@
 from typing import Any
 
 from django.conf import settings
-from django.contrib.auth.mixins import (
-    AccessMixin,
-    PermissionRequiredMixin,
-)
+from django.contrib.auth.mixins import AccessMixin, PermissionRequiredMixin
 from django.core.exceptions import PermissionDenied, ValidationError
 from django.db.models import Max
 from django.forms.models import modelform_factory
@@ -47,13 +44,13 @@
 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.models import User
-from core.views import (
+from core.auth.mixins import (
     CanEditPropMixin,
     CanViewMixin,
-    QuickNotifMixin,
-    TabedViewMixin,
+    PermissionOrAuthorRequiredMixin,
 )
+from core.models import User
+from core.views.mixins import QuickNotifMixin, TabedViewMixin
 from core.views.widgets.markdown import MarkdownInput
 
 # Sith object
@@ -169,24 +166,17 @@ def get_initial(self):
         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()
+        response = super().form_valid(form)  # Does the saving part
         IcsCalendar.make_internal()
-        return super().form_valid(form)
+        return response
 
     def get_date_form_kwargs(self) -> dict[str, Any]:
         """Get initial data for NewsDateForm"""
@@ -209,7 +199,7 @@ def get_form_kwargs(self):
         }
 
 
-class NewsDeleteView(PermissionRequiredMixin, DeleteView):
+class NewsDeleteView(PermissionOrAuthorRequiredMixin, DeleteView):
     model = News
     pk_url_kwarg = "news_id"
     template_name = "core/delete_confirm.jinja"

core/api.py

@@ -11,10 +11,7 @@
 from ninja_extra.schemas import PaginatedResponseSchema
 
 from club.models import Mailing
-from core.api_permissions import (
-    CanAccessLookup,
-    CanView,
-)
+from core.auth.api_permissions import CanAccessLookup, CanView
 from core.models import Group, SithFile, User
 from core.schemas import (
     FamilyGodfatherSchema,

core/api_permissions.py → core/auth/api_permissions.py

@@ -3,7 +3,8 @@
 Some permissions are global (like `IsInGroup` or `IsRoot`),
 and some others are per-object (like `CanView` or `CanEdit`).
 
-Examples:
+Example:
+    ```python
     # restrict all the routes of this controller
     # to subscribed users
     @api_controller("/foo", permissions=[IsSubscriber])
@@ -33,6 +34,7 @@ def bar_get_one(self, bar_id: int):
         ]
         def bar_delete(self, bar_id: int):
             # ...
+    ```
 """
 
 from typing import Any