From 117c819bece2be5b25ce976a2a449fd089a2287e Mon Sep 17 00:00:00 2001 From: Peter Thomassen Date: Mon, 4 Nov 2024 13:35:59 +0000 Subject: [PATCH] feat(api): add perm_create_domain / perm_delete_domain Related: #885 --- ..._create_domain_token_perm_delete_domain.py | 36 +++++++++ api/desecapi/models/tokens.py | 2 + api/desecapi/permissions.py | 42 ++++++++--- api/desecapi/serializers/tokens.py | 2 + api/desecapi/tests/base.py | 4 +- api/desecapi/tests/test_domains.py | 74 +++++++++++++++++++ .../tests/test_token_domain_policy.py | 15 +--- api/desecapi/tests/test_tokens.py | 16 +++- api/desecapi/views/domains.py | 5 +- api/desecapi/views/users.py | 2 + docs/auth/tokens.rst | 43 +++++++++-- docs/dns/domains.rst | 8 +- 12 files changed, 209 insertions(+), 40 deletions(-) create mode 100644 api/desecapi/migrations/0039_token_perm_create_domain_token_perm_delete_domain.py diff --git a/api/desecapi/migrations/0039_token_perm_create_domain_token_perm_delete_domain.py b/api/desecapi/migrations/0039_token_perm_create_domain_token_perm_delete_domain.py new file mode 100644 index 000000000..264b0eb75 --- /dev/null +++ b/api/desecapi/migrations/0039_token_perm_create_domain_token_perm_delete_domain.py @@ -0,0 +1,36 @@ +# Generated by Django 5.1.2 on 2024-11-01 14:29 + +import django.db.migrations.operations.special +from django.db import migrations, models + + +def forwards_func(apps, schema_editor): + Token = apps.get_model("desecapi", "Token") + db_alias = schema_editor.connection.alias + Token.objects.using(db_alias).filter(domain_policies__isnull=False).update( + perm_create_domain=True, perm_delete_domain=True + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ("desecapi", "0038_user_throttle_daily_rate"), + ] + + operations = [ + migrations.AddField( + model_name="token", + name="perm_create_domain", + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name="token", + name="perm_delete_domain", + field=models.BooleanField(default=False), + ), + migrations.RunPython( + code=forwards_func, + reverse_code=django.db.migrations.operations.special.RunPython.noop, + ), + ] diff --git a/api/desecapi/models/tokens.py b/api/desecapi/models/tokens.py index 48c42b2c3..2d27cc623 100644 --- a/api/desecapi/models/tokens.py +++ b/api/desecapi/models/tokens.py @@ -41,6 +41,8 @@ def _allowed_subnets_default(): name = models.CharField("Name", blank=True, max_length=64) last_used = models.DateTimeField(null=True, blank=True) mfa = models.BooleanField(default=None, null=True) + perm_create_domain = models.BooleanField(default=False) + perm_delete_domain = models.BooleanField(default=False) perm_manage_tokens = models.BooleanField(default=False) allowed_subnets = ArrayField( CidrAddressField(), default=_allowed_subnets_default.__func__ diff --git a/api/desecapi/permissions.py b/api/desecapi/permissions.py index 69d106bf2..53c1131e5 100644 --- a/api/desecapi/permissions.py +++ b/api/desecapi/permissions.py @@ -76,15 +76,6 @@ def has_permission(self, request, view): return request.user == view.domain.owner -class TokenNoDomainPolicy(permissions.BasePermission): - """ - Permission to check whether a token is unrestricted by any policy. - """ - - def has_permission(self, request, view): - return not request.auth.tokendomainpolicy_set.exists() - - class TokenHasRRsetPermission(permissions.BasePermission): """ Permission to check whether a token authorizes writing the view's RRset. @@ -121,6 +112,39 @@ def has_permission(self, request, view): return ip in IPv4Network("10.8.0.0/24") +class HasCreateDomainPermission(permissions.BasePermission): + """ + Permission to check whether a token has "create domain" permission. + """ + + def has_permission(self, request, view): + return request.auth.perm_create_domain + + +class HasDeleteDomainPermission(permissions.BasePermission): + """ + Permission to check whether a token has "delete domian" permission. + """ + + def has_permission(self, request, view): + return request.auth.perm_delete_domain + + def has_object_permission(self, request, view, obj): + policy_set = request.auth.tokendomainpolicy_set + + return ( + # Ensure token is not explicitly prohibited from writing some RRsets in this domain + not policy_set.filter(domain=obj, perm_write=False).exists() + and + # Ensure applicable domain-independent policies are not constraining + ( + policy_set.filter(domain=obj, subname=None, type=None).exists() + # Conservative (might be shadowed by domain=obj for same subname/type) + or not policy_set.filter(domain=None, perm_write=False).exists() + ) + ) + + class HasManageTokensPermission(permissions.BasePermission): """ Permission to check whether a token has "manage tokens" permission. diff --git a/api/desecapi/serializers/tokens.py b/api/desecapi/serializers/tokens.py index 35e47cb29..6fe403498 100644 --- a/api/desecapi/serializers/tokens.py +++ b/api/desecapi/serializers/tokens.py @@ -19,6 +19,8 @@ class Meta: "max_age", "max_unused_period", "name", + "perm_create_domain", + "perm_delete_domain", "perm_manage_tokens", "allowed_subnets", "is_valid", diff --git a/api/desecapi/tests/base.py b/api/desecapi/tests/base.py index 85c7c58ac..de15ab1c8 100644 --- a/api/desecapi/tests/base.py +++ b/api/desecapi/tests/base.py @@ -1352,7 +1352,9 @@ def setUpTestDataWithPdns(cls): cls.create_rr_set(cls.my_domain, ["127.0.0.1", "3.2.2.3"], type="A", ttl=123) cls.create_rr_set(cls.other_domain, ["40.1.1.1"], type="A", ttl=456) - cls.token = cls.create_token(user=cls.owner) + cls.token = cls.create_token( + user=cls.owner, perm_create_domain=True, perm_delete_domain=True + ) def setUp(self): super().setUp() diff --git a/api/desecapi/tests/test_domains.py b/api/desecapi/tests/test_domains.py index fcea951c4..6316de592 100644 --- a/api/desecapi/tests/test_domains.py +++ b/api/desecapi/tests/test_domains.py @@ -1,3 +1,5 @@ +from contextlib import nullcontext + from django.conf import settings from django.core import mail from django.core.exceptions import ValidationError @@ -279,6 +281,61 @@ def test_delete_my_domain(self): response = self.client.get(url) self.assertStatus(response, status.HTTP_404_NOT_FOUND) + def test_delete_my_domain_policy_constraints(self): + policy_sets = [ + ([dict(domain=None, subname=None, type=None, perm_write=True)], True), + ([dict(domain=None, subname=None, type=None, perm_write=False)], False), + ( + [ + dict(domain=None, subname=None, type=None, perm_write=False), + dict( + domain=self.my_domain, subname=None, type=None, perm_write=True + ), + ], + True, + ), + ( + [ + dict(domain=None, subname=None, type=None, perm_write=False), + dict( + domain=self.my_domain, subname=None, type=None, perm_write=True + ), + dict( + domain=self.my_domain, + subname="_acme-challenge", + type=None, + perm_write=False, + ), + ], + False, + ), + ] + for policies, permitted in policy_sets: + for policy in policies: + self.token.tokendomainpolicy_set.create(**policy) + + url = self.reverse("v1:domain-detail", name=self.my_domain.name) + with ( + self.assertRequests( + self.requests_desec_domain_deletion(domain=self.my_domain) + ) + if permitted + else nullcontext() + ): + response = self.client.delete(url) + self.assertStatus( + response, + ( + status.HTTP_204_NO_CONTENT + if permitted + else status.HTTP_403_FORBIDDEN + ), + ) + + # Clean-up + self.token.tokendomainpolicy_set.all().delete() + self.my_domain.save() + def test_delete_other_domain(self): url = self.reverse("v1:domain-detail", name=self.other_domain.name) response = self.client.delete(url) @@ -379,6 +436,15 @@ def test_create_domains(self): self.assertFalse(domain.is_locally_registrable) self.assertEqual(domain.renewal_state, Domain.RenewalState.IMMORTAL) + def test_create_domain_no_permission(self): + self.token.perm_create_domain = False + self.token.save() + + response = self.client.post( + self.reverse("v1:domain-list"), {"name": "foobar.example"} + ) + self.assertStatus(response, status.HTTP_403_FORBIDDEN) + def test_create_domain_zonefile_import(self): zonefile = """$ORIGIN . $TTL 43200 ; 12 hours @@ -829,6 +895,14 @@ def test_delete_my_domain(self): response = self.client.get(url) self.assertStatus(response, status.HTTP_404_NOT_FOUND) + def test_delete_my_domain_no_permission(self): + self.token.perm_delete_domain = False + self.token.save() + + url = self.reverse("v1:domain-detail", name=self.my_domain.name) + response = self.client.delete(url) + self.assertStatus(response, status.HTTP_403_FORBIDDEN) + def test_delete_other_domains(self): url = self.reverse("v1:domain-detail", name=self.other_domain.name) with self.assertRequests(): diff --git a/api/desecapi/tests/test_token_domain_policy.py b/api/desecapi/tests/test_token_domain_policy.py index 781d1f9cc..21f0031b8 100644 --- a/api/desecapi/tests/test_token_domain_policy.py +++ b/api/desecapi/tests/test_token_domain_policy.py @@ -458,19 +458,6 @@ def _reset_policies(token): setattr(policy, perm, value) policy.save() - # Can't create domain - data = {"name": self.random_domain_name()} - response = self.client.post( - self.reverse("v1:domain-list"), data, **kwargs - ) - 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 response = self.client.get(self.reverse("v1:account"), **kwargs) self.assertStatus(response, status.HTTP_403_FORBIDDEN) @@ -597,7 +584,7 @@ def test_domain_owner_equals_token_user(self): with transaction.atomic(): # https://stackoverflow.com/a/23326971/6867099 self.token.save() - def test_domain_deletion(self): + def test_domain_deletion_policy_cleanup(self): domains = [None] + self.my_domains[:2] for domain in domains: models.TokenDomainPolicy( diff --git a/api/desecapi/tests/test_tokens.py b/api/desecapi/tests/test_tokens.py index 1854dd535..98239bd3a 100644 --- a/api/desecapi/tests/test_tokens.py +++ b/api/desecapi/tests/test_tokens.py @@ -59,6 +59,8 @@ def test_retrieve_my_token(self): "max_age", "max_unused_period", "name", + "perm_create_domain", + "perm_delete_domain", "perm_manage_tokens", "allowed_subnets", "is_valid", @@ -111,6 +113,8 @@ def test_create_token(self): datas = [ {}, {"name": "", "perm_manage_tokens": True}, + {"name": "", "perm_delete_domain": True}, + {"name": "", "perm_create_domain": True, "perm_delete_domain": True}, {"name": "foobar"}, {"allowed_subnets": ["1.2.3.32/28", "bade::affe/128"]}, ] @@ -126,6 +130,8 @@ def test_create_token(self): "max_age", "max_unused_period", "name", + "perm_create_domain", + "perm_delete_domain", "perm_manage_tokens", "allowed_subnets", "is_valid", @@ -137,10 +143,12 @@ def test_create_token(self): response.data["allowed_subnets"], data.get("allowed_subnets", ["0.0.0.0/0", "::/0"]), ) - self.assertEqual( - response.data["perm_manage_tokens"], - data.get("perm_manage_tokens", False), - ) + for perm in [ + "perm_create_domain", + "perm_delete_domain", + "perm_manage_tokens", + ]: + self.assertEqual(response.data[perm], data.get(perm, False)) self.assertIsNone(response.data["last_used"]) self.assertIsNone(Token.objects.get(pk=response.data["id"]).mfa) diff --git a/api/desecapi/views/domains.py b/api/desecapi/views/domains.py index 4e572e8c7..2cd01493b 100644 --- a/api/desecapi/views/domains.py +++ b/api/desecapi/views/domains.py @@ -40,9 +40,10 @@ def permission_classes(self): permissions.IsOwner, ] if self.action == "create": + ret.append(permissions.HasCreateDomainPermission) ret.append(permissions.WithinDomainLimit) - if self.request.method not in SAFE_METHODS: - ret.append(permissions.TokenNoDomainPolicy) + if self.action == "destroy": + ret.append(permissions.HasDeleteDomainPermission) return ret @property diff --git a/api/desecapi/views/users.py b/api/desecapi/views/users.py index 482d2edeb..ce75e00fd 100644 --- a/api/desecapi/views/users.py +++ b/api/desecapi/views/users.py @@ -101,6 +101,8 @@ def post(self, request, *args, **kwargs): user = self.request.user token = Token.objects.create( user=user, + perm_create_domain=True, + perm_delete_domain=True, perm_manage_tokens=True, max_age=timedelta(days=7), max_unused_period=timedelta(hours=1), diff --git a/docs/auth/tokens.rst b/docs/auth/tokens.rst index 5cc6589d0..7e84f419c 100644 --- a/docs/auth/tokens.rst +++ b/docs/auth/tokens.rst @@ -28,6 +28,8 @@ A JSON object representing a token has the following structure:: "id": "3a6b94b5-d20e-40bd-a7cc-521f5c79fab3", "last_used": null, "name": "my new token", + "perm_create_domain": false, + "perm_delete_domain": false, "perm_manage_tokens": false, "allowed_subnets": [ "0.0.0.0/0", @@ -116,6 +118,19 @@ Field details: Certain API operations will automatically populate the ``name`` field with suitable values such as "login" or "dyndns". +``perm_create_domain`` + :Access mode: read, write + :Type: boolean + + Permission to create a new domain. + +``perm_delete_domain`` + :Access mode: read, write + :Type: boolean + + Permission to delete a domain. When using :ref:`token scoping policies`, + deleting a domain also requires write permission on all its RRsets. + ``perm_manage_tokens`` :Access mode: read, write :Type: boolean @@ -151,6 +166,8 @@ Note that the name and other fields are optional. The server will reply with "id": "3a6b94b5-d20e-40bd-a7cc-521f5c79fab3", "last_used": null, "name": "my new token", + "perm_create_domain": false, + "perm_delete_domain": false, "perm_manage_tokens": false, "allowed_subnets": [ "0.0.0.0/0", @@ -164,10 +181,10 @@ In particular, the ``perm_manage_tokens`` flag will not be set, so that the new token cannot be used to retrieve, modify, or delete any tokens (including itself). -With the default set of permissions, tokens qualify for carrying out all API -operations related to DNS management (i.e. managing both domains and DNS -records). Note that it is always possible to use the :ref:`log-out` endpoint -to delete a token. +Similarly, tokens by default cannot create or delete any domains (although they +can manage DNS records of existing domains, unless restricted through +:ref:`token scoping policies`). Note that it is always possible to use the +:ref:`log-out` endpoint to delete a token. If you require tokens with extra permissions, you can provide the desired configuration during creation: @@ -177,6 +194,12 @@ configuration during creation: provided, access is not restricted based on the IP address. Both IPv4 and IPv6 are supported. +- ``perm_create_domain``: If set to ``true``, the token can be used to + create domains. + +- ``perm_delete_domain``: If set to ``true``, the token can be used to + delete domains. + - ``perm_manage_tokens``: If set to ``true``, the token can be used to authorize token management operations (as described in this chapter). @@ -269,6 +292,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. +.. _`token scoping policies`: + Token Scoping: Policies ``````````````````````` @@ -326,10 +351,12 @@ 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 -limited to DNS record management. -They can neither :ref:`retrieve-account-information` nor perform -:ref:`domain-management` (such as domain creation or deletion). +Tokens with at least one policy are considered *restricted*, with their DNS +record management capabilities limited as per policy configuration. +Whether :ref:`domain-management` is allowed depends on the +``perm_create_domain`` and ``perm_delete_domain`` permissions. +Restricted tokens cannot be used to perform other actions (e.g., +:ref:`retrieve-account-information`). **Note:** Token policies are *independent* of high-level token permissions that can be assigned when `Creating a Token`_. diff --git a/docs/dns/domains.rst b/docs/dns/domains.rst index c20d0fa73..6ac056417 100644 --- a/docs/dns/domains.rst +++ b/docs/dns/domains.rst @@ -149,6 +149,7 @@ endpoint, like this:: --header "Content-Type: application/json" --data @- <<< \ '{"name": "example.com"}' +This operation requires the ``perm_create_domain`` permission on the token. Only the ``name`` field is mandatory. Upon success, the response status code will be ``201 Created``, with the @@ -283,5 +284,8 @@ Deleting a Domain ~~~~~~~~~~~~~~~~~ To delete a domain, send a ``DELETE`` request to the endpoint representing the -domain. Upon success or if the domain did not exist in your account, the -response status code is ``204 No Content``. +domain. This operation requires the ``perm_delete_domain`` permission on the +token, and no conflicting token scoping policies. + +Upon success or if the domain did not exist in your account, the response +status code is ``204 No Content``.