diff --git a/backend/device_registry/api_views.py b/backend/device_registry/api_views.py index cf1be7b3c..550fdab19 100644 --- a/backend/device_registry/api_views.py +++ b/backend/device_registry/api_views.py @@ -106,9 +106,6 @@ def post(self, request, *args, **kwargs): device.cpu = data.get('cpu', {}) device.os_release = os_release device.mysql_root_access = data.get('mysql_root_access') - # Un-snooze recommended actions which were "Fixed" (i.e. snoozed until next ping) - device.recommendedaction_set.filter(snoozed=RecommendedAction.Snooze.UNTIL_PING)\ - .update(snoozed=RecommendedAction.Snooze.NOT_SNOOZED) device_info_object, _ = DeviceInfo.objects.get_or_create(device=device) device_info_object.device__last_ping = timezone.now() device_info_object.device_operating_system_version = data.get('device_operating_system_version') @@ -152,6 +149,10 @@ def post(self, request, *args, **kwargs): device.save(update_fields=['last_ping', 'agent_version', 'audit_files', 'deb_packages_hash', 'update_trust_score', 'os_release', 'auto_upgrades', 'mysql_root_access', 'cpu', 'kernel_deb_package']) + # Un-snooze recommended actions which were "Fixed" (i.e. snoozed until next ping) + device.recommendedaction_set.filter(status=RecommendedAction.Status.SNOOZED_UNTIL_PING) \ + .update(status=RecommendedAction.Status.AFFECTED) + device.generate_recommended_actions() if datastore_client: task_key = datastore_client.key('Ping') @@ -1028,10 +1029,10 @@ def post(self, request, *args, **kwargs): action_id = serializer.validated_data['action_id'] duration = serializer.validated_data['duration'] if duration is None: - snoozed = RecommendedAction.Snooze.UNTIL_PING + snoozed = RecommendedAction.Status.SNOOZED_UNTIL_PING elif duration == 0: - snoozed = RecommendedAction.Snooze.FOREVER + snoozed = RecommendedAction.Status.SNOOZED_FOREVER else: - snoozed = RecommendedAction.Snooze.UNTIL_TIME + snoozed = RecommendedAction.Status.SNOOZED_UNTIL_TIME dev.snooze_action(action_id, snoozed, duration) return Response(status=status.HTTP_200_OK) diff --git a/backend/device_registry/celery_tasks/github.py b/backend/device_registry/celery_tasks/github.py index a4fbb6428..221a3a552 100644 --- a/backend/device_registry/celery_tasks/github.py +++ b/backend/device_registry/celery_tasks/github.py @@ -272,7 +272,7 @@ def file_issues(): logger.debug(f'action class {action_class.action_id}') # top-level ints in a JSON dict are auto-converted to strings, so we have to use strings here issue_number = profile.github_issues.get(str(action_class.action_id)) - description = action_class.get_description(profile.user, devices='your nodes') + description = action_class.get_description(profile.user, additional_context={'devices': 'your nodes'}) logger.debug(f'issue #{issue_number}') try: if description: diff --git a/backend/device_registry/migrations/0072_auto_20191202_1725.py b/backend/device_registry/migrations/0072_auto_20191202_1725.py index 2a6f11907..343897d96 100644 --- a/backend/device_registry/migrations/0072_auto_20191202_1725.py +++ b/backend/device_registry/migrations/0072_auto_20191202_1725.py @@ -21,7 +21,7 @@ class Migration(migrations.Migration): fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('action_id', models.PositiveSmallIntegerField()), - ('snoozed', models.PositiveSmallIntegerField(choices=[(device_registry.models.RecommendedAction.Snooze(0), 0), (device_registry.models.RecommendedAction.Snooze(1), 1), (device_registry.models.RecommendedAction.Snooze(2), 2), (device_registry.models.RecommendedAction.Snooze(3), 3)], default=device_registry.models.RecommendedAction.Snooze(0))), + ('snoozed', models.PositiveSmallIntegerField(choices=[(device_registry.models.RecommendedAction.Status(0), 0), (device_registry.models.RecommendedAction.Status(1), 1), (device_registry.models.RecommendedAction.Status(2), 2), (device_registry.models.RecommendedAction.Status(3), 3)], default=device_registry.models.RecommendedAction.Status(0))), ('snoozed_until', models.DateTimeField(null=True)), ('device', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='device_registry.Device')), ], diff --git a/backend/device_registry/migrations/0073_auto_20191202_1732.py b/backend/device_registry/migrations/0073_auto_20191202_1732.py index e38c54837..a3421cdb0 100644 --- a/backend/device_registry/migrations/0073_auto_20191202_1732.py +++ b/backend/device_registry/migrations/0073_auto_20191202_1732.py @@ -14,7 +14,7 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='recommendedaction', name='snoozed', - field=models.PositiveSmallIntegerField(choices=[(device_registry.models.RecommendedAction.Snooze(0), 0), (device_registry.models.RecommendedAction.Snooze(1), 1), (device_registry.models.RecommendedAction.Snooze(2), 2), (device_registry.models.RecommendedAction.Snooze(3), 3)], default=0), + field=models.PositiveSmallIntegerField(choices=[(device_registry.models.RecommendedAction.Status(0), 0), (device_registry.models.RecommendedAction.Status(1), 1), (device_registry.models.RecommendedAction.Status(2), 2), (device_registry.models.RecommendedAction.Status(3), 3)], default=0), ), migrations.AlterField( model_name='recommendedaction', diff --git a/backend/device_registry/migrations/0074_recommendedaction_status.py b/backend/device_registry/migrations/0074_recommendedaction_status.py new file mode 100644 index 000000000..04125086f --- /dev/null +++ b/backend/device_registry/migrations/0074_recommendedaction_status.py @@ -0,0 +1,23 @@ +# Generated by Django 2.2.5 on 2019-12-19 06:32 + +import device_registry.models +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('device_registry', '0073_auto_20191202_1732'), + ] + + operations = [ + migrations.RemoveField( + model_name='recommendedaction', + name='snoozed', + ), + migrations.AddField( + model_name='recommendedaction', + name='status', + field=models.PositiveSmallIntegerField(choices=[(device_registry.models.RecommendedAction.Status(0), 0), (device_registry.models.RecommendedAction.Status(1), 1), (device_registry.models.RecommendedAction.Status(2), 2), (device_registry.models.RecommendedAction.Status(3), 3), (device_registry.models.RecommendedAction.Status(4), 4)], default=0), + ), + ] diff --git a/backend/device_registry/migrations/0075_auto_20191223_1436.py b/backend/device_registry/migrations/0075_auto_20191223_1436.py new file mode 100644 index 000000000..25116e2f5 --- /dev/null +++ b/backend/device_registry/migrations/0075_auto_20191223_1436.py @@ -0,0 +1,23 @@ +# Generated by Django 2.2.6 on 2019-12-23 14:36 + +from django.db import migrations + + +def generate_recommended_actions(apps, schema_editor): + from device_registry.models import RecommendedAction + RecommendedAction.update_all_devices() + + +class Migration(migrations.Migration): + + dependencies = [ + ('device_registry', '0074_recommendedaction_status'), + ] + + operations = [ + migrations.AlterUniqueTogether( + name='recommendedaction', + unique_together={('device', 'action_id')}, + ), + migrations.RunPython(generate_recommended_actions) + ] diff --git a/backend/device_registry/models.py b/backend/device_registry/models.py index 7fee51194..1fe707d7b 100644 --- a/backend/device_registry/models.py +++ b/backend/device_registry/models.py @@ -7,6 +7,7 @@ from django.conf import settings from django.db import models, transaction +from django.db.models import Q from django.utils import timezone from django.contrib.postgres.fields import ArrayField, JSONField from django.core.exceptions import ObjectDoesNotExist @@ -112,6 +113,9 @@ class SshdIssueItem(NamedTuple): auto_upgrades = models.BooleanField(null=True, blank=True) mysql_root_access = models.BooleanField(null=True, blank=True) + class Meta: + ordering = ('created',) + @property def eol_info(self): """ @@ -128,8 +132,8 @@ def eol_info(self): def snooze_action(self, action_id, snoozed, duration=None): action, _ = RecommendedAction.objects.get_or_create(action_id=action_id, device=self) - action.snoozed = snoozed - if snoozed == RecommendedAction.Snooze.UNTIL_TIME: + action.status = snoozed + if snoozed == RecommendedAction.Status.SNOOZED_UNTIL_TIME: action.snoozed_until = timezone.now() + datetime.timedelta(hours=duration) else: action.snoozed_until = None @@ -302,11 +306,7 @@ def hostname(self): @property def actions_count(self): - if hasattr(self, 'firewallstate') and hasattr(self, 'portscan'): - action_classes = ActionMeta.all_classes() - return sum( - [action_class.affected_devices(self.owner, self.pk).exists() for action_class in action_classes] - ) + return self.recommendedaction_set.filter(RecommendedAction.get_affected_query()).count() def _get_listening_sockets(self, port): return [r for r in self.portscan.scan_info if @@ -325,6 +325,8 @@ def public_services(self): Looks for open ports and known services (declared in PUBLIC_SERVICE_PORTS) listening on them. :return: a set of service names (keys from PUBLIC_SERVICE_PORTS) which are listening. """ + if not hasattr(self, 'deviceinfo'): + return processes = self.deviceinfo.processes found = set() for p in processes.values(): @@ -461,8 +463,43 @@ def vulnerable_packages(self): self.os_release.get('codename') in DEBIAN_SUITES + UBUNTU_SUITES: return self.deb_packages.filter(vulnerabilities__isnull=False).distinct().order_by('name') - class Meta: - ordering = ('created',) + def generate_recommended_actions(self, classes=None): + """ + Generate RAs for this device and store them as RecommendedAction objects in database. + + If a RecommendedAction object exists and it is in snoozed status: if the RA still affects the device + then it will stay snoozed, otherwise its status changes to NOT_AFFECTED. + + If a RecommendedAction object for some RA does not exist it will be created. + :param classes: if supplied, limit the scope of this method to this list of BaseAction classes. + :return: + """ + ra_all = self.recommendedaction_set.all().values_list('action_id', flat=True) + ra_affected = self.recommendedaction_set.exclude(status=RecommendedAction.Status.NOT_AFFECTED)\ + .values_list('action_id', flat=True) + newly_affected = [] + newly_not_affected = [] + added = [] + for action_class in ActionMeta.all_classes() if classes is None else classes: + is_affected = action_class.is_affected(self) + if action_class.action_id not in ra_all: + # If a RecommendedAction object for some RA does not exist it will be created. + added.append((action_class.action_id, is_affected)) + elif is_affected and action_class.action_id not in ra_affected: + # A RecommendedAction object is not in AFFECTED status, but the RA affects the device + newly_affected.append(action_class.action_id) + elif not is_affected and action_class.action_id in ra_affected: + # A RecommendedAction object is in AFFECTED or SNOOZED_* status, but the RA doesn't affect the device + newly_not_affected.append(action_class.action_id) + n_affected = self.recommendedaction_set.filter(action_id__in=newly_affected)\ + .update(status=RecommendedAction.Status.AFFECTED) + n_unaffected = self.recommendedaction_set.filter(action_id__in=newly_not_affected)\ + .update(status=RecommendedAction.Status.NOT_AFFECTED) + ra_new = [RecommendedAction(action_id=action_id, device=self, + status=RecommendedAction.Status.AFFECTED if affected else + RecommendedAction.Status.NOT_AFFECTED) for action_id, affected in added] + self.recommendedaction_set.bulk_create(ra_new) + return n_affected, n_unaffected, len(ra_new) class DeviceInfo(models.Model): @@ -820,14 +857,53 @@ class Distro(models.Model): class RecommendedAction(models.Model): - class Snooze(IntEnum): - NOT_SNOOZED = 0 - UNTIL_PING = 1 - UNTIL_TIME = 2 - FOREVER = 3 + class Meta: + unique_together = ['device', 'action_id'] + + class Status(IntEnum): + AFFECTED = 0 + SNOOZED_UNTIL_PING = 1 + SNOOZED_UNTIL_TIME = 2 + SNOOZED_FOREVER = 3 + NOT_AFFECTED = 4 + + @staticmethod + def get_affected_query(): + return Q(status=RecommendedAction.Status.AFFECTED) | \ + Q(status=RecommendedAction.Status.SNOOZED_UNTIL_TIME, + snoozed_until__lt=timezone.now()) device = models.ForeignKey(Device, on_delete=models.CASCADE) action_id = models.PositiveSmallIntegerField() - snoozed = models.PositiveSmallIntegerField(choices=[(tag, tag.value) for tag in Snooze], - default=Snooze.NOT_SNOOZED.value) + status = models.PositiveSmallIntegerField(choices=[(tag, tag.value) for tag in Status], + default=Status.AFFECTED.value) snoozed_until = models.DateTimeField(null=True, blank=True) + + @classmethod + def update_all_devices(cls, classes=None): + """ + Generate RAs for all devices which don't have them. Tries to do this in bulk by using .affected_devices() + therefore if it's written properly this method will execute quickly. + It is to be used during migration when a new RA is added. + If classes is supplied the scope of this method will be limited to this list of RA classes. + :param classes: a list of BaseAction child classes. + :return: a number of new RecommendedAction objects created. + """ + created = [] + if classes is None: + classes = ActionMeta.all_classes() + for action_class in classes: + # Select devices which were not yet processed with this RA. + qs = Device.objects.exclude(Q(recommendedaction__action_id=action_class.action_id) | + Q(owner__isnull=True)).only('pk') + if not qs.exists(): + continue + all_devices = set(qs) + affected = set(action_class.affected_devices(qs)) + not_affected = all_devices.difference(affected) + created = [cls(device=d, action_id=action_class.action_id, status=cls.Status.NOT_AFFECTED) + for d in not_affected] + \ + [cls(device=d, action_id=action_class.action_id, status=cls.Status.AFFECTED) + for d in affected] + cls.objects.bulk_create(created) + return len(created) diff --git a/backend/device_registry/recommended_actions.py b/backend/device_registry/recommended_actions.py index a265a2b3e..3880b5c9e 100644 --- a/backend/device_registry/recommended_actions.py +++ b/backend/device_registry/recommended_actions.py @@ -52,6 +52,29 @@ class PubliclyAccessiblePort(NamedTuple): sub_id: int +class Action(NamedTuple): + """ + :param title: Title text + :param description: Body text + :param action_id: the value of action_id field of BaseAction subclasses + :param devices: list of device ids + :param severity: Action severity (Severity) + :param doc_url: "Learn more" URL, optional + :param issue_url: Github issue URL, optional + """ + title: str + description: str + action_id: int + devices: List[int] + severity: Severity + doc_url: str = '/' + issue_url: str = None + + @property + def html(self): + return markdown.markdown(self.description) + + INSECURE_SERVICES = [ InsecureService('fingerd', 1, Severity.MED), InsecureService('tftpd', 2, Severity.MED), @@ -86,33 +109,6 @@ class PubliclyAccessiblePort(NamedTuple): } -class Action: - """ - Action class. - - Its only purpose is to store particular actions info and be passed from a - view to a template. - """ - def __init__(self, title, description, action_id, devices, severity: Severity, doc_url="/", issue_url=None): - """ - - :param title: Title text - :param description: Body text - :param action_id: the value of action_id field of BaseAction subclasses - :param devices: list of device ids - :param severity: one of 'low', 'medium', 'high' - :param doc_url: "Learn more" URL - :param issue_url: Github issue URL, optional - """ - self.title = title - self.description = markdown.markdown(description) - self.action_id = action_id - self.devices = devices - self.issue_url = issue_url - self.doc_url = doc_url - self.severity = severity - - def device_link(device, absolute=False): """Create a device's page html link code""" url = reverse('device-detail', kwargs={'pk': device.pk}) @@ -131,34 +127,35 @@ class BaseAction: doc_url = 'https://wott.io/documentation/faq' @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True) -> QuerySet: + def affected_devices(cls, qs: QuerySet) -> QuerySet: """ - Select user's devices which are affected by this recommended action. May return empty queryset. - If device_pk is given, will select only one device with this pk. - :param user: the owner of the processed devices. - :param device_pk: if given, then only one device with this pk is processed. - :param exclude_snoozed: if True, snoozed actions will be excluded. - :return: + Select all devices which are affected by this recommended action. + This method is to be used during migration when a new RA is added. It is supposed to be optimized for quick + selection of devices, preferably with a single request. However the default implementation provided here is + suboptimal because it calls is_affected() for every device. + :param qs: QuerySet of Device which will be additionally filtered. + :return: QuerySet """ - from .models import RecommendedAction - if exclude_snoozed: - devices = user.devices.exclude(Q(recommendedaction__action_id=cls.action_id) & - (Q(recommendedaction__snoozed__in=[RecommendedAction.Snooze.FOREVER, - RecommendedAction.Snooze.UNTIL_PING]) | - Q(recommendedaction__snoozed=RecommendedAction.Snooze.UNTIL_TIME, - recommendedaction__snoozed_until__gte=timezone.now()))) - else: - devices = user.devices.all() - if device_pk is not None: - devices = devices.filter(pk=device_pk) - return devices + from .models import Device + return Device.objects.filter(pk__in=[ + dev.pk for dev in qs if cls.is_affected(dev) + ]) + + @classmethod + def is_affected(cls, device) -> bool: + """ + Whether the supplied device is affected by this RA. + :param device: a Device + :return: bool + """ + raise NotImplementedError @classmethod - def get_action_description_context(cls, devices_qs, device_pk=None) -> dict: + def get_context(cls, devices, device_pk=None) -> dict: """ Method for producing a tuple of values used (as string formatting parameters) for action description text rendering. - :param devices: queryset for Device model instances affected by the action; + :param devices: list of Device model instances affected by the action; :param device_pk: int/None - single affected device id; :return: iterable (tuple/list); """ @@ -189,43 +186,41 @@ def _create_action(cls, profile, context, devices_list) -> Action: ) @classmethod - def actions(cls, user, device_pk=None) -> List[Action]: + def action(cls, user, devices, device_pk=None) -> Action: """ Generate a list of Action objects for the user. Excludes snoozed actions. :param user: the owner of the processed devices. + :param devices: a list of Device :param device_pk: if given, then only one device with this pk is processed. :return: """ - actions_list = [] - devices = cls.affected_devices(user, device_pk) - if devices.exists(): - context = cls.get_action_description_context(devices_qs=devices, device_pk=device_pk) - context['devices'] = ', '.join([device_link(dev) for dev in devices]) if device_pk is None else 'this node' - actions_list.append(cls._create_action(user.profile, context, list(devices.values_list('pk', flat=True)))) - return actions_list + context = cls.get_context(devices=devices, device_pk=device_pk) + context['devices'] = ', '.join([device_link(dev) for dev in devices]) if device_pk is None else 'this node' + return cls._create_action(user.profile, context, [d.pk for d in devices]) @classmethod - def action_blocks_count(cls, user) -> int: - return int(cls.affected_devices(user).exists()) - - @classmethod - def get_description(cls, user, body=None, **kwargs) -> (str, str): + def get_description(cls, user, body=None, additional_context=None) -> (str, str): """ Generate a Markdown-formatted descriptive text for this recommended action. Mainly used for filing Github issues. Does not exclude snoozed actions. Uses - cls.action_description as a template and formats it using cls.get_action_description_context(). + cls.action_description as a template and formats it using cls.get_context(). :param user: the owner of the processed devices. :param body: if given, will be used as template instead of cls.action_description. - :param kwargs: additional context for formatting the body. + :param additional_context: additional context for formatting the body. :return: (title, text) """ + from .models import RecommendedAction day_ago = timezone.now() - timedelta(hours=24) - affected_devices = cls.affected_devices(user, exclude_snoozed=False).filter(last_ping__gte=day_ago) - if not affected_devices.exists(): + actions = RecommendedAction.objects.filter(device__owner=user, action_id=cls.action_id, + device__last_ping__gte=day_ago)\ + .exclude(status=RecommendedAction.Status.NOT_AFFECTED) + affected_devices = [action.device for action in actions] + if not affected_devices: return affected_list = '\n'.join([f'- {device_link(d, absolute=True)}' for d in affected_devices]) - context = cls.get_action_description_context(affected_devices) - context.update(kwargs) + context = cls.get_context(affected_devices) + if additional_context is not None: + context.update(additional_context) if 'subject' in context: # Workaround for AutoUpdatesAction three-way logic context['subject'] = '' @@ -352,6 +347,10 @@ def is_action_id(meta, id) -> bool: """ return id in meta._action_classes or id in meta._ungrouped_action_classes + @classmethod + def get_class(meta, id): + return meta._action_classes.get(id) or meta._ungrouped_action_classes.get(id) + # Below is the code for real actions classes. # Don't forget to add metaclass=ActionMeta. @@ -365,8 +364,12 @@ class DefaultCredentialsAction(BaseAction, metaclass=ActionMeta): severity = Severity.HI @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True): - return super().affected_devices(user, device_pk, exclude_snoozed).filter(deviceinfo__default_password=True) + def affected_devices(cls, qs): + return qs.filter(deviceinfo__default_password=True) + + @classmethod + def is_affected(cls, device) -> bool: + return hasattr(device, 'deviceinfo') and device.deviceinfo.default_password is True # Firewall disabled action. @@ -378,12 +381,21 @@ class FirewallDisabledAction(BaseAction, metaclass=ActionMeta): severity = Severity.MED @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True): + def affected_devices(cls, qs): from .models import FirewallState, GlobalPolicy - return super().affected_devices(user, device_pk, exclude_snoozed).exclude( + return qs.exclude( (Q(firewallstate__global_policy=None) & Q(firewallstate__policy=FirewallState.POLICY_ENABLED_BLOCK)) | Q(firewallstate__global_policy__policy=GlobalPolicy.POLICY_BLOCK)) + @classmethod + def is_affected(cls, device) -> bool: + from .models import FirewallState, GlobalPolicy + firewallstate = getattr(device, 'firewallstate', None) + return firewallstate is not None and \ + (firewallstate.policy != FirewallState.POLICY_ENABLED_BLOCK \ + if firewallstate.global_policy is None \ + else firewallstate.global_policy.policy != GlobalPolicy.POLICY_BLOCK) + # Vulnerable packages found action. class VulnerablePackagesAction(BaseAction, metaclass=ActionMeta): @@ -399,8 +411,12 @@ class VulnerablePackagesAction(BaseAction, metaclass=ActionMeta): severity = Severity.MED @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True): - return super().affected_devices(user, device_pk, exclude_snoozed).filter(deb_packages__vulnerabilities__isnull=False).distinct() + def affected_devices(cls, qs): + return qs.filter(deb_packages__vulnerabilities__isnull=False).distinct() + + @classmethod + def is_affected(cls, device) -> bool: + return device.deb_packages.filter(vulnerabilities__isnull=False).exists() class InsecureServicesGroupAction(GroupedAction): @@ -416,14 +432,18 @@ class BaseInsecureServicesAction(BaseAction): group_action = InsecureServicesGroupAction @classmethod - def get_action_description_context(cls, devices_qs, device_pk=None): + def get_context(cls, devices, device_pk=None): return {'service': cls.service_name} @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True): - return super().affected_devices(user, device_pk, exclude_snoozed).exclude(deb_packages_hash='').filter( + def affected_devices(cls, qs): + return qs.exclude(deb_packages_hash='').filter( deb_packages__name=cls.service_name).distinct() + @classmethod + def is_affected(cls, device) -> bool: + return device.deb_packages.filter(name=cls.service_name).exists() + for name, sub_id, severity in INSECURE_SERVICES: class ConcreteInsecureServicesAction(BaseInsecureServicesAction, metaclass=ActionMeta): @@ -455,22 +475,16 @@ class BaseOpensshIssueAction(BaseAction): 'the security posture of your node, please consider changing {param_name} to "{safe_value}".' @classmethod - def get_action_description_context(cls, devices_qs, device_pk=None): + def get_context(cls, devices, device_pk=None): safe_value, doc_url, _, _ = SSHD_CONFIG_PARAMS_INFO[cls.sshd_param] return dict(param_name=cls.sshd_param, safe_value=safe_value, doc_url=doc_url) @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True): - from .models import Device - dev_ids = [] - devices = super().affected_devices(user, device_pk, exclude_snoozed).exclude(audit_files__in=('', [])) - for dev in devices: - issues = dev.sshd_issues - if issues and cls.sshd_param in issues: - dev_ids.append(dev.pk) - return Device.objects.filter(pk__in=dev_ids) + def is_affected(cls, device) -> bool: + issues = device.sshd_issues + return cls.sshd_param in issues if issues is not None else False for param_name, param_info in SSHD_CONFIG_PARAMS_INFO.items(): @@ -498,19 +512,19 @@ class AutoUpdatesAction(BaseAction, metaclass=ActionMeta): def get_doc_url(cls, devices): debian_url = 'https://wiki.debian.org/UnattendedUpgrades' ubuntu_url = 'https://help.ubuntu.com/lts/serverguide/automatic-updates.html' - if devices.count() > 1: + if len(devices) > 1: # Provide Debian's link if more than 1 device. return debian_url else: - if devices.first().os_release.get('distro') == 'ubuntu': + if devices[0].os_release.get('distro') == 'ubuntu': return ubuntu_url else: # Everything besides Ubuntu is Debian. return debian_url @classmethod - def get_action_description_context(cls, devices_qs, device_pk=None): + def get_context(cls, devices, device_pk=None): if device_pk is None: - if devices_qs.count() > 1: + if len(devices) > 1: subject, verb = 'your nodes ', 'are' else: subject, verb = 'your node ', 'is' @@ -519,12 +533,16 @@ def get_action_description_context(cls, devices_qs, device_pk=None): return { 'subject': subject, 'verb': verb, - 'doc_url': cls.get_doc_url(devices_qs) + 'doc_url': cls.get_doc_url(devices) } @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True): - return super().affected_devices(user, device_pk, exclude_snoozed).filter(auto_upgrades=False) + def affected_devices(cls, qs): + return qs.filter(auto_upgrades=False) + + @classmethod + def is_affected(cls, device) -> bool: + return device.auto_upgrades is False # FTP listening on port 21 action. @@ -538,13 +556,8 @@ class FtpServerAction(BaseAction, metaclass=ActionMeta): severity = Severity.MED @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True): - from .models import Device - dev_ids = [] - for dev in super().affected_devices(user, device_pk, exclude_snoozed): - if dev.is_ftp_public: - dev_ids.append(dev.pk) - return Device.objects.filter(pk__in=dev_ids) + def is_affected(cls, device) -> bool: + return device.is_ftp_public is True # MySQL root default password action. @@ -562,8 +575,12 @@ class MySQLDefaultRootPasswordAction(BaseAction, metaclass=ActionMeta): severity = Severity.HI @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True): - return super().affected_devices(user, device_pk, exclude_snoozed).filter(mysql_root_access=True) + def affected_devices(cls, qs): + return qs.filter(mysql_root_access=True) + + @classmethod + def is_affected(cls, device) -> bool: + return device.mysql_root_access is True class PubliclyAccessibleServiceGroupAction(GroupedAction): @@ -582,17 +599,13 @@ class BasePubliclyAccessibleServiceAction(BaseAction): severity = Severity.HI @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True): - from .models import Device - dev_ids = [] - for dev in super().affected_devices(user, device_pk, exclude_snoozed): - if cls.service in dev.public_services: - dev_ids.append(dev.pk) - return Device.objects.filter(pk__in=dev_ids) + def get_context(cls, devices, device_pk=None): + return dict(service=cls.service_name, port=cls.port) @classmethod - def get_action_description_context(cls, devices_qs, device_pk=None): - return dict(service=cls.service_name, port=cls.port) + def is_affected(cls, device) -> bool: + services = device.public_services + return cls.service in services if services is not None else False for service, service_info in PUBLIC_SERVICE_PORTS.items(): @@ -612,8 +625,5 @@ class CpuVulnerableAction(BaseAction, metaclass=ActionMeta): severity = Severity.HI @classmethod - def affected_devices(cls, user, device_pk=None, exclude_snoozed=True): - from .models import Device - return Device.objects.filter(pk__in=[ - dev.pk for dev in super().affected_devices(user, device_pk, exclude_snoozed) if dev.cpu_vulnerable - ]) + def is_affected(cls, device) -> bool: + return device.cpu_vulnerable is True diff --git a/backend/device_registry/templates/actions.html b/backend/device_registry/templates/actions.html index 28407700a..99533b3dc 100644 --- a/backend/device_registry/templates/actions.html +++ b/backend/device_registry/templates/actions.html @@ -25,13 +25,15 @@

{% endif %}

-

{{ action.description|safe }}

+

{{ action.html|safe }}

Learn More {% ifnotequal action.action_id 0 %} - {% with action_id=action.action_id action_devices=action.devices %}
+ wott-action + wott-action-id="{{ action.action_id }}" + wott-action-devices="{{ action.devices }}" + wott-action-title="{{ action.title }}"> @@ -40,7 +42,6 @@

7 days

- {% endwith %} {% endifnotequal %} diff --git a/backend/device_registry/tests/test_all.py b/backend/device_registry/tests/test_all.py index cddb560ab..1595080f4 100644 --- a/backend/device_registry/tests/test_all.py +++ b/backend/device_registry/tests/test_all.py @@ -500,6 +500,7 @@ def test_get(self): def test_actions_btn_pos(self): self.client.login(username='test', password='123') + self.device_no_logins.generate_recommended_actions() url = reverse('device-detail', kwargs={'pk': self.device_no_logins.pk}) response = self.client.get(url) self.assertEqual(response.status_code, 200) @@ -881,6 +882,7 @@ def setUp(self): PortScan.objects.create(device=self.device1) FirewallState.objects.create(device=self.device0) FirewallState.objects.create(device=self.device1) + [d.generate_recommended_actions() for d in (self.device0, self.device1)] def test_wizard(self): self.client.login(username='test', password='123') diff --git a/backend/device_registry/tests/test_api.py b/backend/device_registry/tests/test_api.py index ec0e0098e..e3b2e6839 100644 --- a/backend/device_registry/tests/test_api.py +++ b/backend/device_registry/tests/test_api.py @@ -1327,9 +1327,11 @@ def setUp(self): PortScan.objects.create(device=self.device) FirewallState.objects.create(device=self.device) self.client.login(username='test', password='123') + self.device.generate_recommended_actions() def test_snooze_until_ping(self): - self.assertEqual(self.device.recommendedaction_set.filter(action_id=self.action_class.action_id).count(), 0) + self.assertEqual(self.device.recommendedaction_set.get( + action_id=self.action_class.action_id).status, RecommendedAction.Status.AFFECTED) self.assertEqual(self.device.actions_count, 2) # 'duration': None means "snooze until ping" response = self.client.post(self.url, {'device_ids': [self.device.pk], @@ -1339,11 +1341,12 @@ def test_snooze_until_ping(self): self.device.refresh_from_db() actions = self.device.recommendedaction_set.filter(action_id=self.action_class.action_id) self.assertQuerysetEqual(actions.values_list('action_id', flat=True), [str(self.action_class.action_id)]) - self.assertEqual(actions[0].snoozed, RecommendedAction.Snooze.UNTIL_PING) + self.assertEqual(actions[0].status, RecommendedAction.Status.SNOOZED_UNTIL_PING) self.assertEqual(self.device.actions_count, 1) def test_snooze_forever(self): - self.assertFalse(self.device.recommendedaction_set.filter(action_id=self.action_class.action_id).exists()) + self.assertEqual(self.device.recommendedaction_set.get( + action_id=self.action_class.action_id).status, RecommendedAction.Status.AFFECTED) self.assertEqual(self.device.actions_count, 2) # 'duration': 0 means "snooze forever" response = self.client.post(self.url, {'device_ids': [self.device.pk], @@ -1353,11 +1356,13 @@ def test_snooze_forever(self): self.device.refresh_from_db() actions = self.device.recommendedaction_set.filter(action_id=self.action_class.action_id) self.assertQuerysetEqual(actions.values_list('action_id', flat=True), [str(self.action_class.action_id)]) - self.assertEqual(actions[0].snoozed, RecommendedAction.Snooze.FOREVER) + self.assertEqual(self.device.recommendedaction_set.get( + action_id=self.action_class.action_id).status, RecommendedAction.Status.SNOOZED_FOREVER) self.assertEqual(self.device.actions_count, 1) def test_snooze_until_time(self): - self.assertFalse(self.device.recommendedaction_set.filter(action_id=self.action_class.action_id).exists()) + self.assertEquals(self.device.recommendedaction_set.get( + action_id=self.action_class.action_id).status, RecommendedAction.Status.AFFECTED) self.assertEqual(self.device.actions_count, 2) # Snooze for 7 hours @@ -1369,15 +1374,17 @@ def test_snooze_until_time(self): self.device.refresh_from_db() actions = self.device.recommendedaction_set.filter(action_id=self.action_class.action_id) self.assertQuerysetEqual(actions.values_list('action_id', flat=True), [str(self.action_class.action_id)]) - self.assertEqual(actions[0].snoozed, RecommendedAction.Snooze.UNTIL_TIME) + ra = self.device.recommendedaction_set.get(action_id=self.action_class.action_id) + self.assertEqual(ra.status, RecommendedAction.Status.SNOOZED_UNTIL_TIME) # Should be snoozed for at least 6 hours from now. It's actually 7 hours minus a couple seconds. - self.assertGreaterEqual((actions[0].snoozed_until - timezone.now()).total_seconds() // 3600, 6) + self.assertGreaterEqual((ra.snoozed_until - timezone.now()).total_seconds() // 3600, 6) self.assertEqual(self.device.actions_count, 1) def test_wrong_action_id(self): - self.assertFalse(self.device.recommendedaction_set.filter(action_id=self.action_class.action_id).exists()) + self.assertEqual(self.device.recommendedaction_set.get( + action_id=self.action_class.action_id).status, RecommendedAction.Status.AFFECTED) action_id = 9999 self.assertFalse(ActionMeta.is_action_id(action_id)) response = self.client.post(self.url, {'device_ids': [self.device.pk], @@ -1386,27 +1393,29 @@ def test_wrong_action_id(self): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.data, {'action_id': [ErrorDetail(string='Invalid recommended action id', code='invalid')]}) - self.device.refresh_from_db() - self.assertFalse(self.device.recommendedaction_set.filter(action_id=self.action_class.action_id).exists()) + self.assertEqual(self.device.recommendedaction_set.get( + action_id=self.action_class.action_id).status, RecommendedAction.Status.AFFECTED) def test_wrong_device_id(self): - self.assertFalse(self.device.recommendedaction_set.filter(action_id=self.action_class.action_id).exists()) + self.assertEqual(self.device.recommendedaction_set.get( + action_id=self.action_class.action_id).status, RecommendedAction.Status.AFFECTED) response = self.client.post(self.url, {'device_ids': [self.device.pk + 1], 'action_id': self.action_class.action_id, 'duration': None}) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.data, {'device_ids': [ErrorDetail(string='Invalid device id(s) provided', code='invalid')]}) - self.device.refresh_from_db() - self.assertFalse(self.device.recommendedaction_set.filter(action_id=self.action_class.action_id).exists()) + self.assertEqual(self.device.recommendedaction_set.get( + action_id=self.action_class.action_id).status, RecommendedAction.Status.AFFECTED) def test_wrong_duration(self): - self.assertFalse(self.device.recommendedaction_set.filter(action_id=self.action_class.action_id).exists()) + self.assertEqual(self.device.recommendedaction_set.get( + action_id=self.action_class.action_id).status, RecommendedAction.Status.AFFECTED) response = self.client.post(self.url, {'device_ids': [self.device.pk], 'action_id': self.action_class.action_id, 'duration': -1}) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.data, {'duration': [ ErrorDetail(string='Ensure this value is greater than or equal to 0.', code='min_value')]}) - self.device.refresh_from_db() - self.assertFalse(self.device.recommendedaction_set.filter(action_id=self.action_class.action_id).exists()) + self.assertEqual(self.device.recommendedaction_set.get( + action_id=self.action_class.action_id).status, RecommendedAction.Status.AFFECTED) diff --git a/backend/device_registry/tests/test_recommended_actions.py b/backend/device_registry/tests/test_recommended_actions.py index 103eedc0d..49ded157d 100644 --- a/backend/device_registry/tests/test_recommended_actions.py +++ b/backend/device_registry/tests/test_recommended_actions.py @@ -46,9 +46,89 @@ def test_get(self): self.assertContains(response, search_string) +class GenerateActionsTest(TestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + + class TestActionOne(BaseAction, metaclass=ActionMeta): + """ + A simple dummy subclass of BaseAction which always reports devices as affected and has a hopefully unique id. + """ + action_id = 9991 + affected = False + + @classmethod + def is_affected(cls, device) -> bool: + return cls.affected + + class TestActionTwo(BaseAction, metaclass=ActionMeta): + """ + A simple dummy subclass of BaseAction which always reports devices as affected and has a hopefully unique id. + """ + action_id = 9992 + affected = False + + @classmethod + def is_affected(cls, device) -> bool: + return cls.affected + + cls.TestActionOne = TestActionOne + cls.TestActionTwo = TestActionTwo + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + ActionMeta.unregister(cls.TestActionOne) + ActionMeta.unregister(cls.TestActionTwo) + + def setUp(self): + User = get_user_model() + self.user = User.objects.create_user('test') + self.user.set_password('123') + self.user.save() + self.device = Device.objects.create(device_id='device0.d.wott-dev.local', owner=self.user) + + def check_actions_status(self, status_one, status_two, classes=None): + self.device.generate_recommended_actions(classes) + self.assertQuerysetEqual(self.device.recommendedaction_set.filter(action_id__in=[self.TestActionOne.action_id, + self.TestActionTwo.action_id]) + .order_by('action_id') + .values_list('action_id', 'status'), + [(self.TestActionOne.action_id, status_one.value), + (self.TestActionTwo.action_id, status_two.value)], + transform=lambda v: v) + + def test_generate_recommended_actions(self): + self.check_actions_status(RecommendedAction.Status.NOT_AFFECTED, RecommendedAction.Status.NOT_AFFECTED) + + self.TestActionOne.affected = True + self.check_actions_status(RecommendedAction.Status.AFFECTED, RecommendedAction.Status.NOT_AFFECTED) + + self.TestActionTwo.affected = True + self.check_actions_status(RecommendedAction.Status.AFFECTED, RecommendedAction.Status.AFFECTED) + + self.TestActionOne.affected = False + self.check_actions_status(RecommendedAction.Status.NOT_AFFECTED, RecommendedAction.Status.AFFECTED) + + self.TestActionOne.affected = True + self.device.snooze_action(self.TestActionOne.action_id, RecommendedAction.Status.SNOOZED_FOREVER) + self.check_actions_status(RecommendedAction.Status.SNOOZED_FOREVER, RecommendedAction.Status.AFFECTED) + + self.TestActionOne.affected = False + self.check_actions_status(RecommendedAction.Status.NOT_AFFECTED, RecommendedAction.Status.AFFECTED) + + self.TestActionTwo.affected = False + self.check_actions_status(RecommendedAction.Status.NOT_AFFECTED, RecommendedAction.Status.NOT_AFFECTED) + self.TestActionOne.affected = True + self.TestActionTwo.affected = True + self.check_actions_status(RecommendedAction.Status.AFFECTED, RecommendedAction.Status.NOT_AFFECTED, + classes=[self.TestActionOne]) + + class SnoozeTest(TestCase): """ - Test snoozing functionality implemented in Device and RecommendedAction models only. + Test snoozing functionality. setUpClass() and tearDownClass() were overloaded to register and unregister TestAction only once. """ @@ -61,6 +141,13 @@ class TestAction(BaseAction, metaclass=ActionMeta): A simple dummy subclass of BaseAction which always reports devices as affected and has a hopefully unique id. """ action_id = 9999 + action_title = "" + action_description = "" + + @classmethod + def is_affected(cls, device) -> bool: + return True + cls.TestAction = TestAction @classmethod @@ -73,30 +160,34 @@ def setUp(self): self.user = User.objects.create_user('test') self.user.set_password('123') self.user.save() + self.client.login(username='test', password='123') self.device = Device.objects.create(device_id='device0.d.wott-dev.local', owner=self.user) + self.device.generate_recommended_actions() + self.common_actions_url = reverse('actions') + self.snooze_url = reverse('snooze_action') - def _assertHasAction(self, has_action, exclude_snoozed=True): + def _assertHasAction(self, has_action): + self.device.generate_recommended_actions(classes=[self.TestAction]) + response = self.client.get(self.common_actions_url) + self.assertEqual(response.status_code, 200) if has_action: - self.assertQuerysetEqual(self.TestAction.affected_devices(self.user, exclude_snoozed=exclude_snoozed) - .values_list('pk', flat=True), [str(self.device.pk)]) + self.assertEqual(len(response.context['actions']), 1) + self.assertEqual(response.context['actions'][0].action_id, self.TestAction.action_id) else: - self.assertFalse(self.TestAction.affected_devices(self.user, exclude_snoozed=exclude_snoozed).exists()) - - def test_exclude_snoozed(self): - self._assertHasAction(True) - self.device.snooze_action(self.TestAction.action_id, RecommendedAction.Snooze.FOREVER) - self._assertHasAction(True, exclude_snoozed=False) + self.assertEqual(len(response.context['actions']), 0) def test_snooze_forever(self): self._assertHasAction(True) - self.device.snooze_action(self.TestAction.action_id, RecommendedAction.Snooze.FOREVER) + self.snooze_action(0) self._assertHasAction(False) + with freeze_time(timezone.now() + timezone.timedelta(days=10)): + self._assertHasAction(False) def test_snooze_until_ping(self): self._assertHasAction(True) - self.device.snooze_action(self.TestAction.action_id, RecommendedAction.Snooze.UNTIL_PING) + self.snooze_action(None) self._assertHasAction(False) - self.device.snooze_action(self.TestAction.action_id, RecommendedAction.Snooze.NOT_SNOOZED) + self.device.snooze_action(self.TestAction.action_id, RecommendedAction.Status.AFFECTED) self._assertHasAction(True) def test_snooze_interval(self): @@ -104,7 +195,8 @@ def test_snooze_interval(self): with freeze_time(timezone.now() - timezone.timedelta(hours=23)): # 23 hours ago this action had been snoozed for 24 hours - self.device.snooze_action(self.TestAction.action_id, RecommendedAction.Snooze.UNTIL_TIME, 24) + # self.device.snooze_action(self.TestAction.action_id, RecommendedAction.Status.SNOOZED_UNTIL_TIME, 24) + self.snooze_action(24) # ... which means now it's still snoozed self._assertHasAction(False) @@ -113,6 +205,13 @@ def test_snooze_interval(self): with freeze_time(timezone.now() + timezone.timedelta(hours=1)): self._assertHasAction(True) + def snooze_action(self, duration): + response = self.client.post(self.snooze_url, data={'device_ids': [self.device.pk], + 'action_id': self.TestAction.action_id, + 'duration': duration}, + content_type='application/json') + self.assertEqual(response.status_code, 200) + class TestsMixin: """ @@ -122,6 +221,20 @@ class TestsMixin: is that otherwise the Django test runner considers the base test class as a regular test class and run its tests which isn't what we want from it. """ + def setUp(self): + User = get_user_model() + self.user = User.objects.create_user('test') + self.user.set_password('123') + self.user.save() + self.device = Device.objects.create(device_id='device0.d.wott-dev.local', owner=self.user, auto_upgrades=True, + mysql_root_access=False, last_ping=timezone.now()) + FirewallState.objects.create(device=self.device, policy=FirewallState.POLICY_ENABLED_BLOCK) + PortScan.objects.create(device=self.device) + DeviceInfo.objects.create(device=self.device, default_password=False) + self.client.login(username='test', password='123') + self.device_page_url = reverse('device-detail', kwargs={'pk': self.device.pk}) + self.common_actions_url = reverse('actions') + self.device_actions_url = reverse('device_actions', kwargs={'device_pk': self.device.pk}) def assertOneAction(self, url): self.assertEqual(self.device.actions_count, 1) @@ -140,57 +253,36 @@ def test_get(self): search_string_common_page = self.get_search_string() # No action at the beginning. + self.device.generate_recommended_actions() + self.assertFalse(self.action_class.is_affected(self.device)) + self.assertFalse(self.action_class.affected_devices(self.user.devices.all()).exists()) self.assertNoAction(self.common_actions_url) self.assertNoAction(self.device_actions_url) self.assertIsNone(self.action_class.get_description(self.user)) # Enable the action. self.enable_action() + self.device.generate_recommended_actions() + self.assertTrue(self.action_class.is_affected(self.device)) + self.assertTrue(self.action_class.affected_devices(self.user.devices.all()).exists()) self.check_action(self.assertOneAction(self.common_actions_url), search_string_common_page) self.check_action(self.assertOneAction(self.device_actions_url), self.search_pattern_device_page) self.check_description() - # Snooze the action. - self.snooze_action() - self.assertNoAction(self.common_actions_url) - self.assertNoAction(self.device_actions_url) - def check_description(self): - title, text = self.action_class.get_description(self.user, devices='this node') + title, text = self.action_class.get_description(self.user, additional_context=dict(devices='this node')) self.assertIn(self.search_pattern_device_page, text) self.assertIn(reverse('device-detail', kwargs={'pk': self.device.pk}), text) def check_action(self, action: Action, text, title=None): self.assertIn(text, action.description) - def setUp(self): - User = get_user_model() - self.user = User.objects.create_user('test') - self.user.set_password('123') - self.user.save() - self.device = Device.objects.create(device_id='device0.d.wott-dev.local', owner=self.user, auto_upgrades=True, - mysql_root_access=False, last_ping=timezone.now()) - FirewallState.objects.create(device=self.device, policy=FirewallState.POLICY_ENABLED_BLOCK) - PortScan.objects.create(device=self.device) - DeviceInfo.objects.create(device=self.device, default_password=False) - self.client.login(username='test', password='123') - self.device_page_url = reverse('device-detail', kwargs={'pk': self.device.pk}) - self.common_actions_url = reverse('actions') - self.device_actions_url = reverse('device_actions', kwargs={'device_pk': self.device.pk}) - def get_search_string(self): return self.search_pattern_common_page.format(url=self.device_page_url, name=self.device.get_name()) - def snooze_action(self): - self.assertTrue(ActionMeta.is_action_id(self.action_class.action_id)) - self.device.snooze_action(self.action_class.action_id, RecommendedAction.Snooze.FOREVER) - - def unsnooze_action(self): - self.device.snooze_action(self.action_class.action_id, RecommendedAction.Snooze.NOT_SNOOZED) - class DefaultCredentialsActionTest(TestsMixin, TestCase): - search_pattern_common_page = 'We found default credentials present on {name}' + search_pattern_common_page = 'We found default credentials present on [{name}]({url})' search_pattern_device_page = 'We found default credentials present on this node' action_class = DefaultCredentialsAction @@ -200,7 +292,7 @@ def enable_action(self): class FirewallDisabledActionTest(TestsMixin, TestCase): - search_pattern_common_page = 'We found permissive firewall policy present on {name}' + search_pattern_common_page = 'We found permissive firewall policy present on [{name}]({url})' search_pattern_device_page = 'We found permissive firewall policy present on this node' action_class = FirewallDisabledAction @@ -222,7 +314,7 @@ def enable_action(self): class VulnerablePackagesActionTest(TestsMixin, TestCase): - search_pattern_common_page = 'We found vulnerable packages on {name}' + search_pattern_common_page = 'We found vulnerable packages on [{name}]({url})' search_pattern_device_page = 'We found vulnerable packages on this node' action_class = VulnerablePackagesAction @@ -238,7 +330,7 @@ def enable_action(self): class AutoUpdatesActionTest(TestsMixin, TestCase): - search_pattern_common_page = 'We found that your node {name} is not configured to automatically ' \ + search_pattern_common_page = 'We found that your node [{name}]({url}) is not configured to automatically ' \ 'install security updates' search_pattern_device_page = 'We found that this node is not configured to automatically install security updates' action_class = AutoUpdatesAction @@ -250,7 +342,7 @@ def enable_action(self): class MySQLDefaultRootPasswordActionTest(TestsMixin, TestCase): search_pattern_common_page = 'We detected that there is no root password set for MySQL/MariaDB on ' \ - '{name}' + '[{name}]({url})' search_pattern_device_page = 'We detected that there is no root password set for MySQL/MariaDB on this node' action_class = MySQLDefaultRootPasswordAction @@ -278,9 +370,8 @@ def test_get(self): for subclass in InsecureServicesGroupAction.subclasses: self.action_class = subclass self.search_pattern_device_page = 'We found ' + self.action_class.service_name + ' installed on this node' - self.search_pattern_common_page = 'We found ' + self.action_class.service_name + ' installed on {name}' + self.search_pattern_common_page = 'We found ' + self.action_class.service_name + ' installed on [{name}]({url})' super().test_get() - self.unsnooze_action() self.disable_action() def check_description(self): @@ -295,8 +386,10 @@ def test_group(self): source_version='sversion1', arch='amd64', os_release_codename='jessie') self.device.deb_packages.add(deb_package) + self.device.generate_recommended_actions() - title, text = InsecureServicesGroupAction.get_description(self.user, devices='your nodes') + title, text = InsecureServicesGroupAction.get_description(self.user, + additional_context=dict(devices='your nodes')) self.assertEqual(title, 'Insecure services found') self.assertIn('We found insecure services installed on your nodes.', text) for pkg_name in services_installed: @@ -307,7 +400,7 @@ def test_group(self): class OpensshIssueActionTest(TestsMixin, TestCase): - search_pattern_common = 'We found insecure configuration issue with OpenSSH on {name}: ' \ + search_pattern_common = 'We found insecure configuration issue with OpenSSH on [{name}]({url}): ' \ 'insecure parameter ' search_pattern_device = 'We found insecure configuration issue with OpenSSH on this node: ' \ 'insecure parameter ' @@ -322,6 +415,8 @@ def setUp(self): self.device.audit_files = [{'name': '/etc/ssh/sshd_config', 'issues': {}, 'sha256': 'abcd', 'last_modified': 1554718384.0}] + self.device.save() + self.device.generate_recommended_actions() def enable_action(self): self.device.audit_files[0]['issues'] = \ @@ -338,7 +433,6 @@ def test_get(self): self.search_pattern_common_page = self.search_pattern_common + self.action_class.sshd_param self.search_pattern_device_page = self.search_pattern_device + self.action_class.sshd_param super().test_get() - self.unsnooze_action() self.disable_action() def test_group(self): @@ -347,8 +441,10 @@ def test_group(self): del(bad_config[good_config_name]) self.device.audit_files[0]['issues'] = bad_config self.device.save(update_fields=['audit_files']) + self.device.generate_recommended_actions() - title, text = OpensshIssueGroupAction.get_description(self.user, devices='your nodes') + title, text = OpensshIssueGroupAction.get_description(self.user, + additional_context=dict(devices='your nodes')) self.assertIn('We found insecure configuration issues with OpenSSH on your nodes. To improve the ' 'security posture of your node, please consider making the following changes:', text) self.assertEqual(title, 'Insecure configuration for OpenSSH found') @@ -363,7 +459,7 @@ def check_description(self): class FtpServerActionTest(TestsMixin, TestCase): - search_pattern_common_page = 'There appears to be an FTP server running on {name}' + search_pattern_common_page = 'There appears to be an FTP server running on [{name}]({url})' search_pattern_device_page = 'There appears to be an FTP server running on this node' action_class = FtpServerAction @@ -375,13 +471,13 @@ def enable_action(self): class PubliclyAccessibleServiceActionTest(TestsMixin, TestCase): - search_pattern_common = 'We detected that a {service} instance on {{name}} may be ' \ + search_pattern_common = 'We detected that a {service} instance on [{{name}}]({{url}}) may be ' \ 'accessible remotely' search_pattern_device = 'We detected that a {service} instance on this node may be accessible remotely' def enable_action(self): dummy_pid = 34568 - self.device.deviceinfo.processes = {dummy_pid: (self.action_class.service, '', '', None)} + self.device.deviceinfo.processes = {str(dummy_pid): (self.action_class.service, '', '', None)} self.device.deviceinfo.save(update_fields=['processes']) self.device.portscan.scan_info = [ {'ip_version': 4, 'proto': 'tcp', 'state': 'LISTEN', 'host': '0.0.0.0', @@ -404,12 +500,11 @@ def check_description(self): pass def test_get(self): - for subclass in PubliclyAccessibleServiceGroupAction.subclasses: + for subclass in [PubliclyAccessibleServiceGroupAction.subclasses[0]]: self.action_class = subclass self.search_pattern_common_page = self.search_pattern_common.format(service=subclass.service_name) self.search_pattern_device_page = self.search_pattern_device.format(service=subclass.service_name) super().test_get() - self.unsnooze_action() self.disable_action() def test_group(self): @@ -417,7 +512,7 @@ def test_group(self): publicly_available = ['mongod', 'mysqld', 'memcached'] not_publicly_available = 'redis-server' for service in publicly_available: - self.device.deviceinfo.processes[dummy_pid] = (service, '', '', None) + self.device.deviceinfo.processes[str(dummy_pid)] = (service, '', '', None) self.device.portscan.scan_info.append( {'ip_version': 4, 'proto': 'tcp', 'state': 'LISTEN', 'host': '0.0.0.0', 'port': PUBLIC_SERVICE_PORTS[service][0], 'pid': dummy_pid} @@ -425,8 +520,10 @@ def test_group(self): dummy_pid += 1 self.device.deviceinfo.save(update_fields=['processes']) self.device.portscan.save(update_fields=['scan_info']) + self.device.generate_recommended_actions() - title, text = PubliclyAccessibleServiceGroupAction.get_description(self.user, devices='your nodes') + title, text = PubliclyAccessibleServiceGroupAction.get_description(self.user, + additional_context=dict(devices='your nodes')) self.assertEqual(title, 'Your services may be publicly accessible') for service in publicly_available: port, name = PUBLIC_SERVICE_PORTS[service][:2] @@ -444,7 +541,7 @@ def test_group(self): class CpuVulnerableActionTest(TestsMixin, TestCase): - search_pattern_common_page = 'We detected that {name} is vulnerable to Meltdown/Spectre' + search_pattern_common_page = 'We detected that [{name}]({url}) is vulnerable to Meltdown/Spectre' search_pattern_device_page = 'We detected that this node is vulnerable to Meltdown/Spectre' action_class = CpuVulnerableAction diff --git a/backend/device_registry/views.py b/backend/device_registry/views.py index a21c9c497..84da97e34 100644 --- a/backend/device_registry/views.py +++ b/backend/device_registry/views.py @@ -1,5 +1,6 @@ import json import uuid +from collections import defaultdict from django.views.generic import DetailView, ListView, TemplateView, View, UpdateView, CreateView, DeleteView from django.http import HttpResponseRedirect, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden @@ -335,9 +336,7 @@ def post(self, request, *args, **kwargs): out_data.append(ports_form_data[2][port_record_index]) portscan.block_ports = out_data firewallstate.policy = form.cleaned_data['policy'] - # Stop snoozing 'Permissive firewall policy detected' recommended action. - if int(firewallstate.policy) == FirewallState.POLICY_ENABLED_BLOCK: - self.object.snooze_action(FirewallDisabledAction.action_id, RecommendedAction.Snooze.NOT_SNOOZED) + self.object.generate_recommended_actions(classes=[FirewallDisabledAction]) with transaction.atomic(): portscan.save(update_fields=['block_ports']) firewallstate.save(update_fields=['policy']) @@ -356,6 +355,8 @@ def post(self, request, *args, **kwargs): out_data.append(connections_form_data[2][connection_record_index]) portscan.block_networks = out_data portscan.save(update_fields=['block_networks']) + + self.object.generate_recommended_actions(classes=[FirewallDisabledAction]) return HttpResponseRedirect(reverse('device-detail-security', kwargs={'pk': kwargs['pk']})) @@ -488,24 +489,43 @@ def get_context_data(self, **kwargs): if device_pk is not None: dev = get_object_or_404(Device, pk=device_pk, owner=self.request.user) device_name = dev.get_name() + actions_qs = dev.recommendedaction_set.all() else: device_name = None - - for action_class in ActionMeta.all_classes(): - actions.extend(action_class.actions(self.request.user, device_pk)) + actions_qs = RecommendedAction.objects.filter(device__owner=self.request.user).order_by('device__pk') + + # Select all RAs for all user's devices which are not snoozed + active_actions = actions_qs.filter(RecommendedAction.get_affected_query()) + + # Gather a dict of action_id: [device_pk] where an action with action_id affects the list of device_pk's. + actions_by_id = defaultdict(list) + affected_devices = set() + for ra in active_actions: + affected_devices.add(ra.device.pk) + actions_by_id[ra.action_id].append(ra.device.pk) + affected_devices = {d.pk: d for d in Device.objects.filter(pk__in=affected_devices)} + + # Generate Action objects to be rendered on page for every affected RA. + for ra_id, device_pks in actions_by_id.items(): + devices = [affected_devices[d] for d in device_pks] + a = ActionMeta.get_class(ra_id).action(self.request.user, devices, device_pk) + actions.append(a) else: # User has no devices - display the special action. device_name = None - action = Action( + actions = [Action( 'Enroll your node(s) to unlock this feature', 'In order to receive recommended actions, click "Add Node" under "Dashboard" to receive instructions ' 'on how to enroll your nodes.', action_id=0, devices=[], severity=Severity.LO - ) - actions.append(action) + )] context = super().get_context_data(**kwargs) + + # Sort actions by severity and then by action id, effectively grouping subclasses together. + actions.sort(key=lambda a: (a.severity.value, a.action_id)) + context['actions'] = actions context['device_name'] = device_name return context diff --git a/backend/profile_page/models.py b/backend/profile_page/models.py index 91c8c218f..fd686d210 100644 --- a/backend/profile_page/models.py +++ b/backend/profile_page/models.py @@ -4,13 +4,14 @@ from django.contrib.auth.models import User from django.contrib.postgres.fields import JSONField from django.db import models +from django.db.models import Q from django.db.models.signals import pre_save from django.dispatch import receiver from mixpanel import Mixpanel, MixpanelException from phonenumber_field.modelfields import PhoneNumberField -from device_registry.recommended_actions import ActionMeta +from device_registry.models import RecommendedAction from device_registry.celery_tasks import github logger = logging.getLogger(__name__) @@ -47,7 +48,9 @@ class Profile(models.Model): @property def actions_count(self): - return sum([action_class.action_blocks_count(self.user) for action_class in ActionMeta.all_classes()]) + return RecommendedAction.objects.filter( + Q(device__owner=self.user) & RecommendedAction.get_affected_query()) \ + .values('action_id').distinct().count() @property def github_repos(self):