From d58db453392212a062a897fb9c02d4cf2bfb79b6 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Mon, 26 Sep 2022 16:00:32 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(back)=20do=20not=20use=20view.get?= =?UTF-8?q?=5Fobject=20in=20permissions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since DRF 3.14.0 it is not possible to use view.get_object in the has_permission method. If we do it, we have a max recursion error. In this version, they fix a bug related to the usage of th OR and AND operator. To fix this issue, we created a dedicated mixin responsible to fetch the related object we are working on in the database. https://github.com/encode/django-rest-framework/pull/7522 --- src/backend/marsha/core/api/base.py | 14 ++++++++++++++ .../marsha/core/api/shared_live_media.py | 6 ++++-- src/backend/marsha/core/api/thumbnail.py | 3 ++- .../marsha/core/api/timed_text_track.py | 6 ++++-- src/backend/marsha/core/permissions.py | 18 +++++++++--------- src/backend/marsha/deposit/api.py | 3 ++- src/backend/marsha/deposit/permissions.py | 8 +++++--- src/backend/marsha/markdown/api.py | 3 ++- src/backend/marsha/markdown/permissions.py | 6 +++--- 9 files changed, 45 insertions(+), 22 deletions(-) diff --git a/src/backend/marsha/core/api/base.py b/src/backend/marsha/core/api/base.py index c9e76daa9d..1dc5241653 100644 --- a/src/backend/marsha/core/api/base.py +++ b/src/backend/marsha/core/api/base.py @@ -25,6 +25,20 @@ def get_object_pk(self): return self.kwargs.get(lookup_url_kwarg) +class ObjectRelatedMixin: + """ + Get the related video belonging to the current object. + + Using view.get_object is permissions is not possible anymore due to + infinite recursion between has_permission and has_object_permission call. + """ + + def get_related_object(self): + """Get the video related to the current object.""" + queryset = self.filter_queryset(self.get_queryset()) + return queryset.get(pk=self.get_object_pk()) + + @api_view(["POST"]) def update_state(request): """View handling AWS POST request to update the state of an object by key. diff --git a/src/backend/marsha/core/api/shared_live_media.py b/src/backend/marsha/core/api/shared_live_media.py index ebb351f202..2e5199084f 100644 --- a/src/backend/marsha/core/api/shared_live_media.py +++ b/src/backend/marsha/core/api/shared_live_media.py @@ -13,10 +13,12 @@ from ..models import SharedLiveMedia from ..utils.s3_utils import create_presigned_post from ..utils.time_utils import to_timestamp -from .base import APIViewMixin, ObjectPkMixin +from .base import APIViewMixin, ObjectPkMixin, ObjectRelatedMixin -class SharedLiveMediaViewSet(APIViewMixin, ObjectPkMixin, viewsets.ModelViewSet): +class SharedLiveMediaViewSet( + APIViewMixin, ObjectPkMixin, ObjectRelatedMixin, viewsets.ModelViewSet +): """Viewset for the API of the SharedLiveMedia object.""" permission_classes = [permissions.NotAllowed] diff --git a/src/backend/marsha/core/api/thumbnail.py b/src/backend/marsha/core/api/thumbnail.py index 5f58127e1f..da81d1830f 100644 --- a/src/backend/marsha/core/api/thumbnail.py +++ b/src/backend/marsha/core/api/thumbnail.py @@ -10,12 +10,13 @@ from ..models import Thumbnail from ..utils.s3_utils import create_presigned_post from ..utils.time_utils import to_timestamp -from .base import APIViewMixin, ObjectPkMixin +from .base import APIViewMixin, ObjectPkMixin, ObjectRelatedMixin class ThumbnailViewSet( APIViewMixin, ObjectPkMixin, + ObjectRelatedMixin, mixins.CreateModelMixin, mixins.DestroyModelMixin, mixins.RetrieveModelMixin, diff --git a/src/backend/marsha/core/api/timed_text_track.py b/src/backend/marsha/core/api/timed_text_track.py index 6a99c2f989..dc0391df03 100644 --- a/src/backend/marsha/core/api/timed_text_track.py +++ b/src/backend/marsha/core/api/timed_text_track.py @@ -10,10 +10,12 @@ from ..models import TimedTextTrack from ..utils.s3_utils import create_presigned_post from ..utils.time_utils import to_timestamp -from .base import APIViewMixin, ObjectPkMixin +from .base import APIViewMixin, ObjectPkMixin, ObjectRelatedMixin -class TimedTextTrackViewSet(APIViewMixin, ObjectPkMixin, viewsets.ModelViewSet): +class TimedTextTrackViewSet( + APIViewMixin, ObjectPkMixin, ObjectRelatedMixin, viewsets.ModelViewSet +): """Viewset for the API of the TimedTextTrack object.""" permission_classes = [permissions.NotAllowed] diff --git a/src/backend/marsha/core/permissions.py b/src/backend/marsha/core/permissions.py index 462e778070..39d7bd05cd 100644 --- a/src/backend/marsha/core/permissions.py +++ b/src/backend/marsha/core/permissions.py @@ -1,8 +1,8 @@ """Custom permission classes for the Marsha project.""" import uuid +from django.core.exceptions import ObjectDoesNotExist from django.db.models import Q -from django.http.response import Http404 from rest_framework import permissions from rest_framework.permissions import IsAuthenticated @@ -190,9 +190,9 @@ def has_permission(self, request, view): try: return ( request.resource - and str(view.get_object().video.id) == request.resource.id + and str(view.get_related_object().video.id) == request.resource.id ) - except (AssertionError, Http404): + except ObjectDoesNotExist: return False @@ -460,13 +460,13 @@ def has_permission(self, request, view): this video is a part of. """ try: - video_related_object = view.get_object() - except (Http404, AssertionError): + video_related_object = view.get_related_object().video + except ObjectDoesNotExist: return False return models.PlaylistAccess.objects.filter( role=ADMINISTRATOR, - playlist__videos=video_related_object.video, + playlist__videos=video_related_object, user__id=request.user.id, ).exists() @@ -488,13 +488,13 @@ def has_permission(self, request, view): this video is a part of. """ try: - video_related_object = view.get_object() - except (Http404, AssertionError): + video_related_object = view.get_related_object().video + except ObjectDoesNotExist: return False return models.OrganizationAccess.objects.filter( role=ADMINISTRATOR, - organization__playlists__videos=video_related_object.video, + organization__playlists__videos=video_related_object, user__id=request.user.id, ).exists() diff --git a/src/backend/marsha/deposit/api.py b/src/backend/marsha/deposit/api.py index e94c59f056..a3882f5b08 100644 --- a/src/backend/marsha/deposit/api.py +++ b/src/backend/marsha/deposit/api.py @@ -9,7 +9,7 @@ from rest_framework.response import Response from marsha.core import defaults, permissions as core_permissions -from marsha.core.api import APIViewMixin, ObjectPkMixin +from marsha.core.api import APIViewMixin, ObjectPkMixin, ObjectRelatedMixin from marsha.core.utils.s3_utils import create_presigned_post from marsha.core.utils.time_utils import to_timestamp from marsha.core.utils.url_utils import build_absolute_uri_behind_proxy @@ -165,6 +165,7 @@ def depositedfiles(self, request, pk=None): class DepositedFileViewSet( APIViewMixin, ObjectPkMixin, + ObjectRelatedMixin, mixins.CreateModelMixin, mixins.UpdateModelMixin, viewsets.GenericViewSet, diff --git a/src/backend/marsha/deposit/permissions.py b/src/backend/marsha/deposit/permissions.py index 839ef2336f..3476e35cbc 100644 --- a/src/backend/marsha/deposit/permissions.py +++ b/src/backend/marsha/deposit/permissions.py @@ -1,5 +1,5 @@ """Custom permission classes for the Deposit app.""" -from django.http import Http404 +from django.core.exceptions import ObjectDoesNotExist from rest_framework import permissions @@ -30,6 +30,8 @@ def has_permission(self, request, view): True if the request is authorized, False otherwise """ try: - return str(view.get_object().file_depository.id) == request.resource.id - except (AssertionError, Http404): + return ( + str(view.get_related_object().file_depository.id) == request.resource.id + ) + except ObjectDoesNotExist: return False diff --git a/src/backend/marsha/markdown/api.py b/src/backend/marsha/markdown/api.py index 524c1067ec..5b629f9f73 100644 --- a/src/backend/marsha/markdown/api.py +++ b/src/backend/marsha/markdown/api.py @@ -9,7 +9,7 @@ from rest_framework.status import HTTP_200_OK, HTTP_400_BAD_REQUEST from marsha.core import defaults, permissions as core_permissions -from marsha.core.api import APIViewMixin, ObjectPkMixin +from marsha.core.api import APIViewMixin, ObjectPkMixin, ObjectRelatedMixin from marsha.core.utils.time_utils import to_timestamp from marsha.core.utils.url_utils import build_absolute_uri_behind_proxy @@ -195,6 +195,7 @@ def update_translations(self, request, *args, **kwargs): class MarkdownImageViewSet( APIViewMixin, ObjectPkMixin, + ObjectRelatedMixin, mixins.CreateModelMixin, mixins.DestroyModelMixin, mixins.RetrieveModelMixin, diff --git a/src/backend/marsha/markdown/permissions.py b/src/backend/marsha/markdown/permissions.py index b7fd7ca49d..6924118cec 100644 --- a/src/backend/marsha/markdown/permissions.py +++ b/src/backend/marsha/markdown/permissions.py @@ -1,5 +1,5 @@ """Custom permission classes for the "markdown" app in Marsha project.""" -from django.http.response import Http404 +from django.core.exceptions import ObjectDoesNotExist from rest_framework import permissions @@ -38,13 +38,13 @@ def has_permission(self, request, view): return ( str( getattr( - view.get_object(), + view.get_related_object(), self.linked_resource_attribute, ).id ) == request.resource.id ) - except (AssertionError, Http404): + except ObjectDoesNotExist: return False