fix(api): fix unknown RRset PUT/PATCH on RRsetDetail endpoint, fix #229

get_object() is now very close to the standard DRF implementation

api/desecapi/tests/test_rrsets.py

@@ -256,14 +256,26 @@ class AuthenticatedRRSetTestCase(AuthenticatedRRSetBaseTestCase):
         self.assertEqual(response.data['domain'], self.my_rr_set_domain.name)
         self.assertEqual(response.data['name'], 'test.' + self.my_rr_set_domain.name + '.')
 
+    def test_update_unknown_rrset(self):
+        url = self.reverse('v1:rrset', name=self.my_rr_set_domain.name, subname='doesnotexist', type='A')
+        data = {'records': ['3.2.3.4'], 'ttl': 120}
+
+        response = self.client.patch(url, data)
+        self.assertStatus(response, status.HTTP_404_NOT_FOUND)
+
+        response = self.client.put(url, data)
+        self.assertStatus(response, status.HTTP_404_NOT_FOUND)
+
     def test_delete_my_rr_sets_with_patch(self):
         for subname in self.SUBNAMES:
             with self.assertPdnsRequests(self.requests_desec_rr_sets_update(name=self.my_rr_set_domain.name)):
                 response = self.client.patch_rr_set(self.my_rr_set_domain.name, subname=subname, type_='A', records=[])
                 self.assertStatus(response, status.HTTP_204_NO_CONTENT)
 
+            # Deletion is only idempotent via DELETE. For PATCH/PUT, the view raises 404 if the instance does not
+            # exist. By that time, the view has not parsed the payload yet and does not know it is a deletion.
             response = self.client.patch_rr_set(self.my_rr_set_domain.name, subname=subname, type_='A', records=[])
-            self.assertStatus(response, status.HTTP_204_NO_CONTENT)
+            self.assertStatus(response, status.HTTP_404_NOT_FOUND)
 
             response = self.client.get_rr_set(self.my_rr_set_domain.name, subname=subname, type_='A')
             self.assertStatus(response, status.HTTP_404_NOT_FOUND)
@@ -274,6 +286,9 @@ class AuthenticatedRRSetTestCase(AuthenticatedRRSetBaseTestCase):
                 response = self.client.delete_rr_set(self.my_rr_set_domain.name, subname=subname, type_='A')
                 self.assertStatus(response, status.HTTP_204_NO_CONTENT)
 
+            response = self.client.delete_rr_set(self.my_rr_set_domain.name, subname=subname, type_='A')
+            self.assertStatus(response, status.HTTP_204_NO_CONTENT)
+
             response = self.client.get_rr_set(self.my_rr_set_domain.name, subname=subname, type_='A')
             self.assertStatus(response, status.HTTP_404_NOT_FOUND)
 

api/desecapi/views.py

@@ -24,7 +24,7 @@ from rest_framework import mixins
 from rest_framework import status
 from rest_framework.authentication import get_authorization_header
 from rest_framework.exceptions import (NotFound, PermissionDenied, ValidationError)
-from rest_framework.generics import ListCreateAPIView, RetrieveUpdateDestroyAPIView, UpdateAPIView
+from rest_framework.generics import ListCreateAPIView, RetrieveUpdateDestroyAPIView, UpdateAPIView, get_object_or_404
 from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
 from rest_framework.reverse import reverse
@@ -240,17 +240,16 @@ class RRsetDetail(IdempotentDestroy, DomainView, RetrieveUpdateDestroyAPIView):
     def get_queryset(self):
         return self.domain.rrset_set
 
-    def get_object(self, raise_exception=True):
+    def get_object(self):
         queryset = self.filter_queryset(self.get_queryset())
-        result = queryset.filter(type=self.kwargs['type'], subname=self.kwargs['subname'])
-        if result:
-            self.check_object_permissions(self.request, result[0])
-            return result[0]
-        else:
-            if raise_exception:
-                raise Http404
-            else:
-                return None
+
+        filter_kwargs = {k: self.kwargs[k] for k in ['subname', 'type']}
+        obj = get_object_or_404(queryset, **filter_kwargs)
+
+        # May raise a permission denied
+        self.check_object_permissions(self.request, obj)
+
+        return obj
 
     def get_serializer(self, *args, **kwargs):
         kwargs['domain'] = self.domain
@@ -264,7 +263,7 @@ class RRsetDetail(IdempotentDestroy, DomainView, RetrieveUpdateDestroyAPIView):
             data[k] = request.data.pop(k, self.kwargs[k])
 
         partial = kwargs.pop('partial', False)
-        instance = self.get_object(raise_exception=False)
+        instance = self.get_object()
         serializer = self.get_serializer(instance, data=data, partial=partial)
         serializer.is_valid(raise_exception=True)