From 25f1a213225299ecb5dc0ae4960630f68f8d8480 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Wed, 27 May 2026 01:25:51 +0530 Subject: [PATCH 01/34] [feature] Extended Template model for standalone X.509 certificates #1356 - Added 'cert' to TYPE_CHOICES. - Introduced 'ca' and 'blueprint_cert' ForeignKeys with organization validation. - Updated the clean() method to clear unneeded relations, require a CA for cert types, and validate that a blueprint certificate is not already assigned to a device. Fixes #1356 --- openwisp_controller/config/base/template.py | 72 ++++++++++++++++++- ...ate_blueprint_cert_template_ca_and_more.py | 59 +++++++++++++++ ...ate_blueprint_cert_template_ca_and_more.py | 57 +++++++++++++++ 3 files changed, 185 insertions(+), 3 deletions(-) create mode 100644 openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py create mode 100644 tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index 555366b7b..0608b62bb 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -22,7 +22,11 @@ logger = logging.getLogger(__name__) -TYPE_CHOICES = (("generic", _("Generic")), ("vpn", _("VPN-client"))) +TYPE_CHOICES = ( + ("generic", _("Generic")), + ("vpn", _("VPN-client")), + ("cert", _("Certificate")), +) def default_auto_cert(): @@ -55,6 +59,28 @@ class AbstractTemplate(ShareableOrgMixinUniqueName, BaseConfig): null=True, on_delete=models.CASCADE, ) + ca = models.ForeignKey( + get_model_name("django_x509", "Ca"), + on_delete=models.CASCADE, + verbose_name=_("Certificate Authority"), + blank=True, + null=True, + help_text=_( + "The Certificate Authority that will sign certificates generated " + "by this template." + ), + ) + blueprint_cert = models.ForeignKey( + get_model_name("django_x509", "Cert"), + on_delete=models.SET_NULL, + verbose_name=_("Blueprint Certificate"), + blank=True, + null=True, + help_text=_( + "Optional: Select an unassigned certificate to copy extensions and " + "properties from." + ), + ) type = models.CharField( _("type"), max_length=16, @@ -222,6 +248,8 @@ def clean(self, *args, **kwargs): * if flagged as required forces it also to be default """ self._validate_org_relation("vpn") + self._validate_org_relation("ca") + self._validate_org_relation("blueprint_cert") if not self.default_values: self.default_values = {} if not isinstance(self.default_values, dict): @@ -234,15 +262,53 @@ def clean(self, *args, **kwargs): ) elif self.type != "vpn": self.vpn = None - self.auto_cert = False + if self.type != "cert": + self.auto_cert = False if self.type == "vpn" and not self.config: self.config = self.vpn.auto_client( auto_cert=self.auto_cert, template_backend_class=self.backend_class ) + if self.type == "cert": + if not self.ca: + raise ValidationError( + { + "ca": _( + "A Certificate Authority is required when the template " + "type is certificate." + ) + } + ) + if self.blueprint_cert and self.blueprint_cert.ca_id != self.ca_id: + raise ValidationError( + { + "blueprint_cert": _( + "The selected certificate must match the selected " + "Certificate Authority." + ) + } + ) + if self.blueprint_cert and hasattr( + self.blueprint_cert, "devicecertificate_set" + ): + if self.blueprint_cert.devicecertificate_set.exists(): + raise ValidationError( + { + "blueprint_cert": _( + "This certificate is already assigned to a device. " + "Please select an unassigned certificate to use as a " + "blueprint." + ) + } + ) + if not self.config: + self.config = {} + else: + self.ca = None + self.blueprint_cert = None if self.required and not self.default: self.default = True super().clean(*args, **kwargs) - if not self.config: + if not self.config and self.type != "cert": raise ValidationError(_("The configuration field cannot be empty.")) def get_context(self, system=False): diff --git a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py new file mode 100644 index 000000000..856dd881d --- /dev/null +++ b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py @@ -0,0 +1,59 @@ +# Generated by Django 5.2.14 on 2026-05-26 19:18 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("config", "0063_replace_jsonfield_with_django_builtin"), + migrations.swappable_dependency(settings.DJANGO_X509_CA_MODEL), + migrations.swappable_dependency(settings.DJANGO_X509_CERT_MODEL), + ] + + operations = [ + migrations.AddField( + model_name="template", + name="blueprint_cert", + field=models.ForeignKey( + blank=True, + help_text="Optional: Select an unassigned certificate to " + "copy extensions and properties from.", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to=settings.DJANGO_X509_CERT_MODEL, + verbose_name="Blueprint Certificate", + ), + ), + migrations.AddField( + model_name="template", + name="ca", + field=models.ForeignKey( + blank=True, + help_text="The Certificate Authority that will sign " + "certificates generated by this template.", + null=True, + on_delete=django.db.models.deletion.CASCADE, + to=settings.DJANGO_X509_CA_MODEL, + verbose_name="Certificate Authority", + ), + ), + migrations.AlterField( + model_name="template", + name="type", + field=models.CharField( + choices=[ + ("generic", "Generic"), + ("vpn", "VPN-client"), + ("cert", "Certificate"), + ], + db_index=True, + default="generic", + help_text="template type, determines which features are available", + max_length=16, + verbose_name="type", + ), + ), + ] diff --git a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py new file mode 100644 index 000000000..ad9d3af7f --- /dev/null +++ b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py @@ -0,0 +1,57 @@ +# Generated by Django 5.2.14 on 2026-05-26 19:52 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("sample_config", "0009_replace_jsonfield_with_django_builtin"), + ("sample_pki", "0004_alter_ca_extensions_alter_ca_key_length_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="template", + name="blueprint_cert", + field=models.ForeignKey( + blank=True, + help_text="Optional: Select an unassigned certificate to copy " + "extensions and properties from.", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="sample_pki.cert", + verbose_name="Blueprint Certificate", + ), + ), + migrations.AddField( + model_name="template", + name="ca", + field=models.ForeignKey( + blank=True, + help_text="The Certificate Authority that will sign certificates " + "generated by this template.", + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="sample_pki.ca", + verbose_name="Certificate Authority", + ), + ), + migrations.AlterField( + model_name="template", + name="type", + field=models.CharField( + choices=[ + ("generic", "Generic"), + ("vpn", "VPN-client"), + ("cert", "Certificate"), + ], + db_index=True, + default="generic", + help_text="template type, determines which features are available", + max_length=16, + verbose_name="type", + ), + ), + ] From a795e0996d4e075d40445a164416445622c62b43 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Wed, 27 May 2026 01:58:21 +0530 Subject: [PATCH 02/34] [fix] Updated and added tests Fixes #1356 Updated previous tests and added new tests for implemetation. Fixes #1356 --- .../config/tests/test_template.py | 116 ++++++++++++++++++ openwisp_controller/pki/tests/test_api.py | 4 +- 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index a8118a71c..ec1faf8f6 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -967,3 +967,119 @@ def test_auto_add_template_to_existing_configs_not_triggered(self, mocked_task): required_template.full_clean() required_template.save() mocked_task.assert_not_called() + + +class TestTemplateCertificates(CreateConfigTemplateMixin, TestVpnX509Mixin, TestCase): + """ + tests for standalone X.509 certificate Template configurations + """ + + def test_cert_template_requires_ca(self): + """Test that creating a template with type='cert' requires a CA.""" + try: + self._create_template(type="cert", config={}) + except ValidationError as err: + self.assertIn("ca", err.message_dict) + self.assertIn("required", str(err.message_dict["ca"][0])) + else: + self.fail("ValidationError not raised for missing CA") + + def test_blueprint_must_match_ca(self): + """Test that blueprint_cert must match the selected CA.""" + org = self._get_org() + ca_main = self._create_ca( + name="Main CA", common_name="Main CA", organization=org + ) + ca_other = self._create_ca( + name="Other CA", common_name="Other CA", organization=org + ) + blueprint = self._create_cert( + name="Master Blueprint", ca=ca_main, organization=org + ) + try: + self._create_template( + type="cert", + ca=ca_other, + blueprint_cert=blueprint, + organization=org, + config={}, + ) + except ValidationError as err: + self.assertIn("blueprint_cert", err.message_dict) + self.assertIn("match", str(err.message_dict["blueprint_cert"][0])) + else: + self.fail("ValidationError not raised for CA mismatch") + + def test_blueprint_cannot_be_already_assigned(self): + """Test that blueprint_cert cannot be already assigned to a device.""" + org = self._get_org() + ca = self._create_ca(organization=org) + blueprint = self._create_cert(ca=ca, organization=org) + mock_manager = mock.Mock() + mock_manager.exists.return_value = True + blueprint.devicecertificate_set = mock_manager + try: + self._create_template( + type="cert", + ca=ca, + blueprint_cert=blueprint, + organization=org, + config={}, + ) + except ValidationError as err: + self.assertIn("blueprint_cert", err.message_dict) + self.assertIn( + "already assigned", str(err.message_dict["blueprint_cert"][0]) + ) + else: + self.fail("ValidationError not raised for assigned blueprint") + finally: + del blueprint.devicecertificate_set + + def test_non_cert_clears_fields(self): + """Test that non-cert template types clear ca and blueprint_cert fields.""" + org = self._get_org() + ca = self._create_ca(organization=org) + blueprint = self._create_cert(ca=ca, organization=org) + t = self._create_template( + type="cert", + ca=ca, + blueprint_cert=blueprint, + organization=org, + config={}, + ) + t.type = "generic" + t.backend = "netjsonconfig.OpenWrt" + t.config = {"interfaces": [{"name": "eth0", "type": "ethernet"}]} + t.full_clean() + self.assertIsNone(t.ca) + self.assertIsNone(t.blueprint_cert) + + def test_cert_template_allows_empty_config(self): + """Test that cert templates can have empty config (unlike other types).""" + ca = self._create_ca() + t = self._create_template( + type="cert", + ca=ca, + config={}, + ) + self.assertEqual(t.config, {}) + + def test_organization_validation_for_relations(self): + """Test organization validation for ca field.""" + org1 = self._get_org() + org2 = self._create_org(name="Org2", slug="org2") + ca_org2 = self._create_ca(organization=org2) + + try: + self._create_template( + type="cert", + ca=ca_org2, + organization=org1, + config={}, + ) + except ValidationError as err: + self.assertIn("organization", err.message_dict) + self.assertIn("related CA match", str(err.message_dict["organization"][0])) + else: + self.fail("ValidationError not raised for cross-organization CA relation") diff --git a/openwisp_controller/pki/tests/test_api.py b/openwisp_controller/pki/tests/test_api.py index 5d4c84e3b..93145bb92 100644 --- a/openwisp_controller/pki/tests/test_api.py +++ b/openwisp_controller/pki/tests/test_api.py @@ -152,7 +152,7 @@ def test_crl_download_api(self): def test_ca_delete_api(self): ca1 = self._create_ca(name="ca1", organization=self._get_org()) path = reverse("pki_api:ca_detail", args=[ca1.pk]) - with self.assertNumQueries(5): + with self.assertNumQueries(6): r = self.client.delete(path) self.assertEqual(r.status_code, 204) self.assertEqual(Ca.objects.count(), 0) @@ -272,7 +272,7 @@ def test_cert_patch_api(self): def test_cert_delete_api(self): cert1 = self._create_cert(name="cert1") path = reverse("pki_api:cert_detail", args=[cert1.pk]) - with self.assertNumQueries(6): + with self.assertNumQueries(7): r = self.client.delete(path) self.assertEqual(r.status_code, 204) self.assertEqual(Cert.objects.count(), 0) From b946d266dd547045f678d9e130ca766356545836 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Wed, 27 May 2026 02:12:42 +0530 Subject: [PATCH 03/34] [fix] Fixed auto_cert help text Fixed help text for auto cert and updated migration files --- openwisp_controller/config/base/template.py | 3 ++- ...ate_blueprint_cert_template_ca_and_more.py | 22 +++++++++++++--- ...ate_blueprint_cert_template_ca_and_more.py | 26 +++++++++++++++---- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index 0608b62bb..28dfd3e6a 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -116,7 +116,8 @@ class AbstractTemplate(ShareableOrgMixinUniqueName, BaseConfig): help_text=_( "whether tunnel specific configuration (cryptographic keys, ip addresses, " "etc) should be automatically generated and managed behind the scenes " - "for each configuration using this template, valid only for the VPN type" + "for each configuration using this template, valid only for the VPN and " + "certificate template types" ), ) default_values = JSONField( diff --git a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py index 856dd881d..ca66bf9da 100644 --- a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py +++ b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py @@ -1,9 +1,11 @@ -# Generated by Django 5.2.14 on 2026-05-26 19:18 +# Generated by Django 5.2.14 on 2026-05-26 20:33 import django.db.models.deletion from django.conf import settings from django.db import migrations, models +import openwisp_controller.config.base.template + class Migration(migrations.Migration): @@ -19,8 +21,8 @@ class Migration(migrations.Migration): name="blueprint_cert", field=models.ForeignKey( blank=True, - help_text="Optional: Select an unassigned certificate to " - "copy extensions and properties from.", + help_text="Optional: Select an unassigned certificate " + "to copy extensions and properties from.", null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.DJANGO_X509_CERT_MODEL, @@ -40,6 +42,20 @@ class Migration(migrations.Migration): verbose_name="Certificate Authority", ), ), + migrations.AlterField( + model_name="template", + name="auto_cert", + field=models.BooleanField( + db_index=True, + default=openwisp_controller.config.base.template.default_auto_cert, + help_text="whether tunnel specific configuration " + "(cryptographic keys, ip addresses, etc) should be " + "automatically generated and managed behind the scenes for " + "each configuration using this template, valid only for the " + "VPN and certificate template types", + verbose_name="automatic tunnel provisioning", + ), + ), migrations.AlterField( model_name="template", name="type", diff --git a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py index ad9d3af7f..c0d684f1c 100644 --- a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py +++ b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py @@ -1,8 +1,10 @@ -# Generated by Django 5.2.14 on 2026-05-26 19:52 +# Generated by Django 5.2.14 on 2026-05-26 20:33 import django.db.models.deletion from django.db import migrations, models +import openwisp_controller.config.base.template + class Migration(migrations.Migration): @@ -17,8 +19,8 @@ class Migration(migrations.Migration): name="blueprint_cert", field=models.ForeignKey( blank=True, - help_text="Optional: Select an unassigned certificate to copy " - "extensions and properties from.", + help_text="Optional: Select an unassigned certificate " + "to copy extensions and properties from.", null=True, on_delete=django.db.models.deletion.SET_NULL, to="sample_pki.cert", @@ -30,14 +32,28 @@ class Migration(migrations.Migration): name="ca", field=models.ForeignKey( blank=True, - help_text="The Certificate Authority that will sign certificates " - "generated by this template.", + help_text="The Certificate Authority that will sign " + "certificates generated by this template.", null=True, on_delete=django.db.models.deletion.CASCADE, to="sample_pki.ca", verbose_name="Certificate Authority", ), ), + migrations.AlterField( + model_name="template", + name="auto_cert", + field=models.BooleanField( + db_index=True, + default=openwisp_controller.config.base.template.default_auto_cert, + help_text="whether tunnel specific configuration " + "(cryptographic keys, ip addresses, etc) should be " + "automatically generated and managed behind the scenes for " + "each configuration using this template, valid only for the " + "VPN and certificate template types", + verbose_name="automatic tunnel provisioning", + ), + ), migrations.AlterField( model_name="template", name="type", From 0053bcfa87f7c1bc29d7e545b39cab84760cab2d Mon Sep 17 00:00:00 2001 From: stktyagi Date: Wed, 27 May 2026 09:39:46 +0530 Subject: [PATCH 04/34] [fix] Validate cert relations only inside the cert branch #1356 Validate cert relations only inside the cert branch and Only coerce missing cert configs, not every falsy value. Fixes #1356 --- openwisp_controller/config/base/template.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index 28dfd3e6a..e7f23167b 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -249,8 +249,6 @@ def clean(self, *args, **kwargs): * if flagged as required forces it also to be default """ self._validate_org_relation("vpn") - self._validate_org_relation("ca") - self._validate_org_relation("blueprint_cert") if not self.default_values: self.default_values = {} if not isinstance(self.default_values, dict): @@ -270,6 +268,8 @@ def clean(self, *args, **kwargs): auto_cert=self.auto_cert, template_backend_class=self.backend_class ) if self.type == "cert": + self._validate_org_relation("ca") + self._validate_org_relation("blueprint_cert") if not self.ca: raise ValidationError( { @@ -301,7 +301,7 @@ def clean(self, *args, **kwargs): ) } ) - if not self.config: + if self.config is None: self.config = {} else: self.ca = None From 01221d990ff03c292460c2c0ce5bb27731ed1f40 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Wed, 27 May 2026 09:56:39 +0530 Subject: [PATCH 05/34] [fix] Added test for cert validation path #1356 Added test for the validation branch that now skips ca / blueprint_cert checks for non-cert templates Fixes #1356 --- .../config/tests/test_template.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index ec1faf8f6..e1722de5d 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1083,3 +1083,32 @@ def test_organization_validation_for_relations(self): self.assertIn("related CA match", str(err.message_dict["organization"][0])) else: self.fail("ValidationError not raised for cross-organization CA relation") + + def test_organization_validation_skipped_for_non_cert(self): + """ + Organization validation for 'ca' and 'blueprint_cert' + should not run if the template is not type 'cert'. + """ + org1 = self._get_org() + org2 = self._create_org(name="Org2", slug="org2") + ca_org2 = self._create_ca(organization=org2) + blueprint_org2 = self._create_cert(ca=ca_org2, organization=org2) + t = self._create_template( + name="stale-relations", + type="generic", + backend="netjsonconfig.OpenWrt", + ca=ca_org2, + blueprint_cert=blueprint_org2, + organization=org1, + config={"interfaces": [{"name": "eth0", "type": "ethernet"}]}, + ) + try: + t.full_clean() + except ValidationError as err: + if "organization" in err.message_dict: + self.fail( + "Organization validation ran on stale cert relations for a non-cert template." + ) + raise err + self.assertIsNone(t.ca) + self.assertIsNone(t.blueprint_cert) From 5c551c7878ff056b4a66ae16164f1197c6596cd8 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Wed, 27 May 2026 10:12:48 +0530 Subject: [PATCH 06/34] [fix] Fixed flake error #1356 Fixed line too long flake error Fixes #1356 --- openwisp_controller/config/tests/test_template.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index e1722de5d..1c36f49e0 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1107,7 +1107,8 @@ def test_organization_validation_skipped_for_non_cert(self): except ValidationError as err: if "organization" in err.message_dict: self.fail( - "Organization validation ran on stale cert relations for a non-cert template." + "Organization validation ran on stale cert relations " + "for a non-cert template." ) raise err self.assertIsNone(t.ca) From d6fd19385a0e0151ca781e5f9c89a189ee6335b3 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Thu, 28 May 2026 20:40:48 +0530 Subject: [PATCH 07/34] [feature] Add DeviceCertificate M2M model and cert template support #1377 Implemented the DeviceCertificate M2M through-model to act as a strict relational bridge between Config, Template, and django_x509.Cert. Fixes #1377 --- openwisp_controller/config/api/serializers.py | 5 +- openwisp_controller/config/base/config.py | 8 +++ .../config/base/device_certificate.py | 41 +++++++++++ openwisp_controller/config/base/template.py | 22 +++--- ...ate_blueprint_cert_template_ca_and_more.py | 1 + ...ecertificate_config_device_certificates.py | 70 +++++++++++++++++++ openwisp_controller/config/models.py | 11 +++ .../config/tests/test_template.py | 22 ++++-- ...ate_blueprint_cert_template_ca_and_more.py | 1 + ...ecertificate_config_device_certificates.py | 69 ++++++++++++++++++ tests/openwisp2/sample_config/models.py | 10 +++ tests/openwisp2/settings.py | 1 + 12 files changed, 243 insertions(+), 18 deletions(-) create mode 100644 openwisp_controller/config/base/device_certificate.py create mode 100644 openwisp_controller/config/migrations/0065_devicecertificate_config_device_certificates.py create mode 100644 tests/openwisp2/sample_config/migrations/0011_devicecertificate_config_device_certificates.py diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index f07627e60..7fd64d83a 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -38,6 +38,8 @@ class Meta(BaseMeta): "type", "backend", "vpn", + "ca", + "blueprint_cert", "tags", "default", "required", @@ -62,7 +64,8 @@ def validate_config(self, value): """ Display appropriate field name. """ - if self.initial_data.get("type") == "generic" and value == {}: + template_type = self.initial_data.get("type") + if template_type == "generic" and value == {}: raise serializers.ValidationError( _("The configuration field cannot be empty.") ) diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index 855d94541..5519d0d59 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -3,6 +3,7 @@ import re from collections import defaultdict +import swapper from cache_memoize import cache_memoize from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError @@ -63,6 +64,13 @@ class AbstractConfig(ChecksumCacheMixin, BaseConfig): related_name="vpn_relations", blank=True, ) + device_certificates = models.ManyToManyField( + swapper.get_model_name("config", "Template"), + through=swapper.get_model_name("config", "DeviceCertificate"), + related_name="config_device_certificates", + blank=True, + verbose_name=_("device certificates"), + ) STATUS = Choices("modified", "applied", "error", "deactivating", "deactivated") status = StatusField( diff --git a/openwisp_controller/config/base/device_certificate.py b/openwisp_controller/config/base/device_certificate.py new file mode 100644 index 000000000..939ee87a9 --- /dev/null +++ b/openwisp_controller/config/base/device_certificate.py @@ -0,0 +1,41 @@ +from django.core.exceptions import ValidationError +from django.db import models +from django.utils.translation import gettext_lazy as _ +from swapper import get_model_name, load_model + + +class AbstractDeviceCertificate(models.Model): + config = models.ForeignKey( + get_model_name("config", "Config"), on_delete=models.CASCADE + ) + template = models.ForeignKey( + get_model_name("config", "Template"), on_delete=models.CASCADE + ) + cert = models.OneToOneField( + get_model_name("django_x509", "Cert"), on_delete=models.CASCADE + ) + auto_cert = models.BooleanField(default=False) + + class Meta: + abstract = True + unique_together = ("config", "template") + verbose_name = _("Device certificate") + verbose_name_plural = _("Device certificates") + + def __str__(self): + return f"{self.config.device.name} - {self.cert.name}" + + def clean(self): + Template = load_model("config", "Template") + if ( + self.cert_id + and Template.objects.filter(blueprint_cert_id=self.cert_id).exists() + ): + raise ValidationError( + { + "cert": _( + "This certificate is currently used as a blueprint " + "by a template and cannot be directly assigned to a device." + ) + } + ) diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index e7f23167b..6c3dec2ad 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -76,6 +76,7 @@ class AbstractTemplate(ShareableOrgMixinUniqueName, BaseConfig): verbose_name=_("Blueprint Certificate"), blank=True, null=True, + limit_choices_to={"devicecertificate__isnull": True}, help_text=_( "Optional: Select an unassigned certificate to copy extensions and " "properties from." @@ -289,18 +290,17 @@ def clean(self, *args, **kwargs): } ) if self.blueprint_cert and hasattr( - self.blueprint_cert, "devicecertificate_set" + self.blueprint_cert, "devicecertificate" ): - if self.blueprint_cert.devicecertificate_set.exists(): - raise ValidationError( - { - "blueprint_cert": _( - "This certificate is already assigned to a device. " - "Please select an unassigned certificate to use as a " - "blueprint." - ) - } - ) + raise ValidationError( + { + "blueprint_cert": _( + "This certificate is already assigned to a device. " + "Please select an unassigned certificate to use as a " + "blueprint." + ) + } + ) if self.config is None: self.config = {} else: diff --git a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py index ca66bf9da..daf260b23 100644 --- a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py +++ b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py @@ -23,6 +23,7 @@ class Migration(migrations.Migration): blank=True, help_text="Optional: Select an unassigned certificate " "to copy extensions and properties from.", + limit_choices_to={"devicecertificate__isnull": True}, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.DJANGO_X509_CERT_MODEL, diff --git a/openwisp_controller/config/migrations/0065_devicecertificate_config_device_certificates.py b/openwisp_controller/config/migrations/0065_devicecertificate_config_device_certificates.py new file mode 100644 index 000000000..8074e733b --- /dev/null +++ b/openwisp_controller/config/migrations/0065_devicecertificate_config_device_certificates.py @@ -0,0 +1,70 @@ +# Generated by Django 5.2.14 on 2026-05-27 08:39 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("config", "0064_template_blueprint_cert_template_ca_and_more"), + migrations.swappable_dependency(settings.DJANGO_X509_CERT_MODEL), + ] + + operations = [ + migrations.CreateModel( + name="DeviceCertificate", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("auto_cert", models.BooleanField(default=False)), + ( + "cert", + models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + to=settings.DJANGO_X509_CERT_MODEL, + ), + ), + ( + "config", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to=settings.CONFIG_CONFIG_MODEL, + ), + ), + ( + "template", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to=settings.CONFIG_TEMPLATE_MODEL, + ), + ), + ], + options={ + "verbose_name": "Device certificate", + "verbose_name_plural": "Device certificates", + "abstract": False, + "swappable": "CONFIG_DEVICECERTIFICATE_MODEL", + "unique_together": {("config", "template")}, + }, + ), + migrations.AddField( + model_name="config", + name="device_certificates", + field=models.ManyToManyField( + blank=True, + related_name="config_device_certificates", + through="config.DeviceCertificate", + to=settings.CONFIG_TEMPLATE_MODEL, + verbose_name="device certificates", + ), + ), + ] diff --git a/openwisp_controller/config/models.py b/openwisp_controller/config/models.py index e36280319..8fcae7b1c 100644 --- a/openwisp_controller/config/models.py +++ b/openwisp_controller/config/models.py @@ -2,6 +2,7 @@ from .base.config import AbstractConfig from .base.device import AbstractDevice +from .base.device_certificate import AbstractDeviceCertificate from .base.device_group import AbstractDeviceGroup from .base.multitenancy import ( AbstractOrganizationConfigSettings, @@ -93,6 +94,16 @@ class Meta(AbstractVpnClient.Meta): swappable = swapper.swappable_setting("config", "VpnClient") +class DeviceCertificate(AbstractDeviceCertificate): + """ + m2m through model + """ + + class Meta(AbstractDeviceCertificate.Meta): + abstract = False + swappable = swapper.swappable_setting("config", "DeviceCertificate") + + class OrganizationConfigSettings(AbstractOrganizationConfigSettings): """ Configuration management settings diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index 1c36f49e0..759cab068 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1015,9 +1015,21 @@ def test_blueprint_cannot_be_already_assigned(self): org = self._get_org() ca = self._create_ca(organization=org) blueprint = self._create_cert(ca=ca, organization=org) - mock_manager = mock.Mock() - mock_manager.exists.return_value = True - blueprint.devicecertificate_set = mock_manager + config = self._create_config(organization=org) + template = self._create_template( + name="cert-template", + type="cert", + ca=ca, + organization=org, + config={}, + ) + DeviceCertificate = load_model("config", "DeviceCertificate") + DeviceCertificate.objects.create( + config=config, + template=template, + cert=blueprint, + auto_cert=True, + ) try: self._create_template( type="cert", @@ -1029,12 +1041,10 @@ def test_blueprint_cannot_be_already_assigned(self): except ValidationError as err: self.assertIn("blueprint_cert", err.message_dict) self.assertIn( - "already assigned", str(err.message_dict["blueprint_cert"][0]) + "already assigned", str(err.message_dict["blueprint_cert"]) ) else: self.fail("ValidationError not raised for assigned blueprint") - finally: - del blueprint.devicecertificate_set def test_non_cert_clears_fields(self): """Test that non-cert template types clear ca and blueprint_cert fields.""" diff --git a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py index c0d684f1c..a186ae9bb 100644 --- a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py +++ b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py @@ -21,6 +21,7 @@ class Migration(migrations.Migration): blank=True, help_text="Optional: Select an unassigned certificate " "to copy extensions and properties from.", + limit_choices_to={"devicecertificate__isnull": True}, null=True, on_delete=django.db.models.deletion.SET_NULL, to="sample_pki.cert", diff --git a/tests/openwisp2/sample_config/migrations/0011_devicecertificate_config_device_certificates.py b/tests/openwisp2/sample_config/migrations/0011_devicecertificate_config_device_certificates.py new file mode 100644 index 000000000..3b7c7a5c6 --- /dev/null +++ b/tests/openwisp2/sample_config/migrations/0011_devicecertificate_config_device_certificates.py @@ -0,0 +1,69 @@ +# Generated by Django 5.2.14 on 2026-05-27 08:42 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("sample_config", "0010_template_blueprint_cert_template_ca_and_more"), + ("sample_pki", "0004_alter_ca_extensions_alter_ca_key_length_and_more"), + ] + + operations = [ + migrations.CreateModel( + name="DeviceCertificate", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("auto_cert", models.BooleanField(default=False)), + ("details", models.CharField(blank=True, max_length=64, null=True)), + ( + "cert", + models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + to="sample_pki.cert", + ), + ), + ( + "config", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="sample_config.config", + ), + ), + ( + "template", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="sample_config.template", + ), + ), + ], + options={ + "verbose_name": "Device certificate", + "verbose_name_plural": "Device certificates", + "abstract": False, + "unique_together": {("config", "template")}, + }, + ), + migrations.AddField( + model_name="config", + name="device_certificates", + field=models.ManyToManyField( + blank=True, + related_name="config_device_certificates", + through="sample_config.DeviceCertificate", + to="sample_config.template", + verbose_name="device certificates", + ), + ), + ] diff --git a/tests/openwisp2/sample_config/models.py b/tests/openwisp2/sample_config/models.py index e0e6448a5..50d401bbc 100644 --- a/tests/openwisp2/sample_config/models.py +++ b/tests/openwisp2/sample_config/models.py @@ -2,6 +2,7 @@ from openwisp_controller.config.base.config import AbstractConfig from openwisp_controller.config.base.device import AbstractDevice +from openwisp_controller.config.base.device_certificate import AbstractDeviceCertificate from openwisp_controller.config.base.device_group import AbstractDeviceGroup from openwisp_controller.config.base.multitenancy import ( AbstractOrganizationConfigSettings, @@ -95,6 +96,15 @@ class Meta(AbstractVpnClient.Meta): abstract = False +class DeviceCertificate(DetailsModel, AbstractDeviceCertificate): + """ + m2m through model + """ + + class Meta(AbstractDeviceCertificate.Meta): + abstract = False + + class OrganizationConfigSettings(DetailsModel, AbstractOrganizationConfigSettings): """ Configuration management settings diff --git a/tests/openwisp2/settings.py b/tests/openwisp2/settings.py index 64cf854af..010190bb3 100644 --- a/tests/openwisp2/settings.py +++ b/tests/openwisp2/settings.py @@ -272,6 +272,7 @@ CONFIG_TEMPLATE_MODEL = "sample_config.Template" CONFIG_VPN_MODEL = "sample_config.Vpn" CONFIG_VPNCLIENT_MODEL = "sample_config.VpnClient" + CONFIG_DEVICECERTIFICATE_MODEL = "sample_config.DeviceCertificate" CONFIG_ORGANIZATIONCONFIGSETTINGS_MODEL = "sample_config.OrganizationConfigSettings" CONFIG_ORGANIZATIONLIMITS_MODEL = "sample_config.OrganizationLimits" CONFIG_WHOISINFO_MODEL = "sample_config.WHOISInfo" From cae38a511830d4934d75958ad37990171a172253 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Thu, 28 May 2026 21:00:24 +0530 Subject: [PATCH 08/34] [fix] Updated test for double checks #1377 Updated test by joining the list of strings into one sentence. Fixes #1377 --- openwisp_controller/config/tests/test_template.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index 759cab068..f537c57dc 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1040,9 +1040,8 @@ def test_blueprint_cannot_be_already_assigned(self): ) except ValidationError as err: self.assertIn("blueprint_cert", err.message_dict) - self.assertIn( - "already assigned", str(err.message_dict["blueprint_cert"]) - ) + error_messages = " ".join(err.message_dict["blueprint_cert"]) + self.assertIn("already assigned", error_messages) else: self.fail("ValidationError not raised for assigned blueprint") From 02912a00bfdbd02a9ec5bbfb6c774695a9e14e98 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Fri, 29 May 2026 18:39:57 +0530 Subject: [PATCH 09/34] [feature] Updated django admin and API #1357 #1361 Updated django admin and REST API serializer for certificate templates Fixes #1357 #1361 --- openwisp_controller/config/admin.py | 15 +++- openwisp_controller/config/api/serializers.py | 74 +++++++++++++++++++ openwisp_controller/config/base/template.py | 53 +++++++++---- ...ate_blueprint_cert_template_ca_and_more.py | 58 ++++++++++++++- ...ecertificate_config_device_certificates.py | 70 ------------------ .../config/static/config/js/template_ui.js | 17 +++++ openwisp_controller/config/tests/test_api.py | 2 +- ...ate_blueprint_cert_template_ca_and_more.py | 58 ++++++++++++++- ...ecertificate_config_device_certificates.py | 69 ----------------- 9 files changed, 256 insertions(+), 160 deletions(-) delete mode 100644 openwisp_controller/config/migrations/0065_devicecertificate_config_device_certificates.py create mode 100644 openwisp_controller/config/static/config/js/template_ui.js delete mode 100644 tests/openwisp2/sample_config/migrations/0011_devicecertificate_config_device_certificates.py diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index df1ffabba..b3a37af1f 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -117,7 +117,12 @@ class Media: css = {"all": (f"{prefix}css/admin.css",)} js = list(CopyableFieldsAdmin.Media.js) + [ f"{prefix}js/{file_}" - for file_ in ("preview.js", "unsaved_changes.js", "switcher.js") + for file_ in ( + "preview.js", + "unsaved_changes.js", + "switcher.js", + "template_ui.js", + ) ] def get_extra_context(self, pk=None): @@ -1059,6 +1064,8 @@ class TemplateAdmin(MultitenantAdminMixin, BaseConfigAdmin, SystemDefinedVariabl "organization", "type", "backend", + "ca", + "blueprint_cert", "default", "required", "created", @@ -1073,13 +1080,15 @@ class TemplateAdmin(MultitenantAdminMixin, BaseConfigAdmin, SystemDefinedVariabl "created", ] search_fields = ["name"] - multitenant_shared_relations = ("vpn",) + multitenant_shared_relations = ("vpn", "ca", "blueprint_cert") fields = [ "name", "organization", "type", "backend", "vpn", + "ca", + "blueprint_cert", "auto_cert", "tags", "default", @@ -1091,7 +1100,7 @@ class TemplateAdmin(MultitenantAdminMixin, BaseConfigAdmin, SystemDefinedVariabl "modified", ] readonly_fields = ["system_context"] - autocomplete_fields = ["vpn"] + autocomplete_fields = ["vpn", "ca", "blueprint_cert"] @admin.action(permissions=["add"]) def clone_selected_templates(self, request, queryset): diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index 7fd64d83a..6936e0d51 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -17,6 +17,7 @@ DeviceGroup = load_model("config", "DeviceGroup") Config = load_model("config", "Config") Organization = load_model("openwisp_users", "Organization") +DeviceCertificate = load_model("config", "DeviceCertificate") class BaseMeta: @@ -48,6 +49,16 @@ class Meta(BaseMeta): "created", "modified", ] + extra_kwargs = { + "blueprint_cert": { + "error_messages": { + "does_not_exist": _( + "This certificate does not exist or is already " + "assigned to an active device." + ) + } + } + } def validate_vpn(self, value): """ @@ -71,6 +82,66 @@ def validate_config(self, value): ) return value + def validate(self, data): + """ + Explicitly validate certificate template fields and locks for the API. + """ + template_type = data.get("type", getattr(self.instance, "type", "generic")) + ca = data.get("ca", getattr(self.instance, "ca", None)) + blueprint_cert = data.get( + "blueprint_cert", getattr(self.instance, "blueprint_cert", None) + ) + if template_type == "cert" and not ca: + raise serializers.ValidationError( + { + "ca": _( + "A Certificate Authority is required when " + "the template type is certificate." + ) + } + ) + elif template_type != "cert": + data["ca"] = None + data["blueprint_cert"] = None + if blueprint_cert and ca: + if blueprint_cert.ca_id != ca.id: + raise serializers.ValidationError( + { + "blueprint_cert": _( + "The selected certificate must match " + "the selected Certificate Authority." + ) + } + ) + if self.instance and self.instance.pk: + if Config.objects.filter(templates=self.instance).exists(): + + if "ca" in data and data["ca"] != getattr( + self.instance, "ca_id", self.instance.ca + ): + raise serializers.ValidationError( + { + "ca": _( + "This template is already assigned to active devices. " + "You cannot change the CA or Blueprint Certificate " + "on an active template." + ) + } + ) + if "blueprint_cert" in data and data["blueprint_cert"] != getattr( + self.instance, "blueprint_cert_id", self.instance.blueprint_cert + ): + raise serializers.ValidationError( + { + "blueprint_cert": _( + "This template is already assigned to active devices. " + "You cannot change the CA or Blueprint Certificate " + "on an active template." + ) + } + ) + return super().validate(data) + class VpnSerializer(BaseSerializer): config = serializers.JSONField(initial={}) @@ -206,6 +277,9 @@ def _update_config(self, device, config_data): vpn_list = config.templates.filter(type="vpn").values_list("vpn") if vpn_list: config.vpnclient_set.exclude(vpn__in=vpn_list).delete() + DeviceCertificate.objects.filter(config=config).exclude( + template_id__in=config_templates + ).delete() config.templates.set(config_templates, clear=True) config.save() except ValidationError as error: diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index 6c3dec2ad..55bfede6d 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -70,13 +70,18 @@ class AbstractTemplate(ShareableOrgMixinUniqueName, BaseConfig): "by this template." ), ) + + def get_unassigned_certs(): + Cert = load_model("django_x509", "Cert") + return {"pk__in": Cert.objects.exclude(devicecertificate__isnull=False)} + blueprint_cert = models.ForeignKey( get_model_name("django_x509", "Cert"), on_delete=models.SET_NULL, verbose_name=_("Blueprint Certificate"), blank=True, null=True, - limit_choices_to={"devicecertificate__isnull": True}, + limit_choices_to=get_unassigned_certs, help_text=_( "Optional: Select an unassigned certificate to copy extensions and " "properties from." @@ -249,6 +254,26 @@ def clean(self, *args, **kwargs): * automatically determines configuration if necessary * if flagged as required forces it also to be default """ + if not self._state.adding: + try: + current = self.__class__.objects.get(pk=self.pk) + if ( + current.ca_id != self.ca_id + or current.blueprint_cert_id != self.blueprint_cert_id + ): + Config = load_model("config", "Config") + if Config.objects.filter(templates=self).exists(): + raise ValidationError( + { + "ca": _( + "This template is already assigned " + "to active devices. You cannot change the CA " + "or Blueprint Certificate on an active template." + ) + } + ) + except self.__class__.DoesNotExist: + pass self._validate_org_relation("vpn") if not self.default_values: self.default_values = {} @@ -289,18 +314,20 @@ def clean(self, *args, **kwargs): ) } ) - if self.blueprint_cert and hasattr( - self.blueprint_cert, "devicecertificate" - ): - raise ValidationError( - { - "blueprint_cert": _( - "This certificate is already assigned to a device. " - "Please select an unassigned certificate to use as a " - "blueprint." - ) - } - ) + if self.blueprint_cert_id: + DeviceCertificate = load_model("config", "DeviceCertificate") + if DeviceCertificate.objects.filter( + cert_id=self.blueprint_cert_id + ).exists(): + raise ValidationError( + { + "blueprint_cert": _( + "This certificate is already assigned to a device. " + "Please select an unassigned certificate to " + "use as a blueprint." + ) + } + ) if self.config is None: self.config = {} else: diff --git a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py index daf260b23..1158b92a8 100644 --- a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py +++ b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 5.2.14 on 2026-05-26 20:33 +# Generated by Django 5.2.14 on 2026-05-29 11:11 import django.db.models.deletion from django.conf import settings @@ -23,7 +23,7 @@ class Migration(migrations.Migration): blank=True, help_text="Optional: Select an unassigned certificate " "to copy extensions and properties from.", - limit_choices_to={"devicecertificate__isnull": True}, + limit_choices_to=openwisp_controller.config.base.template.AbstractTemplate.get_unassigned_certs, # noqa null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.DJANGO_X509_CERT_MODEL, @@ -73,4 +73,58 @@ class Migration(migrations.Migration): verbose_name="type", ), ), + migrations.CreateModel( + name="DeviceCertificate", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("auto_cert", models.BooleanField(default=False)), + ( + "cert", + models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + to=settings.DJANGO_X509_CERT_MODEL, + ), + ), + ( + "config", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to=settings.CONFIG_CONFIG_MODEL, + ), + ), + ( + "template", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to=settings.CONFIG_TEMPLATE_MODEL, + ), + ), + ], + options={ + "verbose_name": "Device certificate", + "verbose_name_plural": "Device certificates", + "abstract": False, + "swappable": "CONFIG_DEVICECERTIFICATE_MODEL", + "unique_together": {("config", "template")}, + }, + ), + migrations.AddField( + model_name="config", + name="device_certificates", + field=models.ManyToManyField( + blank=True, + related_name="config_device_certificates", + through="config.DeviceCertificate", + to=settings.CONFIG_TEMPLATE_MODEL, + verbose_name="device certificates", + ), + ), ] diff --git a/openwisp_controller/config/migrations/0065_devicecertificate_config_device_certificates.py b/openwisp_controller/config/migrations/0065_devicecertificate_config_device_certificates.py deleted file mode 100644 index 8074e733b..000000000 --- a/openwisp_controller/config/migrations/0065_devicecertificate_config_device_certificates.py +++ /dev/null @@ -1,70 +0,0 @@ -# Generated by Django 5.2.14 on 2026-05-27 08:39 - -import django.db.models.deletion -from django.conf import settings -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("config", "0064_template_blueprint_cert_template_ca_and_more"), - migrations.swappable_dependency(settings.DJANGO_X509_CERT_MODEL), - ] - - operations = [ - migrations.CreateModel( - name="DeviceCertificate", - fields=[ - ( - "id", - models.AutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ("auto_cert", models.BooleanField(default=False)), - ( - "cert", - models.OneToOneField( - on_delete=django.db.models.deletion.CASCADE, - to=settings.DJANGO_X509_CERT_MODEL, - ), - ), - ( - "config", - models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, - to=settings.CONFIG_CONFIG_MODEL, - ), - ), - ( - "template", - models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, - to=settings.CONFIG_TEMPLATE_MODEL, - ), - ), - ], - options={ - "verbose_name": "Device certificate", - "verbose_name_plural": "Device certificates", - "abstract": False, - "swappable": "CONFIG_DEVICECERTIFICATE_MODEL", - "unique_together": {("config", "template")}, - }, - ), - migrations.AddField( - model_name="config", - name="device_certificates", - field=models.ManyToManyField( - blank=True, - related_name="config_device_certificates", - through="config.DeviceCertificate", - to=settings.CONFIG_TEMPLATE_MODEL, - verbose_name="device certificates", - ), - ), - ] diff --git a/openwisp_controller/config/static/config/js/template_ui.js b/openwisp_controller/config/static/config/js/template_ui.js new file mode 100644 index 000000000..f340e87d5 --- /dev/null +++ b/openwisp_controller/config/static/config/js/template_ui.js @@ -0,0 +1,17 @@ +django.jQuery(function ($) { + var typeField = $("#id_type"), + caField = $(".field-ca"), + blueprintField = $(".field-blueprint_cert"); + + function toggleCertFields() { + if (typeField.val() === "cert") { + caField.show(); + blueprintField.show(); + } else { + caField.hide(); + blueprintField.hide(); + } + } + typeField.on("change", toggleCertFields); + toggleCertFields(); +}); diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 6fcf9c527..b8b7bf94f 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -738,7 +738,7 @@ def _assert_template_list_filter(response=None, template=None): self.assertEqual(response.status_code, 200) data = response.data self.assertEqual(data["count"], 1) - self.assertEqual(len(data["results"][0]), 13) + self.assertEqual(len(data["results"][0]), 15) self.assertEqual(data["results"][0]["id"], str(template.pk)) self.assertEqual(data["results"][0]["name"], str(template.name)) self.assertEqual( diff --git a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py index a186ae9bb..c2925cd0b 100644 --- a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py +++ b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 5.2.14 on 2026-05-26 20:33 +# Generated by Django 5.2.14 on 2026-05-29 11:14 import django.db.models.deletion from django.db import migrations, models @@ -21,7 +21,7 @@ class Migration(migrations.Migration): blank=True, help_text="Optional: Select an unassigned certificate " "to copy extensions and properties from.", - limit_choices_to={"devicecertificate__isnull": True}, + limit_choices_to=openwisp_controller.config.base.template.AbstractTemplate.get_unassigned_certs, # noqa null=True, on_delete=django.db.models.deletion.SET_NULL, to="sample_pki.cert", @@ -71,4 +71,58 @@ class Migration(migrations.Migration): verbose_name="type", ), ), + migrations.CreateModel( + name="DeviceCertificate", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("auto_cert", models.BooleanField(default=False)), + ("details", models.CharField(blank=True, max_length=64, null=True)), + ( + "cert", + models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + to="sample_pki.cert", + ), + ), + ( + "config", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="sample_config.config", + ), + ), + ( + "template", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="sample_config.template", + ), + ), + ], + options={ + "verbose_name": "Device certificate", + "verbose_name_plural": "Device certificates", + "abstract": False, + "unique_together": {("config", "template")}, + }, + ), + migrations.AddField( + model_name="config", + name="device_certificates", + field=models.ManyToManyField( + blank=True, + related_name="config_device_certificates", + through="sample_config.DeviceCertificate", + to="sample_config.template", + verbose_name="device certificates", + ), + ), ] diff --git a/tests/openwisp2/sample_config/migrations/0011_devicecertificate_config_device_certificates.py b/tests/openwisp2/sample_config/migrations/0011_devicecertificate_config_device_certificates.py deleted file mode 100644 index 3b7c7a5c6..000000000 --- a/tests/openwisp2/sample_config/migrations/0011_devicecertificate_config_device_certificates.py +++ /dev/null @@ -1,69 +0,0 @@ -# Generated by Django 5.2.14 on 2026-05-27 08:42 - -import django.db.models.deletion -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("sample_config", "0010_template_blueprint_cert_template_ca_and_more"), - ("sample_pki", "0004_alter_ca_extensions_alter_ca_key_length_and_more"), - ] - - operations = [ - migrations.CreateModel( - name="DeviceCertificate", - fields=[ - ( - "id", - models.AutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ("auto_cert", models.BooleanField(default=False)), - ("details", models.CharField(blank=True, max_length=64, null=True)), - ( - "cert", - models.OneToOneField( - on_delete=django.db.models.deletion.CASCADE, - to="sample_pki.cert", - ), - ), - ( - "config", - models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, - to="sample_config.config", - ), - ), - ( - "template", - models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, - to="sample_config.template", - ), - ), - ], - options={ - "verbose_name": "Device certificate", - "verbose_name_plural": "Device certificates", - "abstract": False, - "unique_together": {("config", "template")}, - }, - ), - migrations.AddField( - model_name="config", - name="device_certificates", - field=models.ManyToManyField( - blank=True, - related_name="config_device_certificates", - through="sample_config.DeviceCertificate", - to="sample_config.template", - verbose_name="device certificates", - ), - ), - ] From 34347193837fd06a4ba9e487512debc7e83a64ba Mon Sep 17 00:00:00 2001 From: stktyagi Date: Fri, 29 May 2026 19:28:48 +0530 Subject: [PATCH 10/34] [fix] Fixed tests and improvedd serializers #1357 Fixed failing tests and added improvements in template and serializers Fixes #1357 --- openwisp_controller/config/api/serializers.py | 10 +++--- openwisp_controller/config/base/template.py | 22 +++++++----- .../config/static/config/js/template_ui.js | 7 ++++ .../config/tests/test_selenium.py | 35 +++++++++++++++++++ openwisp_controller/pki/tests/test_api.py | 2 +- 5 files changed, 60 insertions(+), 16 deletions(-) diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index 6936e0d51..f19ea492c 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -115,10 +115,7 @@ def validate(self, data): ) if self.instance and self.instance.pk: if Config.objects.filter(templates=self.instance).exists(): - - if "ca" in data and data["ca"] != getattr( - self.instance, "ca_id", self.instance.ca - ): + if "ca" in data and data["ca"] != self.instance.ca: raise serializers.ValidationError( { "ca": _( @@ -128,8 +125,9 @@ def validate(self, data): ) } ) - if "blueprint_cert" in data and data["blueprint_cert"] != getattr( - self.instance, "blueprint_cert_id", self.instance.blueprint_cert + if ( + "blueprint_cert" in data + and data["blueprint_cert"] != self.instance.blueprint_cert ): raise serializers.ValidationError( { diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index 55bfede6d..d694b1d5e 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -73,7 +73,9 @@ class AbstractTemplate(ShareableOrgMixinUniqueName, BaseConfig): def get_unassigned_certs(): Cert = load_model("django_x509", "Cert") - return {"pk__in": Cert.objects.exclude(devicecertificate__isnull=False)} + DeviceCertificate = load_model("config", "DeviceCertificate") + assigned_cert_ids = DeviceCertificate.objects.values_list("cert_id", flat=True) + return {"pk__in": Cert.objects.exclude(id__in=assigned_cert_ids)} blueprint_cert = models.ForeignKey( get_model_name("django_x509", "Cert"), @@ -263,15 +265,17 @@ def clean(self, *args, **kwargs): ): Config = load_model("config", "Config") if Config.objects.filter(templates=self).exists(): - raise ValidationError( - { - "ca": _( - "This template is already assigned " - "to active devices. You cannot change the CA " - "or Blueprint Certificate on an active template." - ) - } + message = _( + "This template is already assigned to active devices. " + "You cannot change the CA or Blueprint Certificate " + "on an active template." ) + errors = {} + if current.ca_id != self.ca_id: + errors["ca"] = message + if current.blueprint_cert_id != self.blueprint_cert_id: + errors["blueprint_cert"] = message + raise ValidationError(errors) except self.__class__.DoesNotExist: pass self._validate_org_relation("vpn") diff --git a/openwisp_controller/config/static/config/js/template_ui.js b/openwisp_controller/config/static/config/js/template_ui.js index f340e87d5..c22e9f51b 100644 --- a/openwisp_controller/config/static/config/js/template_ui.js +++ b/openwisp_controller/config/static/config/js/template_ui.js @@ -1,8 +1,15 @@ +// Created to show or hide the ca and blueprint_cert fields +// based on the selected template type. django.jQuery(function ($) { var typeField = $("#id_type"), caField = $(".field-ca"), blueprintField = $(".field-blueprint_cert"); + // Only the Template admin form has the type selector; bail out elsewhere + // (e.g. VpnAdmin) to avoid hiding its unrelated `.field-ca` row. + if (!typeField.length) { + return; + } function toggleCertFields() { if (typeField.val() === "cert") { caField.show(); diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index 4322c00c5..76e53b29a 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -767,3 +767,38 @@ def test_vpn_edit(self): backend.select_by_visible_text("OpenVPN") self.wait_for_invisibility(by=By.CLASS_NAME, value="field-webhook_endpoint") self.wait_for_invisibility(by=By.CLASS_NAME, value="field-auth_token") + + +@tag("selenium_tests") +class TestTemplateAdmin( + SeleniumTestMixin, + CreateConfigTemplateMixin, + TestVpnX509Mixin, + StaticLiveServerTestCase, +): + def test_certificate_fields_visibility(self): + """ + Ensure CA and Blueprint fields are only visible when type is 'cert'. + """ + self.login() + self.open(reverse(f"admin:{self.config_app_label}_template_add")) + + # Wait for the Type dropdown to load + self.wait_for_presence(By.ID, "id_type") + + with self.subTest("CA and Cert fields should be hidden by default (Generic)"): + self.wait_for_invisibility(By.CLASS_NAME, "field-ca") + self.wait_for_invisibility(By.CLASS_NAME, "field-blueprint_cert") + + with self.subTest("Changing type to 'Certificate' should show fields"): + type_select = Select(self.find_element(by=By.ID, value="id_type")) + type_select.select_by_value("cert") + + self.wait_for_visibility(By.CLASS_NAME, "field-ca") + self.wait_for_visibility(By.CLASS_NAME, "field-blueprint_cert") + + with self.subTest("Changing type back to 'Generic' should hide fields"): + type_select.select_by_value("generic") + + self.wait_for_invisibility(By.CLASS_NAME, "field-ca") + self.wait_for_invisibility(By.CLASS_NAME, "field-blueprint_cert") diff --git a/openwisp_controller/pki/tests/test_api.py b/openwisp_controller/pki/tests/test_api.py index 93145bb92..2a6cdf93c 100644 --- a/openwisp_controller/pki/tests/test_api.py +++ b/openwisp_controller/pki/tests/test_api.py @@ -272,7 +272,7 @@ def test_cert_patch_api(self): def test_cert_delete_api(self): cert1 = self._create_cert(name="cert1") path = reverse("pki_api:cert_detail", args=[cert1.pk]) - with self.assertNumQueries(7): + with self.assertNumQueries(8): r = self.client.delete(path) self.assertEqual(r.status_code, 204) self.assertEqual(Cert.objects.count(), 0) From 298572f18c5a8e3f254afc11205e709e83029f9d Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sat, 30 May 2026 01:56:58 +0530 Subject: [PATCH 11/34] [feature] Implemented certificate lifecycle management #1358 - Implement synchronous lifecycle signals for DeviceCertificate: - Post-add: Auto-generate physical Cert (with OID injection). - Post-remove/Pre-clear: Auto-revoke physical Cert. - Centralize certificate lifecycle management in Config model m2m_changed signals. Fixes #1358 --- openwisp_controller/config/apps.py | 11 ++ openwisp_controller/config/base/config.py | 17 +++ .../config/base/device_certificate.py | 120 +++++++++++++++- openwisp_controller/config/base/template.py | 113 ++++++++------- ...ate_blueprint_cert_template_ca_and_more.py | 30 +++- openwisp_controller/config/tests/test_api.py | 133 ++++++++++++++++++ .../config/tests/test_template.py | 2 +- ...ate_blueprint_cert_template_ca_and_more.py | 30 +++- 8 files changed, 395 insertions(+), 61 deletions(-) diff --git a/openwisp_controller/config/apps.py b/openwisp_controller/config/apps.py index 4c7e233f4..0de65ed40 100644 --- a/openwisp_controller/config/apps.py +++ b/openwisp_controller/config/apps.py @@ -69,6 +69,7 @@ def __setmodels__(self): self.org_limits = load_model("config", "OrganizationLimits") self.cert_model = load_model("django_x509", "Cert") self.org_model = load_model("openwisp_users", "Organization") + self.devicecert_model = load_model("config", "DeviceCertificate") def connect_signals(self): """ @@ -94,6 +95,11 @@ def connect_signals(self): sender=self.config_model.templates.through, dispatch_uid="config.manage_vpn_clients", ) + m2m_changed.connect( + self.config_model.manage_device_certs, + sender=self.config_model.templates.through, + dispatch_uid="config.manage_device_certs", + ) m2m_changed.connect( self.config_model.templates_changed, sender=self.config_model.templates.through, @@ -115,6 +121,11 @@ def connect_signals(self): sender=self.vpnclient_model, dispatch_uid="vpnclient.post_delete", ) + post_delete.connect( + self.devicecert_model.post_delete, + sender=self.devicecert_model, + dispatch_uid="devicecert.post_delete", + ) vpn_peers_changed.connect( self.vpn_model.update_vpn_server_configuration, sender=self.vpn_model, diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index 5519d0d59..6dfcdb2e0 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -503,6 +503,23 @@ def register_context_function(cls, func): if func not in cls._config_context_functions: cls._config_context_functions.append(func) + @classmethod + def manage_device_certs(cls, sender, instance, action, pk_set, **kwargs): + """ + Syncs DeviceCertificate objects when templates are added/removed + """ + if action == "post_add": + templates = instance.templates.filter(pk__in=pk_set, type="cert") + for tpl in templates: + instance.devicecertificate_set.get_or_create( + template=tpl, defaults={"auto_cert": tpl.auto_cert} + ) + elif action in ["post_remove", "pre_clear"]: + certs_to_delete = instance.devicecertificate_set.all() + if action == "post_remove": + certs_to_delete = certs_to_delete.filter(template_id__in=pk_set) + certs_to_delete.delete() + def get_default_templates(self): """ retrieves default templates of a Config object diff --git a/openwisp_controller/config/base/device_certificate.py b/openwisp_controller/config/base/device_certificate.py index 939ee87a9..d3349eff4 100644 --- a/openwisp_controller/config/base/device_certificate.py +++ b/openwisp_controller/config/base/device_certificate.py @@ -1,10 +1,14 @@ -from django.core.exceptions import ValidationError +import shortuuid +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db import models from django.utils.translation import gettext_lazy as _ from swapper import get_model_name, load_model +from openwisp_controller.config import settings as app_settings +from openwisp_utils.base import TimeStampedEditableModel -class AbstractDeviceCertificate(models.Model): + +class AbstractDeviceCertificate(TimeStampedEditableModel): config = models.ForeignKey( get_model_name("config", "Config"), on_delete=models.CASCADE ) @@ -12,7 +16,10 @@ class AbstractDeviceCertificate(models.Model): get_model_name("config", "Template"), on_delete=models.CASCADE ) cert = models.OneToOneField( - get_model_name("django_x509", "Cert"), on_delete=models.CASCADE + get_model_name("django_x509", "Cert"), + on_delete=models.CASCADE, + blank=True, + null=True, ) auto_cert = models.BooleanField(default=False) @@ -23,7 +30,8 @@ class Meta: verbose_name_plural = _("Device certificates") def __str__(self): - return f"{self.config.device.name} - {self.cert.name}" + cert_name = self.cert.name if self.cert else "Pending Generation" + return f"{self.config.device.name} - {cert_name}" def clean(self): Template = load_model("config", "Template") @@ -39,3 +47,107 @@ def clean(self): ) } ) + super().clean() + + def save(self, *args, **kwargs): + """Performs automatic provisioning if ``auto_cert`` is True.""" + if self.auto_cert and not self.cert: + self._auto_x509() + super().save(*args, **kwargs) + + def _auto_x509(self): + """ + Automatically creates an x509 certificate. + """ + if self.cert: + return + cn = self._get_common_name() + self._auto_create_cert(name=self.config.device.name, common_name=cn) + + def _get_common_name(self): + """ + Returns a unique common name for a new certificate, mirroring VPN client logic. + """ + d = self.config.device + end = 63 - len(d.mac_address) + d.name = d.name[:end] + unique_slug = shortuuid.ShortUUID().random(length=8) + cn_format = app_settings.COMMON_NAME_FORMAT + if cn_format == "{mac_address}-{name}" and d.name == d.mac_address: + cn_format = "{mac_address}" + common_name = cn_format.format(**d.__dict__)[:55] + common_name = f"{common_name}-{unique_slug}" + return common_name + + def _auto_create_cert(self, name, common_name): + """ + Automatically creates and assigns a client x509 certificate + using Blueprint cloning and custom hardware OID injection. + """ + import copy + + ca = self.template.ca + blueprint = self.template.blueprint_cert + # device = self.config.device + cert_model = self.__class__.cert.field.related_model + + # blueprint property cloning with CA fallback + key_length = blueprint.key_length if blueprint else ca.key_length + digest = blueprint.digest if blueprint else str(ca.digest) + country_code = blueprint.country_code if blueprint else ca.country_code + state = blueprint.state if blueprint else ca.state + city = blueprint.city if blueprint else ca.city + organization_name = ( + blueprint.organization_name if blueprint else ca.organization_name + ) + email = blueprint.email if blueprint else ca.email + + if blueprint and blueprint.extensions: + extensions = copy.deepcopy(blueprint.extensions) + else: + extensions = [{"name": "nsCertType", "value": "client", "critical": False}] + + # inject MAC and UUID as custom OIDs, prerequisite: #228 in django-x509 + # mac_oid = "1.3.6.1.4.1.65901.1" + # uuid_oid = "1.3.6.1.4.1.65901.2" + # extensions.extend([ + # {"name": mac_oid, "value": device.mac_address, "critical": False}, + # {"name": uuid_oid, "value": str(device.id), "critical": False} + # ]) + cert = cert_model( + name=name, + ca=ca, + key_length=key_length, + digest=digest, + country_code=country_code, + state=state, + city=city, + organization_name=organization_name, + email=email, + common_name=common_name, + extensions=extensions, + ) + cert = self._auto_create_cert_extra(cert) + cert.full_clean() + cert.save() + self.cert = cert + return cert + + def _auto_create_cert_extra(self, cert): + """ + Sets the organization on the created client certificate. + """ + cert.organization = self.config.device.organization + return cert + + @classmethod + def post_delete(cls, instance, **kwargs): + """ + Receiver of ``post_delete`` signal. + Automatically revokes the certificate when the template is unassigned. + """ + try: + if instance.cert and instance.auto_cert: + instance.cert.revoke() + except ObjectDoesNotExist: + pass diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index d694b1d5e..437b4632b 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -247,56 +247,40 @@ def _auto_add_to_existing_configs(self): f"config {config.pk}: {e}" ) - def clean(self, *args, **kwargs): + def _validate_cert_template_changes(self): """ - * validates org relationship of VPN if present - * validates default_values field - * ensures VPN is selected if type is VPN - * clears VPN specific fields if type is not VPN - * automatically determines configuration if necessary - * if flagged as required forces it also to be default + Prevents changing the CA or Blueprint Certificate of a certificate template + if it is already assigned to active devices. + """ + if self._state.adding: + return + try: + current = self.__class__.objects.get(pk=self.pk) + except self.__class__.DoesNotExist: + return + if ( + current.ca_id != self.ca_id + or current.blueprint_cert_id != self.blueprint_cert_id + ): + Config = load_model("config", "Config") + if Config.objects.filter(templates=self).exists(): + message = _( + "This template is already assigned to active devices. " + "You cannot change the CA or Blueprint Certificate " + "on an active template." + ) + errors = {} + if current.ca_id != self.ca_id: + errors["ca"] = message + if current.blueprint_cert_id != self.blueprint_cert_id: + errors["blueprint_cert"] = message + raise ValidationError(errors) + + def _clean_cert_template(self): + """ + Validates requirements specific to templates of type 'cert'. + Clears cert-related fields if the type is not 'cert'. """ - if not self._state.adding: - try: - current = self.__class__.objects.get(pk=self.pk) - if ( - current.ca_id != self.ca_id - or current.blueprint_cert_id != self.blueprint_cert_id - ): - Config = load_model("config", "Config") - if Config.objects.filter(templates=self).exists(): - message = _( - "This template is already assigned to active devices. " - "You cannot change the CA or Blueprint Certificate " - "on an active template." - ) - errors = {} - if current.ca_id != self.ca_id: - errors["ca"] = message - if current.blueprint_cert_id != self.blueprint_cert_id: - errors["blueprint_cert"] = message - raise ValidationError(errors) - except self.__class__.DoesNotExist: - pass - self._validate_org_relation("vpn") - if not self.default_values: - self.default_values = {} - if not isinstance(self.default_values, dict): - raise ValidationError( - {"default_values": _("the supplied value is not a JSON object")} - ) - if self.type == "vpn" and not self.vpn: - raise ValidationError( - {"vpn": _('A VPN must be selected when template type is "VPN"')} - ) - elif self.type != "vpn": - self.vpn = None - if self.type != "cert": - self.auto_cert = False - if self.type == "vpn" and not self.config: - self.config = self.vpn.auto_client( - auto_cert=self.auto_cert, template_backend_class=self.backend_class - ) if self.type == "cert": self._validate_org_relation("ca") self._validate_org_relation("blueprint_cert") @@ -337,6 +321,39 @@ def clean(self, *args, **kwargs): else: self.ca = None self.blueprint_cert = None + + def clean(self, *args, **kwargs): + """ + * validates org relationship of VPN if present + * validates default_values field + * ensures VPN is selected if type is VPN + * clears VPN specific fields if type is not VPN + * automatically determines configuration if necessary + * if flagged as required forces it also to be default + * prevents mutating CA or Blueprint on active cert templates + * enforces CA and Blueprint requirements for cert templates + """ + self._validate_cert_template_changes() + self._clean_cert_template() + self._validate_org_relation("vpn") + if not self.default_values: + self.default_values = {} + if not isinstance(self.default_values, dict): + raise ValidationError( + {"default_values": _("the supplied value is not a JSON object")} + ) + if self.type == "vpn" and not self.vpn: + raise ValidationError( + {"vpn": _('A VPN must be selected when template type is "VPN"')} + ) + elif self.type != "vpn": + self.vpn = None + if self.type != "cert": + self.auto_cert = False + if self.type == "vpn" and not self.config: + self.config = self.vpn.auto_client( + auto_cert=self.auto_cert, template_backend_class=self.backend_class + ) if self.required and not self.default: self.default = True super().clean(*args, **kwargs) diff --git a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py index 1158b92a8..d5a5d549d 100644 --- a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py +++ b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py @@ -1,6 +1,10 @@ -# Generated by Django 5.2.14 on 2026-05-29 11:11 +# Generated by Django 5.2.14 on 2026-05-29 20:16 + +import uuid import django.db.models.deletion +import django.utils.timezone +import model_utils.fields from django.conf import settings from django.db import migrations, models @@ -78,17 +82,35 @@ class Migration(migrations.Migration): fields=[ ( "id", - models.AutoField( - auto_created=True, + models.UUIDField( + default=uuid.uuid4, + editable=False, primary_key=True, serialize=False, - verbose_name="ID", + ), + ), + ( + "created", + model_utils.fields.AutoCreatedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="created", + ), + ), + ( + "modified", + model_utils.fields.AutoLastModifiedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="modified", ), ), ("auto_cert", models.BooleanField(default=False)), ( "cert", models.OneToOneField( + blank=True, + null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.DJANGO_X509_CERT_MODEL, ), diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index b8b7bf94f..2466b1a4a 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -30,6 +30,7 @@ Config = load_model("config", "Config") DeviceGroup = load_model("config", "DeviceGroup") OrganizationUser = load_model("openwisp_users", "OrganizationUser") +DeviceCertificate = load_model("config", "DeviceCertificate") class ApiTestMixin: @@ -1329,6 +1330,138 @@ def test_bearer_authentication(self): ) self.assertEqual(response.status_code, 200) + def test_template_create_cert_type_api(self): + """Create a template of type 'cert' with a CA""" + org = self._get_org() + ca = self._create_ca(organization=org) + path = reverse("config_api:template_list") + data = self._template_data + data.update( + { + "name": "API Cert Template", + "type": "cert", + "ca": ca.pk, + "organization": str(org.pk), + "config": {}, + } + ) + r = self.client.post(path, data, content_type="application/json") + self.assertEqual(r.status_code, 201) + self.assertEqual(r.data["ca"], ca.pk) + + def test_template_create_cert_rejects_without_ca(self): + """Rejects cert template if CA is missing""" + org = self._get_org() + path = reverse("config_api:template_list") + data = self._template_data + data.update( + { + "name": "API Invalid Cert Template", + "type": "cert", + "organization": str(org.pk), + "config": {}, + } + ) + r = self.client.post(path, data, content_type="application/json") + self.assertEqual(r.status_code, 400) + self.assertIn("ca", r.data) + + def test_template_create_cert_blueprint_assignment(self): + """Can assign a blueprint certificate""" + org = self._get_org() + ca = self._create_ca(organization=org) + blueprint = self._create_cert(ca=ca, organization=org) + path = reverse("config_api:template_list") + data = self._template_data + data.update( + { + "name": "API Blueprint Template", + "type": "cert", + "ca": ca.pk, + "blueprint_cert": blueprint.pk, + "organization": str(org.pk), + "config": {}, + } + ) + r = self.client.post(path, data, content_type="application/json") + self.assertEqual(r.status_code, 201) + self.assertEqual(r.data["blueprint_cert"], blueprint.pk) + + def test_template_create_api_blueprint_ca_mismatch(self): + """API: Blueprint cert must belong to the selected CA""" + org = self._get_org() + ca1 = self._create_ca(name="CA1", common_name="CA1", organization=org) + ca2 = self._create_ca(name="CA2", common_name="CA2", organization=org) + blueprint = self._create_cert(name="BP", ca=ca1, organization=org) + path = reverse("config_api:template_list") + data = self._template_data + data.update( + { + "name": "Mismatch Template", + "type": "cert", + "ca": ca2.pk, + "blueprint_cert": blueprint.pk, + "organization": str(org.pk), + "config": {}, + } + ) + r = self.client.post(path, data, content_type="application/json") + self.assertEqual(r.status_code, 400) + self.assertIn("blueprint_cert", r.data) + self.assertIn( + "match the selected Certificate Authority", str(r.data["blueprint_cert"]) + ) + + def test_template_create_api_blueprint_already_assigned(self): + """Serializer correctly rejects an already assigned blueprint_cert""" + org = self._get_org() + ca = self._create_ca(organization=org) + blueprint = self._create_cert(ca=ca, organization=org) + device = self._create_device(organization=org) + config = self._create_config(device=device) + dummy_template = self._create_template( + type="cert", ca=ca, organization=org, config={} + ) + DeviceCertificate.objects.create( + config=config, template=dummy_template, cert=blueprint + ) + path = reverse("config_api:template_list") + data = self._template_data + data.update( + { + "name": "Assigned BP Template", + "type": "cert", + "ca": str(ca.pk), + "blueprint_cert": str(blueprint.pk), + "organization": str(org.pk), + "config": {}, + } + ) + r = self.client.post(path, data, content_type="application/json") + self.assertEqual(r.status_code, 400) + self.assertIn("blueprint_cert", r.data) + self.assertIn( + "already assigned to an active device", str(r.data["blueprint_cert"]) + ) + + def test_template_update_api_active_change_blocked(self): + """Cannot mutate CA or Blueprint on a template attached to active devices""" + org = self._get_org() + ca1 = self._create_ca(name="CA1", common_name="CA1", organization=org) + ca2 = self._create_ca(name="CA2", common_name="CA2", organization=org) + template = self._create_template( + name="Active Template", type="cert", ca=ca1, organization=org, config={} + ) + device = self._create_device(organization=org) + config = self._create_config(device=device) + config.templates.add(template) + path = reverse("config_api:template_detail", args=[template.pk]) + data = {"ca": ca2.pk} + r = self.client.patch(path, data, content_type="application/json") + self.assertEqual(r.status_code, 400) + self.assertIn("ca", r.data) + self.assertIn("already assigned to active devices", str(r.data["ca"])) + class TestConfigApiTransaction( ApiTestMixin, diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index f537c57dc..09c35d834 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -26,6 +26,7 @@ Ca = load_model("django_x509", "Ca") Cert = load_model("django_x509", "Cert") User = get_user_model() +DeviceCertificate = load_model("config", "DeviceCertificate") _original_context = app_settings.CONTEXT.copy() @@ -1023,7 +1024,6 @@ def test_blueprint_cannot_be_already_assigned(self): organization=org, config={}, ) - DeviceCertificate = load_model("config", "DeviceCertificate") DeviceCertificate.objects.create( config=config, template=template, diff --git a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py index c2925cd0b..caf4fb91f 100644 --- a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py +++ b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py @@ -1,6 +1,10 @@ -# Generated by Django 5.2.14 on 2026-05-29 11:14 +# Generated by Django 5.2.14 on 2026-05-29 20:18 + +import uuid import django.db.models.deletion +import django.utils.timezone +import model_utils.fields from django.db import migrations, models import openwisp_controller.config.base.template @@ -76,11 +80,27 @@ class Migration(migrations.Migration): fields=[ ( "id", - models.AutoField( - auto_created=True, + models.UUIDField( + default=uuid.uuid4, + editable=False, primary_key=True, serialize=False, - verbose_name="ID", + ), + ), + ( + "created", + model_utils.fields.AutoCreatedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="created", + ), + ), + ( + "modified", + model_utils.fields.AutoLastModifiedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="modified", ), ), ("auto_cert", models.BooleanField(default=False)), @@ -88,6 +108,8 @@ class Migration(migrations.Migration): ( "cert", models.OneToOneField( + blank=True, + null=True, on_delete=django.db.models.deletion.CASCADE, to="sample_pki.cert", ), From 052cebf0871e43771982392d963c8d4e63836594 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sat, 30 May 2026 01:56:58 +0530 Subject: [PATCH 12/34] [tests] Added tests for cert template type #1358 Implemented tests for certificate lifecycle in template tests Fixes #1358 --- .../config/tests/test_template.py | 179 +++++++++++++++++- 1 file changed, 169 insertions(+), 10 deletions(-) diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index 09c35d834..ea78c2869 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1064,16 +1064,6 @@ def test_non_cert_clears_fields(self): self.assertIsNone(t.ca) self.assertIsNone(t.blueprint_cert) - def test_cert_template_allows_empty_config(self): - """Test that cert templates can have empty config (unlike other types).""" - ca = self._create_ca() - t = self._create_template( - type="cert", - ca=ca, - config={}, - ) - self.assertEqual(t.config, {}) - def test_organization_validation_for_relations(self): """Test organization validation for ca field.""" org1 = self._get_org() @@ -1122,3 +1112,172 @@ def test_organization_validation_skipped_for_non_cert(self): raise err self.assertIsNone(t.ca) self.assertIsNone(t.blueprint_cert) + + def test_active_mutation_blocked(self): + """ + Test that ca and blueprint_cert cannot be + changed if assigned to active devices. + """ + org = self._get_org() + ca1 = self._create_ca(name="CA1", common_name="CA1", organization=org) + ca2 = self._create_ca(name="CA2", common_name="CA2", organization=org) + blueprint1 = self._create_cert( + name="BP1", common_name="BP1_CN", ca=ca1, organization=org + ) + blueprint2 = self._create_cert( + name="BP2", common_name="BP2_CN", ca=ca1, organization=org + ) + template = self._create_template( + name="Active Cert Template", + type="cert", + ca=ca1, + blueprint_cert=blueprint1, + organization=org, + config={}, + ) + device = self._create_device(organization=org) + config = self._create_config(device=device) + config.templates.add(template) + with self.subTest("Cannot mutate CA on active template"): + template.ca = ca2 + template.blueprint_cert = None + try: + template.full_clean() + except ValidationError as err: + self.assertIn("ca", err.message_dict) + self.assertIn( + "already assigned to active devices", str(err.message_dict["ca"][0]) + ) + else: + self.fail("ValidationError not raised for active mutation of CA") + + with self.subTest("Cannot mutate blueprint_cert on active template"): + template.refresh_from_db() + template.blueprint_cert = blueprint2 + try: + template.full_clean() + except ValidationError as err: + self.assertIn("blueprint_cert", err.message_dict) + self.assertIn( + "already assigned to active devices", + str(err.message_dict["blueprint_cert"][0]), + ) + else: + self.fail( + "ValidationError not raised for active mutation of blueprint_cert" + ) + + def test_cert_generation_fallback_to_ca_defaults(self): + """ + Verify synchronous generation falls back to CA defaults + when no blueprint_cert is specified. + """ + org = self._get_org() + ca = self._create_ca( + name="Fallback CA", + organization=org, + country_code="DE", + city="Berlin", + key_length="2048", + digest="sha256", + ) + template = self._create_template( + name="No Blueprint Template", + type="cert", + ca=ca, + blueprint_cert=None, + organization=org, + config={}, + ) + device = self._create_device(organization=org) + config = self._create_config(device=device) + config.templates.add(template) + dev_cert = config.devicecertificate_set.first() + self.assertIsNotNone(dev_cert) + generated_cert = dev_cert.cert + self.assertEqual(generated_cert.country_code, "DE") + self.assertEqual(generated_cert.city, "Berlin") + self.assertEqual(generated_cert.key_length, "2048") + self.assertEqual(generated_cert.digest, "sha256") + + def test_cert_generation_and_revocation_lifecycle(self): + """ + Verify synchronous generation copies blueprint attributes, injects CN/OIDs, + and securely revokes the underlying certificate upon removal. + """ + org = self._get_org() + ca = self._create_ca(organization=org) + blueprint = self._create_cert( + name="Master Blueprint", + common_name="Master Blueprint CN", + ca=ca, + organization=org, + key_length="4096", + digest="sha512", + city="Rome", + country_code="IT", + extensions=[ + {"name": "nsComment", "value": "Test comment", "critical": False} + ], + ) + template = self._create_template( + name="Deep Lifecycle Template", + type="cert", + ca=ca, + blueprint_cert=blueprint, + organization=org, + config={}, + ) + device = self._create_device(organization=org) + config = self._create_config(device=device) + config.templates.add(template) + dev_cert = config.devicecertificate_set.first() + self.assertIsNotNone(dev_cert, "DeviceCertificate was not created") + generated_cert = dev_cert.cert + with self.subTest("Copies blueprint attributes and sets CN"): + self.assertEqual(generated_cert.key_length, "4096") + self.assertEqual(generated_cert.digest, "sha512") + self.assertEqual(generated_cert.city, "Rome") + self.assertEqual(generated_cert.country_code, "IT") + self.assertTrue( + generated_cert.common_name.startswith( + f"{device.mac_address}-{device.name}" + ) + ) + + # uncomment after #228 in django-x509 + # with self.subTest("Injects custom MAC and UUID OIDs"): + # extensions = generated_cert.extensions + # mac_oid = "1.3.6.1.4.1.65901.1" + # uuid_oid = "1.3.6.1.4.1.65901.2" + # mac_ext = next( + # ( + # ext + # for ext in extensions + # if ext.get("oid") == mac_oid + # or ext.get("name") == mac_oid + # ), + # None, + # ) + # uuid_ext = next( + # ( + # ext + # for ext in extensions + # if ext.get("oid") == uuid_oid + # or ext.get("name") == uuid_oid + # ), + # None, + # ) + # self.assertIsNotNone(mac_ext, "MAC OID extension missing") + # self.assertIn(device.mac_address, mac_ext["value"]) + # self.assertIsNotNone(uuid_ext, "UUID OID extension missing") + # self.assertIn(str(device.id), uuid_ext["value"]) + + with self.subTest("Secure Revocation (post_remove)"): + cert_pk = generated_cert.pk + config.templates.remove(template) + self.assertEqual(config.devicecertificate_set.count(), 0) + revoked_cert = Cert.objects.get(pk=cert_pk) + self.assertTrue( + revoked_cert.revoked, "Underlying certificate was not revoked!" + ) From 148bfd25baeab47a938af27d5f9cc8cec3365f0f Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sat, 30 May 2026 23:41:10 +0530 Subject: [PATCH 13/34] [tests] Added tests for API #1361 Added lifecycle and general test in test api for certificate template Fixes #1361 --- openwisp_controller/config/tests/test_api.py | 71 +++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 2466b1a4a..08bbb9c60 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -1388,7 +1388,7 @@ def test_template_create_cert_blueprint_assignment(self): self.assertEqual(r.data["blueprint_cert"], blueprint.pk) def test_template_create_api_blueprint_ca_mismatch(self): - """API: Blueprint cert must belong to the selected CA""" + """Blueprint cert must belong to the selected CA""" org = self._get_org() ca1 = self._create_ca(name="CA1", common_name="CA1", organization=org) ca2 = self._create_ca(name="CA2", common_name="CA2", organization=org) @@ -1449,6 +1449,9 @@ def test_template_update_api_active_change_blocked(self): org = self._get_org() ca1 = self._create_ca(name="CA1", common_name="CA1", organization=org) ca2 = self._create_ca(name="CA2", common_name="CA2", organization=org) + blueprint = self._create_cert( + name="BP", common_name="BP_CN", ca=ca1, organization=org + ) template = self._create_template( name="Active Template", type="cert", ca=ca1, organization=org, config={} ) @@ -1461,6 +1464,72 @@ def test_template_update_api_active_change_blocked(self): self.assertEqual(r.status_code, 400) self.assertIn("ca", r.data) self.assertIn("already assigned to active devices", str(r.data["ca"])) + r = self.client.patch( + path, {"blueprint_cert": blueprint.pk}, content_type="application/json" + ) + self.assertEqual(r.status_code, 400) + self.assertIn("blueprint_cert", r.data) + self.assertIn( + "already assigned to active devices", str(r.data["blueprint_cert"]) + ) + + def test_template_create_api_org_scoping(self): + """Rejects CA or Blueprint from a different organization""" + org1 = self._get_org() + org2 = self._create_org(name="Org2", slug="org2") + ca_org2 = self._create_ca(name="CA2", common_name="CA2", organization=org2) + path = reverse("config_api:template_list") + data = self._template_data + data.update( + { + "name": "Org Scope Template", + "type": "cert", + "ca": ca_org2.pk, + "organization": str(org1.pk), + "config": {}, + } + ) + r = self.client.post(path, data, content_type="application/json") + self.assertEqual(r.status_code, 400) + self.assertIn("organization", r.data) + self.assertIn("related CA match", str(r.data["organization"])) + + def test_device_api_cert_template_lifecycle(self): + """Assigning/removing a cert template triggers DeviceCertificate lifecycle""" + org = self._get_org() + ca = self._create_ca(organization=org) + cert_template = self._create_template( + name="API Lifecycle Cert", type="cert", ca=ca, organization=org, config={} + ) + device = self._create_device(organization=org) + config = self._create_config(device=device) + path = reverse("config_api:device_detail", args=[device.pk]) + data = self._device_data + data.update( + { + "name": device.name, + "organization": str(org.pk), + "mac_address": device.mac_address, + } + ) + with self.subTest("Assigning template via API creates DeviceCertificate"): + data["config"]["templates"] = [str(cert_template.pk)] + response = self.client.put(path, data, content_type="application/json") + self.assertEqual(response.status_code, 200, response.data) + self.assertEqual(config.templates.count(), 1) + self.assertEqual(config.devicecertificate_set.count(), 1) + generated_cert = config.devicecertificate_set.get().cert + + with self.subTest( + "Removing template via API deletes/revokes DeviceCertificate" + ): + data["config"]["templates"] = [] + response = self.client.put(path, data, content_type="application/json") + self.assertEqual(response.status_code, 200, response.data) + self.assertEqual(config.templates.count(), 0) + self.assertEqual(config.devicecertificate_set.count(), 0) + generated_cert.refresh_from_db() + self.assertTrue(generated_cert.revoked) class TestConfigApiTransaction( From badf1c15384b39590eafb491a91af0c2fb7492df Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sun, 31 May 2026 00:11:36 +0530 Subject: [PATCH 14/34] [fix] Addressed model improvements #1377 Added minor improvements to model structure Fixes #1377 --- openwisp_controller/config/base/config.py | 5 ++--- .../config/base/device_certificate.py | 19 ++++++++++++++----- openwisp_controller/config/base/template.py | 13 +++++++------ ...ate_blueprint_cert_template_ca_and_more.py | 4 ++-- ...ate_blueprint_cert_template_ca_and_more.py | 4 ++-- 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index f3f0a3518..55477b93a 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -3,7 +3,6 @@ import re from collections import defaultdict -import swapper from cache_memoize import cache_memoize from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError @@ -65,8 +64,8 @@ class AbstractConfig(ChecksumCacheMixin, BaseConfig): blank=True, ) device_certificates = models.ManyToManyField( - swapper.get_model_name("config", "Template"), - through=swapper.get_model_name("config", "DeviceCertificate"), + get_model_name("config", "Template"), + through=get_model_name("config", "DeviceCertificate"), related_name="config_device_certificates", blank=True, verbose_name=_("device certificates"), diff --git a/openwisp_controller/config/base/device_certificate.py b/openwisp_controller/config/base/device_certificate.py index d3349eff4..2c483cf6d 100644 --- a/openwisp_controller/config/base/device_certificate.py +++ b/openwisp_controller/config/base/device_certificate.py @@ -70,12 +70,13 @@ def _get_common_name(self): """ d = self.config.device end = 63 - len(d.mac_address) - d.name = d.name[:end] + truncated_name = d.name[:end] unique_slug = shortuuid.ShortUUID().random(length=8) cn_format = app_settings.COMMON_NAME_FORMAT - if cn_format == "{mac_address}-{name}" and d.name == d.mac_address: + if cn_format == "{mac_address}-{name}" and truncated_name == d.mac_address: cn_format = "{mac_address}" - common_name = cn_format.format(**d.__dict__)[:55] + format_dict = {**d.__dict__, "name": truncated_name} + common_name = cn_format.format(**format_dict)[:55] common_name = f"{common_name}-{unique_slug}" return common_name @@ -111,8 +112,16 @@ def _auto_create_cert(self, name, common_name): # mac_oid = "1.3.6.1.4.1.65901.1" # uuid_oid = "1.3.6.1.4.1.65901.2" # extensions.extend([ - # {"name": mac_oid, "value": device.mac_address, "critical": False}, - # {"name": uuid_oid, "value": str(device.id), "critical": False} + # { + # "oid": mac_oid, + # "value": f"ASN1:UTF8:string:{device.mac_address}", + # "critical": False + # }, + # { + # "oid": uuid_oid, + # "value": f"ASN1:UTF8:string:{device.id}", + # "critical": False + # } # ]) cert = cert_model( name=name, diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index 437b4632b..318346bf7 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -37,6 +37,13 @@ def default_auto_cert(): return DEFAULT_AUTO_CERT +def get_unassigned_certs(): + Cert = load_model("django_x509", "Cert") + DeviceCertificate = load_model("config", "DeviceCertificate") + assigned_cert_ids = DeviceCertificate.objects.values_list("cert_id", flat=True) + return {"pk__in": Cert.objects.exclude(id__in=assigned_cert_ids)} + + class AbstractTemplate(ShareableOrgMixinUniqueName, BaseConfig): """ Abstract model implementing a @@ -71,12 +78,6 @@ class AbstractTemplate(ShareableOrgMixinUniqueName, BaseConfig): ), ) - def get_unassigned_certs(): - Cert = load_model("django_x509", "Cert") - DeviceCertificate = load_model("config", "DeviceCertificate") - assigned_cert_ids = DeviceCertificate.objects.values_list("cert_id", flat=True) - return {"pk__in": Cert.objects.exclude(id__in=assigned_cert_ids)} - blueprint_cert = models.ForeignKey( get_model_name("django_x509", "Cert"), on_delete=models.SET_NULL, diff --git a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py index d5a5d549d..083c21df8 100644 --- a/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py +++ b/openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 5.2.14 on 2026-05-29 20:16 +# Generated by Django 5.2.14 on 2026-05-30 18:31 import uuid @@ -27,7 +27,7 @@ class Migration(migrations.Migration): blank=True, help_text="Optional: Select an unassigned certificate " "to copy extensions and properties from.", - limit_choices_to=openwisp_controller.config.base.template.AbstractTemplate.get_unassigned_certs, # noqa + limit_choices_to=openwisp_controller.config.base.template.get_unassigned_certs, # noqa null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.DJANGO_X509_CERT_MODEL, diff --git a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py index caf4fb91f..359912509 100644 --- a/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py +++ b/tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 5.2.14 on 2026-05-29 20:18 +# Generated by Django 5.2.14 on 2026-05-30 18:33 import uuid @@ -25,7 +25,7 @@ class Migration(migrations.Migration): blank=True, help_text="Optional: Select an unassigned certificate " "to copy extensions and properties from.", - limit_choices_to=openwisp_controller.config.base.template.AbstractTemplate.get_unassigned_certs, # noqa + limit_choices_to=openwisp_controller.config.base.template.get_unassigned_certs, # noqa null=True, on_delete=django.db.models.deletion.SET_NULL, to="sample_pki.cert", From a692c77bd8dfed10bfb076574621e72ed8db2918 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sun, 31 May 2026 16:01:42 +0530 Subject: [PATCH 15/34] [fix] Fixed certificate template reordering lifecycle #1358 Refactors to remove the action from the m2m_changed signal, preventing accidental deletion and revocation of valid device certificates when templates are reordered or updated. Fixes #1358 --- openwisp_controller/config/api/serializers.py | 6 ++- openwisp_controller/config/base/config.py | 31 ++++++++++---- .../config/tests/test_template.py | 41 +++++++++++++++++++ 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index 8768a2d64..ccbdfeb52 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -113,7 +113,11 @@ def validate(self, data): ) } ) - if self.instance and self.instance.pk: + if ( + self.instance + and self.instance.pk + and ("ca" in data or "blueprint_cert" in data) + ): if Config.objects.filter(templates=self.instance).exists(): if "ca" in data and data["ca"] != self.instance.ca: raise serializers.ValidationError( diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index 55477b93a..eb195f011 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -507,17 +507,32 @@ def manage_device_certs(cls, sender, instance, action, pk_set, **kwargs): """ Syncs DeviceCertificate objects when templates are added/removed """ + if instance._state.adding or action not in [ + "post_add", + "post_remove", + "post_clear", + ]: + return + if action == "post_clear": + if instance.is_deactivating_or_deactivated(): + instance.devicecertificate_set.all().delete() + return + if isinstance(pk_set, set): + template_model = cls.get_template_model() + templates = template_model.objects.filter(pk__in=list(pk_set)).order_by( + "created" + ) + else: + templates = pk_set + if len(pk_set) != templates.filter(required=True).count(): + instance.devicecertificate_set.exclude( + template_id__in=instance.templates.values_list("id", flat=True) + ).delete() if action == "post_add": - templates = instance.templates.filter(pk__in=pk_set, type="cert") - for tpl in templates: + for template in templates.filter(type="cert"): instance.devicecertificate_set.get_or_create( - template=tpl, defaults={"auto_cert": tpl.auto_cert} + template=template, defaults={"auto_cert": template.auto_cert} ) - elif action in ["post_remove", "pre_clear"]: - certs_to_delete = instance.devicecertificate_set.all() - if action == "post_remove": - certs_to_delete = certs_to_delete.filter(template_id__in=pk_set) - certs_to_delete.delete() def get_default_templates(self): """ diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index ea78c2869..cf8ca9c74 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1281,3 +1281,44 @@ def test_cert_generation_and_revocation_lifecycle(self): self.assertTrue( revoked_cert.revoked, "Underlying certificate was not revoked!" ) + + def test_cert_template_reorder_does_not_revoke(self): + """ + Verify that reordering templates or performing bulk .set() updates + does not delete and recreate the existing DeviceCertificate. + """ + org = self._get_org() + ca = self._create_ca(organization=org) + cert_template = self._create_template( + name="Cert Template", type="cert", ca=ca, organization=org, config={} + ) + regular_template = self._create_template( + name="Regular Template", + organization=org, + config={"system": {"hostname": "test_router"}}, + ) + device = self._create_device(organization=org) + config = self._create_config(device=device) + config.templates.set([regular_template, cert_template]) + dev_cert = config.devicecertificate_set.first() + self.assertIsNotNone(dev_cert, "DeviceCertificate should be created.") + original_dev_cert_id = dev_cert.pk + original_cert_id = dev_cert.cert.pk + config.templates.set([cert_template, regular_template], clear=True) + dev_certs = config.devicecertificate_set.all() + self.assertEqual(dev_certs.count(), 1) + surviving_dev_cert = dev_certs.first() + self.assertEqual( + surviving_dev_cert.pk, + original_dev_cert_id, + "DeviceCertificate was deleted and recreated during reordering!", + ) + self.assertEqual( + surviving_dev_cert.cert.pk, + original_cert_id, + "Underlying X.509 Cert was replaced during reordering!", + ) + self.assertFalse( + surviving_dev_cert.cert.revoked, + "Certificate was erroneously revoked during reordering!", + ) From 16703f1e427e78eb44ec12477bd1036eefc28c49 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sun, 31 May 2026 16:36:47 +0530 Subject: [PATCH 16/34] [fix] Minor improvements Improved mutation lock and string formatting --- openwisp_controller/config/api/serializers.py | 6 +++++- openwisp_controller/config/base/device_certificate.py | 2 +- openwisp_controller/config/base/template.py | 6 +++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index ccbdfeb52..383ab4521 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -118,7 +118,11 @@ def validate(self, data): and self.instance.pk and ("ca" in data or "blueprint_cert" in data) ): - if Config.objects.filter(templates=self.instance).exists(): + if ( + Config.objects.filter(templates=self.instance) + .exclude(status__in=["deactivating", "deactivated"]) + .exists() + ): if "ca" in data and data["ca"] != self.instance.ca: raise serializers.ValidationError( { diff --git a/openwisp_controller/config/base/device_certificate.py b/openwisp_controller/config/base/device_certificate.py index 2c483cf6d..207b453f2 100644 --- a/openwisp_controller/config/base/device_certificate.py +++ b/openwisp_controller/config/base/device_certificate.py @@ -30,7 +30,7 @@ class Meta: verbose_name_plural = _("Device certificates") def __str__(self): - cert_name = self.cert.name if self.cert else "Pending Generation" + cert_name = self.cert.name if self.cert else str(_("Pending Generation")) return f"{self.config.device.name} - {cert_name}" def clean(self): diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index 318346bf7..48185439e 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -264,7 +264,11 @@ def _validate_cert_template_changes(self): or current.blueprint_cert_id != self.blueprint_cert_id ): Config = load_model("config", "Config") - if Config.objects.filter(templates=self).exists(): + if ( + Config.objects.filter(templates=self) + .exclude(status__in=["deactivating", "deactivated"]) + .exists() + ): message = _( "This template is already assigned to active devices. " "You cannot change the CA or Blueprint Certificate " From 212b8394fa8e089432c800a7e0267d59c15de9e0 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Mon, 1 Jun 2026 15:27:43 +0530 Subject: [PATCH 17/34] [fix] Made cert creation transactional - Wrap DeviceCertificate.save() in transaction.atomic() to ensure automatic X.509 provisioning does not leak orphaned rows on failure. - Add validation to block changing the template type away from 'cert' on templates currently assigned to active devices. --- openwisp_controller/config/api/serializers.py | 25 ++++++++- openwisp_controller/config/base/config.py | 11 ++++ .../config/base/device_certificate.py | 13 +++-- openwisp_controller/config/base/template.py | 53 +++++++++++------- openwisp_controller/config/tests/test_api.py | 11 +++- .../config/tests/test_template.py | 55 ++++++++++++++++++- 6 files changed, 135 insertions(+), 33 deletions(-) diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index 383ab4521..d7f46619e 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -54,7 +54,7 @@ class Meta(BaseMeta): "error_messages": { "does_not_exist": _( "This certificate does not exist or is already " - "assigned to an active device." + "assigned to a device configuration profile." ) } } @@ -91,6 +91,7 @@ def validate(self, data): blueprint_cert = data.get( "blueprint_cert", getattr(self.instance, "blueprint_cert", None) ) + # cert templates must have a CA if template_type == "cert" and not ca: raise serializers.ValidationError( { @@ -100,9 +101,12 @@ def validate(self, data): ) } ) + # clear certificate-specific fields if the template is not a certificate elif template_type != "cert": data["ca"] = None data["blueprint_cert"] = None + + # assert structural binding matches between CA and template blueprints if blueprint_cert and ca: if blueprint_cert.ca_id != ca.id: raise serializers.ValidationError( @@ -113,16 +117,32 @@ def validate(self, data): ) } ) + + # apply mutation protections over protected fields if ( self.instance and self.instance.pk - and ("ca" in data or "blueprint_cert" in data) + and ("ca" in data or "blueprint_cert" in data or "type" in data) ): + # only enforce locks if the template is assigned + # to active/activating devices if ( Config.objects.filter(templates=self.instance) .exclude(status__in=["deactivating", "deactivated"]) .exists() ): + # block changing a certificate template to a generic template + if self.instance.type == "cert" and template_type != "cert": + raise serializers.ValidationError( + { + "type": _( + "This template is already assigned to active devices. " + "You cannot change the template type from certificate " + "on an active template." + ) + } + ) + # block altering the assigned Certificate Authority if "ca" in data and data["ca"] != self.instance.ca: raise serializers.ValidationError( { @@ -133,6 +153,7 @@ def validate(self, data): ) } ) + # block altering the assigned Blueprint Certificate if ( "blueprint_cert" in data and data["blueprint_cert"] != self.instance.blueprint_cert diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index eb195f011..bdbe23460 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -507,16 +507,21 @@ def manage_device_certs(cls, sender, instance, action, pk_set, **kwargs): """ Syncs DeviceCertificate objects when templates are added/removed """ + # ignore signals during initial creation or for irrelevant M2M actions if instance._state.adding or action not in [ "post_add", "post_remove", "post_clear", ]: return + + # handle full cleanup if the device configuration profile is being wiped if action == "post_clear": if instance.is_deactivating_or_deactivated(): instance.devicecertificate_set.all().delete() return + + # normalize templates across standard M2M sets vs Admin ModelForm querysets if isinstance(pk_set, set): template_model = cls.get_template_model() templates = template_model.objects.filter(pk__in=list(pk_set)).order_by( @@ -524,10 +529,16 @@ def manage_device_certs(cls, sender, instance, action, pk_set, **kwargs): ) else: templates = pk_set + + # deletes orphaned certificates that are no + # longer assigned in the templates list. if len(pk_set) != templates.filter(required=True).count(): instance.devicecertificate_set.exclude( template_id__in=instance.templates.values_list("id", flat=True) ).delete() + + # allocate new DeviceCertificate associations + # for newly added certificate templates if action == "post_add": for template in templates.filter(type="cert"): instance.devicecertificate_set.get_or_create( diff --git a/openwisp_controller/config/base/device_certificate.py b/openwisp_controller/config/base/device_certificate.py index 207b453f2..dc81fe4bc 100644 --- a/openwisp_controller/config/base/device_certificate.py +++ b/openwisp_controller/config/base/device_certificate.py @@ -1,6 +1,8 @@ +import copy + import shortuuid from django.core.exceptions import ObjectDoesNotExist, ValidationError -from django.db import models +from django.db import models, transaction from django.utils.translation import gettext_lazy as _ from swapper import get_model_name, load_model @@ -51,9 +53,10 @@ def clean(self): def save(self, *args, **kwargs): """Performs automatic provisioning if ``auto_cert`` is True.""" - if self.auto_cert and not self.cert: - self._auto_x509() - super().save(*args, **kwargs) + with transaction.atomic(): + if self.auto_cert and not self.cert: + self._auto_x509() + super().save(*args, **kwargs) def _auto_x509(self): """ @@ -85,8 +88,6 @@ def _auto_create_cert(self, name, common_name): Automatically creates and assigns a client x509 certificate using Blueprint cloning and custom hardware OID injection. """ - import copy - ca = self.template.ca blueprint = self.template.blueprint_cert # device = self.config.device diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index 48185439e..22eb61a49 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -250,7 +250,7 @@ def _auto_add_to_existing_configs(self): def _validate_cert_template_changes(self): """ - Prevents changing the CA or Blueprint Certificate of a certificate template + Prevents changing cert-specific settings of a certificate template if it is already assigned to active devices. """ if self._state.adding: @@ -259,27 +259,40 @@ def _validate_cert_template_changes(self): current = self.__class__.objects.get(pk=self.pk) except self.__class__.DoesNotExist: return - if ( + changing_protected_fields = ( current.ca_id != self.ca_id or current.blueprint_cert_id != self.blueprint_cert_id + or (current.type == "cert" and self.type != "cert") + ) + if not changing_protected_fields: + return + + Config = load_model("config", "Config") + if not ( + Config.objects.filter(templates=self) + .exclude(status__in=["deactivating", "deactivated"]) + .exists() ): - Config = load_model("config", "Config") - if ( - Config.objects.filter(templates=self) - .exclude(status__in=["deactivating", "deactivated"]) - .exists() - ): - message = _( - "This template is already assigned to active devices. " - "You cannot change the CA or Blueprint Certificate " - "on an active template." - ) - errors = {} - if current.ca_id != self.ca_id: - errors["ca"] = message - if current.blueprint_cert_id != self.blueprint_cert_id: - errors["blueprint_cert"] = message - raise ValidationError(errors) + return + + message = _( + "This template is already assigned to active devices. " + "You cannot change the CA or Blueprint Certificate " + "on an active template." + ) + errors = {} + if current.ca_id != self.ca_id: + errors["ca"] = message + if current.blueprint_cert_id != self.blueprint_cert_id: + errors["blueprint_cert"] = message + if current.type == "cert" and self.type != "cert": + errors["type"] = _( + "This template is already assigned to active devices. " + "You cannot change the template type from certificate " + "on an active template." + ) + if errors: + raise ValidationError(errors) def _clean_cert_template(self): """ @@ -335,7 +348,7 @@ def clean(self, *args, **kwargs): * clears VPN specific fields if type is not VPN * automatically determines configuration if necessary * if flagged as required forces it also to be default - * prevents mutating CA or Blueprint on active cert templates + * prevents mutating cert-specific fields on active cert templates * enforces CA and Blueprint requirements for cert templates """ self._validate_cert_template_changes() diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 08bbb9c60..48fd09a2b 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -1441,11 +1441,12 @@ def test_template_create_api_blueprint_already_assigned(self): self.assertEqual(r.status_code, 400) self.assertIn("blueprint_cert", r.data) self.assertIn( - "already assigned to an active device", str(r.data["blueprint_cert"]) + "already assigned to a device configuration profile.", + str(r.data["blueprint_cert"]), ) def test_template_update_api_active_change_blocked(self): - """Cannot mutate CA or Blueprint on a template attached to active devices""" + """Cannot mutate cert-specific fields on active templates""" org = self._get_org() ca1 = self._create_ca(name="CA1", common_name="CA1", organization=org) ca2 = self._create_ca(name="CA2", common_name="CA2", organization=org) @@ -1472,6 +1473,12 @@ def test_template_update_api_active_change_blocked(self): self.assertIn( "already assigned to active devices", str(r.data["blueprint_cert"]) ) + r = self.client.patch( + path, {"type": "generic"}, content_type="application/json" + ) + self.assertEqual(r.status_code, 400) + self.assertIn("type", r.data) + self.assertIn("already assigned to active devices", str(r.data["type"])) def test_template_create_api_org_scoping(self): """Rejects CA or Blueprint from a different organization""" diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index cf8ca9c74..0e2e5abf5 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -4,7 +4,7 @@ from celery.exceptions import SoftTimeLimitExceeded from django.contrib.auth import get_user_model from django.core.exceptions import PermissionDenied, ValidationError -from django.db import transaction +from django.db import IntegrityError, transaction from django.test import TestCase, TransactionTestCase from netjsonconfig import OpenWrt from netjsonconfig.exceptions import ValidationError as NetjsonconfigValidationError @@ -1115,8 +1115,8 @@ def test_organization_validation_skipped_for_non_cert(self): def test_active_mutation_blocked(self): """ - Test that ca and blueprint_cert cannot be - changed if assigned to active devices. + Test that cert-specific fields cannot be changed + if assigned to active devices. """ org = self._get_org() ca1 = self._create_ca(name="CA1", common_name="CA1", organization=org) @@ -1167,6 +1167,22 @@ def test_active_mutation_blocked(self): "ValidationError not raised for active mutation of blueprint_cert" ) + with self.subTest("Cannot change type away from cert on active template"): + template.refresh_from_db() + template.type = "generic" + try: + template.full_clean() + except ValidationError as err: + self.assertIn("type", err.message_dict) + self.assertIn( + "already assigned to active devices", + str(err.message_dict["type"][0]), + ) + else: + self.fail( + "ValidationError not raised for active mutation of template type" + ) + def test_cert_generation_fallback_to_ca_defaults(self): """ Verify synchronous generation falls back to CA defaults @@ -1282,6 +1298,39 @@ def test_cert_generation_and_revocation_lifecycle(self): revoked_cert.revoked, "Underlying certificate was not revoked!" ) + def test_device_certificate_autocert_save_is_atomic(self): + """ + Ensure certificate auto-provisioning does not leak Cert rows + when DeviceCertificate save fails. + """ + org = self._get_org() + ca = self._create_ca(organization=org) + template = self._create_template( + name="Atomic Cert Template", + type="cert", + ca=ca, + organization=org, + config={}, + ) + device = self._create_device(organization=org) + config = self._create_config(device=device) + DeviceCertificate.objects.create( + config=config, + template=template, + cert=self._create_cert(ca=ca, organization=org), + auto_cert=False, + ) + cert_count = Cert.objects.count() + + with self.assertRaises(IntegrityError): + DeviceCertificate.objects.create( + config=config, + template=template, + auto_cert=True, + ) + + self.assertEqual(Cert.objects.count(), cert_count) + def test_cert_template_reorder_does_not_revoke(self): """ Verify that reordering templates or performing bulk .set() updates From 37ff9a91f54c1df67d578e1ab202fce11c7212b7 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Mon, 1 Jun 2026 15:49:34 +0530 Subject: [PATCH 18/34] [fix] Fixed NULL handling in get_unassigned_certs() Fixed NULL handling in get_unassigned_certs() to avoid empty blueprint_cert choices --- openwisp_controller/config/base/template.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index 22eb61a49..5651b9fe3 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -40,7 +40,9 @@ def default_auto_cert(): def get_unassigned_certs(): Cert = load_model("django_x509", "Cert") DeviceCertificate = load_model("config", "DeviceCertificate") - assigned_cert_ids = DeviceCertificate.objects.values_list("cert_id", flat=True) + assigned_cert_ids = DeviceCertificate.objects.filter( + cert_id__isnull=False + ).values_list("cert_id", flat=True) return {"pk__in": Cert.objects.exclude(id__in=assigned_cert_ids)} From ef90a6c5f52d31365554d7721bbff99c6a7d4a92 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Tue, 2 Jun 2026 13:46:17 +0530 Subject: [PATCH 19/34] [fix] Filtered revoked certs from blueprint list Updated get_unassigned_certs to include revoked certificates --- openwisp_controller/config/base/template.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openwisp_controller/config/base/template.py b/openwisp_controller/config/base/template.py index 5651b9fe3..cd152ac08 100644 --- a/openwisp_controller/config/base/template.py +++ b/openwisp_controller/config/base/template.py @@ -43,7 +43,10 @@ def get_unassigned_certs(): assigned_cert_ids = DeviceCertificate.objects.filter( cert_id__isnull=False ).values_list("cert_id", flat=True) - return {"pk__in": Cert.objects.exclude(id__in=assigned_cert_ids)} + return { + "pk__in": Cert.objects.exclude(id__in=assigned_cert_ids), + "revoked": False, + } class AbstractTemplate(ShareableOrgMixinUniqueName, BaseConfig): From bca38093a2efee82ca7d7b73a07e6162dbe5abc4 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Wed, 3 Jun 2026 15:57:29 +0530 Subject: [PATCH 20/34] [fix] Prevent zombie certs Updated post_clear to prevent stale DeviceCertificate rows --- openwisp_controller/config/base/config.py | 8 +++++++ .../config/tests/test_template.py | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index bdbe23460..0da5a13f6 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -519,6 +519,14 @@ def manage_device_certs(cls, sender, instance, action, pk_set, **kwargs): if action == "post_clear": if instance.is_deactivating_or_deactivated(): instance.devicecertificate_set.all().delete() + else: + # If this is a reorder, the subsequent 'add' will complete first, + # saving the certs. If it is a pure clear, the certs will be deleted. + transaction.on_commit( + lambda: instance.devicecertificate_set.exclude( + template_id__in=instance.templates.values_list("id", flat=True) + ).delete() + ) return # normalize templates across standard M2M sets vs Admin ModelForm querysets diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index 0e2e5abf5..48705012f 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -13,6 +13,7 @@ from openwisp_utils.tests import catch_signal from .. import settings as app_settings +from ..base.template import get_unassigned_certs from ..signals import config_modified, config_status_changed from ..tasks import auto_add_template_to_existing_configs from ..tasks import logger as task_logger @@ -1371,3 +1372,24 @@ def test_cert_template_reorder_does_not_revoke(self): surviving_dev_cert.cert.revoked, "Certificate was erroneously revoked during reordering!", ) + + def test_get_unassigned_certs_with_null_device_cert(self): + """ + Test that a DeviceCertificate with cert=None does not poison + the get_unassigned_certs() SQL query due to NULL semantics. + """ + org = self._get_org() + ca = self._create_ca(name="Test-CA", organization=org) + unassigned_cert = self._create_cert( + name="Available-Blueprint", ca=ca, organization=org + ) + device = self._create_device(name="Test-Device", organization=org) + config = self._create_config(device=device) + template = self._create_template( + name="Test-Template", type="cert", ca=ca, organization=org, config={} + ) + DeviceCertificate.objects.create(config=config, template=template, cert=None) + choices = get_unassigned_certs() + queryset = choices.get("pk__in") + self.assertIsNotNone(queryset) + self.assertIn(unassigned_cert, queryset) From 99cd151b8348d16a9996d16d7e9bb476b5141772 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Wed, 3 Jun 2026 16:23:17 +0530 Subject: [PATCH 21/34] [tests] Cover the partial replacement path too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A companion regression for “old cert removed, other templates kept” --- .../config/tests/test_template.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index 48705012f..c9ed89710 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1373,6 +1373,42 @@ def test_cert_template_reorder_does_not_revoke(self): "Certificate was erroneously revoked during reordering!", ) + def test_cert_template_partial_replacement_cleans_up(self): + """ + Verify that when a subset of templates is replaced via .set(..., clear=True), + the DeviceCertificates for the removed templates are correctly deleted, + while the kept ones survive. + """ + org = self._get_org() + ca = self._create_ca(organization=org) + + cert_template_1 = self._create_template( + name="Cert Template 1", type="cert", ca=ca, organization=org, config={} + ) + cert_template_2 = self._create_template( + name="Cert Template 2", type="cert", ca=ca, organization=org, config={} + ) + device = self._create_device(organization=org) + config = self._create_config(device=device) + config.templates.set([cert_template_1, cert_template_2]) + self.assertEqual( + config.devicecertificate_set.count(), 2, "Two certs should be created" + ) + kept_cert_id = config.devicecertificate_set.get(template=cert_template_1).pk + removed_cert_id = config.devicecertificate_set.get(template=cert_template_2).pk + config.templates.set([cert_template_1], clear=True) + self.assertEqual( + config.devicecertificate_set.count(), 1, "Only one cert should remain" + ) + self.assertTrue( + config.devicecertificate_set.filter(pk=kept_cert_id).exists(), + "The kept template's certificate should have survived.", + ) + self.assertFalse( + config.devicecertificate_set.filter(pk=removed_cert_id).exists(), + "The removed template's certificate should have been deleted.", + ) + def test_get_unassigned_certs_with_null_device_cert(self): """ Test that a DeviceCertificate with cert=None does not poison From ee79c9391b43e149990d38b027c89cade9e7522c Mon Sep 17 00:00:00 2001 From: stktyagi Date: Thu, 4 Jun 2026 15:22:47 +0530 Subject: [PATCH 22/34] [chores] Added custom oid extensions for mac and uuid #1377 Uncommented code for custom oid extensions used for mac address and uuid support in certificate with relevant test Fixes #1377 --- .../config/base/device_certificate.py | 32 ++++++------ .../config/tests/test_template.py | 51 +++++++++---------- 2 files changed, 41 insertions(+), 42 deletions(-) diff --git a/openwisp_controller/config/base/device_certificate.py b/openwisp_controller/config/base/device_certificate.py index dc81fe4bc..b51dcfb37 100644 --- a/openwisp_controller/config/base/device_certificate.py +++ b/openwisp_controller/config/base/device_certificate.py @@ -90,7 +90,7 @@ def _auto_create_cert(self, name, common_name): """ ca = self.template.ca blueprint = self.template.blueprint_cert - # device = self.config.device + device = self.config.device cert_model = self.__class__.cert.field.related_model # blueprint property cloning with CA fallback @@ -110,20 +110,22 @@ def _auto_create_cert(self, name, common_name): extensions = [{"name": "nsCertType", "value": "client", "critical": False}] # inject MAC and UUID as custom OIDs, prerequisite: #228 in django-x509 - # mac_oid = "1.3.6.1.4.1.65901.1" - # uuid_oid = "1.3.6.1.4.1.65901.2" - # extensions.extend([ - # { - # "oid": mac_oid, - # "value": f"ASN1:UTF8:string:{device.mac_address}", - # "critical": False - # }, - # { - # "oid": uuid_oid, - # "value": f"ASN1:UTF8:string:{device.id}", - # "critical": False - # } - # ]) + mac_oid = "1.3.6.1.4.1.65901.1" + uuid_oid = "1.3.6.1.4.1.65901.2" + extensions.extend( + [ + { + "oid": mac_oid, + "value": f"ASN1:UTF8:string:{device.mac_address}", + "critical": False, + }, + { + "oid": uuid_oid, + "value": f"ASN1:UTF8:string:{device.id}", + "critical": False, + }, + ] + ) cert = cert_model( name=name, ca=ca, diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index c9ed89710..a465a1bcf 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1262,33 +1262,30 @@ def test_cert_generation_and_revocation_lifecycle(self): ) ) - # uncomment after #228 in django-x509 - # with self.subTest("Injects custom MAC and UUID OIDs"): - # extensions = generated_cert.extensions - # mac_oid = "1.3.6.1.4.1.65901.1" - # uuid_oid = "1.3.6.1.4.1.65901.2" - # mac_ext = next( - # ( - # ext - # for ext in extensions - # if ext.get("oid") == mac_oid - # or ext.get("name") == mac_oid - # ), - # None, - # ) - # uuid_ext = next( - # ( - # ext - # for ext in extensions - # if ext.get("oid") == uuid_oid - # or ext.get("name") == uuid_oid - # ), - # None, - # ) - # self.assertIsNotNone(mac_ext, "MAC OID extension missing") - # self.assertIn(device.mac_address, mac_ext["value"]) - # self.assertIsNotNone(uuid_ext, "UUID OID extension missing") - # self.assertIn(str(device.id), uuid_ext["value"]) + with self.subTest("Injects custom MAC and UUID OIDs"): + extensions = generated_cert.extensions + mac_oid = "1.3.6.1.4.1.65901.1" + uuid_oid = "1.3.6.1.4.1.65901.2" + mac_ext = next( + ( + ext + for ext in extensions + if ext.get("oid") == mac_oid or ext.get("name") == mac_oid + ), + None, + ) + uuid_ext = next( + ( + ext + for ext in extensions + if ext.get("oid") == uuid_oid or ext.get("name") == uuid_oid + ), + None, + ) + self.assertIsNotNone(mac_ext, "MAC OID extension missing") + self.assertIn(device.mac_address, mac_ext["value"]) + self.assertIsNotNone(uuid_ext, "UUID OID extension missing") + self.assertIn(str(device.id), uuid_ext["value"]) with self.subTest("Secure Revocation (post_remove)"): cert_pk = generated_cert.pk From dfa650c96ad020981b89a666ff07585ff91343ca Mon Sep 17 00:00:00 2001 From: stktyagi Date: Fri, 5 Jun 2026 20:39:25 +0530 Subject: [PATCH 23/34] [docs] Added inital documentation Added inital intro commit for starting docs --- docs/user/certificate-templates.rst | 163 ++++++++++++++++++++++++++++ docs/user/templates.rst | 17 +++ 2 files changed, 180 insertions(+) create mode 100644 docs/user/certificate-templates.rst diff --git a/docs/user/certificate-templates.rst b/docs/user/certificate-templates.rst new file mode 100644 index 000000000..1b45647bc --- /dev/null +++ b/docs/user/certificate-templates.rst @@ -0,0 +1,163 @@ +X.509 Certificate Generator Templates +===================================== + +.. contents:: **Table of Contents**: + :depth: 3 + :local: + +Introduction +------------ + +A Certificate Template is a specific type of :doc:`Configuration Template +` that allows OpenWISP to centrally manage and +automatically provision X.509 client certificates for devices. + +Unlike VPN templates, which generate certificates as part of a larger +tunnel configuration (like OpenVPN or WireGuard), Certificate Templates +are standalone. They are ideal for use cases where devices need +cryptographic identities for external services, such as: + +- Mutual TLS (mTLS) authentication against internal APIs. +- Cryptographically signed device identities for 802.1x or captive + portals. +- Secure payload signing. + +.. _certificate_templates_setup: + +Setting Up a Certificate Template +--------------------------------- + +To create a Certificate Template, navigate to the Templates section in the +OpenWISP admin and set the **Type** to :guilabel:`Certificate` (``cert``). +This will reveal the certificate-specific configuration fields. + +.. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/certificate-template.png + :target: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/certificate-template.png + :alt: Certificate Template admin form + +:guilabel:`Certificate Authority` (required) + The Certificate Authority that will sign the X.509 certificates + generated for each device using the template. The CA must belong to + the same organization as the template (or be shared). + +:guilabel:`Blueprint Certificate` (optional) + An existing, unassigned, non-revoked X.509 certificate that will be + used as a *blueprint*. The extensions, key length, digest, and other + subject fields of the blueprint are copied to each newly generated + certificate. The blueprint must be signed by the selected + :guilabel:`Certificate Authority` and must not already be bound to a + device. + +:guilabel:`Automatic tunnel provisioning` (``auto_cert``) + When enabled (which is the default behavior), an X.509 certificate is + automatically created and signed by the template's CA the moment the + template is assigned to a device configuration. + +.. _certificate_templates_validation: + +Validation Rules +---------------- + +To ensure cryptographic integrity and prevent misconfigurations, +Certificate Templates enforce the following validation rules upon saving: + +- A :guilabel:`Certificate Authority` is strictly required. +- The :guilabel:`Blueprint Certificate` **must** be signed by the selected + :guilabel:`Certificate Authority`. Saving with a mismatched pair will + fail. +- The :guilabel:`Blueprint Certificate` must be *unassigned* (it cannot be + currently bound to any device). The blueprint dropdown in the admin + panel is pre-filtered to show only unassigned, non-revoked certificates. +- Both the :guilabel:`Certificate Authority` and the :guilabel:`Blueprint + Certificate` must belong to the same organization as the template, or be + marked as shared. + +.. _certificate_templates_lifecycle: + +Provisioning and Revocation Lifecycle +------------------------------------- + +Certificate Templates are tightly coupled with the device configuration +lifecycle to ensure certificates are only valid while the device is +actively authorized to use them. + +**When assigned to a device:** When a Certificate Template is added to a +device configuration, a ``DeviceCertificate`` relationship is established. +If ``auto_cert`` is enabled, an X.509 certificate is generated and signed +in the same database transaction: + +- The certificate's subject and extensions are copied from the + :guilabel:`Blueprint Certificate` (or the CA defaults). +- The certificate is augmented with two custom OpenWISP OIDs that + cryptographically identify the device (see + :ref:`certificate_templates_oid_extensions`). +- The certificate's :guilabel:`Common Name` is generated using the + :ref:`OPENWISP_CONTROLLER_COMMON_NAME_FORMAT + ` setting, suffixed with a + unique slug to prevent collisions. + +**When removed from a device:** When the Certificate Template is +unassigned from a device configuration, the ``DeviceCertificate`` +relationship is deleted. Crucially, the underlying X.509 certificate is +**automatically revoked** (provided it was created via +``auto_cert=True``). This ensures that compromised or decommissioned +devices immediately lose their cryptographic access. + +.. _certificate_templates_active_lock: + +Active Template Mutation Lock +----------------------------- + +To prevent breaking the cryptographic binding with devices that are +already using a template, certain destructive changes are blocked while +the template is assigned to *active* or *activating* device +configurations. + +You **cannot** change the following on an actively used template: - +:guilabel:`Type` (only changing a ``cert`` template to a different type is +blocked) - :guilabel:`Certificate Authority` - :guilabel:`Blueprint +Certificate` + +If you need to update these core parameters, you must first unassign the +Certificate Template from all affected device configurations (or +deactivate the devices), apply your template changes, and then reassign +them. + +.. _certificate_templates_oid_extensions: + +Custom Device Identification OIDs +--------------------------------- + +To allow external systems to uniquely and securely identify devices by +parsing their X.509 certificates, every automatically generated +certificate includes two custom ASN.1 Object Identifiers (OIDs): + +- ``1.3.6.1.4.1.65901.1``: Contains the MAC address of the device + (``ASN1:UTF8:string:``). +- ``1.3.6.1.4.1.65901.2``: Contains the UUID of the device + (``ASN1:UTF8:string:``). + +These OIDs are appended securely to the certificate in addition to any +extensions inherited from the :guilabel:`Blueprint Certificate`. + +.. _certificate_templates_api: + +Certificate Templates via the REST API +-------------------------------------- + +The Certificate Template architecture is fully supported by the +:doc:`OpenWISP REST API `: + +- The ``type`` field accepts the ``cert`` enumeration. +- ``ca`` and ``blueprint_cert`` are writable fields on the + ``/api/v1/controller/template/`` endpoint. +- The ``blueprint_cert`` field is strictly filtered. Attempting to pass + the UUID of an already-assigned or revoked certificate will return a + ``400 Bad Request``. +- The Active Mutation Lock is enforced at the serializer level: attempting + to patch the ``type``, ``ca``, or ``blueprint_cert`` of an actively + deployed template will return a ``400 Bad Request``. + +Additionally, you can trigger the automated creation and revocation +lifecycle by patching the ``config.templates`` array on the :ref:`Device +endpoint `. diff --git a/docs/user/templates.rst b/docs/user/templates.rst index a58530b1b..a1029a982 100644 --- a/docs/user/templates.rst +++ b/docs/user/templates.rst @@ -209,3 +209,20 @@ engine: netjsonconfig. For more advanced technical information about templates, consult the netjsonconfig documentation: `Basic Concepts, Template `_. + +.. _certificate_templates: + +Certificate Templates +--------------------- + +A Certificate Template is a :doc:`Template ` +whose **Type** is set to :guilabel:`Certificate` (``cert``). + +It allows declaring the *Certificate Authority* and an optional *Blueprint +Certificate* that will be used to issue an X.509 certificate for each +device the template is assigned to, without needing a VPN backend. + +Certificate Templates are useful for any use case in which a fleet of +devices needs an X.509 client certificate managed centrally by OpenWISP, +for example: mutual TLS authentication against an internal service, signed +device identities for captive portals, etc. From 88003e207381aa865618603cba571278cd7c10a6 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sat, 6 Jun 2026 21:40:41 +0530 Subject: [PATCH 24/34] [feature] Added implementation for context injection #1360 Added tests and implementation for context injection Fixes #1360 --- openwisp_controller/config/base/config.py | 28 ++++++++++++++ .../config/tests/test_template.py | 37 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index aa57060d4..537ce24cb 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -973,6 +973,33 @@ def get_vpn_context(self): context[vpn_context_keys["secret"]] = vpnclient.secret return context + def get_cert_context(self): + """ + Retrieves standalone certificates generated by Certificate Templates + and exposes them as UUID-namespaced variables for the configuration engine. + """ + cert_context = collections.OrderedDict() + for dc in self.devicecertificate_set.select_related( + "template", "cert" + ).order_by("created"): + if dc.cert: + template_hex = dc.template_id.hex + prefix = f"cert_{template_hex}" + cert_filename = "cert-{0}.pem".format(template_hex) + cert_path = "{0}/{1}".format(app_settings.CERT_PATH, cert_filename) + key_filename = "key-{0}.pem".format(template_hex) + key_path = "{0}/{1}".format(app_settings.CERT_PATH, key_filename) + cert_context.update( + { + f"{prefix}_path": cert_path, + f"{prefix}_pem": dc.cert.certificate, + f"{prefix}_key_path": key_path, + f"{prefix}_key": dc.cert.private_key, + f"{prefix}_uuid": str(dc.cert.id), + } + ) + return cert_context + def get_context(self, system=False): """ additional context passed to netjsonconfig @@ -1000,6 +1027,7 @@ def get_context(self, system=False): context.update(self.device._get_group().get_context()) # Add predefined variables context.update(self.get_vpn_context()) + context.update(self.get_cert_context()) for func in self._config_context_functions: context.update(func(config=self)) if app_settings.HARDWARE_ID_ENABLED: diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index a465a1bcf..b8fd58692 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1426,3 +1426,40 @@ def test_get_unassigned_certs_with_null_device_cert(self): queryset = choices.get("pk__in") self.assertIsNotNone(queryset) self.assertIn(unassigned_cert, queryset) + + def test_certificate_template_context_injection(self): + """ + Verify that Certificate Templates automatically inject their + file paths, UUIDs, and PEM payloads into the configuration context. + """ + org = self._create_org() + ca = self._create_ca(organization=org) + template = self._create_template( + name="context-injection-test", + type="cert", + ca=ca, + auto_cert=True, + organization=org, + ) + device = self._create_device(organization=org) + config = self._create_config(device=device) + config.templates.add(template) + context = config.get_context() + prefix = f"cert_{template.pk.hex}" + device_cert = config.devicecertificate_set.first() + self.assertIsNotNone(device_cert, "DeviceCertificate was not created") + cert = device_cert.cert + self.assertIn(f"{prefix}_path", context) + self.assertIn(f"{prefix}_key_path", context) + self.assertTrue( + context[f"{prefix}_path"].endswith(f"cert-{template.pk.hex}.pem") + ) + self.assertTrue( + context[f"{prefix}_key_path"].endswith(f"key-{template.pk.hex}.pem") + ) + self.assertIn(f"{prefix}_uuid", context) + self.assertEqual(context[f"{prefix}_uuid"], str(cert.id)) + self.assertIn(f"{prefix}_pem", context) + self.assertIn(f"{prefix}_key", context) + self.assertEqual(context[f"{prefix}_pem"], cert.certificate) + self.assertEqual(context[f"{prefix}_key"], cert.private_key) From b9db466e4a9e92272fdfe368ea3769ee78ca2416 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sun, 7 Jun 2026 16:53:46 +0530 Subject: [PATCH 25/34] [tests] Updated tests to pass CI Updated tests with increased number of queries to pass CI --- .../config/static/config/js/switcher.js | 23 ++++++++++++++---- .../config/static/config/js/template_ui.js | 24 ------------------- .../config/tests/test_admin.py | 2 +- openwisp_controller/config/tests/test_api.py | 2 +- .../config/tests/test_config.py | 4 ++-- .../config/tests/test_template.py | 2 +- 6 files changed, 23 insertions(+), 34 deletions(-) delete mode 100644 openwisp_controller/config/static/config/js/template_ui.js diff --git a/openwisp_controller/config/static/config/js/switcher.js b/openwisp_controller/config/static/config/js/switcher.js index c46acaae3..0db54367b 100644 --- a/openwisp_controller/config/static/config/js/switcher.js +++ b/openwisp_controller/config/static/config/js/switcher.js @@ -1,14 +1,17 @@ "use strict"; django.jQuery(function ($) { var type_select = $("#id_type"), - vpn_specific = $(".field-vpn, .field-auto_cert"), + vpn_specific = $(".field-vpn"), + cert_specific = $(".field-ca, .field-blueprint_cert"), + auto_cert_field = $(".field-auto_cert"), gettext = window.gettext || function (v) { return v; }, - toggle_vpn_specific = function (changed) { - if (type_select.val() == "vpn") { + toggle_specific_fields = function (changed) { + var val = type_select.val(); + if (val === "vpn") { vpn_specific.show(); if ( changed === true && @@ -35,9 +38,19 @@ django.jQuery(function ($) { $(".autovpn").hide(); } } + if (val === "cert") { + cert_specific.show(); + } else { + cert_specific.hide(); + } + if (val === "vpn" || val === "cert") { + auto_cert_field.show(); + } else { + auto_cert_field.hide(); + } }; type_select.on("change", function () { - toggle_vpn_specific(true); + toggle_specific_fields(true); }); - toggle_vpn_specific(); + toggle_specific_fields(); }); diff --git a/openwisp_controller/config/static/config/js/template_ui.js b/openwisp_controller/config/static/config/js/template_ui.js deleted file mode 100644 index c22e9f51b..000000000 --- a/openwisp_controller/config/static/config/js/template_ui.js +++ /dev/null @@ -1,24 +0,0 @@ -// Created to show or hide the ca and blueprint_cert fields -// based on the selected template type. -django.jQuery(function ($) { - var typeField = $("#id_type"), - caField = $(".field-ca"), - blueprintField = $(".field-blueprint_cert"); - - // Only the Template admin form has the type selector; bail out elsewhere - // (e.g. VpnAdmin) to avoid hiding its unrelated `.field-ca` row. - if (!typeField.length) { - return; - } - function toggleCertFields() { - if (typeField.val() === "cert") { - caField.show(); - blueprintField.show(); - } else { - caField.hide(); - blueprintField.hide(); - } - } - typeField.on("change", toggleCertFields); - toggleCertFields(); -}); diff --git a/openwisp_controller/config/tests/test_admin.py b/openwisp_controller/config/tests/test_admin.py index 90bca8a8a..a287230da 100644 --- a/openwisp_controller/config/tests/test_admin.py +++ b/openwisp_controller/config/tests/test_admin.py @@ -2332,7 +2332,7 @@ def _verify_template_queries(self, config, count): path = reverse(f"admin:{self.app_label}_device_change", args=[config.device.pk]) for i in range(count): self._create_template(name=f"template-{i}") - expected_count = 22 + expected_count = 23 if django.VERSION < (5, 2): # In django version < 5.2, there is an extra SAVEPOINT query # leading to extra RELEASE SAVEPOINT query, thus 2 extra queries diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index bceb6ddff..764d18a31 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -554,7 +554,7 @@ def test_device_download_api(self): d1 = self._create_device() self._create_config(device=d1) path = reverse("config_api:download_device_config", args=[d1.pk]) - with self.assertNumQueries(7): + with self.assertNumQueries(8): r = self.client.get(path) self.assertEqual(r.status_code, 200) diff --git a/openwisp_controller/config/tests/test_config.py b/openwisp_controller/config/tests/test_config.py index cda1ad2f9..cdae519d2 100644 --- a/openwisp_controller/config/tests/test_config.py +++ b/openwisp_controller/config/tests/test_config.py @@ -861,13 +861,13 @@ def test_config_modified_sent(self): def test_check_changes_query(self): config = self._create_config(organization=self._get_org()) with self.subTest("No changes made to the config object"): - with self.assertNumQueries(3): + with self.assertNumQueries(4): config._check_changes() with self.subTest("Changes made to the config object"): config.templates.add(self._create_template()) config.config = {"general": {"description": "test"}} - with self.assertNumQueries(4): + with self.assertNumQueries(5): config._check_changes() def test_config_get_system_context(self): diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index b8fd58692..1b54d0e3c 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -588,7 +588,7 @@ def test_config_status_modified_after_change(self): with catch_signal(config_status_changed) as handler: t.config["interfaces"][0]["name"] = "eth2" t.full_clean() - with self.assertNumQueries(13): + with self.assertNumQueries(15): t.save() c.refresh_from_db() handler.assert_not_called() From 28f9cce48c0a67db3bcb1c5a2e6d568b0029cd38 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sun, 7 Jun 2026 18:12:36 +0530 Subject: [PATCH 26/34] [fix] Addressed review changes Added dynamic JS logic in 'switcher.js' to change the auto-cert label text between 'Automatic tunnel provisioning' and 'Automatic certificate provisioning' --- docs/user/certificate-templates.rst | 12 ++++---- openwisp_controller/config/api/serializers.py | 29 ++++++++++++------- .../config/base/device_certificate.py | 1 + .../config/static/config/js/switcher.js | 12 ++++++++ .../config/tests/test_template.py | 10 ++++++- 5 files changed, 47 insertions(+), 17 deletions(-) diff --git a/docs/user/certificate-templates.rst b/docs/user/certificate-templates.rst index 1b45647bc..2a2fd9dbe 100644 --- a/docs/user/certificate-templates.rst +++ b/docs/user/certificate-templates.rst @@ -48,7 +48,7 @@ This will reveal the certificate-specific configuration fields. :guilabel:`Certificate Authority` and must not already be bound to a device. -:guilabel:`Automatic tunnel provisioning` (``auto_cert``) +:guilabel:`Automatic certificate provisioning` (``auto_cert``) When enabled (which is the default behavior), an X.509 certificate is automatically created and signed by the template's CA the moment the template is assigned to a device configuration. @@ -113,10 +113,12 @@ already using a template, certain destructive changes are blocked while the template is assigned to *active* or *activating* device configurations. -You **cannot** change the following on an actively used template: - -:guilabel:`Type` (only changing a ``cert`` template to a different type is -blocked) - :guilabel:`Certificate Authority` - :guilabel:`Blueprint -Certificate` +You **cannot** change the following on an actively used template: + +- :guilabel:`Type` (only changing a ``cert`` template to a different type + is blocked) +- :guilabel:`Certificate Authority` +- :guilabel:`Blueprint Certificate` If you need to update these core parameters, you must first unassign the Certificate Template from all affected device configurations (or diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index d7f46619e..b2afc856e 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -75,7 +75,9 @@ def validate_config(self, value): """ Display appropriate field name. """ - template_type = self.initial_data.get("type") + template_type = self.initial_data.get( + "type", getattr(self.instance, "type", None) + ) if template_type == "generic" and value == {}: raise serializers.ValidationError( _("The configuration field cannot be empty.") @@ -105,9 +107,11 @@ def validate(self, data): elif template_type != "cert": data["ca"] = None data["blueprint_cert"] = None + ca = None + blueprint_cert = None # assert structural binding matches between CA and template blueprints - if blueprint_cert and ca: + if template_type == "cert" and blueprint_cert and ca: if blueprint_cert.ca_id != ca.id: raise serializers.ValidationError( { @@ -435,15 +439,18 @@ def update(self, instance, validated_data): # The value of the organization field is set here to # prevent access of the old value stored in the database # while performing above operations. - instance.config.device.organization = validated_data.get("organization") - instance.config.templates.clear() - Config.enforce_required_templates( - action="post_clear", - instance=instance.config, - sender=instance.config.templates, - pk_set=None, - raw_data=raw_data_for_signal_handlers, - ) + with transaction.atomic(): + instance.config.device.organization = validated_data.get( + "organization" + ) + instance.config.templates.clear() + Config.enforce_required_templates( + action="post_clear", + instance=instance.config, + sender=instance.config.templates, + pk_set=None, + raw_data=raw_data_for_signal_handlers, + ) return super().update(instance, validated_data) diff --git a/openwisp_controller/config/base/device_certificate.py b/openwisp_controller/config/base/device_certificate.py index b51dcfb37..00bc6d1b2 100644 --- a/openwisp_controller/config/base/device_certificate.py +++ b/openwisp_controller/config/base/device_certificate.py @@ -56,6 +56,7 @@ def save(self, *args, **kwargs): with transaction.atomic(): if self.auto_cert and not self.cert: self._auto_x509() + self.full_clean(validate_unique=False) super().save(*args, **kwargs) def _auto_x509(self): diff --git a/openwisp_controller/config/static/config/js/switcher.js b/openwisp_controller/config/static/config/js/switcher.js index 0db54367b..7a409fc8f 100644 --- a/openwisp_controller/config/static/config/js/switcher.js +++ b/openwisp_controller/config/static/config/js/switcher.js @@ -4,6 +4,7 @@ django.jQuery(function ($) { vpn_specific = $(".field-vpn"), cert_specific = $(".field-ca, .field-blueprint_cert"), auto_cert_field = $(".field-auto_cert"), + auto_cert_label = $("label[for='id_auto_cert']"), gettext = window.gettext || function (v) { @@ -48,6 +49,17 @@ django.jQuery(function ($) { } else { auto_cert_field.hide(); } + if (val === "vpn" || val === "cert") { + auto_cert_field.show(); + + if (val === "vpn") { + auto_cert_label.text(gettext("Automatic tunnel provisioning")); + } else if (val === "cert") { + auto_cert_label.text(gettext("Automatic certificate provisioning")); + } + } else { + auto_cert_field.hide(); + } }; type_select.on("change", function () { toggle_specific_fields(true); diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index 1b54d0e3c..66d58ee0c 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1392,7 +1392,10 @@ def test_cert_template_partial_replacement_cleans_up(self): config.devicecertificate_set.count(), 2, "Two certs should be created" ) kept_cert_id = config.devicecertificate_set.get(template=cert_template_1).pk - removed_cert_id = config.devicecertificate_set.get(template=cert_template_2).pk + removed_dc = config.devicecertificate_set.get(template=cert_template_2) + removed_cert_id = removed_dc.pk + removed_cert_pk = removed_dc.cert_id + removed_cert = Cert.objects.get(pk=removed_cert_pk) config.templates.set([cert_template_1], clear=True) self.assertEqual( config.devicecertificate_set.count(), 1, "Only one cert should remain" @@ -1405,6 +1408,11 @@ def test_cert_template_partial_replacement_cleans_up(self): config.devicecertificate_set.filter(pk=removed_cert_id).exists(), "The removed template's certificate should have been deleted.", ) + removed_cert.refresh_from_db() + self.assertTrue( + removed_cert.revoked, + "The removed template's underlying certificate should be revoked.", + ) def test_get_unassigned_certs_with_null_device_cert(self): """ From 1e9e81393b6d0e7db1055920c1f1d3fbfb1f2652 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Tue, 9 Jun 2026 19:41:28 +0530 Subject: [PATCH 27/34] [docs] Added docs for context injection #1360 Added documentation for context injection Fixes #1360 --- docs/user/certificate-templates.rst | 54 +++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/docs/user/certificate-templates.rst b/docs/user/certificate-templates.rst index 2a2fd9dbe..ead354d58 100644 --- a/docs/user/certificate-templates.rst +++ b/docs/user/certificate-templates.rst @@ -142,6 +142,60 @@ certificate includes two custom ASN.1 Object Identifiers (OIDs): These OIDs are appended securely to the certificate in addition to any extensions inherited from the :guilabel:`Blueprint Certificate`. +.. _certificate_templates_context: + +Using Certificates in Configuration (Context Injection) +------------------------------------------------------- + +To deploy the certificate to the device, administrators must reference the +automatically generated variables within the template's JSON configuration +payload. + +To prevent variable collisions when multiple Certificate Templates are +assigned to a single device, OpenWISP namespaces the Jinja2 variables +using the template's 32-character hexadecimal UUID (the UUID with dashes +removed). + +The following variables are dynamically injected into the configuration +context: + +- ``{{ cert__pem }}``: The public certificate. +- ``{{ cert__key }}``: The private key. +- ``{{ cert__uuid }}``: The UUID of the + ``DeviceCertificate`` object. + +**Workflow Example:** + +1. Create a Certificate Template and click **Save and continue editing** + to generate its UUID. +2. Note the UUID from the URL (e.g., + ``d7d20eb8-b3c8-420c-9103-401e7960cfb3``). +3. Strip the dashes to get the hex string + (``d7d20eb8b3c8420c9103401e7960cfb3``). +4. Add the following JSON payload to write the certificate to the device's + filesystem: + +.. code-block:: json + + { + "files": [ + { + "path": "/etc/ssl/certs/device-cert.pem", + "mode": "0600", + "contents": "{{ cert_d7d20eb8b3c8420c9103401e7960cfb3_pem }}" + }, + { + "path": "/etc/ssl/private/device-key.pem", + "mode": "0600", + "contents": "{{ cert_d7d20eb8b3c8420c9103401e7960cfb3_key }}" + } + ] + } + +When this template is assigned to a device, OpenWISP will automatically +replace the variables with the exact, unique X.509 certificate and private +key generated for that specific device. + .. _certificate_templates_api: Certificate Templates via the REST API From 717c3e174f07e6190ddff8b8265d177f0492031a Mon Sep 17 00:00:00 2001 From: stktyagi Date: Fri, 12 Jun 2026 21:49:49 +0530 Subject: [PATCH 28/34] [feature] Implemented certificate regeneration #1359 Implemented certificate regeneration if device property change. Fixes #1359 --- openwisp_controller/config/handlers.py | 43 ++++++ openwisp_controller/config/settings.py | 3 + openwisp_controller/config/tasks.py | 90 +++++++++++ .../config/tests/test_device.py | 141 ++++++++++++++++++ 4 files changed, 277 insertions(+) diff --git a/openwisp_controller/config/handlers.py b/openwisp_controller/config/handlers.py index d838d1ad3..b78e4d2e6 100644 --- a/openwisp_controller/config/handlers.py +++ b/openwisp_controller/config/handlers.py @@ -1,4 +1,5 @@ from django.db import transaction +from django.db.models.signals import post_save, pre_save from django.dispatch import receiver from django.utils.translation import gettext_lazy as _ from openwisp_notifications.signals import notify @@ -6,6 +7,7 @@ from openwisp_controller.config.controller.views import DeviceChecksumView +from . import settings as app_settings from . import tasks from .signals import config_status_changed, device_registered @@ -43,6 +45,47 @@ def device_registered_notification(sender, instance, is_new, **kwargs): ) +@receiver(pre_save, sender=Device, dispatch_uid="capture_old_hardware_properties") +def capture_old_hardware_properties(sender, instance, **kwargs): + """ + Temporarily caches the old name and mac_address before saving + to detect hardware drift in post_save. + """ + if not instance.pk: + return + update_fields = kwargs.get("update_fields") + if update_fields is not None: + if "name" not in update_fields and "mac_address" not in update_fields: + return + try: + old_instance = sender.objects.only("name", "mac_address").get(pk=instance.pk) + instance._old_name = old_instance.name + instance._old_mac = old_instance.mac_address + except sender.DoesNotExist: + pass + + +@receiver(post_save, sender=Device, dispatch_uid="detect_hardware_drift") +def detect_hardware_drift(sender, instance, created, **kwargs): + """ + Triggers certificate regeneration if hardware properties (name/mac) change. + """ + if created or not app_settings.REGENERATE_CERTS_ON_HARDWARE_CHANGE: + return + update_fields = kwargs.get("update_fields") + if update_fields is not None: + if "name" not in update_fields and "mac_address" not in update_fields: + return + name_changed = getattr(instance, "_old_name", instance.name) != instance.name + mac_changed = ( + getattr(instance, "_old_mac", instance.mac_address) != instance.mac_address + ) + if name_changed or mac_changed: + transaction.on_commit( + lambda: tasks.regenerate_device_certificates_task.delay(str(instance.id)) + ) + + def devicegroup_change_handler(instance, **kwargs): if type(instance) is list: # changes group templates for multiple devices diff --git a/openwisp_controller/config/settings.py b/openwisp_controller/config/settings.py index 55ad86ce3..57b2bf219 100644 --- a/openwisp_controller/config/settings.py +++ b/openwisp_controller/config/settings.py @@ -34,6 +34,9 @@ CONTEXT = get_setting("CONTEXT", {}) assert isinstance(CONTEXT, dict), "OPENWISP_CONTROLLER_CONTEXT must be a dictionary" DEFAULT_AUTO_CERT = get_setting("DEFAULT_AUTO_CERT", True) +REGENERATE_CERTS_ON_HARDWARE_CHANGE = get_setting( + "REGENERATE_CERTS_ON_HARDWARE_CHANGE", True +) CERT_PATH = get_setting("CERT_PATH", "/etc/x509") COMMON_NAME_FORMAT = get_setting("COMMON_NAME_FORMAT", "{mac_address}-{name}") MANAGEMENT_IP_DEVICE_LIST = get_setting("MANAGEMENT_IP_DEVICE_LIST", True) diff --git a/openwisp_controller/config/tasks.py b/openwisp_controller/config/tasks.py index 798e40f8c..381bb41db 100644 --- a/openwisp_controller/config/tasks.py +++ b/openwisp_controller/config/tasks.py @@ -1,3 +1,4 @@ +import copy import logging import requests @@ -5,10 +6,14 @@ from celery.exceptions import SoftTimeLimitExceeded from django.conf import settings from django.core.exceptions import ObjectDoesNotExist +from django.db import transaction +from django.utils.translation import gettext_lazy as _ +from openwisp_notifications.signals import notify from swapper import load_model from openwisp_utils.tasks import OpenwispCeleryTask +from . import settings as app_settings from .utils import handle_error_notification, handle_recovery_notification logger = logging.getLogger(__name__) @@ -217,3 +222,88 @@ def invalidate_controller_views_cache(organization_id): Vpn.objects.filter(organization_id=organization_id).only("id").iterator() ): GetVpnView.invalidate_get_vpn_cache(vpn) + + +@shared_task(soft_time_limit=1200) +def regenerate_device_certificates_task(device_id): + """ + Revokes stale certificates and mints fresh ones when hardware drift occurs. + """ + if not app_settings.REGENERATE_CERTS_ON_HARDWARE_CHANGE: + return + Device = load_model("config", "Device") + DeviceCertificate = load_model("config", "DeviceCertificate") + Cert = load_model("django_x509", "Cert") + try: + device = Device.objects.get(id=device_id) + except Device.DoesNotExist: + return + active_device_certs = DeviceCertificate.objects.filter( + config__device=device, + auto_cert=True, + cert__revoked=False, + template__type="cert", + ).select_related("cert", "config", "template") + if not active_device_certs.exists(): + return + configs_to_update = set() + certs_regenerated = 0 + + with transaction.atomic(): + for dc in active_device_certs: + old_cert = dc.cert + old_cert.revoke() + blueprint = dc.template.blueprint_cert + if blueprint and blueprint.extensions: + extensions = copy.deepcopy(blueprint.extensions) + else: + extensions = [ + {"name": "nsCertType", "value": "client", "critical": False} + ] + extensions.extend( + [ + { + "oid": "1.3.6.1.4.1.65901.1", + "value": f"ASN1:UTF8:string:{device.mac_address}", + "critical": False, + }, + { + "oid": "1.3.6.1.4.1.65901.2", + "value": f"ASN1:UTF8:string:{device.id}", + "critical": False, + }, + ] + ) + new_cert = Cert( + name=device.name, + ca=old_cert.ca, + organization=device.organization, + common_name=dc._get_common_name(), + extensions=extensions, + ) + new_cert.full_clean() + new_cert.save() + dc.cert = new_cert + dc.save() + configs_to_update.add(dc.config) + certs_regenerated += 1 + for config in configs_to_update: + config.update_status_if_checksum_changed() + try: + message = _( + "Hardware drift detected on device {device_name}. " + "Successfully regenerated {certs_regenerated} bound X.509 certificate(s)." + ).format(device_name=str(device.name), certs_regenerated=certs_regenerated) + notify.send( + sender=device, + target=device, + action_object=device, + type="generic_message", + verb=_("experienced hardware drift"), + message=message, + level="info", + ) + except (ImportError, Exception) as e: + logger.warning( + f"Could not push regeneration notification for {device.name}: {e}" + ) diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index 1e2bd4a3e..a29d559da 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -5,6 +5,7 @@ from django.test import TestCase, TransactionTestCase from swapper import load_model +from openwisp_controller.config.tasks import regenerate_device_certificates_task from openwisp_utils.tests import AssertNumQueriesSubTestMixin, catch_signal from .. import settings as app_settings @@ -25,6 +26,9 @@ Config = load_model("config", "Config") Device = load_model("config", "Device") DeviceGroup = load_model("config", "DeviceGroup") +DeviceCertificate = load_model("config", "DeviceCertificate") +Cert = load_model("django_x509", "Cert") +Ca = load_model("django_x509", "Ca") OrganizationConfigSettings = load_model("config", "OrganizationConfigSettings") _original_context = app_settings.CONTEXT.copy() @@ -653,6 +657,143 @@ def test_create_default_config_existing(self): self.assertEqual(device.config.context, {"ssid": "test"}) self.assertEqual(device.config.config, {"general": {}}) + @mock.patch( + "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" + ) + def test_hardware_drift_signal_triggers_on_name_change(self, mocked_task): + """Proof that changing the hostname fires the celery task.""" + org = self._create_org() + device = self._create_device(organization=org, name="old-router-name") + device.name = "new-router-name" + with self.captureOnCommitCallbacks(execute=True): + device.save() + mocked_task.assert_called_once_with(str(device.id)) + + @mock.patch( + "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" + ) + def test_hardware_drift_signal_triggers_on_mac_change(self, mocked_task): + """Proof that changing the MAC address fires the celery task.""" + org = self._create_org() + device = self._create_device(organization=org, mac_address="00:11:22:33:44:55") + device.mac_address = "AA:BB:CC:DD:EE:FF" + with self.captureOnCommitCallbacks(execute=True): + device.save() + mocked_task.assert_called_once_with(str(device.id)) + + @mock.patch( + "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" + ) + def test_hardware_drift_signal_ignores_unrelated_changes(self, mocked_task): + """Proof that saving a device without changing name/MAC does nothing.""" + org = self._create_org() + device = self._create_device(organization=org) + device.key = "new-management-key" + device.save() + mocked_task.assert_not_called() + + @mock.patch( + "openwisp_controller.config.settings.REGENERATE_CERTS_ON_HARDWARE_CHANGE", False + ) + @mock.patch( + "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" + ) + def test_hardware_drift_setting_disables_regeneration(self, mocked_task): + """Proof that the user-configurable setting turns the feature off.""" + org = self._create_org() + device = self._create_device(organization=org) + device.name = "another-new-name" + device.save() + mocked_task.assert_not_called() + + @mock.patch( + "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" + ) + def test_task_regenerates_and_revokes(self, mocked_task): + """Proof that the task physically revokes the old and mints the new.""" + org = self._create_org() + device = self._create_device( + organization=org, name="old-router-name", mac_address="00:11:22:33:44:55" + ) + ca = Ca.objects.create(name="test-ca", organization=org) + template = self._create_template( + organization=org, type="cert", ca=ca, auto_cert=True + ) + config = self._create_config(device=device) + config.templates.add(template) + device_cert = DeviceCertificate.objects.get(config=config, template=template) + old_cert_id = device_cert.cert.id + self.assertFalse(device_cert.cert.revoked) + device.name = "renamed-router" + device.mac_address = "AA:BB:CC:DD:EE:FF" + device.save() + regenerate_device_certificates_task(str(device.id)) + device_cert.refresh_from_db() + old_cert = Cert.objects.get(id=old_cert_id) + self.assertTrue(old_cert.revoked) + self.assertNotEqual(old_cert_id, device_cert.cert.id) + self.assertIn("renamed-router", device_cert.cert.common_name) + self.assertIn("AA:BB:CC:DD:EE:FF", device_cert.cert.common_name) + extensions = device_cert.cert.extensions + mac_ext = next( + (ext for ext in extensions if ext.get("oid") == "1.3.6.1.4.1.65901.1"), None + ) + uuid_ext = next( + (ext for ext in extensions if ext.get("oid") == "1.3.6.1.4.1.65901.2"), None + ) + self.assertIsNotNone(mac_ext, "MAC Address OID 1.3.6.1.4.1.65901.1 is missing") + self.assertIn("AA:BB:CC:DD:EE:FF", mac_ext["value"]) + self.assertIsNotNone(uuid_ext, "Device UUID OID 1.3.6.1.4.1.65901.2 is missing") + self.assertIn(str(device.id), uuid_ext["value"]) + + @mock.patch( + "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" + ) + def test_regeneration_preserves_blueprint_extensions(self, mocked_task): + """Proof that blueprint extensions carry over alongside custom OIDs.""" + org = self._create_org() + device = self._create_device( + organization=org, name="old-router-name", mac_address="00:11:22:33:44:55" + ) + ca = Ca.objects.create(name="test-ca", organization=org) + blueprint_cert = Cert( + name="enterprise-blueprint", + ca=ca, + organization=org, + common_name="blueprint", + extensions=[{"name": "nsCertType", "value": "server", "critical": False}], + ) + blueprint_cert.full_clean() + blueprint_cert.save() + template = self._create_template( + organization=org, + type="cert", + ca=ca, + auto_cert=True, + blueprint_cert=blueprint_cert, + ) + config = self._create_config(device=device) + config.templates.add(template) + device_cert = DeviceCertificate.objects.get(config=config, template=template) + device.mac_address = "AA:BB:CC:DD:EE:FF" + device.save() + regenerate_device_certificates_task(str(device.id)) + device_cert.refresh_from_db() + extensions = device_cert.cert.extensions + ns_ext = next( + (ext for ext in extensions if ext.get("name") == "nsCertType"), None + ) + self.assertIsNotNone( + ns_ext, "Blueprint extension was lost during regeneration!" + ) + self.assertEqual(ns_ext["value"], "server") + self.assertFalse(ns_ext.get("critical", False)) + mac_ext = next( + (ext for ext in extensions if ext.get("oid") == "1.3.6.1.4.1.65901.1"), None + ) + self.assertIsNotNone(mac_ext, "MAC Address OID is missing!") + self.assertIn("AA:BB:CC:DD:EE:FF", mac_ext["value"]) + class TestTransactionDevice( CreateConfigTemplateMixin, From 7f8dba903327a211ac0a2646308096a32cf62434 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Fri, 12 Jun 2026 23:14:32 +0530 Subject: [PATCH 29/34] [fix] Improved certificate regeneration Made task safe against overlapping runs for the same device. --- openwisp_controller/config/api/serializers.py | 2 - openwisp_controller/config/base/config.py | 4 -- openwisp_controller/config/tasks.py | 40 ++++++++++++++----- 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index b2afc856e..53ea54e5a 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -109,7 +109,6 @@ def validate(self, data): data["blueprint_cert"] = None ca = None blueprint_cert = None - # assert structural binding matches between CA and template blueprints if template_type == "cert" and blueprint_cert and ca: if blueprint_cert.ca_id != ca.id: @@ -121,7 +120,6 @@ def validate(self, data): ) } ) - # apply mutation protections over protected fields if ( self.instance diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index 537ce24cb..cfd0e9c71 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -505,7 +505,6 @@ def manage_device_certs(cls, sender, instance, action, pk_set, **kwargs): "post_clear", ]: return - # handle full cleanup if the device configuration profile is being wiped if action == "post_clear": if instance.is_deactivating_or_deactivated(): @@ -519,7 +518,6 @@ def manage_device_certs(cls, sender, instance, action, pk_set, **kwargs): ).delete() ) return - # normalize templates across standard M2M sets vs Admin ModelForm querysets if isinstance(pk_set, set): template_model = cls.get_template_model() @@ -528,14 +526,12 @@ def manage_device_certs(cls, sender, instance, action, pk_set, **kwargs): ) else: templates = pk_set - # deletes orphaned certificates that are no # longer assigned in the templates list. if len(pk_set) != templates.filter(required=True).count(): instance.devicecertificate_set.exclude( template_id__in=instance.templates.values_list("id", flat=True) ).delete() - # allocate new DeviceCertificate associations # for newly added certificate templates if action == "post_add": diff --git a/openwisp_controller/config/tasks.py b/openwisp_controller/config/tasks.py index 381bb41db..3daefab94 100644 --- a/openwisp_controller/config/tasks.py +++ b/openwisp_controller/config/tasks.py @@ -238,18 +238,23 @@ def regenerate_device_certificates_task(device_id): device = Device.objects.get(id=device_id) except Device.DoesNotExist: return - active_device_certs = DeviceCertificate.objects.filter( - config__device=device, - auto_cert=True, - cert__revoked=False, - template__type="cert", - ).select_related("cert", "config", "template") - if not active_device_certs.exists(): - return + configs_to_update = set() certs_regenerated = 0 with transaction.atomic(): + active_device_certs = ( + DeviceCertificate.objects.select_for_update() + .filter( + config__device=device, + auto_cert=True, + cert__revoked=False, + template__type="cert", + ) + .select_related("cert", "config", "template") + ) + if not active_device_certs.exists(): + return for dc in active_device_certs: old_cert = dc.cert old_cert.revoke() @@ -260,6 +265,16 @@ def regenerate_device_certificates_task(device_id): extensions = [ {"name": "nsCertType", "value": "client", "critical": False} ] + ca = dc.template.ca + key_length = blueprint.key_length if blueprint else ca.key_length + digest = blueprint.digest if blueprint else str(ca.digest) + country_code = blueprint.country_code if blueprint else ca.country_code + state = blueprint.state if blueprint else ca.state + city = blueprint.city if blueprint else ca.city + organization_name = ( + blueprint.organization_name if blueprint else ca.organization_name + ) + email = blueprint.email if blueprint else ca.email extensions.extend( [ { @@ -276,8 +291,15 @@ def regenerate_device_certificates_task(device_id): ) new_cert = Cert( name=device.name, - ca=old_cert.ca, + ca=ca, organization=device.organization, + key_length=key_length, + digest=digest, + country_code=country_code, + state=state, + city=city, + organization_name=organization_name, + email=email, common_name=dc._get_common_name(), extensions=extensions, ) From e7a6b166a25a4f47c96a7c218336f71337414ffe Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sat, 13 Jun 2026 15:40:43 +0530 Subject: [PATCH 30/34] [fix] Synchronize config status on standalone cert renewal Updated certificate_updated to synchronize config status --- openwisp_controller/config/base/config.py | 12 +++++++--- .../config/tests/test_template.py | 24 +++++++++++++++++++ openwisp_controller/pki/tests/test_api.py | 6 ++--- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index cfd0e9c71..7a4b1d503 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -473,13 +473,19 @@ def enforce_required_templates( @classmethod def certificate_updated(cls, instance, created, **kwargs): + DeviceCertificate = load_model("config", "DeviceCertificate") if created or instance.revoked: return + configs_to_update = set() try: - config = instance.vpnclient.config + configs_to_update.add(instance.vpnclient.config) except ObjectDoesNotExist: - return - else: + pass + for dc in DeviceCertificate.objects.filter(cert=instance).select_related( + "config" + ): + configs_to_update.add(dc.config) + for config in configs_to_update: transaction.on_commit(config.update_status_if_checksum_changed) @classmethod diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index 66d58ee0c..1bfae0da7 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -970,6 +970,30 @@ def test_auto_add_template_to_existing_configs_not_triggered(self, mocked_task): required_template.save() mocked_task.assert_not_called() + def test_standalone_cert_renewal_updates_config_status(self): + org = self._get_org() + ca = self._create_ca(organization=org) + template = self._create_template( + name="cert-renewal-test", + type="cert", + ca=ca, + organization=org, + config={}, + ) + device = self._create_device(organization=org) + config = self._create_config(device=device) + config.templates.add(template) + dc = config.devicecertificate_set.first() + self.assertIsNotNone(dc) + config.status = "applied" + config.save(update_fields=["status"]) + config.refresh_from_db() + with mock.patch.object( + Config, "update_status_if_checksum_changed" + ) as mocked_update: + dc.cert.renew() + mocked_update.assert_called_once() + class TestTemplateCertificates(CreateConfigTemplateMixin, TestVpnX509Mixin, TestCase): """ diff --git a/openwisp_controller/pki/tests/test_api.py b/openwisp_controller/pki/tests/test_api.py index 8e6282b21..98812d43b 100644 --- a/openwisp_controller/pki/tests/test_api.py +++ b/openwisp_controller/pki/tests/test_api.py @@ -253,7 +253,7 @@ def test_cert_put_api(self): "organization": org2.pk, "notes": "new-notes", } - with self.assertNumQueries(10): + with self.assertNumQueries(11): r = self.client.put(path, data, content_type="application/json") self.assertEqual(r.status_code, 200) self.assertEqual(r.data["name"], "cert1-change") @@ -264,7 +264,7 @@ def test_cert_patch_api(self): cert1 = self._create_cert(name="cert1") path = reverse("pki_api:cert_detail", args=[cert1.pk]) data = {"name": "cert1-change"} - with self.assertNumQueries(8): + with self.assertNumQueries(9): r = self.client.patch(path, data, content_type="application/json") self.assertEqual(r.status_code, 200) self.assertEqual(r.data["name"], "cert1-change") @@ -289,7 +289,7 @@ def test_post_cert_renew_api(self): cert1 = self._create_cert(name="cert1") old_serial_num = cert1.serial_number path = reverse("pki_api:cert_renew", args=[cert1.pk]) - with self.assertNumQueries(5): + with self.assertNumQueries(6): r = self.client.post(path) self.assertEqual(r.status_code, 200) cert1.refresh_from_db() From 406e78060584bb87c28266f8250b3b6258294278 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sat, 13 Jun 2026 16:18:08 +0530 Subject: [PATCH 31/34] [docs] Addressed review comments Addressed review comments for updating documentation --- docs/developer/extending.rst | 1 + docs/index.rst | 1 + docs/user/certificate-templates.rst | 8 ++++---- docs/user/intro.rst | 2 ++ docs/user/rest-api.rst | 2 +- docs/user/settings.rst | 6 +++--- docs/user/templates.rst | 3 ++- 7 files changed, 14 insertions(+), 9 deletions(-) diff --git a/docs/developer/extending.rst b/docs/developer/extending.rst index ce8d74706..01cb53c04 100644 --- a/docs/developer/extending.rst +++ b/docs/developer/extending.rst @@ -342,6 +342,7 @@ Once you have created the models, add the following to your CONFIG_TEMPLATE_MODEL = "sample_config.Template" CONFIG_VPN_MODEL = "sample_config.Vpn" CONFIG_VPNCLIENT_MODEL = "sample_config.VpnClient" + CONFIG_DEVICECERTIFICATE_MODEL = "sample_config.DeviceCertificate" CONFIG_ORGANIZATIONCONFIGSETTINGS_MODEL = "sample_config.OrganizationConfigSettings" CONFIG_ORGANIZATIONLIMITS_MODEL = "sample_config.OrganizationLimits" CONFIG_WHOISINFO_MODEL = "sample_config.WHOISInfo" diff --git a/docs/index.rst b/docs/index.rst index ab621e8f8..68bd76150 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -37,6 +37,7 @@ the OpenWISP architecture. user/intro.rst user/device-config-status.rst user/templates.rst + user/certificate-templates.rst user/variables.rst user/device-groups.rst user/push-operations.rst diff --git a/docs/user/certificate-templates.rst b/docs/user/certificate-templates.rst index ead354d58..f627c7c13 100644 --- a/docs/user/certificate-templates.rst +++ b/docs/user/certificate-templates.rst @@ -31,8 +31,8 @@ To create a Certificate Template, navigate to the Templates section in the OpenWISP admin and set the **Type** to :guilabel:`Certificate` (``cert``). This will reveal the certificate-specific configuration fields. -.. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/certificate-template.png - :target: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/certificate-template.png +.. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/1.4/certificate-templates/certificate-template.png + :target: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/1.4/certificate-templates/certificate-template.png :alt: Certificate Template admin form :guilabel:`Certificate Authority` (required) @@ -161,8 +161,8 @@ context: - ``{{ cert__pem }}``: The public certificate. - ``{{ cert__key }}``: The private key. -- ``{{ cert__uuid }}``: The UUID of the - ``DeviceCertificate`` object. +- ``{{ cert__uuid }}``: The UUID of the generated + certificate. **Workflow Example:** diff --git a/docs/user/intro.rst b/docs/user/intro.rst index 1c4b123ae..4eb2a0d7b 100644 --- a/docs/user/intro.rst +++ b/docs/user/intro.rst @@ -35,6 +35,8 @@ following features: - **VPN management**: automatically provision VPN tunnel configurations, including cryptographic keys and IP addresses, e.g.: :doc:`OpenVPN `, :doc:`WireGuard ` +- :doc:`Certificate Templates `: automatically + generate and manage X.509 client certificates for devices - :doc:`whois`: display information about the public IP address used by devices to communicate with OpenWISP - :doc:`import-export` diff --git a/docs/user/rest-api.rst b/docs/user/rest-api.rst index c9702920d..9c2131a78 100644 --- a/docs/user/rest-api.rst +++ b/docs/user/rest-api.rst @@ -1112,7 +1112,7 @@ You can filter a list of templates based on their backend using the GET /api/v1/controller/template/?backend={backend} You can filter a list of templates based on their type using the ``type`` -(e.g. vpn or generic). +(e.g. vpn, cert or generic). .. code-block:: text diff --git a/docs/user/settings.rst b/docs/user/settings.rst index aa59425ef..5f5685ec5 100644 --- a/docs/user/settings.rst +++ b/docs/user/settings.rst @@ -308,9 +308,9 @@ levels of OpenWISP, see `netjsonconfig context: configuration variables The default value of the ``auto_cert`` field for new ``Template`` objects. The ``auto_cert`` field is valid only for templates which have ``type`` -set to ``VPN`` and indicates whether configuration regarding the VPN -tunnel is provisioned automatically to each device using the template, -e.g.: +set to ``VPN`` or ``cert`` and indicates whether configuration regarding +the VPN tunnel (or the x509 certificate) is provisioned automatically to +each device using the template, e.g.: - when using OpenVPN, new `x509 `_ certificates will be generated automatically using the same CA assigned diff --git a/docs/user/templates.rst b/docs/user/templates.rst index a1029a982..f46796ed1 100644 --- a/docs/user/templates.rst +++ b/docs/user/templates.rst @@ -216,7 +216,8 @@ Certificate Templates --------------------- A Certificate Template is a :doc:`Template ` -whose **Type** is set to :guilabel:`Certificate` (``cert``). +whose **Type** is set to :guilabel:`Certificate` (``cert``). See +:doc:`/controller/user/certificate-templates` for detailed documentation. It allows declaring the *Certificate Authority* and an optional *Blueprint Certificate* that will be used to issue an X.509 certificate for each From 25ab4c38e7c919c05ae1e71efe2bf0f27eed70db Mon Sep 17 00:00:00 2001 From: stktyagi Date: Tue, 16 Jun 2026 15:22:04 +0530 Subject: [PATCH 32/34] [fix] Made hardware drift cert regeneration idempotent Addressed review feedback to ensure the hardware drift certificate regeneration lifecycle is secure, extensible, and idempotent --- docs/user/certificate-templates.rst | 4 + docs/user/settings.rst | 32 +++++-- openwisp_controller/config/handlers.py | 16 +++- openwisp_controller/config/tasks.py | 64 +++++++------- .../config/tests/test_device.py | 86 ++++++++++++++++++- 5 files changed, 162 insertions(+), 40 deletions(-) diff --git a/docs/user/certificate-templates.rst b/docs/user/certificate-templates.rst index f627c7c13..c64ccc95b 100644 --- a/docs/user/certificate-templates.rst +++ b/docs/user/certificate-templates.rst @@ -163,6 +163,10 @@ context: - ``{{ cert__key }}``: The private key. - ``{{ cert__uuid }}``: The UUID of the generated certificate. +- ``{{ cert__path }}``: The file system path where the + certificate file will be installed on the device. +- ``{{ cert__key_path }}``: The file system path where + the private key file will be installed on the device. **Workflow Example:** diff --git a/docs/user/settings.rst b/docs/user/settings.rst index 5f5685ec5..36dd87e59 100644 --- a/docs/user/settings.rst +++ b/docs/user/settings.rst @@ -330,6 +330,24 @@ The objects that are automatically created will also be removed when they are not needed anymore (e.g.: when the VPN template is removed from a configuration object). +``OPENWISP_CONTROLLER_REGENERATE_CERTS_ON_HARDWARE_CHANGE`` +----------------------------------------------------------- + +============ ======== +**type**: ``bool`` +**default**: ``True`` +============ ======== + +When a device's name or MAC address changes, OpenWISP automatically +revokes the existing X.509 client certificates and provisions new ones +with the updated identity attributes. Set this to ``False`` to disable +this automatic regeneration. + +.. note:: + + The regeneration only applies to certificates created via Certificate + Templates with ``auto_cert`` enabled. + ``OPENWISP_CONTROLLER_CERT_PATH`` --------------------------------- @@ -350,13 +368,13 @@ default). **default**: ``{mac_address}-{name}`` ============ ======================== -Defines the format of the ``common_name`` attribute of VPN client -certificates that are automatically created when using VPN templates which -have ``auto_cert`` set to ``True``. A unique slug generated using -`shortuuid `_ is appended to -the common name to introduce uniqueness. Therefore, resulting common names -will have ``{OPENWISP_CONTROLLER_COMMON_NAME_FORMAT}-{unique-slug}`` -format. +Defines the format of the ``common_name`` attribute of X.509 client +certificates that are automatically created when using VPN or Certificate +Templates which have ``auto_cert`` set to ``True``. A unique slug +generated using `shortuuid `_ +is appended to the common name to introduce uniqueness. Therefore, +resulting common names will have +``{OPENWISP_CONTROLLER_COMMON_NAME_FORMAT}-{unique-slug}`` format. .. note:: diff --git a/openwisp_controller/config/handlers.py b/openwisp_controller/config/handlers.py index b78e4d2e6..73561437d 100644 --- a/openwisp_controller/config/handlers.py +++ b/openwisp_controller/config/handlers.py @@ -81,9 +81,21 @@ def detect_hardware_drift(sender, instance, created, **kwargs): getattr(instance, "_old_mac", instance.mac_address) != instance.mac_address ) if name_changed or mac_changed: - transaction.on_commit( - lambda: tasks.regenerate_device_certificates_task.delay(str(instance.id)) + DeviceCertificate = load_model("config", "DeviceCertificate") + expected_cert_ids = list( + DeviceCertificate.objects.filter( + config__device=instance, + auto_cert=True, + cert__revoked=False, + template__type="cert", + ).values_list("id", "cert_id") ) + if expected_cert_ids: + transaction.on_commit( + lambda: tasks.regenerate_device_certificates_task.delay( + str(instance.id), expected_cert_ids + ) + ) def devicegroup_change_handler(instance, **kwargs): diff --git a/openwisp_controller/config/tasks.py b/openwisp_controller/config/tasks.py index 3daefab94..82a2f1a9b 100644 --- a/openwisp_controller/config/tasks.py +++ b/openwisp_controller/config/tasks.py @@ -225,7 +225,7 @@ def invalidate_controller_views_cache(organization_id): @shared_task(soft_time_limit=1200) -def regenerate_device_certificates_task(device_id): +def regenerate_device_certificates_task(device_id, expected_cert_ids=None): """ Revokes stale certificates and mints fresh ones when hardware drift occurs. """ @@ -243,19 +243,23 @@ def regenerate_device_certificates_task(device_id): certs_regenerated = 0 with transaction.atomic(): - active_device_certs = ( - DeviceCertificate.objects.select_for_update() - .filter( - config__device=device, - auto_cert=True, - cert__revoked=False, - template__type="cert", - ) - .select_related("cert", "config", "template") + qs = DeviceCertificate.objects.select_for_update().filter( + config__device=device, + auto_cert=True, + cert__revoked=False, + template__type="cert", ) + if expected_cert_ids: + valid_cert_ids = [cert_id for _dc_id, cert_id in expected_cert_ids] + qs = qs.filter(cert_id__in=valid_cert_ids) + active_device_certs = qs.select_related("cert", "config", "template") if not active_device_certs.exists(): return + expected_map = dict(expected_cert_ids) if expected_cert_ids else {} for dc in active_device_certs: + expected_cert_id = expected_map.get(dc.id) + if expected_cert_id is not None and dc.cert_id != expected_cert_id: + continue old_cert = dc.cert old_cert.revoke() blueprint = dc.template.blueprint_cert @@ -292,7 +296,6 @@ def regenerate_device_certificates_task(device_id): new_cert = Cert( name=device.name, ca=ca, - organization=device.organization, key_length=key_length, digest=digest, country_code=country_code, @@ -303,6 +306,7 @@ def regenerate_device_certificates_task(device_id): common_name=dc._get_common_name(), extensions=extensions, ) + new_cert = dc._auto_create_cert_extra(new_cert) new_cert.full_clean() new_cert.save() dc.cert = new_cert @@ -311,21 +315,23 @@ def regenerate_device_certificates_task(device_id): certs_regenerated += 1 for config in configs_to_update: config.update_status_if_checksum_changed() - try: - message = _( - "Hardware drift detected on device {device_name}. " - "Successfully regenerated {certs_regenerated} bound X.509 certificate(s)." - ).format(device_name=str(device.name), certs_regenerated=certs_regenerated) - notify.send( - sender=device, - target=device, - action_object=device, - type="generic_message", - verb=_("experienced hardware drift"), - message=message, - level="info", - ) - except (ImportError, Exception) as e: - logger.warning( - f"Could not push regeneration notification for {device.name}: {e}" - ) + if certs_regenerated: + try: + message = _( + "Hardware drift detected on device {device_name}. " + "Successfully regenerated {certs_regenerated} " + "bound X.509 certificate(s)." + ).format(device_name=str(device.name), certs_regenerated=certs_regenerated) + notify.send( + sender=device, + target=device, + action_object=device, + type="generic_message", + verb=_("experienced hardware drift"), + message=message, + level="info", + ) + except (ImportError, Exception) as e: + logger.warning( + f"Could not push regeneration notification for {device.name}: {e}" + ) diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index a29d559da..b2e9ccd7d 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -664,10 +664,16 @@ def test_hardware_drift_signal_triggers_on_name_change(self, mocked_task): """Proof that changing the hostname fires the celery task.""" org = self._create_org() device = self._create_device(organization=org, name="old-router-name") + ca = Ca.objects.create(name="test-ca", organization=org) + template = self._create_template( + organization=org, type="cert", ca=ca, auto_cert=True + ) + config = self._create_config(device=device) + config.templates.add(template) device.name = "new-router-name" with self.captureOnCommitCallbacks(execute=True): device.save() - mocked_task.assert_called_once_with(str(device.id)) + self.assertEqual(mocked_task.call_count, 1) @mock.patch( "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" @@ -676,10 +682,16 @@ def test_hardware_drift_signal_triggers_on_mac_change(self, mocked_task): """Proof that changing the MAC address fires the celery task.""" org = self._create_org() device = self._create_device(organization=org, mac_address="00:11:22:33:44:55") + ca = Ca.objects.create(name="test-ca", organization=org) + template = self._create_template( + organization=org, type="cert", ca=ca, auto_cert=True + ) + config = self._create_config(device=device) + config.templates.add(template) device.mac_address = "AA:BB:CC:DD:EE:FF" with self.captureOnCommitCallbacks(execute=True): device.save() - mocked_task.assert_called_once_with(str(device.id)) + self.assertEqual(mocked_task.call_count, 1) @mock.patch( "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" @@ -794,6 +806,76 @@ def test_regeneration_preserves_blueprint_extensions(self, mocked_task): self.assertIsNotNone(mac_ext, "MAC Address OID is missing!") self.assertIn("AA:BB:CC:DD:EE:FF", mac_ext["value"]) + @mock.patch("openwisp_controller.config.tasks.notify.send") + def test_regeneration_triggers_toast_notification(self, mock_notify): + """Proof that a successful regeneration sends a generic_message toast.""" + org = self._create_org() + device = self._create_device( + organization=org, name="old-router-name", mac_address="00:11:22:33:44:55" + ) + ca = Ca.objects.create(name="test-ca", organization=org) + template = self._create_template( + organization=org, type="cert", ca=ca, auto_cert=True + ) + config = self._create_config(device=device) + config.templates.add(template) + device.name = "renamed-router" + device.save() + regenerate_device_certificates_task(str(device.id)) + mock_notify.assert_called_once() + _, kwargs = mock_notify.call_args + self.assertEqual(kwargs.get("type"), "generic_message") + self.assertEqual(kwargs.get("verb"), "experienced hardware drift") + self.assertIn("renamed-router", kwargs.get("message")) + + @mock.patch( + "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" + ) + def test_regeneration_idempotent_when_duplicate_tasks(self, mocked_task): + """Proof that a duplicate task with stale cert IDs does not rotate again.""" + org = self._create_org() + device = self._create_device( + organization=org, name="old-router-name", mac_address="00:11:22:33:44:55" + ) + ca = Ca.objects.create(name="test-ca", organization=org) + template = self._create_template( + organization=org, type="cert", ca=ca, auto_cert=True + ) + config = self._create_config(device=device) + config.templates.add(template) + device_cert = DeviceCertificate.objects.get(config=config, template=template) + original_cert_id = device_cert.cert.id + self.assertFalse(device_cert.cert.revoked) + expected_cert_ids = list( + DeviceCertificate.objects.filter( + config__device=device, + auto_cert=True, + cert__revoked=False, + template__type="cert", + ).values_list("id", "cert_id") + ) + device.name = "renamed-router" + device.save() + regenerate_device_certificates_task(str(device.id), expected_cert_ids) + device_cert.refresh_from_db() + first_new_cert_id = device_cert.cert.id + self.assertNotEqual(original_cert_id, first_new_cert_id) + self.assertFalse(device_cert.cert.revoked) + old_cert = Cert.objects.get(id=original_cert_id) + self.assertTrue(old_cert.revoked) + # Run the same task again with the same stale expected_cert_ids + # to simulate a duplicate task that was queued before the first + # task completed. The second task should skip because the cert + # IDs no longer match. + regenerate_device_certificates_task(str(device.id), expected_cert_ids) + device_cert.refresh_from_db() + self.assertEqual( + first_new_cert_id, + device_cert.cert.id, + "Duplicate task rotated the cert again despite stale expected_cert_ids", + ) + self.assertFalse(device_cert.cert.revoked) + class TestTransactionDevice( CreateConfigTemplateMixin, From a819721a36e5faf99f08ac2a3e145c8791113c4f Mon Sep 17 00:00:00 2001 From: stktyagi Date: Thu, 18 Jun 2026 15:49:58 +0530 Subject: [PATCH 33/34] [chores] Updated DeviceCertificate model and tests Added selenium tests and refactored DeviceCertificate model --- .../config/base/device_certificate.py | 101 ++++++++++-------- openwisp_controller/config/handlers.py | 18 ++-- openwisp_controller/config/tasks.py | 51 +-------- .../config/tests/test_selenium.py | 67 ++++++++++++ 4 files changed, 136 insertions(+), 101 deletions(-) diff --git a/openwisp_controller/config/base/device_certificate.py b/openwisp_controller/config/base/device_certificate.py index 00bc6d1b2..6faeec721 100644 --- a/openwisp_controller/config/base/device_certificate.py +++ b/openwisp_controller/config/base/device_certificate.py @@ -9,6 +9,9 @@ from openwisp_controller.config import settings as app_settings from openwisp_utils.base import TimeStampedEditableModel +MAC_ADDRESS_OID = "1.3.6.1.4.1.65901.1" +DEVICE_UUID_OID = "1.3.6.1.4.1.65901.2" + class AbstractDeviceCertificate(TimeStampedEditableModel): config = models.ForeignKey( @@ -84,63 +87,69 @@ def _get_common_name(self): common_name = f"{common_name}-{unique_slug}" return common_name - def _auto_create_cert(self, name, common_name): - """ - Automatically creates and assigns a client x509 certificate - using Blueprint cloning and custom hardware OID injection. - """ + def _build_cert(self, name, common_name): + """Build (but do not save) a Cert instance from template + blueprint.""" ca = self.template.ca blueprint = self.template.blueprint_cert - device = self.config.device cert_model = self.__class__.cert.field.related_model - # blueprint property cloning with CA fallback - key_length = blueprint.key_length if blueprint else ca.key_length - digest = blueprint.digest if blueprint else str(ca.digest) - country_code = blueprint.country_code if blueprint else ca.country_code - state = blueprint.state if blueprint else ca.state - city = blueprint.city if blueprint else ca.city - organization_name = ( - blueprint.organization_name if blueprint else ca.organization_name + attrs = self._clone_blueprint_attrs(ca, blueprint) + extensions = self._build_extensions(blueprint) + cert = cert_model( + name=name, + ca=ca, + common_name=common_name, + extensions=extensions, + **attrs, + ) + return self._auto_create_cert_extra(cert) + + def _clone_blueprint_attrs(self, ca, blueprint): + """ + Extracts base X.509 attributes (such as key length, digest, and + location data) from the provided blueprint certificate. + """ + source = blueprint or ca + digest = str(source.digest) if not blueprint else source.digest + return dict( + key_length=source.key_length, + digest=digest, + country_code=source.country_code, + state=source.state, + city=source.city, + organization_name=source.organization_name, + email=source.email, ) - email = blueprint.email if blueprint else ca.email + def _build_extensions(self, blueprint): + """Compiles the list of X.509 extensions for the new certificate.""" if blueprint and blueprint.extensions: extensions = copy.deepcopy(blueprint.extensions) else: extensions = [{"name": "nsCertType", "value": "client", "critical": False}] + extensions.extend(self._get_hardware_oid_extensions()) + return extensions - # inject MAC and UUID as custom OIDs, prerequisite: #228 in django-x509 - mac_oid = "1.3.6.1.4.1.65901.1" - uuid_oid = "1.3.6.1.4.1.65901.2" - extensions.extend( - [ - { - "oid": mac_oid, - "value": f"ASN1:UTF8:string:{device.mac_address}", - "critical": False, - }, - { - "oid": uuid_oid, - "value": f"ASN1:UTF8:string:{device.id}", - "critical": False, - }, - ] - ) - cert = cert_model( - name=name, - ca=ca, - key_length=key_length, - digest=digest, - country_code=country_code, - state=state, - city=city, - organization_name=organization_name, - email=email, - common_name=common_name, - extensions=extensions, - ) - cert = self._auto_create_cert_extra(cert) + def _get_hardware_oid_extensions(self): + device = self.config.device + return [ + { + "oid": MAC_ADDRESS_OID, + "value": f"ASN1:UTF8:string:{device.mac_address}", + "critical": False, + }, + { + "oid": DEVICE_UUID_OID, + "value": f"ASN1:UTF8:string:{device.id}", + "critical": False, + }, + ] + + def _auto_create_cert(self, name, common_name): + """ + Automatically creates and assigns a client x509 certificate + """ + cert = self._build_cert(name=name, common_name=common_name) cert.full_clean() cert.save() self.cert = cert diff --git a/openwisp_controller/config/handlers.py b/openwisp_controller/config/handlers.py index 73561437d..a63871fa5 100644 --- a/openwisp_controller/config/handlers.py +++ b/openwisp_controller/config/handlers.py @@ -45,6 +45,12 @@ def device_registered_notification(sender, instance, is_new, **kwargs): ) +def _hardware_fields_changed(update_fields): + return update_fields is None or ( + "name" in update_fields or "mac_address" in update_fields + ) + + @receiver(pre_save, sender=Device, dispatch_uid="capture_old_hardware_properties") def capture_old_hardware_properties(sender, instance, **kwargs): """ @@ -53,10 +59,8 @@ def capture_old_hardware_properties(sender, instance, **kwargs): """ if not instance.pk: return - update_fields = kwargs.get("update_fields") - if update_fields is not None: - if "name" not in update_fields and "mac_address" not in update_fields: - return + if not _hardware_fields_changed(kwargs.get("update_fields")): + return try: old_instance = sender.objects.only("name", "mac_address").get(pk=instance.pk) instance._old_name = old_instance.name @@ -72,10 +76,8 @@ def detect_hardware_drift(sender, instance, created, **kwargs): """ if created or not app_settings.REGENERATE_CERTS_ON_HARDWARE_CHANGE: return - update_fields = kwargs.get("update_fields") - if update_fields is not None: - if "name" not in update_fields and "mac_address" not in update_fields: - return + if not _hardware_fields_changed(kwargs.get("update_fields")): + return name_changed = getattr(instance, "_old_name", instance.name) != instance.name mac_changed = ( getattr(instance, "_old_mac", instance.mac_address) != instance.mac_address diff --git a/openwisp_controller/config/tasks.py b/openwisp_controller/config/tasks.py index 82a2f1a9b..9f73b713b 100644 --- a/openwisp_controller/config/tasks.py +++ b/openwisp_controller/config/tasks.py @@ -1,4 +1,3 @@ -import copy import logging import requests @@ -233,7 +232,6 @@ def regenerate_device_certificates_task(device_id, expected_cert_ids=None): return Device = load_model("config", "Device") DeviceCertificate = load_model("config", "DeviceCertificate") - Cert = load_model("django_x509", "Cert") try: device = Device.objects.get(id=device_id) except Device.DoesNotExist: @@ -262,51 +260,9 @@ def regenerate_device_certificates_task(device_id, expected_cert_ids=None): continue old_cert = dc.cert old_cert.revoke() - blueprint = dc.template.blueprint_cert - if blueprint and blueprint.extensions: - extensions = copy.deepcopy(blueprint.extensions) - else: - extensions = [ - {"name": "nsCertType", "value": "client", "critical": False} - ] - ca = dc.template.ca - key_length = blueprint.key_length if blueprint else ca.key_length - digest = blueprint.digest if blueprint else str(ca.digest) - country_code = blueprint.country_code if blueprint else ca.country_code - state = blueprint.state if blueprint else ca.state - city = blueprint.city if blueprint else ca.city - organization_name = ( - blueprint.organization_name if blueprint else ca.organization_name - ) - email = blueprint.email if blueprint else ca.email - extensions.extend( - [ - { - "oid": "1.3.6.1.4.1.65901.1", - "value": f"ASN1:UTF8:string:{device.mac_address}", - "critical": False, - }, - { - "oid": "1.3.6.1.4.1.65901.2", - "value": f"ASN1:UTF8:string:{device.id}", - "critical": False, - }, - ] - ) - new_cert = Cert( - name=device.name, - ca=ca, - key_length=key_length, - digest=digest, - country_code=country_code, - state=state, - city=city, - organization_name=organization_name, - email=email, - common_name=dc._get_common_name(), - extensions=extensions, + new_cert = dc._build_cert( + name=device.name, common_name=dc._get_common_name() ) - new_cert = dc._auto_create_cert_extra(new_cert) new_cert.full_clean() new_cert.save() dc.cert = new_cert @@ -314,8 +270,9 @@ def regenerate_device_certificates_task(device_id, expected_cert_ids=None): configs_to_update.add(dc.config) certs_regenerated += 1 for config in configs_to_update: + config.refresh_from_db() config.update_status_if_checksum_changed() - if certs_regenerated: + if certs_regenerated > 0: try: message = _( "Hardware drift detected on device {device_name}. " diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index 399aff0eb..461e592a7 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -22,6 +22,9 @@ Device = load_model("config", "Device") DeviceGroup = load_model("config", "DeviceGroup") Cert = load_model("django_x509", "Cert") +Ca = load_model("django_x509", "Ca") +DeviceCertificate = load_model("config", "DeviceCertificate") +Notification = load_model("openwisp_notifications", "Notification") class SeleniumTestMixin(BaseSeleniumTestMixin): @@ -486,6 +489,70 @@ def test_relevant_templates_duplicates(self): self.find_element(by=By.NAME, value="_save").click() self.wait_for_presence(By.CSS_SELECTOR, ".messagelist .success", timeout=5) + def test_e2e_certificate_provisioning(self): + """ + End-to-end flow: create CA, create certificate + template, assign to device, verify certificate generation. + """ + org = self._get_org() + ca = Ca.objects.create(name="test-ca", organization=org) + template = self._create_template( + organization=org, type="cert", ca=ca, auto_cert=True + ) + device = self._create_device(organization=org, name="e2e-router") + self._create_config(device=device) + + self.login() + self.open( + reverse(f"admin:{self.config_app_label}_device_change", args=[device.id]) + + "#config-group" + ) + self.hide_loading_overlay() + self.find_element(by=By.XPATH, value=f'//*[@value="{template.id}"]').click() + self.web_driver.execute_script( + 'document.querySelector("#ow-user-tools").style.display="none"' + ) + self.find_element(by=By.NAME, value="_continue").click() + self.wait_for_presence(By.CSS_SELECTOR, ".messagelist .success", timeout=5) + device.config.refresh_from_db() + device_cert = DeviceCertificate.objects.get( + config=device.config, template=template + ) + self.assertIsNotNone( + device_cert.cert, "Certificate was not generated after UI assignment!" + ) + self.assertFalse(device_cert.cert.revoked) + self.assertEqual(device_cert.cert.name, "e2e-router") + + def test_hardware_drift_notification(self): + org = self._get_org() + ca = Ca.objects.create(name="test-ca", organization=org) + template = self._create_template( + organization=org, type="cert", ca=ca, auto_cert=True + ) + device = self._create_device(organization=org, name="old-router-name") + config = self._create_config(device=device) + config.templates.add(template) + self.login() + self.open( + reverse(f"admin:{self.config_app_label}_device_change", args=[device.id]) + ) + self.hide_loading_overlay() + name_input = self.find_element(by=By.NAME, value="name", wait_for="presence") + name_input.clear() + name_input.send_keys("renamed-router") + # Hide user tools because it covers the save button + self.web_driver.execute_script( + 'document.querySelector("#ow-user-tools").style.display="none"' + ) + self.find_element(by=By.NAME, value="_save").click() + self.wait_for_presence(By.CSS_SELECTOR, ".messagelist .success", timeout=5) + self.find_element(by=By.ID, value="openwisp_notifications").click() + notification = self.wait_for_visibility( + By.CLASS_NAME, "ow-notification-elem", timeout=10 + ) + self.assertIn("Hardware drift detected", notification.text) + @tag("selenium_tests") class TestDeviceGroupAdmin( From 8c322821023cd1510bdbbb72823d38df25d0bf24 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Fri, 19 Jun 2026 13:00:53 +0530 Subject: [PATCH 34/34] [chores] Minor improvements Addressed coderabbit review comments. --- openwisp_controller/config/admin.py | 1 - openwisp_controller/config/api/serializers.py | 1 + openwisp_controller/config/handlers.py | 13 +++++- .../config/tests/test_device.py | 41 ++++++++++++++++++- .../config/tests/test_selenium.py | 13 ++++-- .../config/tests/test_template.py | 5 ++- 6 files changed, 65 insertions(+), 9 deletions(-) diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index 09fb34494..3d8d5f4a5 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -121,7 +121,6 @@ class Media: "preview.js", "unsaved_changes.js", "switcher.js", - "template_ui.js", ) ] diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index 53ea54e5a..0dba8035f 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -441,6 +441,7 @@ def update(self, instance, validated_data): instance.config.device.organization = validated_data.get( "organization" ) + DeviceCertificate.objects.filter(config=instance.config).delete() instance.config.templates.clear() Config.enforce_required_templates( action="post_clear", diff --git a/openwisp_controller/config/handlers.py b/openwisp_controller/config/handlers.py index a63871fa5..faee94bec 100644 --- a/openwisp_controller/config/handlers.py +++ b/openwisp_controller/config/handlers.py @@ -51,6 +51,10 @@ def _hardware_fields_changed(update_fields): ) +def _hardware_field_was_saved(update_fields, field_name): + return update_fields is None or field_name in update_fields + + @receiver(pre_save, sender=Device, dispatch_uid="capture_old_hardware_properties") def capture_old_hardware_properties(sender, instance, **kwargs): """ @@ -78,9 +82,14 @@ def detect_hardware_drift(sender, instance, created, **kwargs): return if not _hardware_fields_changed(kwargs.get("update_fields")): return - name_changed = getattr(instance, "_old_name", instance.name) != instance.name + update_fields = kwargs.get("update_fields") + name_changed = ( + _hardware_field_was_saved(update_fields, "name") + and getattr(instance, "_old_name", instance.name) != instance.name + ) mac_changed = ( - getattr(instance, "_old_mac", instance.mac_address) != instance.mac_address + _hardware_field_was_saved(update_fields, "mac_address") + and getattr(instance, "_old_mac", instance.mac_address) != instance.mac_address ) if name_changed or mac_changed: DeviceCertificate = load_model("config", "DeviceCertificate") diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index b2e9ccd7d..f3188f410 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -700,8 +700,15 @@ def test_hardware_drift_signal_ignores_unrelated_changes(self, mocked_task): """Proof that saving a device without changing name/MAC does nothing.""" org = self._create_org() device = self._create_device(organization=org) + ca = Ca.objects.create(name="test-ca", organization=org) + template = self._create_template( + organization=org, type="cert", ca=ca, auto_cert=True + ) + config = self._create_config(device=device) + config.templates.add(template) device.key = "new-management-key" - device.save() + with self.captureOnCommitCallbacks(execute=True): + device.save() mocked_task.assert_not_called() @mock.patch( @@ -714,8 +721,38 @@ def test_hardware_drift_setting_disables_regeneration(self, mocked_task): """Proof that the user-configurable setting turns the feature off.""" org = self._create_org() device = self._create_device(organization=org) + ca = Ca.objects.create(name="test-ca", organization=org) + template = self._create_template( + organization=org, type="cert", ca=ca, auto_cert=True + ) + config = self._create_config(device=device) + config.templates.add(template) device.name = "another-new-name" - device.save() + with self.captureOnCommitCallbacks(execute=True): + device.save() + mocked_task.assert_not_called() + + @mock.patch( + "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" + ) + def test_hardware_drift_partial_save_ignores_dirty_memory(self, mocked_task): + """ + Proof that dirty in-memory fields are + not evaluated if not in update_fields. + """ + org = self._create_org() + device = self._create_device( + organization=org, name="old-name", mac_address="00:11:22:33:44:55" + ) + ca = Ca.objects.create(name="test-ca", organization=org) + template = self._create_template( + organization=org, type="cert", ca=ca, auto_cert=True + ) + config = self._create_config(device=device) + config.templates.add(template) + device.mac_address = "AA:BB:CC:DD:EE:FF" + with self.captureOnCommitCallbacks(execute=True): + device.save(update_fields=["name"]) mocked_task.assert_not_called() @mock.patch( diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index 461e592a7..56028eeb6 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -22,7 +22,6 @@ Device = load_model("config", "Device") DeviceGroup = load_model("config", "DeviceGroup") Cert = load_model("django_x509", "Cert") -Ca = load_model("django_x509", "Ca") DeviceCertificate = load_model("config", "DeviceCertificate") Notification = load_model("openwisp_notifications", "Notification") @@ -495,7 +494,11 @@ def test_e2e_certificate_provisioning(self): template, assign to device, verify certificate generation. """ org = self._get_org() - ca = Ca.objects.create(name="test-ca", organization=org) + ca = self._create_ca( + name="test-ca", + common_name="test-ca", + organization=org, + ) template = self._create_template( organization=org, type="cert", ca=ca, auto_cert=True ) @@ -526,7 +529,11 @@ def test_e2e_certificate_provisioning(self): def test_hardware_drift_notification(self): org = self._get_org() - ca = Ca.objects.create(name="test-ca", organization=org) + ca = self._create_ca( + name="hardware-drift-ca", + common_name="hardware-drift-ca", + organization=org, + ) template = self._create_template( organization=org, type="cert", ca=ca, auto_cert=True ) diff --git a/openwisp_controller/config/tests/test_template.py b/openwisp_controller/config/tests/test_template.py index 1bfae0da7..335625b8e 100644 --- a/openwisp_controller/config/tests/test_template.py +++ b/openwisp_controller/config/tests/test_template.py @@ -1453,7 +1453,10 @@ def test_get_unassigned_certs_with_null_device_cert(self): template = self._create_template( name="Test-Template", type="cert", ca=ca, organization=org, config={} ) - DeviceCertificate.objects.create(config=config, template=template, cert=None) + device_cert = DeviceCertificate.objects.create( + config=config, template=template, cert=None, auto_cert=False + ) + self.assertIsNone(device_cert.cert_id) choices = get_unassigned_certs() queryset = choices.get("pk__in") self.assertIsNotNone(queryset)