Browse Source

fix(api): improve serializers / model validation, fixes #16, fixes #25

- Better-thought-through exposure of information: Don't expose
  domain `id` anymore, but do expose `created` and `updated` fields
- Validate domain `name` to disallow changing it on update request
Peter Thomassen 8 years ago
parent
commit
99fbc267d6

+ 13 - 0
api/desecapi/models.py

@@ -4,6 +4,7 @@ from django.contrib.auth.models import (
     BaseUserManager, AbstractBaseUser
 )
 from django.utils import timezone
+from django.core.exceptions import ValidationError
 from desecapi import pdns
 import datetime, time
 
@@ -98,6 +99,7 @@ class Domain(models.Model):
     arecord = models.CharField(max_length=255, blank=True)
     aaaarecord = models.CharField(max_length=1024, blank=True)
     owner = models.ForeignKey(settings.AUTH_USER_MODEL, related_name='domains')
+    _dirtyName = False
     _dirtyRecords = False
 
     def __setattr__(self, attrname, val):
@@ -107,6 +109,12 @@ class Domain(models.Model):
         else:
             super(Domain, self).__setattr__(attrname, val)
 
+    def setter_name(self, val):
+        if val != self.name:
+            self._dirtyName = True
+
+        return val
+
     def setter_arecord(self, val):
         if val != self.arecord:
             self._dirtyRecords = True
@@ -119,6 +127,10 @@ class Domain(models.Model):
 
         return val
 
+    def clean(self):
+        if self._dirtyName:
+            raise ValidationError('You must not change the domain name')
+
     def pdns_resync(self):
         """
         Make sure that pdns gets the latest information about this domain/zone.
@@ -167,6 +179,7 @@ class Domain(models.Model):
         new_domain = self.pk is None
 
         self.updated = timezone.now()
+        self.clean()
         super(Domain, self).save(*args, **kwargs)
 
         self.pdns_sync(new_domain)

+ 23 - 2
api/desecapi/serializers.py

@@ -1,5 +1,7 @@
 from rest_framework import serializers
-from desecapi.models import Domain, Donation
+from desecapi.models import Domain, Donation, User
+from djoser import serializers as djoserSerializers
+
 
 class DomainSerializer(serializers.ModelSerializer):
     owner = serializers.ReadOnlyField(source='owner.email')
@@ -7,10 +9,29 @@ class DomainSerializer(serializers.ModelSerializer):
 
     class Meta:
         model = Domain
-        fields = ('id', 'name', 'owner', 'arecord', 'aaaarecord')
+        fields = ('name', 'owner', 'arecord', 'aaaarecord', 'created', 'updated')
+        read_only_fields = ('created', 'updated',)
+
 
 class DonationSerializer(serializers.ModelSerializer):
 
     class Meta:
         model = Donation
         fields = ('name', 'iban', 'bic', 'amount', 'message', 'email')
+
+
+class UserSerializer(djoserSerializers.UserSerializer):
+
+    class Meta(djoserSerializers.UserSerializer.Meta):
+        fields = tuple(User.REQUIRED_FIELDS) + (
+            User.USERNAME_FIELD,
+        )
+
+
+class UserRegistrationSerializer(djoserSerializers.UserRegistrationSerializer):
+
+    class Meta(djoserSerializers.UserRegistrationSerializer.Meta):
+        fields = tuple(User.REQUIRED_FIELDS) + (
+            User.USERNAME_FIELD,
+            'password',
+        )

+ 4 - 0
api/desecapi/settings.py

@@ -111,6 +111,10 @@ DJOSER = {
     'ACTIVATION_URL': '#/activate/{uid}/{token}',
     'LOGIN_AFTER_ACTIVATION': True,
     'SEND_ACTIVATION_EMAIL': False,
+    'SERIALIZERS': {
+        'user': 'desecapi.serializers.UserSerializer',
+        'user_registration': 'desecapi.serializers.UserRegistrationSerializer',
+    },
 }
 
 TEMPLATES = [

+ 12 - 2
api/desecapi/tests/testdomains.py

@@ -92,15 +92,25 @@ class AuthenticatedDomainTests(APITestCase):
         self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     def testCanPutOwnedDomain(self):
+        url = reverse('domain-detail', args=(self.ownedDomains[1].pk,))
+        response = self.client.get(url)
+        response.data['arecord'] = '1.2.3.4'
+        response = self.client.put(url, response.data)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data['arecord'], '1.2.3.4')
+
+    def testCantChangeDomainName(self):
         url = reverse('domain-detail', args=(self.ownedDomains[1].pk,))
         response = self.client.get(url)
         newname = utils.generateDomainname()
         response.data['name'] = newname
         response = self.client.put(url, response.data)
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.status_code, status.HTTP_409_CONFLICT)
         response = self.client.get(url)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.data['name'], newname)
+        self.assertEqual(response.data['name'], self.ownedDomains[1].name)
 
     def testCantPutOtherDomains(self):
         url = reverse('domain-detail', args=(self.otherDomains[1].pk,))

+ 9 - 0
api/desecapi/views.py

@@ -18,6 +18,7 @@ from desecapi.authentication import BasicTokenAuthentication, URLParamAuthentica
 import base64
 from desecapi import settings
 from rest_framework.exceptions import ValidationError
+import django.core.exceptions
 from djoser import views, signals
 from rest_framework import status
 from datetime import datetime, timedelta
@@ -106,6 +107,14 @@ class DomainDetail(generics.RetrieveUpdateDestroyAPIView):
     def get_queryset(self):
         return Domain.objects.filter(owner=self.request.user.pk)
 
+    def update(self, request, *args, **kwargs):
+        try:
+            return super(DomainDetail, self).update(request, *args, **kwargs)
+        except django.core.exceptions.ValidationError as e:
+            ex = ValidationError(detail={"detail": str(e)})
+            ex.status_code = status.HTTP_409_CONFLICT
+            raise ex
+
 
 class DomainDetailByName(DomainDetail):
     lookup_field = 'name'