fix(api): POST/PATCH/PUT'ing RRset payload of invalid structure

Before this change, we read from request.data before validation, assuming that
it is a dict. Things went wrong if it was a list of dicts (sent to an
RRsetDetail endpoint accidentally), or a string, or something else.

Instead of adding extra checks, this commit removes usage of request.data
before validation. As a consequence, PUT on RRsetDetail now requires
specifying subname and type. (DRF requires to specify all fields for PUT.)

api/desecapi/tests/test_rrsets.py

@@ -172,7 +172,7 @@ class AuthenticatedRRSetTestCase(AuthenticatedRRSetBaseTestCase):
     def test_update_my_rr_sets(self):
         for subname in self.SUBNAMES:
             with self.assertPdnsRequests(self.requests_desec_rr_sets_update(name=self.my_rr_set_domain.name)):
-                data = {'records': ['2.2.3.4'], 'ttl': 30}
+                data = {'records': ['2.2.3.4'], 'ttl': 30, 'type': 'A', 'subname': subname}
                 response = self.client.put_rr_set(self.my_rr_set_domain.name, subname, 'A', data)
                 self.assertStatus(response, status.HTTP_200_OK)
 
@@ -187,6 +187,20 @@ class AuthenticatedRRSetTestCase(AuthenticatedRRSetBaseTestCase):
             response = self.client.put_rr_set(self.my_rr_set_domain.name, subname, 'A', {'ttl': 37})
             self.assertStatus(response, status.HTTP_400_BAD_REQUEST)
 
+    def test_update_my_rr_set_with_invalid_payload_type(self):
+        for subname in self.SUBNAMES:
+            data = [{'records': ['2.2.3.4'], 'ttl': 30, 'type': 'A', 'subname': subname}]
+            response = self.client.put_rr_set(self.my_rr_set_domain.name, subname, 'A', data)
+            self.assertStatus(response, status.HTTP_400_BAD_REQUEST)
+            self.assertEquals(response.data['non_field_errors'][0],
+                              'Invalid data. Expected a dictionary, but got list.')
+
+            data = 'foobar'
+            response = self.client.put_rr_set(self.my_rr_set_domain.name, subname, 'A', data)
+            self.assertStatus(response, status.HTTP_400_BAD_REQUEST)
+            self.assertEquals(response.data['non_field_errors'][0],
+                              'Invalid data. Expected a dictionary, but got str.')
+
     def test_partially_update_my_rr_sets(self):
         for subname in self.SUBNAMES:
             current_rr_set = self.client.get_rr_set(self.my_rr_set_domain.name, subname, 'A').data

api/desecapi/views.py

@@ -3,7 +3,6 @@ import binascii
 import ipaddress
 import os
 import re
-from copy import deepcopy
 from datetime import timedelta
 
 import django.core.exceptions
@@ -256,19 +255,8 @@ class RRsetDetail(IdempotentDestroy, DomainView, RetrieveUpdateDestroyAPIView):
         return super().get_serializer(*args, **kwargs)
 
     def update(self, request, *args, **kwargs):
-        # Attach URL parameters (self.kwargs) to the data object (copied from request.body),
-        # the latter having preference with both are given.
-        data = deepcopy(request.data)
-        for k in ('type', 'subname'):
-            data[k] = request.data.pop(k, self.kwargs[k])
-
-        partial = kwargs.pop('partial', False)
-        instance = self.get_object()
-        serializer = self.get_serializer(instance, data=data, partial=partial)
-        serializer.is_valid(raise_exception=True)
-
-        self.perform_update(serializer)
-        response = Response(serializer.data)
+        response = super().update(request, *args, **kwargs)
+
         if response.data is None:
             response.status_code = 204
         return response

docs/rrsets.rst

@@ -272,8 +272,9 @@ Modifying an RRset
 To modify an RRset, use the endpoint that you would also use to retrieve that
 specific RRset.  The API allows changing the values of ``records`` and
 ``ttl``.  When using the ``PATCH`` method, only fields you would like to modify
-need to be provided, where the ``PUT`` method requires specification of both
-fields.  Examples::
+need to be provided.  In contrast, if you use ``PUT``, the full resource must
+be specified (that is, all fields, including ``subname`` and ``type``).
+Examples::
 
     http PUT \
         https://desec.io/api/v1/domains/:name/rrsets/:subname/:type/ \

test/e2e/spec/api_spec.js

@@ -679,7 +679,7 @@ describe("API v1", function () {
                         ).then(function () {
                             return chakram.put(
                                 '/domains/' + domain + '/rrsets/single.../AAAA/',
-                                { 'records': ['fefe::bade'], 'ttl': 31 }
+                                { 'subname': 'single', 'type': 'AAAA', 'records': ['fefe::bade'], 'ttl': 31 }
                             );
                         });
                         expect(response).to.have.status(200);