From 4af77bba596e6bebea8405a5eb0c43145fb84791 Mon Sep 17 00:00:00 2001 From: Johanna England Date: Fri, 17 Nov 2023 12:01:08 +0100 Subject: [PATCH 1/4] Fix form validation with equal and in operator --- python/nav/web/alertprofiles/forms.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/python/nav/web/alertprofiles/forms.py b/python/nav/web/alertprofiles/forms.py index 2c87269743..bda5e28e61 100644 --- a/python/nav/web/alertprofiles/forms.py +++ b/python/nav/web/alertprofiles/forms.py @@ -663,16 +663,12 @@ def clean(self) -> Dict[str, Any]: ) else: validated_ip_addresses.append(str(ip)) - # Bring ip address back into original format to be processed below - value = " ".join(validated_ip_addresses) + validated_data["value"] = "|".join(validated_ip_addresses) + return validated_data if operator_type == Operator.IN: - """If input was a multiple choice list we have to join each option in one - string, where each option is separated by a | (pipe). - If input was a IP adress we should replace space with | (pipe).""" - if match_field.data_type == MatchField.IP: - validated_data["value"] = value.replace(' ', '|') - else: - validated_data["value"] = "|".join(value) + validated_data["value"] = "|".join(value) + elif operator_type == Operator.EQUALS: + validated_data["value"] = value[0] return validated_data From f33204cfa4982ecc6f17d426e9f35d13d07a33d2 Mon Sep 17 00:00:00 2001 From: Johanna England Date: Fri, 17 Nov 2023 12:01:20 +0100 Subject: [PATCH 2/4] Add more tests for expression validation --- tests/integration/web/alertprofiles_test.py | 24 +++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/integration/web/alertprofiles_test.py b/tests/integration/web/alertprofiles_test.py index 718475258e..864e48d21c 100644 --- a/tests/integration/web/alertprofiles_test.py +++ b/tests/integration/web/alertprofiles_test.py @@ -429,6 +429,30 @@ def test_alertprofiles_add_expression_with_multiple_alert_types_should_succeed( response.content ) + def test_alertprofiles_add_expression_with_valid_group_should_succeed( + self, client, dummy_filter + ): + """Tests that an expression with a valid group can be added""" + group_match_field = MatchField.objects.get(name="Group") + url = reverse("alertprofiles-filters-saveexpression") + data = { + "filter": dummy_filter.pk, + "match_field": group_match_field.pk, + "operator": Operator.EQUALS, + "value": "AD", + } + response = client.post(url, data=data, follow=True) + assert response.status_code == 200 + assert Expression.objects.filter( + filter=dummy_filter, + match_field=group_match_field, + operator=Operator.EQUALS, + value=data["value"], + ).exists() + assert f"Added expression to filter {dummy_filter}" in smart_str( + response.content + ) + class TestsPermissions: def test_set_accountgroup_permissions_should_not_crash(self, db, client): From e045c428c43a7bb97c8332b1f4241e03f8b63673 Mon Sep 17 00:00:00 2001 From: Johanna England Date: Fri, 17 Nov 2023 13:34:34 +0100 Subject: [PATCH 3/4] Break out ip validation in seperate function --- python/nav/web/alertprofiles/forms.py | 34 ++++++++++++++++----------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/python/nav/web/alertprofiles/forms.py b/python/nav/web/alertprofiles/forms.py index bda5e28e61..1783ecf59d 100644 --- a/python/nav/web/alertprofiles/forms.py +++ b/python/nav/web/alertprofiles/forms.py @@ -650,20 +650,9 @@ def clean(self) -> Dict[str, Any]: value = validated_data["value"] if match_field.data_type == MatchField.IP: - if operator_type == Operator.IN: - ip_list = value.split() - else: - ip_list = [value] - validated_ip_addresses = [] - for ip in ip_list: - if not is_valid_ip(ip=ip, strict=True) and not is_valid_cidr(cidr=ip): - self.add_error( - field="value", - error=forms.ValidationError(("Invalid IP address: %s" % ip)), - ) - else: - validated_ip_addresses.append(str(ip)) - validated_data["value"] = "|".join(validated_ip_addresses) + validated_data["value"] = self._clean_ip_addresses( + operator_type=operator_type, value=value + ) return validated_data if operator_type == Operator.IN: @@ -672,3 +661,20 @@ def clean(self) -> Dict[str, Any]: validated_data["value"] = value[0] return validated_data + + def _clean_ip_addresses(self, operator_type, value): + if operator_type == Operator.IN: + ip_list = value.split() + else: + ip_list = [value] + validated_ip_addresses = [] + for ip in ip_list: + if not is_valid_ip(ip=ip, strict=True) and not is_valid_cidr(cidr=ip): + self.add_error( + field="value", + error=forms.ValidationError(("Invalid IP address: %s" % ip)), + ) + else: + validated_ip_addresses.append(str(ip)) + + return "|".join(validated_ip_addresses) From 4a61445005e92a730969d082aee8694683d5c33c Mon Sep 17 00:00:00 2001 From: Johanna England Date: Fri, 17 Nov 2023 13:34:55 +0100 Subject: [PATCH 4/4] Rename expression tests to focus on operator --- tests/integration/web/alertprofiles_test.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/integration/web/alertprofiles_test.py b/tests/integration/web/alertprofiles_test.py index 864e48d21c..f1e3eec3a4 100644 --- a/tests/integration/web/alertprofiles_test.py +++ b/tests/integration/web/alertprofiles_test.py @@ -405,10 +405,12 @@ def test_alertprofiles_add_expression_with_multiple_non_valid_ip_addresses_shoul ).exists() assert f"Invalid IP address: {invalid_ip}" in smart_str(response.content) - def test_alertprofiles_add_expression_with_multiple_alert_types_should_succeed( + def test_alertprofiles_add_expression_with_in_condition_should_succeed( self, client, dummy_filter ): - """Tests that an expression with multiple alert types can be added""" + """Tests that an expression with an in condition can be added, alert type is + just an example + """ string_match_field = MatchField.objects.get(name="Alert type") url = reverse("alertprofiles-filters-saveexpression") data = { @@ -429,10 +431,12 @@ def test_alertprofiles_add_expression_with_multiple_alert_types_should_succeed( response.content ) - def test_alertprofiles_add_expression_with_valid_group_should_succeed( + def test_alertprofiles_add_expression_with_equals_condition_should_succeed( self, client, dummy_filter ): - """Tests that an expression with a valid group can be added""" + """Tests that an expression with an equals condition can be added, group is + just an example + """ group_match_field = MatchField.objects.get(name="Group") url = reverse("alertprofiles-filters-saveexpression") data = {