Validate fields and their lookups during filtering.

  - Adds a mixin that validates both that the field has been defined, and that the optional lookup requested for the field is enabled for the field.
  - This implies that any filtering field and their lookups *must* be explicitly defined.
  - This further implies that special fields for pagination, ordering, or any other such usage must be defined as well, this is done in the mixin class' approved_base_fields list.
  - We also allow filtering on relation-based fields, but we do not validate these at all.
  - Also removes BaseCVSFilter from JSONFieldExactFilter and CIDRFieldExactFilter to prevent list context from breaking filters.

mreg/api/v1/filters.py

@@ -1,4 +1,6 @@
+from django.db.models import ForeignKey, ManyToManyField, OneToOneField
 from django_filters import rest_framework as filters
+from rest_framework.exceptions import ValidationError
 
 from mreg.models import (
     BACnetID,
@@ -29,8 +31,6 @@
 
 _key_lookups = ["exact"] 
 _boolean_lookups = _key_lookups
-_many_to_many_lookups = _key_lookups
-_many_to_one_lookups = _key_lookups
 
 _textual_lookups = [
     "contains",
@@ -65,7 +65,6 @@
 _zone_fields = {
     "updated": _boolean_lookups,
     "primary_ns": _textual_lookups,
-    "nameservers": _many_to_many_lookups,
     "email": _textual_lookups,
     "serialno": _numeric_lookups,
     "serialno_updated_at": _date_lookups,
@@ -77,35 +76,82 @@
 }
 _zone_fields.update(_mreg_fields)
 
-class JSONFieldExactFilter(filters.BaseCSVFilter, filters.CharFilter):
+class RaiseBadRequestOnBadFilterMixin(filters.FilterSet):
+    """Base filter for all Mreg Filters.
+    
+    This filter mixin checks if the fields and lookups are valid in a filter.
+    If not, it raises a 400 Bad Request error.
+
+    Note that this means that _all_ filter fields and their operators *must* be
+    explicitly defined in the `Meta.fields` attribute of the filter.
+    """
+
+    approved_base_fields = [
+        # Pagination related fields
+        "page_size",
+        "page",
+        "limit",
+        # Ordering related fields
+        "ordering",
+
+        # Mreg-special fields
+        "network", 
+        "data",
+    ]
+
+    def filter_queryset(self, queryset):
+        """Check if the fields and potential lookups are valid."""
+        # These are the fields that are always allowed
+
+        # Get the list of model's field names, only ForeignKey, ManyToManyField, and OneToOneField
+        relational_fields = [f.name for f in self.Meta.model._meta.get_fields()
+                            if isinstance(f, (ForeignKey, ManyToManyField, OneToOneField))]
+
+        if not self.request:
+            return super().filter_queryset(queryset)
+
+        for field in self.request.GET:
+            if "__" in field:
+                field_name, lookup = field.split("__", 1)
+                if field_name not in self.Meta.fields and field_name not in relational_fields:
+                    raise ValidationError(f"Invalid field for filtering: {field}")
+
+                if field_name in self.Meta.fields and lookup not in self.Meta.fields[field_name]:
+                    raise ValidationError(f"Invalid lookup ({lookup} for field ({field_name})")
+            else:                
+                if field in self.approved_base_fields:
+                    continue
+
+                if field not in self.Meta.fields and field not in relational_fields:
+                    raise ValidationError(f"Invalid field for filtering: {field}")
+
+        return super().filter_queryset(queryset)
+
+class JSONFieldExactFilter(filters.CharFilter):
     pass
 
 
-class CIDRFieldExactFilter(filters.BaseCSVFilter, filters.CharFilter):
+class CIDRFieldExactFilter(filters.CharFilter):
     pass
 
 
-class BACnetIDFilterSet(filters.FilterSet):
+class BACnetIDFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = BACnetID
-        fields = {
-            "id": _key_lookups,
-            "host": _key_lookups,
-        }
+        fields = '__all__'  # all our fields are relations.
 
 
-class CnameFilterSet(filters.FilterSet):
+class CnameFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Cname
         fields = {
             "name": _textual_lookups,
-            "host": _key_lookups,
             "ttl": _numeric_lookups,
         }
         fields.update(_mreg_fields)
 
 
-class ForwardZoneFilterSet(filters.FilterSet):
+class ForwardZoneFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = ForwardZone
         fields = {
@@ -114,30 +160,27 @@ class Meta:
         fields.update(_zone_fields)
 
 
-class ForwardZoneDelegationFilterSet(filters.FilterSet):
+class ForwardZoneDelegationFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = ForwardZoneDelegation
         fields = {
-            "zone": _key_lookups,
             "name": _textual_lookups,
-            "nameservers": _many_to_many_lookups,
             "comment": _textual_lookups,
         }
         fields.update(_mreg_fields)
 
 
-class HinfoFilterSet(filters.FilterSet):
+class HinfoFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Hinfo
         fields = {
-            "host": _key_lookups,
             "cpu": _textual_lookups,
             "os": _textual_lookups,
         }
         fields.update(_mreg_fields)
 
 
-class HistoryFilterSet(filters.FilterSet):
+class HistoryFilterSet(RaiseBadRequestOnBadFilterMixin):
     data = JSONFieldExactFilter(field_name="data")
 
     class Meta:
@@ -153,7 +196,7 @@ class Meta:
         }
 
 
-class HostFilterSet(filters.FilterSet):
+class HostFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Host
         fields = {
@@ -164,33 +207,29 @@ class Meta:
         }
         fields.update(_mreg_fields)
 
-class HostGroupFilterSet(filters.FilterSet):
+class HostGroupFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = HostGroup
         fields = {
             "name": _textual_lookups,
             "description": _textual_lookups,
-            "owners": _many_to_many_lookups,
-            "parent": _many_to_one_lookups,
-            "hosts": _many_to_many_lookups,
         }
         fields.update(_mreg_fields)
 
 
 # It would be very nice with a filter type for IP addresses that
 # understands CIDR notation and can do range lookups.
-class IpaddressFilterSet(filters.FilterSet):
+class IpaddressFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Ipaddress
         fields = {
-            "host": _key_lookups,
             "ipaddress": _textual_lookups,
             "macaddress": _textual_lookups,
         }
         fields.update(_mreg_fields)
 
 
-class LabelFilterSet(filters.FilterSet):
+class LabelFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Label
         fields = {
@@ -200,28 +239,26 @@ class Meta:
         fields.update(_mreg_fields)
 
 
-class LocFilterSet(filters.FilterSet):
+class LocFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Loc
         fields = {
-            "host": _key_lookups,
             "loc": _textual_lookups,
         }
         fields.update(_mreg_fields)
 
 
-class MxFilterSet(filters.FilterSet):
+class MxFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Mx
         fields = {
-            "host": _key_lookups,
             "priority": _numeric_lookups,
             "mx": _textual_lookups,
         }
         fields.update(_mreg_fields)
 
 
-class NameServerFilterSet(filters.FilterSet):
+class NameServerFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = NameServer
         fields = {
@@ -231,11 +268,10 @@ class Meta:
         fields.update(_mreg_fields)
 
 
-class NaptrFilterSet(filters.FilterSet):
+class NaptrFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Naptr
         fields = {
-            "host": _key_lookups,
             "preference": _numeric_lookups,
             "order": _numeric_lookups,
             "flag": _textual_lookups,
@@ -245,20 +281,19 @@ class Meta:
         }
         fields.update(_mreg_fields)
 
-class NetGroupRegexPermissionFilterSet(filters.FilterSet):
+class NetGroupRegexPermissionFilterSet(RaiseBadRequestOnBadFilterMixin):
     range = CIDRFieldExactFilter(field_name="range")
 
     class Meta:
         model = NetGroupRegexPermission
         fields = {
             "group": _textual_lookups,
             "regex": _textual_lookups,
-            "labels": _many_to_many_lookups,
         }
         fields.update(_mreg_fields)
 
 
-class NetworkFilterSet(filters.FilterSet):
+class NetworkFilterSet(RaiseBadRequestOnBadFilterMixin):
     network = CIDRFieldExactFilter(field_name="network")
 
     class Meta:
@@ -275,28 +310,26 @@ class Meta:
         fields.update(_mreg_fields)
 
 
-class NetworkExcludedRangeFilterSet(filters.FilterSet):
+class NetworkExcludedRangeFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = NetworkExcludedRange
         fields = {
-            "network": _key_lookups,
             "start_ip": _textual_lookups,
             "end_ip": _textual_lookups,
         }
         fields.update(_mreg_fields)
 
 
-class PtrOverrideFilterSet(filters.FilterSet):
+class PtrOverrideFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = PtrOverride
         fields = {
-            "host": _key_lookups,
             "ipaddress": _textual_lookups,
         }
         fields.update(_mreg_fields)
 
 
-class ReverseZoneFilterSet(filters.FilterSet):
+class ReverseZoneFilterSet(RaiseBadRequestOnBadFilterMixin):
     network = CIDRFieldExactFilter(field_name="network")
 
     class Meta:
@@ -306,20 +339,18 @@ class Meta:
         }
         fields.update(_zone_fields)
 
-class ReverseZoneDelegationFilterSet(filters.FilterSet):
+class ReverseZoneDelegationFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = ReverseZoneDelegation
         fields = {
-            "zone": _key_lookups,
             "name": _textual_lookups,
-            "nameservers": _many_to_many_lookups,
             "comment": _textual_lookups,
         }
         fields.update(_mreg_fields)
 
 
 
-class SrvFilterSet(filters.FilterSet):
+class SrvFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Srv
         fields = {
@@ -328,28 +359,25 @@ class Meta:
             "weight": _numeric_lookups,
             "port": _numeric_lookups,
             "ttl": _numeric_lookups,
-            "host": _key_lookups,
         }
         fields.update(_mreg_fields) 
 
 
-class SshfpFilterSet(filters.FilterSet):
+class SshfpFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Sshfp
         fields = {
-            "host": _key_lookups,
             "ttl": _numeric_lookups,
             "algorithm": _numeric_lookups,
             "hash_type": _numeric_lookups,
             "fingerprint": _textual_lookups,
         }
         fields.update(_mreg_fields)
 
-class TxtFilterSet(filters.FilterSet):
+class TxtFilterSet(RaiseBadRequestOnBadFilterMixin):
     class Meta:
         model = Txt
         fields = {
-            "host": _key_lookups,
             "txt": _textual_lookups,
         }
         fields.update(_mreg_fields)

mreg/api/v1/tests/test_hostgroups.py

@@ -70,6 +70,14 @@ def test_hostgroups_list_200_ok(self):
         ret = self.assert_get('/hostgroups/?name__contains=NOTFOUND')
         self.assertEqual(ret.json()['count'], 0)
 
+    def test_hostgroups_list_400_bad_request(self):
+        """Filtering on an invalid field or using a mismatched lookup returns 400."""
+        self.assert_get_and_400('/hostgroups/?nosuchfield=foo')
+        self.assert_get_and_400('/hostgroups/?nosuchfield__withlookup=foo')
+        self.assert_get_and_400('/hostgroups/?name__nosuchlookup=foo')
+        # name__gt is not supported, it's a numeric lookup on a textual field
+        self.assert_get_and_400('/hostgroups/?name__gt=foo')
+
     def test_hostgroups_get_404_not_found(self):
         """"Getting a non-existing entry should return 404"""
         self.assert_get_and_404('/hostgroups/nonexisting-group')

mreg/api/v1/tests/test_networks.py

@@ -153,6 +153,7 @@ def test_ipv6_networks_post_409_overlap_conflict(self):
     def test_networks_get_200_ok(self):
         """GET on an existing ip-network should return 200 OK."""
         self.assert_get('/networks/%s' % self.network_sample.network)
+        self.assert_get('/networks/?network=%s' % self.network_sample.network)
 
     def test_ipv6_networks_get_200_ok(self):
         """GET on an existing ipv6-network should return 200 OK."""

mreg/api/v1/views.py

@@ -48,7 +48,6 @@ class MregMixin:
     filter_backends = (filters.SearchFilter, filters.OrderingFilter, rf_filters.DjangoFilterBackend,)
     ordering_fields = '__all__'
 
-
 class HostLogMixin(HistoryLog):
 
     log_resource = 'host'