11

Using ModelViewSet and DjangoObjectPermissions.

Django REST framework seems to not call check_object_permission for a "create" request (POST). I need to check the user is allowed to create THIS object before it's saved into database (because permission check depend of object values)

I suppose I need to override "create" method of the ModelViewSet but I didn't know how to get the instance from serializer without saving it to database.

  1. How to get the object instance from serializer without saving to database ?
  2. Or how to have DRF check for object permission for a POST/create request ?

Thanks

EDIT:

After deeping into DRF code, I'm able to get the instance without save :

def create(self, request, *args, **kwargs):
    serializer = WorkedHourSerializer(data=request.data)
    if serializer.is_valid():
        instance = MyModel(**serializer.validated_data)

But Django refuse to check perm for an object without primary key so I have to force one :

        instance.id = 0
        self.check_object_permissions(request, instance)
fabien-michel
  • 1,873
  • 19
  • 38

4 Answers4

2
  1. There's no way to get the instance before saving it (see more)

  2. The best approach would seem to be to implement a custom permission (probably subclassing rest_framework.permissions.BasePermission or rest_framework.permissions.IsAuthenticated) and adding the logic of permission checking in has_permission(self, request, view) (see more). This way, you would access request.user and then you would be able to determine whether that user has permission to create that object.

Community
  • 1
  • 1
lucasnadalutti
  • 5,818
  • 1
  • 28
  • 48
  • 1
    The problem is more generic, in a update request, the `check_object_permission` is called in `get_object` which use the object version in database and not the data sent by client. So i've written a Mixin to apply on ModelViewSet which perform check_object_permission on object sent by client and before it's saved into database (see below) – fabien-michel Nov 16 '16 at 09:12
1

My solution was to create a Mixin to apply on ModelViewSet which perform check_object_permission with an instance created using request.data and not object retrieved from database before the data are saved to database :

import uuid


class CheckObjectPermissionBeforeSaveMixin():

    def create(self, request, *args, **kwargs):
        self.check_instance_from_data_permission(request)
        return super(CheckObjectPermissionBeforeSaveMixin, self).create(request, *args, **kwargs)

    def update(self, request, *args, **kwargs):
        self.check_instance_from_data_permission(request)
        return super(CheckObjectPermissionBeforeSaveMixin, self).update(request, *args, **kwargs)

    def destroy(self, request, *args, **kwargs):
        self.check_instance_from_data_permission(request)
        return super(CheckObjectPermissionBeforeSaveMixin, self).destroy(request, *args, **kwargs)

    def check_instance_from_data_permission(self, request):
        instance = self.get_instance_from_data(request.data)
        if instance:
            self.check_object_permissions(request, instance)

    def get_instance_from_data(self, data):
        ModelClass = self.serializer_class.Meta.model
        serializer = self.get_serializer(data=data)
        if serializer.is_valid():
            instance = ModelClass(**serializer.validated_data)
            instance.id = data.get('id') or uuid.uuid4().hex  # Django's has_perm need a primary key to be set...
            return instance
        return None


class MyModelViewSet(CheckObjectPermissionBeforeSaveMixin, viewsets.ModelViewSet):
    queryset = MyModel.objects.all()
    serializer_class = MyModelSerializer
fabien-michel
  • 1,873
  • 19
  • 38
1

This is a massive security flaw in my view. Another way of securing it - if not generating nice HTML responses is to write a custom Model Serializer if you're using both it and the REST API Views / ViewSets and are using Object Level Permissions:

class CreatePermModelSerializer(ModelSerializer):
def create(self, validated_data):
    obj = self.Meta.model(**validated_data)
    view = self._context['view']
    request = self._context['request']
    for permission in view.permission_classes:
        if not permission.has_object_permission(self, request, view, obj):
            raise ValueError('not authorized')
    super(CreatePermModelSerializer, self).create(validated_data)
Ben Watts
  • 59
  • 1
  • 1
1

Something about security flaws.

Django DRF permissions system works well to enforce permissions on objects related to other objects...

... But you might have what I would call a "root object" in your app : an object which is only related to a userid.

While testing my app, I noticed, that a logged in user could create such "root object" for another user.

Example that have a security flow :

models.py :

class MyObject(models.Model):
    name = models.CharField(max_length=20)
    owner = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)

permissions.py : You would use IsOwner in the permission classes of your associated viewset.

class IsOwner(permissions.BasePermission):
    def has_permission(self, request, view):
        obj = MyObject.objects.get(pk=view.kwargs['something'])
        return obj.owner == request.user

While this will restrict accessing objects belonging to the logged in user, it won't disallow a logged in user to CREATE an object belonging to ANOTHER user.

I used to let the frontend fill the "owner" property. Like posting this (that's easy to guess that user 1 exists) :

{
    "name": "A prank object",
    "owner": 1,
}

Object level permission (has_object_permission) cannot be used here. It just won't be called.

Therefore, a malicious user could create "prank objects" that could ruin your user experience.

Actually, Django DRF gives a simple solution

In your viewset.py :

def perform_create(self, serializer):
     serializer.save(owner=self.request.user)

And, as the owner property is now managed by Django DRF, this field can by excluded from your serializer Meta. Therefore your REST API is less exposed.

Then whatever a potentially malicious user would post as a owner, it will be replaced by its own id and therefore not disturbing other users...

Well, and using UUID as userid instead of incremental integers may be a good advice, as probability to guess other userid will be low.

stockersky
  • 1,531
  • 2
  • 20
  • 36
  • The DRF solution is cool, but isn't `obj.owner == request.user` going to prevent a logged in user to CREATE an object belonging to ANOTHER user? – Munim Munna Oct 28 '20 at 14:42