deprecate CanCreateMixin

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é.
This commit is contained in:
imperosol 2025-01-13 17:31:04 +01:00
parent e500cf92ee
commit d0b1a49300
13 changed files with 94 additions and 56 deletions

View File

@ -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

View File

@ -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):

View File

@ -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()

View File

@ -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:

View File

@ -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(

View File

@ -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):

View File

@ -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):

View File

@ -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"

View File

@ -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):

View File

@ -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()

View File

@ -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):

View File

@ -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()

View File

@ -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"