From 7ac41ac5cb479eae003b6a9f62b0112bb3a56a1f Mon Sep 17 00:00:00 2001 From: imperosol Date: Fri, 10 Jan 2025 15:17:41 +0100 Subject: [PATCH 1/9] remove `UserIsRootMixin` --- club/views.py | 5 +++-- core/templates/core/user_clubs.jinja | 4 ++-- core/views/__init__.py | 12 ------------ docs/tutorial/perms.md | 18 +++++++++--------- 4 files changed, 14 insertions(+), 25 deletions(-) diff --git a/club/views.py b/club/views.py index d713a1cc..eda5ac48 100644 --- a/club/views.py +++ b/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 @@ -58,7 +59,6 @@ from core.views import ( DetailFormView, PageEditViewBase, TabedViewMixin, - UserIsRootMixin, ) from counter.models import Selling @@ -512,12 +512,13 @@ class MembershipSetOldView(CanEditMixin, DetailView): ) -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}) diff --git a/core/templates/core/user_clubs.jinja b/core/templates/core/user_clubs.jinja index 324f8389..51b6827f 100644 --- a/core/templates/core/user_clubs.jinja +++ b/core/templates/core/user_clubs.jinja @@ -30,7 +30,7 @@ {% if m.can_be_edited_by(user) %} {% trans %}Mark as old{% endtrans %} {% endif %} - {% if user.is_root %} + {% if user.has_perm("club.delete_membership") %} {% trans %}Delete{% endtrans %} {% endif %} @@ -59,7 +59,7 @@ {{ m.description }} {{ m.start_date }} {{ m.end_date }} - {% if user.is_root %} + {% if user.has_perm("club.delete_membership") %} {% trans %}Delete{% endtrans %} {% endif %} diff --git a/core/views/__init__.py b/core/views/__init__.py index f72addb6..debbc810 100644 --- a/core/views/__init__.py +++ b/core/views/__init__.py @@ -225,18 +225,6 @@ class CanViewMixin(GenericContentPermissionMixinBuilder): permission_function = can_view -class UserIsRootMixin(GenericContentPermissionMixinBuilder): - """Allow only root admins. - - Raises: - PermissionDenied: if the user isn't root - """ - - @staticmethod - def permission_function(obj: Any, user: User): - return user.is_root - - class FormerSubscriberMixin(AccessMixin): """Check if the user was at least an old subscriber. diff --git a/docs/tutorial/perms.md b/docs/tutorial/perms.md index c78292ab..0100f24c 100644 --- a/docs/tutorial/perms.md +++ b/docs/tutorial/perms.md @@ -154,7 +154,7 @@ Voici un exemple d'utilisation en reprenant l'objet Article crée précédemment ```python from django.views.generic import CreateView, ListView -from core.views import CanViewMixin, CanCreateMixin +from core.auth.mixins import CanViewMixin, CanCreateMixin from com.models import WeekmailArticle @@ -172,14 +172,14 @@ class ArticlesCreateView(CanCreateMixin, CreateView): Les mixins suivants sont implémentés : -- [CanCreateMixin][core.views.CanCreateMixin] : l'utilisateur peut-il créer l'objet ? -- [CanEditPropMixin][core.views.CanEditPropMixin] : l'utilisateur peut-il éditer les propriétés de l'objet ? -- [CanEditMixin][core.views.CanEditMixin] : L'utilisateur peut-il éditer l'objet ? -- [CanViewMixin][core.views.CanViewMixin] : L'utilisateur peut-il voir l'objet ? -- [UserIsRootMixin][core.views.UserIsRootMixin] : L'utilisateur a-t-il les droit root ? -- [FormerSubscriberMixin][core.views.FormerSubscriberMixin] : L'utilisateur a-t-il déjà été cotisant ? -- [UserIsLoggedMixin][core.views.UserIsLoggedMixin] : L'utilisateur est-il connecté ? - (à éviter ; préférez `LoginRequiredMixin`, fourni par Django) +- [CanCreateMixin][core.auth.mixins.CanCreateMixin] : l'utilisateur peut-il créer l'objet ? +- [CanEditPropMixin][core.auth.mixins.CanEditPropMixin] : l'utilisateur peut-il éditer les propriétés de l'objet ? +- [CanEditMixin][core.auth.mixins.CanEditMixin] : L'utilisateur peut-il éditer l'objet ? +- [CanViewMixin][core.auth.mixins.CanViewMixin] : L'utilisateur peut-il voir l'objet ? +- [FormerSubscriberMixin][core.auth.mixins.FormerSubscriberMixin] : L'utilisateur a-t-il déjà été cotisant ? +- [PermissionOrAuthorRequiredMixin][core.auth.mixins.PermissionOrAuthorRequiredMixin] : + L'utilisateur a-t-il la permission requise, ou bien est-il l'auteur de l'objet + auquel on veut accéder ? !!!danger "Performance" From cba915c34d36415780c42ba90e01faa91c2c4474 Mon Sep 17 00:00:00 2001 From: imperosol Date: Fri, 10 Jan 2025 15:58:22 +0100 Subject: [PATCH 2/9] Move core views mixins to their own file --- accounting/views.py | 9 ++---- club/views.py | 2 +- com/views.py | 13 ++------ core/views/__init__.py | 71 +---------------------------------------- core/views/files.py | 2 +- core/views/mixins.py | 67 ++++++++++++++++++++++++++++++++++++++ core/views/user.py | 9 ++---- counter/views/mixins.py | 2 +- pedagogy/views.py | 1 - trombi/views.py | 10 ++---- 10 files changed, 80 insertions(+), 106 deletions(-) create mode 100644 core/views/mixins.py diff --git a/accounting/views.py b/accounting/views.py index ce0ae45b..dd5d2e09 100644 --- a/accounting/views.py +++ b/accounting/views.py @@ -45,14 +45,9 @@ from accounting.widgets.select import ( from club.models import Club from club.widgets.select import AutoCompleteSelectClub from core.models import User -from core.views import ( - CanCreateMixin, - CanEditMixin, - CanEditPropMixin, - CanViewMixin, - TabedViewMixin, -) +from core.views import CanCreateMixin, CanEditMixin, CanEditPropMixin, CanViewMixin 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 diff --git a/club/views.py b/club/views.py index eda5ac48..a6b6d009 100644 --- a/club/views.py +++ b/club/views.py @@ -58,8 +58,8 @@ from core.views import ( CanViewMixin, DetailFormView, PageEditViewBase, - TabedViewMixin, ) +from core.views.mixins import TabedViewMixin from counter.models import Selling diff --git a/com/views.py b/com/views.py index 54e37578..9216cb00 100644 --- a/com/views.py +++ b/com/views.py @@ -27,10 +27,7 @@ from smtplib import SMTPRecipientsRefused 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 @@ -48,12 +45,8 @@ 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 ( - CanEditPropMixin, - CanViewMixin, - QuickNotifMixin, - TabedViewMixin, -) +from core.views import CanEditPropMixin, CanViewMixin, QuickNotifMixin, TabedViewMixin +from core.views.mixins import QuickNotifMixin, TabedViewMixin from core.views.widgets.markdown import MarkdownInput # Sith object diff --git a/core/views/__init__.py b/core/views/__init__.py index debbc810..0cff8afe 100644 --- a/core/views/__init__.py +++ b/core/views/__init__.py @@ -25,18 +25,13 @@ import types from typing import Any -from django.conf import settings from django.contrib.auth.mixins import AccessMixin -from django.core.exceptions import ( - ImproperlyConfigured, - PermissionDenied, -) +from django.core.exceptions import PermissionDenied from django.http import ( HttpResponseForbidden, HttpResponseNotFound, HttpResponseServerError, ) -from django.shortcuts import render from django.utils.functional import cached_property from django.views.generic.base import View from django.views.generic.detail import SingleObjectMixin @@ -245,62 +240,6 @@ class SubscriberMixin(AccessMixin): return super().dispatch(request, *args, **kwargs) -class TabedViewMixin(View): - """Basic functions for displaying tabs in the template.""" - - def get_tabs_title(self): - if hasattr(self, "tabs_title"): - return self.tabs_title - raise ImproperlyConfigured("tabs_title is required") - - def get_current_tab(self): - if hasattr(self, "current_tab"): - return self.current_tab - raise ImproperlyConfigured("current_tab is required") - - def get_list_of_tabs(self): - if hasattr(self, "list_of_tabs"): - return self.list_of_tabs - raise ImproperlyConfigured("list_of_tabs is required") - - def get_context_data(self, **kwargs): - kwargs = super().get_context_data(**kwargs) - kwargs["tabs_title"] = self.get_tabs_title() - kwargs["current_tab"] = self.get_current_tab() - kwargs["list_of_tabs"] = self.get_list_of_tabs() - return kwargs - - -class QuickNotifMixin: - quick_notif_list = [] - - def dispatch(self, request, *arg, **kwargs): - # In some cases, the class can stay instanciated, so we need to reset the list - self.quick_notif_list = [] - return super().dispatch(request, *arg, **kwargs) - - def get_success_url(self): - ret = super().get_success_url() - if hasattr(self, "quick_notif_url_arg"): - if "?" in ret: - ret += "&" + self.quick_notif_url_arg - else: - ret += "?" + self.quick_notif_url_arg - return ret - - def get_context_data(self, **kwargs): - """Add quick notifications to context.""" - kwargs = super().get_context_data(**kwargs) - kwargs["quick_notifs"] = [] - for n in self.quick_notif_list: - kwargs["quick_notifs"].append(settings.SITH_QUICK_NOTIF[n]) - for key, val in settings.SITH_QUICK_NOTIF.items(): - for gk in self.request.GET: - if key == gk: - kwargs["quick_notifs"].append(val) - return kwargs - - class DetailFormView(SingleObjectMixin, FormView): """Class that allow both a detail view and a form view.""" @@ -314,14 +253,6 @@ class DetailFormView(SingleObjectMixin, FormView): return super().get_object() -class AllowFragment: - """Add `is_fragment` to templates. It's only True if the request is emitted by htmx""" - - def get_context_data(self, **kwargs): - kwargs["is_fragment"] = self.request.headers.get("HX-Request", False) - return super().get_context_data(**kwargs) - - # F403: those star-imports would be hellish to refactor # E402: putting those import at the top of the file would also be difficult from .files import * # noqa: F403 E402 diff --git a/core/views/files.py b/core/views/files.py index d5ffabb6..e2ccaa6b 100644 --- a/core/views/files.py +++ b/core/views/files.py @@ -35,12 +35,12 @@ from django.views.generic.edit import DeleteView, FormMixin, UpdateView from core.models import Notification, SithFile, User from core.views import ( - AllowFragment, CanEditMixin, CanEditPropMixin, CanViewMixin, can_view, ) +from core.views.mixins import AllowFragment from core.views.widgets.select import ( AutoCompleteSelectMultipleGroup, AutoCompleteSelectSithFile, diff --git a/core/views/mixins.py b/core/views/mixins.py new file mode 100644 index 00000000..9687f5d9 --- /dev/null +++ b/core/views/mixins.py @@ -0,0 +1,67 @@ +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured +from django.views import View + + +class TabedViewMixin(View): + """Basic functions for displaying tabs in the template.""" + + def get_tabs_title(self): + if hasattr(self, "tabs_title"): + return self.tabs_title + raise ImproperlyConfigured("tabs_title is required") + + def get_current_tab(self): + if hasattr(self, "current_tab"): + return self.current_tab + raise ImproperlyConfigured("current_tab is required") + + def get_list_of_tabs(self): + if hasattr(self, "list_of_tabs"): + return self.list_of_tabs + raise ImproperlyConfigured("list_of_tabs is required") + + def get_context_data(self, **kwargs): + kwargs = super().get_context_data(**kwargs) + kwargs["tabs_title"] = self.get_tabs_title() + kwargs["current_tab"] = self.get_current_tab() + kwargs["list_of_tabs"] = self.get_list_of_tabs() + return kwargs + + +class QuickNotifMixin: + quick_notif_list = [] + + def dispatch(self, request, *arg, **kwargs): + # In some cases, the class can stay instanciated, so we need to reset the list + self.quick_notif_list = [] + return super().dispatch(request, *arg, **kwargs) + + def get_success_url(self): + ret = super().get_success_url() + if hasattr(self, "quick_notif_url_arg"): + if "?" in ret: + ret += "&" + self.quick_notif_url_arg + else: + ret += "?" + self.quick_notif_url_arg + return ret + + def get_context_data(self, **kwargs): + """Add quick notifications to context.""" + kwargs = super().get_context_data(**kwargs) + kwargs["quick_notifs"] = [] + for n in self.quick_notif_list: + kwargs["quick_notifs"].append(settings.SITH_QUICK_NOTIF[n]) + for key, val in settings.SITH_QUICK_NOTIF.items(): + for gk in self.request.GET: + if key == gk: + kwargs["quick_notifs"].append(val) + return kwargs + + +class AllowFragment: + """Add `is_fragment` to templates. It's only True if the request is emitted by htmx""" + + def get_context_data(self, **kwargs): + kwargs["is_fragment"] = self.request.headers.get("HX-Request", False) + return super().get_context_data(**kwargs) diff --git a/core/views/user.py b/core/views/user.py index 264a8dd6..38966900 100644 --- a/core/views/user.py +++ b/core/views/user.py @@ -55,13 +55,7 @@ from django.views.generic.edit import FormView, UpdateView from honeypot.decorators import check_honeypot from core.models import Gift, Preferences, User -from core.views import ( - CanEditMixin, - CanEditPropMixin, - CanViewMixin, - QuickNotifMixin, - TabedViewMixin, -) +from core.views import CanEditMixin, CanEditPropMixin, CanViewMixin from core.views.forms import ( GiftForm, LoginForm, @@ -70,6 +64,7 @@ from core.views.forms import ( UserGroupsForm, UserProfileForm, ) +from core.views.mixins import QuickNotifMixin, TabedViewMixin from counter.models import Refilling, Selling from counter.views.student_card import StudentCardFormView from eboutic.models import Invoice diff --git a/counter/views/mixins.py b/counter/views/mixins.py index b72d6694..5c01392a 100644 --- a/counter/views/mixins.py +++ b/counter/views/mixins.py @@ -19,7 +19,7 @@ from django.urls import reverse_lazy from django.utils.translation import gettext_lazy as _ from django.views.generic.base import View -from core.views import TabedViewMixin +from core.views.mixins import TabedViewMixin class CounterAdminMixin(View): diff --git a/pedagogy/views.py b/pedagogy/views.py index 99dd8168..c7ef0297 100644 --- a/pedagogy/views.py +++ b/pedagogy/views.py @@ -39,7 +39,6 @@ from core.models import Notification, User from core.views import ( CanCreateMixin, CanEditPropMixin, - CanViewMixin, DetailFormView, FormerSubscriberMixin, ) diff --git a/trombi/views.py b/trombi/views.py index c5a1d205..398b4dee 100644 --- a/trombi/views.py +++ b/trombi/views.py @@ -39,15 +39,9 @@ from django.views.generic.edit import CreateView, DeleteView, UpdateView from club.models import Club from core.models import User -from core.views import ( - CanCreateMixin, - CanEditMixin, - CanEditPropMixin, - CanViewMixin, - QuickNotifMixin, - TabedViewMixin, -) +from core.views import CanCreateMixin, CanEditMixin, CanEditPropMixin, CanViewMixin from core.views.forms import SelectDate +from core.views.mixins import QuickNotifMixin, TabedViewMixin from core.views.widgets.select import AutoCompleteSelectUser from trombi.models import Trombi, TrombiClubMembership, TrombiComment, TrombiUser From 0c01ad1770232349ba2e71b092f348fc1e2f70e1 Mon Sep 17 00:00:00 2001 From: imperosol Date: Fri, 10 Jan 2025 21:37:12 +0100 Subject: [PATCH 3/9] Move core auth mixins to their own file --- accounting/api.py | 2 +- accounting/views.py | 7 +- club/api.py | 2 +- club/views.py | 7 +- com/views.py | 2 +- core/api.py | 5 +- core/auth/__init__.py | 0 core/{ => auth}/api_permissions.py | 0 core/{auth_backends.py => auth/backends.py} | 0 core/auth/mixins.py | 212 ++++++++++++++++++++ core/views/__init__.py | 188 +---------------- core/views/files.py | 4 +- core/views/group.py | 3 +- core/views/page.py | 7 +- core/views/user.py | 2 +- counter/api.py | 2 +- counter/views/admin.py | 2 +- counter/views/cash.py | 2 +- counter/views/click.py | 2 +- counter/views/eticket.py | 2 +- counter/views/home.py | 2 +- counter/views/student_card.py | 2 +- docs/reference/core/api_permissions.md | 2 +- election/views.py | 2 +- forum/views.py | 2 +- galaxy/views.py | 7 +- launderette/views.py | 7 +- matmat/views.py | 3 +- pedagogy/api.py | 2 +- pedagogy/views.py | 9 +- sas/api.py | 2 +- sas/views.py | 2 +- sith/settings.py | 9 +- trombi/views.py | 7 +- 34 files changed, 274 insertions(+), 235 deletions(-) create mode 100644 core/auth/__init__.py rename core/{ => auth}/api_permissions.py (100%) rename core/{auth_backends.py => auth/backends.py} (100%) create mode 100644 core/auth/mixins.py diff --git a/accounting/api.py b/accounting/api.py index a16fb7ab..5ba6c12d 100644 --- a/accounting/api.py +++ b/accounting/api.py @@ -7,7 +7,7 @@ from ninja_extra.schemas import PaginatedResponseSchema 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]) diff --git a/accounting/views.py b/accounting/views.py index dd5d2e09..d379b60d 100644 --- a/accounting/views.py +++ b/accounting/views.py @@ -44,8 +44,13 @@ from accounting.widgets.select import ( ) from club.models import Club from club.widgets.select import AutoCompleteSelectClub +from core.auth.mixins import ( + CanCreateMixin, + CanEditMixin, + CanEditPropMixin, + CanViewMixin, +) from core.models import User -from core.views import CanCreateMixin, CanEditMixin, CanEditPropMixin, CanViewMixin from core.views.forms import SelectDate, SelectFile from core.views.mixins import TabedViewMixin from core.views.widgets.select import AutoCompleteSelectUser diff --git a/club/api.py b/club/api.py index 9a680154..2ad0f5c8 100644 --- a/club/api.py +++ b/club/api.py @@ -7,7 +7,7 @@ from ninja_extra.schemas import PaginatedResponseSchema 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") diff --git a/club/views.py b/club/views.py index a6b6d009..498e16e1 100644 --- a/club/views.py +++ b/club/views.py @@ -50,15 +50,14 @@ from com.views import ( PosterEditBaseView, PosterListBaseView, ) -from core.models import PageRev -from core.views import ( +from core.auth.mixins import ( CanCreateMixin, CanEditMixin, CanEditPropMixin, CanViewMixin, - DetailFormView, - PageEditViewBase, ) +from core.models import PageRev +from core.views import DetailFormView, PageEditViewBase from core.views.mixins import TabedViewMixin from counter.models import Selling diff --git a/com/views.py b/com/views.py index 9216cb00..179c2bed 100644 --- a/com/views.py +++ b/com/views.py @@ -44,8 +44,8 @@ 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.models import User -from core.views import CanEditPropMixin, CanViewMixin, QuickNotifMixin, TabedViewMixin from core.views.mixins import QuickNotifMixin, TabedViewMixin from core.views.widgets.markdown import MarkdownInput diff --git a/core/api.py b/core/api.py index 1662cb84..e1b3bbbd 100644 --- a/core/api.py +++ b/core/api.py @@ -11,10 +11,7 @@ from ninja_extra.pagination import PageNumberPaginationExtra 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, diff --git a/core/auth/__init__.py b/core/auth/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/core/api_permissions.py b/core/auth/api_permissions.py similarity index 100% rename from core/api_permissions.py rename to core/auth/api_permissions.py diff --git a/core/auth_backends.py b/core/auth/backends.py similarity index 100% rename from core/auth_backends.py rename to core/auth/backends.py diff --git a/core/auth/mixins.py b/core/auth/mixins.py new file mode 100644 index 00000000..b25397b0 --- /dev/null +++ b/core/auth/mixins.py @@ -0,0 +1,212 @@ +# +# Copyright 2016,2017 +# - Skia +# - Sli +# +# Ce fichier fait partie du site de l'Association des Étudiants de l'UTBM, +# http://ae.utbm.fr. +# +# This program is free software; you can redistribute it and/or modify it under +# the terms of the GNU General Public License a published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, write to the Free Sofware Foundation, Inc., 59 Temple +# Place - Suite 330, Boston, MA 02111-1307, USA. +# +# + +import types +from typing import Any + +from django.contrib.auth.mixins import AccessMixin +from django.core.exceptions import PermissionDenied +from django.views.generic.base import View + +from core.models import User + + +def can_edit_prop(obj: Any, user: User) -> bool: + """Can the user edit the properties of the object. + + Args: + obj: Object to test for permission + user: core.models.User to test permissions against + + Returns: + True if user is authorized to edit object properties else False + + Examples: + ```python + if not can_edit_prop(self.object ,request.user): + raise PermissionDenied + ``` + """ + return obj is None or user.is_owner(obj) + + +def can_edit(obj: Any, user: User) -> bool: + """Can the user edit the object. + + Args: + obj: Object to test for permission + user: core.models.User to test permissions against + + Returns: + True if user is authorized to edit object else False + + Examples: + ```python + if not can_edit(self.object, request.user): + raise PermissionDenied + ``` + """ + if obj is None or user.can_edit(obj): + return True + return can_edit_prop(obj, user) + + +def can_view(obj: Any, user: User) -> bool: + """Can the user see the object. + + Args: + obj: Object to test for permission + user: core.models.User to test permissions against + + Returns: + True if user is authorized to see object else False + + Examples: + ```python + if not can_view(self.object ,request.user): + raise PermissionDenied + ``` + """ + if obj is None or user.can_view(obj): + return True + return can_edit(obj, user) + + +class GenericContentPermissionMixinBuilder(View): + """Used to build permission mixins. + + This view protect any child view that would be showing an object that is restricted based + on two properties. + + Attributes: + raised_error: permission to be raised + """ + + raised_error = PermissionDenied + + @staticmethod + def permission_function(obj: Any, user: User) -> bool: + """Function to test permission with.""" + return False + + @classmethod + def get_permission_function(cls, obj, user): + return cls.permission_function(obj, user) + + def dispatch(self, request, *arg, **kwargs): + if hasattr(self, "get_object") and callable(self.get_object): + self.object = self.get_object() + if not self.get_permission_function(self.object, request.user): + raise self.raised_error + return super().dispatch(request, *arg, **kwargs) + + # If we get here, it's a ListView + + queryset = self.get_queryset() + l_id = [o.id for o in queryset if self.get_permission_function(o, request.user)] + if not l_id and queryset.count() != 0: + raise self.raised_error + self._get_queryset = self.get_queryset + + def get_qs(self2): + return self2._get_queryset().filter(id__in=l_id) + + self.get_queryset = types.MethodType(get_qs, self) + return super().dispatch(request, *arg, **kwargs) + + +class CanCreateMixin(View): + """Protect any child view that would create an object. + + Raises: + PermissionDenied: + If the user has not the necessary permission + to create the object of the view. + """ + + def dispatch(self, request, *arg, **kwargs): + res = super().dispatch(request, *arg, **kwargs) + if not request.user.is_authenticated: + raise PermissionDenied + return res + + def form_valid(self, form): + obj = form.instance + if can_edit_prop(obj, self.request.user): + return super().form_valid(form) + raise PermissionDenied + + +class CanEditPropMixin(GenericContentPermissionMixinBuilder): + """Ensure the user has owner permissions on the child view object. + + In other word, you can make a view with this view as parent, + and it will be retricted to the users that are in the + object's owner_group or that pass the `obj.can_be_viewed_by` test. + + Raises: + PermissionDenied: If the user cannot see the object + """ + + permission_function = can_edit_prop + + +class CanEditMixin(GenericContentPermissionMixinBuilder): + """Ensure the user has permission to edit this view's object. + + Raises: + PermissionDenied: if the user cannot edit this view's object. + """ + + permission_function = can_edit + + +class CanViewMixin(GenericContentPermissionMixinBuilder): + """Ensure the user has permission to view this view's object. + + Raises: + PermissionDenied: if the user cannot edit this view's object. + """ + + permission_function = can_view + + +class FormerSubscriberMixin(AccessMixin): + """Check if the user was at least an old subscriber. + + Raises: + PermissionDenied: if the user never subscribed. + """ + + def dispatch(self, request, *args, **kwargs): + if not request.user.was_subscribed: + raise PermissionDenied + return super().dispatch(request, *args, **kwargs) + + +class SubscriberMixin(AccessMixin): + def dispatch(self, request, *args, **kwargs): + if not request.user.is_subscribed: + return self.handle_no_permission() + return super().dispatch(request, *args, **kwargs) diff --git a/core/views/__init__.py b/core/views/__init__.py index 0cff8afe..c8152f78 100644 --- a/core/views/__init__.py +++ b/core/views/__init__.py @@ -22,23 +22,17 @@ # # -import types -from typing import Any - -from django.contrib.auth.mixins import AccessMixin -from django.core.exceptions import PermissionDenied from django.http import ( HttpResponseForbidden, HttpResponseNotFound, HttpResponseServerError, ) +from django.shortcuts import render from django.utils.functional import cached_property -from django.views.generic.base import View from django.views.generic.detail import SingleObjectMixin from django.views.generic.edit import FormView from sentry_sdk import last_event_id -from core.models import User from core.views.forms import LoginForm @@ -60,186 +54,6 @@ def internal_servor_error(request): return HttpResponseServerError(render(request, "core/500.jinja")) -def can_edit_prop(obj: Any, user: User) -> bool: - """Can the user edit the properties of the object. - - Args: - obj: Object to test for permission - user: core.models.User to test permissions against - - Returns: - True if user is authorized to edit object properties else False - - Examples: - ```python - if not can_edit_prop(self.object ,request.user): - raise PermissionDenied - ``` - """ - return obj is None or user.is_owner(obj) - - -def can_edit(obj: Any, user: User) -> bool: - """Can the user edit the object. - - Args: - obj: Object to test for permission - user: core.models.User to test permissions against - - Returns: - True if user is authorized to edit object else False - - Examples: - ```python - if not can_edit(self.object, request.user): - raise PermissionDenied - ``` - """ - if obj is None or user.can_edit(obj): - return True - return can_edit_prop(obj, user) - - -def can_view(obj: Any, user: User) -> bool: - """Can the user see the object. - - Args: - obj: Object to test for permission - user: core.models.User to test permissions against - - Returns: - True if user is authorized to see object else False - - Examples: - ```python - if not can_view(self.object ,request.user): - raise PermissionDenied - ``` - """ - if obj is None or user.can_view(obj): - return True - return can_edit(obj, user) - - -class GenericContentPermissionMixinBuilder(View): - """Used to build permission mixins. - - This view protect any child view that would be showing an object that is restricted based - on two properties. - - Attributes: - raised_error: permission to be raised - """ - - raised_error = PermissionDenied - - @staticmethod - def permission_function(obj: Any, user: User) -> bool: - """Function to test permission with.""" - return False - - @classmethod - def get_permission_function(cls, obj, user): - return cls.permission_function(obj, user) - - def dispatch(self, request, *arg, **kwargs): - if hasattr(self, "get_object") and callable(self.get_object): - self.object = self.get_object() - if not self.get_permission_function(self.object, request.user): - raise self.raised_error - return super().dispatch(request, *arg, **kwargs) - - # If we get here, it's a ListView - - queryset = self.get_queryset() - l_id = [o.id for o in queryset if self.get_permission_function(o, request.user)] - if not l_id and queryset.count() != 0: - raise self.raised_error - self._get_queryset = self.get_queryset - - def get_qs(self2): - return self2._get_queryset().filter(id__in=l_id) - - self.get_queryset = types.MethodType(get_qs, self) - return super().dispatch(request, *arg, **kwargs) - - -class CanCreateMixin(View): - """Protect any child view that would create an object. - - Raises: - PermissionDenied: - If the user has not the necessary permission - to create the object of the view. - """ - - def dispatch(self, request, *arg, **kwargs): - res = super().dispatch(request, *arg, **kwargs) - if not request.user.is_authenticated: - raise PermissionDenied - return res - - def form_valid(self, form): - obj = form.instance - if can_edit_prop(obj, self.request.user): - return super().form_valid(form) - raise PermissionDenied - - -class CanEditPropMixin(GenericContentPermissionMixinBuilder): - """Ensure the user has owner permissions on the child view object. - - In other word, you can make a view with this view as parent, - and it will be retricted to the users that are in the - object's owner_group or that pass the `obj.can_be_viewed_by` test. - - Raises: - PermissionDenied: If the user cannot see the object - """ - - permission_function = can_edit_prop - - -class CanEditMixin(GenericContentPermissionMixinBuilder): - """Ensure the user has permission to edit this view's object. - - Raises: - PermissionDenied: if the user cannot edit this view's object. - """ - - permission_function = can_edit - - -class CanViewMixin(GenericContentPermissionMixinBuilder): - """Ensure the user has permission to view this view's object. - - Raises: - PermissionDenied: if the user cannot edit this view's object. - """ - - permission_function = can_view - - -class FormerSubscriberMixin(AccessMixin): - """Check if the user was at least an old subscriber. - - Raises: - PermissionDenied: if the user never subscribed. - """ - - def dispatch(self, request, *args, **kwargs): - if not request.user.was_subscribed: - raise PermissionDenied - return super().dispatch(request, *args, **kwargs) - - -class SubscriberMixin(AccessMixin): - def dispatch(self, request, *args, **kwargs): - if not request.user.is_subscribed: - return self.handle_no_permission() - return super().dispatch(request, *args, **kwargs) - - class DetailFormView(SingleObjectMixin, FormView): """Class that allow both a detail view and a form view.""" diff --git a/core/views/files.py b/core/views/files.py index e2ccaa6b..04498d5c 100644 --- a/core/views/files.py +++ b/core/views/files.py @@ -33,13 +33,13 @@ from django.views.generic import DetailView, ListView from django.views.generic.detail import SingleObjectMixin from django.views.generic.edit import DeleteView, FormMixin, UpdateView -from core.models import Notification, SithFile, User -from core.views import ( +from core.auth.mixins import ( CanEditMixin, CanEditPropMixin, CanViewMixin, can_view, ) +from core.models import Notification, SithFile, User from core.views.mixins import AllowFragment from core.views.widgets.select import ( AutoCompleteSelectMultipleGroup, diff --git a/core/views/group.py b/core/views/group.py index 978fe686..afd6874d 100644 --- a/core/views/group.py +++ b/core/views/group.py @@ -21,8 +21,9 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import ListView from django.views.generic.edit import CreateView, DeleteView, UpdateView +from core.auth.mixins import CanCreateMixin, CanEditMixin from core.models import Group, User -from core.views import CanCreateMixin, CanEditMixin, DetailFormView +from core.views import DetailFormView from core.views.widgets.select import AutoCompleteSelectMultipleUser # Forms diff --git a/core/views/page.py b/core/views/page.py index 51a0e1a5..f4b04f9c 100644 --- a/core/views/page.py +++ b/core/views/page.py @@ -21,8 +21,13 @@ from django.urls import reverse_lazy from django.views.generic import DetailView, ListView from django.views.generic.edit import CreateView, DeleteView, UpdateView +from core.auth.mixins import ( + CanCreateMixin, + CanEditMixin, + CanEditPropMixin, + CanViewMixin, +) from core.models import LockError, Page, PageRev -from core.views import CanCreateMixin, CanEditMixin, CanEditPropMixin, CanViewMixin from core.views.forms import PageForm, PagePropForm from core.views.widgets.markdown import MarkdownInput diff --git a/core/views/user.py b/core/views/user.py index 38966900..5647d720 100644 --- a/core/views/user.py +++ b/core/views/user.py @@ -54,8 +54,8 @@ from django.views.generic.dates import MonthMixin, YearMixin from django.views.generic.edit import FormView, UpdateView from honeypot.decorators import check_honeypot +from core.auth.mixins import CanEditMixin, CanEditPropMixin, CanViewMixin from core.models import Gift, Preferences, User -from core.views import CanEditMixin, CanEditPropMixin, CanViewMixin from core.views.forms import ( GiftForm, LoginForm, diff --git a/counter/api.py b/counter/api.py index dd7b75f0..44b58488 100644 --- a/counter/api.py +++ b/counter/api.py @@ -20,7 +20,7 @@ from ninja_extra import ControllerBase, api_controller, paginate, route from ninja_extra.pagination import PageNumberPaginationExtra from ninja_extra.schemas import PaginatedResponseSchema -from core.api_permissions import CanAccessLookup, CanView, IsInGroup, IsRoot +from core.auth.api_permissions import CanAccessLookup, CanView, IsInGroup, IsRoot from counter.models import Counter, Product, ProductType from counter.schemas import ( CounterFilterSchema, diff --git a/counter/views/admin.py b/counter/views/admin.py index ab46581b..ffe81ea0 100644 --- a/counter/views/admin.py +++ b/counter/views/admin.py @@ -24,8 +24,8 @@ from django.utils import timezone from django.views.generic import DetailView, ListView, TemplateView from django.views.generic.edit import CreateView, DeleteView, UpdateView +from core.auth.mixins import CanEditMixin, CanViewMixin from core.utils import get_semester_code, get_start_of_semester -from core.views import CanEditMixin, CanViewMixin from counter.forms import CounterEditForm, ProductEditForm from counter.models import Counter, Product, ProductType, Refilling, Selling from counter.utils import is_logged_in_counter diff --git a/counter/views/cash.py b/counter/views/cash.py index d4a03af1..711f6864 100644 --- a/counter/views/cash.py +++ b/counter/views/cash.py @@ -23,7 +23,7 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView, ListView from django.views.generic.edit import UpdateView -from core.views import CanViewMixin +from core.auth.mixins import CanViewMixin from counter.forms import CashSummaryFormBase from counter.models import ( CashRegisterSummary, diff --git a/counter/views/click.py b/counter/views/click.py index c1815dec..4a1e1c88 100644 --- a/counter/views/click.py +++ b/counter/views/click.py @@ -31,9 +31,9 @@ from django.views.generic import FormView from django.views.generic.detail import SingleObjectMixin from ninja.main import HttpRequest +from core.auth.mixins import CanViewMixin from core.models import User from core.utils import FormFragmentTemplateData -from core.views import CanViewMixin from counter.forms import RefillForm from counter.models import Counter, Customer, Product, Selling from counter.utils import is_logged_in_counter diff --git a/counter/views/eticket.py b/counter/views/eticket.py index a05020d6..c5b4b872 100644 --- a/counter/views/eticket.py +++ b/counter/views/eticket.py @@ -18,7 +18,7 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView, ListView from django.views.generic.edit import CreateView, UpdateView -from core.views import CanViewMixin +from core.auth.mixins import CanViewMixin from counter.forms import EticketForm from counter.models import Eticket, Selling from counter.views.mixins import CounterAdminMixin, CounterAdminTabsMixin diff --git a/counter/views/home.py b/counter/views/home.py index d66b0969..0f0cd6dd 100644 --- a/counter/views/home.py +++ b/counter/views/home.py @@ -22,7 +22,7 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView from django.views.generic.edit import FormMixin, ProcessFormView -from core.views import CanViewMixin +from core.auth.mixins import CanViewMixin from core.views.forms import LoginForm from counter.forms import GetUserForm from counter.models import Counter diff --git a/counter/views/student_card.py b/counter/views/student_card.py index f916260b..6e2a3358 100644 --- a/counter/views/student_card.py +++ b/counter/views/student_card.py @@ -21,8 +21,8 @@ from django.urls import reverse from django.utils.translation import gettext as _ from django.views.generic.edit import DeleteView, FormView +from core.auth.mixins import can_edit from core.utils import FormFragmentTemplateData -from core.views import can_edit from counter.forms import StudentCardForm from counter.models import Customer, StudentCard from counter.utils import is_logged_in_counter diff --git a/docs/reference/core/api_permissions.md b/docs/reference/core/api_permissions.md index 2d693fca..4ab3a2e0 100644 --- a/docs/reference/core/api_permissions.md +++ b/docs/reference/core/api_permissions.md @@ -1 +1 @@ -::: core.api_permissions \ No newline at end of file +::: core.auth.api_permissions \ No newline at end of file diff --git a/election/views.py b/election/views.py index 422205fd..1b5439f3 100644 --- a/election/views.py +++ b/election/views.py @@ -10,7 +10,7 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView, ListView from django.views.generic.edit import CreateView, DeleteView, FormView, UpdateView -from core.views import CanCreateMixin, CanEditMixin, CanViewMixin +from core.auth.mixins import CanCreateMixin, CanEditMixin, CanViewMixin from core.views.forms import SelectDateTime from core.views.widgets.markdown import MarkdownInput from core.views.widgets.select import ( diff --git a/forum/views.py b/forum/views.py index 074f496d..9501cf1b 100644 --- a/forum/views.py +++ b/forum/views.py @@ -43,7 +43,7 @@ from haystack.query import RelatedSearchQuerySet from honeypot.decorators import check_honeypot from club.widgets.select import AutoCompleteSelectClub -from core.views import ( +from core.auth.mixins import ( CanCreateMixin, CanEditMixin, CanEditPropMixin, diff --git a/galaxy/views.py b/galaxy/views.py index fe27f978..cb116d02 100644 --- a/galaxy/views.py +++ b/galaxy/views.py @@ -27,12 +27,9 @@ from django.http import Http404, JsonResponse from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView, View +from core.auth.mixins import CanViewMixin, FormerSubscriberMixin from core.models import User -from core.views import ( - CanViewMixin, - FormerSubscriberMixin, - UserTabsMixin, -) +from core.views import UserTabsMixin from galaxy.models import Galaxy, GalaxyLane diff --git a/launderette/views.py b/launderette/views.py index 7886d1e7..be2bfceb 100644 --- a/launderette/views.py +++ b/launderette/views.py @@ -28,8 +28,13 @@ from django.views.generic import DetailView, ListView, TemplateView from django.views.generic.edit import BaseFormView, CreateView, DeleteView, UpdateView from club.models import Club +from core.auth.mixins import ( + CanCreateMixin, + CanEditMixin, + CanEditPropMixin, + CanViewMixin, +) from core.models import Page, User -from core.views import CanCreateMixin, CanEditMixin, CanEditPropMixin, CanViewMixin from counter.forms import GetUserForm from counter.models import Counter, Customer, Selling from launderette.models import Launderette, Machine, Slot, Token diff --git a/matmat/views.py b/matmat/views.py index 47840c2d..1f037234 100644 --- a/matmat/views.py +++ b/matmat/views.py @@ -32,8 +32,9 @@ from django.views.generic.detail import SingleObjectMixin from django.views.generic.edit import FormView from phonenumber_field.widgets import RegionalPhoneNumberWidget +from core.auth.mixins import FormerSubscriberMixin from core.models import User -from core.views import FormerSubscriberMixin, search_user +from core.views import search_user from core.views.forms import SelectDate # Enum to select search type diff --git a/pedagogy/api.py b/pedagogy/api.py index d7a8d457..68e3d5e2 100644 --- a/pedagogy/api.py +++ b/pedagogy/api.py @@ -7,7 +7,7 @@ from ninja_extra import ControllerBase, api_controller, paginate, route from ninja_extra.exceptions import NotFound from ninja_extra.pagination import PageNumberPaginationExtra, PaginatedResponseSchema -from core.api_permissions import IsInGroup, IsRoot, IsSubscriber +from core.auth.api_permissions import IsInGroup, IsRoot, IsSubscriber from pedagogy.models import UV from pedagogy.schemas import SimpleUvSchema, UvFilterSchema, UvSchema from pedagogy.utbm_api import find_uv diff --git a/pedagogy/views.py b/pedagogy/views.py index c7ef0297..770dc2a4 100644 --- a/pedagogy/views.py +++ b/pedagogy/views.py @@ -35,13 +35,14 @@ from django.views.generic import ( UpdateView, ) -from core.models import Notification, User -from core.views import ( +from core.auth.mixins import ( CanCreateMixin, CanEditPropMixin, - DetailFormView, + CanViewMixin, FormerSubscriberMixin, ) +from core.models import Notification, User +from core.views import DetailFormView from pedagogy.forms import ( UVCommentForm, UVCommentModerationForm, @@ -50,8 +51,6 @@ from pedagogy.forms import ( ) from pedagogy.models import UV, UVComment, UVCommentReport -# Acutal views - class UVDetailFormView(CanViewMixin, DetailFormView): """Display every comment of an UV and detailed infos about it. diff --git a/sas/api.py b/sas/api.py index 6a25607a..96bafb87 100644 --- a/sas/api.py +++ b/sas/api.py @@ -12,7 +12,7 @@ from ninja_extra.permissions import IsAuthenticated from ninja_extra.schemas import PaginatedResponseSchema from pydantic import NonNegativeInt -from core.api_permissions import CanAccessLookup, CanView, IsInGroup, IsRoot +from core.auth.api_permissions import CanAccessLookup, CanView, IsInGroup, IsRoot from core.models import Notification, User from sas.models import Album, PeoplePictureRelation, Picture from sas.schemas import ( diff --git a/sas/views.py b/sas/views.py index 748dc718..7c8c8ea2 100644 --- a/sas/views.py +++ b/sas/views.py @@ -23,8 +23,8 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView, TemplateView from django.views.generic.edit import FormMixin, FormView, UpdateView +from core.auth.mixins import CanEditMixin, CanViewMixin from core.models import SithFile, User -from core.views import CanEditMixin, CanViewMixin from core.views.files import FileView, send_file from sas.forms import ( AlbumEditForm, diff --git a/sith/settings.py b/sith/settings.py index 61079d64..9929995e 100644 --- a/sith/settings.py +++ b/sith/settings.py @@ -155,13 +155,12 @@ TEMPLATES = [ "add_attr": "core.templatetags.renderer.add_attr", }, "globals": { - "can_edit_prop": "core.views.can_edit_prop", - "can_edit": "core.views.can_edit", - "can_view": "core.views.can_view", + "can_edit_prop": "core.auth.mixins.can_edit_prop", + "can_edit": "core.auth.mixins.can_edit", + "can_view": "core.auth.mixins.can_view", "settings": "sith.settings", "Launderette": "launderette.models.Launderette", "Counter": "counter.models.Counter", - "ProductType": "counter.models.ProductType", "timezone": "django.utils.timezone", "get_sith": "com.views.sith", "get_language": "django.utils.translation.get_language", @@ -292,7 +291,7 @@ STORAGES = { # Auth configuration AUTH_USER_MODEL = "core.User" AUTH_ANONYMOUS_MODEL = "core.models.AnonymousUser" -AUTHENTICATION_BACKENDS = ["core.auth_backends.SithModelBackend"] +AUTHENTICATION_BACKENDS = ["core.auth.backends.SithModelBackend"] LOGIN_URL = "/login" LOGOUT_URL = "/logout" LOGIN_REDIRECT_URL = "/" diff --git a/trombi/views.py b/trombi/views.py index 398b4dee..7f43e199 100644 --- a/trombi/views.py +++ b/trombi/views.py @@ -38,8 +38,13 @@ from django.views.generic import DetailView, RedirectView, TemplateView, View from django.views.generic.edit import CreateView, DeleteView, UpdateView from club.models import Club +from core.auth.mixins import ( + CanCreateMixin, + CanEditMixin, + CanEditPropMixin, + CanViewMixin, +) from core.models import User -from core.views import CanCreateMixin, CanEditMixin, CanEditPropMixin, CanViewMixin from core.views.forms import SelectDate from core.views.mixins import QuickNotifMixin, TabedViewMixin from core.views.widgets.select import AutoCompleteSelectUser From 551091f6501388b1962b60a438a98bdf29c3aa4a Mon Sep 17 00:00:00 2001 From: imperosol Date: Sat, 11 Jan 2025 02:22:12 +0100 Subject: [PATCH 4/9] 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 From e500cf92ee72f31cb40d2f8e52a1e8fd87eeae77 Mon Sep 17 00:00:00 2001 From: imperosol Date: Mon, 13 Jan 2025 15:57:01 +0100 Subject: [PATCH 5/9] Remove `SubscriberMixin` --- core/auth/mixins.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/core/auth/mixins.py b/core/auth/mixins.py index 794021d5..422d7bce 100644 --- a/core/auth/mixins.py +++ b/core/auth/mixins.py @@ -208,13 +208,6 @@ class FormerSubscriberMixin(AccessMixin): return super().dispatch(request, *args, **kwargs) -class SubscriberMixin(AccessMixin): - def dispatch(self, request, *args, **kwargs): - 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. From d0b1a4930014d5e7310312e4b416163664d98073 Mon Sep 17 00:00:00 2001 From: imperosol Date: Mon, 13 Jan 2025 17:31:04 +0100 Subject: [PATCH 6/9] deprecate `CanCreateMixin` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Les motifs de cette déprécation sont indiqués dans la documentation. Le mixin a été remplacé par `PermissionRequiredMixin` dans les endroits où ce remplacement était aisé. --- accounting/views.py | 7 +++++-- club/views.py | 3 ++- com/tests/test_views.py | 6 +++--- core/auth/mixins.py | 19 ++++++++++++++++++ core/management/commands/populate.py | 4 +++- core/tests/test_core.py | 9 +++------ core/views/group.py | 6 ++++-- docs/tutorial/perms.md | 30 +++++++++++++++++++++++++--- election/views.py | 24 +++++++++------------- launderette/views.py | 14 ++++++------- pedagogy/tests/tests.py | 9 +++++---- pedagogy/views.py | 15 ++++++-------- sith/settings.py | 4 ++-- 13 files changed, 94 insertions(+), 56 deletions(-) diff --git a/accounting/views.py b/accounting/views.py index d379b60d..f9fd8412 100644 --- a/accounting/views.py +++ b/accounting/views.py @@ -17,6 +17,7 @@ import collections 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 @@ -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 diff --git a/club/views.py b/club/views.py index 498e16e1..de5ccaee 100644 --- a/club/views.py +++ b/club/views.py @@ -473,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): diff --git a/com/tests/test_views.py b/com/tests/test_views.py index c526dee5..100a83ef 100644 --- a/com/tests/test_views.py +++ b/com/tests/test_views.py @@ -159,13 +159,13 @@ class TestNews(TestCase): 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() diff --git a/core/auth/mixins.py b/core/auth/mixins.py index 422d7bce..c25ecb57 100644 --- a/core/auth/mixins.py +++ b/core/auth/mixins.py @@ -23,6 +23,7 @@ # import types +import warnings from typing import TYPE_CHECKING, Any, LiteralString from django.contrib.auth.mixins import AccessMixin, PermissionRequiredMixin @@ -148,6 +149,24 @@ class CanCreateMixin(View): to create the object of the view. """ + def __init_subclass__(cls, **kwargs): + warnings.warn( + f"{cls.__name__} is deprecated and should be replaced " + "by other permission verification mecanism.", + DeprecationWarning, + stacklevel=2, + ) + super().__init_subclass__(**kwargs) + + def __init__(self, *args, **kwargs): + warnings.warn( + f"{self.__class__.__name__} is deprecated and should be replaced " + "by other permission verification mecanism.", + DeprecationWarning, + stacklevel=2, + ) + super().__init__(*args, **kwargs) + def dispatch(self, request, *arg, **kwargs): res = super().dispatch(request, *arg, **kwargs) if not request.user.is_authenticated: diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index a5131d64..5e0f099d 100644 --- a/core/management/commands/populate.py +++ b/core/management/commands/populate.py @@ -894,7 +894,9 @@ Welcome to the wiki page! public_group = Group.objects.create(name="Public") subscribers = Group.objects.create(name="Subscribers") - subscribers.permissions.add(*list(perms.filter(codename__in=["add_news"]))) + subscribers.permissions.add( + *list(perms.filter(codename__in=["add_news", "add_uvcommentreport"])) + ) old_subscribers = Group.objects.create(name="Old subscribers") old_subscribers.permissions.add( *list( diff --git a/core/tests/test_core.py b/core/tests/test_core.py index 878db4e4..e6b37e5c 100644 --- a/core/tests/test_core.py +++ b/core/tests/test_core.py @@ -327,12 +327,9 @@ http://git.an class TestUserTools: def test_anonymous_user_unauthorized(self, client): """An anonymous user shouldn't have access to the tools page.""" - response = client.get(reverse("core:user_tools")) - assertRedirects( - response, - expected_url="/login?next=%2Fuser%2Ftools%2F", - target_status_code=301, - ) + url = reverse("core:user_tools") + response = client.get(url) + assertRedirects(response, expected_url=reverse("core:login") + f"?next={url}") @pytest.mark.parametrize("username", ["guy", "root", "skia", "comunity"]) def test_page_is_working(self, client, username): diff --git a/core/views/group.py b/core/views/group.py index afd6874d..e17db138 100644 --- a/core/views/group.py +++ b/core/views/group.py @@ -16,12 +16,13 @@ """Views to manage Groups.""" from django import forms +from django.contrib.auth.mixins import PermissionRequiredMixin from django.urls import reverse_lazy from django.utils.translation import gettext_lazy as _ from django.views.generic import ListView from django.views.generic.edit import CreateView, DeleteView, UpdateView -from core.auth.mixins import CanCreateMixin, CanEditMixin +from core.auth.mixins import CanEditMixin from core.models import Group, User from core.views import DetailFormView from core.views.widgets.select import AutoCompleteSelectMultipleUser @@ -74,13 +75,14 @@ class GroupEditView(CanEditMixin, UpdateView): fields = ["name", "description"] -class GroupCreateView(CanCreateMixin, CreateView): +class GroupCreateView(PermissionRequiredMixin, CreateView): """Add a new Group.""" model = Group queryset = Group.objects.filter(is_manually_manageable=True) template_name = "core/create.jinja" fields = ["name", "description"] + permission_required = "core.add_group" class GroupTemplateView(CanEditMixin, DetailFormView): diff --git a/docs/tutorial/perms.md b/docs/tutorial/perms.md index 0100f24c..8a84fda7 100644 --- a/docs/tutorial/perms.md +++ b/docs/tutorial/perms.md @@ -173,13 +173,37 @@ class ArticlesCreateView(CanCreateMixin, CreateView): Les mixins suivants sont implémentés : - [CanCreateMixin][core.auth.mixins.CanCreateMixin] : l'utilisateur peut-il créer l'objet ? + Ce mixin existe, mais est déprécié et ne doit plus être utilisé ! - [CanEditPropMixin][core.auth.mixins.CanEditPropMixin] : l'utilisateur peut-il éditer les propriétés de l'objet ? - [CanEditMixin][core.auth.mixins.CanEditMixin] : L'utilisateur peut-il éditer l'objet ? - [CanViewMixin][core.auth.mixins.CanViewMixin] : L'utilisateur peut-il voir l'objet ? - [FormerSubscriberMixin][core.auth.mixins.FormerSubscriberMixin] : L'utilisateur a-t-il déjà été cotisant ? -- [PermissionOrAuthorRequiredMixin][core.auth.mixins.PermissionOrAuthorRequiredMixin] : - L'utilisateur a-t-il la permission requise, ou bien est-il l'auteur de l'objet - auquel on veut accéder ? + +!!!danger "CanCreateMixin" + + L'usage de `CanCreateMixin` est dangereux et ne doit en aucun cas être + étendu. + La façon dont ce mixin marche est qu'il valide le formulaire + de création et crée l'objet sans le persister en base de données, puis + vérifie les droits sur cet objet non-persisté. + Le danger de ce système vient de multiples raisons : + - Les vérifications se faisant sur un objet non persisté, + l'utilisation de mécanismes nécessitant une persistance préalable + peut mener à des comportements indésirés, voire à des erreurs. + - Les développeurs de django ayant tendance à restreindre progressivement + les actions qui peuvent être faites sur des objets non-persistés, + les mises-à-jour de django deviennent plus compliquées. + - La vérification des droits ne se fait que dans les requêtes POST, + à la toute fin de la requête. + Tout ce qui arrive avant n'est absolument pas protégé. + Toute opération (même les suppressions et les créations) qui ont + lieu avant la persistance de l'objet seront appliquées, + même sans permission. + - Si un développeur du site fait l'erreur de surcharger + la méthode `form_valid` (ce qui est plutôt courant, + lorsqu'on veut accomplir certaines actions + quand un formulaire est valide), on peut se retrouver + dans une situation où l'objet est persisté sans aucune protection. !!!danger "Performance" diff --git a/election/views.py b/election/views.py index 1b5439f3..cd367b63 100644 --- a/election/views.py +++ b/election/views.py @@ -1,6 +1,7 @@ from typing import TYPE_CHECKING from django import forms +from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin from django.core.exceptions import PermissionDenied from django.db import transaction from django.db.models.query import QuerySet @@ -300,7 +301,7 @@ class VoteFormView(CanCreateMixin, FormView): # Create views -class CandidatureCreateView(CanCreateMixin, CreateView): +class CandidatureCreateView(LoginRequiredMixin, CreateView): """View dedicated to a cundidature creation.""" form_class = CandidateForm @@ -326,12 +327,13 @@ class CandidatureCreateView(CanCreateMixin, CreateView): def form_valid(self, form): """Verify that the selected user is in candidate group.""" obj = form.instance - obj.election = Election.objects.get(id=self.election.id) - obj.user = obj.user if hasattr(obj, "user") else self.request.user + obj.election = self.election + if not hasattr(obj, "user"): + obj.user = self.request.user if (obj.election.can_candidate(obj.user)) and ( obj.user == self.request.user or self.can_edit ): - return super(CreateView, self).form_valid(form) + return super().form_valid(form) raise PermissionDenied def get_context_data(self, **kwargs): @@ -343,22 +345,14 @@ class CandidatureCreateView(CanCreateMixin, CreateView): return reverse_lazy("election:detail", kwargs={"election_id": self.election.id}) -class ElectionCreateView(CanCreateMixin, CreateView): +class ElectionCreateView(PermissionRequiredMixin, CreateView): model = Election form_class = ElectionForm template_name = "core/create.jinja" - - def dispatch(self, request, *args, **kwargs): - if not request.user.is_subscribed: - raise PermissionDenied - return super().dispatch(request, *args, **kwargs) - - def form_valid(self, form): - """Allow every user that had passed the dispatch to create an election.""" - return super(CreateView, self).form_valid(form) + permission_required = "election.add_election" def get_success_url(self, **kwargs): - return reverse_lazy("election:detail", kwargs={"election_id": self.object.id}) + return reverse("election:detail", kwargs={"election_id": self.object.id}) class RoleCreateView(CanCreateMixin, CreateView): diff --git a/launderette/views.py b/launderette/views.py index be2bfceb..92a81dad 100644 --- a/launderette/views.py +++ b/launderette/views.py @@ -19,6 +19,7 @@ from datetime import timezone as tz from django import forms from django.conf import settings +from django.contrib.auth.mixins import PermissionRequiredMixin from django.db import transaction from django.template import defaultfilters from django.urls import reverse_lazy @@ -28,12 +29,7 @@ from django.views.generic import DetailView, ListView, TemplateView from django.views.generic.edit import BaseFormView, CreateView, DeleteView, UpdateView from club.models import Club -from core.auth.mixins import ( - CanCreateMixin, - CanEditMixin, - CanEditPropMixin, - CanViewMixin, -) +from core.auth.mixins import CanEditMixin, CanEditPropMixin, CanViewMixin from core.models import Page, User from counter.forms import GetUserForm from counter.models import Counter, Customer, Selling @@ -191,12 +187,13 @@ class LaunderetteEditView(CanEditPropMixin, UpdateView): template_name = "core/edit.jinja" -class LaunderetteCreateView(CanCreateMixin, CreateView): +class LaunderetteCreateView(PermissionRequiredMixin, CreateView): """Create a new launderette.""" model = Launderette fields = ["name"] template_name = "core/create.jinja" + permission_required = "launderette.add_launderette" def form_valid(self, form): club = Club.objects.filter( @@ -497,12 +494,13 @@ class MachineDeleteView(CanEditPropMixin, DeleteView): success_url = reverse_lazy("launderette:launderette_list") -class MachineCreateView(CanCreateMixin, CreateView): +class MachineCreateView(PermissionRequiredMixin, CreateView): """Create a new machine.""" model = Machine fields = ["name", "launderette", "type"] template_name = "core/create.jinja" + permission_required = "launderette.add_machine" def get_initial(self): ret = super().get_initial() diff --git a/pedagogy/tests/tests.py b/pedagogy/tests/tests.py index cc36f3c3..6e04b949 100644 --- a/pedagogy/tests/tests.py +++ b/pedagogy/tests/tests.py @@ -26,6 +26,7 @@ from django.conf import settings from django.test import Client, TestCase from django.urls import reverse from django.utils.translation import gettext_lazy as _ +from pytest_django.asserts import assertRedirects from core.models import Notification, User from pedagogy.models import UV, UVComment, UVCommentReport @@ -106,7 +107,7 @@ class TestUVCreation(TestCase): def test_create_uv_unauthorized_fail(self): # Test with anonymous user response = self.client.post(self.create_uv_url, create_uv_template(0)) - assert response.status_code == 403 + assertRedirects(response, reverse("core:login") + f"?next={self.create_uv_url}") # Test with subscribed user self.client.force_login(self.sli) @@ -815,11 +816,11 @@ class TestUVCommentReportCreate(TestCase): self.create_report_test("guy", success=False) def test_create_report_anonymous_fail(self): + url = reverse("pedagogy:comment_report", kwargs={"comment_id": self.comment.id}) response = self.client.post( - reverse("pedagogy:comment_report", kwargs={"comment_id": self.comment.id}), - {"comment": self.comment.id, "reporter": 0, "reason": "C'est moche"}, + url, {"comment": self.comment.id, "reporter": 0, "reason": "C'est moche"} ) - assert response.status_code == 403 + assertRedirects(response, reverse("core:login") + f"?next={url}") assert not UVCommentReport.objects.all().exists() def test_notifications(self): diff --git a/pedagogy/views.py b/pedagogy/views.py index 770dc2a4..88e9c186 100644 --- a/pedagogy/views.py +++ b/pedagogy/views.py @@ -22,7 +22,7 @@ # from django.conf import settings -from django.contrib.auth.mixins import LoginRequiredMixin +from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin from django.core.exceptions import PermissionDenied from django.db.models import Exists, OuterRef from django.shortcuts import get_object_or_404 @@ -35,12 +35,7 @@ from django.views.generic import ( UpdateView, ) -from core.auth.mixins import ( - CanCreateMixin, - CanEditPropMixin, - CanViewMixin, - FormerSubscriberMixin, -) +from core.auth.mixins import CanEditPropMixin, CanViewMixin, FormerSubscriberMixin from core.models import Notification, User from core.views import DetailFormView from pedagogy.forms import ( @@ -136,12 +131,13 @@ class UVGuideView(LoginRequiredMixin, FormerSubscriberMixin, TemplateView): } -class UVCommentReportCreateView(CanCreateMixin, CreateView): +class UVCommentReportCreateView(PermissionRequiredMixin, CreateView): """Create a new report for an inapropriate comment.""" model = UVCommentReport form_class = UVCommentReportForm template_name = "core/edit.jinja" + permission_required = "pedagogy.add_uvcommentreport" def dispatch(self, request, *args, **kwargs): self.uv_comment = get_object_or_404(UVComment, pk=kwargs["comment_id"]) @@ -202,12 +198,13 @@ class UVModerationFormView(FormView): return reverse_lazy("pedagogy:moderation") -class UVCreateView(CanCreateMixin, CreateView): +class UVCreateView(PermissionRequiredMixin, CreateView): """Add a new UV (Privileged).""" model = UV form_class = UVForm template_name = "pedagogy/uv_edit.jinja" + permission_required = "pedagogy.add_uv" def get_form_kwargs(self): kwargs = super().get_form_kwargs() diff --git a/sith/settings.py b/sith/settings.py index 9929995e..d3acabec 100644 --- a/sith/settings.py +++ b/sith/settings.py @@ -292,8 +292,8 @@ STORAGES = { AUTH_USER_MODEL = "core.User" AUTH_ANONYMOUS_MODEL = "core.models.AnonymousUser" AUTHENTICATION_BACKENDS = ["core.auth.backends.SithModelBackend"] -LOGIN_URL = "/login" -LOGOUT_URL = "/logout" +LOGIN_URL = "/login/" +LOGOUT_URL = "/logout/" LOGIN_REDIRECT_URL = "/" DEFAULT_FROM_EMAIL = "bibou@git.an" SITH_COM_EMAIL = "bibou_com@git.an" From 9b5f08e13c8e07e56f0660ec7b5f3f5b6d791c75 Mon Sep 17 00:00:00 2001 From: imperosol Date: Mon, 13 Jan 2025 18:02:47 +0100 Subject: [PATCH 7/9] Improve permission documentation --- docs/tutorial/groups.md | 48 ++++- docs/tutorial/perms.md | 434 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 440 insertions(+), 42 deletions(-) diff --git a/docs/tutorial/groups.md b/docs/tutorial/groups.md index fbb58aab..9e310c75 100644 --- a/docs/tutorial/groups.md +++ b/docs/tutorial/groups.md @@ -157,7 +157,9 @@ il est automatiquement ajouté au groupe des membres du club. Lorsqu'il quitte le club, il est retiré du groupe. -## Les principaux groupes utilisés +## Les groupes utilisés + +### Groupes principaux Les groupes les plus notables gérables par les administrateurs du site sont : @@ -168,15 +170,45 @@ Les groupes les plus notables gérables par les administrateurs du site sont : - `SAS admin` : les administrateurs du SAS - `Forum admin` : les administrateurs du forum - `Pedagogy admin` : les administrateurs de la pédagogie (guide des UVs) -- `Banned from buying alcohol` : les utilisateurs interdits de vente d'alcool (non mineurs) -- `Banned from counters` : les utilisateurs interdits d'utilisation des comptoirs -- `Banned to subscribe` : les utilisateurs interdits de cotisation - En plus de ces groupes, on peut noter : -- `Public` : tous les utilisateurs du site -- `Subscribers` : tous les cotisants du site -- `Old subscribers` : tous les anciens cotisants +- `Public` : tous les utilisateurs du site. + Un utilisateur est automatiquement ajouté à ce group + lors de la création de son compte. +- `Subscribers` : tous les cotisants du site. + Les utilisateurs ne sont pas réellement ajoutés ce groupe ; + cependant, les utilisateurs cotisants sont implicitement + considérés comme membres du groupe lors de l'appel + à la méthode `User.has_perm`. +- `Old subscribers` : tous les anciens cotisants. + Un utilisateur est automatiquement ajouté à ce groupe + lors de sa première cotisation +### Groupes de club +Chaque club est associé à deux groupes : +le groupe des membres et le groupe du bureau. + +Lorsqu'un utilisateur rejoint un club, il est automatiquement +ajouté au groupe des membres. +S'il rejoint le club en tant que membre du bureau, +il est également ajouté au groupe du bureau. + +Lorsqu'un utilisateur quitte le club, il est automatiquement +retiré des groupes liés au club. +S'il quitte le bureau, mais reste dans le club, +il est retiré du groupe du bureau, mais reste dans le groupe des membres. + +### Groupes de ban + +Les groupes de ban sont une catégorie de groupes à part, +qui ne sont pas stockés dans la même table +et qui ne sont pas gérés sur la même interface +que les autres groupes. + +Les groupes de ban existants sont les suivants : + +- `Banned from buying alcohol` : les utilisateurs interdits de vente d'alcool (non mineurs) +- `Banned from counters` : les utilisateurs interdits d'utilisation des comptoirs +- `Banned to subscribe` : les utilisateurs interdits de cotisation diff --git a/docs/tutorial/perms.md b/docs/tutorial/perms.md index 8a84fda7..680c8ad3 100644 --- a/docs/tutorial/perms.md +++ b/docs/tutorial/perms.md @@ -1,15 +1,292 @@ -## Les permissions +## Objectifs du système de permissions -Le fonctionnement de l'AE ne permet pas d'utiliser le système de permissions -intégré à Django tel quel. Lors de la conception du Sith, ce qui paraissait le -plus simple à l'époque était de concevoir un système maison afin de se calquer -sur ce que faisait l'ancien site. +Les permissions attendues sur le site sont relativement spécifiques. +L'accès à une ressource peut se faire selon un certain nombre +de paramètres différents : -### Protéger un modèle +`L'état de la ressource` +: Certaines ressources + sont visibles par tous les cotisants (voire tous les utilisateurs), + à condition qu'elles aient passé une étape de modération. + La visibilité des ressources non-modérées nécessite des permissions + supplémentaires. -La gestion des permissions se fait directement par modèle. -Il existe trois niveaux de permission : +`L'appartenance à un groupe` +: Les groupes Root, Admin Com, Admin SAS, etc. + sont associés à des jeux de permissions. + Par exemple, les membres du groupe Admin SAS ont tous les droits sur + les ressources liées au SAS : ils peuvent voir, + créer, éditer, supprimer et éventuellement modérer + des images, des albums, des identifications de personnes... + Il en va de même avec les admins Com pour la communication, + les admins pédagogie pour le guide des UEs et ainsi de suite. + Quant aux membres du groupe Root, ils ont tous les droits + sur toutes les ressources du site. + +`Le statut de la cotisation` +: Les non-cotisants n'ont presque aucun + droit sur les ressources du site (ils peuvent seulement en voir une poignée), + les anciens cotisants peuvent voir un grand nombre de ressources + et les cotisants actuels ont la plupart des droits qui ne sont + pas liés à un club ou à l'administration du site. + +`L'appartenance à un club` +: Être dans un club donne le droit + de voir la plupart des ressources liées au club dans lequel ils + sont ; être dans le bureau du club donne en outre des droits + d'édition et de création sur ces ressources. + +`Être l'auteur ou le possesseur d'une ressource` +: Certaines ressources, comme les nouvelles, + enregistrent l'utilisateur qui les a créées ; + ce dernier a les droits de voir, de modifier et éventuellement + de supprimer ses ressources, quand bien même + elles ne seraient pas visibles pour les utilisateurs normaux + (par exemple, parce qu'elles ne sont pas encore modérées.) + + +Le système de permissions inclus par défaut dans django +permet de modéliser aisément l'accès à des ressources au niveau +de la table. +Ainsi, il n'est pas compliqué de gérer les permissions liées +aux groupes d'administration. + +Cependant, une surcouche est nécessaire dès lors que l'on veut +gérer les droits liés à une ligne en particulier +d'une table de la base de données. + +Nous essayons le plus possible de nous tenir aux fonctionnalités +de django, sans pour autant hésiter à nous rabattre sur notre +propre surcouche dès lors que les permissions attendues +deviennent trop spécifiques pour être gérées avec juste django. + +!!!info "Un peu d'histoire" + + Les permissions du site n'ont pas toujours été gérées + avec un mélange de fonctionnalités de django et de notre + propre code. + Pendant très longtemps, seule la surcouche était utilisée, + ce qui menait souvent à des vérifications de droits + inefficaces et à une gestion complexe de certaines + parties qui auraient pu être manipulées beaucoup plus simplement. + + En plus de ça, les permissions liées à la plupart + des groupes se faisait de manière hardcodée : + plutôt que d'associer un groupe à un jeu de permission + et de faire une jointure en db sur les groupes de l'utilisateur + ayant cette permissions, + on conservait la clef primaire du groupe dans la config + et on vérifiait en dur dans le code que l'utilisateur + était un des groupes voulus. + + Ce système possédait le triple désavantage de prendre énormément + de temps, d'être extrêmement limité (de fait, si tout est hardcodé, + on est obligé d'avoir le moins de groupes possibles pour que ça reste + gérable) et d'être désespérément dangereux (par exemple : fin novembre 2024, + une erreur dans le code a donné les accès à la création des cotisations + à tout le monde ; mi-octobre 2019, le calcul des permissions des etickets + pouvait faire tomber le site, cf. + [ce topic du forum](https://ae.utbm.fr/forum/topic/17943/?page=1msg2277272)) + +## Accès à toutes les ressources d'une table + +Gérer ce genre d'accès (par exemple : voir toutes les nouvelles +ou pouvoir supprimer n'importe quelle photo) +est exactement le problème que le système de permissions de django résout. +Nous utilisons donc ce système dans ce genre de situations. + +!!!note + + Nous décrivons ci-dessous l'usage que nous faisons du système + de permissions de django, + mais la seule source d'information complète et pleinement fiable + sur le fonctionnement réel de ce système est + [la documentation de django](https://docs.djangoproject.com/fr/stable/topics/auth/default/). + +### Permissions d'un modèle + +Par défaut, django crée quatre permissions pour chaque table de la base de données : + +- `add_` : créer un objet dans cette table +- `view_` : voir le contenu de la table +- `change_` : éditer des objets de la table +- `delete_` : supprimer des objets de la table + +Ces permissions sont créées au même moment que le modèle. +Si la table existe en base de données, ces permissions existent aussi. + +Il est également possible de rajouter nos propres permissions, +directement dans les options Meta du modèle. +Par exemple, prenons le modèle suivant : + +```python +from django.db import models + +class News(models.Model): + # ... + + class Meta: + permissions = [ + ("moderate_news", "Can moderate news"), + ("view_unmoderated_news", "Can view non-moderated news"), + ] +``` + +Ce dernier aura les permissions : `view_news`, `add_news`, `change_news`, +`delete_news`, `moderate_news` et `view_unmoderated_news`. + +### Utilisation des permissions d'un modèle + +Pour vérifier qu'un utilisateur a une permission, +on utilise les fonctions suivantes : + +- `User.has_perm(perm)` : retourne `True` si l'utilisateur + a la permission voulue, sinon `False` +- `User.has_perms([perm_a, perm_b, perm_c])` : retourne `True` si l'utilisateur + a toutes les permissions voulues, sinon `False`. + +Ces fonctions attendent un string suivant le format : +`.`. +Par exemple, la permission pour vérifier qu'un utilisateur +peut modérer une nouvelle sera : `com.moderate_news`. + +Ces fonctions sont utilisables aussi bien dans les templates Jinja +que dans le code Python : + +=== "Jinja" + + ```jinja + {% if user.has_perm("com.moderate_news") %} +
+ +
+ {% endif %} + ``` + +=== "Python" + + ```python + from com.models import News + from core.models import User + + + user = User.objects.get(username="bibou") + news = News.objects.get(id=387) + if user.has_perm("com.moderate_news"): + news.is_moderated = True + news.save() + else: + raise PermissionDenied + ``` + +Pour utiliser ce système de permissions dans une class-based view +(c'est-à-dire la plus grande partie de nos vues), +Django met à disposition `PermissionRequiredMixin`, +qui restreint l'accès à la vue aux utilisateurs ayant +la ou les permissions requises. +Pour les vues sous forme de fonction, il y a le décorateur +`permission_required`. + +=== "Class-Based View" + + ```python + from com.models import News + + from django.contrib.auth.mixins import PermissionRequiredMixin + from django.shortcuts import redirect + from django.urls import reverse + from django.views import View + from django.views.generic.detail import SingleObjectMixin + + class NewsModerateView(PermissionRequiredMixin, SingleObjectMixin, View): + model = News + pk_url_kwarg = "news_id" + permission_required = "com.moderate_news" + # On peut aussi fournir plusieurs permissions, par exemple : + # permission_required = ["com.moderate_news", "com.delete_news"] + + def post(self, request, *args, **kwargs): + # Si nous sommes ici, nous pouvons être certains que l'utilisateur + # a la permission requise + obj = self.get_object() + obj.is_moderated = True + obj.save() + return redirect(reverse("com:news_list")) + ``` + +=== "Function-based view" + + ```python + from com.models import News + + from django.contrib.auth.decorators import permission_required + from django.shortcuts import get_object_or_404, redirect + from django.urls import reverse + from django.views.decorators.http import require_POST + + @permission_required("com.moderate_news") + @require_POST + def moderate_news(request, news_id: int): + # Si nous sommes ici, nous pouvons être certains que l'utilisateur + # a la permission requise + news = get_object_or_404(News, id=news_id) + news.is_moderated = True + news.save() + return redirect(reverse("com:news_list")) + ``` + +## Accès à des éléments en particulier + +### Accès à l'auteur de la ressource + +Dans ce genre de cas, on peut identifier trois acteurs possibles : + +- les administrateurs peuvent accéder à toutes les ressources, + y compris non-modérées +- l'auteur d'une ressource non-modérée peut y accéder +- Les autres utilisateurs ne peuvent pas voir les ressources + non-modérées dont ils ne sont pas l'auteur + +Dans ce genre de cas, on souhaite donc accorder l'accès aux +utilisateurs qui ont la permission globale, selon le système +décrit plus haut, ou bien à l'auteur de la ressource. + +Pour cela, nous avons le mixin `PermissionOrAuthorRequired`. +Ce dernier va effectuer les mêmes vérifications que `PermissionRequiredMixin` +puis, si l'utilisateur n'a pas la permission requise, vérifier +s'il est l'auteur de la ressource. + +```python +from com.models import News +from core.auth.mixins import PermissionOrAuthorRequiredMixin + +from django.views.generic import UpdateView + +class NewsUpdateView(PermissionOrAuthorRequiredMixin, UpdateView): + model = News + pk_url_kwarg = "news_id" + permission_required = "com.change_news" + author_field = "author" # (1)! +``` + +1. Nom du champ du modèle utilisé comme clef étrangère vers l'auteur. + Par exemple, ici, la permission sera accordée si + l'utilisateur connecté correspond à l'utilisateur + désigné par `News.author`. + +### Accès en fonction de règles plus complexes + +Tout ce que nous avons décrit précédemment permet de couvrir +la plupart des cas simples. +Cependant, il arrivera souvent que les permissions attendues soient +plus complexes. +Dans ce genre de cas, on rentre entièrement dans notre surcouche. + +#### Implémentation dans les modèles + +La gestion de ce type de permissions se fait directement par modèle. +Il en existe trois niveaux : - Éditer des propriétés de l'objet - Éditer certaines valeurs l'objet @@ -47,28 +324,43 @@ Voici un exemple d'implémentation de ce système : from core.models import User, Group - # Utilisation de la protection par fonctions class Article(models.Model): title = models.CharField(_("title"), max_length=100) content = models.TextField(_("content")) - # Donne ou non les droits d'édition des propriétés de l'objet - # Un utilisateur dans le bureau AE aura tous les droits sur cet objet - def is_owned_by(self, user): + def is_owned_by(self, user): # (1)! return user.is_board_member - # Donne ou non les droits d'édition de l'objet - # L'objet ne sera modifiable que par un utilisateur cotisant - def can_be_edited_by(self, user): + def can_be_edited_by(self, user): # (2)! return user.is_subscribed - # Donne ou non les droits de vue de l'objet - # Ici, l'objet n'est visible que par un utilisateur connecté - def can_be_viewed_by(self, user): + def can_be_viewed_by(self, user): # (3)! return not user.is_anonymous ``` + 1. Donne ou non les droits d'édition des propriétés de l'objet. + Ici, un utilisateur dans le bureau AE aura tous les droits sur cet objet + 2. Donne ou non les droits d'édition de l'objet + Ici, l'objet ne sera modifiable que par un utilisateur cotisant + 3. Donne ou non les droits de vue de l'objet + Ici, l'objet n'est visible que par un utilisateur connecté + + !!!note + + Dans cet exemple, nous utilisons des permissions très simples + pour que vous puissiez constater le squelette de ce système, + plutôt que la logique de validation dans ce cas particulier. + + En réalité, il serait ici beaucoup plus approprié de + donner les permissions `com.delete_article` et + `com.change_article_properties` (en créant ce dernier + s'il n'existe pas encore) au groupe du bureau AE, + de donner également la permission `com.change_article` + au groupe `Cotisants` et enfin de restreindre l'accès + aux vues d'accès aux articles avec `LoginRequiredMixin`. + + === "Avec les groupes de permission" ```python @@ -83,15 +375,12 @@ Voici un exemple d'implémentation de ce système : content = models.TextField(_("content")) # relation one-to-many - # Groupe possédant l'objet - # Donne les droits d'édition des propriétés de l'objet - owner_group = models.ForeignKey( + owner_group = models.ForeignKey( # (1)! Group, related_name="owned_articles", default=settings.SITH_GROUP_ROOT_ID ) # relation many-to-many - # Tous les groupes qui seront ajouté dans ce champ auront les droits d'édition de l'objet - edit_groups = models.ManyToManyField( + edit_groups = models.ManyToManyField( # (2)! Group, related_name="editable_articles", verbose_name=_("edit groups"), @@ -99,8 +388,7 @@ Voici un exemple d'implémentation de ce système : ) # relation many-to-many - # Tous les groupes qui seront ajouté dans ce champ auront les droits de vue de l'objet - view_groups = models.ManyToManyField( + view_groups = models.ManyToManyField( # (3)! Group, related_name="viewable_articles", verbose_name=_("view groups"), @@ -108,9 +396,16 @@ Voici un exemple d'implémentation de ce système : ) ``` -### Appliquer les permissions + 1. Groupe possédant l'objet + Donne les droits d'édition des propriétés de l'objet. + Il ne peut y avoir qu'un seul groupe `owner` par objet. + 2. Tous les groupes ayant droit d'édition sur l'objet. + Il peut y avoir autant de groupes d'édition que l'on veut par objet. + 3. Tous les groupes ayant droit de voir l'objet. + Il peut y avoir autant de groupes de vue que l'on veut par objet. + -#### Dans un template +#### Application dans les templates Il existe trois fonctions de base sur lesquelles reposent les vérifications de permission. @@ -130,7 +425,7 @@ Voici un exemple d'utilisation dans un template : {% endif %} ``` -#### Dans une vue +#### Application dans les vues Généralement, les vérifications de droits dans les templates se limitent aux urls à afficher puisqu'il @@ -138,7 +433,7 @@ ne faut normalement pas mettre de logique autre que d'affichage à l'intérieur (en réalité, c'est un principe qu'on a beaucoup violé, mais promis on le fera plus). C'est donc habituellement au niveau des vues que cela a lieu. -Notre système s'appuie sur un système de mixin +Pour cela, nous avons rajouté des mixins à hériter lors de la création d'une vue basée sur une classe. Ces mixins ne sont compatibles qu'avec les classes récupérant un objet ou une liste d'objet. @@ -152,22 +447,23 @@ l'utilisateur recevra une liste vide d'objet. Voici un exemple d'utilisation en reprenant l'objet Article crée précédemment : ```python -from django.views.generic import CreateView, ListView +from django.views.generic import CreateView, DetailView from core.auth.mixins import CanViewMixin, CanCreateMixin from com.models import WeekmailArticle + # Il est important de mettre le mixin avant la classe héritée de Django # L'héritage multiple se fait de droite à gauche et les mixins ont besoin # d'une classe de base pour fonctionner correctement. -class ArticlesListView(CanViewMixin, ListView): - model = WeekmailArticle +class ArticlesDetailView(CanViewMixin, DetailView): + model = WeekmailArticle + - # Même chose pour une vue de création de l'objet Article class ArticlesCreateView(CanCreateMixin, CreateView): - model = WeekmailArticle + model = WeekmailArticle ``` Les mixins suivants sont implémentés : @@ -221,6 +517,76 @@ Les mixins suivants sont implémentés : Mais sur les `ListView`, on peut arriver à des temps de réponse extrêmement élevés. +### Filtrage des querysets + +Récupérer tous les objets d'un queryset et vérifier pour chacun que +l'utilisateur a le droit de les voir peut-être excessivement +coûteux en ressources +(cf. l'encart ci-dessus). + +Lorsqu'il est nécessaire de récupérer un certain nombre +d'objets depuis la base de données, il est donc préférable +de filtrer directement depuis le queryset. + +Pour cela, certains modèles, tels que [Picture][sas.models.Picture] +peuvent être filtrés avec la méthode de queryset `viewable_by`. +Cette dernière s'utilise comme n'importe quelle autre méthode +de queryset : + +```python +from sas.models import Picture +from core.models import User + +user = User.objects.get(username="bibou") +pictures = Picture.objects.viewable_by(user) +``` + +Le résultat de la requête contiendra uniquement des éléments +que l'utilisateur sélectionné a effectivement le droit de voir. + +Si vous désirez utiliser cette méthode sur un modèle +qui ne la possède pas, il est relativement facile de l'écrire : + +```python +from typing import Self + +from django.db import models + +from core.models import User + + +class NewsQuerySet(models.QuerySet): # (1)! + def viewable_by(self, user: User) -> Self: + if user.has_perm("com.view_unmoderated_news"): + # si l'utilisateur peut tout voir, on retourne tout + return self + # sinon, on retourne les nouvelles modérées ou dont l'utilisateur + # est l'auteur + return self.filter( + models.Q(is_moderated=True) + | models.Q(author=user) + ) + + +class News(models.Model): + is_moderated = models.BooleanField(default=False) + author = models.ForeignKey(User, on_delete=models.PROTECT) + # ... + + objects = NewsQuerySet.as_manager() # (2)! + + class Meta: + permissions = [("view_unmoderated_news", "Can view non moderated news")] +``` + +1. On crée un `QuerySet` maison, dans lequel on définit la méthode `viewable_by` +2. Puis, on attache ce `QuerySet` à notre modèle + +!!!note + + Pour plus d'informations sur la création de `QuerySet` personnalisés, voir + [la documentation de django](https://docs.djangoproject.com/fr/stable/topics/db/managers/) + ## API L'API utilise son propre système de permissions. From 9272f53beaca5e37a1901c6b3c4c9d7f8cc25ba4 Mon Sep 17 00:00:00 2001 From: imperosol Date: Tue, 14 Jan 2025 17:14:59 +0100 Subject: [PATCH 8/9] fix doc display --- core/auth/api_permissions.py | 4 ++- core/auth/mixins.py | 11 +++---- docs/reference/core/api_permissions.md | 1 - docs/reference/core/auth.md | 32 ++++++++++++++++++++ docs/tutorial/perms.md | 41 +++++++++++++------------- mkdocs.yml | 2 +- 6 files changed, 63 insertions(+), 28 deletions(-) delete mode 100644 docs/reference/core/api_permissions.md create mode 100644 docs/reference/core/auth.md diff --git a/core/auth/api_permissions.py b/core/auth/api_permissions.py index f4da67af..4d83143e 100644 --- a/core/auth/api_permissions.py +++ b/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 @@ Examples: ] def bar_delete(self, bar_id: int): # ... + ``` """ from typing import Any diff --git a/core/auth/mixins.py b/core/auth/mixins.py index c25ecb57..974e9bd1 100644 --- a/core/auth/mixins.py +++ b/core/auth/mixins.py @@ -21,6 +21,7 @@ # Place - Suite 330, Boston, MA 02111-1307, USA. # # +from __future__ import annotations import types import warnings @@ -30,11 +31,11 @@ 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 + from core.models import User + def can_edit_prop(obj: Any, user: User) -> bool: """Can the user edit the properties of the object. @@ -46,7 +47,7 @@ def can_edit_prop(obj: Any, user: User) -> bool: Returns: True if user is authorized to edit object properties else False - Examples: + Example: ```python if not can_edit_prop(self.object ,request.user): raise PermissionDenied @@ -65,7 +66,7 @@ def can_edit(obj: Any, user: User) -> bool: Returns: True if user is authorized to edit object else False - Examples: + Example: ```python if not can_edit(self.object, request.user): raise PermissionDenied @@ -86,7 +87,7 @@ def can_view(obj: Any, user: User) -> bool: Returns: True if user is authorized to see object else False - Examples: + Example: ```python if not can_view(self.object ,request.user): raise PermissionDenied diff --git a/docs/reference/core/api_permissions.md b/docs/reference/core/api_permissions.md deleted file mode 100644 index 4ab3a2e0..00000000 --- a/docs/reference/core/api_permissions.md +++ /dev/null @@ -1 +0,0 @@ -::: core.auth.api_permissions \ No newline at end of file diff --git a/docs/reference/core/auth.md b/docs/reference/core/auth.md new file mode 100644 index 00000000..ade23f49 --- /dev/null +++ b/docs/reference/core/auth.md @@ -0,0 +1,32 @@ +## Backend + +::: core.auth.backends + handler: python + options: + heading_level: 3 + members: + - SithModelBackend + +## Mixins + +::: core.auth.mixins + handler: python + options: + heading_level: 3 + members: + - can_edit_prop + - can_edit + - can_view + - CanCreateMixin + - CanEditMixin + - CanViewMixin + - FormerSubscriberMixin + - PermissionOrAuthorRequiredMixin + + +## API Permissions + +::: core.auth.api_permissions + handler: python + options: + heading_level: 3 \ No newline at end of file diff --git a/docs/tutorial/perms.md b/docs/tutorial/perms.md index 680c8ad3..c23ca25f 100644 --- a/docs/tutorial/perms.md +++ b/docs/tutorial/perms.md @@ -412,9 +412,9 @@ reposent les vérifications de permission. Elles sont disponibles dans le contexte par défaut du moteur de template et peuvent être utilisées à tout moment. -- [can_edit_prop(obj, user)][core.views.can_edit_prop] : équivalent de `obj.is_owned_by(user)` -- [can_edit(obj, user)][core.views.can_edit] : équivalent de `obj.can_be_edited_by(user)` -- [can_view(obj, user)][core.views.can_view] : équivalent de `obj.can_be_viewed_by(user)` +- [can_edit_prop(obj, user)][core.auth.mixins.can_edit_prop] : équivalent de `obj.is_owned_by(user)` +- [can_edit(obj, user)][core.auth.mixins.can_edit] : équivalent de `obj.can_be_edited_by(user)` +- [can_view(obj, user)][core.auth.mixins.can_view] : équivalent de `obj.can_be_viewed_by(user)` Voici un exemple d'utilisation dans un template : @@ -483,23 +483,24 @@ Les mixins suivants sont implémentés : de création et crée l'objet sans le persister en base de données, puis vérifie les droits sur cet objet non-persisté. Le danger de ce système vient de multiples raisons : - - Les vérifications se faisant sur un objet non persisté, - l'utilisation de mécanismes nécessitant une persistance préalable - peut mener à des comportements indésirés, voire à des erreurs. - - Les développeurs de django ayant tendance à restreindre progressivement - les actions qui peuvent être faites sur des objets non-persistés, - les mises-à-jour de django deviennent plus compliquées. - - La vérification des droits ne se fait que dans les requêtes POST, - à la toute fin de la requête. - Tout ce qui arrive avant n'est absolument pas protégé. - Toute opération (même les suppressions et les créations) qui ont - lieu avant la persistance de l'objet seront appliquées, - même sans permission. - - Si un développeur du site fait l'erreur de surcharger - la méthode `form_valid` (ce qui est plutôt courant, - lorsqu'on veut accomplir certaines actions - quand un formulaire est valide), on peut se retrouver - dans une situation où l'objet est persisté sans aucune protection. + + - Les vérifications se faisant sur un objet non persisté, + l'utilisation de mécanismes nécessitant une persistance préalable + peut mener à des comportements indésirés, voire à des erreurs. + - Les développeurs de django ayant tendance à restreindre progressivement + les actions qui peuvent être faites sur des objets non-persistés, + les mises-à-jour de django deviennent plus compliquées. + - La vérification des droits ne se fait que dans les requêtes POST, + à la toute fin de la requête. + Tout ce qui arrive avant n'est absolument pas protégé. + Toute opération (même les suppressions et les créations) qui ont + lieu avant la persistance de l'objet seront appliquées, + même sans permission. + - Si un développeur du site fait l'erreur de surcharger + la méthode `form_valid` (ce qui est plutôt courant, + lorsqu'on veut accomplir certaines actions + quand un formulaire est valide), on peut se retrouver + dans une situation où l'objet est persisté sans aucune protection. !!!danger "Performance" diff --git a/mkdocs.yml b/mkdocs.yml index 70075794..f307cb8a 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -98,7 +98,7 @@ nav: - Champs de modèle: reference/core/model_fields.md - reference/core/views.md - reference/core/schemas.md - - reference/core/api_permissions.md + - reference/core/auth.md - counter: - reference/counter/models.md - reference/counter/views.md From 71b096f9ef3de8c34aefe9e798d90f7a496fa10e Mon Sep 17 00:00:00 2001 From: imperosol Date: Tue, 14 Jan 2025 17:17:31 +0100 Subject: [PATCH 9/9] Apply review comment --- com/views.py | 6 ++---- docs/tutorial/groups.md | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/com/views.py b/com/views.py index 7d29dd62..a6faf214 100644 --- a/com/views.py +++ b/com/views.py @@ -174,11 +174,9 @@ class NewsUpdateView(PermissionOrAuthorRequiredMixin, UpdateView): 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() - # Don't call `super().form_valid()`, - # because it would trigger a second call to `form.save()` - return HttpResponseRedirect(self.get_success_url()) + return response def get_date_form_kwargs(self) -> dict[str, Any]: """Get initial data for NewsDateForm""" diff --git a/docs/tutorial/groups.md b/docs/tutorial/groups.md index 9e310c75..bccd713f 100644 --- a/docs/tutorial/groups.md +++ b/docs/tutorial/groups.md @@ -185,6 +185,22 @@ En plus de ces groupes, on peut noter : Un utilisateur est automatiquement ajouté à ce groupe lors de sa première cotisation +!!!note "Utilisation du groupe Public" + + Le groupe Public est un groupe particulier. + Tout le monde faisant partie de ce groupe + (même les utilisateurs non-connectés en sont implicitement + considérés comme membres), + il ne doit pas être utilisé pour résoudre les + permissions d'une vue. + + En revanche, il est utile pour attribuer une ressource + à tout le monde. + Par exemple, un produit avec le groupe de vente Public + est considéré comme achetable par tous utilisateurs. + S'il n'avait eu aucun group de vente, il n'aurait + été accessible à personne. + ### Groupes de club Chaque club est associé à deux groupes :