-
-
Notifications
You must be signed in to change notification settings - Fork 298
[feature] Introduced standalone certificate templates and device bindings #1378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: gsoc26-x509-certificate-generator-templates
Are you sure you want to change the base?
Changes from 10 commits
25f1a21
a795e09
b946d26
ff1e8a1
0053bcf
01221d9
5c551c7
d6fd193
cae38a5
02912a0
3434719
298572f
5c522e9
052cebf
148bfd2
badf1c1
a692c77
16703f1
212b839
37ff9a9
ef90a6c
b8bc3e0
88af90a
bca3809
99cd151
7390020
ee79c93
c92cc5e
dfa650c
88003e2
b9db466
28f9cce
228e9bc
1e9e813
717c3e1
7f8dba9
e7a6b16
406e780
25ab4c3
4711fa3
a819721
8c32282
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -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: | ||||
|
|
@@ -38,6 +39,8 @@ class Meta(BaseMeta): | |||
| "type", | ||||
| "backend", | ||||
| "vpn", | ||||
| "ca", | ||||
| "blueprint_cert", | ||||
| "tags", | ||||
| "default", | ||||
| "required", | ||||
|
|
@@ -46,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." | ||||
| ) | ||||
| } | ||||
| } | ||||
| } | ||||
|
Aryamanz29 marked this conversation as resolved.
|
||||
|
|
||||
| def validate_vpn(self, value): | ||||
| """ | ||||
|
|
@@ -62,12 +75,73 @@ 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( | ||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||
| _("The configuration field cannot be empty.") | ||||
| ) | ||||
| return value | ||||
|
|
||||
| def validate(self, data): | ||||
| """ | ||||
| Explicitly validate certificate template fields and locks for the API. | ||||
| """ | ||||
|
Aryamanz29 marked this conversation as resolved.
|
||||
| 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." | ||||
| ) | ||||
| } | ||||
| ) | ||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||
| if self.instance and self.instance.pk: | ||||
| if Config.objects.filter(templates=self.instance).exists(): | ||||
|
|
||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
| if "ca" in data and data["ca"] != getattr( | ||||
|
stktyagi marked this conversation as resolved.
Outdated
|
||||
| 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." | ||||
| ) | ||||
| } | ||||
| ) | ||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||
| return super().validate(data) | ||||
|
|
||||
|
|
||||
| class VpnSerializer(BaseSerializer): | ||||
| config = serializers.JSONField(initial={}) | ||||
|
|
@@ -203,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() | ||||
|
coderabbitai[bot] marked this conversation as resolved.
stktyagi marked this conversation as resolved.
|
||||
| config.templates.set(config_templates, clear=True) | ||||
|
stktyagi marked this conversation as resolved.
|
||||
| config.save() | ||||
| except ValidationError as error: | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| import re | ||
| from collections import defaultdict | ||
|
|
||
| import swapper | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| 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"), | ||
| ) | ||
|
stktyagi marked this conversation as resolved.
Comment on lines
+66
to
+72
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please address the through-model lifecycle consistently. Right now the relation is added, but assignments through |
||
|
|
||
| STATUS = Choices("modified", "applied", "error", "deactivating", "deactivated") | ||
| status = StatusField( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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." | ||
| ) | ||
| } | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,34 @@ 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." | ||
| ), | ||
| ) | ||
|
|
||
| def get_unassigned_certs(): | ||
| Cert = load_model("django_x509", "Cert") | ||
| return {"pk__in": Cert.objects.exclude(devicecertificate__isnull=False)} | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| 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=get_unassigned_certs, | ||
| help_text=_( | ||
| "Optional: Select an unassigned certificate to copy extensions and " | ||
| "properties from." | ||
| ), | ||
| ) | ||
| type = models.CharField( | ||
| _("type"), | ||
| max_length=16, | ||
|
|
@@ -90,7 +122,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( | ||
|
|
@@ -221,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: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this clean method is becoming quite big and the docstring was not updated to reflect these new validations: try to concentrate the logic for this new feature in specific methods so it remains as much separated as possible from the exsiting code and do not forget to update docstring |
||
| try: | ||
| current = self.__class__.objects.get(pk=self.pk) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need to put the entire block in the try/except or can we leave only the get? |
||
| 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." | ||
| ) | ||
| } | ||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| except self.__class__.DoesNotExist: | ||
| pass | ||
| self._validate_org_relation("vpn") | ||
| if not self.default_values: | ||
| self.default_values = {} | ||
|
|
@@ -234,15 +287,56 @@ 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: | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| 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") | ||
|
stktyagi marked this conversation as resolved.
|
||
| self._validate_org_relation("blueprint_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_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: | ||
| 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): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.