Browse Source

feat(api): remove perm_dyndns, honor policies in serializers

Peter Thomassen 1 year ago
parent
commit
a34b963d59

+ 54 - 0
api/desecapi/migrations/0037_remove_tokendomainpolicy_perm_dyndns.py

@@ -0,0 +1,54 @@
+# Generated by Django 5.0rc1 on 2023-12-01 15:01
+
+from django.db import migrations, transaction
+from django.db.models import OuterRef, Subquery
+
+
+@transaction.atomic
+def forwards_func(apps, schema_editor):
+    TokenDomainPolicy = apps.get_model("desecapi", "TokenDomainPolicy")
+    db_alias = schema_editor.connection.alias
+
+    # Tokens with perm_dyndns effectively have perm_write for type=A/AAAA on any subname of their domain. We create
+    # corresponding policies explicitly. Uniqueness violation cannot occur (no polices with non-NULL type exist).
+    # We don't need to do anything for policies with perm_dyndns=False; perm_write determines their capabilities.
+    queryset = TokenDomainPolicy.objects.using(db_alias)
+    TokenDomainPolicy.objects.bulk_create(
+        [
+            TokenDomainPolicy(
+                token=policy.token,
+                domain=policy.domain,
+                subname=None,
+                type=type_,
+                perm_write=True,
+            )
+            for policy in queryset.filter(perm_dyndns=True).all()
+            for type_ in ("A", "AAAA")
+        ]
+    )
+    # Now clean up (non-default) policies which have no further use, i.e. where perm_dyndns was different from the
+    # default policy (that was taken care of above), but perm_write is equal to the default policy (that's useless).
+    default_policy = queryset.filter(
+        token=OuterRef("token"),
+        domain__isnull=True,
+        subname__isnull=True,
+        type__isnull=True,
+    )
+    queryset.filter(
+        domain__isnull=False, perm_write=Subquery(default_policy.values("perm_write"))
+    ).exclude(perm_dyndns=Subquery(default_policy.values("perm_dyndns"))).delete()
+
+
+class Migration(migrations.Migration):
+    atomic = False
+    dependencies = [
+        ("desecapi", "0036_remove_tokendomainpolicy_default_policy_on_insert_and_more"),
+    ]
+
+    operations = [
+        migrations.RunPython(forwards_func, atomic=True),
+        migrations.RemoveField(
+            model_name="tokendomainpolicy",
+            name="perm_dyndns",
+        ),
+    ]

+ 4 - 5
api/desecapi/models/tokens.py

@@ -89,7 +89,7 @@ class Token(ExportModelOperationsMixin("Token"), rest_framework.authtoken.models
     def make_hash(plain):
     def make_hash(plain):
         return make_password(plain, salt="static", hasher="pbkdf2_sha256_iter1")
         return make_password(plain, salt="static", hasher="pbkdf2_sha256_iter1")
 
 
-    def get_policy(self, *, domain=None, subname=None, type=None):
+    def get_policy(self, rrset=None):
         order_by = [
         order_by = [
             F(field).asc(
             F(field).asc(
                 nulls_last=True  # default Postgres sorting, but: explicit is better than implicit
                 nulls_last=True  # default Postgres sorting, but: explicit is better than implicit
@@ -98,9 +98,9 @@ class Token(ExportModelOperationsMixin("Token"), rest_framework.authtoken.models
         ]
         ]
         return (
         return (
             self.tokendomainpolicy_set.filter(
             self.tokendomainpolicy_set.filter(
-                Q(domain=domain) | Q(domain__isnull=True),
-                Q(subname=subname) | Q(subname__isnull=True),
-                Q(type=type) | Q(type__isnull=True),
+                Q(domain=rrset.domain if rrset else None) | Q(domain__isnull=True),
+                Q(subname=rrset.subname if rrset else None) | Q(subname__isnull=True),
+                Q(type=rrset.type if rrset else None) | Q(type__isnull=True),
             )
             )
             .order_by(*order_by)
             .order_by(*order_by)
             .first()
             .first()
@@ -120,7 +120,6 @@ class TokenDomainPolicy(ExportModelOperationsMixin("TokenDomainPolicy"), models.
     type = models.CharField(
     type = models.CharField(
         max_length=10, null=True, validators=RRset.type.field._validators
         max_length=10, null=True, validators=RRset.type.field._validators
     )
     )
-    perm_dyndns = models.BooleanField(default=False)
     perm_write = models.BooleanField(default=False)
     perm_write = models.BooleanField(default=False)
     # Token user, filled via trigger. Used by compound FK constraints to tie domain.owner to token.user (see migration).
     # Token user, filled via trigger. Used by compound FK constraints to tie domain.owner to token.user (see migration).
     token_user = models.ForeignKey(
     token_user = models.ForeignKey(

+ 14 - 40
api/desecapi/permissions.py

@@ -2,6 +2,8 @@ from ipaddress import IPv4Address, IPv4Network
 
 
 from rest_framework import permissions
 from rest_framework import permissions
 
 
+from desecapi.models import RRset
+
 
 
 class IsActiveUser(permissions.BasePermission):
 class IsActiveUser(permissions.BasePermission):
     """
     """
@@ -70,60 +72,32 @@ class IsDomainOwner(permissions.BasePermission):
     Custom permission to only allow owners of a domain to view or edit an object owned by that domain.
     Custom permission to only allow owners of a domain to view or edit an object owned by that domain.
     """
     """
 
 
-    def has_object_permission(self, request, view, obj):
-        return obj.domain.owner == request.user
-
-
-class TokenNoDomainPolicy(permissions.BasePermission):
-    """
-    Permission to check whether a token is unrestricted by any domain policy.
-    """
-
     def has_permission(self, request, view):
     def has_permission(self, request, view):
-        return request.auth.get_policy(domain=None) is None
-
+        return request.user == view.domain.owner
 
 
-class TokenDomainPolicyBasePermission(permissions.BasePermission):
-    """
-    Base permission to check whether a token authorizes specific actions on a domain.
-    """
 
 
-    perm_field = None
-
-    def _has_object_permission(self, request, view, obj):
-        policy = request.auth.get_policy(domain=obj)
-
-        # If the token has no domain policy, there are no restrictions
-        if policy is None:
-            return True
-
-        # Otherwise, return the requested permission
-        return getattr(policy, self.perm_field)
-
-
-class TokenHasDomainBasePermission(TokenDomainPolicyBasePermission):
+class TokenNoDomainPolicy(permissions.BasePermission):
     """
     """
-    Base permission for checking a token's domain policy, for the view domain.
+    Permission to check whether a token is unrestricted by any policy.
     """
     """
 
 
     def has_permission(self, request, view):
     def has_permission(self, request, view):
-        return self._has_object_permission(request, view, view.domain)
+        return not request.auth.tokendomainpolicy_set.exists()
 
 
 
 
-class TokenHasDomainDynDNSPermission(TokenHasDomainBasePermission):
+class TokenHasRRsetPermission(permissions.BasePermission):
     """
     """
-    Custom permission to check whether a token authorizes using the dynDNS interface for the view domain.
+    Permission to check whether a token authorizes writing the view's RRset.
     """
     """
 
 
-    perm_field = "perm_dyndns"
-
+    code = "forbidden"
+    message = "Insufficient token permissions."
 
 
-class TokenHasDomainRRsetsPermission(TokenHasDomainBasePermission):
-    """
-    Custom permission to check whether a token authorizes accessing RRsets for the view domain.
-    """
+    def has_object_permission(self, request, view, obj):
+        policy = request.auth.get_policy(obj)
 
 
-    perm_field = "perm_write"
+        # Pass if there's no policy, otherwise return the permission
+        return (policy is None) or policy.perm_write
 
 
 
 
 class AuthTokenCorrespondsToViewToken(permissions.BasePermission):
 class AuthTokenCorrespondsToViewToken(permissions.BasePermission):

+ 1 - 1
api/desecapi/serializers/domains.py

@@ -140,7 +140,7 @@ class DomainSerializer(serializers.ModelSerializer):
             ]
             ]
 
 
             rrset_list_serializer = RRsetSerializer(
             rrset_list_serializer = RRsetSerializer(
-                data=data, context=dict(domain=domain), many=True
+                data=data, context=dict(self.context, domain=domain), many=True
             )
             )
             # The following line raises if data passed validation by dnspython during zone file parsing,
             # The following line raises if data passed validation by dnspython during zone file parsing,
             # but is rejected by validation in RRsetSerializer. See also
             # but is rejected by validation in RRsetSerializer. See also

+ 6 - 6
api/desecapi/serializers/records.py

@@ -12,8 +12,7 @@ from rest_framework.settings import api_settings
 from rest_framework.validators import UniqueTogetherValidator
 from rest_framework.validators import UniqueTogetherValidator
 
 
 from api import settings
 from api import settings
-from desecapi import metrics, models
-from desecapi.validators import ExclusionConstraintValidator, ReadOnlyOnUpdateValidator
+from desecapi import metrics, models, validators
 
 
 
 
 class ConditionalExistenceModelSerializer(serializers.ModelSerializer):
 class ConditionalExistenceModelSerializer(serializers.ModelSerializer):
@@ -83,7 +82,7 @@ class NonBulkOnlyDefault:
     def __call__(self, serializer_field):
     def __call__(self, serializer_field):
         is_many = getattr(serializer_field.root, "many", False)
         is_many = getattr(serializer_field.root, "many", False)
         if is_many:
         if is_many:
-            raise serializers.SkipField()
+            serializer_field.fail("required")
         if callable(self.default):
         if callable(self.default):
             if getattr(self.default, "requires_context", False):
             if getattr(self.default, "requires_context", False):
                 return self.default(serializer_field)
                 return self.default(serializer_field)
@@ -394,19 +393,20 @@ class RRsetSerializer(ConditionalExistenceModelSerializer):
 
 
     def get_fields(self):
     def get_fields(self):
         fields = super().get_fields()
         fields = super().get_fields()
-        fields["subname"].validators.append(ReadOnlyOnUpdateValidator())
-        fields["type"].validators.append(ReadOnlyOnUpdateValidator())
+        fields["subname"].validators.append(validators.ReadOnlyOnUpdateValidator())
+        fields["type"].validators.append(validators.ReadOnlyOnUpdateValidator())
         fields["ttl"].validators.append(MinValueValidator(limit_value=self.minimum_ttl))
         fields["ttl"].validators.append(MinValueValidator(limit_value=self.minimum_ttl))
         return fields
         return fields
 
 
     def get_validators(self):
     def get_validators(self):
         return [
         return [
+            validators.PermissionValidator(),
             UniqueTogetherValidator(
             UniqueTogetherValidator(
                 self.domain.rrset_set,
                 self.domain.rrset_set,
                 ("subname", "type"),
                 ("subname", "type"),
                 message="Another RRset with the same subdomain and type exists for this domain.",
                 message="Another RRset with the same subdomain and type exists for this domain.",
             ),
             ),
-            ExclusionConstraintValidator(
+            validators.ExclusionConstraintValidator(
                 self.domain.rrset_set,
                 self.domain.rrset_set,
                 ("subname",),
                 ("subname",),
                 exclusion_condition=(
                 exclusion_condition=(

+ 0 - 1
api/desecapi/serializers/tokens.py

@@ -52,7 +52,6 @@ class TokenDomainPolicySerializer(serializers.ModelSerializer):
             "domain",
             "domain",
             "subname",
             "subname",
             "type",
             "type",
-            "perm_dyndns",
             "perm_write",
             "perm_write",
         )
         )
         extra_kwargs = {
         extra_kwargs = {

+ 76 - 0
api/desecapi/tests/test_rrsets.py

@@ -1,3 +1,4 @@
+from contextlib import nullcontext
 from ipaddress import IPv4Network
 from ipaddress import IPv4Network
 import re
 import re
 from itertools import product
 from itertools import product
@@ -1541,6 +1542,81 @@ class AuthenticatedRRSetTestCase(AuthenticatedRRSetBaseTestCase):
                 ],
                 ],
             )
             )
 
 
+    def test_rrsets_policies(self):
+        domain = self.my_empty_domain
+
+        def assertRequests(*, allowed):
+            cm = (
+                self.assertRequests(self.requests_desec_rr_sets_update(domain.name))
+                if allowed
+                else nullcontext()
+            )
+
+            data = {"subname": "www", "type": "A", "ttl": 3600, "records": ["1.2.3.4"]}
+            with cm:
+                self.assertStatus(
+                    self.client.post_rr_set(domain_name=domain.name, **data),
+                    status.HTTP_201_CREATED if allowed else status.HTTP_403_FORBIDDEN,
+                )
+
+            data["records"] = ["4.3.2.1"]
+            with cm:
+                self.assertStatus(
+                    self.client.put_rr_set(domain.name, "www", "A", data),
+                    status.HTTP_200_OK if allowed else status.HTTP_404_NOT_FOUND,
+                )
+
+            data["records"] = []  # delete
+            with cm:
+                self.assertStatus(
+                    self.client.patch_rr_set(domain.name, "www", "A", data),
+                    status.HTTP_204_NO_CONTENT
+                    if allowed
+                    else status.HTTP_404_NOT_FOUND,
+                )
+
+            self.assertStatus(
+                self.client.patch_rr_set(domain.name, "www", "A", data),
+                status.HTTP_404_NOT_FOUND,  # no permission needed to see that
+            )
+
+            self.assertStatus(
+                self.client.delete_rr_set(domain.name, "www", "A"),
+                status.HTTP_204_NO_CONTENT,  # no permission needed for idempotency
+            )
+
+            if not allowed:
+                # Create RRset manually so we cn try manipulating it
+                data["contents"] = data.pop("records")
+                self.my_empty_domain.rrset_set.create(**data)
+                data["records"] = data.pop("contents")
+
+                for response in [
+                    self.client.patch_rr_set(domain.name, "www", "A", data),
+                    self.client.put_rr_set(domain.name, "www", "A", data),
+                    self.client.delete_rr_set(domain.name, "www", "A"),
+                ]:
+                    self.assertStatus(response, status.HTTP_403_FORBIDDEN)
+
+            # Clean up
+            rrset_qs = domain.rrset_set.filter(subname="www", type="A")
+            if not allowed:
+                self.assertTrue(rrset_qs.exists())
+                rrset_qs.delete()
+            self.assertFalse(rrset_qs.exists())
+
+        assertRequests(allowed=True)
+
+        qs = self.token.tokendomainpolicy_set
+        qs.create(domain=None, subname=None, type=None)
+        assertRequests(allowed=False)
+
+        qs.create(domain=domain, subname=None, type="A", perm_write=True)
+        assertRequests(allowed=True)
+
+        qs.create(domain=domain, subname="www", type="A", perm_write=False)
+        assertRequests(allowed=False)
+
 
 
 class AuthenticatedRRSetLPSTestCase(AuthenticatedRRSetBaseTestCase):
 class AuthenticatedRRSetLPSTestCase(AuthenticatedRRSetBaseTestCase):
     DYN = True
     DYN = True

+ 74 - 0
api/desecapi/tests/test_rrsets_bulk.py

@@ -1,3 +1,4 @@
+from contextlib import nullcontext
 import copy
 import copy
 
 
 from django.conf import settings
 from django.conf import settings
@@ -737,3 +738,76 @@ class AuthenticatedRRSetBulkTestCase(AuthenticatedRRSetBaseTestCase):
             ),
             ),
             status.HTTP_405_METHOD_NOT_ALLOWED,
             status.HTTP_405_METHOD_NOT_ALLOWED,
         )
         )
+
+    def test_bulk_rrsets_policies(self):
+        domain = self.my_empty_domain
+
+        def assertRequests(*, allowed):
+            cm = (
+                self.assertRequests(self.requests_desec_rr_sets_update(domain.name))
+                if allowed
+                else nullcontext()
+            )
+
+            data = [
+                {"subname": "www", "type": "A", "ttl": 3600, "records": ["1.2.3.4"]},
+                {"subname": "sub", "type": "A", "ttl": 3600, "records": ["1.2.3.4"]},
+            ]
+            with cm:
+                self.assertStatus(
+                    self.client.bulk_post_rr_sets(domain.name, data),
+                    status.HTTP_201_CREATED if allowed else status.HTTP_403_FORBIDDEN,
+                )
+
+            data[0]["records"] = ["4.3.2.1"]
+            with cm:
+                self.assertStatus(
+                    self.client.bulk_put_rr_sets(domain.name, data),
+                    status.HTTP_200_OK if allowed else status.HTTP_403_FORBIDDEN,
+                )
+
+            rrset_qs = domain.rrset_set.filter(type="A")
+            self.assertEqual(rrset_qs.exists(), allowed)
+
+            data[0]["records"] = data[1]["records"] = []  # delete
+            with cm:
+                self.assertStatus(
+                    self.client.bulk_patch_rr_sets(domain.name, data),
+                    status.HTTP_200_OK if allowed else status.HTTP_403_FORBIDDEN,
+                )
+
+            self.assertStatus(
+                self.client.bulk_patch_rr_sets(domain.name, data),
+                status.HTTP_200_OK if allowed else status.HTTP_403_FORBIDDEN,
+            )
+
+            if not allowed:
+                # Create RRset manually so we cn try manipulating it
+                for item in data:
+                    item["contents"] = item.pop("records")
+                    self.my_empty_domain.rrset_set.create(**item)
+                    item["records"] = item.pop("contents")
+
+                for response in [
+                    self.client.bulk_patch_rr_sets(domain.name, data),
+                    self.client.bulk_put_rr_sets(domain.name, data),
+                ]:
+                    self.assertStatus(response, status.HTTP_403_FORBIDDEN)
+
+            # Clean up
+            if not allowed:
+                self.assertTrue(rrset_qs.exists())
+            rrset_qs.delete()
+            self.assertFalse(rrset_qs.exists())
+
+        assertRequests(allowed=True)
+
+        qs = self.token.tokendomainpolicy_set
+        qs.create(domain=None, subname=None, type=None)
+        assertRequests(allowed=False)
+
+        qs.create(domain=domain, subname=None, type="A", perm_write=True)
+        assertRequests(allowed=True)
+
+        qs.create(domain=domain, subname="www", type="A", perm_write=False)
+        assertRequests(allowed=False)

+ 82 - 58
api/desecapi/tests/test_token_domain_policy.py

@@ -1,5 +1,3 @@
-from contextlib import nullcontext
-
 from django.db import transaction
 from django.db import transaction
 from django.db.utils import IntegrityError
 from django.db.utils import IntegrityError
 from rest_framework import status
 from rest_framework import status
@@ -49,7 +47,7 @@ class TokenDomainPolicyClient(APIClient):
 
 
 class TokenDomainPolicyTestCase(DomainOwnerTestCase):
 class TokenDomainPolicyTestCase(DomainOwnerTestCase):
     client_class = TokenDomainPolicyClient
     client_class = TokenDomainPolicyClient
-    default_data = dict(perm_dyndns=False, perm_write=False)
+    default_data = dict(perm_write=False)
 
 
     def setUp(self):
     def setUp(self):
         super().setUp()
         super().setUp()
@@ -59,7 +57,9 @@ class TokenDomainPolicyTestCase(DomainOwnerTestCase):
 
 
     def test_get_policy(self):
     def test_get_policy(self):
         def get_policy(domain, subname, type):
         def get_policy(domain, subname, type):
-            return self.token.get_policy(domain=domain, subname=subname, type=type)
+            return self.token.get_policy(
+                models.RRset(domain=domain, subname=subname, type=type)
+            )
 
 
         def assertPolicy(policy, domain, subname, type):
         def assertPolicy(policy, domain, subname, type):
             self.assertEqual(policy.domain, domain)
             self.assertEqual(policy.domain, domain)
@@ -174,7 +174,7 @@ class TokenDomainPolicyTestCase(DomainOwnerTestCase):
             self.assertStatus(response, status.HTTP_403_FORBIDDEN)
             self.assertStatus(response, status.HTTP_403_FORBIDDEN)
 
 
             # Change
             # Change
-            data = dict(perm_dyndns=False, perm_write=True)
+            data = dict(perm_write=True)
             policy = target.get_policy()
             policy = target.get_policy()
             response = self.client.patch_policy(
             response = self.client.patch_policy(
                 target, using=self.token, policy_id=policy.pk, data=data
                 target, using=self.token, policy_id=policy.pk, data=data
@@ -265,7 +265,6 @@ class TokenDomainPolicyTestCase(DomainOwnerTestCase):
             "domain": self.my_domains[0].name,
             "domain": self.my_domains[0].name,
             "subname": None,
             "subname": None,
             "type": None,
             "type": None,
-            "perm_dyndns": True,
         }
         }
         response = self.client.create_policy(
         response = self.client.create_policy(
             self.token, using=self.token_manage, data=data
             self.token, using=self.token_manage, data=data
@@ -298,7 +297,6 @@ class TokenDomainPolicyTestCase(DomainOwnerTestCase):
             domain=self.my_domains[1].name,
             domain=self.my_domains[1].name,
             subname="_acme-challenge",
             subname="_acme-challenge",
             type="TXT",
             type="TXT",
-            perm_dyndns=False,
             perm_write=True,
             perm_write=True,
         )
         )
         response = self.client.patch_policy(
         response = self.client.patch_policy(
@@ -332,7 +330,7 @@ class TokenDomainPolicyTestCase(DomainOwnerTestCase):
         )
         )
 
 
         ## partially modify the default policy
         ## partially modify the default policy
-        data = dict(perm_dyndns=True)
+        data = dict()
         response = self.client.patch_policy(
         response = self.client.patch_policy(
             self.token, using=self.token_manage, policy_id=default_policy_id, data=data
             self.token, using=self.token_manage, policy_id=default_policy_id, data=data
         )
         )
@@ -398,47 +396,6 @@ class TokenDomainPolicyTestCase(DomainOwnerTestCase):
                     setattr(policy, perm, False)
                     setattr(policy, perm, False)
                 policy.save()
                 policy.save()
 
 
-        def _perform_requests(name, perm, value, **kwargs):
-            responses = []
-            if value:
-                pdns_name = self._normalize_name(name).lower()
-                cm = self.assertNoRequestsBut(
-                    self.request_pdns_zone_update(name=pdns_name),
-                    self.request_pdns_zone_axfr(name=pdns_name),
-                )
-            else:
-                cm = nullcontext()
-
-            if perm == "perm_dyndns":
-                data = {"username": name, "password": self.token.plain}
-                with cm:
-                    responses.append(
-                        self.client.get(self.reverse("v1:dyndns12update"), data)
-                    )
-                return responses
-
-            if perm == "perm_write":
-                url_detail = self.reverse("v1:rrset@", name=name, subname="", type="A")
-                url_list = self.reverse("v1:rrsets", name=name)
-
-                responses.append(self.client.get(url_list, **kwargs))
-                responses.append(self.client.patch(url_list, [], **kwargs))
-                responses.append(self.client.put(url_list, [], **kwargs))
-                responses.append(self.client.post(url_list, [], **kwargs))
-
-                data = {"subname": "", "type": "A", "ttl": 3600, "records": ["1.2.3.4"]}
-                with cm:
-                    responses += [
-                        self.client.delete(url_detail, **kwargs),
-                        self.client.post(url_list, data=data, **kwargs),
-                        self.client.put(url_detail, data=data, **kwargs),
-                        self.client.patch(url_detail, data=data, **kwargs),
-                        self.client.get(url_detail, **kwargs),
-                    ]
-                return responses
-
-            raise ValueError(f"Unexpected permission: {perm}")
-
         # Create
         # Create
         ## default policy
         ## default policy
         data = {"domain": None, "subname": None, "type": None}
         data = {"domain": None, "subname": None, "type": None}
@@ -484,15 +441,6 @@ class TokenDomainPolicyTestCase(DomainOwnerTestCase):
                     setattr(policy, perm, value)
                     setattr(policy, perm, value)
                     policy.save()
                     policy.save()
 
 
-                    # Perform requests that test this permission and inspect responses
-                    for response in _perform_requests(
-                        domain.name, perm, value, **kwargs
-                    ):
-                        if value:
-                            self.assertIn(response.status_code, range(200, 300))
-                        else:
-                            self.assertStatus(response, status.HTTP_403_FORBIDDEN)
-
                     # Can't create domain
                     # Can't create domain
                     data = {"name": self.random_domain_name()}
                     data = {"name": self.random_domain_name()}
                     response = self.client.post(
                     response = self.client.post(
@@ -500,10 +448,86 @@ class TokenDomainPolicyTestCase(DomainOwnerTestCase):
                     )
                     )
                     self.assertStatus(response, status.HTTP_403_FORBIDDEN)
                     self.assertStatus(response, status.HTTP_403_FORBIDDEN)
 
 
+                    # Can't delete domain
+                    response = self.client.delete(
+                        self.reverse("v1:domain-detail", name=domain), {}, **kwargs
+                    )
+                    self.assertStatus(response, status.HTTP_403_FORBIDDEN)
+
                     # Can't access account details
                     # Can't access account details
                     response = self.client.get(self.reverse("v1:account"), **kwargs)
                     response = self.client.get(self.reverse("v1:account"), **kwargs)
                     self.assertStatus(response, status.HTTP_403_FORBIDDEN)
                     self.assertStatus(response, status.HTTP_403_FORBIDDEN)
 
 
+    def test_dyndns_permission(self):
+        def _perform_request(**kwargs):
+            return self.client.get(
+                self.reverse("v1:dyndns12update"),
+                {
+                    "username": self.my_domains[1].name,
+                    "password": self.token.plain,
+                    **kwargs,
+                },
+            )
+
+        def assert_allowed(**kwargs):
+            response = _perform_request(**kwargs)
+            self.assertStatus(response, status.HTTP_200_OK)
+            self.assertEqual(response.data, "good")
+
+        def assert_forbidden(**kwargs):
+            response = _perform_request(**kwargs)
+            self.assertStatus(response, status.HTTP_403_FORBIDDEN)
+            self.assertEqual(response.data["detail"], "Insufficient token permissions.")
+
+        # No policy
+        assert_allowed(
+            myipv4=""
+        )  # empty IPv4 and delete IPv6 (no-op, prevents pdns request)
+
+        # Default policy (deny)
+        qs = self.token.tokendomainpolicy_set
+        qs.create(domain=None, subname=None, type=None)
+        assert_forbidden(myipv4="")
+        assert_allowed(
+            myipv4="preserve", myipv6="preserve"
+        )  # no-op needs no permissions
+
+        # Only A permission
+        qs.create(domain=self.my_domains[1], subname=None, type="A", perm_write=True)
+        assert_forbidden(myipv4="")
+        assert_allowed(myipv4="", myipv6="preserve")  # just IPv4
+
+        # Only A permission
+        qs.create(domain=self.my_domains[1], subname=None, type="AAAA")
+        assert_forbidden(myipv4="")
+        assert_allowed(myipv4="", myipv6="preserve")  # just IPv4
+
+        # A + AAAA permission
+        qs.filter(domain=self.my_domains[1], type="AAAA").update(perm_write=True)
+        assert_allowed(myipv4="")  # empty IPv4 and delete IPv6
+
+        # Only AAAA permission
+        qs.filter(domain=self.my_domains[1], type="A").update(perm_write=False)
+        assert_forbidden(myipv4="")
+        assert_allowed(myipv4="preserve", myipv6="")  # just IPv6
+
+        # Update default policy to allow, but A deny policy overrides
+        qs.filter(domain__isnull=True).update(perm_write=True)
+        assert_forbidden(myipv4="")
+        assert_allowed(myipv4="preserve", myipv6="")  # just IPv6
+
+        # AAAA (allow) and A (allow via default policy fallback)
+        qs.filter(domain=self.my_domains[1], type="A").delete()
+        assert_allowed(myipv4="", myipv6="")
+
+        # Default policy (allow)
+        qs.filter(domain=self.my_domains[1]).delete()
+        assert_allowed(myipv4="", myipv6="")
+
+        # No policy
+        qs.filter().delete()
+        assert_allowed(myipv4="", myipv6="")
+
     def test_domain_owner_consistency(self):
     def test_domain_owner_consistency(self):
         models.TokenDomainPolicy(
         models.TokenDomainPolicy(
             token=self.token, domain=None, subname=None, type=None
             token=self.token, domain=None, subname=None, type=None

+ 31 - 1
api/desecapi/validators.py

@@ -1,9 +1,11 @@
 from django.db import DataError
 from django.db import DataError
 from django.db.models import Model
 from django.db.models import Model
-from rest_framework import serializers
+from rest_framework import exceptions, serializers
 from rest_framework.exceptions import ValidationError
 from rest_framework.exceptions import ValidationError
 from rest_framework.validators import qs_exists, qs_filter, UniqueTogetherValidator
 from rest_framework.validators import qs_exists, qs_filter, UniqueTogetherValidator
 
 
+from desecapi.permissions import TokenHasRRsetPermission
+
 
 
 def qs_exclude(queryset, **kwargs):
 def qs_exclude(queryset, **kwargs):
     try:
     try:
@@ -59,6 +61,34 @@ class ExclusionConstraintValidator(UniqueTogetherValidator):
             raise ValidationError(message, code="exclusive")
             raise ValidationError(message, code="exclusive")
 
 
 
 
+class PermissionValidator:
+    """
+    Validator that checks write permission for an RRset.
+    """
+
+    requires_context = True
+
+    def __call__(self, attrs, serializer):
+        # On the RRsetDetail apex endpoint, subname is not in attrs
+        subname = attrs.get("subname")
+        if subname is None:
+            subname = serializer.context["view"].kwargs["subname"]
+        # On the RRsetDetail endpoint, the type is not in attrs
+        type_ = attrs.get("type") or serializer.instance.type
+
+        rrset = serializer.Meta.model(
+            domain=serializer.domain, subname=subname, type=type_
+        )
+        permission = TokenHasRRsetPermission()
+        if not permission.has_object_permission(
+            serializer.context.get("request"), None, rrset
+        ):
+            raise exceptions.PermissionDenied(
+                detail=getattr(permission, "message", None),
+                code=getattr(permission, "code", None),
+            )
+
+
 class Validator:
 class Validator:
     message = "This field did not pass validation."
     message = "This field did not pass validation."
 
 

+ 0 - 2
api/desecapi/views/domains.py

@@ -40,8 +40,6 @@ class DomainViewSet(
         ]
         ]
         if self.action == "create":
         if self.action == "create":
             ret.append(permissions.WithinDomainLimit)
             ret.append(permissions.WithinDomainLimit)
-        if self.action == "zonefile":
-            ret.append(permissions.TokenHasDomainRRsetsPermission)
         if self.request.method not in SAFE_METHODS:
         if self.request.method not in SAFE_METHODS:
             ret.append(permissions.TokenNoDomainPolicy)
             ret.append(permissions.TokenNoDomainPolicy)
         return ret
         return ret

+ 2 - 2
api/desecapi/views/dyndns.py

@@ -17,7 +17,7 @@ from desecapi.authentication import (
 from desecapi.exceptions import ConcurrencyException
 from desecapi.exceptions import ConcurrencyException
 from desecapi.models import Domain
 from desecapi.models import Domain
 from desecapi.pdns_change_tracker import PDNSChangeTracker
 from desecapi.pdns_change_tracker import PDNSChangeTracker
-from desecapi.permissions import TokenHasDomainDynDNSPermission
+from desecapi.permissions import IsDomainOwner
 from desecapi.renderers import PlainTextRenderer
 from desecapi.renderers import PlainTextRenderer
 from desecapi.serializers import RRsetSerializer
 from desecapi.serializers import RRsetSerializer
 
 
@@ -28,7 +28,7 @@ class DynDNS12UpdateView(generics.GenericAPIView):
         BasicTokenAuthentication,
         BasicTokenAuthentication,
         URLParamAuthentication,
         URLParamAuthentication,
     )
     )
-    permission_classes = (TokenHasDomainDynDNSPermission,)
+    permission_classes = (IsDomainOwner,)
     renderer_classes = [PlainTextRenderer]
     renderer_classes = [PlainTextRenderer]
     serializer_class = RRsetSerializer
     serializer_class = RRsetSerializer
     throttle_scope = "dyndns"
     throttle_scope = "dyndns"

+ 9 - 3
api/desecapi/views/records.py

@@ -40,7 +40,6 @@ class RRsetView(DomainViewMixin):
         IsAuthenticated,
         IsAuthenticated,
         permissions.IsAPIToken | permissions.MFARequiredIfEnabled,
         permissions.IsAPIToken | permissions.MFARequiredIfEnabled,
         permissions.IsDomainOwner,
         permissions.IsDomainOwner,
-        permissions.TokenHasDomainRRsetsPermission,
     )
     )
 
 
     @property
     @property
@@ -74,6 +73,13 @@ class RRsetView(DomainViewMixin):
 class RRsetDetail(
 class RRsetDetail(
     RRsetView, IdempotentDestroyMixin, generics.RetrieveUpdateDestroyAPIView
     RRsetView, IdempotentDestroyMixin, generics.RetrieveUpdateDestroyAPIView
 ):
 ):
+    @property
+    def permission_classes(self):
+        ret = list(super().permission_classes)
+        if self.request.method not in SAFE_METHODS:
+            ret.append(permissions.TokenHasRRsetPermission)
+        return ret
+
     def get_object(self):
     def get_object(self):
         queryset = self.filter_queryset(self.get_queryset())
         queryset = self.filter_queryset(self.get_queryset())
 
 
@@ -124,8 +130,8 @@ class RRsetList(
     def get_object(self):
     def get_object(self):
         # For this view, the object we're operating on is the queryset that one can also GET. Serializing a queryset
         # For this view, the object we're operating on is the queryset that one can also GET. Serializing a queryset
         # is fine as per https://www.django-rest-framework.org/api-guide/serializers/#serializing-multiple-objects.
         # is fine as per https://www.django-rest-framework.org/api-guide/serializers/#serializing-multiple-objects.
-        # We skip checking object permissions here to avoid evaluating the queryset. The user can access all his RRsets
-        # anyways.
+        # To avoid evaluating the queryset, object permissions are checked in the serializer for write operations only.
+        # The user can read all their RRsets anyway.
         return self.filter_queryset(self.get_queryset())
         return self.filter_queryset(self.get_queryset())
 
 
     def get_serializer(self, *args, **kwargs):
     def get_serializer(self, *args, **kwargs):

+ 1 - 1
api/desecapi/views/users.py

@@ -56,7 +56,7 @@ class AccountView(generics.RetrieveUpdateAPIView):
     permission_classes = (
     permission_classes = (
         IsAuthenticated,
         IsAuthenticated,
         permissions.IsAPIToken | permissions.MFARequiredIfEnabled,
         permissions.IsAPIToken | permissions.MFARequiredIfEnabled,
-        permissions.TokenNoDomainPolicy,
+        permissions.HasManageTokensPermission,
     )
     )
     serializer_class = serializers.UserSerializer
     serializer_class = serializers.UserSerializer
     throttle_scope = "account_management_passive"
     throttle_scope = "account_management_passive"

+ 92 - 51
docs/auth/tokens.rst

@@ -269,16 +269,8 @@ If you do not have the token ID, but you do have the token secret, you
 can use the :ref:`log-out` endpoint to delete it.
 can use the :ref:`log-out` endpoint to delete it.
 
 
 
 
-Token Scoping: Domain Policies
-``````````````````````````````
-
-.. warning::
-    The Token Scoping interface **will change** in late 2023. The below
-    description is **deprecated**.
-
-    The changes are necessary in order to enable higher scoping granularity
-    (on the RRset level). For development details, see
-    https://github.com/desec-io/desec-stack/pull/840.
+Token Scoping: Policies
+```````````````````````
 
 
 Tokens by default can be used to authorize arbitrary actions within the user's
 Tokens by default can be used to authorize arbitrary actions within the user's
 account, including DNS operations on any domain and some administrative tasks.
 account, including DNS operations on any domain and some administrative tasks.
@@ -290,19 +282,52 @@ Tokens can be *restricted* using Token Policies, which narrow down the scope
 of influence for a given API token.
 of influence for a given API token.
 Using policies, the token's power can be limited in two ways:
 Using policies, the token's power can be limited in two ways:
 
 
-1. the types of DNS operations that can be performed, such as :ref:`dynDNS
-   updates <update-api>` or :ref:`general RRset management <manage-rrsets>`.
-
-2. the set of domains on which these actions can be performed.
-
-Policies can be configured on a per-domain basis.
-Domains for which no explicit policy is configured are subject to the token's
-default policy.
-It is required to create such a default policy before any domain-specific
-policies can be created on a given token.
+1. the type of access control (*allow-by-default* or *deny-by-default)* for DNS
+   write operations, such as :ref:`dynDNS updates <update-api>` or
+   :ref:`general RRset management <manage-rrsets>`;
+
+2. explicit access control for specific RRsets through the policy's ``domain``,
+   ``subname``, and ``type`` fields.
+
+All tokens can, regardless of their policy configuration, read any RRset (for
+all domains in the account).  This is because essentially the same information
+is also available through the DNS.  Note that the API in addition exposes some
+metadata, such as the RRset's ``created`` or ``touched`` timestamps.
+
+Write permissions can be configured on a per-RRset basis. When attempting to
+manipulate an RRset, the applicable policy is identified by matching the RRset
+against existing policies in the following order:
+
++----------+------------+-------------+----------+
+| Priority | ``domain`` | ``subname`` | ``type`` |
++==========+============+=============+==========+
+| 1        | match      | match       | match    |
++----------+------------+-------------+----------+
+| 2        | match      | match       | *null*   |
++----------+------------+-------------+----------+
+| 3        | match      | *null*      | match    |
++----------+------------+-------------+----------+
+| 4        | match      | *null*      | *null*   |
++----------+------------+-------------+----------+
+| 5        | *null*     | match       | match    |
++----------+------------+-------------+----------+
+| 6        | *null*     | match       | *null*   |
++----------+------------+-------------+----------+
+| 7        | *null*     | *null*      | match    |
++----------+------------+-------------+----------+
+| 8        | *null*     | *null*      | *null*   |
++----------+------------+-------------+----------+
+
+Taking the (``domain``, ``subname``, ``type``) tuple as a path, this can be
+considered a longest-prefix match algorithm. Wildcards are not expanded and
+match only RRsets with an identical wildcard ``subname``.
+
+RRsets for which no more specific policy is configured are eventually caught by
+the token's default policy.  It is therefore required to create such a default
+policy before any more specific policies can be created on a given token.
 
 
 Tokens with at least one policy are considered *restricted*, with their scope
 Tokens with at least one policy are considered *restricted*, with their scope
-explicitly limited to DNS record management.
+limited to DNS record management.
 They can neither :ref:`retrieve-account-information` nor perform
 They can neither :ref:`retrieve-account-information` nor perform
 :ref:`domain-management` (such as domain creation or deletion).
 :ref:`domain-management` (such as domain creation or deletion).
 
 
@@ -313,44 +338,60 @@ In particular, a restricted token that at the same time has the
 restrictions (see `Token Field Reference`_).
 restrictions (see `Token Field Reference`_).
 
 
 
 
-Token Domain Policy Field Reference
------------------------------------
+Token Policy Field Reference
+----------------------------
 
 
-A JSON object representing a token domain policy has the following structure::
+A JSON object representing a token policy has the following structure::
 
 
     {
     {
+        "id": "7aed3f71-bc81-4f7e-90ae-8f0df0d1c211",
         "domain": "example.com",
         "domain": "example.com",
-        "perm_dyndns": false,
+        "subname": null,
+        "type": null,
         "perm_write": true
         "perm_write": true
     }
     }
 
 
 Field details:
 Field details:
 
 
+``id``
+    :Access mode: read-only
+    :Type: UUID
+
+    Token policy ID, used for identification only (e.g. when modifying a
+    policy). (Not to be confused with the token's ID.)
+
 ``domain``
 ``domain``
     :Access mode: read, write
     :Access mode: read, write
     :Type: string or ``null``
     :Type: string or ``null``
 
 
     Domain name to which the policy applies.  ``null`` for the default policy.
     Domain name to which the policy applies.  ``null`` for the default policy.
 
 
-``perm_dyndns``
+``subname``
     :Access mode: read, write
     :Access mode: read, write
-    :Type: boolean
+    :Type: string or ``null``
+
+    Subname to which the policy applies.  ``null`` for the default policy.
 
 
-    Indicates whether :ref:`dynDNS updates <update-api>` are allowed.
-    Defaults to ``false``.
+``type``
+    :Access mode: read, write
+    :Type: string or ``null``
+
+    Record type to which the policy applies.  ``null`` for the default policy.
 
 
 ``perm_write``
 ``perm_write``
     :Access mode: read, write
     :Access mode: read, write
     :Type: boolean
     :Type: boolean
 
 
-    Indicates whether :ref:`general RRset management <manage-rrsets>` is
-    allowed.  Defaults to ``false``.
+    Indicates write permission for the RRset specified by (``domain``,
+    ``subname``, ``type``) when using the :ref:`general RRset management
+    <manage-rrsets>` or :ref:`dynDNS <update-api>` interface.  Defaults to
+    ``false``.
 
 
 
 
-Token Domain Policy Management
-------------------------------
-Token Domain Policies are managed using the ``policies/rrsets/`` endpoint
-under the token's URL.
+Token Policy Management
+-----------------------
+Token Policies are managed using the ``policies/rrsets/`` endpoint under the
+token's URL.
 Usage of this endpoint requires that the request's authorization token has the
 Usage of this endpoint requires that the request's authorization token has the
 ``perm_manage_tokens`` flag.
 ``perm_manage_tokens`` flag.
 
 
@@ -362,42 +403,42 @@ request as follows::
     curl -X GET https://desec.io/api/v1/auth/tokens/{id}/policies/rrsets/ \
     curl -X GET https://desec.io/api/v1/auth/tokens/{id}/policies/rrsets/ \
         --header "Authorization: Token mu4W4MHuSc0Hy-GD1h_dnKuZBond"
         --header "Authorization: Token mu4W4MHuSc0Hy-GD1h_dnKuZBond"
 
 
-The server will respond with a list of token domain policy objects.
+The server will respond with a list of token policy objects.
 
 
 To create the default policy, send a request like::
 To create the default policy, send a request like::
 
 
     curl -X POST https://desec.io/api/v1/auth/tokens/{id}/policies/rrsets/ \
     curl -X POST https://desec.io/api/v1/auth/tokens/{id}/policies/rrsets/ \
         --header "Authorization: Token mu4W4MHuSc0Hy-GD1h_dnKuZBond" \
         --header "Authorization: Token mu4W4MHuSc0Hy-GD1h_dnKuZBond" \
         --header "Content-Type: application/json" --data @- <<< \
         --header "Content-Type: application/json" --data @- <<< \
-        '{"domain": null}'
+        '{"domain": null, "subname": null, "type": null}'
+
+This will create a default policy.  If the ``perm_write`` permission flag is
+not given, it is assumed to be ``false``.
 
 
-This will create a default policy.  Permission flags that are not given are
-assumed to be ``false``.  To enable permissions, they have to be set to
-``true`` explicitly.  As an example, let's create a policy that only allows
-dynDNS updates for a specific domain::
+As an example, let's create a policy that only allows manipulating all A
+records for a specific domain::
 
 
     curl -X POST https://desec.io/api/v1/auth/tokens/{id}/policies/rrsets/ \
     curl -X POST https://desec.io/api/v1/auth/tokens/{id}/policies/rrsets/ \
         --header "Authorization: Token mu4W4MHuSc0Hy-GD1h_dnKuZBond" \
         --header "Authorization: Token mu4W4MHuSc0Hy-GD1h_dnKuZBond" \
         --header "Content-Type: application/json" --data @- <<< \
         --header "Content-Type: application/json" --data @- <<< \
-        '{"domain": "example.dedyn.io", "perm_dyndns": true}'
+        '{"domain": "example.dedyn.io", "subname": null, "type": "A", "perm_write": true}'
+
+**Tip:** To authorize dual-stack dynDNS updates, create two policies (for
+access to the A and AAAA RRsets, respectively).
 
 
 You can retrieve (``GET``), update (``PATCH``, ``PUT``), and remove
 You can retrieve (``GET``), update (``PATCH``, ``PUT``), and remove
-(``DELETE``) policies by appending their ``domain`` to the endpoint::
+(``DELETE``) policies by appending their ``id`` to the endpoint::
 
 
-    curl -X DELETE https://desec.io/api/v1/auth/tokens/{id}/policies/rrsets/{domain}/ \
+    curl -X DELETE https://desec.io/api/v1/auth/tokens/{token.id}/policies/rrsets/{policy.id}/ \
         --header "Authorization: Token mu4W4MHuSc0Hy-GD1h_dnKuZBond"
         --header "Authorization: Token mu4W4MHuSc0Hy-GD1h_dnKuZBond"
 
 
-The default policy can be accessed using the special domain name ``default``
-(``/api/v1/auth/tokens/{id}/policies/rrsets/default/``).
-
 When modifying or deleting policies, the API enforces the default policy's
 When modifying or deleting policies, the API enforces the default policy's
 primacy:
 primacy:
-You cannot create domain-specific policies without first creating a default
-policy, and you cannot remove a default policy when other policies are still
-in place.
+You cannot create specific policies without first creating a default policy,
+and you cannot remove a default policy when other policies are still in place.
 
 
 During deletion of tokens, users, or domains, policies are cleaned up
 During deletion of tokens, users, or domains, policies are cleaned up
-automatically.  (It is not necessary to first remove policies manually.)
+automatically.
 
 
 Security Considerations
 Security Considerations
 ```````````````````````
 ```````````````````````

+ 6 - 6
docs/endpoint-reference.rst

@@ -35,18 +35,18 @@ for :ref:`managing users <manage-account>` and :ref:`tokens <manage-tokens>`.
 |                                                      +------------+---------------------------------------------+
 |                                                      +------------+---------------------------------------------+
 |                                                      | ``DELETE`` | Delete token                                |
 |                                                      | ``DELETE`` | Delete token                                |
 +------------------------------------------------------+------------+---------------------------------------------+
 +------------------------------------------------------+------------+---------------------------------------------+
-| ...\ ``/auth/tokens/{id}/policies/rrsets/``          | ``GET``    | Retrieve all domain policies for the given  |
+| ...\ ``/auth/tokens/{id}/policies/rrsets/``          | ``GET``    | Retrieve all RRset policies for the given   |
 |                                                      |            | token                                       |
 |                                                      |            | token                                       |
 |                                                      +------------+---------------------------------------------+
 |                                                      +------------+---------------------------------------------+
-|                                                      | ``POST``   | Create a domain policy for the given token  |
+|                                                      | ``POST``   | Create an RRset policy for the given token  |
 +------------------------------------------------------+------------+---------------------------------------------+
 +------------------------------------------------------+------------+---------------------------------------------+
-| ...\ ``/auth/tokens/{id}/policies/rrsets/{domain}/`` | ``GET``    | Retrieve a specific token domain policy     |
+| ...\ ``/auth/tokens/{id}/policies/rrsets/{policy}/`` | ``GET``    | Retrieve a specific token RRset policy      |
 |                                                      +------------+---------------------------------------------+
 |                                                      +------------+---------------------------------------------+
-|                                                      | ``PATCH``  | Modify a token domain policy                |
+|                                                      | ``PATCH``  | Modify a token policy                       |
 |                                                      +------------+---------------------------------------------+
 |                                                      +------------+---------------------------------------------+
-|                                                      | ``PUT``    | Replace a token domain policy               |
+|                                                      | ``PUT``    | Replace a token policy                      |
 |                                                      +------------+---------------------------------------------+
 |                                                      +------------+---------------------------------------------+
-|                                                      | ``DELETE`` | Delete a token domain policy                |
+|                                                      | ``DELETE`` | Delete a token policy                       |
 +------------------------------------------------------+------------+---------------------------------------------+
 +------------------------------------------------------+------------+---------------------------------------------+
 | ...\ ``/auth/totp/``                                 |            | 2FA-related, interface subject to change    |
 | ...\ ``/auth/totp/``                                 |            | 2FA-related, interface subject to change    |
 +------------------------------------------------------+------------+---------------------------------------------+
 +------------------------------------------------------+------------+---------------------------------------------+