0

I have a table that stores parent - child records. Today, I noticed a problem while writing a unit test. Even if the requester is not the parent of the child, they can delete the record. But on the view side, if the user is not the owner of the record, the delete button is not active. So it works in the view side. If I make the request using postman, the record is deleted regardless of who owns it.

How can I check that whether the user is the owner of the record? I am following an example project and the creator of that project has done a similar permission.

Child List Model

class ChildList(models.Model):
    parent = models.ForeignKey(
        "account.ParentProfile",
        on_delete=models.CASCADE,
        related_name="parent_children",
        verbose_name=AccountStrings.ChildListString.parent_verbose_name)
    child = models.OneToOneField(
        "account.ChildProfile",
        on_delete=models.CASCADE,
        related_name="child_list",
        verbose_name=AccountStrings.ChildListString.child_verbose_name)

    def __str__(self):
        return f"{self.parent.user.first_name} {self.parent.user.last_name} - {self.child.user.first_name} {self.child.user.last_name}"

    class Meta:
        verbose_name = AccountStrings.ChildListString.meta_verbose_name
        verbose_name_plural = AccountStrings.ChildListString.meta_verbose_name_plural

Child List Destroy View

class ChildListItemDestroyAPIView(RetrieveDestroyAPIView):
    """
        Returns a destroy view by child id value.
    """
    queryset = ChildList.objects.all()
    serializer_class = ChildListSerializer
    permission_classes = [IsAuthenticated, IsParent, IsOwnChild]
    
    def get_object(self):
        queryset = self.get_queryset()
        obj = get_object_or_404(ChildList,child = self.kwargs["child_id"])
        return obj

Permission

class IsOwnChild(BasePermission):
    """
        To edit or destroy a child list record, the user must be owner of that record.
    """
    def has_permission(self, request, view):
        return request.user.user_parent and request.user.is_authenticated

    def has_object_permission(self, request, view, obj):
        return (obj.parent == request.user.user_parent) or request.user.is_superuser
    message = AccountStrings.PermissionStrings.is_own_child_message

Unit Test

class ChildListItemDestroyTests(APITestCase):
    login_url = reverse("token_obtain_pair")

    def setUp(self):
        self.username = "johndoe"
        self.password = "test1234"
        self.user_parent = User.objects.create_user(username=self.username, password=self.password, email = "email1@example.com", identity_number = "12345678910", user_type = 3)
        self.user_parent2 = User.objects.create_user(username= "davedoe", password=self.password, email = "email3@example.com", identity_number = "12345678912", user_type = 3)
        self.user_child = User.objects.create_user(username="janedoe", password=self.password, email = "email2@example.com", identity_number = "12345678911", user_type = 2)
        self.child_list_item = ChildList.objects.create(parent = self.user_parent.user_parent, child = self.user_child.user_child)
        self.test_jwt_authentication()

    def test_jwt_authentication(self, username="johndoe", password="test1234"):
        response = self.client.post(self.login_url, data={"username": username, "password": password})
        self.assertEqual(200, response.status_code)
        self.assertTrue("access" in json.loads(response.content))
        self.token = response.data["access"]
        self.client.credentials(HTTP_AUTHORIZATION='Bearer ' + self.token)
  
    def test_child_list_item_delete(self):
        url = reverse("account:child_list_item_destroy", kwargs={"child_id": self.user_child.user_child.user_id})
        response = self.client.delete(url)
        self.assertEqual(204, response.status_code)
   
   
    def test_child_list_item_delete_is_own_child(self):
        self.test_jwt_authentication(username = "davedoe")
        url = reverse("account:child_list_item_destroy", kwargs={"child_id": self.user_child.user_child.user_id})
        response = self.client.delete(url)
        self.assertEqual(403, response.status_code)
ATK
  • 103
  • 1
  • 11

2 Answers2

2

I think it's because you are overriding the get_object method. In the get_object method from the GenericApiView class, there is the following:

# May raise a permission denied
self.check_object_permissions(self.request, obj)

So you added new permissions, but removed the permissions check mechanism. Adding back this line should solve your problem.

jkoestinger
  • 980
  • 6
  • 8
  • Maybe you actually don't need to override it at all, you can maybe use the `lookup_field` and `lookup_url_kwarg` (https://www.django-rest-framework.org/api-guide/generic-views/#genericapiview) to change what property should match which keyword in the url – jkoestinger Jul 27 '21 at 13:18
  • was that all? I could solve it quite simply. Thank you. If I don't override it, I faced problems. I tried to use lookup_field, but doesn't work for this time. However, I usually use lookup_field. – ATK Jul 27 '21 at 13:32
1

How can I check that whether the user is the owner of the record? Your ChildList class has a pk that points to the profile. So the onwer of the record here is account.ParentProfile. The reason you are getting 203 instead of 403 in the test_child_list_item_delete_is_own_child

is because in the view permission_classes you are passing IsAuthenticated which means any user with valid access token can interact with this view.

Therefore the solution is to customize delete and get responses like this

class ChildListItemDestroyAPIView(RetrieveDestroyAPIView):
    """
        Returns a destroy view by child id value.
    """
    queryset = ChildList.objects.all()
    serializer_class = ChildListSerializer
    permission_classes = [IsAuthenticated, IsParent, IsOwnChild]
    
    def get_object(self):
        queryset = self.get_queryset()
        obj = get_object_or_404(ChildList,child = self.kwargs["child_id"])
        return obj

    def delete(self, *args, **kwargs): 
        pass 
        # business logic goes here 

    def get(self, *args, **kwargs): 
        pass 
        # business logic goes here

araldhafeeri
  • 179
  • 9
  • This looks useful. As far as I understand, if Permission Classes already provide all 3 together, it allows in my example. I guess there is no need for customization. The problem was because I did the override and didn't add the permission check. – ATK Jul 27 '21 at 13:42
  • Yes something like that. – araldhafeeri Jul 27 '21 at 13:50