Skip to content
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

Move the rest_filters reserved names to a setting #384

Merged
merged 1 commit into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions ansible_base/lib/dynamic_config/dynamic_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,21 @@

if 'ansible_base.rest_filters' in INSTALLED_APPS:
REST_FRAMEWORK.update({'DEFAULT_FILTER_BACKENDS': ANSIBLE_BASE_ALL_REST_FILTERS})

ANSIBLE_BASE_REST_FILTERS_RESERVED_NAMES = (
'page',
'page_size',
'format',
'order',
'order_by',
'search',
'type',
'host_filter',
'count_disabled',
'no_truncate',
'limit',
'validate',
)
else:
# Explanation - these are the filters for views provided by DAB like /authenticators/
# we want them to be enabled by default _even if_ the rest_filters app is not used
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ class FieldLookupBackend(BaseFilterBackend):
Filter using field lookups provided via query string parameters.
"""

RESERVED_NAMES = ('page', 'page_size', 'format', 'order', 'order_by', 'search', 'type', 'host_filter', 'count_disabled', 'no_truncate', 'limit', 'validate')
AlanCoding marked this conversation as resolved.
Show resolved Hide resolved

SUPPORTED_LOOKUPS = (
'exact',
'iexact',
Expand Down Expand Up @@ -143,6 +141,17 @@ def value_to_python(self, model, lookup, value):
value = self.value_to_python_for_field(field, value)
return value, new_lookup, needs_distinct

def reserved_names(self, view):
"""The names in query_params to ignore given the current settings and current view"""
from django.conf import settings

reserved_set = set(settings.ANSIBLE_BASE_REST_FILTERS_RESERVED_NAMES)
AlanCoding marked this conversation as resolved.
Show resolved Hide resolved

if hasattr(view, 'rest_filters_reserved_names'):
reserved_set |= set(view.rest_filters_reserved_names)

return reserved_set
AlanCoding marked this conversation as resolved.
Show resolved Hide resolved

def filter_queryset(self, request, queryset, view):
try:
# Apply filters specified via query_params. Each entry in the lists
Expand All @@ -158,7 +167,7 @@ def filter_queryset(self, request, queryset, view):
# If 'OR' is used, an item just needs to satisfy one condition to appear in results.
search_filter_relation = 'OR'
for key, values in request.query_params.lists():
if key in self.RESERVED_NAMES:
if key in self.reserved_names(view):
continue

# HACK: make `created` available via API for the Django User ORM model
Expand Down
28 changes: 28 additions & 0 deletions docs/apps/rest_filters.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,34 @@ REST_FRAMEWORK = {
}
```

## Letting Extra Query Params Through

Sometimes you may have a view that needs to use a query param for a reason unrelated to filtering.
If the rest_filters filtering is enabled, then this will not work, resulting in a 400 response code
due to the model not having an expected field.

To deal with this, after including the dynamic settings, you can add your field to the "reserved" list:

```python
from ansible_base.lib import dynamic_config
dab_settings = os.path.join(os.path.dirname(dynamic_config.__file__), 'dynamic_settings.py')
include(dab_settings)

ANSIBLE_BASE_REST_FILTERS_RESERVED_NAMES += ('extra_querystring',)
```

This will prevent 400 errors for requests like `/api/v1/organizations/?extra_querystring=foo`.
No filtering would be done in this case, the query string would simply be ignored.

If you want to do this on a view level, not for the whole app, then add `rest_filters_reserved_names` to the view.

```python
class CowViewSet(ModelViewSet, AnsibleBaseView):
serializer_class = MySerializer
queryset = MyModel.objects.all()
rest_filters_reserved_names = ('extra_querystring',)
```

## Preventing Field Searching

### prevent_search function
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from unittest.mock import MagicMock, Mock

import pytest
from django.conf import settings
from django.core.exceptions import FieldDoesNotExist, FieldError, ValidationError
from django.test.utils import override_settings
from django.urls import reverse
from django.utils.http import urlencode
from rest_framework.exceptions import ParseError, PermissionDenied
Expand Down Expand Up @@ -173,3 +175,28 @@ def test_filter_jsonfield_as_text(admin_api_client):
response = admin_api_client.get(url + '?' + urlencode(query_params))
assert response.status_code == 200
assert response.data['count'] == 1


def test_filter_unexpected_field(admin_api_client):
url = reverse('organization-list')
response = admin_api_client.get(url, data={'foofield': 'bar'})
assert response.status_code == 400, response.data
AlanCoding marked this conversation as resolved.
Show resolved Hide resolved


def test_app_ignore_field(admin_api_client):
base_list = tuple(settings.ANSIBLE_BASE_REST_FILTERS_RESERVED_NAMES)
with override_settings(ANSIBLE_BASE_REST_FILTERS_RESERVED_NAMES=base_list + ('foofield',)):
url = reverse('organization-list')
response = admin_api_client.get(url, data={'foofield': 'bar'})
assert response.status_code == 200, response.data
AlanCoding marked this conversation as resolved.
Show resolved Hide resolved


def test_view_level_ignore_field(admin_api_client):
"""See CowViewSet definition which corresponds to expectations of this test"""
url = reverse('cow-list')
response = admin_api_client.get(url, data={'cud': 'chew'})
assert response.status_code == 200, response.data

# Make sure that normal function is not disrupted by this customization
response = admin_api_client.get(url, data={'foofield': 'bar'})
assert response.status_code == 400, response.data
3 changes: 3 additions & 0 deletions test_app/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ class CowViewSet(TestAppViewSet):
serializer_class = serializers.CowSerializer
queryset = models.Cow.objects.all()
rbac_action = None
# Reserved names corresponds to
# test_app/tests/rest_filters/rest_framework/test_field_lookup_backend.py::test_view_level_ignore_field
rest_filters_reserved_names = ['cud']

@action(detail=True, rbac_action='say', methods=['post'])
def cowsay(self, request, pk=None):
Expand Down
Loading