From 1fb8bd37a7e2591ff475e736286fb24bb316066d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Falconnier?= Date: Tue, 1 Aug 2023 11:11:30 +0200 Subject: [PATCH] Protect FileVaultConfig from deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … when the FileVaultConfig is used in a Blueprint. --- tests/mdm/test_api_filevault_configs_views.py | 10 ++++- tests/mdm/test_management_filevault_config.py | 38 ++++++++++++++++++- .../mdm/api_views/filevault_configs.py | 7 ++++ zentral/contrib/mdm/models.py | 10 +++++ .../templates/mdm/filevaultconfig_detail.html | 2 +- .../templates/mdm/filevaultconfig_list.html | 2 +- zentral/contrib/mdm/views/management.py | 8 +++- 7 files changed, 70 insertions(+), 7 deletions(-) diff --git a/tests/mdm/test_api_filevault_configs_views.py b/tests/mdm/test_api_filevault_configs_views.py index 30f2b5bcfd..81a68322f0 100644 --- a/tests/mdm/test_api_filevault_configs_views.py +++ b/tests/mdm/test_api_filevault_configs_views.py @@ -9,7 +9,7 @@ from accounts.models import APIToken, User from zentral.contrib.mdm.models import FileVaultConfig from zentral.core.events.base import AuditEvent -from .utils import force_filevault_config +from .utils import force_blueprint, force_filevault_config @override_settings(STATICFILES_STORAGE='django.contrib.staticfiles.storage.StaticFilesStorage') @@ -335,6 +335,14 @@ def test_delete_filevault_config_permission_denied(self): response = self.delete(reverse("mdm_api:filevault_config", args=(fv_config.pk,))) self.assertEqual(response.status_code, 403) + def test_delete_filevault_config_cannot_be_deleted(self): + fv_config = force_filevault_config() + force_blueprint(filevault_config=fv_config) + self.set_permissions("mdm.delete_filevaultconfig") + response = self.delete(reverse("mdm_api:filevault_config", args=(fv_config.pk,))) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), ["This FileVault configuration cannot be deleted"]) + @patch("zentral.core.queues.backends.kombu.EventQueues.post_event") def test_delete_filevault_config(self, post_event): fv_config = force_filevault_config() diff --git a/tests/mdm/test_management_filevault_config.py b/tests/mdm/test_management_filevault_config.py index e7be21bf32..0c76746443 100644 --- a/tests/mdm/test_management_filevault_config.py +++ b/tests/mdm/test_management_filevault_config.py @@ -8,7 +8,7 @@ from django.utils.crypto import get_random_string from accounts.models import User from zentral.core.events.base import AuditEvent -from .utils import force_filevault_config +from .utils import force_blueprint, force_filevault_config @override_settings(STATICFILES_STORAGE='django.contrib.staticfiles.storage.StaticFilesStorage') @@ -51,13 +51,30 @@ def test_filevault_configurations_permission_denied(self): response = self.client.get(reverse("mdm:filevault_configs")) self.assertEqual(response.status_code, 403) - def test_filevault_configurations(self): + def test_filevault_configurations_no_links(self): fv_config = force_filevault_config() self._login("mdm.view_filevaultconfig") response = self.client.get(reverse("mdm:filevault_configs")) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, "mdm/filevaultconfig_list.html") self.assertContains(response, fv_config.name) + self.assertNotContains(response, reverse("mdm:update_filevault_config", args=(fv_config.pk,))) + self.assertNotContains(response, reverse("mdm:delete_filevault_config", args=(fv_config.pk,))) + + def test_filevault_configurations_all_links(self): + fv_config1 = force_filevault_config() + force_blueprint(filevault_config=fv_config1) + fv_config2 = force_filevault_config() + self._login("mdm.view_filevaultconfig", "mdm.change_filevaultconfig", "mdm.delete_filevaultconfig") + response = self.client.get(reverse("mdm:filevault_configs")) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "mdm/filevaultconfig_list.html") + self.assertContains(response, fv_config1.name) + self.assertContains(response, fv_config2.name) + self.assertContains(response, reverse("mdm:update_filevault_config", args=(fv_config1.pk,))) + self.assertNotContains(response, reverse("mdm:delete_filevault_config", args=(fv_config1.pk,))) + self.assertContains(response, reverse("mdm:update_filevault_config", args=(fv_config2.pk,))) + self.assertContains(response, reverse("mdm:delete_filevault_config", args=(fv_config2.pk,))) # create FileVault configuration @@ -190,6 +207,16 @@ def test_filevault_configuration_get_no_perm_no_delete_link(self): self.assertContains(response, fv_config.name) self.assertNotContains(response, reverse("mdm:delete_filevault_config", args=(fv_config.pk,))) + def test_filevault_configuration_get_cannot_be_deleted_no_delete_link(self): + fv_config = force_filevault_config() + force_blueprint(filevault_config=fv_config) + self._login("mdm.view_filevaultconfig", "mdm.delete_filevaultconfig") + response = self.client.get(reverse("mdm:filevault_config", args=(fv_config.pk,))) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "mdm/filevaultconfig_detail.html") + self.assertContains(response, fv_config.name) + self.assertNotContains(response, reverse("mdm:delete_filevault_config", args=(fv_config.pk,))) + # update FileVault configuration def test_update_filevault_configuration_redirect(self): @@ -284,6 +311,13 @@ def test_delete_filevault_configuration_permission_denied(self): response = self.client.get(reverse("mdm:delete_filevault_config", args=(fv_config.pk,))) self.assertEqual(response.status_code, 403) + def test_delete_filevault_configuration_404(self): + fv_config = force_filevault_config() + force_blueprint(filevault_config=fv_config) + self._login("mdm.delete_filevaultconfig") + response = self.client.get(reverse("mdm:delete_filevault_config", args=(fv_config.pk,))) + self.assertEqual(response.status_code, 404) + def test_delete_filevault_configuration_get(self): fv_config = force_filevault_config() self._login("mdm.delete_filevaultconfig") diff --git a/zentral/contrib/mdm/api_views/filevault_configs.py b/zentral/contrib/mdm/api_views/filevault_configs.py index a36ff199a0..f767fa423d 100644 --- a/zentral/contrib/mdm/api_views/filevault_configs.py +++ b/zentral/contrib/mdm/api_views/filevault_configs.py @@ -1,3 +1,4 @@ +from rest_framework.exceptions import ValidationError from zentral.utils.drf import ListCreateAPIViewWithAudit, RetrieveUpdateDestroyAPIViewWithAudit from zentral.contrib.mdm.models import FileVaultConfig from zentral.contrib.mdm.serializers import FileVaultConfigSerializer @@ -18,3 +19,9 @@ class FileVaultConfigDetail(RetrieveUpdateDestroyAPIViewWithAudit): """ queryset = FileVaultConfig.objects.all() serializer_class = FileVaultConfigSerializer + + def perform_destroy(self, instance): + if not instance.can_be_deleted(): + raise ValidationError('This FileVault configuration cannot be deleted') + else: + return super().perform_destroy(instance) diff --git a/zentral/contrib/mdm/models.py b/zentral/contrib/mdm/models.py index 9e29dfeacd..a4bfff66ff 100644 --- a/zentral/contrib/mdm/models.py +++ b/zentral/contrib/mdm/models.py @@ -97,6 +97,11 @@ def rewrap_secrets(self): # FileVault +class FileVaultConfigManager(models.Manager): + def can_be_deleted(self): + return self.annotate(bp_count=Count("blueprint")).filter(bp_count=0) + + class FileVaultConfig(models.Model): name = models.CharField(max_length=256, unique=True) escrow_location_display_name = models.CharField( @@ -133,6 +138,8 @@ class FileVaultConfig(models.Model): created_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) + objects = FileVaultConfigManager() + class Meta: ordering = ("name",) verbose_name = "filevault config" @@ -150,6 +157,9 @@ def uuid(self): f"{self.destroy_key_on_standby}|{self.prk_rotation_interval_days}".encode("utf-8")) return uuid.UUID(hex=h.hexdigest()) + def can_be_deleted(self): + return FileVaultConfig.objects.can_be_deleted().filter(pk=self.pk).exists() + def serialize_for_event(self, keys_only=False): d = {"pk": self.pk, "name": self.name} if keys_only: diff --git a/zentral/contrib/mdm/templates/mdm/filevaultconfig_detail.html b/zentral/contrib/mdm/templates/mdm/filevaultconfig_detail.html index 1fe44da52b..b3cd25b002 100644 --- a/zentral/contrib/mdm/templates/mdm/filevaultconfig_detail.html +++ b/zentral/contrib/mdm/templates/mdm/filevaultconfig_detail.html @@ -84,7 +84,7 @@

{{ object }}

Edit {% endif %} - {% if perms.mdm.delete_filevaultconfig %} + {% if perms.mdm.delete_filevaultconfig and object.can_be_deleted %} diff --git a/zentral/contrib/mdm/templates/mdm/filevaultconfig_list.html b/zentral/contrib/mdm/templates/mdm/filevaultconfig_list.html index 009b50a0e7..61bdc21b4d 100644 --- a/zentral/contrib/mdm/templates/mdm/filevaultconfig_list.html +++ b/zentral/contrib/mdm/templates/mdm/filevaultconfig_list.html @@ -56,7 +56,7 @@

{{ page_obj.paginator.count }} FileVault configuration{{ page_obj.paginator. {% endif %} - {% if perms.mdm.delete_filevaultconfig %} + {% if perms.mdm.delete_filevaultconfig and filevault_config.can_be_deleted %} diff --git a/zentral/contrib/mdm/views/management.py b/zentral/contrib/mdm/views/management.py index a1e115667b..29a5c1f107 100644 --- a/zentral/contrib/mdm/views/management.py +++ b/zentral/contrib/mdm/views/management.py @@ -974,9 +974,11 @@ def form_valid(self, form): class DeleteBlueprintView(PermissionRequiredMixin, DeleteViewWithAudit): permission_required = "mdm.delete_blueprint" - queryset = Blueprint.objects.can_be_deleted() success_url = reverse_lazy("mdm:blueprints") + def get_queryset(self): + return Blueprint.objects.can_be_deleted() + # FileVault Configurations @@ -1012,9 +1014,11 @@ class UpdateFileVaultConfigView(PermissionRequiredMixin, UpdateViewWithAudit): class DeleteFileVaultConfigView(PermissionRequiredMixin, DeleteViewWithAudit): permission_required = "mdm.delete_filevaultconfig" - model = FileVaultConfig success_url = reverse_lazy("mdm:filevault_configs") + def get_queryset(self): + return FileVaultConfig.objects.can_be_deleted() + # SCEP Configurations