Skip to content

Commit

Permalink
🐛(back) do not use view.get_object in permissions
Browse files Browse the repository at this point in the history
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.
encode/django-rest-framework#7522
  • Loading branch information
lunika committed Sep 26, 2022
1 parent f00ecbc commit d58db45
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 22 deletions.
14 changes: 14 additions & 0 deletions src/backend/marsha/core/api/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 4 additions & 2 deletions src/backend/marsha/core/api/shared_live_media.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
3 changes: 2 additions & 1 deletion src/backend/marsha/core/api/thumbnail.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions src/backend/marsha/core/api/timed_text_track.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
18 changes: 9 additions & 9 deletions src/backend/marsha/core/permissions.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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()

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

Expand Down
3 changes: 2 additions & 1 deletion src/backend/marsha/deposit/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -165,6 +165,7 @@ def depositedfiles(self, request, pk=None):
class DepositedFileViewSet(
APIViewMixin,
ObjectPkMixin,
ObjectRelatedMixin,
mixins.CreateModelMixin,
mixins.UpdateModelMixin,
viewsets.GenericViewSet,
Expand Down
8 changes: 5 additions & 3 deletions src/backend/marsha/deposit/permissions.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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
3 changes: 2 additions & 1 deletion src/backend/marsha/markdown/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -195,6 +195,7 @@ def update_translations(self, request, *args, **kwargs):
class MarkdownImageViewSet(
APIViewMixin,
ObjectPkMixin,
ObjectRelatedMixin,
mixins.CreateModelMixin,
mixins.DestroyModelMixin,
mixins.RetrieveModelMixin,
Expand Down
6 changes: 3 additions & 3 deletions src/backend/marsha/markdown/permissions.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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


Expand Down

0 comments on commit d58db45

Please sign in to comment.