From 2b3962d2d881f2ff34fc743b794c257c11c2cabb Mon Sep 17 00:00:00 2001 From: Peter Thomassen Date: Mon, 4 Nov 2024 13:35:59 +0000 Subject: [PATCH 01/10] feat(api): add perm_create_domain / perm_delete_domain Related: #885 --- ..._create_domain_token_perm_delete_domain.py | 36 +++++++++ api/desecapi/models/tokens.py | 29 ++++++++ api/desecapi/permissions.py | 30 +++++--- 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 | 13 +++- api/desecapi/views/users.py | 2 + docs/auth/tokens.rst | 43 +++++++++-- docs/dns/domains.rst | 8 +- 12 files changed, 231 insertions(+), 41 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..e45078c52 --- /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(tokendomainpolicy__isnull=True).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..49cb8e160 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__ @@ -106,6 +108,33 @@ def get_policy(self, rrset=None): .first() ) + def can_safely_delete_domain(self, domain): + forbidden = ( + # Check if token is explicitly prohibited from writing some RRsets in this domain + # (priority order 1-4, see /docs/auth/tokens.rst#token-scoping-policies) + self.tokendomainpolicy_set.filter(domain=domain) + .filter(perm_write=False) + .exists() + or + # Check that the token has no permissive default policy for this domain + # (priority order 4) and apply fall-through to domain-independent policies (5-8) + ( + not self.tokendomainpolicy_set.filter( + domain=domain, subname=None, type=None + ) + .filter(perm_write=True) + .exists() + # Fall-through. Uses a conservative approximation and does not account for + # permissive policies of priority order 1, 2, 3 shadowing restrictive policies + # of priority order 5, 6, 7, respectively. For details, see + # https://github.com/desec-io/desec-stack/pull/990#discussion_r1864977009. + and self.tokendomainpolicy_set.filter(domain=None) + .filter(perm_write=False) + .exists() + ) + ) + return not forbidden + class TokenDomainPolicy(ExportModelOperationsMixin("TokenDomainPolicy"), models.Model): id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) diff --git a/api/desecapi/permissions.py b/api/desecapi/permissions.py index 69d106bf2..28f908688 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,27 @@ 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): + return request.auth.can_safely_delete_domain(obj) + + 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..829e729ad 100644 --- a/api/desecapi/views/domains.py +++ b/api/desecapi/views/domains.py @@ -39,10 +39,17 @@ def permission_classes(self): permissions.IsAPIToken | permissions.MFARequiredIfEnabled, permissions.IsOwner, ] - if self.action == "create": - ret.append(permissions.WithinDomainLimit) if self.request.method not in SAFE_METHODS: - ret.append(permissions.TokenNoDomainPolicy) + match self.action: + case None: + pass # occurs when HTTP method is not allowed; leads to status 405 + case "create": + ret.append(permissions.HasCreateDomainPermission) + ret.append(permissions.WithinDomainLimit) + case "destroy": + ret.append(permissions.HasDeleteDomainPermission) + case _: + raise ValueError(f"Invalid action: {self.action}") 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``. From 6a310b5ea6feaa5b26de8e3206fe74111baf011c Mon Sep 17 00:00:00 2001 From: Peter Thomassen Date: Tue, 19 Nov 2024 22:29:56 +0100 Subject: [PATCH 02/10] feat(webapp): expose perm_create_domain / perm_delete_domain --- www/webapp/src/views/CrudListToken.vue | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/www/webapp/src/views/CrudListToken.vue b/www/webapp/src/views/CrudListToken.vue index d4d5bd8ed..897007380 100644 --- a/www/webapp/src/views/CrudListToken.vue +++ b/www/webapp/src/views/CrudListToken.vue @@ -45,7 +45,6 @@ export default { perm_manage_tokens: { name: 'item.perm_manage_tokens', text: 'Can manage tokens', - textCreate: 'Can manage tokens?', align: 'left', sortable: true, value: 'perm_manage_tokens', @@ -55,6 +54,28 @@ export default { searchable: false, advanced: true, }, + perm_create_domain: { + name: 'item.perm_create_domain', + text: 'Can create domains', + align: 'left', + sortable: true, + value: 'perm_create_domain', + readonly: false, + writeOnCreate: true, + datatype: GenericSwitchbox.name, + searchable: false, + }, + perm_delete_domain: { + name: 'item.perm_delete_domain', + text: 'Can delete domains', + align: 'left', + sortable: true, + value: 'perm_delete_domain', + readonly: false, + writeOnCreate: true, + datatype: GenericSwitchbox.name, + searchable: false, + }, allowed_subnets: { name: 'item.allowed_subnets', text: 'Client subnets', From a103c9245a29ee2b884105cf88a520e6c9889474 Mon Sep 17 00:00:00 2001 From: Peter Thomassen Date: Wed, 20 Nov 2024 14:34:21 +0100 Subject: [PATCH 03/10] feat(): use sequential output when building dev images --- dev | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev b/dev index 3772762b9..298891afb 100755 --- a/dev +++ b/dev @@ -1,4 +1,4 @@ #!/usr/bin/env bash -docker compose -f docker-compose.yml -f docker-compose.dev.yml --env-file env-dev build "$@" \ +docker compose -f docker-compose.yml -f docker-compose.dev.yml --env-file env-dev build --progress plain "$@" \ && docker compose -f docker-compose.yml -f docker-compose.dev.yml --env-file env-dev up "$@" From f825e4492c05c00e178e7680f3a059ce60110038 Mon Sep 17 00:00:00 2001 From: Peter Thomassen Date: Thu, 21 Nov 2024 11:27:21 +0100 Subject: [PATCH 04/10] refactor(api): code style --- api/desecapi/authentication.py | 2 +- api/desecapi/models/mfa.py | 4 ++-- api/desecapi/tests/base.py | 4 +--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/api/desecapi/authentication.py b/api/desecapi/authentication.py index fef5050b5..3154c0907 100644 --- a/api/desecapi/authentication.py +++ b/api/desecapi/authentication.py @@ -76,7 +76,7 @@ def authenticate_credentials(self, key): if not token.is_valid: raise exceptions.AuthenticationFailed("Invalid token.") token.last_used = timezone.now() - token.save() + token.save(update_fields=["last_used"]) return user, token diff --git a/api/desecapi/models/mfa.py b/api/desecapi/models/mfa.py index 71b96bfc0..6e5742211 100644 --- a/api/desecapi/models/mfa.py +++ b/api/desecapi/models/mfa.py @@ -23,13 +23,13 @@ class Meta: models.UniqueConstraint(fields=["user", "name"], name="unique_user_name"), ] - @transaction.atomic() + @transaction.atomic def delete(self): if self.last_used is not None: self.user.save(credentials_changed=True) return super().delete() - @transaction.atomic() + @transaction.atomic def save(self, *args, **kwargs): if not self.user.mfa_enabled: # enabling MFA self.user.save(credentials_changed=True) diff --git a/api/desecapi/tests/base.py b/api/desecapi/tests/base.py index de15ab1c8..5c8fc4af3 100644 --- a/api/desecapi/tests/base.py +++ b/api/desecapi/tests/base.py @@ -1002,9 +1002,7 @@ def has_local_suffix(cls, domain_name: str): @classmethod def create_token(cls, user, **kwargs): - token = Token.objects.create(user=user, **kwargs) - token.save() - return token + return Token.objects.create(user=user, **kwargs) @classmethod def create_user(cls, needs_captcha=False, **kwargs): From 8b5c822fc03e63269c7acb3b6ed82a5a4754a0fc Mon Sep 17 00:00:00 2001 From: Peter Thomassen Date: Wed, 20 Nov 2024 18:35:04 +0100 Subject: [PATCH 05/10] feat(api): add Token.auto_policy (including default policy logic) Related: #885 --- ...policy_token_token_auto_policy_and_more.py | 50 ++++++++++ api/desecapi/models/tokens.py | 76 +++++++++++++++- api/desecapi/serializers/tokens.py | 7 ++ .../tests/test_token_domain_policy.py | 91 ++++++++++++++++++- api/desecapi/tests/test_tokens.py | 2 + 5 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 api/desecapi/migrations/0040_token_auto_policy_token_token_auto_policy_and_more.py diff --git a/api/desecapi/migrations/0040_token_auto_policy_token_token_auto_policy_and_more.py b/api/desecapi/migrations/0040_token_auto_policy_token_token_auto_policy_and_more.py new file mode 100644 index 000000000..9e686a802 --- /dev/null +++ b/api/desecapi/migrations/0040_token_auto_policy_token_token_auto_policy_and_more.py @@ -0,0 +1,50 @@ +# Generated by Django 5.1.3 on 2024-11-21 15:45 + +import pgtrigger.compiler +import pgtrigger.migrations +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("desecapi", "0039_token_perm_create_domain_token_perm_delete_domain"), + ] + + operations = [ + migrations.AddField( + model_name="token", + name="auto_policy", + field=models.BooleanField(default=False), + ), + pgtrigger.migrations.AddTrigger( + model_name="token", + trigger=pgtrigger.compiler.Trigger( + name="token_auto_policy", + sql=pgtrigger.compiler.UpsertTriggerSql( + constraint="CONSTRAINT", + func="\n IF\n NEW.auto_policy = true AND NOT EXISTS(\n SELECT * FROM desecapi_tokendomainpolicy WHERE token_id = NEW.id AND domain_id IS NULL AND subname IS NULL AND type IS NULL\n )\n THEN\n RAISE EXCEPTION 'Token auto policy without a default policy is not allowed. (token.id=%s)', NEW.id;\n END IF;\n RETURN NULL;\n ", + hash="f2cc6e70892817e04cbfbbff63bd4ddbe05b9aa5", + operation="UPDATE OR INSERT", + pgid="pgtrigger_token_auto_policy_8e6d9", + table="desecapi_token", + timing="DEFERRABLE INITIALLY DEFERRED", + when="AFTER", + ), + ), + ), + pgtrigger.migrations.AddTrigger( + model_name="tokendomainpolicy", + trigger=pgtrigger.compiler.Trigger( + name="default_policy_when_auto_policy", + sql=pgtrigger.compiler.UpsertTriggerSql( + func="\n IF\n OLD.domain_id IS NULL AND OLD.subname IS NULL AND OLD.type IS NULL AND (SELECT auto_policy FROM desecapi_token WHERE id = OLD.token_id) = true\n THEN\n RAISE EXCEPTION 'Cannot delete default policy while auto_policy is in effect. (tokendomainpolicy.id=%s)', OLD.id;\n END IF;\n RETURN OLD;\n ", + hash="3d55e73e6ae7d089b5ff136ac16f0c2675285bf2", + operation="DELETE", + pgid="pgtrigger_default_policy_when_auto_policy_a1fd2", + table="desecapi_tokendomainpolicy", + when="BEFORE", + ), + ), + ), + ] diff --git a/api/desecapi/models/tokens.py b/api/desecapi/models/tokens.py index 49cb8e160..3480384f9 100644 --- a/api/desecapi/models/tokens.py +++ b/api/desecapi/models/tokens.py @@ -11,7 +11,7 @@ from django.contrib.postgres.fields import ArrayField from django.core import validators from django.core.exceptions import ValidationError -from django.db import models +from django.db import models, transaction from django.db.models import F, Q from django.utils import timezone from django_prometheus.models import ExportModelOperationsMixin @@ -52,6 +52,7 @@ def _allowed_subnets_default(): null=True, default=None, validators=_validators ) domain_policies = models.ManyToManyField("Domain", through="TokenDomainPolicy") + auto_policy = models.BooleanField(default=False) plain = None objects = NetManager() @@ -60,6 +61,27 @@ class Meta: constraints = [ models.UniqueConstraint(fields=["id", "user"], name="unique_id_user") ] + triggers = [ + # Ensure that a default policy is defined when auto_policy=true + pgtrigger.Trigger( + name="token_auto_policy", + operation=pgtrigger.Update | pgtrigger.Insert, + when=pgtrigger.After, + timing=pgtrigger.Deferred, + func=pgtrigger.Func( + """ + IF + NEW.auto_policy = true AND NOT EXISTS( + SELECT * FROM {meta.many_to_many[0].remote_field.through._meta.db_table} WHERE token_id = NEW.id AND domain_id IS NULL AND subname IS NULL AND type IS NULL + ) + THEN + RAISE EXCEPTION 'Token auto policy without a default policy is not allowed. (token.id=%s)', NEW.id; + END IF; + RETURN NULL; + """ + ), + ), + ] @property def is_valid(self): @@ -135,6 +157,24 @@ def can_safely_delete_domain(self, domain): ) return not forbidden + def clean(self): + if not self.auto_policy: + return + default_policy = self.get_policy() + if default_policy and default_policy.perm_write: + raise ValidationError( + {"auto_policy": ["Auto policy requires a restrictive default policy."]} + ) + + @transaction.atomic + def save(self, *args, **kwargs): + # Do not perform policy checks when only updating fields like last_used + if "auto_policy" in kwargs.get("update_fields", ["auto_policy"]): + self.clean() + super().save(*args, **kwargs) + if self.auto_policy and self.get_policy() is None: + TokenDomainPolicy(token=self).save() + class TokenDomainPolicy(ExportModelOperationsMixin("TokenDomainPolicy"), models.Model): id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) @@ -190,6 +230,22 @@ class Meta: """ ), ), + # Ensure default policy when auto_policy is in effect + pgtrigger.Trigger( + name="default_policy_when_auto_policy", + operation=pgtrigger.Delete, + when=pgtrigger.Before, + func=pgtrigger.Func( + """ + IF + OLD.domain_id IS NULL AND OLD.subname IS NULL AND OLD.type IS NULL AND (SELECT auto_policy FROM {fields.token.remote_field.model._meta.db_table} WHERE id = OLD.token_id) = true + THEN + RAISE EXCEPTION 'Cannot delete default policy while auto_policy is in effect. (tokendomainpolicy.id=%s)', OLD.id; + END IF; + RETURN OLD; + """ + ), + ), ] @property @@ -223,6 +279,15 @@ def clean(self): ] } ) + # Can't relax default policy if auto_policy is in effect + if self.perm_write and self.is_default_policy and self.token.auto_policy: + raise ValidationError( + { + "perm_write": [ + "Must be false when auto_policy is in effect for the token." + ] + } + ) def delete(self, *args, **kwargs): # Can't delete default policy when others exist @@ -237,6 +302,15 @@ def delete(self, *args, **kwargs): ] } ) + # Can't delete default policy when auto_policy is in effect + if self.is_default_policy and self.token.auto_policy: + raise ValidationError( + { + "non_field_errors": [ + "Can't delete default policy when auto_policy is in effect for the token." + ] + } + ) return super().delete(*args, **kwargs) def save(self, *args, **kwargs): diff --git a/api/desecapi/serializers/tokens.py b/api/desecapi/serializers/tokens.py index 6fe403498..1f9b21a55 100644 --- a/api/desecapi/serializers/tokens.py +++ b/api/desecapi/serializers/tokens.py @@ -23,6 +23,7 @@ class Meta: "perm_delete_domain", "perm_manage_tokens", "allowed_subnets", + "auto_policy", "is_valid", "token", ) @@ -38,6 +39,12 @@ def get_fields(self): fields.pop("token") return fields + def save(self, **kwargs): + try: + return super().save(**kwargs) + except django.core.exceptions.ValidationError as exc: + raise serializers.ValidationError(exc.message_dict) + class DomainSlugRelatedField(serializers.SlugRelatedField): def get_queryset(self): diff --git a/api/desecapi/tests/test_token_domain_policy.py b/api/desecapi/tests/test_token_domain_policy.py index 21f0031b8..1ad1d047c 100644 --- a/api/desecapi/tests/test_token_domain_policy.py +++ b/api/desecapi/tests/test_token_domain_policy.py @@ -1,4 +1,4 @@ -from django.db import transaction +from django.db import connection, transaction from django.db.utils import IntegrityError from rest_framework import status from rest_framework.test import APIClient @@ -632,3 +632,92 @@ def test_user_deletion(self): self.token.user.delete() self.assertFalse(models.TokenDomainPolicy.objects.filter(pk=policy_pk).exists()) + + +class TokenAutoPolicyTestCase(DomainOwnerTestCase): + client_class = TokenDomainPolicyClient + + def setUp(self): + super().setUp() + self.client.credentials() # remove default credential (corresponding to domain owner) + self.token_manage = self.create_token(self.owner, perm_manage_tokens=True) + self.other_token = self.create_token(self.user) + + def test_default_policy_constraints(self): + self.assertFalse(self.token.tokendomainpolicy_set.exists()) + + # Restrictive default policy created when setting auto_policy=true + url = DomainOwnerTestCase.reverse("v1:token-detail", pk=self.token.id) + response = self.client._request( + self.client.patch, url, using=self.token_manage, data={"auto_policy": True} + ) + self.assertStatus(response, status.HTTP_200_OK) + self.assertEqual(self.token.tokendomainpolicy_set.count(), 1) + default_policy = self.token.get_policy() + self.assertFalse(default_policy.perm_write) + + # Can't relax default policy + response = self.client.patch_policy( + self.token, + using=self.token_manage, + policy_id=default_policy.id, + data={"perm_write": True}, + ) + self.assertStatus(response, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + response.data["perm_write"][0], + "Must be false when auto_policy is in effect for the token.", + ) + + # Can't delete default policy + response = self.client.delete_policy( + self.token, using=self.token_manage, policy_id=default_policy.id + ) + self.assertStatus(response, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + response.data["non_field_errors"][0], + "Can't delete default policy when auto_policy is in effect for the token.", + ) + + # Can relax default policy when auto_policy=false + self.token.auto_policy = False + self.token.save() + connection.check_constraints() # simulate transaction commit + + response = self.client.patch_policy( + self.token, + using=self.token_manage, + policy_id=default_policy.id, + data={"perm_write": True}, + ) + self.assertStatus(response, status.HTTP_200_OK) + connection.check_constraints() # simulate transaction commit + + # Can't set auto_policy when default policy is permissive + response = self.client._request( + self.client.patch, url, using=self.token_manage, data={"auto_policy": True} + ) + self.assertStatus(response, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + response.data["auto_policy"][0], + "Auto policy requires a restrictive default policy.", + ) + + # Can delete default policy when auto_policy=false + response = self.client.delete_policy( + self.token, using=self.token_manage, policy_id=default_policy.id + ) + self.assertStatus(response, status.HTTP_204_NO_CONTENT) + + def test_auto_policy_from_creation(self): + url = DomainOwnerTestCase.reverse("v1:token-list") + response = self.client._request( + self.client.post, url, using=self.token_manage, data={"auto_policy": True} + ) + self.assertStatus(response, status.HTTP_201_CREATED) + self.assertTrue(response.data["auto_policy"]) + + # Check that restrictive default policy has been created + token = models.Token.objects.get(pk=response.data["id"]) + self.assertEqual(token.tokendomainpolicy_set.count(), 1) + self.assertFalse(token.get_policy().perm_write) diff --git a/api/desecapi/tests/test_tokens.py b/api/desecapi/tests/test_tokens.py index 98239bd3a..60374f9ba 100644 --- a/api/desecapi/tests/test_tokens.py +++ b/api/desecapi/tests/test_tokens.py @@ -63,6 +63,7 @@ def test_retrieve_my_token(self): "perm_delete_domain", "perm_manage_tokens", "allowed_subnets", + "auto_policy", "is_valid", }, ) @@ -134,6 +135,7 @@ def test_create_token(self): "perm_delete_domain", "perm_manage_tokens", "allowed_subnets", + "auto_policy", "is_valid", "token", }, From e39fe08a207183ad9238768bfa854447f0fdb56e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 04:58:45 +0000 Subject: [PATCH 06/10] chore(deps): update django requirement from ~=5.1.2 to ~=5.1.3 in /api Updates the requirements on [django](https://github.com/django/django) to permit the latest version. - [Commits](https://github.com/django/django/compare/5.1.2...5.1.3) --- updated-dependencies: - dependency-name: django dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- api/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/requirements.txt b/api/requirements.txt index 23ea91f3e..fde2d09d7 100644 --- a/api/requirements.txt +++ b/api/requirements.txt @@ -2,7 +2,7 @@ captcha~=0.6.0 celery~=5.4.0 coverage~=7.6.4 cryptography~=43.0.3 -Django~=5.1.2 +Django~=5.1.3 django-cors-headers~=4.6.0 djangorestframework~=3.14.0 django-celery-email~=3.0.0 From 490efa8de6df0fe2671eec9aa9ad48a8606bc92c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 25 Nov 2024 04:48:40 +0000 Subject: [PATCH 07/10] chore(deps): update django-pgtrigger requirement in /api Updates the requirements on [django-pgtrigger](https://github.com/Opus10/django-pgtrigger) to permit the latest version. - [Release notes](https://github.com/Opus10/django-pgtrigger/releases) - [Changelog](https://github.com/Opus10/django-pgtrigger/blob/main/CHANGELOG.md) - [Commits](https://github.com/Opus10/django-pgtrigger/compare/4.12.2...4.13.2) --- updated-dependencies: - dependency-name: django-pgtrigger dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- api/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/requirements.txt b/api/requirements.txt index fde2d09d7..456d58b7f 100644 --- a/api/requirements.txt +++ b/api/requirements.txt @@ -7,7 +7,7 @@ django-cors-headers~=4.6.0 djangorestframework~=3.14.0 django-celery-email~=3.0.0 django-netfields~=1.3.2 -django-pgtrigger~=4.12.2 +django-pgtrigger~=4.13.2 django-prometheus~=2.3.1 dnspython~=2.7.0 httpretty~=1.0.5 # 1.1 breaks tests. Does not run in production, so stick to it. From 0bdf05de62280f73236c9337bbfd1c8532b0dc4d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 25 Nov 2024 04:48:47 +0000 Subject: [PATCH 08/10] chore(deps): update coverage requirement from ~=7.6.4 to ~=7.6.8 in /api Updates the requirements on [coverage](https://github.com/nedbat/coveragepy) to permit the latest version. - [Release notes](https://github.com/nedbat/coveragepy/releases) - [Changelog](https://github.com/nedbat/coveragepy/blob/master/CHANGES.rst) - [Commits](https://github.com/nedbat/coveragepy/compare/7.6.4...7.6.8) --- updated-dependencies: - dependency-name: coverage dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- api/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/requirements.txt b/api/requirements.txt index 456d58b7f..2cc2bee06 100644 --- a/api/requirements.txt +++ b/api/requirements.txt @@ -1,6 +1,6 @@ captcha~=0.6.0 celery~=5.4.0 -coverage~=7.6.4 +coverage~=7.6.8 cryptography~=43.0.3 Django~=5.1.3 django-cors-headers~=4.6.0 From 1ade9d0b205f6be2782bc08b12bdccd42f829fca Mon Sep 17 00:00:00 2001 From: Peter Thomassen Date: Fri, 22 Nov 2024 13:27:09 +0100 Subject: [PATCH 09/10] feat(api): honor auto_policy during domain creation, closes #885 --- .../tests/test_token_domain_policy.py | 31 +++++++++++++++++++ api/desecapi/views/domains.py | 4 +++ docs/auth/tokens.rst | 15 +++++++++ 3 files changed, 50 insertions(+) diff --git a/api/desecapi/tests/test_token_domain_policy.py b/api/desecapi/tests/test_token_domain_policy.py index 1ad1d047c..efe635348 100644 --- a/api/desecapi/tests/test_token_domain_policy.py +++ b/api/desecapi/tests/test_token_domain_policy.py @@ -721,3 +721,34 @@ def test_auto_policy_from_creation(self): token = models.Token.objects.get(pk=response.data["id"]) self.assertEqual(token.tokendomainpolicy_set.count(), 1) self.assertFalse(token.get_policy().perm_write) + + def test_create_domain(self): + self.token.auto_policy = True + self.token.save() + self.token_manage.perm_create_domain = True + self.token_manage.save() + + name_auto = "domain.example" + name_other = "other.example" + with self.assertRequests(self.requests_desec_domain_creation(name_other)): + response = self.client._request( + self.client.post, + self.reverse("v1:domain-list"), + using=self.token_manage, + data={"name": name_other}, + ) + self.assertStatus(response, status.HTTP_201_CREATED) + with self.assertRequests(self.requests_desec_domain_creation(name_auto)): + response = self.client._request( + self.client.post, + self.reverse("v1:domain-list"), + using=self.token, + data={"name": name_auto}, + ) + self.assertStatus(response, status.HTTP_201_CREATED) + + self.assertEqual(self.token.tokendomainpolicy_set.count(), 2) + rrset = models.RRset(domain=models.Domain.objects.get(name=name_auto)) + self.assertTrue(self.token.get_policy(rrset).perm_write) + rrset = models.RRset(domain=models.Domain.objects.get(name=name_other)) + self.assertFalse(self.token.get_policy(rrset).perm_write) diff --git a/api/desecapi/views/domains.py b/api/desecapi/views/domains.py index 829e729ad..6b305ba07 100644 --- a/api/desecapi/views/domains.py +++ b/api/desecapi/views/domains.py @@ -103,6 +103,10 @@ def perform_create(self, serializer): ) with PDNSChangeTracker(): domain = serializer.save(owner=self.request.user) + if self.request.auth.auto_policy: + self.request.auth.tokendomainpolicy_set.create( + domain=domain, perm_write=True + ) # TODO this line raises if the local public suffix is not in our database! PDNSChangeTracker.track(lambda: self.auto_delegate(domain)) diff --git a/docs/auth/tokens.rst b/docs/auth/tokens.rst index 7e84f419c..014ca1b4a 100644 --- a/docs/auth/tokens.rst +++ b/docs/auth/tokens.rst @@ -35,6 +35,7 @@ A JSON object representing a token has the following structure:: "0.0.0.0/0", "::/0" ], + "auto_policy": false, "max_age": "365 00:00:00", "max_unused_period": null, "token": "4pnk7u-NHvrEkFzrhFDRTjGFyX_S" @@ -50,6 +51,17 @@ Field details: order to successfully authenticate with the token. Both IPv4 and IPv6 are supported. Defaults to ``0.0.0.0/0, ::/0`` (no restriction). +``auto_policy`` + :Access mode: read, write + :Type: boolean + + When using this token to create a domain, automatically configure a + permissive scoping policy for it (``perm_write=true``). Requires a + restrictive default policy (``perm_write=false``), which is created + automatically when setting this flag. Cannot be set to true if a + permissive default policy exists. For details, see + :ref:`token scoping policies`. + ``created`` :Access mode: read-only :Type: timestamp @@ -173,6 +185,7 @@ Note that the name and other fields are optional. The server will reply with "0.0.0.0/0", "::/0" ], + "auto_policy": false, "token": "4pnk7u-NHvrEkFzrhFDRTjGFyX_S" } @@ -350,6 +363,8 @@ 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. +A domain-wide permissive policy can be configured automatically during domain +creation by setting the token's ``auto_policy`` flag. Tokens with at least one policy are considered *restricted*, with their DNS record management capabilities limited as per policy configuration. From a7d2cc72f8924b83ea49ed41635e0f9d3031bf99 Mon Sep 17 00:00:00 2001 From: Peter Thomassen Date: Fri, 22 Nov 2024 13:27:43 +0100 Subject: [PATCH 10/10] chore(api): comment typo --- api/desecapi/permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/desecapi/permissions.py b/api/desecapi/permissions.py index 28f908688..516cc8415 100644 --- a/api/desecapi/permissions.py +++ b/api/desecapi/permissions.py @@ -123,7 +123,7 @@ def has_permission(self, request, view): class HasDeleteDomainPermission(permissions.BasePermission): """ - Permission to check whether a token has "delete domian" permission. + Permission to check whether a token has "delete domain" permission. """ def has_permission(self, request, view):