From 5b85d45067b690d46ae9bfdeac629b61e432ccc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Falconnier?= Date: Sun, 6 Aug 2023 12:48:12 +0200 Subject: [PATCH] Validate no rotation for static recovery password --- ...est_api_recovery_password_configs_views.py | 56 +++++++++++++------ ...est_management_recovery_password_config.py | 35 ++++++++---- zentral/contrib/mdm/forms.py | 9 +++ zentral/contrib/mdm/serializers.py | 18 +++++- .../mdm/recoverypasswordconfig_form.html | 4 ++ 5 files changed, 91 insertions(+), 31 deletions(-) diff --git a/tests/mdm/test_api_recovery_password_configs_views.py b/tests/mdm/test_api_recovery_password_configs_views.py index c6938da92c..18082f353f 100644 --- a/tests/mdm/test_api_recovery_password_configs_views.py +++ b/tests/mdm/test_api_recovery_password_configs_views.py @@ -170,16 +170,18 @@ def test_create_recovery_password_config(self, post_event): with self.captureOnCommitCallbacks(execute=True) as callbacks: response = self.post(reverse("mdm_api:recovery_password_configs"), {"name": name, - "static_password": None}) + "dynamic_password": False, + "static_password": "12345678"}) self.assertEqual(response.status_code, 201) self.assertEqual(len(callbacks), 1) rp_config = RecoveryPasswordConfig.objects.get(name=name) + self.assertEqual(rp_config.get_static_password(), "12345678") self.assertEqual( response.json(), {'id': rp_config.pk, 'name': rp_config.name, - 'dynamic_password': True, - 'static_password': None, + 'dynamic_password': False, + 'static_password': "12345678", 'rotation_interval_days': 0, 'rotate_firmware_password': False, 'created_at': rp_config.created_at.isoformat(), @@ -196,7 +198,7 @@ def test_create_recovery_password_config(self, post_event): "new_value": { "pk": rp_config.pk, "name": name, - "dynamic_password": True, + "dynamic_password": False, "rotation_interval_days": 0, "rotate_firmware_password": False, "created_at": rp_config.created_at, @@ -226,15 +228,18 @@ def test_update_recovery_password_config_permission_denied(self): def test_update_recovery_password_dynamic_and_static_password_error(self): rp_config = force_recovery_password_config() self.set_permissions("mdm.change_recoverypasswordconfig") - static_password = get_random_string(12) response = self.put(reverse("mdm_api:recovery_password_config", args=(rp_config.pk,)), {"name": get_random_string(12), "dynamic_password": True, - "static_password": static_password, - "rotation_interval_days": 17, + "static_password": get_random_string(12), + "rotation_interval_days": 0, "rotate_firmware_password": True}) self.assertEqual(response.status_code, 400) - self.assertEqual(response.json(), {'static_password': ['Cannot be set when dynamic_password is true']}) + self.assertEqual( + response.json(), + {'static_password': ['Cannot be set when dynamic_password is true'], + 'rotate_firmware_password': ['Cannot be set without a rotation interval']} + ) def test_update_recovery_password_required_static_password_error(self): rp_config = force_recovery_password_config() @@ -245,7 +250,27 @@ def test_update_recovery_password_required_static_password_error(self): "rotation_interval_days": 17, "rotate_firmware_password": True}) self.assertEqual(response.status_code, 400) - self.assertEqual(response.json(), {'static_password': ['Required when dynamic_password is false']}) + self.assertEqual( + response.json(), + {'static_password': ['Required when dynamic_password is false'], + 'rotation_interval_days': ['Cannot be set with a static password'], + 'rotate_firmware_password': ['Cannot be set with a static password']} + ) + + def test_update_recovery_password_required_static_rotation_error(self): + rp_config = force_recovery_password_config() + self.set_permissions("mdm.change_recoverypasswordconfig") + response = self.put(reverse("mdm_api:recovery_password_config", args=(rp_config.pk,)), + {"name": get_random_string(12), + "dynamic_password": False, + "static_password": "1234568", + "rotation_interval_days": 17, + "rotate_firmware_password": True}) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), { + 'rotation_interval_days': ['Cannot be set with a static password'], + 'rotate_firmware_password': ['Cannot be set with a static password']} + ) @patch("zentral.core.queues.backends.kombu.EventQueues.post_event") def test_update_recovery_password_config(self, post_event): @@ -253,28 +278,25 @@ def test_update_recovery_password_config(self, post_event): prev_value = rp_config.serialize_for_event() self.set_permissions("mdm.change_recoverypasswordconfig") new_name = get_random_string(12) - static_password = get_random_string(12) with self.captureOnCommitCallbacks(execute=True) as callbacks: response = self.put(reverse("mdm_api:recovery_password_config", args=(rp_config.pk,)), {"name": new_name, - "dynamic_password": False, - "static_password": static_password, + "dynamic_password": True, "rotation_interval_days": 17, "rotate_firmware_password": True}) self.assertEqual(response.status_code, 200) self.assertEqual(len(callbacks), 1) rp_config.refresh_from_db() self.assertEqual(rp_config.name, new_name) - self.assertFalse(rp_config.dynamic_password) - self.assertEqual(rp_config.get_static_password(), static_password) + self.assertTrue(rp_config.dynamic_password) self.assertEqual(rp_config.rotation_interval_days, 17) self.assertTrue(rp_config.rotate_firmware_password) self.assertEqual( response.json(), {'id': rp_config.pk, 'name': new_name, - 'dynamic_password': False, - 'static_password': static_password, + 'dynamic_password': True, + 'static_password': None, 'rotation_interval_days': 17, 'rotate_firmware_password': True, 'created_at': rp_config.created_at.isoformat(), @@ -291,7 +313,7 @@ def test_update_recovery_password_config(self, post_event): "new_value": { "pk": rp_config.pk, "name": new_name, - "dynamic_password": False, + "dynamic_password": True, "rotation_interval_days": 17, "rotate_firmware_password": True, "created_at": rp_config.created_at, diff --git a/tests/mdm/test_management_recovery_password_config.py b/tests/mdm/test_management_recovery_password_config.py index 15b2bd8a8c..75daeb7cb8 100644 --- a/tests/mdm/test_management_recovery_password_config.py +++ b/tests/mdm/test_management_recovery_password_config.py @@ -154,6 +154,20 @@ def test_create_recovery_password_configuration_static_password_non_ascii(self): "to ensure that all characters are enterable on the EFI login screen." ) + def test_create_recovery_password_configuration_static_firmware_rotation_error(self): + self._login("mdm.add_recoverypasswordconfig") + response = self.client.post(reverse("mdm:create_recovery_password_config"), + {"name": get_random_string(12), + "dynamic_password": True, + "rotation_interval_days": 0, + "rotate_firmware_password": True}, + follow=True) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "mdm/recoverypasswordconfig_form.html") + self.assertFormError( + response.context["form"], "rotate_firmware_password", "Cannot be set without a rotation interval." + ) + @patch("zentral.core.queues.backends.kombu.EventQueues.post_event") def test_create_recovery_password_configuration_post(self, post_event): self._login("mdm.add_recoverypasswordconfig", "mdm.view_recoverypasswordconfig") @@ -161,8 +175,7 @@ def test_create_recovery_password_configuration_post(self, post_event): with self.captureOnCommitCallbacks(execute=True) as callbacks: response = self.client.post(reverse("mdm:create_recovery_password_config"), {"name": name, - "dynamic_password": False, - "static_password": "12345678", + "dynamic_password": True, "rotation_interval_days": 90, "rotate_firmware_password": True}, follow=True) @@ -171,8 +184,8 @@ def test_create_recovery_password_configuration_post(self, post_event): self.assertTemplateUsed(response, "mdm/recoverypasswordconfig_detail.html") rp_config = response.context["object"] self.assertEqual(rp_config.name, name) - self.assertFalse(rp_config.dynamic_password) - self.assertEqual(rp_config.get_static_password(), "12345678") + self.assertTrue(rp_config.dynamic_password) + self.assertIsNone(rp_config.static_password) self.assertEqual(rp_config.rotation_interval_days, 90) self.assertTrue(rp_config.rotate_firmware_password) event = post_event.call_args_list[0].args[0] @@ -186,7 +199,7 @@ def test_create_recovery_password_configuration_post(self, post_event): "new_value": { "pk": rp_config.pk, "name": name, - "dynamic_password": False, + "dynamic_password": True, "rotation_interval_days": 90, "rotate_firmware_password": True, "created_at": rp_config.created_at, @@ -274,8 +287,8 @@ def test_update_recovery_password_configuration_post(self, post_event): {"name": new_name, "dynamic_password": False, "static_password": "12345678", - "rotation_interval_days": 90, - "rotate_firmware_password": True}, + "rotation_interval_days": 0, + "rotate_firmware_password": False}, follow=True) self.assertEqual(response.status_code, 200) self.assertEqual(len(callbacks), 1) @@ -285,8 +298,8 @@ def test_update_recovery_password_configuration_post(self, post_event): self.assertEqual(rp_config2.name, new_name) self.assertFalse(rp_config2.dynamic_password) self.assertEqual(rp_config2.get_static_password(), "12345678") - self.assertEqual(rp_config2.rotation_interval_days, 90) - self.assertTrue(rp_config2.rotate_firmware_password) + self.assertEqual(rp_config2.rotation_interval_days, 0) + self.assertFalse(rp_config2.rotate_firmware_password) event = post_event.call_args_list[0].args[0] self.assertIsInstance(event, AuditEvent) self.assertEqual( @@ -299,8 +312,8 @@ def test_update_recovery_password_configuration_post(self, post_event): "pk": rp_config2.pk, "name": new_name, "dynamic_password": False, - "rotation_interval_days": 90, - "rotate_firmware_password": True, + "rotation_interval_days": 0, + "rotate_firmware_password": False, "created_at": rp_config2.created_at, "updated_at": rp_config2.updated_at }, diff --git a/zentral/contrib/mdm/forms.py b/zentral/contrib/mdm/forms.py index d581537a0e..0e1c51a04f 100644 --- a/zentral/contrib/mdm/forms.py +++ b/zentral/contrib/mdm/forms.py @@ -906,6 +906,15 @@ def clean(self): static_password = self.cleaned_data.get("static_password") if not static_password and "static_password" not in self.errors: self.add_error("static_password", "This field is required when not using dynamic passwords.") + self.cleaned_data["rotation_interval_days"] = 0 + self.cleaned_data["rotate_firmware_password"] = False + else: + if ( + self.cleaned_data.get("rotate_firmware_password") + and not self.cleaned_data.get("rotation_interval_days") + ): + self.add_error("rotate_firmware_password", + "Cannot be set without a rotation interval.") def save(self): if self.instance.pk and not self.cleaned_data.get("dynamic_password"): diff --git a/zentral/contrib/mdm/serializers.py b/zentral/contrib/mdm/serializers.py index f65b32efa9..de50594f0f 100644 --- a/zentral/contrib/mdm/serializers.py +++ b/zentral/contrib/mdm/serializers.py @@ -53,11 +53,23 @@ class Meta: def validate(self, data): dynamic_password = data.get("dynamic_password", True) static_password = data.get("get_static_password") + rotation_interval_days = data.get("rotation_interval_days") + rotate_firmware_password = data.get("rotate_firmware_password") + errors = {} if dynamic_password: if static_password: - raise serializers.ValidationError({"static_password": "Cannot be set when dynamic_password is true"}) - elif not static_password: - raise serializers.ValidationError({"static_password": "Required when dynamic_password is false"}) + errors["static_password"] = "Cannot be set when dynamic_password is true" + else: + if not static_password: + errors["static_password"] = "Required when dynamic_password is false" + if rotation_interval_days: + errors["rotation_interval_days"] = "Cannot be set with a static password" + if rotate_firmware_password: + errors["rotate_firmware_password"] = "Cannot be set with a static password" + if rotate_firmware_password and not rotation_interval_days: + errors["rotate_firmware_password"] = "Cannot be set without a rotation interval" + if errors: + raise serializers.ValidationError(errors) return data def create(self, validated_data): diff --git a/zentral/contrib/mdm/templates/mdm/recoverypasswordconfig_form.html b/zentral/contrib/mdm/templates/mdm/recoverypasswordconfig_form.html index 78a6e08fe9..724f091b36 100644 --- a/zentral/contrib/mdm/templates/mdm/recoverypasswordconfig_form.html +++ b/zentral/contrib/mdm/templates/mdm/recoverypasswordconfig_form.html @@ -35,6 +35,10 @@

{% if object %}Update recovery password configuration {{ object }}{% var checked = $("#id_dynamic_password").prop("checked"); var $input = $("#id_static_password"); $input.parent().parent().toggle(!checked); + var $rotIntInput = $("#id_rotation_interval_days"); + $rotIntInput.parent().parent().toggle(checked); + var $rotFirmPwd = $("#id_rotate_firmware_password"); + $rotFirmPwd.parent().parent().parent().parent().toggle(checked); } $(document).ready(function () {